linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 5/5][RFT] ACPI: Drop parent field from struct acpi_device
       [not found] <12036348.O9o76ZdvQC@kreacher>
@ 2022-08-10 16:23 ` Rafael J. Wysocki
  2022-08-10 16:33   ` Mark Brown
                     ` (3 more replies)
  0 siblings, 4 replies; 10+ 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] 10+ 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; 10+ 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] 10+ 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   ` [PATCH v2 5/5] " Rafael J. Wysocki
  3 siblings, 0 replies; 10+ 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] 10+ 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   ` [PATCH v2 5/5] " Rafael J. Wysocki
  3 siblings, 0 replies; 10+ 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] 10+ 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
                     ` (2 preceding siblings ...)
  2022-08-12 15:14   ` Wei Liu
@ 2022-08-24 16:59   ` Rafael J. Wysocki
  2022-08-24 18:23     ` Andy Shevchenko
                       ` (2 more replies)
  3 siblings, 3 replies; 10+ 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] 10+ messages in thread

* Re: [PATCH v2 5/5] ACPI: Drop parent field from struct acpi_device
  2022-08-24 16:59   ` [PATCH v2 5/5] " Rafael J. Wysocki
@ 2022-08-24 18:23     ` Andy Shevchenko
  2022-08-24 18:34       ` Rafael J. Wysocki
  2022-08-27 13:19     ` Hanjun Guo
  2022-08-30 21:29     ` Michael Kelley (LINUX)
  2 siblings, 1 reply; 10+ 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] 10+ 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
  0 siblings, 0 replies; 10+ 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] 10+ messages in thread

* Re: [PATCH v2 5/5] ACPI: Drop parent field from struct acpi_device
  2022-08-24 16:59   ` [PATCH v2 5/5] " Rafael J. Wysocki
  2022-08-24 18:23     ` Andy Shevchenko
@ 2022-08-27 13:19     ` Hanjun Guo
  2022-08-29 15:54       ` Rafael J. Wysocki
  2022-08-30 21:29     ` Michael Kelley (LINUX)
  2 siblings, 1 reply; 10+ 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] 10+ 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
  0 siblings, 0 replies; 10+ 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] 10+ messages in thread

* RE: [PATCH v2 5/5] ACPI: Drop parent field from struct acpi_device
  2022-08-24 16:59   ` [PATCH v2 5/5] " Rafael J. Wysocki
  2022-08-24 18:23     ` Andy Shevchenko
  2022-08-27 13:19     ` Hanjun Guo
@ 2022-08-30 21:29     ` Michael Kelley (LINUX)
  2 siblings, 0 replies; 10+ 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] 10+ messages in thread

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

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <12036348.O9o76ZdvQC@kreacher>
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 18:23     ` Andy Shevchenko
2022-08-24 18:34       ` Rafael J. Wysocki
2022-08-27 13:19     ` Hanjun Guo
2022-08-29 15:54       ` Rafael J. Wysocki
2022-08-30 21:29     ` Michael Kelley (LINUX)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).