All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/5] ACPI: Device enumeration rearrangements and parent field elimination
@ 2022-08-10 16:11 Rafael J. Wysocki
  2022-08-10 16:14 ` [PATCH v1 1/5] ACPI: Rename acpi_bus_get/put_acpi_device() Rafael J. Wysocki
                   ` (7 more replies)
  0 siblings, 8 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2022-08-10 16:11 UTC (permalink / raw)
  To: Linux ACPI; +Cc: LKML, Andy Shevchenko, Linux PM

Hi All,

There are still opportunities to clean up the ACPI support code and
this series is part of the effort in that direction.

It makes changes without functional impact (AFAICS) to the core ACPI
code related to devices and to some of its users.

Please refer to the patch changelogs for details.

Thanks!




^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v1 1/5] ACPI: Rename acpi_bus_get/put_acpi_device()
  2022-08-10 16:11 [PATCH v1 0/5] ACPI: Device enumeration rearrangements and parent field elimination Rafael J. Wysocki
@ 2022-08-10 16:14 ` Rafael J. Wysocki
  2022-08-10 16:30   ` Guenter Roeck
  2022-08-10 16:15 ` [PATCH v1 2/5] ACPI: scan: Rename acpi_bus_get_parent() and rearrange it Rafael J. Wysocki
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 28+ messages in thread
From: Rafael J. Wysocki @ 2022-08-10 16:14 UTC (permalink / raw)
  To: Linux ACPI
  Cc: LKML, Andy Shevchenko, Linux PM, linux-hwmon, Jean Delvare,
	Guenter Roeck

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Because acpi_bus_get_acpi_device() is completely analogous to
acpi_fetch_acpi_dev(), rename it to acpi_get_acpi_dev() and
add a kerneldoc comment to it.

Accordingly, rename acpi_bus_put_acpi_device() to acpi_put_acpi_dev()
and update all of the users of these two functions.

While at it, move the acpi_fetch_acpi_dev() header next to the
acpi_get_acpi_dev() header in the header file holding them.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/bus.c               |    6 +++---
 drivers/acpi/device_pm.c         |    4 ++--
 drivers/acpi/irq.c               |    4 ++--
 drivers/acpi/scan.c              |   21 ++++++++++++++++-----
 drivers/hwmon/acpi_power_meter.c |    2 +-
 include/acpi/acpi_bus.h          |    6 +++---
 6 files changed, 27 insertions(+), 16 deletions(-)

Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -511,7 +511,6 @@ extern int unregister_acpi_notifier(stru
  * External Functions
  */
 
-struct acpi_device *acpi_fetch_acpi_dev(acpi_handle handle);
 acpi_status acpi_bus_get_status_handle(acpi_handle handle,
 				       unsigned long long *sta);
 int acpi_bus_get_status(struct acpi_device *device);
@@ -766,9 +765,10 @@ static inline void acpi_dev_put(struct a
 		put_device(&adev->dev);
 }
 
-struct acpi_device *acpi_bus_get_acpi_device(acpi_handle handle);
+struct acpi_device *acpi_fetch_acpi_dev(acpi_handle handle);
+struct acpi_device *acpi_get_acpi_dev(acpi_handle handle);
 
-static inline void acpi_bus_put_acpi_device(struct acpi_device *adev)
+static inline void acpi_put_acpi_dev(struct acpi_device *adev)
 {
 	acpi_dev_put(adev);
 }
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -429,7 +429,7 @@ void acpi_device_hotplug(struct acpi_dev
 	acpi_evaluate_ost(adev->handle, src, ost_code, NULL);
 
  out:
-	acpi_bus_put_acpi_device(adev);
+	acpi_put_acpi_dev(adev);
 	mutex_unlock(&acpi_scan_lock);
 	unlock_device_hotplug();
 }
@@ -599,11 +599,22 @@ static void get_acpi_device(void *dev)
 	acpi_dev_get(dev);
 }
 
-struct acpi_device *acpi_bus_get_acpi_device(acpi_handle handle)
+/**
+ * acpi_get_acpi_dev - Retrieve ACPI device object and reference count it.
+ * @handle: ACPI handle associated with the requested ACPI device object.
+ *
+ * Return a pointer to the ACPI device object associated with @handle and bump
+ * up that object's reference counter (under the ACPI Namespace lock), if
+ * present, or return NULL otherwise.
+ *
+ * The ACPI device object reference acquired by this function needs to be
+ * dropped via acpi_dev_put().
+ */
+struct acpi_device *acpi_get_acpi_dev(acpi_handle handle)
 {
 	return handle_to_device(handle, get_acpi_device);
 }
-EXPORT_SYMBOL_GPL(acpi_bus_get_acpi_device);
+EXPORT_SYMBOL_GPL(acpi_get_acpi_dev);
 
 static struct acpi_device_bus_id *acpi_device_bus_id_match(const char *dev_id)
 {
@@ -2238,7 +2249,7 @@ static int acpi_dev_get_first_consumer_d
 {
 	struct acpi_device *adev;
 
-	adev = acpi_bus_get_acpi_device(dep->consumer);
+	adev = acpi_get_acpi_dev(dep->consumer);
 	if (adev) {
 		*(struct acpi_device **)data = adev;
 		return 1;
@@ -2291,7 +2302,7 @@ static bool acpi_scan_clear_dep_queue(st
 
 static int acpi_scan_clear_dep(struct acpi_dep_data *dep, void *data)
 {
-	struct acpi_device *adev = acpi_bus_get_acpi_device(dep->consumer);
+	struct acpi_device *adev = acpi_get_acpi_dev(dep->consumer);
 
 	if (adev) {
 		adev->dep_unmet--;
Index: linux-pm/drivers/acpi/bus.c
===================================================================
--- linux-pm.orig/drivers/acpi/bus.c
+++ linux-pm/drivers/acpi/bus.c
@@ -511,7 +511,7 @@ static void acpi_bus_notify(acpi_handle
 		break;
 	}
 
-	adev = acpi_bus_get_acpi_device(handle);
+	adev = acpi_get_acpi_dev(handle);
 	if (!adev)
 		goto err;
 
@@ -524,14 +524,14 @@ static void acpi_bus_notify(acpi_handle
 	}
 
 	if (!hotplug_event) {
-		acpi_bus_put_acpi_device(adev);
+		acpi_put_acpi_dev(adev);
 		return;
 	}
 
 	if (ACPI_SUCCESS(acpi_hotplug_schedule(adev, type)))
 		return;
 
-	acpi_bus_put_acpi_device(adev);
+	acpi_put_acpi_dev(adev);
 
  err:
 	acpi_evaluate_ost(handle, type, ost_code, NULL);
Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -497,7 +497,7 @@ static void acpi_pm_notify_handler(acpi_
 
 	acpi_handle_debug(handle, "Wake notify\n");
 
-	adev = acpi_bus_get_acpi_device(handle);
+	adev = acpi_get_acpi_dev(handle);
 	if (!adev)
 		return;
 
@@ -515,7 +515,7 @@ static void acpi_pm_notify_handler(acpi_
 
 	mutex_unlock(&acpi_pm_notifier_lock);
 
-	acpi_bus_put_acpi_device(adev);
+	acpi_put_acpi_dev(adev);
 }
 
 /**
Index: linux-pm/drivers/acpi/irq.c
===================================================================
--- linux-pm.orig/drivers/acpi/irq.c
+++ linux-pm/drivers/acpi/irq.c
@@ -111,12 +111,12 @@ acpi_get_irq_source_fwhandle(const struc
 	if (WARN_ON(ACPI_FAILURE(status)))
 		return NULL;
 
-	device = acpi_bus_get_acpi_device(handle);
+	device = acpi_get_acpi_dev(handle);
 	if (WARN_ON(!device))
 		return NULL;
 
 	result = &device->fwnode;
-	acpi_bus_put_acpi_device(device);
+	acpi_put_acpi_dev(device);
 	return result;
 }
 
Index: linux-pm/drivers/hwmon/acpi_power_meter.c
===================================================================
--- linux-pm.orig/drivers/hwmon/acpi_power_meter.c
+++ linux-pm/drivers/hwmon/acpi_power_meter.c
@@ -598,7 +598,7 @@ static int read_domain_devices(struct ac
 			continue;
 
 		/* Create a symlink to domain objects */
-		obj = acpi_bus_get_acpi_device(element->reference.handle);
+		obj = acpi_get_acpi_dev(element->reference.handle);
 		resource->domain_devices[i] = obj;
 		if (!obj)
 			continue;




^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v1 2/5] ACPI: scan: Rename acpi_bus_get_parent() and rearrange it
  2022-08-10 16:11 [PATCH v1 0/5] ACPI: Device enumeration rearrangements and parent field elimination Rafael J. Wysocki
  2022-08-10 16:14 ` [PATCH v1 1/5] ACPI: Rename acpi_bus_get/put_acpi_device() Rafael J. Wysocki
@ 2022-08-10 16:15 ` Rafael J. Wysocki
  2022-08-12 13:08   ` Punit Agrawal
  2022-08-10 16:16 ` [PATCH v1 3/5] ACPI: scan: Rearrange initialization of ACPI device objects Rafael J. Wysocki
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 28+ messages in thread
From: Rafael J. Wysocki @ 2022-08-10 16:15 UTC (permalink / raw)
  To: Linux ACPI; +Cc: LKML, Andy Shevchenko, Linux PM

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The acpi_bus_get_parent() name doesn't really reflect the
purpose of the function so change it to a more accurate
acpi_find_parent_acpi_dev().

While at it, rearrange the code inside that function the make it
easier to read.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/scan.c |   24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -816,10 +816,9 @@ static const char * const acpi_honor_dep
 	NULL
 };
 
-static struct acpi_device *acpi_bus_get_parent(acpi_handle handle)
+static struct acpi_device *acpi_find_parent_acpi_dev(acpi_handle handle)
 {
-	struct acpi_device *device;
-	acpi_status status;
+	struct acpi_device *adev;
 
 	/*
 	 * Fixed hardware devices do not appear in the namespace and do not
@@ -830,13 +829,18 @@ static struct acpi_device *acpi_bus_get_
 		return acpi_root;
 
 	do {
-		status = acpi_get_parent(handle, &handle);
-		if (ACPI_FAILURE(status))
-			return status == AE_NULL_ENTRY ? NULL : acpi_root;
+		acpi_status status;
 
-		device = acpi_fetch_acpi_dev(handle);
-	} while (!device);
-	return device;
+		status = acpi_get_parent(handle, &handle);
+		if (ACPI_FAILURE(status)) {
+			if (status != AE_NULL_ENTRY)
+				return acpi_root;
+
+			return NULL;
+		}
+		adev = acpi_fetch_acpi_dev(handle);
+	} while (!adev);
+	return adev;
 }
 
 acpi_status
@@ -1777,7 +1781,7 @@ void acpi_init_device_object(struct acpi
 	INIT_LIST_HEAD(&device->pnp.ids);
 	device->device_type = type;
 	device->handle = handle;
-	device->parent = acpi_bus_get_parent(handle);
+	device->parent = acpi_find_parent_acpi_dev(handle);
 	fwnode_init(&device->fwnode, &acpi_device_fwnode_ops);
 	acpi_set_device_status(device, ACPI_STA_DEFAULT);
 	acpi_device_get_busid(device);




^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v1 3/5] ACPI: scan: Rearrange initialization of ACPI device objects
  2022-08-10 16:11 [PATCH v1 0/5] ACPI: Device enumeration rearrangements and parent field elimination Rafael J. Wysocki
  2022-08-10 16:14 ` [PATCH v1 1/5] ACPI: Rename acpi_bus_get/put_acpi_device() Rafael J. Wysocki
  2022-08-10 16:15 ` [PATCH v1 2/5] ACPI: scan: Rename acpi_bus_get_parent() and rearrange it Rafael J. Wysocki
@ 2022-08-10 16:16 ` Rafael J. Wysocki
  2022-08-10 16:17 ` [PATCH v1 4/5] ACPI: scan: Eliminate __acpi_device_add() Rafael J. Wysocki
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2022-08-10 16:16 UTC (permalink / raw)
  To: Linux ACPI; +Cc: LKML, Andy Shevchenko, Linux PM

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The initialization of ACPI device objects is split between
acpi_init_device_object() and __acpi_device_add() that initializes
the dev field in struct acpi_device.  The "release" function pointer
is passed to __acpi_device_add() for this reason.

However, that split is artificial and all of the initialization can
be carried out by acpi_init_device_object(), so rearrange the code
to that end.  In particular, make acpi_init_device_object() take the
"release" pointer as an argument, along with the "type" which is
related to it, instead of __acpi_device_add().

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/internal.h |    5 ++---
 drivers/acpi/power.c    |    5 +++--
 drivers/acpi/scan.c     |   27 ++++++++++++++-------------
 3 files changed, 19 insertions(+), 18 deletions(-)

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -673,8 +673,7 @@ static void acpi_store_pld_crc(struct ac
 	ACPI_FREE(pld);
 }
 
-static int __acpi_device_add(struct acpi_device *device,
-			     void (*release)(struct device *))
+static int __acpi_device_add(struct acpi_device *device)
 {
 	struct acpi_device_bus_id *acpi_device_bus_id;
 	int result;
@@ -730,11 +729,6 @@ static int __acpi_device_add(struct acpi
 
 	mutex_unlock(&acpi_device_lock);
 
-	if (device->parent)
-		device->dev.parent = &device->parent->dev;
-
-	device->dev.bus = &acpi_bus_type;
-	device->dev.release = release;
 	result = device_add(&device->dev);
 	if (result) {
 		dev_err(&device->dev, "Error registering device\n");
@@ -761,7 +755,7 @@ err_unlock:
 	return result;
 }
 
-int acpi_device_add(struct acpi_device *adev, void (*release)(struct device *))
+int acpi_device_add(struct acpi_device *adev)
 {
 	int ret;
 
@@ -769,7 +763,7 @@ int acpi_device_add(struct acpi_device *
 	if (ret)
 		return ret;
 
-	return __acpi_device_add(adev, release);
+	return __acpi_device_add(adev);
 }
 
 /* --------------------------------------------------------------------------
@@ -1776,12 +1770,19 @@ static bool acpi_device_enumeration_by_p
 }
 
 void acpi_init_device_object(struct acpi_device *device, acpi_handle handle,
-			     int type)
+			     int type, void (*release)(struct device *))
 {
+	struct acpi_device *parent = acpi_find_parent_acpi_dev(handle);
+
 	INIT_LIST_HEAD(&device->pnp.ids);
 	device->device_type = type;
 	device->handle = handle;
-	device->parent = acpi_find_parent_acpi_dev(handle);
+	if (parent) {
+		device->parent = parent;
+		device->dev.parent = &parent->dev;
+	}
+	device->dev.release = release;
+	device->dev.bus = &acpi_bus_type;
 	fwnode_init(&device->fwnode, &acpi_device_fwnode_ops);
 	acpi_set_device_status(device, ACPI_STA_DEFAULT);
 	acpi_device_get_busid(device);
@@ -1835,7 +1836,7 @@ static int acpi_add_single_object(struct
 	if (!device)
 		return -ENOMEM;
 
-	acpi_init_device_object(device, handle, type);
+	acpi_init_device_object(device, handle, type, acpi_device_release);
 	/*
 	 * Getting the status is delayed till here so that we can call
 	 * acpi_bus_get_status() and use its quirk handling.  Note that
@@ -1865,7 +1866,7 @@ static int acpi_add_single_object(struct
 		mutex_unlock(&acpi_dep_list_lock);
 
 	if (!result)
-		result = __acpi_device_add(device, acpi_device_release);
+		result = __acpi_device_add(device);
 
 	if (result) {
 		acpi_device_release(&device->dev);
Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -102,10 +102,9 @@ struct acpi_device_bus_id {
 	struct list_head node;
 };
 
-int acpi_device_add(struct acpi_device *device,
-		    void (*release)(struct device *));
 void acpi_init_device_object(struct acpi_device *device, acpi_handle handle,
-			     int type);
+			     int type, void (*release)(struct device *));
+int acpi_device_add(struct acpi_device *device);
 int acpi_device_setup_files(struct acpi_device *dev);
 void acpi_device_remove_files(struct acpi_device *dev);
 void acpi_device_add_finalize(struct acpi_device *device);
Index: linux-pm/drivers/acpi/power.c
===================================================================
--- linux-pm.orig/drivers/acpi/power.c
+++ linux-pm/drivers/acpi/power.c
@@ -944,7 +944,8 @@ struct acpi_device *acpi_add_power_resou
 		return NULL;
 
 	device = &resource->device;
-	acpi_init_device_object(device, handle, ACPI_BUS_TYPE_POWER);
+	acpi_init_device_object(device, handle, ACPI_BUS_TYPE_POWER,
+				acpi_release_power_resource);
 	mutex_init(&resource->resource_lock);
 	INIT_LIST_HEAD(&resource->list_node);
 	INIT_LIST_HEAD(&resource->dependents);
@@ -968,7 +969,7 @@ struct acpi_device *acpi_add_power_resou
 	pr_info("%s [%s]\n", acpi_device_name(device), acpi_device_bid(device));
 
 	device->flags.match_driver = true;
-	result = acpi_device_add(device, acpi_release_power_resource);
+	result = acpi_device_add(device);
 	if (result)
 		goto err;
 




^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v1 4/5] ACPI: scan: Eliminate __acpi_device_add()
  2022-08-10 16:11 [PATCH v1 0/5] ACPI: Device enumeration rearrangements and parent field elimination Rafael J. Wysocki
                   ` (2 preceding siblings ...)
  2022-08-10 16:16 ` [PATCH v1 3/5] ACPI: scan: Rearrange initialization of ACPI device objects Rafael J. Wysocki
@ 2022-08-10 16:17 ` Rafael J. Wysocki
  2022-08-10 16:23 ` [PATCH v1 5/5][RFT] ACPI: Drop parent field from struct acpi_device Rafael J. Wysocki
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2022-08-10 16:17 UTC (permalink / raw)
  To: Linux ACPI; +Cc: LKML, Andy Shevchenko, Linux PM

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Instead of having acpi_device_add() defined as a wrapper around
__acpi_device_add(), export acpi_tie_acpi_dev() so it can be called
directly by acpi_add_power_resource(), fold acpi_device_add() into the
latter and rename __acpi_device_add() to acpi_device_add().

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/internal.h |    1 +
 drivers/acpi/power.c    |    6 +++++-
 drivers/acpi/scan.c     |   17 +++--------------
 3 files changed, 9 insertions(+), 15 deletions(-)

Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -104,6 +104,7 @@ struct acpi_device_bus_id {
 
 void acpi_init_device_object(struct acpi_device *device, acpi_handle handle,
 			     int type, void (*release)(struct device *));
+int acpi_tie_acpi_dev(struct acpi_device *adev);
 int acpi_device_add(struct acpi_device *device);
 int acpi_device_setup_files(struct acpi_device *dev);
 void acpi_device_remove_files(struct acpi_device *dev);
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -643,7 +643,7 @@ static int acpi_device_set_name(struct a
 	return 0;
 }
 
-static int acpi_tie_acpi_dev(struct acpi_device *adev)
+int acpi_tie_acpi_dev(struct acpi_device *adev)
 {
 	acpi_handle handle = adev->handle;
 	acpi_status status;
@@ -673,7 +673,7 @@ static void acpi_store_pld_crc(struct ac
 	ACPI_FREE(pld);
 }
 
-static int __acpi_device_add(struct acpi_device *device)
+int acpi_device_add(struct acpi_device *device)
 {
 	struct acpi_device_bus_id *acpi_device_bus_id;
 	int result;
@@ -755,17 +755,6 @@ err_unlock:
 	return result;
 }
 
-int acpi_device_add(struct acpi_device *adev)
-{
-	int ret;
-
-	ret = acpi_tie_acpi_dev(adev);
-	if (ret)
-		return ret;
-
-	return __acpi_device_add(adev);
-}
-
 /* --------------------------------------------------------------------------
                                  Device Enumeration
    -------------------------------------------------------------------------- */
@@ -1866,7 +1855,7 @@ static int acpi_add_single_object(struct
 		mutex_unlock(&acpi_dep_list_lock);
 
 	if (!result)
-		result = __acpi_device_add(device);
+		result = acpi_device_add(device);
 
 	if (result) {
 		acpi_device_release(&device->dev);
Index: linux-pm/drivers/acpi/power.c
===================================================================
--- linux-pm.orig/drivers/acpi/power.c
+++ linux-pm/drivers/acpi/power.c
@@ -952,6 +952,7 @@ struct acpi_device *acpi_add_power_resou
 	strcpy(acpi_device_name(device), ACPI_POWER_DEVICE_NAME);
 	strcpy(acpi_device_class(device), ACPI_POWER_CLASS);
 	device->power.state = ACPI_STATE_UNKNOWN;
+	device->flags.match_driver = true;
 
 	/* Evaluate the object to get the system level and resource order. */
 	status = acpi_evaluate_object(handle, NULL, NULL, &buffer);
@@ -968,7 +969,10 @@ struct acpi_device *acpi_add_power_resou
 
 	pr_info("%s [%s]\n", acpi_device_name(device), acpi_device_bid(device));
 
-	device->flags.match_driver = true;
+	result = acpi_tie_acpi_dev(device);
+	if (result)
+		goto err;
+
 	result = acpi_device_add(device);
 	if (result)
 		goto err;




^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v1 5/5][RFT] ACPI: Drop parent field from struct acpi_device
  2022-08-10 16:11 [PATCH v1 0/5] ACPI: Device enumeration rearrangements and parent field elimination Rafael J. Wysocki
                   ` (3 preceding siblings ...)
  2022-08-10 16:17 ` [PATCH v1 4/5] ACPI: scan: Eliminate __acpi_device_add() Rafael J. Wysocki
@ 2022-08-10 16:23 ` Rafael J. Wysocki
  2022-08-10 16:33   ` Mark Brown
                     ` (3 more replies)
  2022-08-12 13:11 ` [PATCH v1 0/5] ACPI: Device enumeration rearrangements and parent field elimination Punit Agrawal
                   ` (2 subsequent siblings)
  7 siblings, 4 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2022-08-10 16:23 UTC (permalink / raw)
  To: Linux ACPI
  Cc: LKML, Andy Shevchenko, Linux PM, K. Y. Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Wei Liu, Dexuan Cui, Mark Brown,
	Andreas Noever, Michael Jamet, Mika Westerberg, Yehezkel Bernat,
	linux-hyperv, linux-spi, linux-usb

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The parent field in struct acpi_device is, in fact, redundant,
because the dev.parent field in it effectively points to the same
object and it is used by the driver core.

