linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/6] ACPI: glue / driver core: Eliminate acpi_platform_notify() and split device_platform_notify()
@ 2021-07-12 17:19 Rafael J. Wysocki
  2021-07-12 17:23 ` [PATCH v1 1/6] ACPI: glue: Rearrange acpi_device_notify() Rafael J. Wysocki
                   ` (7 more replies)
  0 siblings, 8 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2021-07-12 17:19 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rafael J. Wysocki, Andy Shevchenko, Linux ACPI, LKML, Krogerus, Heikki

Hi Greg et al,

This series doesn't change functionality (at least not intentionally), but
it get rids of a few unneeded checks, parameter passing etc.

Patches [1-2/6] simplify the ACPI "glue" code.

Patch [3/6] renames a couple of ACPI functions to avoid name collisions going
forward.

Patch [4/6] gets rid of acpi_platform_notify().

Patch [5/6] rearranges the software nodes code along the lines of what happens
to the ACPI "glue" code in patch [4/6].

Patch [6/6] deals with device_platform_notify().

Please review and let me know if there are any concerns regarding this.

Thanks!




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

* [PATCH v1 1/6] ACPI: glue: Rearrange acpi_device_notify()
  2021-07-12 17:19 [PATCH v1 0/6] ACPI: glue / driver core: Eliminate acpi_platform_notify() and split device_platform_notify() Rafael J. Wysocki
@ 2021-07-12 17:23 ` Rafael J. Wysocki
  2021-07-12 17:23 ` [PATCH v1 2/6] ACPI: glue: Change return type of two functions to void Rafael J. Wysocki
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2021-07-12 17:23 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rafael J. Wysocki, Andy Shevchenko, Linux ACPI, LKML, Krogerus, Heikki

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

Make the code flow in acpi_device_notify() more straightforward and
make it use dev_dbg() and acpi_handle_debug() for printing debug
messages.

The only expected functional impact of this change is the content of
the debug messages printed by acpi_device_notify().

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/glue.c |   30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

