diff mbox series

[v1,06/16] ACPI: container: Use acpi_dev_for_each_child()

Message ID 3131424.5fSG56mABF@kreacher
State Superseded
Headers show
Series ACPI: Get rid of the list of children in struct acpi_device | expand

Commit Message

Rafael J. Wysocki June 9, 2022, 1:58 p.m. UTC 
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Instead of walking the list of children of an ACPI device directly,
use acpi_dev_for_each_child() to carry out an action for all of
the given ACPI device's children.

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/acpi/container.c |   18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

Comments

Andy Shevchenko June 9, 2022, 3:29 p.m. UTC | #1 
On Thu, Jun 09, 2022 at 03:58:24PM +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,
> use acpi_dev_for_each_child() to carry out an action for all of
> the given ACPI device's children.

...

> +	return acpi_dev_for_each_child(ACPI_COMPANION(&cdev->dev),
> +				       check_offline, NULL);

I would find this on one line better and not missing important details after
80th character.
Rafael J. Wysocki June 9, 2022, 3:58 p.m. UTC | #2 
On Thu, Jun 9, 2022 at 5:29 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Jun 09, 2022 at 03:58:24PM +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,
> > use acpi_dev_for_each_child() to carry out an action for all of
> > the given ACPI device's children.
>
> ...
>
> > +     return acpi_dev_for_each_child(ACPI_COMPANION(&cdev->dev),
> > +                                    check_offline, NULL);
>
> I would find this on one line better and not missing important details after
> 80th character.

I see that you've made similar comments on a few other patches.

I'll change all of them to be one line (longer that 80 chars), but
OTOH there are people who still don't like that.
diff mbox series

Patch

Index: linux-pm/drivers/acpi/container.c
===================================================================
--- linux-pm.orig/drivers/acpi/container.c
+++ linux-pm/drivers/acpi/container.c
@@ -23,17 +23,19 @@  static const struct acpi_device_id conta
 
 #ifdef CONFIG_ACPI_CONTAINER
 
-static int acpi_container_offline(struct container_dev *cdev)
+static int check_offline(struct acpi_device *adev, void *not_used)
 {
-	struct acpi_device *adev = ACPI_COMPANION(&cdev->dev);
-	struct acpi_device *child;
+	if (acpi_scan_is_offline(adev, false))
+		return 0;
 
-	/* Check all of the dependent devices' physical companions. */
-	list_for_each_entry(child, &adev->children, node)
-		if (!acpi_scan_is_offline(child, false))
-			return -EBUSY;
+	return -EBUSY;
+}
 
-	return 0;
+static int acpi_container_offline(struct container_dev *cdev)
+{
+	/* Check all of the dependent devices' physical companions. */
+	return acpi_dev_for_each_child(ACPI_COMPANION(&cdev->dev),
+				       check_offline, NULL);
 }
 
 static void acpi_container_release(struct device *dev)