Accordingly, the parent field can be dropped from struct acpi_device
and for this purpose define acpi_dev_parent() to retrieve the parent
struct acpi_device pointer from the dev.parent field in struct
acpi_device.  Next, update all of the users of the parent field
in struct acpi_device to use acpi_dev_parent() instead of it and
drop it.

While at it, drop the ACPI_IS_ROOT_DEVICE() macro that is only used
in one place in a confusing way.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

I may have missed some places where adev->parent is used directly, so
if that's happened, please let me know (I'm assuming that 0-day will
pick up this patch and run it through all of the relevant configs
anyway).

---
 drivers/acpi/acpi_platform.c |    6 +++---
 drivers/acpi/acpi_video.c    |    2 +-
 drivers/acpi/device_pm.c     |   19 ++++++++++---------
 drivers/acpi/property.c      |    6 ++++--
 drivers/acpi/sbs.c           |    2 +-
 drivers/acpi/sbshc.c         |    2 +-
 drivers/acpi/scan.c          |   17 ++++++-----------
 drivers/hv/vmbus_drv.c       |    3 ++-
 drivers/spi/spi.c            |    2 +-
 drivers/thunderbolt/acpi.c   |    2 +-
 include/acpi/acpi_bus.h      |   10 +++++++++-
 11 files changed, 39 insertions(+), 32 deletions(-)

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -29,8 +29,6 @@ extern struct acpi_device *acpi_root;
 #define ACPI_BUS_HID			"LNXSYBUS"
 #define ACPI_BUS_DEVICE_NAME		"System Bus"
 
-#define ACPI_IS_ROOT_DEVICE(device)    (!(device)->parent)
-
 #define INVALID_ACPI_HANDLE	((acpi_handle)empty_zero_page)
 
 static const char *dummy_hid = "device";
