* [RFC] firmware: arm_scmi: Optimize the iteration of scmi_requested_devices
@ 2025-01-06 6:57 Peng Fan (OSS)
2025-01-06 15:09 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Peng Fan (OSS) @ 2025-01-06 6:57 UTC (permalink / raw)
To: sudeep.holla, cristian.marussi, arm-scmi
Cc: linux-arm-kernel, linux-kernel, Peng Fan
From: Peng Fan <peng.fan@nxp.com>
scmi_requested_devices is organized in IDR based link lists, so only
need to search the link lists when there is a match protocol_id.
Back to search the next id with 'continue' to save cpu cycles, if
protocol_id does not match.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
RFC:
Tested on i.MX95.
drivers/firmware/arm_scmi/bus.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index 157172a5f2b5..42deab5903fb 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -79,6 +79,8 @@ static int scmi_protocol_device_request(const struct scmi_device_id *id_table)
if (rdev->id_table->protocol_id ==
id_table->protocol_id)
phead = head;
+ else
+ continue;
}
list_for_each_entry(rdev, head, node) {
if (!strcmp(rdev->id_table->name, id_table->name)) {
@@ -89,6 +91,7 @@ static int scmi_protocol_device_request(const struct scmi_device_id *id_table)
goto out;
}
}
+ break;
}
/*
--
2.37.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RFC] firmware: arm_scmi: Optimize the iteration of scmi_requested_devices
2025-01-06 6:57 [RFC] firmware: arm_scmi: Optimize the iteration of scmi_requested_devices Peng Fan (OSS)
@ 2025-01-06 15:09 ` Dan Carpenter
2025-01-07 1:49 ` Peng Fan
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2025-01-06 15:09 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: sudeep.holla, cristian.marussi, arm-scmi, linux-arm-kernel,
linux-kernel, Peng Fan
On Mon, Jan 06, 2025 at 02:57:24PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> scmi_requested_devices is organized in IDR based link lists, so only
> need to search the link lists when there is a match protocol_id.
>
> Back to search the next id with 'continue' to save cpu cycles, if
> protocol_id does not match.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>
> RFC:
> Tested on i.MX95.
>
> drivers/firmware/arm_scmi/bus.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
> index 157172a5f2b5..42deab5903fb 100644
> --- a/drivers/firmware/arm_scmi/bus.c
> +++ b/drivers/firmware/arm_scmi/bus.c
> @@ -79,6 +79,8 @@ static int scmi_protocol_device_request(const struct scmi_device_id *id_table)
> if (rdev->id_table->protocol_id ==
> id_table->protocol_id)
> phead = head;
> + else
> + continue;
> }
> list_for_each_entry(rdev, head, node) {
> if (!strcmp(rdev->id_table->name, id_table->name)) {
> @@ -89,6 +91,7 @@ static int scmi_protocol_device_request(const struct scmi_device_id *id_table)
> goto out;
> }
> }
> + break;
> }
I suspect your patch is correct but we could go further. Removing the
test for if (!phead) really helps readability.
regards,
dan carpenter
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index a3386bf36de5..2c853c84b58f 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -72,14 +72,11 @@ static int scmi_protocol_device_request(const struct scmi_device_id *id_table)
*/
mutex_lock(&scmi_requested_devices_mtx);
idr_for_each_entry(&scmi_requested_devices, head, id) {
- if (!phead) {
- /* A list found registered in the IDR is never empty */
- rdev = list_first_entry(head, struct scmi_requested_dev,
- node);
- if (rdev->id_table->protocol_id ==
- id_table->protocol_id)
- phead = head;
- }
+ /* A list found registered in the IDR is never empty */
+ rdev = list_first_entry(head, struct scmi_requested_dev, node);
+ if (rdev->id_table->protocol_id != id_table->protocol_id)
+ continue;
+
list_for_each_entry(rdev, head, node) {
if (!strcmp(rdev->id_table->name, id_table->name)) {
pr_err("Ignoring duplicate request [%d] %s\n",
@@ -89,6 +86,8 @@ static int scmi_protocol_device_request(const struct scmi_device_id *id_table)
goto out;
}
}
+ phead = head;
+ break;
}
/*
^ permalink raw reply related [flat|nested] 3+ messages in thread
* RE: [RFC] firmware: arm_scmi: Optimize the iteration of scmi_requested_devices
2025-01-06 15:09 ` Dan Carpenter
@ 2025-01-07 1:49 ` Peng Fan
0 siblings, 0 replies; 3+ messages in thread
From: Peng Fan @ 2025-01-07 1:49 UTC (permalink / raw)
To: Dan Carpenter, Peng Fan (OSS)
Cc: sudeep.holla@arm.com, cristian.marussi@arm.com,
arm-scmi@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Hi Dan,
> Subject: Re: [RFC] firmware: arm_scmi: Optimize the iteration of
> scmi_requested_devices
>
> On Mon, Jan 06, 2025 at 02:57:24PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > scmi_requested_devices is organized in IDR based link lists, so only
> > need to search the link lists when there is a match protocol_id.
> >
> > Back to search the next id with 'continue' to save cpu cycles, if
> > protocol_id does not match.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >
> > RFC:
> > Tested on i.MX95.
> >
> > drivers/firmware/arm_scmi/bus.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/firmware/arm_scmi/bus.c
> > b/drivers/firmware/arm_scmi/bus.c index
> 157172a5f2b5..42deab5903fb
> > 100644
> > --- a/drivers/firmware/arm_scmi/bus.c
> > +++ b/drivers/firmware/arm_scmi/bus.c
> > @@ -79,6 +79,8 @@ static int scmi_protocol_device_request(const
> struct scmi_device_id *id_table)
> > if (rdev->id_table->protocol_id ==
> > id_table->protocol_id)
> > phead = head;
> > + else
> > + continue;
> > }
> > list_for_each_entry(rdev, head, node) {
> > if (!strcmp(rdev->id_table->name, id_table-
> >name)) { @@ -89,6
> > +91,7 @@ static int scmi_protocol_device_request(const struct
> scmi_device_id *id_table)
> > goto out;
> > }
> > }
> > + break;
> > }
>
> I suspect your patch is correct but we could go further. Removing the
> test for if (!phead) really helps readability.
Thanks. Looks better.
Thanks,
Peng.
>
> regards,
> dan carpenter
>
> diff --git a/drivers/firmware/arm_scmi/bus.c
> b/drivers/firmware/arm_scmi/bus.c index
> a3386bf36de5..2c853c84b58f 100644
> --- a/drivers/firmware/arm_scmi/bus.c
> +++ b/drivers/firmware/arm_scmi/bus.c
> @@ -72,14 +72,11 @@ static int scmi_protocol_device_request(const
> struct scmi_device_id *id_table)
> */
> mutex_lock(&scmi_requested_devices_mtx);
> idr_for_each_entry(&scmi_requested_devices, head, id) {
> - if (!phead) {
> - /* A list found registered in the IDR is never
> empty */
> - rdev = list_first_entry(head, struct
> scmi_requested_dev,
> - node);
> - if (rdev->id_table->protocol_id ==
> - id_table->protocol_id)
> - phead = head;
> - }
> + /* A list found registered in the IDR is never empty */
> + rdev = list_first_entry(head, struct
> scmi_requested_dev, node);
> + if (rdev->id_table->protocol_id != id_table-
> >protocol_id)
> + continue;
> +
> list_for_each_entry(rdev, head, node) {
> if (!strcmp(rdev->id_table->name, id_table-
> >name)) {
> pr_err("Ignoring duplicate request
> [%d] %s\n", @@ -89,6 +86,8 @@ static int
> scmi_protocol_device_request(const struct scmi_device_id *id_table)
> goto out;
> }
> }
> + phead = head;
> + break;
> }
>
> /*
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-01-07 1:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-06 6:57 [RFC] firmware: arm_scmi: Optimize the iteration of scmi_requested_devices Peng Fan (OSS)
2025-01-06 15:09 ` Dan Carpenter
2025-01-07 1:49 ` Peng Fan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox