| Message ID | 3459925.iIbC2pHGDl@kreacher (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On Thu, Jun 09, 2022 at 03:56:42PM +0200, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > 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);
On Thu, Jun 9, 2022 at 5:27 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Jun 09, 2022 at 03:56:42PM +0200, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > 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 <rafael.j.wysocki@intel.com> > > 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 <rafael.j.wysocki@intel.com> > --- > 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,
On Fri, Jun 10, 2022 at 8:47 AM Heikki Krogerus <heikki.krogerus@linux.intel.com> wrote: > > On Thu, Jun 09, 2022 at 03:56:42PM +0200, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > 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 <rafael.j.wysocki@intel.com> > > --- > > 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.
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 *