* [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
* [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
* [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 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 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 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
* 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 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 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 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 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
* 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
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).