Index: linux-pm/drivers/acpi/glue.c
===================================================================
--- linux-pm.orig/drivers/acpi/glue.c
+++ linux-pm/drivers/acpi/glue.c
@@ -292,22 +292,21 @@ static int acpi_device_notify(struct dev
 	int ret;
 
 	ret = acpi_bind_one(dev, NULL);
-	if (ret && type) {
-		struct acpi_device *adev;
+	if (ret) {
+		if (!type)
+			goto err;
 
 		adev = type->find_companion(dev);
 		if (!adev) {
-			pr_debug("Unable to get handle for %s\n", dev_name(dev));
+			dev_dbg(dev, "ACPI companion not found\n");
 			ret = -ENODEV;
-			goto out;
+			goto err;
 		}
 		ret = acpi_bind_one(dev, adev);
 		if (ret)
-			goto out;
+			goto err;
 	}
 	adev = ACPI_COMPANION(dev);
-	if (!adev)
-		goto out;
 
 	if (dev_is_platform(dev))
 		acpi_configure_pmsi_domain(dev);
@@ -317,16 +316,13 @@ static int acpi_device_notify(struct dev
 	else if (adev->handler && adev->handler->bind)
 		adev->handler->bind(dev);
 
- out:
-	if (!ret) {
-		struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
-
-		acpi_get_name(ACPI_HANDLE(dev), ACPI_FULL_PATHNAME, &buffer);
-		pr_debug("Device %s -> %s\n", dev_name(dev), (char *)buffer.pointer);
-		kfree(buffer.pointer);
-	} else {
-		pr_debug("Device %s -> No ACPI support\n", dev_name(dev));
-	}
+	acpi_handle_debug(ACPI_HANDLE(dev), "Bound to device %s\n",
+			  dev_name(dev));
+
+	return 0;
+
+err:
+	dev_dbg(dev, "No ACPI support\n");
 
 	return ret;
 }




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

* [PATCH v1 2/6] ACPI: glue: Change return type of two functions to void
  2021-07-12 17:19 [PATCH v1 0/6] ACPI: glue / driver core: Eliminate acpi_platform_notify() and split device_platform_notify() Rafael J. Wysocki
  2021-07-12 17:23 ` [PATCH v1 1/6] ACPI: glue: Rearrange acpi_device_notify() Rafael J. Wysocki
@ 2021-07-12 17:23 ` Rafael J. Wysocki
  2021-07-12 17:24 ` [PATCH v1 3/6] ACPI: bus: Rename functions to avoid name collision Rafael J. Wysocki
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2021-07-12 17:23 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rafael J. Wysocki, Andy Shevchenko, Linux ACPI, LKML, Krogerus, Heikki

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

Since the return values of acpi_device_notify() and
acpi_device_notify_remove() are discarded by their only caller,
change their return type to void.

No functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/glue.c |   12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

Index: linux-pm/drivers/acpi/glue.c
===================================================================
--- linux-pm.orig/drivers/acpi/glue.c
+++ linux-pm/drivers/acpi/glue.c
@@ -285,7 +285,7 @@ int acpi_unbind_one(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(acpi_unbind_one);
 
-static int acpi_device_notify(struct device *dev)
+static void acpi_device_notify(struct device *dev)
 {
 	struct acpi_bus_type *type = acpi_get_bus_type(dev);
 	struct acpi_device *adev;
@@ -299,7 +299,6 @@ static int acpi_device_notify(struct dev
 		adev = type->find_companion(dev);
 		if (!adev) {
 			dev_dbg(dev, "ACPI companion not found\n");
-			ret = -ENODEV;
 			goto err;
 		}
 		ret = acpi_bind_one(dev, adev);
@@ -319,21 +318,19 @@ static int acpi_device_notify(struct dev
 	acpi_handle_debug(ACPI_HANDLE(dev), "Bound to device %s\n",
 			  dev_name(dev));
 
-	return 0;
+	return;
 
 err:
 	dev_dbg(dev, "No ACPI support\n");
-
-	return ret;
 }
 
-static int acpi_device_notify_remove(struct device *dev)
+static void acpi_device_notify_remove(struct device *dev)
 {
 	struct acpi_device *adev = ACPI_COMPANION(dev);
 	struct acpi_bus_type *type;
 
 	if (!adev)
-		return 0;
+		return;
 
 	type = acpi_get_bus_type(dev);
 	if (type && type->cleanup)
@@ -342,7 +339,6 @@ static int acpi_device_notify_remove(str
 		adev->handler->unbind(dev);
 
 	acpi_unbind_one(dev);
-	return 0;
 }
 
 int acpi_platform_notify(struct device *dev, enum kobject_action action)




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

* [PATCH v1 3/6] ACPI: bus: Rename functions to avoid name collision
  2021-07-12 17:19 [PATCH v1 0/6] ACPI: glue / driver core: Eliminate acpi_platform_notify() and split device_platform_notify() Rafael J. Wysocki
  2021-07-12 17:23 ` [PATCH v1 1/6] ACPI: glue: Rearrange acpi_device_notify() Rafael J. Wysocki
  2021-07-12 17:23 ` [PATCH v1 2/6] ACPI: glue: Change return type of two functions to void Rafael J. Wysocki
@ 2021-07-12 17:24 ` Rafael J. Wysocki
  2021-07-12 17:25 ` [PATCH v1 4/6] ACPI: glue: Eliminate acpi_platform_notify() Rafael J. Wysocki
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2021-07-12 17:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rafael J. Wysocki, Andy Shevchenko, Linux ACPI, LKML, Krogerus, Heikki

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

There is a name collision between acpi_device_notify() defined in
bus.c and another static function defined in glue.c.

Since the latter is going to be exported from that file, rename the
former to acpi_notify_device() and rename acpi_device_notify_fixed()
to follow the same naming pattern.

No functional impact.

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

Index: linux-pm/drivers/acpi/bus.c
===================================================================
--- linux-pm.orig/drivers/acpi/bus.c
+++ linux-pm/drivers/acpi/bus.c
@@ -498,24 +498,24 @@ static void acpi_bus_notify(acpi_handle
 	acpi_evaluate_ost(handle, type, ost_code, NULL);
 }
 
-static void acpi_device_notify(acpi_handle handle, u32 event, void *data)
+static void acpi_notify_device(acpi_handle handle, u32 event, void *data)
 {
 	struct acpi_device *device = data;
 
 	device->driver->ops.notify(device, event);
 }
 
-static void acpi_device_notify_fixed(void *data)
+static void acpi_notify_device_fixed(void *data)
 {
 	struct acpi_device *device = data;
 
 	/* Fixed hardware devices have no handles */
-	acpi_device_notify(NULL, ACPI_FIXED_HARDWARE_EVENT, device);
+	acpi_notify_device(NULL, ACPI_FIXED_HARDWARE_EVENT, device);
 }
 
 static u32 acpi_device_fixed_event(void *data)
 {
-	acpi_os_execute(OSL_NOTIFY_HANDLER, acpi_device_notify_fixed, data);
+	acpi_os_execute(OSL_NOTIFY_HANDLER, acpi_notify_device_fixed, data);
 	return ACPI_INTERRUPT_HANDLED;
 }
 
@@ -536,7 +536,7 @@ static int acpi_device_install_notify_ha
 	else
 		status = acpi_install_notify_handler(device->handle,
 						     ACPI_DEVICE_NOTIFY,
-						     acpi_device_notify,
+						     acpi_notify_device,
 						     device);
 
 	if (ACPI_FAILURE(status))
@@ -554,7 +554,7 @@ static void acpi_device_remove_notify_ha
 						acpi_device_fixed_event);
 	else
 		acpi_remove_notify_handler(device->handle, ACPI_DEVICE_NOTIFY,
-					   acpi_device_notify);
+					   acpi_notify_device);
 }
 
 /* Handle events targeting \_SB device (at present only graceful shutdown) */




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

* [PATCH v1 4/6] ACPI: glue: Eliminate acpi_platform_notify()
  2021-07-12 17:19 [PATCH v1 0/6] ACPI: glue / driver core: Eliminate acpi_platform_notify() and split device_platform_notify() Rafael J. Wysocki
                   ` (2 preceding siblings ...)
  2021-07-12 17:24 ` [PATCH v1 3/6] ACPI: bus: Rename functions to avoid name collision Rafael J. Wysocki
@ 2021-07-12 17:25 ` Rafael J. Wysocki
  2021-07-12 18:35   ` Andy Shevchenko
  2021-07-12 17:27 ` [PATCH v1 5/6] software nodes: Split software_node_notify() Rafael J. Wysocki
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Rafael J. Wysocki @ 2021-07-12 17:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rafael J. Wysocki, Andy Shevchenko, Linux ACPI, LKML, Krogerus, Heikki

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

Get rid of acpi_platform_notify() which is redundant and
make device_platform_notify() in the driver core call
acpi_device_notify() and acpi_device_notify_remove() directly.

No functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/glue.c  |   19 ++-----------------
 drivers/base/core.c  |    7 ++++---
 include/linux/acpi.h |   10 ++++------
 3 files changed, 10 insertions(+), 26 deletions(-)

Index: linux-pm/drivers/base/core.c
===================================================================
--- linux-pm.orig/drivers/base/core.c
+++ linux-pm/drivers/base/core.c
@@ -2005,9 +2005,10 @@ device_platform_notify(struct device *de
 {
 	int ret;
 
-	ret = acpi_platform_notify(dev, action);
-	if (ret)
-		return ret;
+	if (action == KOBJ_ADD)
+		acpi_device_notify(dev);
+	else if (action == KOBJ_REMOVE)
+		acpi_device_notify_remove(dev);
 
 	ret = software_node_notify(dev, action);
 	if (ret)
Index: linux-pm/include/linux/acpi.h
===================================================================
--- linux-pm.orig/include/linux/acpi.h
+++ linux-pm/include/linux/acpi.h
@@ -1380,13 +1380,11 @@ static inline int find_acpi_cpu_cache_to
 #endif
 
 #ifdef CONFIG_ACPI
-extern int acpi_platform_notify(struct device *dev, enum kobject_action action);
+extern void acpi_device_notify(struct device *dev);
+extern void acpi_device_notify_remove(struct device *dev);
 #else
-static inline int
-acpi_platform_notify(struct device *dev, enum kobject_action action)
-{
-	return 0;
-}
+static inline void acpi_device_notify(struct device *dev) { }
+static inline void acpi_device_notify_remove(struct device *dev) { }
 #endif
 
 #endif	/*_LINUX_ACPI_H*/
Index: linux-pm/drivers/acpi/glue.c
===================================================================
--- linux-pm.orig/drivers/acpi/glue.c
+++ linux-pm/drivers/acpi/glue.c
@@ -285,7 +285,7 @@ int acpi_unbind_one(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(acpi_unbind_one);
 
-static void acpi_device_notify(struct device *dev)
+void acpi_device_notify(struct device *dev)
 {
 	struct acpi_bus_type *type = acpi_get_bus_type(dev);
 	struct acpi_device *adev;
@@ -324,7 +324,7 @@ err:
 	dev_dbg(dev, "No ACPI support\n");
 }
 
-static void acpi_device_notify_remove(struct device *dev)
+void acpi_device_notify_remove(struct device *dev)
 {
 	struct acpi_device *adev = ACPI_COMPANION(dev);
 	struct acpi_bus_type *type;
@@ -340,18 +340,3 @@ static void acpi_device_notify_remove(st
 
 	acpi_unbind_one(dev);
 }
-
-int acpi_platform_notify(struct device *dev, enum kobject_action action)
-{
-	switch (action) {
-	case KOBJ_ADD:
-		acpi_device_notify(dev);
-		break;
-	case KOBJ_REMOVE:
-		acpi_device_notify_remove(dev);
-		break;
-	default:
-		break;
-	}
-	return 0;
-}




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

* [PATCH v1 5/6] software nodes: Split software_node_notify()
  2021-07-12 17:19 [PATCH v1 0/6] ACPI: glue / driver core: Eliminate acpi_platform_notify() and split device_platform_notify() Rafael J. Wysocki
                   ` (3 preceding siblings ...)
  2021-07-12 17:25 ` [PATCH v1 4/6] ACPI: glue: Eliminate acpi_platform_notify() Rafael J. Wysocki
@ 2021-07-12 17:27 ` Rafael J. Wysocki
  2021-07-12 18:03   ` Greg Kroah-Hartman
  2021-07-12 17:28 ` [PATCH v1 6/6] driver core: Split device_platform_notify() Rafael J. Wysocki
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Rafael J. Wysocki @ 2021-07-12 17:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rafael J. Wysocki, Andy Shevchenko, Linux ACPI, LKML, Krogerus, Heikki

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

Split software_node_notify_remove) out of software_node_notify()
and make device_platform_notify() call the latter on device addition
and the former on device removal.

While at it, put the headers of the above functions into base.h,
because they don't need to be present in a global header file.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/base/base.h      |    3 ++
 drivers/base/core.c      |    9 +++---
 drivers/base/swnode.c    |   61 ++++++++++++++++++++++++-----------------------
 include/linux/property.h |    2 -
 4 files changed, 39 insertions(+), 36 deletions(-)

Index: linux-pm/drivers/base/swnode.c
===================================================================
--- linux-pm.orig/drivers/base/swnode.c
+++ linux-pm/drivers/base/swnode.c
@@ -11,6 +11,8 @@
 #include <linux/property.h>
 #include <linux/slab.h>
 
+#include "base.h"
+
 struct swnode {
 	struct kobject kobj;
 	struct fwnode_handle fwnode;
@@ -1053,7 +1055,7 @@ int device_add_software_node(struct devi
 	 * balance.
 	 */
 	if (device_is_registered(dev))
-		software_node_notify(dev, KOBJ_ADD);
+		software_node_notify(dev);
 
 	return 0;
 }
@@ -1074,7 +1076,8 @@ void device_remove_software_node(struct
 		return;
 
 	if (device_is_registered(dev))
-		software_node_notify(dev, KOBJ_REMOVE);
+		software_node_notify_remove(dev);
+
 	set_secondary_fwnode(dev, NULL);
 	kobject_put(&swnode->kobj);
 }
@@ -1117,44 +1120,44 @@ int device_create_managed_software_node(
 }
 EXPORT_SYMBOL_GPL(device_create_managed_software_node);
 
-int software_node_notify(struct device *dev, unsigned long action)
+void software_node_notify(struct device *dev)
 {
 	struct swnode *swnode;
 	int ret;
 
 	swnode = dev_to_swnode(dev);
 	if (!swnode)
-		return 0;
+		return;
 
-	switch (action) {
-	case KOBJ_ADD:
-		ret = sysfs_create_link(&dev->kobj, &swnode->kobj, "software_node");
-		if (ret)
-			break;
+	ret = sysfs_create_link(&dev->kobj, &swnode->kobj, "software_node");
+	if (ret)
+		return;
 
-		ret = sysfs_create_link(&swnode->kobj, &dev->kobj,
-					dev_name(dev));
-		if (ret) {
-			sysfs_remove_link(&dev->kobj, "software_node");
-			break;
-		}
-		kobject_get(&swnode->kobj);
-		break;
-	case KOBJ_REMOVE:
-		sysfs_remove_link(&swnode->kobj, dev_name(dev));
+	ret = sysfs_create_link(&swnode->kobj, &dev->kobj, dev_name(dev));
+	if (ret) {
 		sysfs_remove_link(&dev->kobj, "software_node");
-		kobject_put(&swnode->kobj);
-
-		if (swnode->managed) {
-			set_secondary_fwnode(dev, NULL);
-			kobject_put(&swnode->kobj);
-		}
-		break;
-	default:
-		break;
+		return;
 	}
 
-	return 0;
+	kobject_get(&swnode->kobj);
+}
+
+void software_node_notify_remove(struct device *dev)
+{
+	struct swnode *swnode;
+
+	swnode = dev_to_swnode(dev);
+	if (!swnode)
+		return;
+
+	sysfs_remove_link(&swnode->kobj, dev_name(dev));
+	sysfs_remove_link(&dev->kobj, "software_node");
+	kobject_put(&swnode->kobj);
+
+	if (swnode->managed) {
+		set_secondary_fwnode(dev, NULL);
+		kobject_put(&swnode->kobj);
+	}
 }
 
 static int __init software_node_init(void)
Index: linux-pm/include/linux/property.h
===================================================================
--- linux-pm.orig/include/linux/property.h
+++ linux-pm/include/linux/property.h
@@ -484,8 +484,6 @@ void software_node_unregister_node_group
 int software_node_register(const struct software_node *node);
 void software_node_unregister(const struct software_node *node);
 
-int software_node_notify(struct device *dev, unsigned long action);
-
 struct fwnode_handle *
 fwnode_create_software_node(const struct property_entry *properties,
 			    const struct fwnode_handle *parent);
Index: linux-pm/drivers/base/core.c
===================================================================
--- linux-pm.orig/drivers/base/core.c
+++ linux-pm/drivers/base/core.c
@@ -2003,16 +2003,15 @@ static inline int device_is_not_partitio
 static int
 device_platform_notify(struct device *dev, enum kobject_action action)
 {
-	int ret;
-
 	if (action == KOBJ_ADD)
 		acpi_device_notify(dev);
 	else if (action == KOBJ_REMOVE)
 		acpi_device_notify_remove(dev);
 
-	ret = software_node_notify(dev, action);
-	if (ret)
-		return ret;
+	if (action == KOBJ_ADD)
+		software_node_notify(dev);
+	else if (action == KOBJ_REMOVE)
+		software_node_notify_remove(dev);
 
 	if (platform_notify && action == KOBJ_ADD)
 		platform_notify(dev);
Index: linux-pm/drivers/base/base.h
===================================================================
--- linux-pm.orig/drivers/base/base.h
+++ linux-pm/drivers/base/base.h
@@ -202,3 +202,6 @@ int devtmpfs_delete_node(struct device *
 static inline int devtmpfs_create_node(struct device *dev) { return 0; }
 static inline int devtmpfs_delete_node(struct device *dev) { return 0; }
 #endif
+
+void software_node_notify(struct device *dev);
+void software_node_notify_remove(struct device *dev);




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

* [PATCH v1 6/6] driver core: Split device_platform_notify()
  2021-07-12 17:19 [PATCH v1 0/6] ACPI: glue / driver core: Eliminate acpi_platform_notify() and split device_platform_notify() Rafael J. Wysocki
                   ` (4 preceding siblings ...)
  2021-07-12 17:27 ` [PATCH v1 5/6] software nodes: Split software_node_notify() Rafael J. Wysocki
@ 2021-07-12 17:28 ` Rafael J. Wysocki
  2021-07-12 18:04   ` Greg Kroah-Hartman
  2021-07-12 18:04 ` [PATCH v1 0/6] ACPI: glue / driver core: Eliminate acpi_platform_notify() and split device_platform_notify() Greg Kroah-Hartman
  2021-07-12 18:39 ` Andy Shevchenko
  7 siblings, 1 reply; 20+ messages in thread
From: Rafael J. Wysocki @ 2021-07-12 17:28 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rafael J. Wysocki, Andy Shevchenko, Linux ACPI, LKML, Krogerus, Heikki

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

Split device_platform_notify_remove) out of device_platform_notify()
and call the latter on device addition and the former on device
removal.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/base/core.c |   37 +++++++++++++++++--------------------
 1 file changed, 17 insertions(+), 20 deletions(-)

Index: linux-pm/drivers/base/core.c
===================================================================
--- linux-pm.orig/drivers/base/core.c
+++ linux-pm/drivers/base/core.c
@@ -2000,24 +2000,24 @@ static inline int device_is_not_partitio
 }
 #endif
 
-static int
-device_platform_notify(struct device *dev, enum kobject_action action)
+static void device_platform_notify(struct device *dev)
 {
-	if (action == KOBJ_ADD)
-		acpi_device_notify(dev);
-	else if (action == KOBJ_REMOVE)
-		acpi_device_notify_remove(dev);
-
-	if (action == KOBJ_ADD)
-		software_node_notify(dev);
-	else if (action == KOBJ_REMOVE)
-		software_node_notify_remove(dev);
+	acpi_device_notify(dev);
 
-	if (platform_notify && action == KOBJ_ADD)
+	software_node_notify(dev);
+
+	if (platform_notify)
 		platform_notify(dev);
-	else if (platform_notify_remove && action == KOBJ_REMOVE)
+}
+
+static void device_platform_notify_remove(struct device *dev)
+{
+	acpi_device_notify_remove(dev);
+
+	software_node_notify_remove(dev);
+
+	if (platform_notify_remove)
 		platform_notify_remove(dev);
-	return 0;
 }
 
 /**
@@ -3289,9 +3289,7 @@ int device_add(struct device *dev)
 	}
 
 	/* notify platform of device entry */
-	error = device_platform_notify(dev, KOBJ_ADD);
-	if (error)
-		goto platform_error;
+	device_platform_notify(dev);
 
 	error = device_create_file(dev, &dev_attr_uevent);
 	if (error)
@@ -3394,8 +3392,7 @@ done:
  SymlinkError:
 	device_remove_file(dev, &dev_attr_uevent);
  attrError:
-	device_platform_notify(dev, KOBJ_REMOVE);
-platform_error:
+	device_platform_notify_remove(dev);
 	kobject_uevent(&dev->kobj, KOBJ_REMOVE);
 	glue_dir = get_glue_dir(dev);
 	kobject_del(&dev->kobj);
@@ -3540,7 +3537,7 @@ void device_del(struct device *dev)
 	bus_remove_device(dev);
 	device_pm_remove(dev);
 	driver_deferred_probe_del(dev);
-	device_platform_notify(dev, KOBJ_REMOVE);
+	device_platform_notify_remove(dev);
 	device_remove_properties(dev);
 	device_links_purge(dev);
 




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

* Re: [PATCH v1 5/6] software nodes: Split software_node_notify()
  2021-07-12 17:27 ` [PATCH v1 5/6] software nodes: Split software_node_notify() Rafael J. Wysocki
@ 2021-07-12 18:03   ` Greg Kroah-Hartman
  2021-07-12 18:30     ` Rafael J. Wysocki
  0 siblings, 1 reply; 20+ messages in thread
From: Greg Kroah-Hartman @ 2021-07-12 18:03 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Rafael J. Wysocki, Andy Shevchenko, Linux ACPI, LKML, Krogerus, Heikki

On Mon, Jul 12, 2021 at 07:27:12PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Split software_node_notify_remove) out of software_node_notify()
> and make device_platform_notify() call the latter on device addition
> and the former on device removal.
> 
> While at it, put the headers of the above functions into base.h,
> because they don't need to be present in a global header file.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/base/base.h      |    3 ++
>  drivers/base/core.c      |    9 +++---
>  drivers/base/swnode.c    |   61 ++++++++++++++++++++++++-----------------------
>  include/linux/property.h |    2 -
>  4 files changed, 39 insertions(+), 36 deletions(-)
> 
> Index: linux-pm/drivers/base/swnode.c
> ===================================================================
> --- linux-pm.orig/drivers/base/swnode.c
> +++ linux-pm/drivers/base/swnode.c
> @@ -11,6 +11,8 @@
>  #include <linux/property.h>
>  #include <linux/slab.h>
>  
> +#include "base.h"
> +
>  struct swnode {
>  	struct kobject kobj;
>  	struct fwnode_handle fwnode;
> @@ -1053,7 +1055,7 @@ int device_add_software_node(struct devi
>  	 * balance.
>  	 */
>  	if (device_is_registered(dev))
> -		software_node_notify(dev, KOBJ_ADD);
> +		software_node_notify(dev);

Should this now be called "software_node_notify_add()" to match up with:

>  	if (device_is_registered(dev))
> -		software_node_notify(dev, KOBJ_REMOVE);
> +		software_node_notify_remove(dev);

The other being called "_remove"?

Makes it more obvious to me :)

thanks,

greg k-h

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

* Re: [PATCH v1 6/6] driver core: Split device_platform_notify()
  2021-07-12 17:28 ` [PATCH v1 6/6] driver core: Split device_platform_notify() Rafael J. Wysocki
@ 2021-07-12 18:04   ` Greg Kroah-Hartman
  2021-07-12 18:30     ` Rafael J. Wysocki
  0 siblings, 1 reply; 20+ messages in thread
From: Greg Kroah-Hartman @ 2021-07-12 18:04 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Rafael J. Wysocki, Andy Shevchenko, Linux ACPI, LKML, Krogerus, Heikki

On Mon, Jul 12, 2021 at 07:28:16PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Split device_platform_notify_remove) out of device_platform_notify()
> and call the latter on device addition and the former on device
> removal.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/base/core.c |   37 +++++++++++++++++--------------------
>  1 file changed, 17 insertions(+), 20 deletions(-)

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

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

* Re: [PATCH v1 0/6] ACPI: glue / driver core: Eliminate acpi_platform_notify() and split device_platform_notify()
  2021-07-12 17:19 [PATCH v1 0/6] ACPI: glue / driver core: Eliminate acpi_platform_notify() and split device_platform_notify() Rafael J. Wysocki
                   ` (5 preceding siblings ...)
  2021-07-12 17:28 ` [PATCH v1 6/6] driver core: Split device_platform_notify() Rafael J. Wysocki
@ 2021-07-12 18:04 ` Greg Kroah-Hartman
  2021-07-12 18:39 ` Andy Shevchenko
  7 siblings, 0 replies; 20+ messages in thread
From: Greg Kroah-Hartman @ 2021-07-12 18:04 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Rafael J. Wysocki, Andy Shevchenko, Linux ACPI, LKML, Krogerus, Heikki

On Mon, Jul 12, 2021 at 07:19:57PM +0200, Rafael J. Wysocki wrote:
> Hi Greg et al,
> 
> This series doesn't change functionality (at least not intentionally), but
> it get rids of a few unneeded checks, parameter passing etc.
> 
> Patches [1-2/6] simplify the ACPI "glue" code.
> 
> Patch [3/6] renames a couple of ACPI functions to avoid name collisions going
> forward.
> 
> Patch [4/6] gets rid of acpi_platform_notify().
> 
> Patch [5/6] rearranges the software nodes code along the lines of what happens
> to the ACPI "glue" code in patch [4/6].
> 
> Patch [6/6] deals with device_platform_notify().
> 
> Please review and let me know if there are any concerns regarding this.

Looks good, I only had one small naming comment on patch 5/6.

thanks,

greg k-h

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

* Re: [PATCH v1 5/6] software nodes: Split software_node_notify()
  2021-07-12 18:03   ` Greg Kroah-Hartman
@ 2021-07-12 18:30     ` Rafael J. Wysocki
  2021-07-12 18:57       ` Greg Kroah-Hartman
  2021-07-13  7:46       ` Heikki Krogerus
  0 siblings, 2 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2021-07-12 18:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rafael J. Wysocki, Rafael J. Wysocki, Andy Shevchenko,
	Linux ACPI, LKML, Krogerus, Heikki

On Mon, Jul 12, 2021 at 8:03 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Jul 12, 2021 at 07:27:12PM +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Split software_node_notify_remove) out of software_node_notify()
> > and make device_platform_notify() call the latter on device addition
> > and the former on device removal.
> >
> > While at it, put the headers of the above functions into base.h,
> > because they don't need to be present in a global header file.
> >
> > No intentional functional impact.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >  drivers/base/base.h      |    3 ++
> >  drivers/base/core.c      |    9 +++---
> >  drivers/base/swnode.c    |   61 ++++++++++++++++++++++++-----------------------
> >  include/linux/property.h |    2 -
> >  4 files changed, 39 insertions(+), 36 deletions(-)
> >
> > Index: linux-pm/drivers/base/swnode.c
> > ===================================================================
> > --- linux-pm.orig/drivers/base/swnode.c
> > +++ linux-pm/drivers/base/swnode.c
> > @@ -11,6 +11,8 @@
> >  #include <linux/property.h>
> >  #include <linux/slab.h>
> >
> > +#include "base.h"
> > +
> >  struct swnode {
> >       struct kobject kobj;
> >       struct fwnode_handle fwnode;
> > @@ -1053,7 +1055,7 @@ int device_add_software_node(struct devi
> >        * balance.
> >        */
> >       if (device_is_registered(dev))
> > -             software_node_notify(dev, KOBJ_ADD);
> > +             software_node_notify(dev);
>
> Should this now be called "software_node_notify_add()" to match up with:
>
> >       if (device_is_registered(dev))
> > -             software_node_notify(dev, KOBJ_REMOVE);
> > +             software_node_notify_remove(dev);
>
> The other being called "_remove"?
>
> Makes it more obvious to me :)

The naming convention used here follows platform_notify() and
platform_notify_remove(), and the analogous function names in ACPI for
that matter.

I thought that adding _add in just one case would be sort of odd, but
of course I can do that, so please let me know what you want me to do.

Cheers!

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

* Re: [PATCH v1 6/6] driver core: Split device_platform_notify()
  2021-07-12 18:04   ` Greg Kroah-Hartman
@ 2021-07-12 18:30     ` Rafael J. Wysocki
  0 siblings, 0 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2021-07-12 18:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rafael J. Wysocki, Rafael J. Wysocki, Andy Shevchenko,
	Linux ACPI, LKML, Krogerus, Heikki

On Mon, Jul 12, 2021 at 8:04 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Jul 12, 2021 at 07:28:16PM +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Split device_platform_notify_remove) out of device_platform_notify()
> > and call the latter on device addition and the former on device
> > removal.
> >
> > No intentional functional impact.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >  drivers/base/core.c |   37 +++++++++++++++++--------------------
> >  1 file changed, 17 insertions(+), 20 deletions(-)
>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Thank you!

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

* Re: [PATCH v1 4/6] ACPI: glue: Eliminate acpi_platform_notify()
  2021-07-12 17:25 ` [PATCH v1 4/6] ACPI: glue: Eliminate acpi_platform_notify() Rafael J. Wysocki
@ 2021-07-12 18:35   ` Andy Shevchenko
  2021-07-12 18:38     ` Rafael J. Wysocki
  0 siblings, 1 reply; 20+ messages in thread
From: Andy Shevchenko @ 2021-07-12 18:35 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Linux ACPI, LKML,
	Krogerus, Heikki

On Mon, Jul 12, 2021 at 07:25:55PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Get rid of acpi_platform_notify() which is redundant and
> make device_platform_notify() in the driver core call
> acpi_device_notify() and acpi_device_notify_remove() directly.

> +	if (action == KOBJ_ADD)
> +		acpi_device_notify(dev);
> +	else if (action == KOBJ_REMOVE)
> +		acpi_device_notify_remove(dev);

In most of the cases we are using switch-case approach with
KOBJ_ADD/KOBJ_REMOVE. Would it make sense to keep that pattern?

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v1 4/6] ACPI: glue: Eliminate acpi_platform_notify()
  2021-07-12 18:35   ` Andy Shevchenko
@ 2021-07-12 18:38     ` Rafael J. Wysocki
  0 siblings, 0 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2021-07-12 18:38 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Rafael J. Wysocki, Greg Kroah-Hartman, Rafael J. Wysocki,
	Linux ACPI, LKML, Krogerus, Heikki

On Mon, Jul 12, 2021 at 8:36 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Mon, Jul 12, 2021 at 07:25:55PM +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Get rid of acpi_platform_notify() which is redundant and
> > make device_platform_notify() in the driver core call
> > acpi_device_notify() and acpi_device_notify_remove() directly.
>
> > +     if (action == KOBJ_ADD)
> > +             acpi_device_notify(dev);
> > +     else if (action == KOBJ_REMOVE)
> > +             acpi_device_notify_remove(dev);
>
> In most of the cases we are using switch-case approach with
> KOBJ_ADD/KOBJ_REMOVE. Would it make sense to keep that pattern?

Well, this goes away in the next patches anyway.

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

* Re: [PATCH v1 0/6] ACPI: glue / driver core: Eliminate acpi_platform_notify() and split device_platform_notify()
  2021-07-12 17:19 [PATCH v1 0/6] ACPI: glue / driver core: Eliminate acpi_platform_notify() and split device_platform_notify() Rafael J. Wysocki
                   ` (6 preceding siblings ...)
  2021-07-12 18:04 ` [PATCH v1 0/6] ACPI: glue / driver core: Eliminate acpi_platform_notify() and split device_platform_notify() Greg Kroah-Hartman
@ 2021-07-12 18:39 ` Andy Shevchenko
  2021-07-14 18:17   ` Rafael J. Wysocki
  7 siblings, 1 reply; 20+ messages in thread
From: Andy Shevchenko @ 2021-07-12 18:39 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Linux ACPI, LKML,
	Krogerus, Heikki

On Mon, Jul 12, 2021 at 07:19:57PM +0200, Rafael J. Wysocki wrote:
> Hi Greg et al,
> 
> This series doesn't change functionality (at least not intentionally), but
> it get rids of a few unneeded checks, parameter passing etc.
> 
> Patches [1-2/6] simplify the ACPI "glue" code.
> 
> Patch [3/6] renames a couple of ACPI functions to avoid name collisions going
> forward.
> 
> Patch [4/6] gets rid of acpi_platform_notify().
> 
> Patch [5/6] rearranges the software nodes code along the lines of what happens
> to the ACPI "glue" code in patch [4/6].
> 
> Patch [6/6] deals with device_platform_notify().
> 
> Please review and let me know if there are any concerns regarding this.

The result looks good to me, but perhaps the ordering can be changed to
minimize addition of the lines that are going to be removed inside the same
series.

In either case, feel free to add
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v1 5/6] software nodes: Split software_node_notify()
  2021-07-12 18:30     ` Rafael J. Wysocki
@ 2021-07-12 18:57       ` Greg Kroah-Hartman
  2021-07-12 19:04         ` Rafael J. Wysocki
  2021-07-13  7:46       ` Heikki Krogerus
  1 sibling, 1 reply; 20+ messages in thread
From: Greg Kroah-Hartman @ 2021-07-12 18:57 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Rafael J. Wysocki, Andy Shevchenko, Linux ACPI, LKML, Krogerus, Heikki

On Mon, Jul 12, 2021 at 08:30:06PM +0200, Rafael J. Wysocki wrote:
> On Mon, Jul 12, 2021 at 8:03 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Mon, Jul 12, 2021 at 07:27:12PM +0200, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > >
> > > Split software_node_notify_remove) out of software_node_notify()
> > > and make device_platform_notify() call the latter on device addition
> > > and the former on device removal.
> > >
> > > While at it, put the headers of the above functions into base.h,
> > > because they don't need to be present in a global header file.
> > >
> > > No intentional functional impact.
> > >
> > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > ---
> > >  drivers/base/base.h      |    3 ++
> > >  drivers/base/core.c      |    9 +++---
> > >  drivers/base/swnode.c    |   61 ++++++++++++++++++++++++-----------------------
> > >  include/linux/property.h |    2 -
> > >  4 files changed, 39 insertions(+), 36 deletions(-)
> > >
> > > Index: linux-pm/drivers/base/swnode.c
> > > ===================================================================
> > > --- linux-pm.orig/drivers/base/swnode.c
> > > +++ linux-pm/drivers/base/swnode.c
> > > @@ -11,6 +11,8 @@
> > >  #include <linux/property.h>
> > >  #include <linux/slab.h>
> > >
> > > +#include "base.h"
> > > +
> > >  struct swnode {
> > >       struct kobject kobj;
> > >       struct fwnode_handle fwnode;
> > > @@ -1053,7 +1055,7 @@ int device_add_software_node(struct devi
> > >        * balance.
> > >        */
> > >       if (device_is_registered(dev))
> > > -             software_node_notify(dev, KOBJ_ADD);
> > > +             software_node_notify(dev);
> >
> > Should this now be called "software_node_notify_add()" to match up with:
> >
> > >       if (device_is_registered(dev))
> > > -             software_node_notify(dev, KOBJ_REMOVE);
> > > +             software_node_notify_remove(dev);
> >
> > The other being called "_remove"?
> >
> > Makes it more obvious to me :)
> 
> The naming convention used here follows platform_notify() and
> platform_notify_remove(), and the analogous function names in ACPI for
> that matter.
> 
> I thought that adding _add in just one case would be sort of odd, but
> of course I can do that, so please let me know what you want me to do.

Ah, ok, that makes more sense, let's just leave it as-is then:

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

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

* Re: [PATCH v1 5/6] software nodes: Split software_node_notify()
  2021-07-12 18:57       ` Greg Kroah-Hartman
@ 2021-07-12 19:04         ` Rafael J. Wysocki
  0 siblings, 0 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2021-07-12 19:04 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rafael J. Wysocki, Rafael J. Wysocki, Andy Shevchenko,
	Linux ACPI, LKML, Krogerus, Heikki

On Mon, Jul 12, 2021 at 8:57 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Jul 12, 2021 at 08:30:06PM +0200, Rafael J. Wysocki wrote:
> > On Mon, Jul 12, 2021 at 8:03 PM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Mon, Jul 12, 2021 at 07:27:12PM +0200, Rafael J. Wysocki wrote:
> > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > >
> > > > Split software_node_notify_remove) out of software_node_notify()
> > > > and make device_platform_notify() call the latter on device addition
> > > > and the former on device removal.
> > > >
> > > > While at it, put the headers of the above functions into base.h,
> > > > because they don't need to be present in a global header file.
> > > >
> > > > No intentional functional impact.
> > > >
> > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > > ---
> > > >  drivers/base/base.h      |    3 ++
> > > >  drivers/base/core.c      |    9 +++---
> > > >  drivers/base/swnode.c    |   61 ++++++++++++++++++++++++-----------------------
> > > >  include/linux/property.h |    2 -
> > > >  4 files changed, 39 insertions(+), 36 deletions(-)
> > > >
> > > > Index: linux-pm/drivers/base/swnode.c
> > > > ===================================================================
> > > > --- linux-pm.orig/drivers/base/swnode.c
> > > > +++ linux-pm/drivers/base/swnode.c
> > > > @@ -11,6 +11,8 @@
> > > >  #include <linux/property.h>
> > > >  #include <linux/slab.h>
> > > >
> > > > +#include "base.h"
> > > > +
> > > >  struct swnode {
> > > >       struct kobject kobj;
> > > >       struct fwnode_handle fwnode;
> > > > @@ -1053,7 +1055,7 @@ int device_add_software_node(struct devi
> > > >        * balance.
> > > >        */
> > > >       if (device_is_registered(dev))
> > > > -             software_node_notify(dev, KOBJ_ADD);
> > > > +             software_node_notify(dev);
> > >
> > > Should this now be called "software_node_notify_add()" to match up with:
> > >
> > > >       if (device_is_registered(dev))
> > > > -             software_node_notify(dev, KOBJ_REMOVE);
> > > > +             software_node_notify_remove(dev);
> > >
> > > The other being called "_remove"?
> > >
> > > Makes it more obvious to me :)
> >
> > The naming convention used here follows platform_notify() and
> > platform_notify_remove(), and the analogous function names in ACPI for
> > that matter.
> >
> > I thought that adding _add in just one case would be sort of odd, but
> > of course I can do that, so please let me know what you want me to do.
>
> Ah, ok, that makes more sense, let's just leave it as-is then:
>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Thanks!

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

* Re: [PATCH v1 5/6] software nodes: Split software_node_notify()
  2021-07-12 18:30     ` Rafael J. Wysocki
  2021-07-12 18:57       ` Greg Kroah-Hartman
@ 2021-07-13  7:46       ` Heikki Krogerus
  2021-07-14 18:17         ` Rafael J. Wysocki
  1 sibling, 1 reply; 20+ messages in thread
From: Heikki Krogerus @ 2021-07-13  7:46 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Andy Shevchenko, Linux ACPI, LKML

On Mon, Jul 12, 2021 at 08:30:06PM +0200, Rafael J. Wysocki wrote:
> On Mon, Jul 12, 2021 at 8:03 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Mon, Jul 12, 2021 at 07:27:12PM +0200, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > >
> > > Split software_node_notify_remove) out of software_node_notify()
> > > and make device_platform_notify() call the latter on device addition
> > > and the former on device removal.
> > >
> > > While at it, put the headers of the above functions into base.h,
> > > because they don't need to be present in a global header file.
> > >
> > > No intentional functional impact.
> > >
> > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > ---
> > >  drivers/base/base.h      |    3 ++
> > >  drivers/base/core.c      |    9 +++---
> > >  drivers/base/swnode.c    |   61 ++++++++++++++++++++++++-----------------------
> > >  include/linux/property.h |    2 -
> > >  4 files changed, 39 insertions(+), 36 deletions(-)
> > >
> > > Index: linux-pm/drivers/base/swnode.c
> > > ===================================================================
> > > --- linux-pm.orig/drivers/base/swnode.c
> > > +++ linux-pm/drivers/base/swnode.c
> > > @@ -11,6 +11,8 @@
> > >  #include <linux/property.h>
> > >  #include <linux/slab.h>
> > >
> > > +#include "base.h"
> > > +
> > >  struct swnode {
> > >       struct kobject kobj;
> > >       struct fwnode_handle fwnode;
> > > @@ -1053,7 +1055,7 @@ int device_add_software_node(struct devi
> > >        * balance.
> > >        */
> > >       if (device_is_registered(dev))
> > > -             software_node_notify(dev, KOBJ_ADD);
> > > +             software_node_notify(dev);
> >
> > Should this now be called "software_node_notify_add()" to match up with:
> >
> > >       if (device_is_registered(dev))
> > > -             software_node_notify(dev, KOBJ_REMOVE);
> > > +             software_node_notify_remove(dev);
> >
> > The other being called "_remove"?
> >
> > Makes it more obvious to me :)
> 
> The naming convention used here follows platform_notify() and
> platform_notify_remove(), and the analogous function names in ACPI for
> that matter.

So why not rename those instead: platform_notify() to
platform_notify_add() and so on? You are in any case modifying
acpi_device_notify() in this series, and I think there is only one
place left where .platform_notify is assigned. I believe you also
wouldn't then need to worry about the function name collision (3/6).

> I thought that adding _add in just one case would be sort of odd, but
> of course I can do that, so please let me know what you want me to do.

I would prefer the "_add" ending, but in any case, FWIW:

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>


-- 
heikki

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

* Re: [PATCH v1 0/6] ACPI: glue / driver core: Eliminate acpi_platform_notify() and split device_platform_notify()
  2021-07-12 18:39 ` Andy Shevchenko
@ 2021-07-14 18:17   ` Rafael J. Wysocki
  0 siblings, 0 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2021-07-14 18:17 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Rafael J. Wysocki, Greg Kroah-Hartman, Rafael J. Wysocki,
	Linux ACPI, LKML, Krogerus, Heikki

On Mon, Jul 12, 2021 at 8:39 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Mon, Jul 12, 2021 at 07:19:57PM +0200, Rafael J. Wysocki wrote:
> > Hi Greg et al,
> >
> > This series doesn't change functionality (at least not intentionally), but
> > it get rids of a few unneeded checks, parameter passing etc.
> >
> > Patches [1-2/6] simplify the ACPI "glue" code.
> >
> > Patch [3/6] renames a couple of ACPI functions to avoid name collisions going
> > forward.
> >
> > Patch [4/6] gets rid of acpi_platform_notify().
> >
> > Patch [5/6] rearranges the software nodes code along the lines of what happens
> > to the ACPI "glue" code in patch [4/6].
> >
> > Patch [6/6] deals with device_platform_notify().
> >
> > Please review and let me know if there are any concerns regarding this.
>
> The result looks good to me, but perhaps the ordering can be changed to
> minimize addition of the lines that are going to be removed inside the same
> series.
>
> In either case, feel free to add
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Thanks!

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

* Re: [PATCH v1 5/6] software nodes: Split software_node_notify()
  2021-07-13  7:46       ` Heikki Krogerus
@ 2021-07-14 18:17         ` Rafael J. Wysocki
  0 siblings, 0 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2021-07-14 18:17 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Rafael J. Wysocki, Greg Kroah-Hartman, Rafael J. Wysocki,
	Andy Shevchenko, Linux ACPI, LKML

On Tue, Jul 13, 2021 at 9:46 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> On Mon, Jul 12, 2021 at 08:30:06PM +0200, Rafael J. Wysocki wrote:
> > On Mon, Jul 12, 2021 at 8:03 PM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Mon, Jul 12, 2021 at 07:27:12PM +0200, Rafael J. Wysocki wrote:
> > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > >
> > > > Split software_node_notify_remove) out of software_node_notify()
> > > > and make device_platform_notify() call the latter on device addition
> > > > and the former on device removal.
> > > >
> > > > While at it, put the headers of the above functions into base.h,
> > > > because they don't need to be present in a global header file.
> > > >
> > > > No intentional functional impact.
> > > >
> > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > > ---
> > > >  drivers/base/base.h      |    3 ++
> > > >  drivers/base/core.c      |    9 +++---
> > > >  drivers/base/swnode.c    |   61 ++++++++++++++++++++++++-----------------------
> > > >  include/linux/property.h |    2 -
> > > >  4 files changed, 39 insertions(+), 36 deletions(-)
> > > >
> > > > Index: linux-pm/drivers/base/swnode.c
> > > > ===================================================================
> > > > --- linux-pm.orig/drivers/base/swnode.c
> > > > +++ linux-pm/drivers/base/swnode.c
> > > > @@ -11,6 +11,8 @@
> > > >  #include <linux/property.h>
> > > >  #include <linux/slab.h>
> > > >
> > > > +#include "base.h"
> > > > +
> > > >  struct swnode {
> > > >       struct kobject kobj;
> > > >       struct fwnode_handle fwnode;
> > > > @@ -1053,7 +1055,7 @@ int device_add_software_node(struct devi
> > > >        * balance.
> > > >        */
> > > >       if (device_is_registered(dev))
> > > > -             software_node_notify(dev, KOBJ_ADD);
> > > > +             software_node_notify(dev);
> > >
> > > Should this now be called "software_node_notify_add()" to match up with:
> > >
> > > >       if (device_is_registered(dev))
> > > > -             software_node_notify(dev, KOBJ_REMOVE);
> > > > +             software_node_notify_remove(dev);
> > >
> > > The other being called "_remove"?
> > >
> > > Makes it more obvious to me :)
> >
> > The naming convention used here follows platform_notify() and
> > platform_notify_remove(), and the analogous function names in ACPI for
> > that matter.
>
> So why not rename those instead: platform_notify() to
> platform_notify_add() and so on? You are in any case modifying
> acpi_device_notify() in this series, and I think there is only one
> place left where .platform_notify is assigned. I believe you also
> wouldn't then need to worry about the function name collision (3/6).
>
> > I thought that adding _add in just one case would be sort of odd, but
> > of course I can do that, so please let me know what you want me to do.
>
> I would prefer the "_add" ending, but in any case, FWIW:
>
> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

Thanks!

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

end of thread, other threads:[~2021-07-14 18:18 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-12 17:19 [PATCH v1 0/6] ACPI: glue / driver core: Eliminate acpi_platform_notify() and split device_platform_notify() Rafael J. Wysocki
2021-07-12 17:23 ` [PATCH v1 1/6] ACPI: glue: Rearrange acpi_device_notify() Rafael J. Wysocki
2021-07-12 17:23 ` [PATCH v1 2/6] ACPI: glue: Change return type of two functions to void Rafael J. Wysocki
2021-07-12 17:24 ` [PATCH v1 3/6] ACPI: bus: Rename functions to avoid name collision Rafael J. Wysocki
2021-07-12 17:25 ` [PATCH v1 4/6] ACPI: glue: Eliminate acpi_platform_notify() Rafael J. Wysocki
2021-07-12 18:35   ` Andy Shevchenko
2021-07-12 18:38     ` Rafael J. Wysocki
2021-07-12 17:27 ` [PATCH v1 5/6] software nodes: Split software_node_notify() Rafael J. Wysocki
2021-07-12 18:03   ` Greg Kroah-Hartman
2021-07-12 18:30     ` Rafael J. Wysocki
2021-07-12 18:57       ` Greg Kroah-Hartman
2021-07-12 19:04         ` Rafael J. Wysocki
2021-07-13  7:46       ` Heikki Krogerus
2021-07-14 18:17         ` Rafael J. Wysocki
2021-07-12 17:28 ` [PATCH v1 6/6] driver core: Split device_platform_notify() Rafael J. Wysocki
2021-07-12 18:04   ` Greg Kroah-Hartman
2021-07-12 18:30     ` Rafael J. Wysocki
2021-07-12 18:04 ` [PATCH v1 0/6] ACPI: glue / driver core: Eliminate acpi_platform_notify() and split device_platform_notify() Greg Kroah-Hartman
2021-07-12 18:39 ` Andy Shevchenko
2021-07-14 18:17   ` Rafael J. Wysocki

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).