@@ -1110,7 +1108,7 @@ static void acpi_device_get_busid(struct
 	 * The device's Bus ID is simply the object name.
 	 * TBD: Shouldn't this value be unique (within the ACPI namespace)?
 	 */
-	if (ACPI_IS_ROOT_DEVICE(device)) {
+	if (!acpi_dev_parent(device)) {
 		strcpy(device->pnp.bus_id, "ACPI");
 		return;
 	}
@@ -1646,7 +1644,7 @@ static void acpi_init_coherency(struct a
 {
 	unsigned long long cca = 0;
 	acpi_status status;
-	struct acpi_device *parent = adev->parent;
+	struct acpi_device *parent = acpi_dev_parent(adev);
 
 	if (parent && parent->flags.cca_seen) {
 		/*
@@ -1690,7 +1688,7 @@ static int acpi_check_serial_bus_slave(s
 
 static bool acpi_is_indirect_io_slave(struct acpi_device *device)
 {
-	struct acpi_device *parent = device->parent;
+	struct acpi_device *parent = acpi_dev_parent(device);
 	static const struct acpi_device_id indirect_io_hosts[] = {
 		{"HISI0191", 0},
 		{}
@@ -1766,10 +1764,7 @@ void acpi_init_device_object(struct acpi
 	INIT_LIST_HEAD(&device->pnp.ids);
 	device->device_type = type;
 	device->handle = handle;
-	if (parent) {
-		device->parent = parent;
-		device->dev.parent = &parent->dev;
-	}
+	device->dev.parent = parent ? &parent->dev : NULL;
 	device->dev.release = release;
 	device->dev.bus = &acpi_bus_type;
 	fwnode_init(&device->fwnode, &acpi_device_fwnode_ops);
@@ -1866,8 +1861,8 @@ static int acpi_add_single_object(struct
 	acpi_device_add_finalize(device);
 
 	acpi_handle_debug(handle, "Added as %s, parent %s\n",
-			  dev_name(&device->dev), device->parent ?
-				dev_name(&device->parent->dev) : "(null)");
+			  dev_name(&device->dev), device->dev.parent ?
+				dev_name(device->dev.parent) : "(null)");
 
 	*child = device;
 	return 0;
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -364,7 +364,6 @@ struct acpi_device {
 	int device_type;
 	acpi_handle handle;		/* no handle for fixed hardware */
 	struct fwnode_handle fwnode;
-	struct acpi_device *parent;
 	struct list_head wakeup_list;
 	struct list_head del_list;
 	struct acpi_device_status status;
@@ -457,6 +456,14 @@ static inline void *acpi_driver_data(str
 #define to_acpi_device(d)	container_of(d, struct acpi_device, dev)
 #define to_acpi_driver(d)	container_of(d, struct acpi_driver, drv)
 
+static inline struct acpi_device *acpi_dev_parent(struct acpi_device *adev)
+{
+	if (adev->dev.parent)
+		return to_acpi_device(adev->dev.parent);
+
+	return NULL;
+}
+
 static inline void acpi_set_device_status(struct acpi_device *adev, u32 sta)
 {
 	*((u32 *)&adev->status) = sta;
@@ -477,6 +484,7 @@ void acpi_initialize_hp_context(struct a
 /* acpi_device.dev.bus == &acpi_bus_type */
 extern struct bus_type acpi_bus_type;
 
+struct acpi_device *acpi_dev_parent(struct acpi_device *adev);
 int acpi_bus_for_each_dev(int (*fn)(struct device *, void *), void *data);
 int acpi_dev_for_each_child(struct acpi_device *adev,
 			    int (*fn)(struct acpi_device *, void *), void *data);
Index: linux-pm/drivers/acpi/property.c
===================================================================
--- linux-pm.orig/drivers/acpi/property.c
+++ linux-pm/drivers/acpi/property.c
@@ -298,8 +298,10 @@ static void acpi_init_of_compatible(stru
 		ret = acpi_dev_get_property(adev, "compatible",
 					    ACPI_TYPE_STRING, &of_compatible);
 		if (ret) {
-			if (adev->parent
-			    && adev->parent->flags.of_compatible_ok)
+			struct acpi_device *parent;
+
+			parent = acpi_dev_parent(adev);
+			if (parent && parent->flags.of_compatible_ok)
 				goto out;
 
 			return;
Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -74,6 +74,7 @@ static int acpi_dev_pm_explicit_get(stru
  */
 int acpi_device_get_power(struct acpi_device *device, int *state)
 {
+	struct acpi_device *parent = acpi_dev_parent(device);
 	int result = ACPI_STATE_UNKNOWN;
 	int error;
 
@@ -82,8 +83,7 @@ int acpi_device_get_power(struct acpi_de
 
 	if (!device->flags.power_manageable) {
 		/* TBD: Non-recursive algorithm for walking up hierarchy. */
-		*state = device->parent ?
-			device->parent->power.state : ACPI_STATE_D0;
+		*state = parent ? parent->power.state : ACPI_STATE_D0;
 		goto out;
 	}
 
@@ -122,10 +122,10 @@ int acpi_device_get_power(struct acpi_de
 	 * point, the fact that the device is in D0 implies that the parent has
 	 * to be in D0 too, except if ignore_parent is set.
 	 */
-	if (!device->power.flags.ignore_parent && device->parent
-	    && device->parent->power.state == ACPI_STATE_UNKNOWN
-	    && result == ACPI_STATE_D0)
-		device->parent->power.state = ACPI_STATE_D0;
+	if (!device->power.flags.ignore_parent && parent &&
+	    parent->power.state == ACPI_STATE_UNKNOWN &&
+	    result == ACPI_STATE_D0)
+		parent->power.state = ACPI_STATE_D0;
 
 	*state = result;
 
@@ -159,6 +159,7 @@ static int acpi_dev_pm_explicit_set(stru
  */
 int acpi_device_set_power(struct acpi_device *device, int state)
 {
+	struct acpi_device *parent = acpi_dev_parent(device);
 	int target_state = state;
 	int result = 0;
 
@@ -191,12 +192,12 @@ int acpi_device_set_power(struct acpi_de
 		return -ENODEV;
 	}
 
-	if (!device->power.flags.ignore_parent && device->parent &&
-	    state < device->parent->power.state) {
+	if (!device->power.flags.ignore_parent && parent &&
+	    state < parent->power.state) {
 		acpi_handle_debug(device->handle,
 				  "Cannot transition to %s for parent in %s\n",
 				  acpi_power_state_string(state),
-				  acpi_power_state_string(device->parent->power.state));
+				  acpi_power_state_string(parent->power.state));
 		return -ENODEV;
 	}
 
Index: linux-pm/drivers/acpi/acpi_platform.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpi_platform.c
+++ linux-pm/drivers/acpi/acpi_platform.c
@@ -78,7 +78,7 @@ static void acpi_platform_fill_resource(
 	 * If the device has parent we need to take its resources into
 	 * account as well because this device might consume part of those.
 	 */
-	parent = acpi_get_first_physical_node(adev->parent);
+	parent = acpi_get_first_physical_node(acpi_dev_parent(adev));
 	if (parent && dev_is_pci(parent))
 		dest->parent = pci_find_resource(to_pci_dev(parent), dest);
 }
@@ -97,6 +97,7 @@ static void acpi_platform_fill_resource(
 struct platform_device *acpi_create_platform_device(struct acpi_device *adev,
 						    const struct property_entry *properties)
 {
+	struct acpi_device *parent = acpi_dev_parent(adev);
 	struct platform_device *pdev = NULL;
 	struct platform_device_info pdevinfo;
 	struct resource_entry *rentry;
@@ -137,8 +138,7 @@ struct platform_device *acpi_create_plat
 	 * attached to it, that physical device should be the parent of the
 	 * platform device we are about to create.
 	 */
-	pdevinfo.parent = adev->parent ?
-		acpi_get_first_physical_node(adev->parent) : NULL;
+	pdevinfo.parent = parent ? acpi_get_first_physical_node(parent) : NULL;
 	pdevinfo.name = dev_name(&adev->dev);
 	pdevinfo.id = -1;
 	pdevinfo.res = resources;
Index: linux-pm/drivers/acpi/acpi_video.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpi_video.c
+++ linux-pm/drivers/acpi/acpi_video.c
@@ -2030,7 +2030,7 @@ static int acpi_video_bus_add(struct acp
 	acpi_status status;
 
 	status = acpi_walk_namespace(ACPI_TYPE_DEVICE,
-				device->parent->handle, 1,
+				acpi_dev_parent(device)->handle, 1,
 				acpi_video_bus_match, NULL,
 				device, NULL);
 	if (status == AE_ALREADY_EXISTS) {
Index: linux-pm/drivers/acpi/sbs.c
===================================================================
--- linux-pm.orig/drivers/acpi/sbs.c
+++ linux-pm/drivers/acpi/sbs.c
@@ -632,7 +632,7 @@ static int acpi_sbs_add(struct acpi_devi
 
 	mutex_init(&sbs->lock);
 
-	sbs->hc = acpi_driver_data(device->parent);
+	sbs->hc = acpi_driver_data(acpi_dev_parent(device));
 	sbs->device = device;
 	strcpy(acpi_device_name(device), ACPI_SBS_DEVICE_NAME);
 	strcpy(acpi_device_class(device), ACPI_SBS_CLASS);
Index: linux-pm/drivers/acpi/sbshc.c
===================================================================
--- linux-pm.orig/drivers/acpi/sbshc.c
+++ linux-pm/drivers/acpi/sbshc.c
@@ -266,7 +266,7 @@ static int acpi_smbus_hc_add(struct acpi
 	mutex_init(&hc->lock);
 	init_waitqueue_head(&hc->wait);
 
-	hc->ec = acpi_driver_data(device->parent);
+	hc->ec = acpi_driver_data(acpi_dev_parent(device));
 	hc->offset = (val >> 8) & 0xff;
 	hc->query_bit = val & 0xff;
 	device->driver_data = hc;
Index: linux-pm/drivers/spi/spi.c
===================================================================
--- linux-pm.orig/drivers/spi/spi.c
+++ linux-pm/drivers/spi/spi.c
@@ -4256,7 +4256,7 @@ static int acpi_spi_notify(struct notifi
 
 	switch (value) {
 	case ACPI_RECONFIG_DEVICE_ADD:
-		ctlr = acpi_spi_find_controller_by_adev(adev->parent);
+		ctlr = acpi_spi_find_controller_by_adev(acpi_dev_parent(adev));
 		if (!ctlr)
 			break;
 
Index: linux-pm/drivers/thunderbolt/acpi.c
===================================================================
--- linux-pm.orig/drivers/thunderbolt/acpi.c
+++ linux-pm/drivers/thunderbolt/acpi.c
@@ -42,7 +42,7 @@ static acpi_status tb_acpi_add_link(acpi
 	 */
 	dev = acpi_get_first_physical_node(adev);
 	while (!dev) {
-		adev = adev->parent;
+		adev = acpi_dev_parent(adev);
 		if (!adev)
 			break;
 		dev = acpi_get_first_physical_node(adev);
Index: linux-pm/drivers/hv/vmbus_drv.c
===================================================================
--- linux-pm.orig/drivers/hv/vmbus_drv.c
+++ linux-pm/drivers/hv/vmbus_drv.c
@@ -2423,7 +2423,8 @@ static int vmbus_acpi_add(struct acpi_de
 	 * Some ancestor of the vmbus acpi device (Gen1 or Gen2
 	 * firmware) is the VMOD that has the mmio ranges. Get that.
 	 */
-	for (ancestor = device->parent; ancestor; ancestor = ancestor->parent) {
+	for (ancestor = acpi_dev_parent(device); ancestor;
+	     ancestor = acpi_dev_parent(ancestor)) {
 		result = acpi_walk_resources(ancestor->handle, METHOD_NAME__CRS,
 					     vmbus_walk_resources, NULL);
 




^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v1 1/5] ACPI: Rename acpi_bus_get/put_acpi_device()
  2022-08-10 16:14 ` [PATCH v1 1/5] ACPI: Rename acpi_bus_get/put_acpi_device() Rafael J. Wysocki
@ 2022-08-10 16:30   ` Guenter Roeck
  0 siblings, 0 replies; 28+ messages in thread
From: Guenter Roeck @ 2022-08-10 16:30 UTC (permalink / raw)
  To: Rafael J. Wysocki, Linux ACPI
  Cc: LKML, Andy Shevchenko, Linux PM, linux-hwmon, Jean Delvare

On 8/10/22 09:14, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Because acpi_bus_get_acpi_device() is completely analogous to
> acpi_fetch_acpi_dev(), rename it to acpi_get_acpi_dev() and
> add a kerneldoc comment to it.
> 
> Accordingly, rename acpi_bus_put_acpi_device() to acpi_put_acpi_dev()
> and update all of the users of these two functions.
> 
> While at it, move the acpi_fetch_acpi_dev() header next to the
> acpi_get_acpi_dev() header in the header file holding them.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>   drivers/acpi/bus.c               |    6 +++---
>   drivers/acpi/device_pm.c         |    4 ++--
>   drivers/acpi/irq.c               |    4 ++--
>   drivers/acpi/scan.c              |   21 ++++++++++++++++-----
>   drivers/hwmon/acpi_power_meter.c |    2 +-

For hwmon:

Acked-by: Guenter Roeck <linux@roeck-us.net>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v1 5/5][RFT] ACPI: Drop parent field from struct acpi_device
  2022-08-10 16:23 ` [PATCH v1 5/5][RFT] ACPI: Drop parent field from struct acpi_device Rafael J. Wysocki
@ 2022-08-10 16:33   ` Mark Brown
  2022-08-10 17:10   ` Mika Westerberg
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 28+ messages in thread
From: Mark Brown @ 2022-08-10 16:33 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux ACPI, LKML, Andy Shevchenko, Linux PM, K. Y. Srinivasan,
	Haiyang Zhang, Stephen Hemminger, Wei Liu, Dexuan Cui,
	Andreas Noever, Michael Jamet, Mika Westerberg, Yehezkel Bernat,
	linux-hyperv, linux-spi, linux-usb

[-- Attachment #1: Type: text/plain, Size: 353 bytes --]

On Wed, Aug 10, 2022 at 06:23:05PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> The parent field in struct acpi_device is, in fact, redundant,
> because the dev.parent field in it effectively points to the same
> object and it is used by the driver core.

Acked-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v1 5/5][RFT] ACPI: Drop parent field from struct acpi_device
  2022-08-10 16:23 ` [PATCH v1 5/5][RFT] ACPI: Drop parent field from struct acpi_device Rafael J. Wysocki
  2022-08-10 16:33   ` Mark Brown
@ 2022-08-10 17:10   ` Mika Westerberg
  2022-08-12 15:14   ` Wei Liu
  2022-08-24 16:59     ` Rafael J. Wysocki
  3 siblings, 0 replies; 28+ messages in thread
From: Mika Westerberg @ 2022-08-10 17:10 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux ACPI, LKML, Andy Shevchenko, Linux PM, K. Y. Srinivasan,
	Haiyang Zhang, Stephen Hemminger, Wei Liu, Dexuan Cui,
	Mark Brown, Andreas Noever, Michael Jamet, Yehezkel Bernat,
	linux-hyperv, linux-spi, linux-usb

On Wed, Aug 10, 2022 at 06:23:05PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> The parent field in struct acpi_device is, in fact, redundant,
> because the dev.parent field in it effectively points to the same
> object and it is used by the driver core.
> 
> Accordingly, the parent field can be dropped from struct acpi_device
> and for this purpose define acpi_dev_parent() to retrieve the parent
> struct acpi_device pointer from the dev.parent field in struct
> acpi_device.  Next, update all of the users of the parent field
> in struct acpi_device to use acpi_dev_parent() instead of it and
> drop it.
> 
> While at it, drop the ACPI_IS_ROOT_DEVICE() macro that is only used
> in one place in a confusing way.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> I may have missed some places where adev->parent is used directly, so
> if that's happened, please let me know (I'm assuming that 0-day will
> pick up this patch and run it through all of the relevant configs
> anyway).
> 
> ---
>  drivers/acpi/acpi_platform.c |    6 +++---
>  drivers/acpi/acpi_video.c    |    2 +-
>  drivers/acpi/device_pm.c     |   19 ++++++++++---------
>  drivers/acpi/property.c      |    6 ++++--
>  drivers/acpi/sbs.c           |    2 +-
>  drivers/acpi/sbshc.c         |    2 +-
>  drivers/acpi/scan.c          |   17 ++++++-----------
>  drivers/hv/vmbus_drv.c       |    3 ++-
>  drivers/spi/spi.c            |    2 +-
>  drivers/thunderbolt/acpi.c   |    2 +-

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v1 2/5] ACPI: scan: Rename acpi_bus_get_parent() and rearrange it
  2022-08-10 16:15 ` [PATCH v1 2/5] ACPI: scan: Rename acpi_bus_get_parent() and rearrange it Rafael J. Wysocki
@ 2022-08-12 13:08   ` Punit Agrawal
  0 siblings, 0 replies; 28+ messages in thread
From: Punit Agrawal @ 2022-08-12 13:08 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Linux ACPI, LKML, Andy Shevchenko, Linux PM

"Rafael J. Wysocki" <rjw@rjwysocki.net> writes:

> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> The acpi_bus_get_parent() name doesn't really reflect the
> purpose of the function so change it to a more accurate
> acpi_find_parent_acpi_dev().
>
> While at it, rearrange the code inside that function the make it

Typo:                                                  to

> easier to read.
>
> No intentional functional impact.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/acpi/scan.c |   24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)
>
> Index: linux-pm/drivers/acpi/scan.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/scan.c
> +++ linux-pm/drivers/acpi/scan.c
> @@ -816,10 +816,9 @@ static const char * const acpi_honor_dep
>  	NULL
>  };
>  
> -static struct acpi_device *acpi_bus_get_parent(acpi_handle handle)
> +static struct acpi_device *acpi_find_parent_acpi_dev(acpi_handle handle)
>  {
> -	struct acpi_device *device;
> -	acpi_status status;
> +	struct acpi_device *adev;
>  
>  	/*
>  	 * Fixed hardware devices do not appear in the namespace and do not
> @@ -830,13 +829,18 @@ static struct acpi_device *acpi_bus_get_
>  		return acpi_root;
>  
>  	do {
> -		status = acpi_get_parent(handle, &handle);
> -		if (ACPI_FAILURE(status))
> -			return status == AE_NULL_ENTRY ? NULL : acpi_root;
> +		acpi_status status;
>  
> -		device = acpi_fetch_acpi_dev(handle);
> -	} while (!device);
> -	return device;
> +		status = acpi_get_parent(handle, &handle);
> +		if (ACPI_FAILURE(status)) {
> +			if (status != AE_NULL_ENTRY)
> +				return acpi_root;
> +
> +			return NULL;
> +		}
> +		adev = acpi_fetch_acpi_dev(handle);
> +	} while (!adev);
> +	return adev;
>  }
>  
>  acpi_status
> @@ -1777,7 +1781,7 @@ void acpi_init_device_object(struct acpi
>  	INIT_LIST_HEAD(&device->pnp.ids);
>  	device->device_type = type;
>  	device->handle = handle;
> -	device->parent = acpi_bus_get_parent(handle);
> +	device->parent = acpi_find_parent_acpi_dev(handle);
>  	fwnode_init(&device->fwnode, &acpi_device_fwnode_ops);
>  	acpi_set_device_status(device, ACPI_STA_DEFAULT);
>  	acpi_device_get_busid(device);

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v1 0/5] ACPI: Device enumeration rearrangements and parent field elimination
  2022-08-10 16:11 [PATCH v1 0/5] ACPI: Device enumeration rearrangements and parent field elimination Rafael J. Wysocki
                   ` (4 preceding siblings ...)
  2022-08-10 16:23 ` [PATCH v1 5/5][RFT] ACPI: Drop parent field from struct acpi_device Rafael J. Wysocki
@ 2022-08-12 13:11 ` Punit Agrawal
  2022-08-23 16:25   ` Rafael J. Wysocki
  2022-08-29 15:21 ` [PATCH v1] ACPI: PM: Fix NULL argument handling in acpi_device_get/set_power() Rafael J. Wysocki
  2022-08-29 15:53 ` [PATCH v1] ACPI: Drop redundant acpi_dev_parent() header Rafael J. Wysocki
  7 siblings, 1 reply; 28+ messages in thread
From: Punit Agrawal @ 2022-08-12 13:11 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Linux ACPI, LKML, Andy Shevchenko, Linux PM

Hi Rafael,

"Rafael J. Wysocki" <rjw@rjwysocki.net> writes:

> Hi All,
>
> There are still opportunities to clean up the ACPI support code and
> this series is part of the effort in that direction.
>
> It makes changes without functional impact (AFAICS) to the core ACPI
> code related to devices and to some of its users.
>
> Please refer to the patch changelogs for details.

Other than the single typo I noticed in Patch 2, the changes look good!

If it helps,

Reviewed-by: Punit Agrawal <punit.agrawal@bytedance.com>

Thanks,
Punit

[...]


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v1 5/5][RFT] ACPI: Drop parent field from struct acpi_device
  2022-08-10 16:23 ` [PATCH v1 5/5][RFT] ACPI: Drop parent field from struct acpi_device Rafael J. Wysocki
  2022-08-10 16:33   ` Mark Brown
  2022-08-10 17:10   ` Mika Westerberg
@ 2022-08-12 15:14   ` Wei Liu
  2022-08-24 16:59     ` Rafael J. Wysocki
  3 siblings, 0 replies; 28+ messages in thread
From: Wei Liu @ 2022-08-12 15:14 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux ACPI, LKML, Andy Shevchenko, Linux PM, K. Y. Srinivasan,
	Haiyang Zhang, Stephen Hemminger, Wei Liu, Dexuan Cui,
	Mark Brown, Andreas Noever, Michael Jamet, Mika Westerberg,
	Yehezkel Bernat, linux-hyperv, linux-spi, linux-usb

On Wed, Aug 10, 2022 at 06:23:05PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> The parent field in struct acpi_device is, in fact, redundant,
> because the dev.parent field in it effectively points to the same
> object and it is used by the driver core.
> 
> Accordingly, the parent field can be dropped from struct acpi_device
> and for this purpose define acpi_dev_parent() to retrieve the parent
> struct acpi_device pointer from the dev.parent field in struct
> acpi_device.  Next, update all of the users of the parent field
> in struct acpi_device to use acpi_dev_parent() instead of it and
> drop it.
> 
> While at it, drop the ACPI_IS_ROOT_DEVICE() macro that is only used
> in one place in a confusing way.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> I may have missed some places where adev->parent is used directly, so
> if that's happened, please let me know (I'm assuming that 0-day will
> pick up this patch and run it through all of the relevant configs
> anyway).
> 
> ---
[...]
>  drivers/hv/vmbus_drv.c       |    3 ++-

Acked-by: Wei Liu <wei.liu@kernel.org>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v1 0/5] ACPI: Device enumeration rearrangements and parent field elimination
  2022-08-12 13:11 ` [PATCH v1 0/5] ACPI: Device enumeration rearrangements and parent field elimination Punit Agrawal
@ 2022-08-23 16:25   ` Rafael J. Wysocki
  0 siblings, 0 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2022-08-23 16:25 UTC (permalink / raw)
  To: Punit Agrawal
  Cc: Rafael J. Wysocki, Linux ACPI, LKML, Andy Shevchenko, Linux PM

On Fri, Aug 12, 2022 at 3:11 PM Punit Agrawal
<punit.agrawal@bytedance.com> wrote:
>
> Hi Rafael,
>
> "Rafael J. Wysocki" <rjw@rjwysocki.net> writes:
>
> > Hi All,
> >
> > There are still opportunities to clean up the ACPI support code and
> > this series is part of the effort in that direction.
> >
> > It makes changes without functional impact (AFAICS) to the core ACPI
> > code related to devices and to some of its users.
> >
> > Please refer to the patch changelogs for details.
>
> Other than the single typo I noticed in Patch 2,

I've fixed that one while applying the patch.

> the changes look good!
>
> If it helps,
>
> Reviewed-by: Punit Agrawal <punit.agrawal@bytedance.com>

Thank you!

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2 5/5] ACPI: Drop parent field from struct acpi_device
  2022-08-10 16:23 ` [PATCH v1 5/5][RFT] ACPI: Drop parent field from struct acpi_device Rafael J. Wysocki
@ 2022-08-24 16:59     ` Rafael J. Wysocki
  2022-08-10 17:10   ` Mika Westerberg
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2022-08-24 16:59 UTC (permalink / raw)
  To: Linux ACPI
  Cc: LKML, Andy Shevchenko, Linux PM, K. Y. Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Wei Liu, Dexuan Cui, Mark Brown,
	Andreas Noever, Michael Jamet, Mika Westerberg, Yehezkel Bernat,
	linux-hyperv, linux-spi, linux-usb, linux-arm-kernel,
	linux-arm-msm, Will Deacon, Mark Rutland, Andy Gross,
	Bjorn Andersson, Konrad Dybcio

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The parent field in struct acpi_device is, in fact, redundant,
because the dev.parent field in it effectively points to the same
object and it is used by the driver core.

Accordingly, the parent field can be dropped from struct acpi_device
and for this purpose define acpi_dev_parent() to retrieve a parent
struct acpi_device pointer from the dev.parent field in struct
acpi_device.  Next, update all of the users of the parent field
in struct acpi_device to use acpi_dev_parent() instead of it and
drop it.

While at it, drop the ACPI_IS_ROOT_DEVICE() macro that is only used
in one place in a confusing way.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Acked-by: Mark Brown <broonie@kernel.org>
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Acked-by: Wei Liu <wei.liu@kernel.org>
Reviewed-by: Punit Agrawal <punit.agrawal@bytedance.com>
---

v1 -> v2:
   * Cover 3 more users of the parent field in struct acpi_device.
   * Add tags collected so far.

---
 drivers/acpi/acpi_amba.c     |    5 +++--
 drivers/acpi/acpi_platform.c |    6 +++---
 drivers/acpi/acpi_video.c    |    2 +-
 drivers/acpi/device_pm.c     |   19 ++++++++++---------
 drivers/acpi/property.c      |    6 ++++--
 drivers/acpi/sbs.c           |    2 +-
 drivers/acpi/sbshc.c         |    2 +-
 drivers/acpi/scan.c          |   17 ++++++-----------
 drivers/hv/vmbus_drv.c       |    3 ++-
 drivers/perf/arm_dsu_pmu.c   |    4 ++--
 drivers/perf/qcom_l3_pmu.c   |    3 ++-
 drivers/spi/spi.c            |    2 +-
 drivers/thunderbolt/acpi.c   |    2 +-
 include/acpi/acpi_bus.h      |   10 +++++++++-
 14 files changed, 46 insertions(+), 37 deletions(-)

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -29,8 +29,6 @@ extern struct acpi_device *acpi_root;
 #define ACPI_BUS_HID			"LNXSYBUS"
 #define ACPI_BUS_DEVICE_NAME		"System Bus"
 
-#define ACPI_IS_ROOT_DEVICE(device)    (!(device)->parent)
-
 #define INVALID_ACPI_HANDLE	((acpi_handle)empty_zero_page)
 
 static const char *dummy_hid = "device";
@@ -1110,7 +1108,7 @@ static void acpi_device_get_busid(struct
 	 * The device's Bus ID is simply the object name.
 	 * TBD: Shouldn't this value be unique (within the ACPI namespace)?
 	 */
-	if (ACPI_IS_ROOT_DEVICE(device)) {
+	if (!acpi_dev_parent(device)) {
 		strcpy(device->pnp.bus_id, "ACPI");
 		return;
 	}
@@ -1646,7 +1644,7 @@ static void acpi_init_coherency(struct a
 {
 	unsigned long long cca = 0;
 	acpi_status status;
-	struct acpi_device *parent = adev->parent;
+	struct acpi_device *parent = acpi_dev_parent(adev);
 
 	if (parent && parent->flags.cca_seen) {
 		/*
@@ -1690,7 +1688,7 @@ static int acpi_check_serial_bus_slave(s
 
 static bool acpi_is_indirect_io_slave(struct acpi_device *device)
 {
-	struct acpi_device *parent = device->parent;
+	struct acpi_device *parent = acpi_dev_parent(device);
 	static const struct acpi_device_id indirect_io_hosts[] = {
 		{"HISI0191", 0},
 		{}
@@ -1767,10 +1765,7 @@ void acpi_init_device_object(struct acpi
 	INIT_LIST_HEAD(&device->pnp.ids);
 	device->device_type = type;
 	device->handle = handle;
-	if (parent) {
-		device->parent = parent;
-		device->dev.parent = &parent->dev;
-	}
+	device->dev.parent = parent ? &parent->dev : NULL;
 	device->dev.release = release;
 	device->dev.bus = &acpi_bus_type;
 	fwnode_init(&device->fwnode, &acpi_device_fwnode_ops);
@@ -1867,8 +1862,8 @@ static int acpi_add_single_object(struct
 	acpi_device_add_finalize(device);
 
 	acpi_handle_debug(handle, "Added as %s, parent %s\n",
-			  dev_name(&device->dev), device->parent ?
-				dev_name(&device->parent->dev) : "(null)");
+			  dev_name(&device->dev), device->dev.parent ?
+				dev_name(device->dev.parent) : "(null)");
 
 	*child = device;
 	return 0;
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -365,7 +365,6 @@ struct acpi_device {
 	int device_type;
 	acpi_handle handle;		/* no handle for fixed hardware */
 	struct fwnode_handle fwnode;
-	struct acpi_device *parent;
 	struct list_head wakeup_list;
 	struct list_head del_list;
 	struct acpi_device_status status;
@@ -458,6 +457,14 @@ static inline void *acpi_driver_data(str
 #define to_acpi_device(d)	container_of(d, struct acpi_device, dev)
 #define to_acpi_driver(d)	container_of(d, struct acpi_driver, drv)
 
+static inline struct acpi_device *acpi_dev_parent(struct acpi_device *adev)
+{
+	if (adev->dev.parent)
+		return to_acpi_device(adev->dev.parent);
+
+	return NULL;
+}
+
 static inline void acpi_set_device_status(struct acpi_device *adev, u32 sta)
 {
 	*((u32 *)&adev->status) = sta;
@@ -478,6 +485,7 @@ void acpi_initialize_hp_context(struct a
 /* acpi_device.dev.bus == &acpi_bus_type */
 extern struct bus_type acpi_bus_type;
 
+struct acpi_device *acpi_dev_parent(struct acpi_device *adev);
 int acpi_bus_for_each_dev(int (*fn)(struct device *, void *), void *data);
 int acpi_dev_for_each_child(struct acpi_device *adev,
 			    int (*fn)(struct acpi_device *, void *), void *data);
Index: linux-pm/drivers/acpi/property.c
===================================================================
--- linux-pm.orig/drivers/acpi/property.c
+++ linux-pm/drivers/acpi/property.c
@@ -304,8 +304,10 @@ static void acpi_init_of_compatible(stru
 		ret = acpi_dev_get_property(adev, "compatible",
 					    ACPI_TYPE_STRING, &of_compatible);
 		if (ret) {
-			if (adev->parent
-			    && adev->parent->flags.of_compatible_ok)
+			struct acpi_device *parent;
+
+			parent = acpi_dev_parent(adev);
+			if (parent && parent->flags.of_compatible_ok)
 				goto out;
 
 			return;
Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -74,6 +74,7 @@ static int acpi_dev_pm_explicit_get(stru
  */
 int acpi_device_get_power(struct acpi_device *device, int *state)
 {
+	struct acpi_device *parent = acpi_dev_parent(device);
 	int result = ACPI_STATE_UNKNOWN;
 	int error;
 
@@ -82,8 +83,7 @@ int acpi_device_get_power(struct acpi_de
 
 	if (!device->flags.power_manageable) {
 		/* TBD: Non-recursive algorithm for walking up hierarchy. */
-		*state = device->parent ?
-			device->parent->power.state : ACPI_STATE_D0;
+		*state = parent ? parent->power.state : ACPI_STATE_D0;
 		goto out;
 	}
 
@@ -122,10 +122,10 @@ int acpi_device_get_power(struct acpi_de
 	 * point, the fact that the device is in D0 implies that the parent has
 	 * to be in D0 too, except if ignore_parent is set.
 	 */
-	if (!device->power.flags.ignore_parent && device->parent
-	    && device->parent->power.state == ACPI_STATE_UNKNOWN
-	    && result == ACPI_STATE_D0)
-		device->parent->power.state = ACPI_STATE_D0;
+	if (!device->power.flags.ignore_parent && parent &&
+	    parent->power.state == ACPI_STATE_UNKNOWN &&
+	    result == ACPI_STATE_D0)
+		parent->power.state = ACPI_STATE_D0;
 
 	*state = result;
 
@@ -159,6 +159,7 @@ static int acpi_dev_pm_explicit_set(stru
  */
 int acpi_device_set_power(struct acpi_device *device, int state)
 {
+	struct acpi_device *parent = acpi_dev_parent(device);
 	int target_state = state;
 	int result = 0;
 
@@ -191,12 +192,12 @@ int acpi_device_set_power(struct acpi_de
 		return -ENODEV;
 	}
 
-	if (!device->power.flags.ignore_parent && device->parent &&
-	    state < device->parent->power.state) {
+	if (!device->power.flags.ignore_parent && parent &&
+	    state < parent->power.state) {
 		acpi_handle_debug(device->handle,
 				  "Cannot transition to %s for parent in %s\n",
 				  acpi_power_state_string(state),
-				  acpi_power_state_string(device->parent->power.state));
+				  acpi_power_state_string(parent->power.state));
 		return -ENODEV;
 	}
 
Index: linux-pm/drivers/acpi/acpi_platform.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpi_platform.c
+++ linux-pm/drivers/acpi/acpi_platform.c
@@ -78,7 +78,7 @@ static void acpi_platform_fill_resource(
 	 * If the device has parent we need to take its resources into
 	 * account as well because this device might consume part of those.
 	 */
-	parent = acpi_get_first_physical_node(adev->parent);
+	parent = acpi_get_first_physical_node(acpi_dev_parent(adev));
 	if (parent && dev_is_pci(parent))
 		dest->parent = pci_find_resource(to_pci_dev(parent), dest);
 }
@@ -97,6 +97,7 @@ static void acpi_platform_fill_resource(
 struct platform_device *acpi_create_platform_device(struct acpi_device *adev,
 						    const struct property_entry *properties)
 {
+	struct acpi_device *parent = acpi_dev_parent(adev);
 	struct platform_device *pdev = NULL;
 	struct platform_device_info pdevinfo;
 	struct resource_entry *rentry;
@@ -137,8 +138,7 @@ struct platform_device *acpi_create_plat
 	 * attached to it, that physical device should be the parent of the
 	 * platform device we are about to create.
 	 */
-	pdevinfo.parent = adev->parent ?
-		acpi_get_first_physical_node(adev->parent) : NULL;
+	pdevinfo.parent = parent ? acpi_get_first_physical_node(parent) : NULL;
 	pdevinfo.name = dev_name(&adev->dev);
 	pdevinfo.id = -1;
 	pdevinfo.res = resources;
Index: linux-pm/drivers/acpi/acpi_video.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpi_video.c
+++ linux-pm/drivers/acpi/acpi_video.c
@@ -2030,7 +2030,7 @@ static int acpi_video_bus_add(struct acp
 	acpi_status status;
 
 	status = acpi_walk_namespace(ACPI_TYPE_DEVICE,
-				device->parent->handle, 1,
+				acpi_dev_parent(device)->handle, 1,
 				acpi_video_bus_match, NULL,
 				device, NULL);
 	if (status == AE_ALREADY_EXISTS) {
Index: linux-pm/drivers/acpi/sbs.c
===================================================================
--- linux-pm.orig/drivers/acpi/sbs.c
+++ linux-pm/drivers/acpi/sbs.c
@@ -632,7 +632,7 @@ static int acpi_sbs_add(struct acpi_devi
 
 	mutex_init(&sbs->lock);
 
-	sbs->hc = acpi_driver_data(device->parent);
+	sbs->hc = acpi_driver_data(acpi_dev_parent(device));
 	sbs->device = device;
 	strcpy(acpi_device_name(device), ACPI_SBS_DEVICE_NAME);
 	strcpy(acpi_device_class(device), ACPI_SBS_CLASS);
Index: linux-pm/drivers/acpi/sbshc.c
===================================================================
--- linux-pm.orig/drivers/acpi/sbshc.c
+++ linux-pm/drivers/acpi/sbshc.c
@@ -266,7 +266,7 @@ static int acpi_smbus_hc_add(struct acpi
 	mutex_init(&hc->lock);
 	init_waitqueue_head(&hc->wait);
 
-	hc->ec = acpi_driver_data(device->parent);
+	hc->ec = acpi_driver_data(acpi_dev_parent(device));
 	hc->offset = (val >> 8) & 0xff;
 	hc->query_bit = val & 0xff;
 	device->driver_data = hc;
Index: linux-pm/drivers/spi/spi.c
===================================================================
--- linux-pm.orig/drivers/spi/spi.c
+++ linux-pm/drivers/spi/spi.c
@@ -4375,7 +4375,7 @@ static int acpi_spi_notify(struct notifi
 
 	switch (value) {
 	case ACPI_RECONFIG_DEVICE_ADD:
-		ctlr = acpi_spi_find_controller_by_adev(adev->parent);
+		ctlr = acpi_spi_find_controller_by_adev(acpi_dev_parent(adev));
 		if (!ctlr)
 			break;
 
Index: linux-pm/drivers/thunderbolt/acpi.c
===================================================================
--- linux-pm.orig/drivers/thunderbolt/acpi.c
+++ linux-pm/drivers/thunderbolt/acpi.c
@@ -42,7 +42,7 @@ static acpi_status tb_acpi_add_link(acpi
 	 */
 	dev = acpi_get_first_physical_node(adev);
 	while (!dev) {
-		adev = adev->parent;
+		adev = acpi_dev_parent(adev);
 		if (!adev)
 			break;
 		dev = acpi_get_first_physical_node(adev);
Index: linux-pm/drivers/hv/vmbus_drv.c
===================================================================
--- linux-pm.orig/drivers/hv/vmbus_drv.c
+++ linux-pm/drivers/hv/vmbus_drv.c
@@ -2427,7 +2427,8 @@ static int vmbus_acpi_add(struct acpi_de
 	 * Some ancestor of the vmbus acpi device (Gen1 or Gen2
 	 * firmware) is the VMOD that has the mmio ranges. Get that.
 	 */
-	for (ancestor = device->parent; ancestor; ancestor = ancestor->parent) {
+	for (ancestor = acpi_dev_parent(device); ancestor;
+	     ancestor = acpi_dev_parent(ancestor)) {
 		result = acpi_walk_resources(ancestor->handle, METHOD_NAME__CRS,
 					     vmbus_walk_resources, NULL);
 
Index: linux-pm/drivers/acpi/acpi_amba.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpi_amba.c
+++ linux-pm/drivers/acpi/acpi_amba.c
@@ -48,6 +48,7 @@ static void amba_register_dummy_clk(void
 static int amba_handler_attach(struct acpi_device *adev,
 				const struct acpi_device_id *id)
 {
+	struct acpi_device *parent = acpi_dev_parent(adev);
 	struct amba_device *dev;
 	struct resource_entry *rentry;
 	struct list_head resource_list;
@@ -97,8 +98,8 @@ static int amba_handler_attach(struct ac
 	 * attached to it, that physical device should be the parent of
 	 * the amba device we are about to create.
 	 */
-	if (adev->parent)
-		dev->dev.parent = acpi_get_first_physical_node(adev->parent);
+	if (parent)
+		dev->dev.parent = acpi_get_first_physical_node(parent);
 
 	ACPI_COMPANION_SET(&dev->dev, adev);
 
Index: linux-pm/drivers/perf/arm_dsu_pmu.c
===================================================================
--- linux-pm.orig/drivers/perf/arm_dsu_pmu.c
+++ linux-pm/drivers/perf/arm_dsu_pmu.c
@@ -639,6 +639,7 @@ static int dsu_pmu_dt_get_cpus(struct de
 static int dsu_pmu_acpi_get_cpus(struct device *dev, cpumask_t *mask)
 {
 #ifdef CONFIG_ACPI
+	struct acpi_device *parent_adev = acpi_dev_parent(ACPI_COMPANION(dev));
 	int cpu;
 
 	/*
@@ -653,8 +654,7 @@ static int dsu_pmu_acpi_get_cpus(struct
 			continue;
 
 		acpi_dev = ACPI_COMPANION(cpu_dev);
-		if (acpi_dev &&
-			acpi_dev->parent == ACPI_COMPANION(dev)->parent)
+		if (acpi_dev && acpi_dev_parent(acpi_dev) == parent_adev)
 			cpumask_set_cpu(cpu, mask);
 	}
 #endif
Index: linux-pm/drivers/perf/qcom_l3_pmu.c
===================================================================
--- linux-pm.orig/drivers/perf/qcom_l3_pmu.c
+++ linux-pm/drivers/perf/qcom_l3_pmu.c
@@ -742,7 +742,8 @@ static int qcom_l3_cache_pmu_probe(struc
 
 	l3pmu = devm_kzalloc(&pdev->dev, sizeof(*l3pmu), GFP_KERNEL);
 	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "l3cache_%s_%s",
-		      acpi_dev->parent->pnp.unique_id, acpi_dev->pnp.unique_id);
+		      acpi_dev_parent(acpi_dev)->pnp.unique_id,
+		      acpi_dev->pnp.unique_id);
 	if (!l3pmu || !name)
 		return -ENOMEM;
 




^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2 5/5] ACPI: Drop parent field from struct acpi_device
@ 2022-08-24 16:59     ` Rafael J. Wysocki
  0 siblings, 0 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2022-08-24 16:59 UTC (permalink / raw)
  To: Linux ACPI
  Cc: LKML, Andy Shevchenko, Linux PM, K. Y. Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Wei Liu, Dexuan Cui, Mark Brown,
	Andreas Noever, Michael Jamet, Mika Westerberg, Yehezkel Bernat,
	linux-hyperv, linux-spi, linux-usb, linux-arm-kernel,
	linux-arm-msm, Will Deacon, Mark Rutland, Andy Gross,
	Bjorn Andersson, Konrad Dybcio

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The parent field in struct acpi_device is, in fact, redundant,
because the dev.parent field in it effectively points to the same
object and it is used by the driver core.

Accordingly, the parent field can be dropped from struct acpi_device
and for this purpose define acpi_dev_parent() to retrieve a parent
struct acpi_device pointer from the dev.parent field in struct
acpi_device.  Next, update all of the users of the parent field
in struct acpi_device to use acpi_dev_parent() instead of it and
drop it.

While at it, drop the ACPI_IS_ROOT_DEVICE() macro that is only used
in one place in a confusing way.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Acked-by: Mark Brown <broonie@kernel.org>
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Acked-by: Wei Liu <wei.liu@kernel.org>
Reviewed-by: Punit Agrawal <punit.agrawal@bytedance.com>
---

v1 -> v2:
   * Cover 3 more users of the parent field in struct acpi_device.
   * Add tags collected so far.

---
 drivers/acpi/acpi_amba.c     |    5 +++--
 drivers/acpi/acpi_platform.c |    6 +++---
 drivers/acpi/acpi_video.c    |    2 +-
 drivers/acpi/device_pm.c     |   19 ++++++++++---------
 drivers/acpi/property.c      |    6 ++++--
 drivers/acpi/sbs.c           |    2 +-
 drivers/acpi/sbshc.c         |    2 +-
 drivers/acpi/scan.c          |   17 ++++++-----------
 drivers/hv/vmbus_drv.c       |    3 ++-
 drivers/perf/arm_dsu_pmu.c   |    4 ++--
 drivers/perf/qcom_l3_pmu.c   |    3 ++-
 drivers/spi/spi.c            |    2 +-
 drivers/thunderbolt/acpi.c   |    2 +-
 include/acpi/acpi_bus.h      |   10 +++++++++-
 14 files changed, 46 insertions(+), 37 deletions(-)

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -29,8 +29,6 @@ extern struct acpi_device *acpi_root;
 #define ACPI_BUS_HID			"LNXSYBUS"
 #define ACPI_BUS_DEVICE_NAME		"System Bus"
 
-#define ACPI_IS_ROOT_DEVICE(device)    (!(device)->parent)
-
 #define INVALID_ACPI_HANDLE	((acpi_handle)empty_zero_page)
 
 static const char *dummy_hid = "device";
@@ -1110,7 +1108,7 @@ static void acpi_device_get_busid(struct
 	 * The device's Bus ID is simply the object name.
 	 * TBD: Shouldn't this value be unique (within the ACPI namespace)?
 	 */
-	if (ACPI_IS_ROOT_DEVICE(device)) {
+	if (!acpi_dev_parent(device)) {
 		strcpy(device->pnp.bus_id, "ACPI");
 		return;
 	}
@@ -1646,7 +1644,7 @@ static void acpi_init_coherency(struct a
 {
 	unsigned long long cca = 0;
 	acpi_status status;
-	struct acpi_device *parent = adev->parent;
+	struct acpi_device *parent = acpi_dev_parent(adev);
 
 	if (parent && parent->flags.cca_seen) {
 		/*
@@ -1690,7 +1688,7 @@ static int acpi_check_serial_bus_slave(s
 
 static bool acpi_is_indirect_io_slave(struct acpi_device *device)
 {
-	struct acpi_device *parent = device->parent;
+	struct acpi_device *parent = acpi_dev_parent(device);
 	static const struct acpi_device_id indirect_io_hosts[] = {
 		{"HISI0191", 0},
 		{}
@@ -1767,10 +1765,7 @@ void acpi_init_device_object(struct acpi
 	INIT_LIST_HEAD(&device->pnp.ids);
 	device->device_type = type;
 	device->handle = handle;
-	if (parent) {
-		device->parent = parent;
-		device->dev.parent = &parent->dev;
-	}
+	device->dev.parent = parent ? &parent->dev : NULL;
 	device->dev.release = release;
 	device->dev.bus = &acpi_bus_type;
 	fwnode_init(&device->fwnode, &acpi_device_fwnode_ops);
@@ -1867,8 +1862,8 @@ static int acpi_add_single_object(struct
 	acpi_device_add_finalize(device);
 
 	acpi_handle_debug(handle, "Added as %s, parent %s\n",
-			  dev_name(&device->dev), device->parent ?
-				dev_name(&device->parent->dev) : "(null)");
+			  dev_name(&device->dev), device->dev.parent ?
+				dev_name(device->dev.parent) : "(null)");
 
 	*child = device;
 	return 0;
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -365,7 +365,6 @@ struct acpi_device {
 	int device_type;
 	acpi_handle handle;		/* no handle for fixed hardware */
 	struct fwnode_handle fwnode;
-	struct acpi_device *parent;
 	struct list_head wakeup_list;
 	struct list_head del_list;
 	struct acpi_device_status status;
@@ -458,6 +457,14 @@ static inline void *acpi_driver_data(str
 #define to_acpi_device(d)	container_of(d, struct acpi_device, dev)
 #define to_acpi_driver(d)	container_of(d, struct acpi_driver, drv)
 
+static inline struct acpi_device *acpi_dev_parent(struct acpi_device *adev)
+{
+	if (adev->dev.parent)
+		return to_acpi_device(adev->dev.parent);
+
+	return NULL;
+}
+
 static inline void acpi_set_device_status(struct acpi_device *adev, u32 sta)
 {
 	*((u32 *)&adev->status) = sta;
@@ -478,6 +485,7 @@ void acpi_initialize_hp_context(struct a
 /* acpi_device.dev.bus == &acpi_bus_type */
 extern struct bus_type acpi_bus_type;
 
+struct acpi_device *acpi_dev_parent(struct acpi_device *adev);
 int acpi_bus_for_each_dev(int (*fn)(struct device *, void *), void *data);
 int acpi_dev_for_each_child(struct acpi_device *adev,
 			    int (*fn)(struct acpi_device *, void *), void *data);
Index: linux-pm/drivers/acpi/property.c
===================================================================
--- linux-pm.orig/drivers/acpi/property.c
+++ linux-pm/drivers/acpi/property.c
@@ -304,8 +304,10 @@ static void acpi_init_of_compatible(stru
 		ret = acpi_dev_get_property(adev, "compatible",
 					    ACPI_TYPE_STRING, &of_compatible);
 		if (ret) {
-			if (adev->parent
-			    && adev->parent->flags.of_compatible_ok)
+			struct acpi_device *parent;
+
+			parent = acpi_dev_parent(adev);
+			if (parent && parent->flags.of_compatible_ok)
 				goto out;
 
 			return;
Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -74,6 +74,7 @@ static int acpi_dev_pm_explicit_get(stru
  */
 int acpi_device_get_power(struct acpi_device *device, int *state)
 {
+	struct acpi_device *parent = acpi_dev_parent(device);
 	int result = ACPI_STATE_UNKNOWN;
 	int error;
 
@@ -82,8 +83,7 @@ int acpi_device_get_power(struct acpi_de
 
 	if (!device->flags.power_manageable) {
 		/* TBD: Non-recursive algorithm for walking up hierarchy. */
-		*state = device->parent ?
-			device->parent->power.state : ACPI_STATE_D0;
+		*state = parent ? parent->power.state : ACPI_STATE_D0;
 		goto out;
 	}
 
@@ -122,10 +122,10 @@ int acpi_device_get_power(struct acpi_de
 	 * point, the fact that the device is in D0 implies that the parent has
 	 * to be in D0 too, except if ignore_parent is set.
 	 */
-	if (!device->power.flags.ignore_parent && device->parent
-	    && device->parent->power.state == ACPI_STATE_UNKNOWN
-	    && result == ACPI_STATE_D0)
-		device->parent->power.state = ACPI_STATE_D0;
+	if (!device->power.flags.ignore_parent && parent &&
+	    parent->power.state == ACPI_STATE_UNKNOWN &&
+	    result == ACPI_STATE_D0)
+		parent->power.state = ACPI_STATE_D0;
 
 	*state = result;
 
@@ -159,6 +159,7 @@ static int acpi_dev_pm_explicit_set(stru
  */
 int acpi_device_set_power(struct acpi_device *device, int state)
 {
+	struct acpi_device *parent = acpi_dev_parent(device);
 	int target_state = state;
 	int result = 0;
 
@@ -191,12 +192,12 @@ int acpi_device_set_power(struct acpi_de
 		return -ENODEV;
 	}
 
-	if (!device->power.flags.ignore_parent && device->parent &&
-	    state < device->parent->power.state) {
+	if (!device->power.flags.ignore_parent && parent &&
+	    state < parent->power.state) {
 		acpi_handle_debug(device->handle,
 				  "Cannot transition to %s for parent in %s\n",
 				  acpi_power_state_string(state),
-				  acpi_power_state_string(device->parent->power.state));
+				  acpi_power_state_string(parent->power.state));
 		return -ENODEV;
 	}
 
Index: linux-pm/drivers/acpi/acpi_platform.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpi_platform.c
+++ linux-pm/drivers/acpi/acpi_platform.c
@@ -78,7 +78,7 @@ static void acpi_platform_fill_resource(
 	 * If the device has parent we need to take its resources into
 	 * account as well because this device might consume part of those.
 	 */
-	parent = acpi_get_first_physical_node(adev->parent);
+	parent = acpi_get_first_physical_node(acpi_dev_parent(adev));
 	if (parent && dev_is_pci(parent))
 		dest->parent = pci_find_resource(to_pci_dev(parent), dest);
 }
@@ -97,6 +97,7 @@ static void acpi_platform_fill_resource(
 struct platform_device *acpi_create_platform_device(struct acpi_device *adev,
 						    const struct property_entry *properties)
 {
+	struct acpi_device *parent = acpi_dev_parent(adev);
 	struct platform_device *pdev = NULL;
 	struct platform_device_info pdevinfo;
 	struct resource_entry *rentry;
@@ -137,8 +138,7 @@ struct platform_device *acpi_create_plat
 	 * attached to it, that physical device should be the parent of the
 	 * platform device we are about to create.
 	 */
-	pdevinfo.parent = adev->parent ?
-		acpi_get_first_physical_node(adev->parent) : NULL;
+	pdevinfo.parent = parent ? acpi_get_first_physical_node(parent) : NULL;
 	pdevinfo.name = dev_name(&adev->dev);
 	pdevinfo.id = -1;
 	pdevinfo.res = resources;
Index: linux-pm/drivers/acpi/acpi_video.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpi_video.c
+++ linux-pm/drivers/acpi/acpi_video.c
@@ -2030,7 +2030,7 @@ static int acpi_video_bus_add(struct acp
 	acpi_status status;
 
 	status = acpi_walk_namespace(ACPI_TYPE_DEVICE,
-				device->parent->handle, 1,
+				acpi_dev_parent(device)->handle, 1,
 				acpi_video_bus_match, NULL,
 				device, NULL);
 	if (status == AE_ALREADY_EXISTS) {
Index: linux-pm/drivers/acpi/sbs.c
===================================================================
--- linux-pm.orig/drivers/acpi/sbs.c
+++ linux-pm/drivers/acpi/sbs.c
@@ -632,7 +632,7 @@ static int acpi_sbs_add(struct acpi_devi
 
 	mutex_init(&sbs->lock);
 
-	sbs->hc = acpi_driver_data(device->parent);
+	sbs->hc = acpi_driver_data(acpi_dev_parent(device));
 	sbs->device = device;
 	strcpy(acpi_device_name(device), ACPI_SBS_DEVICE_NAME);
 	strcpy(acpi_device_class(device), ACPI_SBS_CLASS);
Index: linux-pm/drivers/acpi/sbshc.c
===================================================================
--- linux-pm.orig/drivers/acpi/sbshc.c
+++ linux-pm/drivers/acpi/sbshc.c
@@ -266,7 +266,7 @@ static int acpi_smbus_hc_add(struct acpi
 	mutex_init(&hc->lock);
 	init_waitqueue_head(&hc->wait);
 
-	hc->ec = acpi_driver_data(device->parent);
+	hc->ec = acpi_driver_data(acpi_dev_parent(device));
 	hc->offset = (val >> 8) & 0xff;
 	hc->query_bit = val & 0xff;
 	device->driver_data = hc;
Index: linux-pm/drivers/spi/spi.c
===================================================================
--- linux-pm.orig/drivers/spi/spi.c
+++ linux-pm/drivers/spi/spi.c
@@ -4375,7 +4375,7 @@ static int acpi_spi_notify(struct notifi
 
 	switch (value) {
 	case ACPI_RECONFIG_DEVICE_ADD:
-		ctlr = acpi_spi_find_controller_by_adev(adev->parent);
+		ctlr = acpi_spi_find_controller_by_adev(acpi_dev_parent(adev));
 		if (!ctlr)
 			break;
 
Index: linux-pm/drivers/thunderbolt/acpi.c
===================================================================
--- linux-pm.orig/drivers/thunderbolt/acpi.c
+++ linux-pm/drivers/thunderbolt/acpi.c
@@ -42,7 +42,7 @@ static acpi_status tb_acpi_add_link(acpi
 	 */
 	dev = acpi_get_first_physical_node(adev);
 	while (!dev) {
-		adev = adev->parent;
+		adev = acpi_dev_parent(adev);
 		if (!adev)
 			break;
 		dev = acpi_get_first_physical_node(adev);
Index: linux-pm/drivers/hv/vmbus_drv.c
===================================================================
--- linux-pm.orig/drivers/hv/vmbus_drv.c
+++ linux-pm/drivers/hv/vmbus_drv.c
@@ -2427,7 +2427,8 @@ static int vmbus_acpi_add(struct acpi_de
 	 * Some ancestor of the vmbus acpi device (Gen1 or Gen2
 	 * firmware) is the VMOD that has the mmio ranges. Get that.
 	 */
-	for (ancestor = device->parent; ancestor; ancestor = ancestor->parent) {
+	for (ancestor = acpi_dev_parent(device); ancestor;
+	     ancestor = acpi_dev_parent(ancestor)) {
 		result = acpi_walk_resources(ancestor->handle, METHOD_NAME__CRS,
 					     vmbus_walk_resources, NULL);
 
Index: linux-pm/drivers/acpi/acpi_amba.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpi_amba.c
+++ linux-pm/drivers/acpi/acpi_amba.c
@@ -48,6 +48,7 @@ static void amba_register_dummy_clk(void
 static int amba_handler_attach(struct acpi_device *adev,
 				const struct acpi_device_id *id)
 {
+	struct acpi_device *parent = acpi_dev_parent(adev);
 	struct amba_device *dev;
 	struct resource_entry *rentry;
 	struct list_head resource_list;
@@ -97,8 +98,8 @@ static int amba_handler_attach(struct ac
 	 * attached to it, that physical device should be the parent of
 	 * the amba device we are about to create.
 	 */
-	if (adev->parent)
-		dev->dev.parent = acpi_get_first_physical_node(adev->parent);
+	if (parent)
+		dev->dev.parent = acpi_get_first_physical_node(parent);
 
 	ACPI_COMPANION_SET(&dev->dev, adev);
 
Index: linux-pm/drivers/perf/arm_dsu_pmu.c
===================================================================
--- linux-pm.orig/drivers/perf/arm_dsu_pmu.c
+++ linux-pm/drivers/perf/arm_dsu_pmu.c
@@ -639,6 +639,7 @@ static int dsu_pmu_dt_get_cpus(struct de
 static int dsu_pmu_acpi_get_cpus(struct device *dev, cpumask_t *mask)
 {
 #ifdef CONFIG_ACPI
+	struct acpi_device *parent_adev = acpi_dev_parent(ACPI_COMPANION(dev));
 	int cpu;
 
 	/*
@@ -653,8 +654,7 @@ static int dsu_pmu_acpi_get_cpus(struct
 			continue;
 
 		acpi_dev = ACPI_COMPANION(cpu_dev);
-		if (acpi_dev &&
-			acpi_dev->parent == ACPI_COMPANION(dev)->parent)
+		if (acpi_dev && acpi_dev_parent(acpi_dev) == parent_adev)
 			cpumask_set_cpu(cpu, mask);
 	}
 #endif
Index: linux-pm/drivers/perf/qcom_l3_pmu.c
===================================================================
--- linux-pm.orig/drivers/perf/qcom_l3_pmu.c
+++ linux-pm/drivers/perf/qcom_l3_pmu.c
@@ -742,7 +742,8 @@ static int qcom_l3_cache_pmu_probe(struc
 
 	l3pmu = devm_kzalloc(&pdev->dev, sizeof(*l3pmu), GFP_KERNEL);
 	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "l3cache_%s_%s",
-		      acpi_dev->parent->pnp.unique_id, acpi_dev->pnp.unique_id);
+		      acpi_dev_parent(acpi_dev)->pnp.unique_id,
+		      acpi_dev->pnp.unique_id);
 	if (!l3pmu || !name)
 		return -ENOMEM;
 




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 5/5] ACPI: Drop parent field from struct acpi_device
  2022-08-24 16:59     ` Rafael J. Wysocki
@ 2022-08-24 18:23       ` Andy Shevchenko
  -1 siblings, 0 replies; 28+ messages in thread
From: Andy Shevchenko @ 2022-08-24 18:23 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux ACPI, LKML, Andy Shevchenko, Linux PM, K. Y. Srinivasan,
	Haiyang Zhang, Stephen Hemminger, Wei Liu, Dexuan Cui,
	Mark Brown, Andreas Noever, Michael Jamet, Mika Westerberg,
	Yehezkel Bernat, Linux on Hyper-V List, linux-spi, USB,
	linux-arm Mailing List, linux-arm-msm, Will Deacon, Mark Rutland,
	Andy Gross, Bjorn Andersson, Konrad Dybcio

On Wed, Aug 24, 2022 at 8:13 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> The parent field in struct acpi_device is, in fact, redundant,
> because the dev.parent field in it effectively points to the same
> object and it is used by the driver core.
>
> Accordingly, the parent field can be dropped from struct acpi_device
> and for this purpose define acpi_dev_parent() to retrieve a parent
> struct acpi_device pointer from the dev.parent field in struct
> acpi_device.  Next, update all of the users of the parent field
> in struct acpi_device to use acpi_dev_parent() instead of it and
> drop it.
>
> While at it, drop the ACPI_IS_ROOT_DEVICE() macro that is only used
> in one place in a confusing way.
>
> No intentional functional impact.

Side note: Should we not convert these to use acpi_dev_parent()?

https://elixir.bootlin.com/linux/v6.0-rc2/source/drivers/acpi/property.c#L1271
https://elixir.bootlin.com/linux/v6.0-rc2/source/drivers/bus/hisi_lpc.c#L397

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 5/5] ACPI: Drop parent field from struct acpi_device
@ 2022-08-24 18:23       ` Andy Shevchenko
  0 siblings, 0 replies; 28+ messages in thread
From: Andy Shevchenko @ 2022-08-24 18:23 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux ACPI, LKML, Andy Shevchenko, Linux PM, K. Y. Srinivasan,
	Haiyang Zhang, Stephen Hemminger, Wei Liu, Dexuan Cui,
	Mark Brown, Andreas Noever, Michael Jamet, Mika Westerberg,
	Yehezkel Bernat, Linux on Hyper-V List, linux-spi, USB,
	linux-arm Mailing List, linux-arm-msm, Will Deacon, Mark Rutland,
	Andy Gross, Bjorn Andersson, Konrad Dybcio

On Wed, Aug 24, 2022 at 8:13 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> The parent field in struct acpi_device is, in fact, redundant,
> because the dev.parent field in it effectively points to the same
> object and it is used by the driver core.
>
> Accordingly, the parent field can be dropped from struct acpi_device
> and for this purpose define acpi_dev_parent() to retrieve a parent
> struct acpi_device pointer from the dev.parent field in struct
> acpi_device.  Next, update all of the users of the parent field
> in struct acpi_device to use acpi_dev_parent() instead of it and
> drop it.
>
> While at it, drop the ACPI_IS_ROOT_DEVICE() macro that is only used
> in one place in a confusing way.
>
> No intentional functional impact.

Side note: Should we not convert these to use acpi_dev_parent()?

https://elixir.bootlin.com/linux/v6.0-rc2/source/drivers/acpi/property.c#L1271
https://elixir.bootlin.com/linux/v6.0-rc2/source/drivers/bus/hisi_lpc.c#L397

-- 
With Best Regards,
Andy Shevchenko

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 5/5] ACPI: Drop parent field from struct acpi_device
  2022-08-24 18:23       ` Andy Shevchenko
@ 2022-08-24 18:34         ` Rafael J. Wysocki
  -1 siblings, 0 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2022-08-24 18:34 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Rafael J. Wysocki, Linux ACPI, LKML, Andy Shevchenko, Linux PM,
	K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
	Dexuan Cui, Mark Brown, Andreas Noever, Michael Jamet,
	Mika Westerberg, Yehezkel Bernat, Linux on Hyper-V List,
	linux-spi, USB, linux-arm Mailing List, linux-arm-msm,
	Will Deacon, Mark Rutland, Andy Gross, Bjorn Andersson,
	Konrad Dybcio

On Wed, Aug 24, 2022 at 8:23 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Wed, Aug 24, 2022 at 8:13 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> >
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > The parent field in struct acpi_device is, in fact, redundant,
> > because the dev.parent field in it effectively points to the same
> > object and it is used by the driver core.
> >
> > Accordingly, the parent field can be dropped from struct acpi_device
> > and for this purpose define acpi_dev_parent() to retrieve a parent
> > struct acpi_device pointer from the dev.parent field in struct
> > acpi_device.  Next, update all of the users of the parent field
> > in struct acpi_device to use acpi_dev_parent() instead of it and
> > drop it.
> >
> > While at it, drop the ACPI_IS_ROOT_DEVICE() macro that is only used
> > in one place in a confusing way.
> >
> > No intentional functional impact.
>
> Side note: Should we not convert these to use acpi_dev_parent()?
>
> https://elixir.bootlin.com/linux/v6.0-rc2/source/drivers/acpi/property.c#L1271
> https://elixir.bootlin.com/linux/v6.0-rc2/source/drivers/bus/hisi_lpc.c#L397

That can be done later, but thanks for the pointers!

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 5/5] ACPI: Drop parent field from struct acpi_device
@ 2022-08-24 18:34         ` Rafael J. Wysocki
  0 siblings, 0 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2022-08-24 18:34 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Rafael J. Wysocki, Linux ACPI, LKML, Andy Shevchenko, Linux PM,
	K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
	Dexuan Cui, Mark Brown, Andreas Noever, Michael Jamet,
	Mika Westerberg, Yehezkel Bernat, Linux on Hyper-V List,
	linux-spi, USB, linux-arm Mailing List, linux-arm-msm,
	Will Deacon, Mark Rutland, Andy Gross, Bjorn Andersson,
	Konrad Dybcio

On Wed, Aug 24, 2022 at 8:23 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Wed, Aug 24, 2022 at 8:13 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> >
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > The parent field in struct acpi_device is, in fact, redundant,
> > because the dev.parent field in it effectively points to the same
> > object and it is used by the driver core.
> >
> > Accordingly, the parent field can be dropped from struct acpi_device
> > and for this purpose define acpi_dev_parent() to retrieve a parent
> > struct acpi_device pointer from the dev.parent field in struct
> > acpi_device.  Next, update all of the users of the parent field
> > in struct acpi_device to use acpi_dev_parent() instead of it and
> > drop it.
> >
> > While at it, drop the ACPI_IS_ROOT_DEVICE() macro that is only used
> > in one place in a confusing way.
> >
> > No intentional functional impact.
>
> Side note: Should we not convert these to use acpi_dev_parent()?
>
> https://elixir.bootlin.com/linux/v6.0-rc2/source/drivers/acpi/property.c#L1271
> https://elixir.bootlin.com/linux/v6.0-rc2/source/drivers/bus/hisi_lpc.c#L397

That can be done later, but thanks for the pointers!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 5/5] ACPI: Drop parent field from struct acpi_device
  2022-08-24 16:59     ` Rafael J. Wysocki
@ 2022-08-27 13:19       ` Hanjun Guo
  -1 siblings, 0 replies; 28+ messages in thread
From: Hanjun Guo @ 2022-08-27 13:19 UTC (permalink / raw)
  To: Rafael J. Wysocki, Linux ACPI
  Cc: LKML, Andy Shevchenko, Linux PM, K. Y. Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Wei Liu, Dexuan Cui, Mark Brown,
	Andreas Noever, Michael Jamet, Mika Westerberg, Yehezkel Bernat,
	linux-hyperv, linux-spi, linux-usb, linux-arm-kernel,
	linux-arm-msm, Will Deacon, Mark Rutland, Andy Gross,
	Bjorn Andersson, Konrad Dybcio

Hi Rafael,

On 2022/8/25 0:59, Rafael J. Wysocki wrote:
> Index: linux-pm/include/acpi/acpi_bus.h
> ===================================================================
> --- linux-pm.orig/include/acpi/acpi_bus.h
> +++ linux-pm/include/acpi/acpi_bus.h
> @@ -365,7 +365,6 @@ struct acpi_device {
>   	int device_type;
>   	acpi_handle handle;		/* no handle for fixed hardware */
>   	struct fwnode_handle fwnode;
> -	struct acpi_device *parent;
>   	struct list_head wakeup_list;
>   	struct list_head del_list;
>   	struct acpi_device_status status;
> @@ -458,6 +457,14 @@ static inline void *acpi_driver_data(str
>   #define to_acpi_device(d)	container_of(d, struct acpi_device, dev)
>   #define to_acpi_driver(d)	container_of(d, struct acpi_driver, drv)
>   
> +static inline struct acpi_device *acpi_dev_parent(struct acpi_device *adev)
> +{
> +	if (adev->dev.parent)
> +		return to_acpi_device(adev->dev.parent);
> +
> +	return NULL;
> +}
> +
>   static inline void acpi_set_device_status(struct acpi_device *adev, u32 sta)
>   {
>   	*((u32 *)&adev->status) = sta;
> @@ -478,6 +485,7 @@ void acpi_initialize_hp_context(struct a
>   /* acpi_device.dev.bus == &acpi_bus_type */
>   extern struct bus_type acpi_bus_type;
>   
> +struct acpi_device *acpi_dev_parent(struct acpi_device *adev);

We have a static inline function above, is it duplicated here?
Or did I miss some use cases?

Thanks
Hanjun

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 5/5] ACPI: Drop parent field from struct acpi_device
@ 2022-08-27 13:19       ` Hanjun Guo
  0 siblings, 0 replies; 28+ messages in thread
From: Hanjun Guo @ 2022-08-27 13:19 UTC (permalink / raw)
  To: Rafael J. Wysocki, Linux ACPI
  Cc: LKML, Andy Shevchenko, Linux PM, K. Y. Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Wei Liu, Dexuan Cui, Mark Brown,
	Andreas Noever, Michael Jamet, Mika Westerberg, Yehezkel Bernat,
	linux-hyperv, linux-spi, linux-usb, linux-arm-kernel,
	linux-arm-msm, Will Deacon, Mark Rutland, Andy Gross,
	Bjorn Andersson, Konrad Dybcio

Hi Rafael,

On 2022/8/25 0:59, Rafael J. Wysocki wrote:
> Index: linux-pm/include/acpi/acpi_bus.h
> ===================================================================
> --- linux-pm.orig/include/acpi/acpi_bus.h
> +++ linux-pm/include/acpi/acpi_bus.h
> @@ -365,7 +365,6 @@ struct acpi_device {
>   	int device_type;
>   	acpi_handle handle;		/* no handle for fixed hardware */
>   	struct fwnode_handle fwnode;
> -	struct acpi_device *parent;
>   	struct list_head wakeup_list;
>   	struct list_head del_list;
>   	struct acpi_device_status status;
> @@ -458,6 +457,14 @@ static inline void *acpi_driver_data(str
>   #define to_acpi_device(d)	container_of(d, struct acpi_device, dev)
>   #define to_acpi_driver(d)	container_of(d, struct acpi_driver, drv)
>   
> +static inline struct acpi_device *acpi_dev_parent(struct acpi_device *adev)
> +{
> +	if (adev->dev.parent)
> +		return to_acpi_device(adev->dev.parent);
> +
> +	return NULL;
> +}
> +
>   static inline void acpi_set_device_status(struct acpi_device *adev, u32 sta)
>   {
>   	*((u32 *)&adev->status) = sta;
> @@ -478,6 +485,7 @@ void acpi_initialize_hp_context(struct a
>   /* acpi_device.dev.bus == &acpi_bus_type */
>   extern struct bus_type acpi_bus_type;
>   
> +struct acpi_device *acpi_dev_parent(struct acpi_device *adev);

We have a static inline function above, is it duplicated here?
Or did I miss some use cases?

Thanks
Hanjun

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v1] ACPI: PM: Fix NULL argument handling in acpi_device_get/set_power()
  2022-08-10 16:11 [PATCH v1 0/5] ACPI: Device enumeration rearrangements and parent field elimination Rafael J. Wysocki
                   ` (5 preceding siblings ...)
  2022-08-12 13:11 ` [PATCH v1 0/5] ACPI: Device enumeration rearrangements and parent field elimination Punit Agrawal
@ 2022-08-29 15:21 ` Rafael J. Wysocki
  2022-08-29 15:53 ` [PATCH v1] ACPI: Drop redundant acpi_dev_parent() header Rafael J. Wysocki
  7 siblings, 0 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2022-08-29 15:21 UTC (permalink / raw)
  To: Linux ACPI; +Cc: LKML, Andy Shevchenko, Linux PM, Dan Carpenter

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

In principle, it should be valid to pass NULL as the ACPI device
pointer to acpi_device_get_power() and acpi_device_set_power() and they
both are expected to return -EINVAL in that case, but that has been
broken recently by commit 62fcb99bdf10 ("ACPI: Drop parent field from
struct acpi_device") which has caused the ACPI device pointer to be
dereferenced in these functions before the NULL check.

Fix that and while at it make acpi_device_set_power() only use the
parent field if the target ACPI device object's ignore_parent flag
in not set.

Fixes: 62fcb99bdf10 ("ACPI: Drop parent field from struct acpi_device")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

linux-next material.

---
 drivers/acpi/device_pm.c |   23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -74,13 +74,15 @@ static int acpi_dev_pm_explicit_get(stru
  */
 int acpi_device_get_power(struct acpi_device *device, int *state)
 {
-	struct acpi_device *parent = acpi_dev_parent(device);
 	int result = ACPI_STATE_UNKNOWN;
+	struct acpi_device *parent;
 	int error;
 
 	if (!device || !state)
 		return -EINVAL;
 
+	parent = acpi_dev_parent(device);
+
 	if (!device->flags.power_manageable) {
 		/* TBD: Non-recursive algorithm for walking up hierarchy. */
 		*state = parent ? parent->power.state : ACPI_STATE_D0;
@@ -159,7 +161,6 @@ static int acpi_dev_pm_explicit_set(stru
  */
 int acpi_device_set_power(struct acpi_device *device, int state)
 {
-	struct acpi_device *parent = acpi_dev_parent(device);
 	int target_state = state;
 	int result = 0;
 
@@ -192,13 +193,17 @@ int acpi_device_set_power(struct acpi_de
 		return -ENODEV;
 	}
 
-	if (!device->power.flags.ignore_parent && parent &&
-	    state < parent->power.state) {
-		acpi_handle_debug(device->handle,
-				  "Cannot transition to %s for parent in %s\n",
-				  acpi_power_state_string(state),
-				  acpi_power_state_string(parent->power.state));
-		return -ENODEV;
+	if (!device->power.flags.ignore_parent) {
+		struct acpi_device *parent;
+
+		parent = acpi_dev_parent(device);
+		if (parent && state < parent->power.state) {
+			acpi_handle_debug(device->handle,
+					  "Cannot transition to %s for parent in %s\n",
+					  acpi_power_state_string(state),
+					  acpi_power_state_string(parent->power.state));
+			return -ENODEV;
+		}
 	}
 
 	/*




^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v1] ACPI: Drop redundant acpi_dev_parent() header
  2022-08-10 16:11 [PATCH v1 0/5] ACPI: Device enumeration rearrangements and parent field elimination Rafael J. Wysocki
                   ` (6 preceding siblings ...)
  2022-08-29 15:21 ` [PATCH v1] ACPI: PM: Fix NULL argument handling in acpi_device_get/set_power() Rafael J. Wysocki
@ 2022-08-29 15:53 ` Rafael J. Wysocki
  2022-08-30  2:17   ` Hanjun Guo
  7 siblings, 1 reply; 28+ messages in thread
From: Rafael J. Wysocki @ 2022-08-29 15:53 UTC (permalink / raw)
  To: Linux ACPI; +Cc: LKML, Andy Shevchenko, Linux PM, Hanjun Guo

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Because acpi_dev_parent() is defined as static inline, the extra
header of it in acpi_bus.h is redundant, so drop it.

Fixes: 62fcb99bdf10 ("ACPI: Drop parent field from struct acpi_device")
Reported-by: Hanjun Guo <guohanjun@huawei.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

linux-next material

---
 include/acpi/acpi_bus.h |    1 -
 1 file changed, 1 deletion(-)

Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -485,7 +485,6 @@ void acpi_initialize_hp_context(struct a
 /* acpi_device.dev.bus == &acpi_bus_type */
 extern struct bus_type acpi_bus_type;
 
-struct acpi_device *acpi_dev_parent(struct acpi_device *adev);
 int acpi_bus_for_each_dev(int (*fn)(struct device *, void *), void *data);
 int acpi_dev_for_each_child(struct acpi_device *adev,
 			    int (*fn)(struct acpi_device *, void *), void *data);




^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 5/5] ACPI: Drop parent field from struct acpi_device
  2022-08-27 13:19       ` Hanjun Guo
@ 2022-08-29 15:54         ` Rafael J. Wysocki
  -1 siblings, 0 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2022-08-29 15:54 UTC (permalink / raw)
  To: Linux ACPI, Hanjun Guo
  Cc: LKML, Andy Shevchenko, Linux PM, K. Y. Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Wei Liu, Dexuan Cui, Mark Brown,
	Andreas Noever, Michael Jamet, Mika Westerberg, Yehezkel Bernat,
	linux-hyperv, linux-spi, linux-usb, linux-arm-kernel,
	linux-arm-msm, Will Deacon, Mark Rutland, Andy Gross,
	Bjorn Andersson, Konrad Dybcio

On Saturday, August 27, 2022 3:19:33 PM CEST Hanjun Guo wrote:
> Hi Rafael,
> 
> On 2022/8/25 0:59, Rafael J. Wysocki wrote:
> > Index: linux-pm/include/acpi/acpi_bus.h
> > ===================================================================
> > --- linux-pm.orig/include/acpi/acpi_bus.h
> > +++ linux-pm/include/acpi/acpi_bus.h
> > @@ -365,7 +365,6 @@ struct acpi_device {
> >   	int device_type;
> >   	acpi_handle handle;		/* no handle for fixed hardware */
> >   	struct fwnode_handle fwnode;
> > -	struct acpi_device *parent;
> >   	struct list_head wakeup_list;
> >   	struct list_head del_list;
> >   	struct acpi_device_status status;
> > @@ -458,6 +457,14 @@ static inline void *acpi_driver_data(str
> >   #define to_acpi_device(d)	container_of(d, struct acpi_device, dev)
> >   #define to_acpi_driver(d)	container_of(d, struct acpi_driver, drv)
> >   
> > +static inline struct acpi_device *acpi_dev_parent(struct acpi_device *adev)
> > +{
> > +	if (adev->dev.parent)
> > +		return to_acpi_device(adev->dev.parent);
> > +
> > +	return NULL;
> > +}
> > +
> >   static inline void acpi_set_device_status(struct acpi_device *adev, u32 sta)
> >   {
> >   	*((u32 *)&adev->status) = sta;
> > @@ -478,6 +485,7 @@ void acpi_initialize_hp_context(struct a
> >   /* acpi_device.dev.bus == &acpi_bus_type */
> >   extern struct bus_type acpi_bus_type;
> >   
> > +struct acpi_device *acpi_dev_parent(struct acpi_device *adev);
> 
> We have a static inline function above, is it duplicated here?
> Or did I miss some use cases?

No, you didn't, it is redundant.

I've just sent a fix for this.

Thanks!




^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 5/5] ACPI: Drop parent field from struct acpi_device
@ 2022-08-29 15:54         ` Rafael J. Wysocki
  0 siblings, 0 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2022-08-29 15:54 UTC (permalink / raw)
  To: Linux ACPI, Hanjun Guo
  Cc: LKML, Andy Shevchenko, Linux PM, K. Y. Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Wei Liu, Dexuan Cui, Mark Brown,
	Andreas Noever, Michael Jamet, Mika Westerberg, Yehezkel Bernat,
	linux-hyperv, linux-spi, linux-usb, linux-arm-kernel,
	linux-arm-msm, Will Deacon, Mark Rutland, Andy Gross,
	Bjorn Andersson, Konrad Dybcio

On Saturday, August 27, 2022 3:19:33 PM CEST Hanjun Guo wrote:
> Hi Rafael,
> 
> On 2022/8/25 0:59, Rafael J. Wysocki wrote:
> > Index: linux-pm/include/acpi/acpi_bus.h
> > ===================================================================
> > --- linux-pm.orig/include/acpi/acpi_bus.h
> > +++ linux-pm/include/acpi/acpi_bus.h
> > @@ -365,7 +365,6 @@ struct acpi_device {
> >   	int device_type;
> >   	acpi_handle handle;		/* no handle for fixed hardware */
> >   	struct fwnode_handle fwnode;
> > -	struct acpi_device *parent;
> >   	struct list_head wakeup_list;
> >   	struct list_head del_list;
> >   	struct acpi_device_status status;
> > @@ -458,6 +457,14 @@ static inline void *acpi_driver_data(str
> >   #define to_acpi_device(d)	container_of(d, struct acpi_device, dev)
> >   #define to_acpi_driver(d)	container_of(d, struct acpi_driver, drv)
> >   
> > +static inline struct acpi_device *acpi_dev_parent(struct acpi_device *adev)
> > +{
> > +	if (adev->dev.parent)
> > +		return to_acpi_device(adev->dev.parent);
> > +
> > +	return NULL;
> > +}
> > +
> >   static inline void acpi_set_device_status(struct acpi_device *adev, u32 sta)
> >   {
> >   	*((u32 *)&adev->status) = sta;
> > @@ -478,6 +485,7 @@ void acpi_initialize_hp_context(struct a
> >   /* acpi_device.dev.bus == &acpi_bus_type */
> >   extern struct bus_type acpi_bus_type;
> >   
> > +struct acpi_device *acpi_dev_parent(struct acpi_device *adev);
> 
> We have a static inline function above, is it duplicated here?
> Or did I miss some use cases?

No, you didn't, it is redundant.

I've just sent a fix for this.

Thanks!




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v1] ACPI: Drop redundant acpi_dev_parent() header
  2022-08-29 15:53 ` [PATCH v1] ACPI: Drop redundant acpi_dev_parent() header Rafael J. Wysocki
@ 2022-08-30  2:17   ` Hanjun Guo
  0 siblings, 0 replies; 28+ messages in thread
From: Hanjun Guo @ 2022-08-30  2:17 UTC (permalink / raw)
  To: Rafael J. Wysocki, Linux ACPI; +Cc: LKML, Andy Shevchenko, Linux PM

On 2022/8/29 23:53, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Because acpi_dev_parent() is defined as static inline, the extra
> header of it in acpi_bus.h is redundant, so drop it.
> 
> Fixes: 62fcb99bdf10 ("ACPI: Drop parent field from struct acpi_device")
> Reported-by: Hanjun Guo <guohanjun@huawei.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Reviewed-by: Hanjun Guo <guohanjun@huawei.com>

Thanks
Hanjun

^ permalink raw reply	[flat|nested] 28+ messages in thread

* RE: [PATCH v2 5/5] ACPI: Drop parent field from struct acpi_device
  2022-08-24 16:59     ` Rafael J. Wysocki
@ 2022-08-30 21:29       ` Michael Kelley (LINUX)
  -1 siblings, 0 replies; 28+ messages in thread
From: Michael Kelley (LINUX) @ 2022-08-30 21:29 UTC (permalink / raw)
  To: Rafael J. Wysocki, Linux ACPI
  Cc: LKML, Andy Shevchenko, Linux PM, KY Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Wei Liu, Dexuan Cui, Mark Brown,
	Andreas Noever, Jamet, Michael, Mika Westerberg, Yehezkel Bernat,
	linux-hyperv, linux-spi, linux-usb, linux-arm-kernel,
	linux-arm-msm, Will Deacon, Mark Rutland, Andy Gross,
	Bjorn Andersson, Konrad Dybcio

From: Rafael J. Wysocki <rjw@rjwysocki.net> Sent: Wednesday, August 24, 2022 10:00 AM
> 
> The parent field in struct acpi_device is, in fact, redundant,
> because the dev.parent field in it effectively points to the same
> object and it is used by the driver core.
> 
> Accordingly, the parent field can be dropped from struct acpi_device
> and for this purpose define acpi_dev_parent() to retrieve a parent
> struct acpi_device pointer from the dev.parent field in struct
> acpi_device.  Next, update all of the users of the parent field
> in struct acpi_device to use acpi_dev_parent() instead of it and
> drop it.
> 
> While at it, drop the ACPI_IS_ROOT_DEVICE() macro that is only used
> in one place in a confusing way.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Acked-by: Mark Brown <broonie@kernel.org>
> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Acked-by: Wei Liu <wei.liu@kernel.org>
> Reviewed-by: Punit Agrawal <punit.agrawal@bytedance.com>

Tested the full patch series in Azure VMs running on Hyper-V --
one x86/x64 and one ARM64.  Testing includes standard VMbus
devices as well as virtual PCI devices (Mellanox CX-5 Virtual
Function).   Verified that ACPI device cache coherence property is
propagated through the hierarchy of devices that the VMbus and
Hyper-V virtual PCI drivers set up.

Tested-by: Michael Kelley <mikelley@microsoft.com>

> ---
> 
> v1 -> v2:
>    * Cover 3 more users of the parent field in struct acpi_device.
>    * Add tags collected so far.
> 
> ---
>  drivers/acpi/acpi_amba.c     |    5 +++--
>  drivers/acpi/acpi_platform.c |    6 +++---
>  drivers/acpi/acpi_video.c    |    2 +-
>  drivers/acpi/device_pm.c     |   19 ++++++++++---------
>  drivers/acpi/property.c      |    6 ++++--
>  drivers/acpi/sbs.c           |    2 +-
>  drivers/acpi/sbshc.c         |    2 +-
>  drivers/acpi/scan.c          |   17 ++++++-----------
>  drivers/hv/vmbus_drv.c       |    3 ++-
>  drivers/perf/arm_dsu_pmu.c   |    4 ++--
>  drivers/perf/qcom_l3_pmu.c   |    3 ++-
>  drivers/spi/spi.c            |    2 +-
>  drivers/thunderbolt/acpi.c   |    2 +-
>  include/acpi/acpi_bus.h      |   10 +++++++++-
>  14 files changed, 46 insertions(+), 37 deletions(-)
> 
> Index: linux-pm/drivers/acpi/scan.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/scan.c
> +++ linux-pm/drivers/acpi/scan.c
> @@ -29,8 +29,6 @@ extern struct acpi_device *acpi_root;
>  #define ACPI_BUS_HID			"LNXSYBUS"
>  #define ACPI_BUS_DEVICE_NAME		"System Bus"
> 
> -#define ACPI_IS_ROOT_DEVICE(device)    (!(device)->parent)
> -
>  #define INVALID_ACPI_HANDLE	((acpi_handle)empty_zero_page)
> 
>  static const char *dummy_hid = "device";
> @@ -1110,7 +1108,7 @@ static void acpi_device_get_busid(struct
>  	 * The device's Bus ID is simply the object name.
>  	 * TBD: Shouldn't this value be unique (within the ACPI namespace)?
>  	 */
> -	if (ACPI_IS_ROOT_DEVICE(device)) {
> +	if (!acpi_dev_parent(device)) {
>  		strcpy(device->pnp.bus_id, "ACPI");
>  		return;
>  	}
> @@ -1646,7 +1644,7 @@ static void acpi_init_coherency(struct a
>  {
>  	unsigned long long cca = 0;
>  	acpi_status status;
> -	struct acpi_device *parent = adev->parent;
> +	struct acpi_device *parent = acpi_dev_parent(adev);
> 
>  	if (parent && parent->flags.cca_seen) {
>  		/*
> @@ -1690,7 +1688,7 @@ static int acpi_check_serial_bus_slave(s
> 
>  static bool acpi_is_indirect_io_slave(struct acpi_device *device)
>  {
> -	struct acpi_device *parent = device->parent;
> +	struct acpi_device *parent = acpi_dev_parent(device);
>  	static const struct acpi_device_id indirect_io_hosts[] = {
>  		{"HISI0191", 0},
>  		{}
> @@ -1767,10 +1765,7 @@ void acpi_init_device_object(struct acpi
>  	INIT_LIST_HEAD(&device->pnp.ids);
>  	device->device_type = type;
>  	device->handle = handle;
> -	if (parent) {
> -		device->parent = parent;
> -		device->dev.parent = &parent->dev;
> -	}
> +	device->dev.parent = parent ? &parent->dev : NULL;
>  	device->dev.release = release;
>  	device->dev.bus = &acpi_bus_type;
>  	fwnode_init(&device->fwnode, &acpi_device_fwnode_ops);
> @@ -1867,8 +1862,8 @@ static int acpi_add_single_object(struct
>  	acpi_device_add_finalize(device);
> 
>  	acpi_handle_debug(handle, "Added as %s, parent %s\n",
> -			  dev_name(&device->dev), device->parent ?
> -				dev_name(&device->parent->dev) : "(null)");
> +			  dev_name(&device->dev), device->dev.parent ?
> +				dev_name(device->dev.parent) : "(null)");
> 
>  	*child = device;
>  	return 0;
> Index: linux-pm/include/acpi/acpi_bus.h
> ===================================================================
> --- linux-pm.orig/include/acpi/acpi_bus.h
> +++ linux-pm/include/acpi/acpi_bus.h
> @@ -365,7 +365,6 @@ struct acpi_device {
>  	int device_type;
>  	acpi_handle handle;		/* no handle for fixed hardware */
>  	struct fwnode_handle fwnode;
> -	struct acpi_device *parent;
>  	struct list_head wakeup_list;
>  	struct list_head del_list;
>  	struct acpi_device_status status;
> @@ -458,6 +457,14 @@ static inline void *acpi_driver_data(str
>  #define to_acpi_device(d)	container_of(d, struct acpi_device, dev)
>  #define to_acpi_driver(d)	container_of(d, struct acpi_driver, drv)
> 
> +static inline struct acpi_device *acpi_dev_parent(struct acpi_device *adev)
> +{
> +	if (adev->dev.parent)
> +		return to_acpi_device(adev->dev.parent);
> +
> +	return NULL;
> +}
> +
>  static inline void acpi_set_device_status(struct acpi_device *adev, u32 sta)
>  {
>  	*((u32 *)&adev->status) = sta;
> @@ -478,6 +485,7 @@ void acpi_initialize_hp_context(struct a
>  /* acpi_device.dev.bus == &acpi_bus_type */
>  extern struct bus_type acpi_bus_type;
> 
> +struct acpi_device *acpi_dev_parent(struct acpi_device *adev);
>  int acpi_bus_for_each_dev(int (*fn)(struct device *, void *), void *data);
>  int acpi_dev_for_each_child(struct acpi_device *adev,
>  			    int (*fn)(struct acpi_device *, void *), void *data);
> Index: linux-pm/drivers/acpi/property.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/property.c
> +++ linux-pm/drivers/acpi/property.c
> @@ -304,8 +304,10 @@ static void acpi_init_of_compatible(stru
>  		ret = acpi_dev_get_property(adev, "compatible",
>  					    ACPI_TYPE_STRING, &of_compatible);
>  		if (ret) {
> -			if (adev->parent
> -			    && adev->parent->flags.of_compatible_ok)
> +			struct acpi_device *parent;
> +
> +			parent = acpi_dev_parent(adev);
> +			if (parent && parent->flags.of_compatible_ok)
>  				goto out;
> 
>  			return;
> Index: linux-pm/drivers/acpi/device_pm.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/device_pm.c
> +++ linux-pm/drivers/acpi/device_pm.c
> @@ -74,6 +74,7 @@ static int acpi_dev_pm_explicit_get(stru
>   */
>  int acpi_device_get_power(struct acpi_device *device, int *state)
>  {
> +	struct acpi_device *parent = acpi_dev_parent(device);
>  	int result = ACPI_STATE_UNKNOWN;
>  	int error;
> 
> @@ -82,8 +83,7 @@ int acpi_device_get_power(struct acpi_de
> 
>  	if (!device->flags.power_manageable) {
>  		/* TBD: Non-recursive algorithm for walking up hierarchy. */
> -		*state = device->parent ?
> -			device->parent->power.state : ACPI_STATE_D0;
> +		*state = parent ? parent->power.state : ACPI_STATE_D0;
>  		goto out;
>  	}
> 
> @@ -122,10 +122,10 @@ int acpi_device_get_power(struct acpi_de
>  	 * point, the fact that the device is in D0 implies that the parent has
>  	 * to be in D0 too, except if ignore_parent is set.
>  	 */
> -	if (!device->power.flags.ignore_parent && device->parent
> -	    && device->parent->power.state == ACPI_STATE_UNKNOWN
> -	    && result == ACPI_STATE_D0)
> -		device->parent->power.state = ACPI_STATE_D0;
> +	if (!device->power.flags.ignore_parent && parent &&
> +	    parent->power.state == ACPI_STATE_UNKNOWN &&
> +	    result == ACPI_STATE_D0)
> +		parent->power.state = ACPI_STATE_D0;
> 
>  	*state = result;
> 
> @@ -159,6 +159,7 @@ static int acpi_dev_pm_explicit_set(stru
>   */
>  int acpi_device_set_power(struct acpi_device *device, int state)
>  {
> +	struct acpi_device *parent = acpi_dev_parent(device);
>  	int target_state = state;
>  	int result = 0;
> 
> @@ -191,12 +192,12 @@ int acpi_device_set_power(struct acpi_de
>  		return -ENODEV;
>  	}
> 
> -	if (!device->power.flags.ignore_parent && device->parent &&
> -	    state < device->parent->power.state) {
> +	if (!device->power.flags.ignore_parent && parent &&
> +	    state < parent->power.state) {
>  		acpi_handle_debug(device->handle,
>  				  "Cannot transition to %s for parent in %s\n",
>  				  acpi_power_state_string(state),
> -				  acpi_power_state_string(device->parent-
> >power.state));
> +				  acpi_power_state_string(parent->power.state));
>  		return -ENODEV;
>  	}
> 
> Index: linux-pm/drivers/acpi/acpi_platform.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/acpi_platform.c
> +++ linux-pm/drivers/acpi/acpi_platform.c
> @@ -78,7 +78,7 @@ static void acpi_platform_fill_resource(
>  	 * If the device has parent we need to take its resources into
>  	 * account as well because this device might consume part of those.
>  	 */
> -	parent = acpi_get_first_physical_node(adev->parent);
> +	parent = acpi_get_first_physical_node(acpi_dev_parent(adev));
>  	if (parent && dev_is_pci(parent))
>  		dest->parent = pci_find_resource(to_pci_dev(parent), dest);
>  }
> @@ -97,6 +97,7 @@ static void acpi_platform_fill_resource(
>  struct platform_device *acpi_create_platform_device(struct acpi_device *adev,
>  						    const struct property_entry
> *properties)
>  {
> +	struct acpi_device *parent = acpi_dev_parent(adev);
>  	struct platform_device *pdev = NULL;
>  	struct platform_device_info pdevinfo;
>  	struct resource_entry *rentry;
> @@ -137,8 +138,7 @@ struct platform_device *acpi_create_plat
>  	 * attached to it, that physical device should be the parent of the
>  	 * platform device we are about to create.
>  	 */
> -	pdevinfo.parent = adev->parent ?
> -		acpi_get_first_physical_node(adev->parent) : NULL;
> +	pdevinfo.parent = parent ? acpi_get_first_physical_node(parent) : NULL;
>  	pdevinfo.name = dev_name(&adev->dev);
>  	pdevinfo.id = -1;
>  	pdevinfo.res = resources;
> Index: linux-pm/drivers/acpi/acpi_video.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/acpi_video.c
> +++ linux-pm/drivers/acpi/acpi_video.c
> @@ -2030,7 +2030,7 @@ static int acpi_video_bus_add(struct acp
>  	acpi_status status;
> 
>  	status = acpi_walk_namespace(ACPI_TYPE_DEVICE,
> -				device->parent->handle, 1,
> +				acpi_dev_parent(device)->handle, 1,
>  				acpi_video_bus_match, NULL,
>  				device, NULL);
>  	if (status == AE_ALREADY_EXISTS) {
> Index: linux-pm/drivers/acpi/sbs.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/sbs.c
> +++ linux-pm/drivers/acpi/sbs.c
> @@ -632,7 +632,7 @@ static int acpi_sbs_add(struct acpi_devi
> 
>  	mutex_init(&sbs->lock);
> 
> -	sbs->hc = acpi_driver_data(device->parent);
> +	sbs->hc = acpi_driver_data(acpi_dev_parent(device));
>  	sbs->device = device;
>  	strcpy(acpi_device_name(device), ACPI_SBS_DEVICE_NAME);
>  	strcpy(acpi_device_class(device), ACPI_SBS_CLASS);
> Index: linux-pm/drivers/acpi/sbshc.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/sbshc.c
> +++ linux-pm/drivers/acpi/sbshc.c
> @@ -266,7 +266,7 @@ static int acpi_smbus_hc_add(struct acpi
>  	mutex_init(&hc->lock);
>  	init_waitqueue_head(&hc->wait);
> 
> -	hc->ec = acpi_driver_data(device->parent);
> +	hc->ec = acpi_driver_data(acpi_dev_parent(device));
>  	hc->offset = (val >> 8) & 0xff;
>  	hc->query_bit = val & 0xff;
>  	device->driver_data = hc;
> Index: linux-pm/drivers/spi/spi.c
> ===================================================================
> --- linux-pm.orig/drivers/spi/spi.c
> +++ linux-pm/drivers/spi/spi.c
> @@ -4375,7 +4375,7 @@ static int acpi_spi_notify(struct notifi
> 
>  	switch (value) {
>  	case ACPI_RECONFIG_DEVICE_ADD:
> -		ctlr = acpi_spi_find_controller_by_adev(adev->parent);
> +		ctlr = acpi_spi_find_controller_by_adev(acpi_dev_parent(adev));
>  		if (!ctlr)
>  			break;
> 
> Index: linux-pm/drivers/thunderbolt/acpi.c
> ===================================================================
> --- linux-pm.orig/drivers/thunderbolt/acpi.c
> +++ linux-pm/drivers/thunderbolt/acpi.c
> @@ -42,7 +42,7 @@ static acpi_status tb_acpi_add_link(acpi
>  	 */
>  	dev = acpi_get_first_physical_node(adev);
>  	while (!dev) {
> -		adev = adev->parent;
> +		adev = acpi_dev_parent(adev);
>  		if (!adev)
>  			break;
>  		dev = acpi_get_first_physical_node(adev);
> Index: linux-pm/drivers/hv/vmbus_drv.c
> ===================================================================
> --- linux-pm.orig/drivers/hv/vmbus_drv.c
> +++ linux-pm/drivers/hv/vmbus_drv.c
> @@ -2427,7 +2427,8 @@ static int vmbus_acpi_add(struct acpi_de
>  	 * Some ancestor of the vmbus acpi device (Gen1 or Gen2
>  	 * firmware) is the VMOD that has the mmio ranges. Get that.
>  	 */
> -	for (ancestor = device->parent; ancestor; ancestor = ancestor->parent) {
> +	for (ancestor = acpi_dev_parent(device); ancestor;
> +	     ancestor = acpi_dev_parent(ancestor)) {
>  		result = acpi_walk_resources(ancestor->handle, METHOD_NAME__CRS,
>  					     vmbus_walk_resources, NULL);
> 
> Index: linux-pm/drivers/acpi/acpi_amba.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/acpi_amba.c
> +++ linux-pm/drivers/acpi/acpi_amba.c
> @@ -48,6 +48,7 @@ static void amba_register_dummy_clk(void
>  static int amba_handler_attach(struct acpi_device *adev,
>  				const struct acpi_device_id *id)
>  {
> +	struct acpi_device *parent = acpi_dev_parent(adev);
>  	struct amba_device *dev;
>  	struct resource_entry *rentry;
>  	struct list_head resource_list;
> @@ -97,8 +98,8 @@ static int amba_handler_attach(struct ac
>  	 * attached to it, that physical device should be the parent of
>  	 * the amba device we are about to create.
>  	 */
> -	if (adev->parent)
> -		dev->dev.parent = acpi_get_first_physical_node(adev->parent);
> +	if (parent)
> +		dev->dev.parent = acpi_get_first_physical_node(parent);
> 
>  	ACPI_COMPANION_SET(&dev->dev, adev);
> 
> Index: linux-pm/drivers/perf/arm_dsu_pmu.c
> ===================================================================
> --- linux-pm.orig/drivers/perf/arm_dsu_pmu.c
> +++ linux-pm/drivers/perf/arm_dsu_pmu.c
> @@ -639,6 +639,7 @@ static int dsu_pmu_dt_get_cpus(struct de
>  static int dsu_pmu_acpi_get_cpus(struct device *dev, cpumask_t *mask)
>  {
>  #ifdef CONFIG_ACPI
> +	struct acpi_device *parent_adev = acpi_dev_parent(ACPI_COMPANION(dev));
>  	int cpu;
> 
>  	/*
> @@ -653,8 +654,7 @@ static int dsu_pmu_acpi_get_cpus(struct
>  			continue;
> 
>  		acpi_dev = ACPI_COMPANION(cpu_dev);
> -		if (acpi_dev &&
> -			acpi_dev->parent == ACPI_COMPANION(dev)->parent)
> +		if (acpi_dev && acpi_dev_parent(acpi_dev) == parent_adev)
>  			cpumask_set_cpu(cpu, mask);
>  	}
>  #endif
> Index: linux-pm/drivers/perf/qcom_l3_pmu.c
> ===================================================================
> --- linux-pm.orig/drivers/perf/qcom_l3_pmu.c
> +++ linux-pm/drivers/perf/qcom_l3_pmu.c
> @@ -742,7 +742,8 @@ static int qcom_l3_cache_pmu_probe(struc
> 
>  	l3pmu = devm_kzalloc(&pdev->dev, sizeof(*l3pmu), GFP_KERNEL);
>  	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "l3cache_%s_%s",
> -		      acpi_dev->parent->pnp.unique_id, acpi_dev->pnp.unique_id);
> +		      acpi_dev_parent(acpi_dev)->pnp.unique_id,
> +		      acpi_dev->pnp.unique_id);
>  	if (!l3pmu || !name)
>  		return -ENOMEM;
> 
> 
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* RE: [PATCH v2 5/5] ACPI: Drop parent field from struct acpi_device
@ 2022-08-30 21:29       ` Michael Kelley (LINUX)
  0 siblings, 0 replies; 28+ messages in thread
From: Michael Kelley (LINUX) @ 2022-08-30 21:29 UTC (permalink / raw)
  To: Rafael J. Wysocki, Linux ACPI
  Cc: LKML, Andy Shevchenko, Linux PM, KY Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Wei Liu, Dexuan Cui, Mark Brown,
	Andreas Noever, Jamet, Michael, Mika Westerberg, Yehezkel Bernat,
	linux-hyperv, linux-spi, linux-usb, linux-arm-kernel,
	linux-arm-msm, Will Deacon, Mark Rutland, Andy Gross,
	Bjorn Andersson, Konrad Dybcio

From: Rafael J. Wysocki <rjw@rjwysocki.net> Sent: Wednesday, August 24, 2022 10:00 AM
> 
> The parent field in struct acpi_device is, in fact, redundant,
> because the dev.parent field in it effectively points to the same
> object and it is used by the driver core.
> 
> Accordingly, the parent field can be dropped from struct acpi_device
> and for this purpose define acpi_dev_parent() to retrieve a parent
> struct acpi_device pointer from the dev.parent field in struct
> acpi_device.  Next, update all of the users of the parent field
> in struct acpi_device to use acpi_dev_parent() instead of it and
> drop it.
> 
> While at it, drop the ACPI_IS_ROOT_DEVICE() macro that is only used
> in one place in a confusing way.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Acked-by: Mark Brown <broonie@kernel.org>
> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Acked-by: Wei Liu <wei.liu@kernel.org>
> Reviewed-by: Punit Agrawal <punit.agrawal@bytedance.com>

Tested the full patch series in Azure VMs running on Hyper-V --
one x86/x64 and one ARM64.  Testing includes standard VMbus
devices as well as virtual PCI devices (Mellanox CX-5 Virtual
Function).   Verified that ACPI device cache coherence property is
propagated through the hierarchy of devices that the VMbus and
Hyper-V virtual PCI drivers set up.

Tested-by: Michael Kelley <mikelley@microsoft.com>

> ---
> 
> v1 -> v2:
>    * Cover 3 more users of the parent field in struct acpi_device.
>    * Add tags collected so far.
> 
> ---
>  drivers/acpi/acpi_amba.c     |    5 +++--
>  drivers/acpi/acpi_platform.c |    6 +++---
>  drivers/acpi/acpi_video.c    |    2 +-
>  drivers/acpi/device_pm.c     |   19 ++++++++++---------
>  drivers/acpi/property.c      |    6 ++++--
>  drivers/acpi/sbs.c           |    2 +-
>  drivers/acpi/sbshc.c         |    2 +-
>  drivers/acpi/scan.c          |   17 ++++++-----------
>  drivers/hv/vmbus_drv.c       |    3 ++-
>  drivers/perf/arm_dsu_pmu.c   |    4 ++--
>  drivers/perf/qcom_l3_pmu.c   |    3 ++-
>  drivers/spi/spi.c            |    2 +-
>  drivers/thunderbolt/acpi.c   |    2 +-
>  include/acpi/acpi_bus.h      |   10 +++++++++-
>  14 files changed, 46 insertions(+), 37 deletions(-)
> 
> Index: linux-pm/drivers/acpi/scan.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/scan.c
> +++ linux-pm/drivers/acpi/scan.c
> @@ -29,8 +29,6 @@ extern struct acpi_device *acpi_root;
>  #define ACPI_BUS_HID			"LNXSYBUS"
>  #define ACPI_BUS_DEVICE_NAME		"System Bus"
> 
> -#define ACPI_IS_ROOT_DEVICE(device)    (!(device)->parent)
> -
>  #define INVALID_ACPI_HANDLE	((acpi_handle)empty_zero_page)
> 
>  static const char *dummy_hid = "device";
> @@ -1110,7 +1108,7 @@ static void acpi_device_get_busid(struct
>  	 * The device's Bus ID is simply the object name.
>  	 * TBD: Shouldn't this value be unique (within the ACPI namespace)?
>  	 */
> -	if (ACPI_IS_ROOT_DEVICE(device)) {
> +	if (!acpi_dev_parent(device)) {
>  		strcpy(device->pnp.bus_id, "ACPI");
>  		return;
>  	}
> @@ -1646,7 +1644,7 @@ static void acpi_init_coherency(struct a
>  {
>  	unsigned long long cca = 0;
>  	acpi_status status;
> -	struct acpi_device *parent = adev->parent;
> +	struct acpi_device *parent = acpi_dev_parent(adev);
> 
>  	if (parent && parent->flags.cca_seen) {
>  		/*
> @@ -1690,7 +1688,7 @@ static int acpi_check_serial_bus_slave(s
> 
>  static bool acpi_is_indirect_io_slave(struct acpi_device *device)
>  {
> -	struct acpi_device *parent = device->parent;
> +	struct acpi_device *parent = acpi_dev_parent(device);
>  	static const struct acpi_device_id indirect_io_hosts[] = {
>  		{"HISI0191", 0},
>  		{}
> @@ -1767,10 +1765,7 @@ void acpi_init_device_object(struct acpi
>  	INIT_LIST_HEAD(&device->pnp.ids);
>  	device->device_type = type;
>  	device->handle = handle;
> -	if (parent) {
> -		device->parent = parent;
> -		device->dev.parent = &parent->dev;
> -	}
> +	device->dev.parent = parent ? &parent->dev : NULL;
>  	device->dev.release = release;
>  	device->dev.bus = &acpi_bus_type;
>  	fwnode_init(&device->fwnode, &acpi_device_fwnode_ops);
> @@ -1867,8 +1862,8 @@ static int acpi_add_single_object(struct
>  	acpi_device_add_finalize(device);
> 
>  	acpi_handle_debug(handle, "Added as %s, parent %s\n",
> -			  dev_name(&device->dev), device->parent ?
> -				dev_name(&device->parent->dev) : "(null)");
> +			  dev_name(&device->dev), device->dev.parent ?
> +				dev_name(device->dev.parent) : "(null)");
> 
>  	*child = device;
>  	return 0;
> Index: linux-pm/include/acpi/acpi_bus.h
> ===================================================================
> --- linux-pm.orig/include/acpi/acpi_bus.h
> +++ linux-pm/include/acpi/acpi_bus.h
> @@ -365,7 +365,6 @@ struct acpi_device {
>  	int device_type;
>  	acpi_handle handle;		/* no handle for fixed hardware */
>  	struct fwnode_handle fwnode;
> -	struct acpi_device *parent;
>  	struct list_head wakeup_list;
>  	struct list_head del_list;
>  	struct acpi_device_status status;
> @@ -458,6 +457,14 @@ static inline void *acpi_driver_data(str
>  #define to_acpi_device(d)	container_of(d, struct acpi_device, dev)
>  #define to_acpi_driver(d)	container_of(d, struct acpi_driver, drv)
> 
> +static inline struct acpi_device *acpi_dev_parent(struct acpi_device *adev)
> +{
> +	if (adev->dev.parent)
> +		return to_acpi_device(adev->dev.parent);
> +
> +	return NULL;
> +}
> +
>  static inline void acpi_set_device_status(struct acpi_device *adev, u32 sta)
>  {
>  	*((u32 *)&adev->status) = sta;
> @@ -478,6 +485,7 @@ void acpi_initialize_hp_context(struct a
>  /* acpi_device.dev.bus == &acpi_bus_type */
>  extern struct bus_type acpi_bus_type;
> 
> +struct acpi_device *acpi_dev_parent(struct acpi_device *adev);
>  int acpi_bus_for_each_dev(int (*fn)(struct device *, void *), void *data);
>  int acpi_dev_for_each_child(struct acpi_device *adev,
>  			    int (*fn)(struct acpi_device *, void *), void *data);
> Index: linux-pm/drivers/acpi/property.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/property.c
> +++ linux-pm/drivers/acpi/property.c
> @@ -304,8 +304,10 @@ static void acpi_init_of_compatible(stru
>  		ret = acpi_dev_get_property(adev, "compatible",
>  					    ACPI_TYPE_STRING, &of_compatible);
>  		if (ret) {
> -			if (adev->parent
> -			    && adev->parent->flags.of_compatible_ok)
> +			struct acpi_device *parent;
> +
> +			parent = acpi_dev_parent(adev);
> +			if (parent && parent->flags.of_compatible_ok)
>  				goto out;
> 
>  			return;
> Index: linux-pm/drivers/acpi/device_pm.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/device_pm.c
> +++ linux-pm/drivers/acpi/device_pm.c
> @@ -74,6 +74,7 @@ static int acpi_dev_pm_explicit_get(stru
>   */
>  int acpi_device_get_power(struct acpi_device *device, int *state)
>  {
> +	struct acpi_device *parent = acpi_dev_parent(device);
>  	int result = ACPI_STATE_UNKNOWN;
>  	int error;
> 
> @@ -82,8 +83,7 @@ int acpi_device_get_power(struct acpi_de
> 
>  	if (!device->flags.power_manageable) {
>  		/* TBD: Non-recursive algorithm for walking up hierarchy. */
> -		*state = device->parent ?
> -			device->parent->power.state : ACPI_STATE_D0;
> +		*state = parent ? parent->power.state : ACPI_STATE_D0;
>  		goto out;
>  	}
> 
> @@ -122,10 +122,10 @@ int acpi_device_get_power(struct acpi_de
>  	 * point, the fact that the device is in D0 implies that the parent has
>  	 * to be in D0 too, except if ignore_parent is set.
>  	 */
> -	if (!device->power.flags.ignore_parent && device->parent
> -	    && device->parent->power.state == ACPI_STATE_UNKNOWN
> -	    && result == ACPI_STATE_D0)
> -		device->parent->power.state = ACPI_STATE_D0;
> +	if (!device->power.flags.ignore_parent && parent &&
> +	    parent->power.state == ACPI_STATE_UNKNOWN &&
> +	    result == ACPI_STATE_D0)
> +		parent->power.state = ACPI_STATE_D0;
> 
>  	*state = result;
> 
> @@ -159,6 +159,7 @@ static int acpi_dev_pm_explicit_set(stru
>   */
>  int acpi_device_set_power(struct acpi_device *device, int state)
>  {
> +	struct acpi_device *parent = acpi_dev_parent(device);
>  	int target_state = state;
>  	int result = 0;
> 
> @@ -191,12 +192,12 @@ int acpi_device_set_power(struct acpi_de
>  		return -ENODEV;
>  	}
> 
> -	if (!device->power.flags.ignore_parent && device->parent &&
> -	    state < device->parent->power.state) {
> +	if (!device->power.flags.ignore_parent && parent &&
> +	    state < parent->power.state) {
>  		acpi_handle_debug(device->handle,
>  				  "Cannot transition to %s for parent in %s\n",
>  				  acpi_power_state_string(state),
> -				  acpi_power_state_string(device->parent-
> >power.state));
> +				  acpi_power_state_string(parent->power.state));
>  		return -ENODEV;
>  	}
> 
> Index: linux-pm/drivers/acpi/acpi_platform.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/acpi_platform.c
> +++ linux-pm/drivers/acpi/acpi_platform.c
> @@ -78,7 +78,7 @@ static void acpi_platform_fill_resource(
>  	 * If the device has parent we need to take its resources into
>  	 * account as well because this device might consume part of those.
>  	 */
> -	parent = acpi_get_first_physical_node(adev->parent);
> +	parent = acpi_get_first_physical_node(acpi_dev_parent(adev));
>  	if (parent && dev_is_pci(parent))
>  		dest->parent = pci_find_resource(to_pci_dev(parent), dest);
>  }
> @@ -97,6 +97,7 @@ static void acpi_platform_fill_resource(
>  struct platform_device *acpi_create_platform_device(struct acpi_device *adev,
>  						    const struct property_entry
> *properties)
>  {
> +	struct acpi_device *parent = acpi_dev_parent(adev);
>  	struct platform_device *pdev = NULL;
>  	struct platform_device_info pdevinfo;
>  	struct resource_entry *rentry;
> @@ -137,8 +138,7 @@ struct platform_device *acpi_create_plat
>  	 * attached to it, that physical device should be the parent of the
>  	 * platform device we are about to create.
>  	 */
> -	pdevinfo.parent = adev->parent ?
> -		acpi_get_first_physical_node(adev->parent) : NULL;
> +	pdevinfo.parent = parent ? acpi_get_first_physical_node(parent) : NULL;
>  	pdevinfo.name = dev_name(&adev->dev);
>  	pdevinfo.id = -1;
>  	pdevinfo.res = resources;
> Index: linux-pm/drivers/acpi/acpi_video.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/acpi_video.c
> +++ linux-pm/drivers/acpi/acpi_video.c
> @@ -2030,7 +2030,7 @@ static int acpi_video_bus_add(struct acp
>  	acpi_status status;
> 
>  	status = acpi_walk_namespace(ACPI_TYPE_DEVICE,
> -				device->parent->handle, 1,
> +				acpi_dev_parent(device)->handle, 1,
>  				acpi_video_bus_match, NULL,
>  				device, NULL);
>  	if (status == AE_ALREADY_EXISTS) {
> Index: linux-pm/drivers/acpi/sbs.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/sbs.c
> +++ linux-pm/drivers/acpi/sbs.c
> @@ -632,7 +632,7 @@ static int acpi_sbs_add(struct acpi_devi
> 
>  	mutex_init(&sbs->lock);
> 
> -	sbs->hc = acpi_driver_data(device->parent);
> +	sbs->hc = acpi_driver_data(acpi_dev_parent(device));
>  	sbs->device = device;
>  	strcpy(acpi_device_name(device), ACPI_SBS_DEVICE_NAME);
>  	strcpy(acpi_device_class(device), ACPI_SBS_CLASS);
> Index: linux-pm/drivers/acpi/sbshc.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/sbshc.c
> +++ linux-pm/drivers/acpi/sbshc.c
> @@ -266,7 +266,7 @@ static int acpi_smbus_hc_add(struct acpi
>  	mutex_init(&hc->lock);
>  	init_waitqueue_head(&hc->wait);
> 
> -	hc->ec = acpi_driver_data(device->parent);
> +	hc->ec = acpi_driver_data(acpi_dev_parent(device));
>  	hc->offset = (val >> 8) & 0xff;
>  	hc->query_bit = val & 0xff;
>  	device->driver_data = hc;
> Index: linux-pm/drivers/spi/spi.c
> ===================================================================
> --- linux-pm.orig/drivers/spi/spi.c
> +++ linux-pm/drivers/spi/spi.c
> @@ -4375,7 +4375,7 @@ static int acpi_spi_notify(struct notifi
> 
>  	switch (value) {
>  	case ACPI_RECONFIG_DEVICE_ADD:
> -		ctlr = acpi_spi_find_controller_by_adev(adev->parent);
> +		ctlr = acpi_spi_find_controller_by_adev(acpi_dev_parent(adev));
>  		if (!ctlr)
>  			break;
> 
> Index: linux-pm/drivers/thunderbolt/acpi.c
> ===================================================================
> --- linux-pm.orig/drivers/thunderbolt/acpi.c
> +++ linux-pm/drivers/thunderbolt/acpi.c
> @@ -42,7 +42,7 @@ static acpi_status tb_acpi_add_link(acpi
>  	 */
>  	dev = acpi_get_first_physical_node(adev);
>  	while (!dev) {
> -		adev = adev->parent;
> +		adev = acpi_dev_parent(adev);
>  		if (!adev)
>  			break;
>  		dev = acpi_get_first_physical_node(adev);
> Index: linux-pm/drivers/hv/vmbus_drv.c
> ===================================================================
> --- linux-pm.orig/drivers/hv/vmbus_drv.c
> +++ linux-pm/drivers/hv/vmbus_drv.c
> @@ -2427,7 +2427,8 @@ static int vmbus_acpi_add(struct acpi_de
>  	 * Some ancestor of the vmbus acpi device (Gen1 or Gen2
>  	 * firmware) is the VMOD that has the mmio ranges. Get that.
>  	 */
> -	for (ancestor = device->parent; ancestor; ancestor = ancestor->parent) {
> +	for (ancestor = acpi_dev_parent(device); ancestor;
> +	     ancestor = acpi_dev_parent(ancestor)) {
>  		result = acpi_walk_resources(ancestor->handle, METHOD_NAME__CRS,
>  					     vmbus_walk_resources, NULL);
> 
> Index: linux-pm/drivers/acpi/acpi_amba.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/acpi_amba.c
> +++ linux-pm/drivers/acpi/acpi_amba.c
> @@ -48,6 +48,7 @@ static void amba_register_dummy_clk(void
>  static int amba_handler_attach(struct acpi_device *adev,
>  				const struct acpi_device_id *id)
>  {
> +	struct acpi_device *parent = acpi_dev_parent(adev);
>  	struct amba_device *dev;
>  	struct resource_entry *rentry;
>  	struct list_head resource_list;
> @@ -97,8 +98,8 @@ static int amba_handler_attach(struct ac
>  	 * attached to it, that physical device should be the parent of
>  	 * the amba device we are about to create.
>  	 */
> -	if (adev->parent)
> -		dev->dev.parent = acpi_get_first_physical_node(adev->parent);
> +	if (parent)
> +		dev->dev.parent = acpi_get_first_physical_node(parent);
> 
>  	ACPI_COMPANION_SET(&dev->dev, adev);
> 
> Index: linux-pm/drivers/perf/arm_dsu_pmu.c
> ===================================================================
> --- linux-pm.orig/drivers/perf/arm_dsu_pmu.c
> +++ linux-pm/drivers/perf/arm_dsu_pmu.c
> @@ -639,6 +639,7 @@ static int dsu_pmu_dt_get_cpus(struct de
>  static int dsu_pmu_acpi_get_cpus(struct device *dev, cpumask_t *mask)
>  {
>  #ifdef CONFIG_ACPI
> +	struct acpi_device *parent_adev = acpi_dev_parent(ACPI_COMPANION(dev));
>  	int cpu;
> 
>  	/*
> @@ -653,8 +654,7 @@ static int dsu_pmu_acpi_get_cpus(struct
>  			continue;
> 
>  		acpi_dev = ACPI_COMPANION(cpu_dev);
> -		if (acpi_dev &&
> -			acpi_dev->parent == ACPI_COMPANION(dev)->parent)
> +		if (acpi_dev && acpi_dev_parent(acpi_dev) == parent_adev)
>  			cpumask_set_cpu(cpu, mask);
>  	}
>  #endif
> Index: linux-pm/drivers/perf/qcom_l3_pmu.c
> ===================================================================
> --- linux-pm.orig/drivers/perf/qcom_l3_pmu.c
> +++ linux-pm/drivers/perf/qcom_l3_pmu.c
> @@ -742,7 +742,8 @@ static int qcom_l3_cache_pmu_probe(struc
> 
>  	l3pmu = devm_kzalloc(&pdev->dev, sizeof(*l3pmu), GFP_KERNEL);
>  	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "l3cache_%s_%s",
> -		      acpi_dev->parent->pnp.unique_id, acpi_dev->pnp.unique_id);
> +		      acpi_dev_parent(acpi_dev)->pnp.unique_id,
> +		      acpi_dev->pnp.unique_id);
>  	if (!l3pmu || !name)
>  		return -ENOMEM;
> 
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2022-08-30 21:30 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-10 16:11 [PATCH v1 0/5] ACPI: Device enumeration rearrangements and parent field elimination Rafael J. Wysocki
2022-08-10 16:14 ` [PATCH v1 1/5] ACPI: Rename acpi_bus_get/put_acpi_device() Rafael J. Wysocki
2022-08-10 16:30   ` Guenter Roeck
2022-08-10 16:15 ` [PATCH v1 2/5] ACPI: scan: Rename acpi_bus_get_parent() and rearrange it Rafael J. Wysocki
2022-08-12 13:08   ` Punit Agrawal
2022-08-10 16:16 ` [PATCH v1 3/5] ACPI: scan: Rearrange initialization of ACPI device objects Rafael J. Wysocki
2022-08-10 16:17 ` [PATCH v1 4/5] ACPI: scan: Eliminate __acpi_device_add() Rafael J. Wysocki
2022-08-10 16:23 ` [PATCH v1 5/5][RFT] ACPI: Drop parent field from struct acpi_device Rafael J. Wysocki
2022-08-10 16:33   ` Mark Brown
2022-08-10 17:10   ` Mika Westerberg
2022-08-12 15:14   ` Wei Liu
2022-08-24 16:59   ` [PATCH v2 5/5] " Rafael J. Wysocki
2022-08-24 16:59     ` Rafael J. Wysocki
2022-08-24 18:23     ` Andy Shevchenko
2022-08-24 18:23       ` Andy Shevchenko
2022-08-24 18:34       ` Rafael J. Wysocki
2022-08-24 18:34         ` Rafael J. Wysocki
2022-08-27 13:19     ` Hanjun Guo
2022-08-27 13:19       ` Hanjun Guo
2022-08-29 15:54       ` Rafael J. Wysocki
2022-08-29 15:54         ` Rafael J. Wysocki
2022-08-30 21:29     ` Michael Kelley (LINUX)
2022-08-30 21:29       ` Michael Kelley (LINUX)
2022-08-12 13:11 ` [PATCH v1 0/5] ACPI: Device enumeration rearrangements and parent field elimination Punit Agrawal
2022-08-23 16:25   ` Rafael J. Wysocki
2022-08-29 15:21 ` [PATCH v1] ACPI: PM: Fix NULL argument handling in acpi_device_get/set_power() Rafael J. Wysocki
2022-08-29 15:53 ` [PATCH v1] ACPI: Drop redundant acpi_dev_parent() header Rafael J. Wysocki
2022-08-30  2:17   ` Hanjun Guo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.