[PATCH v1 0/3] ACPI: glue: Rework ACPI companion lookup related to device type

[PATCH v1 0/3] ACPI: glue: Rework ACPI companion lookup related to device type

[Rafael J. Wysocki]

Hi All,

The ACPI "glue" code is responsible for binding Linux devices (like platform
devices for one example) to their ACPI companions (represented by struct
acpi_device objects).  This is done by acpi_device_notify() when the given
device is registered.

If the device's ACPI companion is set already at that point, the binding
simply adds the device to the list of "physical nodes" associated with its
ACPI companion.  That's the case for PCI devices and platform devices, for
example.  However, there are cases in which the ACPI companion is not set
upfront and it needs to be looked up via "lookup helpers" registered by
the subsystems the devices in question belong to.  This series changes
the "lookup helpers" representation and handling.

Before the changes, the "lookup helpers" are represented by struct
acpi_bus_type objects which is doubly confusing because they have nothing
to do with bus types and because the "acpi_bus_type" name is also used
elsewhere.  Patch [1/3] renames struct acpi_bus_type to struct acpi_dev_type
because all of the existing "lookup helpers" operate on device types.

Next, the registration and unregistration functions for "lookup helpers"
are defined to return an int, but all of their existing users effectively
discard their return values (they are discarded right away or passed up the
call chain and then discarded).  Patch [2/3] changes the return type of
those functions to void.

Finally, since all of the existing "lookup helpers" match dev->type to
a certain device type, they can be rearranged to point to the device
types in question and provide a .find_companion() callback, and an
additional .setup() callback in one case.  This is done by patch [3/3]
and allows all of the affected code to be simplified quite a bit.

Thanks!

[PATCH v1 1/3] ACPI: glue: Rename struct acpi_bus_type to acpi_dev_type

[Rafael J. Wysocki]

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

The name of struct acpi_bus_type is misleading because that data type
is only used for ACPI companion lookup for specific types of devices
and it has nothing to do with buses in principle.  Moreover, it clashes
with the name of the acpi_bus_type variable (that is a const struct
bus_type) which increases the overall confusion.

For this reason, rename struct acpi_bus_type to struct acpi_dev_type and
rename the related functions and variables accordingly in all of the
code using it.

No intentional functional impact beyond messaging.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/glue.c         | 32 ++++++++++++++++----------------
 drivers/gpu/drm/drm_sysfs.c | 10 +++++-----
 drivers/thunderbolt/acpi.c  | 10 +++++-----
 drivers/usb/core/usb-acpi.c | 10 +++++-----
 include/acpi/acpi_bus.h     | 10 +++++-----
 5 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
index 0959f58cb77c..817123c968ba 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -23,52 +23,52 @@
 
 #include "internal.h"
 
-static LIST_HEAD(bus_type_list);
-static DECLARE_RWSEM(bus_type_sem);
+static LIST_HEAD(dev_type_list);
+static DECLARE_RWSEM(dev_type_sem);
 
 #define PHYSICAL_NODE_STRING "physical_node"
 #define PHYSICAL_NODE_NAME_SIZE (sizeof(PHYSICAL_NODE_STRING) + 10)
 
-int register_acpi_bus_type(struct acpi_bus_type *type)
+int register_acpi_dev_type(struct acpi_dev_type *type)
 {
 	if (acpi_disabled)
 		return -ENODEV;
 	if (type && type->match && type->find_companion) {
-		down_write(&bus_type_sem);
-		list_add_tail(&type->list, &bus_type_list);
-		up_write(&bus_type_sem);
-		pr_info("bus type %s registered\n", type->name);
+		down_write(&dev_type_sem);
+		list_add_tail(&type->list, &dev_type_list);
+		up_write(&dev_type_sem);
+		pr_info("device type %s registered\n", type->name);
 		return 0;
 	}
 	return -ENODEV;
 }
-EXPORT_SYMBOL_GPL(register_acpi_bus_type);
+EXPORT_SYMBOL_GPL(register_acpi_dev_type);
 
