From: Rafael J. Wysocki <[email protected]>
Instead of walking the list of children of an ACPI device directly
in order to find the child matching a given bus address, use
acpi_find_child_by_adr() for this purpose.
Apart from simplifying the code, this will help to eliminate the
children list head from struct acpi_device as it is redundant and it
is used in questionable ways in some places (in particular, locking is
needed for walking the list pointed to it safely, but it is often
missing).
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/usb/core/usb-acpi.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
Index: linux-pm/drivers/usb/core/usb-acpi.c
===================================================================
--- linux-pm.orig/drivers/usb/core/usb-acpi.c
+++ linux-pm/drivers/usb/core/usb-acpi.c
@@ -127,17 +127,10 @@ out:
static struct acpi_device *usb_acpi_find_port(struct acpi_device *parent,
int raw)
{
- struct acpi_device *adev;
-
if (!parent)
return NULL;
- list_for_each_entry(adev, &parent->children, node) {
- if (acpi_device_adr(adev) == raw)
- return adev;
- }
-
- return acpi_find_child_device(parent, raw, false);
+ return acpi_find_child_by_adr(parent, raw);
}
static struct acpi_device *
On Thu, Jun 09, 2022 at 03:56:42PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> Instead of walking the list of children of an ACPI device directly
> in order to find the child matching a given bus address, use
> acpi_find_child_by_adr() for this purpose.
...
> if (!parent)
> return NULL;
Can be removed because it's embedded in the call below, no?
> + return acpi_find_child_by_adr(parent, raw);
--
With Best Regards,
Andy Shevchenko
On Thu, Jun 9, 2022 at 5:27 PM Andy Shevchenko
<[email protected]> wrote:
>
> On Thu, Jun 09, 2022 at 03:56:42PM +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <[email protected]>
> >
> > Instead of walking the list of children of an ACPI device directly
> > in order to find the child matching a given bus address, use
> > acpi_find_child_by_adr() for this purpose.
>
> ...
>
> > if (!parent)
> > return NULL;
>
> Can be removed because it's embedded in the call below, no?
Yes, it can, in analogy with the thunderbolt code. Will update.
> > + return acpi_find_child_by_adr(parent, raw);
>
> --
On Thu, Jun 09, 2022 at 03:56:42PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> Instead of walking the list of children of an ACPI device directly
> in order to find the child matching a given bus address, use
> acpi_find_child_by_adr() for this purpose.
>
> Apart from simplifying the code, this will help to eliminate the
> children list head from struct acpi_device as it is redundant and it
> is used in questionable ways in some places (in particular, locking is
> needed for walking the list pointed to it safely, but it is often
> missing).
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/usb/core/usb-acpi.c | 9 +--------
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
> Index: linux-pm/drivers/usb/core/usb-acpi.c
> ===================================================================
> --- linux-pm.orig/drivers/usb/core/usb-acpi.c
> +++ linux-pm/drivers/usb/core/usb-acpi.c
> @@ -127,17 +127,10 @@ out:
> static struct acpi_device *usb_acpi_find_port(struct acpi_device *parent,
> int raw)
> {
> - struct acpi_device *adev;
> -
> if (!parent)
> return NULL;
>
> - list_for_each_entry(adev, &parent->children, node) {
> - if (acpi_device_adr(adev) == raw)
> - return adev;
> - }
> -
> - return acpi_find_child_device(parent, raw, false);
> + return acpi_find_child_by_adr(parent, raw);
> }
>
> static struct acpi_device *
I think usb_acpi_find_port() can also be dropped now.
thanks,
--
heikki
On Fri, Jun 10, 2022 at 8:47 AM Heikki Krogerus
<[email protected]> wrote:
>
> On Thu, Jun 09, 2022 at 03:56:42PM +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <[email protected]>
> >
> > Instead of walking the list of children of an ACPI device directly
> > in order to find the child matching a given bus address, use
> > acpi_find_child_by_adr() for this purpose.
> >
> > Apart from simplifying the code, this will help to eliminate the
> > children list head from struct acpi_device as it is redundant and it
> > is used in questionable ways in some places (in particular, locking is
> > needed for walking the list pointed to it safely, but it is often
> > missing).
> >
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> > ---
> > drivers/usb/core/usb-acpi.c | 9 +--------
> > 1 file changed, 1 insertion(+), 8 deletions(-)
> >
> > Index: linux-pm/drivers/usb/core/usb-acpi.c
> > ===================================================================
> > --- linux-pm.orig/drivers/usb/core/usb-acpi.c
> > +++ linux-pm/drivers/usb/core/usb-acpi.c
> > @@ -127,17 +127,10 @@ out:
> > static struct acpi_device *usb_acpi_find_port(struct acpi_device *parent,
> > int raw)
> > {
> > - struct acpi_device *adev;
> > -
> > if (!parent)
> > return NULL;
> >
> > - list_for_each_entry(adev, &parent->children, node) {
> > - if (acpi_device_adr(adev) == raw)
> > - return adev;
> > - }
> > -
> > - return acpi_find_child_device(parent, raw, false);
> > + return acpi_find_child_by_adr(parent, raw);
> > }
> >
> > static struct acpi_device *
>
> I think usb_acpi_find_port() can also be dropped now.
Yes, it can.
I'm dropping it in the new version of the patch to be posted.