Messages in this thread | | | Date | Fri, 9 Mar 2018 08:45:24 -0700 | From | Lina Iyer <> | Subject | Re: [PATCH v3 05/10] drivers: qcom: rpmh-rsc: write sleep/wake requests to TCS |
| |
On Thu, Mar 08 2018 at 16:58 -0700, Lina Iyer wrote: >On Thu, Mar 08 2018 at 12:41 -0700, Stephen Boyd wrote: >>Quoting Lina Iyer (2018-03-02 08:43:12)
>>>+static int find_slots(struct tcs_group *tcs, struct tcs_request *msg, >>>+ int *m, int *n) >>>+{ >>>+ int slot, offset; >>>+ int i = 0; >>>+ >>>+ /* Find if we already have the msg in our TCS */ >>>+ slot = find_match(tcs, msg->payload, msg->num_payload); >>>+ if (slot >= 0) >>>+ goto copy_data; >> >>Shouldn't this goto skip setting the bits in tcs->slots? >> >No, we overwrite what we found with this new data. >>>+ >>>+ /* Do over, until we can fit the full payload in a TCS */ >>>+ do { >>>+ slot = bitmap_find_next_zero_area(tcs->slots, MAX_TCS_SLOTS, >>>+ i, msg->num_payload, 0); >>>+ if (slot == MAX_TCS_SLOTS) >>>+ break; >>>+ i += tcs->ncpt; >>>+ } while (slot + msg->num_payload - 1 >= i); >>>+ >>>+ if (slot == MAX_TCS_SLOTS) >>>+ return -ENOMEM; >> >>Would be nice to remove this duplicate condition somehow. Maybe a goto? >> >I would return instead of the break earlier instead of this here. >>>+ >>>+copy_data: >>>+ bitmap_set(tcs->slots, slot, msg->num_payload); >>>+ /* Copy the addresses of the resources over to the slots */ >>>+ if (tcs->cmd_addr) { >> >>find_match() above didn't check for tcs->cmd_addr. Does this ever happen >>to fail? >> >Not allocated for active TCSes. I should be checking for it there as >well. Not sure how I didnt see a failure. > Ah, this function is never called for active tcs which would have the tcs->cmd_addr to be NULL. I dont need this check.
-- Lina
| |