-int unregister_acpi_bus_type(struct acpi_bus_type *type)
+int unregister_acpi_dev_type(struct acpi_dev_type *type)
 {
 	if (acpi_disabled)
 		return 0;
 	if (type) {
-		down_write(&bus_type_sem);
+		down_write(&dev_type_sem);
 		list_del_init(&type->list);
-		up_write(&bus_type_sem);
-		pr_info("bus type %s unregistered\n", type->name);
+		up_write(&dev_type_sem);
+		pr_info("device type %s unregistered\n", type->name);
 		return 0;
 	}
 	return -ENODEV;
 }
-EXPORT_SYMBOL_GPL(unregister_acpi_bus_type);
+EXPORT_SYMBOL_GPL(unregister_acpi_dev_type);
 
 static struct acpi_device *acpi_companion_lookup(struct device *dev)
 {
-	struct acpi_bus_type *type;
+	struct acpi_dev_type *type;
 
 	if (!dev->type)
 		return NULL;
 
-	guard(rwsem_read)(&bus_type_sem);
+	guard(rwsem_read)(&dev_type_sem);
 
-	list_for_each_entry(type, &bus_type_list, list) {
+	list_for_each_entry(type, &dev_type_list, list) {
 		struct acpi_device *adev;
 
 		if (!type->match(dev))
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 7b16d32b6ff6..585b26515d49 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -64,7 +64,7 @@ static struct device_type drm_sysfs_device_connector = {
 struct class *drm_class;
 
 #ifdef CONFIG_ACPI
-static bool drm_connector_acpi_bus_match(struct device *dev)
+static bool drm_connector_acpi_dev_type_match(struct device *dev)
 {
 	return dev->type == &drm_sysfs_device_connector;
 }
@@ -76,20 +76,20 @@ static struct acpi_device *drm_connector_acpi_find_companion(struct device *dev)
 	return to_acpi_device_node(connector->fwnode);
 }
 
-static struct acpi_bus_type drm_connector_acpi_bus = {
+static struct acpi_dev_type drm_connector_acpi_dev_type = {
 	.name = "drm_connector",
-	.match = drm_connector_acpi_bus_match,
+	.match = drm_connector_acpi_dev_type_match,
 	.find_companion = drm_connector_acpi_find_companion,
 };
 
 static void drm_sysfs_acpi_register(void)
 {
-	register_acpi_bus_type(&drm_connector_acpi_bus);
+	register_acpi_dev_type(&drm_connector_acpi_dev_type);
 }
 
 static void drm_sysfs_acpi_unregister(void)
 {
-	unregister_acpi_bus_type(&drm_connector_acpi_bus);
+	unregister_acpi_dev_type(&drm_connector_acpi_dev_type);
 }
 #else
 static void drm_sysfs_acpi_register(void) { }
diff --git a/drivers/thunderbolt/acpi.c b/drivers/thunderbolt/acpi.c
index be91bc604900..958837e3736e 100644
--- a/drivers/thunderbolt/acpi.c
+++ b/drivers/thunderbolt/acpi.c
@@ -280,7 +280,7 @@ int tb_acpi_power_off_retimers(struct tb_port *port)
 	return tb_acpi_retimer_set_power(port, false);
 }
 
-static bool tb_acpi_bus_match(struct device *dev)
+static bool tb_acpi_dev_type_match(struct device *dev)
 {
 	return tb_is_switch(dev) || tb_is_usb4_port_device(dev);
 }
@@ -351,19 +351,19 @@ static void tb_acpi_setup(struct device *dev)
 		usb4->can_offline = true;
 }
 
-static struct acpi_bus_type tb_acpi_bus = {
+static struct acpi_dev_type tb_acpi_dev_type = {
 	.name = "thunderbolt",
-	.match = tb_acpi_bus_match,
+	.match = tb_acpi_dev_type_match,
 	.find_companion = tb_acpi_find_companion,
 	.setup = tb_acpi_setup,
 };
 
 int tb_acpi_init(void)
 {
-	return register_acpi_bus_type(&tb_acpi_bus);
+	return register_acpi_dev_type(&tb_acpi_dev_type);
 }
 
 void tb_acpi_exit(void)
 {
-	unregister_acpi_bus_type(&tb_acpi_bus);
+	unregister_acpi_dev_type(&tb_acpi_dev_type);
 }
diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
index 489dbdc96f94..f00e8c72ebe6 100644
--- a/drivers/usb/core/usb-acpi.c
+++ b/drivers/usb/core/usb-acpi.c
@@ -364,23 +364,23 @@ static struct acpi_device *usb_acpi_find_companion(struct device *dev)
 	return NULL;
 }
 
-static bool usb_acpi_bus_match(struct device *dev)
+static bool usb_acpi_dev_type_match(struct device *dev)
 {
 	return is_usb_device(dev) || is_usb_port(dev);
 }
 
-static struct acpi_bus_type usb_acpi_bus = {
+static struct acpi_dev_type usb_acpi_dev_type = {
 	.name = "USB",
-	.match = usb_acpi_bus_match,
+	.match = usb_acpi_dev_type_match,
 	.find_companion = usb_acpi_find_companion,
 };
 
 int usb_acpi_register(void)
 {
-	return register_acpi_bus_type(&usb_acpi_bus);
+	return register_acpi_dev_type(&usb_acpi_dev_type);
 }
 
 void usb_acpi_unregister(void)
 {
-	unregister_acpi_bus_type(&usb_acpi_bus);
+	unregister_acpi_dev_type(&usb_acpi_dev_type);
 }
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index f59b2de865e6..8524ceb52b24 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -659,15 +659,15 @@ static inline bool acpi_device_enumerated(struct acpi_device *adev)
 /*
  * Bind physical devices with ACPI devices
  */
-struct acpi_bus_type {
+struct acpi_dev_type {
 	struct list_head list;
 	const char *name;
 	bool (*match)(struct device *dev);
 	struct acpi_device * (*find_companion)(struct device *);
 	void (*setup)(struct device *);
 };
-int register_acpi_bus_type(struct acpi_bus_type *);
-int unregister_acpi_bus_type(struct acpi_bus_type *);
+int register_acpi_dev_type(struct acpi_dev_type *type);
+int unregister_acpi_dev_type(struct acpi_dev_type *type);
 int acpi_bind_one(struct device *dev, struct acpi_device *adev);
 void acpi_unbind_one(struct device *dev);
 
@@ -978,8 +978,8 @@ static inline struct device *acpi_bus_get_primary_device(struct acpi_device *ade
 	return NULL;
 }
 
-static inline int register_acpi_bus_type(void *bus) { return 0; }
-static inline int unregister_acpi_bus_type(void *bus) { return 0; }
+static inline int register_acpi_dev_type(void *type) { return 0; }
+static inline int unregister_acpi_dev_type(void *type) { return 0; }
 
 static inline int acpi_wait_for_acpi_ipmi(void) { return 0; }
 
-- 
2.51.0

Re: [PATCH v1 1/3] ACPI: glue: Rename struct acpi_bus_type to acpi_dev_type

[sashiko-bot]

> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> 
> The name of struct acpi_bus_type is misleading because that data type
> is only used for ACPI companion lookup for specific types of devices
> and it has nothing to do with buses in principle.  Moreover, it clashes
> with the name of the acpi_bus_type variable (that is a const struct
> bus_type) which increases the overall confusion.
> 
> For this reason, rename struct acpi_bus_type to struct acpi_dev_type and
> rename the related functions and variables accordingly in all of the
> code using it.
> 
> No intentional functional impact beyond messaging.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/3471545.44csPzL39Z@rafael.j.wysocki?part=1

[PATCH v1 2/3] ACPI: glue: Change (un)register_acpi_dev_type() to void functions

[Rafael J. Wysocki]

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

The register_acpi_dev_type() and unregister_acpi_dev_type() return
values are effectively ignored in all cases (either directly or up
in the call chain), so make them both void and adjust two callers of
register_acpi_dev_type() accordingly.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/glue.c         | 14 ++++++--------
 drivers/thunderbolt/acpi.c  |  4 ++--
 drivers/thunderbolt/tb.h    |  4 ++--
 drivers/usb/core/usb-acpi.c |  4 ++--
 drivers/usb/core/usb.h      |  4 ++--
 include/acpi/acpi_bus.h     |  8 ++++----
 6 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
index 817123c968ba..51caaa73a43c 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -29,33 +29,31 @@ static DECLARE_RWSEM(dev_type_sem);
 #define PHYSICAL_NODE_STRING "physical_node"
 #define PHYSICAL_NODE_NAME_SIZE (sizeof(PHYSICAL_NODE_STRING) + 10)
 
-int register_acpi_dev_type(struct acpi_dev_type *type)
+void register_acpi_dev_type(struct acpi_dev_type *type)
 {
 	if (acpi_disabled)
-		return -ENODEV;
+		return;
+
 	if (type && type->match && type->find_companion) {
 		down_write(&dev_type_sem);
 		list_add_tail(&type->list, &dev_type_list);
 		up_write(&dev_type_sem);
 		pr_info("device type %s registered\n", type->name);
-		return 0;
 	}
-	return -ENODEV;
 }
 EXPORT_SYMBOL_GPL(register_acpi_dev_type);
 
-int unregister_acpi_dev_type(struct acpi_dev_type *type)
+void unregister_acpi_dev_type(struct acpi_dev_type *type)
 {
 	if (acpi_disabled)
-		return 0;
+		return;
+
 	if (type) {
 		down_write(&dev_type_sem);
 		list_del_init(&type->list);
 		up_write(&dev_type_sem);
 		pr_info("device type %s unregistered\n", type->name);
-		return 0;
 	}
-	return -ENODEV;
 }
 EXPORT_SYMBOL_GPL(unregister_acpi_dev_type);
 
diff --git a/drivers/thunderbolt/acpi.c b/drivers/thunderbolt/acpi.c
index 958837e3736e..209419e3844f 100644
--- a/drivers/thunderbolt/acpi.c
+++ b/drivers/thunderbolt/acpi.c
@@ -358,9 +358,9 @@ static struct acpi_dev_type tb_acpi_dev_type = {
 	.setup = tb_acpi_setup,
 };
 
-int tb_acpi_init(void)
+void tb_acpi_init(void)
 {
-	return register_acpi_dev_type(&tb_acpi_dev_type);
+	register_acpi_dev_type(&tb_acpi_dev_type);
 }
 
 void tb_acpi_exit(void)
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index e86845762e22..ef22cd247104 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -1520,7 +1520,7 @@ bool tb_acpi_may_tunnel_dp(void);
 bool tb_acpi_may_tunnel_pcie(void);
 bool tb_acpi_is_xdomain_allowed(void);
 
-int tb_acpi_init(void);
+void tb_acpi_init(void);
 void tb_acpi_exit(void);
 int tb_acpi_power_on_retimers(struct tb_port *port);
 int tb_acpi_power_off_retimers(struct tb_port *port);
@@ -1533,7 +1533,7 @@ static inline bool tb_acpi_may_tunnel_dp(void) { return true; }
 static inline bool tb_acpi_may_tunnel_pcie(void) { return true; }
 static inline bool tb_acpi_is_xdomain_allowed(void) { return true; }
 
-static inline int tb_acpi_init(void) { return 0; }
+static inline void tb_acpi_init(void) { }
 static inline void tb_acpi_exit(void) { }
 static inline int tb_acpi_power_on_retimers(struct tb_port *port) { return 0; }
 static inline int tb_acpi_power_off_retimers(struct tb_port *port) { return 0; }
diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
index f00e8c72ebe6..05881260a4b7 100644
--- a/drivers/usb/core/usb-acpi.c
+++ b/drivers/usb/core/usb-acpi.c
@@ -375,9 +375,9 @@ static struct acpi_dev_type usb_acpi_dev_type = {
 	.find_companion = usb_acpi_find_companion,
 };
 
-int usb_acpi_register(void)
+void usb_acpi_register(void)
 {
-	return register_acpi_dev_type(&usb_acpi_dev_type);
+	register_acpi_dev_type(&usb_acpi_dev_type);
 }
 
 void usb_acpi_unregister(void)
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index a9b37aeb515b..d025d6d9d346 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -207,11 +207,11 @@ extern void usb_hub_adjust_deviceremovable(struct usb_device *hdev,
 		struct usb_hub_descriptor *desc);
 
 #ifdef CONFIG_ACPI
-extern int usb_acpi_register(void);
+extern void usb_acpi_register(void);
 extern void usb_acpi_unregister(void);
 extern acpi_handle usb_get_hub_port_acpi_handle(struct usb_device *hdev,
 	int port1);
 #else
-static inline int usb_acpi_register(void) { return 0; };
+static inline void usb_acpi_register(void) { }
 static inline void usb_acpi_unregister(void) { };
 #endif
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 8524ceb52b24..da2262d220e0 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -666,8 +666,8 @@ struct acpi_dev_type {
 	struct acpi_device * (*find_companion)(struct device *);
 	void (*setup)(struct device *);
 };
-int register_acpi_dev_type(struct acpi_dev_type *type);
-int unregister_acpi_dev_type(struct acpi_dev_type *type);
+void register_acpi_dev_type(struct acpi_dev_type *type);
+void unregister_acpi_dev_type(struct acpi_dev_type *type);
 int acpi_bind_one(struct device *dev, struct acpi_device *adev);
 void acpi_unbind_one(struct device *dev);
 
@@ -978,8 +978,8 @@ static inline struct device *acpi_bus_get_primary_device(struct acpi_device *ade
 	return NULL;
 }
 
-static inline int register_acpi_dev_type(void *type) { return 0; }
-static inline int unregister_acpi_dev_type(void *type) { return 0; }
+static inline void register_acpi_dev_type(void *type) { }
+static inline void unregister_acpi_dev_type(void *type) { }
 
 static inline int acpi_wait_for_acpi_ipmi(void) { return 0; }
 
-- 
2.51.0

Re: [PATCH v1 2/3] ACPI: glue: Change (un)register_acpi_dev_type() to void functions

[sashiko-bot]

> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> 
> The register_acpi_dev_type() and unregister_acpi_dev_type() return
> values are effectively ignored in all cases (either directly or up
> in the call chain), so make them both void and adjust two callers of
> register_acpi_dev_type() accordingly.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/3471545.44csPzL39Z@rafael.j.wysocki?part=2

[PATCH v1 3/3] ACPI: glue: Simplify struct acpi_dev_type and its usage

[Rafael J. Wysocki]

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

All of the current users of struct acpi_dev_type match the device type
in their .match() callbacks and all of them use const device type
pointers for that which means that the structure can be simplified and
used in a more straightforward way.

Namely, modify struct acpi_dev_type to contain a const device type
pointer instead of the .match() callback and remove the name field
from it.  Next, update the users of struct acpi_dev_type to register
a separate object of that type for each device type that requires an
ACPI companion lookup.

Since the register_acpi_dev_type() and unregister_acpi_dev_type()
function bodies are rewiritten entirely anyway, make them use
guard(rwsem_write) instead of down_write()/up_write() pairs and
print the messages under the semaphore to ensure that the struct
acpi_dev_type objects they refer to will not be freed prematurely.

No intentional functional impact beyond messaging.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/glue.c         | 33 ++++++++++--------
 drivers/gpu/drm/drm_sysfs.c |  8 +----
 drivers/thunderbolt/acpi.c  | 69 +++++++++++++++++--------------------
 drivers/usb/core/usb-acpi.c | 47 ++++++++++---------------
 include/acpi/acpi_bus.h     |  4 +--
 5 files changed, 72 insertions(+), 89 deletions(-)

diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
index 51caaa73a43c..8b75a8c805a4 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -31,29 +31,32 @@ static DECLARE_RWSEM(dev_type_sem);
 
 void register_acpi_dev_type(struct acpi_dev_type *type)
 {
-	if (acpi_disabled)
+	if (acpi_disabled || !type)
 		return;
 
-	if (type && type->match && type->find_companion) {
-		down_write(&dev_type_sem);
-		list_add_tail(&type->list, &dev_type_list);
-		up_write(&dev_type_sem);
-		pr_info("device type %s registered\n", type->name);
-	}
+	INIT_LIST_HEAD(&type->list);
+
+	if (!type->dev_type || !type->find_companion)
+		return;
+
+	guard(rwsem_write)(&dev_type_sem);
+
+	list_add_tail(&type->list, &dev_type_list);
+
+	pr_info("Device type %s added to lookup list\n", type->dev_type->name);
 }
 EXPORT_SYMBOL_GPL(register_acpi_dev_type);
 
 void unregister_acpi_dev_type(struct acpi_dev_type *type)
 {
-	if (acpi_disabled)
+	if (acpi_disabled || !type || !type->dev_type || !type->find_companion)
 		return;
 
-	if (type) {
-		down_write(&dev_type_sem);
-		list_del_init(&type->list);
-		up_write(&dev_type_sem);
-		pr_info("device type %s unregistered\n", type->name);
-	}
+	guard(rwsem_write)(&dev_type_sem);
+
+	list_del(&type->list);
+
+	pr_info("Device type %s removed from lookup list\n", type->dev_type->name);
 }
 EXPORT_SYMBOL_GPL(unregister_acpi_dev_type);
 
@@ -69,7 +72,7 @@ static struct acpi_device *acpi_companion_lookup(struct device *dev)
 	list_for_each_entry(type, &dev_type_list, list) {
 		struct acpi_device *adev;
 
-		if (!type->match(dev))
+		if (type->dev_type != dev->type)
 			continue;
 
 		adev = type->find_companion(dev);
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 585b26515d49..63e0b07e9926 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -64,11 +64,6 @@ static struct device_type drm_sysfs_device_connector = {
 struct class *drm_class;
 
 #ifdef CONFIG_ACPI
-static bool drm_connector_acpi_dev_type_match(struct device *dev)
-{
-	return dev->type == &drm_sysfs_device_connector;
-}
-
 static struct acpi_device *drm_connector_acpi_find_companion(struct device *dev)
 {
 	struct drm_connector *connector = to_drm_connector(dev);
@@ -77,8 +72,7 @@ static struct acpi_device *drm_connector_acpi_find_companion(struct device *dev)
 }
 
 static struct acpi_dev_type drm_connector_acpi_dev_type = {
-	.name = "drm_connector",
-	.match = drm_connector_acpi_dev_type_match,
+	.dev_type = &drm_sysfs_device_connector,
 	.find_companion = drm_connector_acpi_find_companion,
 };
 
diff --git a/drivers/thunderbolt/acpi.c b/drivers/thunderbolt/acpi.c
index 209419e3844f..05cc823a26bb 100644
--- a/drivers/thunderbolt/acpi.c
+++ b/drivers/thunderbolt/acpi.c
@@ -280,13 +280,9 @@ int tb_acpi_power_off_retimers(struct tb_port *port)
 	return tb_acpi_retimer_set_power(port, false);
 }
 
-static bool tb_acpi_dev_type_match(struct device *dev)
-{
-	return tb_is_switch(dev) || tb_is_usb4_port_device(dev);
-}
-
-static struct acpi_device *tb_acpi_switch_find_companion(struct tb_switch *sw)
+static struct acpi_device *tb_acpi_switch_find_companion(struct device *dev)
 {
+	struct tb_switch *sw = tb_to_switch(dev);
 	struct tb_switch *parent_sw = tb_switch_parent(sw);
 	struct acpi_device *adev = NULL;
 
@@ -314,56 +310,55 @@ static struct acpi_device *tb_acpi_switch_find_companion(struct tb_switch *sw)
 	return adev;
 }
 
-static struct acpi_device *tb_acpi_find_companion(struct device *dev)
+static struct acpi_device *tb_acpi_usb4_port_find_companion(struct device *dev)
 {
-	/*
-	 * The Thunderbolt/USB4 hierarchy looks like following:
-	 *
-	 * Device (NHI)
-	 *   Device (HR)		// Host router _ADR == 0
-	 *      Device (DFP0)		// Downstream port _ADR == lane 0 adapter
-	 *        Device (DR)		// Device router _ADR == 0
-	 *          Device (UFP)	// Upstream port _ADR == lane 0 adapter
-	 *      Device (DFP1)		// Downstream port _ADR == lane 0 adapter number
-	 *
-	 * At the moment we bind the host router to the corresponding
-	 * Linux device.
-	 */
-	if (tb_is_switch(dev))
-		return tb_acpi_switch_find_companion(tb_to_switch(dev));
-	if (tb_is_usb4_port_device(dev))
-		return acpi_find_child_by_adr(ACPI_COMPANION(dev->parent),
-					      tb_to_usb4_port_device(dev)->port->port);
-	return NULL;
+	return acpi_find_child_by_adr(ACPI_COMPANION(dev->parent),
+				      tb_to_usb4_port_device(dev)->port->port);
 }
 
-static void tb_acpi_setup(struct device *dev)
+static void tb_acpi_usb4_port_setup(struct device *dev)
 {
 	struct acpi_device *adev = ACPI_COMPANION(dev);
 	struct usb4_port *usb4 = tb_to_usb4_port_device(dev);
 
-	if (!adev || !usb4)
-		return;
-
 	if (acpi_check_dsm(adev->handle, &retimer_dsm_guid, 1,
 			   BIT(RETIMER_DSM_QUERY_ONLINE_STATE) |
 			   BIT(RETIMER_DSM_SET_ONLINE_STATE)))
 		usb4->can_offline = true;
 }
 
-static struct acpi_dev_type tb_acpi_dev_type = {
-	.name = "thunderbolt",
-	.match = tb_acpi_dev_type_match,
-	.find_companion = tb_acpi_find_companion,
-	.setup = tb_acpi_setup,
+/*
+ * The Thunderbolt/USB4 hierarchy looks like following:
+ *
+ * Device (NHI)
+ *   Device (HR)		// Host router _ADR == 0
+ *      Device (DFP0)		// Downstream port _ADR == lane 0 adapter
+ *        Device (DR)		// Device router _ADR == 0
+ *          Device (UFP)	// Upstream port _ADR == lane 0 adapter
+ *      Device (DFP1)		// Downstream port _ADR == lane 0 adapter number
+ *
+ * At the moment we bind the host router to the corresponding
+ * Linux device.
+ */
+static struct acpi_dev_type tb_switch_acpi_dev_type = {
+	.dev_type = &tb_switch_type,
+	.find_companion = tb_acpi_switch_find_companion,
+};
+
+static struct acpi_dev_type tb_usb4_port_acpi_dev_type = {
+	.dev_type = &usb4_port_device_type,
+	.find_companion = tb_acpi_usb4_port_find_companion,
+	.setup = tb_acpi_usb4_port_setup,
 };
 
 void tb_acpi_init(void)
 {
-	register_acpi_dev_type(&tb_acpi_dev_type);
+	register_acpi_dev_type(&tb_switch_acpi_dev_type);
+	register_acpi_dev_type(&tb_usb4_port_acpi_dev_type);
 }
 
 void tb_acpi_exit(void)
 {
-	unregister_acpi_dev_type(&tb_acpi_dev_type);
+	unregister_acpi_dev_type(&tb_usb4_port_acpi_dev_type);
+	unregister_acpi_dev_type(&tb_switch_acpi_dev_type);
 }
diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
index 05881260a4b7..d12eda176034 100644
--- a/drivers/usb/core/usb-acpi.c
+++ b/drivers/usb/core/usb-acpi.c
@@ -278,9 +278,9 @@ usb_acpi_get_companion_for_port(struct usb_port *port_dev)
 	return acpi_find_child_by_adr(adev, port1);
 }
 
-static struct acpi_device *
-usb_acpi_find_companion_for_port(struct usb_port *port_dev)
+static struct acpi_device *usb_acpi_find_companion_for_port(struct device *dev)
 {
+	struct usb_port *port_dev = to_usb_port(dev);
 	struct acpi_device *adev;
 
 	adev = usb_acpi_get_companion_for_port(port_dev);
@@ -292,9 +292,9 @@ usb_acpi_find_companion_for_port(struct usb_port *port_dev)
 	return adev;
 }
 
-static struct acpi_device *
-usb_acpi_find_companion_for_device(struct usb_device *udev)
+static struct acpi_device *usb_acpi_find_companion_for_device(struct device *dev)
 {
+	struct usb_device *udev = to_usb_device(dev);
 	struct acpi_device *adev;
 	struct usb_port *port_dev;
 	struct usb_hub *hub;
@@ -326,7 +326,17 @@ usb_acpi_find_companion_for_device(struct usb_device *udev)
 	return usb_acpi_get_companion_for_port(port_dev);
 }
 
-static struct acpi_device *usb_acpi_find_companion(struct device *dev)
+static struct acpi_dev_type usb_dev_acpi_dev_type = {
+	.dev_type = &usb_device_type,
+	.find_companion = usb_acpi_find_companion_for_device,
+};
+
+static struct acpi_dev_type usb_port_acpi_dev_type = {
+	.dev_type = &usb_port_device_type,
+	.find_companion = usb_acpi_find_companion_for_port,
+};
+
+void usb_acpi_register(void)
 {
 	/*
 	 * The USB hierarchy like following:
@@ -356,31 +366,12 @@ static struct acpi_device *usb_acpi_find_companion(struct device *dev)
 	 * devices yet, for that we would need to assign companions to
 	 * devices corresponding to USB interfaces.
 	 */
-	if (is_usb_device(dev))
-		return usb_acpi_find_companion_for_device(to_usb_device(dev));
-	else if (is_usb_port(dev))
-		return usb_acpi_find_companion_for_port(to_usb_port(dev));
-
-	return NULL;
-}
-
-static bool usb_acpi_dev_type_match(struct device *dev)
-{
-	return is_usb_device(dev) || is_usb_port(dev);
-}
-
-static struct acpi_dev_type usb_acpi_dev_type = {
-	.name = "USB",
-	.match = usb_acpi_dev_type_match,
-	.find_companion = usb_acpi_find_companion,
-};
-
-void usb_acpi_register(void)
-{
-	register_acpi_dev_type(&usb_acpi_dev_type);
+	register_acpi_dev_type(&usb_dev_acpi_dev_type);
+	register_acpi_dev_type(&usb_port_acpi_dev_type);
 }
 
 void usb_acpi_unregister(void)
 {
-	unregister_acpi_dev_type(&usb_acpi_dev_type);
+	unregister_acpi_dev_type(&usb_port_acpi_dev_type);
+	unregister_acpi_dev_type(&usb_dev_acpi_dev_type);
 }
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index da2262d220e0..8edf82b46157 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -661,11 +661,11 @@ static inline bool acpi_device_enumerated(struct acpi_device *adev)
  */
 struct acpi_dev_type {
 	struct list_head list;
-	const char *name;
-	bool (*match)(struct device *dev);
+	const struct device_type *dev_type;
 	struct acpi_device * (*find_companion)(struct device *);
 	void (*setup)(struct device *);
 };
+
 void register_acpi_dev_type(struct acpi_dev_type *type);
 void unregister_acpi_dev_type(struct acpi_dev_type *type);
 int acpi_bind_one(struct device *dev, struct acpi_device *adev);
-- 
2.51.0

Re: [PATCH v1 3/3] ACPI: glue: Simplify struct acpi_dev_type and its usage

[sashiko-bot]

> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> 
> All of the current users of struct acpi_dev_type match the device type
> in their .match() callbacks and all of them use const device type
> pointers for that which means that the structure can be simplified and
> used in a more straightforward way.
> 
> Namely, modify struct acpi_dev_type to contain a const device type
> pointer instead of the .match() callback and remove the name field
> from it.  Next, update the users of struct acpi_dev_type to register
> a separate object of that type for each device type that requires an
> ACPI companion lookup.
> 
> Since the register_acpi_dev_type() and unregister_acpi_dev_type()
> function bodies are rewiritten entirely anyway, make them use
> guard(rwsem_write) instead of down_write()/up_write() pairs and
> print the messages under the semaphore to ensure that the struct
> acpi_dev_type objects they refer to will not be freed prematurely.
> 
> No intentional functional impact beyond messaging.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/3471545.44csPzL39Z@rafael.j.wysocki?part=3