| Message ID | 7414189.EvYhyI6sBW@kreacher (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On Thu, Jun 09, 2022 at 03:54:40PM +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 (!adev) > return NULL; Already checked in the below call, IIUC. Hence can be removed. > + return acpi_find_child_by_adr(adev, port->port);
On Thu, Jun 9, 2022 at 5:26 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Jun 09, 2022 at 03:54:40PM +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 (!adev) > > return NULL; > > Already checked in the below call, IIUC. Hence can be removed. Yes, it can, will update. > > > + return acpi_find_child_by_adr(adev, port->port); > > --
On Thu, Jun 09, 2022 at 03:54:40PM +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/thunderbolt/acpi.c | 9 +-------- > 1 file changed, 1 insertion(+), 8 deletions(-) > > Index: linux-pm/drivers/thunderbolt/acpi.c > =================================================================== > --- linux-pm.orig/drivers/thunderbolt/acpi.c > +++ linux-pm/drivers/thunderbolt/acpi.c > @@ -304,8 +304,6 @@ static bool tb_acpi_bus_match(struct dev > static struct acpi_device *tb_acpi_find_port(struct acpi_device *adev, > const struct tb_port *port) > { > - struct acpi_device *port_adev; > - > if (!adev) > return NULL; > > @@ -313,12 +311,7 @@ static struct acpi_device *tb_acpi_find_ > * Device routers exists under the downstream facing USB4 port > * of the parent router. Their _ADR is always 0. > */ > - list_for_each_entry(port_adev, &adev->children, node) { > - if (acpi_device_adr(port_adev) == port->port) > - return port_adev; > - } > - > - return NULL; > + return acpi_find_child_by_adr(adev, port->port); > } > > static struct acpi_device *tb_acpi_switch_find_companion(struct tb_switch *sw) I don't think you need tb_acpi_find_port() anymore. You can just call acpi_find_child_by_ard(ACPI_COMPANION(...), port->port) directly, no? thanks,
On Fri, Jun 10, 2022 at 8:46 AM Heikki Krogerus <heikki.krogerus@linux.intel.com> wrote: > > On Thu, Jun 09, 2022 at 03:54:40PM +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/thunderbolt/acpi.c | 9 +-------- > > 1 file changed, 1 insertion(+), 8 deletions(-) > > > > Index: linux-pm/drivers/thunderbolt/acpi.c > > =================================================================== > > --- linux-pm.orig/drivers/thunderbolt/acpi.c > > +++ linux-pm/drivers/thunderbolt/acpi.c > > @@ -304,8 +304,6 @@ static bool tb_acpi_bus_match(struct dev > > static struct acpi_device *tb_acpi_find_port(struct acpi_device *adev, > > const struct tb_port *port) > > { > > - struct acpi_device *port_adev; > > - > > if (!adev) > > return NULL; > > > > @@ -313,12 +311,7 @@ static struct acpi_device *tb_acpi_find_ > > * Device routers exists under the downstream facing USB4 port > > * of the parent router. Their _ADR is always 0. > > */ > > - list_for_each_entry(port_adev, &adev->children, node) { > > - if (acpi_device_adr(port_adev) == port->port) > > - return port_adev; > > - } > > - > > - return NULL; > > + return acpi_find_child_by_adr(adev, port->port); > > } > > > > static struct acpi_device *tb_acpi_switch_find_companion(struct tb_switch *sw) > > I don't think you need tb_acpi_find_port() anymore. You can just call > acpi_find_child_by_ard(ACPI_COMPANION(...), port->port) directly, no? Technically I can, but I thought that the comment in tb_acpi_find_port() was worth retaining.
Index: linux-pm/drivers/thunderbolt/acpi.c =================================================================== --- linux-pm.orig/drivers/thunderbolt/acpi.c +++ linux-pm/drivers/thunderbolt/acpi.c @@ -304,8 +304,6 @@ static bool tb_acpi_bus_match(struct dev static struct acpi_device *tb_acpi_find_port(struct acpi_device *adev, const struct tb_port *port) { - struct acpi_device *port_adev; - if (!adev) return NULL; @@ -313,12 +311,7 @@ static struct acpi_device *tb_acpi_find_ * Device routers exists under the downstream facing USB4 port * of the parent router. Their _ADR is always 0. */ - list_for_each_entry(port_adev, &adev->children, node) { - if (acpi_device_adr(port_adev) == port->port) - return port_adev; - } - - return NULL; + return acpi_find_child_by_adr(adev, port->port); } static struct acpi_device *tb_acpi_switch_find_companion(struct tb_switch *sw)