* [PATCH 0/3] Even moar rpmh cleanups
@ 2020-04-24 4:54 Stephen Boyd
2020-04-24 4:54 ` [PATCH 1/3] soc: qcom: rpmh-rsc: Remove tcs_is_free() API Stephen Boyd
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Stephen Boyd @ 2020-04-24 4:54 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson
Cc: linux-kernel, linux-arm-msm, Maulik Shah, Douglas Anderson
Patches based on Doug's latest series[1] on top of linux-next. We remove
the tcs_is_free() API and then do super micro optimizations on the irq
handler. I haven't tested anything here so most likely there's a bug!
Stephen Boyd (3):
soc: qcom: rpmh-rsc: Remove tcs_is_free() API
soc: qcom: rpmh-rsc: Loop over less bits in irq handler
soc: qcom: rpmh-rsc: Fold WARN_ON() into if condition
drivers/soc/qcom/rpmh-rsc.c | 64 ++++++++++++-------------------------
1 file changed, 20 insertions(+), 44 deletions(-)
Cc: Maulik Shah <mkshah@codeaurora.org>
Cc: Douglas Anderson <dianders@chromium.org>
[1] https://lore.kernel.org/r/20200422145408.v4.1.Ic7096b3b9b7828cdd41cd5469a6dee5eb6abf549@changeid
base-commit: 02d8ecc18b8f392389ac9e7b785b0230ecb80833
prerequisite-patch-id: 0d383ea46ef52ab4044886a7d88d85c3c506f4ed
prerequisite-patch-id: a02b0b018404d1a0c79270ab567051656f123b23
prerequisite-patch-id: e59d990462b004a9f8335e87c2d0d747afec49ea
prerequisite-patch-id: 00d26aeb99c48521f74b32d3a6c57919d82ac1b1
prerequisite-patch-id: bb479b9adbe28c58b3ac8f363a306de80b6dcb74
--
Sent by a computer, using git, on the internet
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 1/3] soc: qcom: rpmh-rsc: Remove tcs_is_free() API 2020-04-24 4:54 [PATCH 0/3] Even moar rpmh cleanups Stephen Boyd @ 2020-04-24 4:54 ` Stephen Boyd 2020-04-24 5:36 ` Bjorn Andersson 2020-04-24 17:12 ` Doug Anderson 2020-04-24 4:54 ` [PATCH 2/3] soc: qcom: rpmh-rsc: Loop over less bits in irq handler Stephen Boyd 2020-04-24 4:54 ` [PATCH 3/3] soc: qcom: rpmh-rsc: Fold WARN_ON() into if condition Stephen Boyd 2 siblings, 2 replies; 14+ messages in thread From: Stephen Boyd @ 2020-04-24 4:54 UTC (permalink / raw) To: Andy Gross, Bjorn Andersson Cc: linux-kernel, linux-arm-msm, Maulik Shah, Douglas Anderson 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 | 56 +++++++++++-------------------------- 1 file changed, 17 insertions(+), 39 deletions(-) diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c index 060be10bc491..76e0294a672c 100644 --- a/drivers/soc/qcom/rpmh-rsc.c +++ b/drivers/soc/qcom/rpmh-rsc.c @@ -172,22 +172,6 @@ static void write_tcs_reg_sync(struct rsc_drv *drv, int reg, int tcs_id, } } -/** - * 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. @@ -500,7 +484,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, @@ -508,17 +492,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; @@ -536,18 +517,18 @@ 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; - 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_NR, tcs->offset); + if (i > tcs->offset + tcs->num_tcs) + return -EBUSY; - return -EBUSY; + return i; } /** @@ -744,8 +725,8 @@ 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]; /* * If we made an active request on a RSC that does not have a @@ -756,12 +737,9 @@ 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; - } + set = find_next_bit(drv->tcs_in_use, MAX_TCS_NR, tcs->offset); - return false; + return set < tcs->offset + tcs->num_tcs; } /** -- Sent by a computer, using git, on the internet ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] soc: qcom: rpmh-rsc: Remove tcs_is_free() API 2020-04-24 4:54 ` [PATCH 1/3] soc: qcom: rpmh-rsc: Remove tcs_is_free() API Stephen Boyd @ 2020-04-24 5:36 ` Bjorn Andersson 2020-04-24 17:21 ` Stephen Boyd 2020-04-24 17:12 ` Doug Anderson 1 sibling, 1 reply; 14+ messages in thread From: Bjorn Andersson @ 2020-04-24 5:36 UTC (permalink / raw) To: Stephen Boyd Cc: Andy Gross, linux-kernel, linux-arm-msm, Maulik Shah, Douglas Anderson On Thu 23 Apr 21:54 PDT 2020, Stephen Boyd wrote: > diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c [..] > static int find_free_tcs(struct tcs_group *tcs) > { > - int i; > + const struct rsc_drv *drv = tcs->drv; > + unsigned long i; > > - 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_NR, tcs->offset); > + if (i > tcs->offset + tcs->num_tcs) Afaict this should be >= Regards, Bjorn ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] soc: qcom: rpmh-rsc: Remove tcs_is_free() API 2020-04-24 5:36 ` Bjorn Andersson @ 2020-04-24 17:21 ` Stephen Boyd 0 siblings, 0 replies; 14+ messages in thread From: Stephen Boyd @ 2020-04-24 17:21 UTC (permalink / raw) To: Bjorn Andersson Cc: Andy Gross, linux-kernel, linux-arm-msm, Maulik Shah, Douglas Anderson Quoting Bjorn Andersson (2020-04-23 22:36:26) > On Thu 23 Apr 21:54 PDT 2020, Stephen Boyd wrote: > > diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c > [..] > > static int find_free_tcs(struct tcs_group *tcs) > > { > > - int i; > > + const struct rsc_drv *drv = tcs->drv; > > + unsigned long i; > > > > - 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_NR, tcs->offset); > > + if (i > tcs->offset + tcs->num_tcs) > > Afaict this should be >= > Thanks! ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] soc: qcom: rpmh-rsc: Remove tcs_is_free() API 2020-04-24 4:54 ` [PATCH 1/3] soc: qcom: rpmh-rsc: Remove tcs_is_free() API Stephen Boyd 2020-04-24 5:36 ` Bjorn Andersson @ 2020-04-24 17:12 ` Doug Anderson 2020-04-24 17:28 ` Stephen Boyd 1 sibling, 1 reply; 14+ messages in thread From: Doug Anderson @ 2020-04-24 17:12 UTC (permalink / raw) To: Stephen Boyd Cc: Andy Gross, Bjorn Andersson, LKML, linux-arm-msm, Maulik Shah Hi, On Thu, Apr 23, 2020 at 9:54 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 | 56 +++++++++++-------------------------- > 1 file changed, 17 insertions(+), 39 deletions(-) > > diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c > index 060be10bc491..76e0294a672c 100644 > --- a/drivers/soc/qcom/rpmh-rsc.c > +++ b/drivers/soc/qcom/rpmh-rsc.c > @@ -172,22 +172,6 @@ static void write_tcs_reg_sync(struct rsc_drv *drv, int reg, int tcs_id, > } > } > > -/** > - * 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. > @@ -500,7 +484,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, > @@ -508,17 +492,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; > @@ -536,18 +517,18 @@ 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; > > - 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_NR, tcs->offset); Rather than passing MAX_TCS_NR would it be legit to pass "tcs->offset + tcs->num_tcs"? You are passing that as "size" above in check_for_req_inflight(). > + if (i > tcs->offset + tcs->num_tcs) Should the above be ">=" ? Oh, I guess Bjorn found the same thing. ;-) > + return -EBUSY; > > - return -EBUSY; > + return i; > } > > /** > @@ -744,8 +725,8 @@ 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]; > > /* > * If we made an active request on a RSC that does not have a > @@ -756,12 +737,9 @@ 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; > - } > + set = find_next_bit(drv->tcs_in_use, MAX_TCS_NR, tcs->offset); Similar to above, can you pass "tcs->offset + tcs->num_tcs" instead of MAX_TCS_NR? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] soc: qcom: rpmh-rsc: Remove tcs_is_free() API 2020-04-24 17:12 ` Doug Anderson @ 2020-04-24 17:28 ` Stephen Boyd 0 siblings, 0 replies; 14+ messages in thread From: Stephen Boyd @ 2020-04-24 17:28 UTC (permalink / raw) To: Doug Anderson Cc: Andy Gross, Bjorn Andersson, LKML, linux-arm-msm, Maulik Shah Quoting Doug Anderson (2020-04-24 10:12:04) > On Thu, Apr 23, 2020 at 9:54 PM Stephen Boyd <swboyd@chromium.org> wrote: > > + unsigned long i; > > > > - 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_NR, tcs->offset); > > Rather than passing MAX_TCS_NR would it be legit to pass "tcs->offset > + tcs->num_tcs"? You are passing that as "size" above in > check_for_req_inflight(). Yeah I think that should work. I'll roll it in. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/3] soc: qcom: rpmh-rsc: Loop over less bits in irq handler 2020-04-24 4:54 [PATCH 0/3] Even moar rpmh cleanups Stephen Boyd 2020-04-24 4:54 ` [PATCH 1/3] soc: qcom: rpmh-rsc: Remove tcs_is_free() API Stephen Boyd @ 2020-04-24 4:54 ` Stephen Boyd 2020-04-24 5:37 ` Bjorn Andersson 2020-04-24 17:11 ` Doug Anderson 2020-04-24 4:54 ` [PATCH 3/3] soc: qcom: rpmh-rsc: Fold WARN_ON() into if condition Stephen Boyd 2 siblings, 2 replies; 14+ messages in thread From: Stephen Boyd @ 2020-04-24 4:54 UTC (permalink / raw) To: Andy Gross, Bjorn Andersson Cc: linux-kernel, linux-arm-msm, Maulik Shah, Douglas Anderson readl returns a u32, and BITS_PER_LONG is different on 32-bit vs. 64-bit architectures. Let's make the type we stash the readl into a u32 and then loop over the bits set in that type instead of potentially looping over more bits than we will ever need to. 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 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c index 76e0294a672c..462dd267afef 100644 --- a/drivers/soc/qcom/rpmh-rsc.c +++ b/drivers/soc/qcom/rpmh-rsc.c @@ -365,13 +365,13 @@ static irqreturn_t tcs_tx_done(int irq, void *p) { struct rsc_drv *drv = p; int i, j, err = 0; - unsigned long irq_status; + u32 irq_status; const struct tcs_request *req; struct tcs_cmd *cmd; irq_status = readl_relaxed(drv->tcs_base + RSC_DRV_IRQ_STATUS); - for_each_set_bit(i, &irq_status, BITS_PER_LONG) { + for_each_set_bit(i, &irq_status, BITS_PER_TYPE(u32)) { req = get_req_from_tcs(drv, i); if (!req) { WARN_ON(1); -- Sent by a computer, using git, on the internet ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] soc: qcom: rpmh-rsc: Loop over less bits in irq handler 2020-04-24 4:54 ` [PATCH 2/3] soc: qcom: rpmh-rsc: Loop over less bits in irq handler Stephen Boyd @ 2020-04-24 5:37 ` Bjorn Andersson 2020-04-24 17:11 ` Doug Anderson 1 sibling, 0 replies; 14+ messages in thread From: Bjorn Andersson @ 2020-04-24 5:37 UTC (permalink / raw) To: Stephen Boyd Cc: Andy Gross, linux-kernel, linux-arm-msm, Maulik Shah, Douglas Anderson On Thu 23 Apr 21:54 PDT 2020, Stephen Boyd wrote: > readl returns a u32, and BITS_PER_LONG is different on 32-bit vs. 64-bit > architectures. Let's make the type we stash the readl into a u32 and > then loop over the bits set in that type instead of potentially looping > over more bits than we will ever need to. > > Cc: Maulik Shah <mkshah@codeaurora.org> > Cc: Douglas Anderson <dianders@chromium.org> > Signed-off-by: Stephen Boyd <swboyd@chromium.org> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > drivers/soc/qcom/rpmh-rsc.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c > index 76e0294a672c..462dd267afef 100644 > --- a/drivers/soc/qcom/rpmh-rsc.c > +++ b/drivers/soc/qcom/rpmh-rsc.c > @@ -365,13 +365,13 @@ static irqreturn_t tcs_tx_done(int irq, void *p) > { > struct rsc_drv *drv = p; > int i, j, err = 0; > - unsigned long irq_status; > + u32 irq_status; > const struct tcs_request *req; > struct tcs_cmd *cmd; > > irq_status = readl_relaxed(drv->tcs_base + RSC_DRV_IRQ_STATUS); > > - for_each_set_bit(i, &irq_status, BITS_PER_LONG) { > + for_each_set_bit(i, &irq_status, BITS_PER_TYPE(u32)) { > req = get_req_from_tcs(drv, i); > if (!req) { > WARN_ON(1); > -- > Sent by a computer, using git, on the internet > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] soc: qcom: rpmh-rsc: Loop over less bits in irq handler 2020-04-24 4:54 ` [PATCH 2/3] soc: qcom: rpmh-rsc: Loop over less bits in irq handler Stephen Boyd 2020-04-24 5:37 ` Bjorn Andersson @ 2020-04-24 17:11 ` Doug Anderson 2020-04-24 17:13 ` Doug Anderson 1 sibling, 1 reply; 14+ messages in thread From: Doug Anderson @ 2020-04-24 17:11 UTC (permalink / raw) To: Stephen Boyd Cc: Andy Gross, Bjorn Andersson, LKML, linux-arm-msm, Maulik Shah Hi, On Thu, Apr 23, 2020 at 9:54 PM Stephen Boyd <swboyd@chromium.org> wrote: > > readl returns a u32, and BITS_PER_LONG is different on 32-bit vs. 64-bit > architectures. Let's make the type we stash the readl into a u32 and > then loop over the bits set in that type instead of potentially looping > over more bits than we will ever need to. > > 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 | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Douglas Anderson <dianders@chromium.org> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] soc: qcom: rpmh-rsc: Loop over less bits in irq handler 2020-04-24 17:11 ` Doug Anderson @ 2020-04-24 17:13 ` Doug Anderson 2020-04-24 17:27 ` Stephen Boyd 0 siblings, 1 reply; 14+ messages in thread From: Doug Anderson @ 2020-04-24 17:13 UTC (permalink / raw) To: Stephen Boyd Cc: Andy Gross, Bjorn Andersson, LKML, linux-arm-msm, Maulik Shah Hi, On Fri, Apr 24, 2020 at 10:11 AM Doug Anderson <dianders@chromium.org> wrote: > > Hi, > > On Thu, Apr 23, 2020 at 9:54 PM Stephen Boyd <swboyd@chromium.org> wrote: > > > > readl returns a u32, and BITS_PER_LONG is different on 32-bit vs. 64-bit > > architectures. Let's make the type we stash the readl into a u32 and > > then loop over the bits set in that type instead of potentially looping > > over more bits than we will ever need to. > > > > 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 | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > Reviewed-by: Douglas Anderson <dianders@chromium.org> Oh, I suppose one nit is s/less/fewer/ in the subject. "bits" are discrete / countable, not continuous / uncountable. -Doug ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] soc: qcom: rpmh-rsc: Loop over less bits in irq handler 2020-04-24 17:13 ` Doug Anderson @ 2020-04-24 17:27 ` Stephen Boyd 0 siblings, 0 replies; 14+ messages in thread From: Stephen Boyd @ 2020-04-24 17:27 UTC (permalink / raw) To: Doug Anderson Cc: Andy Gross, Bjorn Andersson, LKML, linux-arm-msm, Maulik Shah Quoting Doug Anderson (2020-04-24 10:13:43) > Hi, > > On Fri, Apr 24, 2020 at 10:11 AM Doug Anderson <dianders@chromium.org> wrote: > > > > Hi, > > > > On Thu, Apr 23, 2020 at 9:54 PM Stephen Boyd <swboyd@chromium.org> wrote: > > > > > > readl returns a u32, and BITS_PER_LONG is different on 32-bit vs. 64-bit > > > architectures. Let's make the type we stash the readl into a u32 and > > > then loop over the bits set in that type instead of potentially looping > > > over more bits than we will ever need to. > > > > > > 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 | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > Reviewed-by: Douglas Anderson <dianders@chromium.org> > > Oh, I suppose one nit is s/less/fewer/ in the subject. "bits" are > discrete / countable, not continuous / uncountable. > Ok that's good. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] soc: qcom: rpmh-rsc: Fold WARN_ON() into if condition 2020-04-24 4:54 [PATCH 0/3] Even moar rpmh cleanups Stephen Boyd 2020-04-24 4:54 ` [PATCH 1/3] soc: qcom: rpmh-rsc: Remove tcs_is_free() API Stephen Boyd 2020-04-24 4:54 ` [PATCH 2/3] soc: qcom: rpmh-rsc: Loop over less bits in irq handler Stephen Boyd @ 2020-04-24 4:54 ` Stephen Boyd 2020-04-24 5:37 ` Bjorn Andersson 2020-04-24 17:11 ` Doug Anderson 2 siblings, 2 replies; 14+ messages in thread From: Stephen Boyd @ 2020-04-24 4:54 UTC (permalink / raw) To: Andy Gross, Bjorn Andersson Cc: linux-kernel, linux-arm-msm, Maulik Shah, Douglas Anderson Move the WARN_ON() into the if condition so the compiler can see that the branch is unlikely() and possibly optimize it better. 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 | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c index 462dd267afef..f7763f008e03 100644 --- a/drivers/soc/qcom/rpmh-rsc.c +++ b/drivers/soc/qcom/rpmh-rsc.c @@ -373,10 +373,8 @@ static irqreturn_t tcs_tx_done(int irq, void *p) for_each_set_bit(i, &irq_status, BITS_PER_TYPE(u32)) { req = get_req_from_tcs(drv, i); - if (!req) { - WARN_ON(1); + if (WARN_ON(!req)) goto skip; - } err = 0; for (j = 0; j < req->num_cmds; j++) { -- Sent by a computer, using git, on the internet ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] soc: qcom: rpmh-rsc: Fold WARN_ON() into if condition 2020-04-24 4:54 ` [PATCH 3/3] soc: qcom: rpmh-rsc: Fold WARN_ON() into if condition Stephen Boyd @ 2020-04-24 5:37 ` Bjorn Andersson 2020-04-24 17:11 ` Doug Anderson 1 sibling, 0 replies; 14+ messages in thread From: Bjorn Andersson @ 2020-04-24 5:37 UTC (permalink / raw) To: Stephen Boyd Cc: Andy Gross, linux-kernel, linux-arm-msm, Maulik Shah, Douglas Anderson On Thu 23 Apr 21:54 PDT 2020, Stephen Boyd wrote: > Move the WARN_ON() into the if condition so the compiler can see that > the branch is unlikely() and possibly optimize it better. > > Cc: Maulik Shah <mkshah@codeaurora.org> > Cc: Douglas Anderson <dianders@chromium.org> > Signed-off-by: Stephen Boyd <swboyd@chromium.org> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > drivers/soc/qcom/rpmh-rsc.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c > index 462dd267afef..f7763f008e03 100644 > --- a/drivers/soc/qcom/rpmh-rsc.c > +++ b/drivers/soc/qcom/rpmh-rsc.c > @@ -373,10 +373,8 @@ static irqreturn_t tcs_tx_done(int irq, void *p) > > for_each_set_bit(i, &irq_status, BITS_PER_TYPE(u32)) { > req = get_req_from_tcs(drv, i); > - if (!req) { > - WARN_ON(1); > + if (WARN_ON(!req)) > goto skip; > - } > > err = 0; > for (j = 0; j < req->num_cmds; j++) { > -- > Sent by a computer, using git, on the internet > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] soc: qcom: rpmh-rsc: Fold WARN_ON() into if condition 2020-04-24 4:54 ` [PATCH 3/3] soc: qcom: rpmh-rsc: Fold WARN_ON() into if condition Stephen Boyd 2020-04-24 5:37 ` Bjorn Andersson @ 2020-04-24 17:11 ` Doug Anderson 1 sibling, 0 replies; 14+ messages in thread From: Doug Anderson @ 2020-04-24 17:11 UTC (permalink / raw) To: Stephen Boyd Cc: Andy Gross, Bjorn Andersson, LKML, linux-arm-msm, Maulik Shah Hi, On Thu, Apr 23, 2020 at 9:54 PM Stephen Boyd <swboyd@chromium.org> wrote: > > Move the WARN_ON() into the if condition so the compiler can see that > the branch is unlikely() and possibly optimize it better. > > 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 | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Douglas Anderson <dianders@chromium.org> ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-04-24 17:28 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-04-24 4:54 [PATCH 0/3] Even moar rpmh cleanups Stephen Boyd 2020-04-24 4:54 ` [PATCH 1/3] soc: qcom: rpmh-rsc: Remove tcs_is_free() API Stephen Boyd 2020-04-24 5:36 ` Bjorn Andersson 2020-04-24 17:21 ` Stephen Boyd 2020-04-24 17:12 ` Doug Anderson 2020-04-24 17:28 ` Stephen Boyd 2020-04-24 4:54 ` [PATCH 2/3] soc: qcom: rpmh-rsc: Loop over less bits in irq handler Stephen Boyd 2020-04-24 5:37 ` Bjorn Andersson 2020-04-24 17:11 ` Doug Anderson 2020-04-24 17:13 ` Doug Anderson 2020-04-24 17:27 ` Stephen Boyd 2020-04-24 4:54 ` [PATCH 3/3] soc: qcom: rpmh-rsc: Fold WARN_ON() into if condition Stephen Boyd 2020-04-24 5:37 ` Bjorn Andersson 2020-04-24 17:11 ` Doug Anderson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).