| Message ID | 20250106065724.3674510-1-peng.fan@oss.nxp.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] firmware: arm_scmi: Optimize the iteration of scmi_requested_devices | expand |
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; } /*
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; > } > > /*
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; } /*