| Message ID | 20200521060425.24285-2-swboyd@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Even moar rpmh cleanups | expand |
Hi, Reviewed-by: Maulik Shah <mkshah@codeaurora.org> Thanks, Maulik On 5/21/2020 11:34 AM, Stephen Boyd wrote: > This API does very little. Let's replace all the callsites with the > normal operations that would be done on top of the bitmap that > tcs_in_use is. This simplifies and reduces the code size. > > Cc: Maulik Shah <mkshah@codeaurora.org> > Cc: Douglas Anderson <dianders@chromium.org> > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > --- > drivers/soc/qcom/rpmh-rsc.c | 59 +++++++++++++------------------------ > 1 file changed, 20 insertions(+), 39 deletions(-) > > diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c > index 076fd27f3081..60fc56987659 100644 > --- a/drivers/soc/qcom/rpmh-rsc.c > +++ b/drivers/soc/qcom/rpmh-rsc.c > @@ -184,22 +184,6 @@ static void write_tcs_reg_sync(const struct rsc_drv *drv, int reg, int tcs_id, > data, tcs_id, reg); > } > > -/** > - * tcs_is_free() - Return if a TCS is totally free. > - * @drv: The RSC controller. > - * @tcs_id: The global ID of this TCS. > - * > - * Returns true if nobody has claimed this TCS (by setting tcs_in_use). > - * > - * Context: Must be called with the drv->lock held. > - * > - * Return: true if the given TCS is free. > - */ > -static bool tcs_is_free(struct rsc_drv *drv, int tcs_id) > -{ > - return !test_bit(tcs_id, drv->tcs_in_use); > -} > - > /** > * tcs_invalidate() - Invalidate all TCSes of the given type (sleep or wake). > * @drv: The RSC controller. > @@ -512,7 +496,7 @@ static void __tcs_buffer_write(struct rsc_drv *drv, int tcs_id, int cmd_id, > * > * Return: 0 if nothing in flight or -EBUSY if we should try again later. > * The caller must re-enable interrupts between tries since that's > - * the only way tcs_is_free() will ever return true and the only way > + * the only way tcs_in_use will ever be updated and the only way > * RSC_DRV_CMD_ENABLE will ever be cleared. > */ > static int check_for_req_inflight(struct rsc_drv *drv, struct tcs_group *tcs, > @@ -520,17 +504,14 @@ static int check_for_req_inflight(struct rsc_drv *drv, struct tcs_group *tcs, > { > unsigned long curr_enabled; > u32 addr; > - int i, j, k; > - int tcs_id = tcs->offset; > + int j, k; > + int i = tcs->offset; > > - for (i = 0; i < tcs->num_tcs; i++, tcs_id++) { > - if (tcs_is_free(drv, tcs_id)) > - continue; > - > - curr_enabled = read_tcs_reg(drv, RSC_DRV_CMD_ENABLE, tcs_id); > + for_each_set_bit_from(i, drv->tcs_in_use, tcs->offset + tcs->num_tcs) { > + curr_enabled = read_tcs_reg(drv, RSC_DRV_CMD_ENABLE, i); > > for_each_set_bit(j, &curr_enabled, MAX_CMDS_PER_TCS) { > - addr = read_tcs_cmd(drv, RSC_DRV_CMD_ADDR, tcs_id, j); > + addr = read_tcs_cmd(drv, RSC_DRV_CMD_ADDR, i, j); > for (k = 0; k < msg->num_cmds; k++) { > if (addr == msg->cmds[k].addr) > return -EBUSY; > @@ -548,18 +529,19 @@ static int check_for_req_inflight(struct rsc_drv *drv, struct tcs_group *tcs, > * > * Must be called with the drv->lock held since that protects tcs_in_use. > * > - * Return: The first tcs that's free. > + * Return: The first tcs that's free or -EBUSY if all in use. > */ > static int find_free_tcs(struct tcs_group *tcs) > { > - int i; > + const struct rsc_drv *drv = tcs->drv; > + unsigned long i; > + unsigned long max = tcs->offset + tcs->num_tcs; > > - for (i = 0; i < tcs->num_tcs; i++) { > - if (tcs_is_free(tcs->drv, tcs->offset + i)) > - return tcs->offset + i; > - } > + i = find_next_zero_bit(drv->tcs_in_use, max, tcs->offset); > + if (i >= max) > + return -EBUSY; > > - return -EBUSY; > + return i; > } > > /** > @@ -757,8 +739,9 @@ int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv, const struct tcs_request *msg) > */ > static bool rpmh_rsc_ctrlr_is_busy(struct rsc_drv *drv) > { > - int m; > - struct tcs_group *tcs = &drv->tcs[ACTIVE_TCS]; > + unsigned long set; > + const struct tcs_group *tcs = &drv->tcs[ACTIVE_TCS]; > + unsigned long max; > > /* > * If we made an active request on a RSC that does not have a > @@ -769,12 +752,10 @@ static bool rpmh_rsc_ctrlr_is_busy(struct rsc_drv *drv) > if (!tcs->num_tcs) > tcs = &drv->tcs[WAKE_TCS]; > > - for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) { > - if (!tcs_is_free(drv, m)) > - return true; > - } > + max = tcs->offset + tcs->num_tcs; > + set = find_next_bit(drv->tcs_in_use, max, tcs->offset); > > - return false; > + return set < max; > } > > /**
Hi, On Wed, May 20, 2020 at 11:04 PM Stephen Boyd <swboyd@chromium.org> wrote: > > This API does very little. Let's replace all the callsites with the > normal operations that would be done on top of the bitmap that > tcs_in_use is. This simplifies and reduces the code size. > > Cc: Maulik Shah <mkshah@codeaurora.org> > Cc: Douglas Anderson <dianders@chromium.org> > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > --- > drivers/soc/qcom/rpmh-rsc.c | 59 +++++++++++++------------------------ > 1 file changed, 20 insertions(+), 39 deletions(-) Sorry for the delay in getting to this review. Looks great. Reviewed-by: Douglas Anderson <dianders@chromium.org>
diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c index 076fd27f3081..60fc56987659 100644 --- a/drivers/soc/qcom/rpmh-rsc.c +++ b/drivers/soc/qcom/rpmh-rsc.c @@ -184,22 +184,6 @@ static void write_tcs_reg_sync(const struct rsc_drv *drv, int reg, int tcs_id, data, tcs_id, reg); } -/** - * tcs_is_free() - Return if a TCS is totally free. - * @drv: The RSC controller. - * @tcs_id: The global ID of this TCS. - * - * Returns true if nobody has claimed this TCS (by setting tcs_in_use). - * - * Context: Must be called with the drv->lock held. - * - * Return: true if the given TCS is free. - */ -static bool tcs_is_free(struct rsc_drv *drv, int tcs_id) -{ - return !test_bit(tcs_id, drv->tcs_in_use); -} - /** * tcs_invalidate() - Invalidate all TCSes of the given type (sleep or wake). * @drv: The RSC controller. @@ -512,7 +496,7 @@ static void __tcs_buffer_write(struct rsc_drv *drv, int tcs_id, int cmd_id, * * Return: 0 if nothing in flight or -EBUSY if we should try again later. * The caller must re-enable interrupts between tries since that's - * the only way tcs_is_free() will ever return true and the only way + * the only way tcs_in_use will ever be updated and the only way * RSC_DRV_CMD_ENABLE will ever be cleared. */ static int check_for_req_inflight(struct rsc_drv *drv, struct tcs_group *tcs, @@ -520,17 +504,14 @@ static int check_for_req_inflight(struct rsc_drv *drv, struct tcs_group *tcs, { unsigned long curr_enabled; u32 addr; - int i, j, k; - int tcs_id = tcs->offset; + int j, k; + int i = tcs->offset; - for (i = 0; i < tcs->num_tcs; i++, tcs_id++) { - if (tcs_is_free(drv, tcs_id)) - continue; - - curr_enabled = read_tcs_reg(drv, RSC_DRV_CMD_ENABLE, tcs_id); + for_each_set_bit_from(i, drv->tcs_in_use, tcs->offset + tcs->num_tcs) { + curr_enabled = read_tcs_reg(drv, RSC_DRV_CMD_ENABLE, i); for_each_set_bit(j, &curr_enabled, MAX_CMDS_PER_TCS) { - addr = read_tcs_cmd(drv, RSC_DRV_CMD_ADDR, tcs_id, j); + addr = read_tcs_cmd(drv, RSC_DRV_CMD_ADDR, i, j); for (k = 0; k < msg->num_cmds; k++) { if (addr == msg->cmds[k].addr) return -EBUSY; @@ -548,18 +529,19 @@ static int check_for_req_inflight(struct rsc_drv *drv, struct tcs_group *tcs, * * Must be called with the drv->lock held since that protects tcs_in_use. * - * Return: The first tcs that's free. + * Return: The first tcs that's free or -EBUSY if all in use. */ static int find_free_tcs(struct tcs_group *tcs) { - int i; + const struct rsc_drv *drv = tcs->drv; + unsigned long i; + unsigned long max = tcs->offset + tcs->num_tcs; - for (i = 0; i < tcs->num_tcs; i++) { - if (tcs_is_free(tcs->drv, tcs->offset + i)) - return tcs->offset + i; - } + i = find_next_zero_bit(drv->tcs_in_use, max, tcs->offset); + if (i >= max) + return -EBUSY; - return -EBUSY; + return i; } /** @@ -757,8 +739,9 @@ int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv, const struct tcs_request *msg) */ static bool rpmh_rsc_ctrlr_is_busy(struct rsc_drv *drv) { - int m; - struct tcs_group *tcs = &drv->tcs[ACTIVE_TCS]; + unsigned long set; + const struct tcs_group *tcs = &drv->tcs[ACTIVE_TCS]; + unsigned long max; /* * If we made an active request on a RSC that does not have a @@ -769,12 +752,10 @@ static bool rpmh_rsc_ctrlr_is_busy(struct rsc_drv *drv) if (!tcs->num_tcs) tcs = &drv->tcs[WAKE_TCS]; - for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) { - if (!tcs_is_free(drv, m)) - return true; - } + max = tcs->offset + tcs->num_tcs; + set = find_next_bit(drv->tcs_in_use, max, tcs->offset); - return false; + return set < max; } /**
This API does very little. Let's replace all the callsites with the normal operations that would be done on top of the bitmap that tcs_in_use is. This simplifies and reduces the code size. Cc: Maulik Shah <mkshah@codeaurora.org> Cc: Douglas Anderson <dianders@chromium.org> Signed-off-by: Stephen Boyd <swboyd@chromium.org> --- drivers/soc/qcom/rpmh-rsc.c | 59 +++++++++++++------------------------ 1 file changed, 20 insertions(+), 39 deletions(-)