Add Legacy PM OPS usage checks to class, bus, and driver register functions

PATCH In review Shuah Khan 2 participants 8 messages Jan 6, 2014 Driver core

Share & tools

Shuah Khan

author

2014-01-06 20:09 UTC #0 lore raw

v2 cover Add Legacy PM OPS usage checks to class, bus, and driver register functions

Add Legacy PM OPS usage checks to class, bus, and driver register functions. If Legacy PM OPS usage is found, print warning message to indicate that the driver code needs updating to use Dev PM OPS interfaces. This will help serve as a way to track drivers that still use Legacy PM OPS and fix them.

This patch set adds Legacy PM OPS usage check and warning to bus_register(). __class_register(), and driver_register().

Individual patches in this series are not dependent on each other. The only reason this is a series is for context and facilitating discussion on the overall change as opposed individual patches.

Shuah Khan (3):
drivers/bus: Add Legacy PM OPS usage check and warning to
bus_register()
drivers/class: Add Legacy PM OPS usage check and warning to

    __class_register()

driver: Add Legacy PM OPS usage check and warning to driver_register()

 drivers/base/bus.c    | 3 +++
 drivers/base/class.c  | 4 ++++
 drivers/base/driver.c | 4 ++++
 3 files changed, 11 insertions(+)

Signature · 1 line

1.8.3.2

Shuah Khan

author

2014-01-06 20:09 UTC #1 lore raw

PATCH v2 1/3 drivers/bus: Add Legacy PM OPS usage check and warning to bus_register()

Add Legacy PM OPS usage checks to bus_register() function. If Legacy PM OPS usage is found, print warning message to indicate that the driver code needs updating to use Dev PM OPS interfaces. This will help serve as a way to track drivers that still use Legacy PM OPS and fix them.

The Legacy PM OPS check looks for suspend(struct device *, pm_message_t) or resume(struct device *) bus level interfaces.

Signed-off-by Shuah Khan <[email protected]>

---
 drivers/base/bus.c | 3 +++
 1 file changed, 3 insertions(+)

drivers/base/bus.c +3 -0

index 73f6c29..e8753a7 100644
@@ -916,6 +916,9 @@ int bus_register(struct bus_type *bus)
 		goto bus_groups_fail;
 
 	pr_debug("bus: '%s': registered\n", bus->name);
+	if (bus->suspend || bus->resume)
+		pr_warn("bus '%s' needs updating - use pm pointer.\n",
+			bus->name);
 	return 0;
 
 bus_groups_fail:

Signature · 1 line

1.8.3.2

Greg KH

maintainer

2014-01-06 20:17 UTC #2 lore raw

Re: [PATCH v2 1/3] drivers/bus: Add Legacy PM OPS usage check and warning to bus_register()

Quoted 18 lines · Shuah Khan

Add Legacy PM OPS usage checks to bus_register() function. If Legacy PM OPS usage is found, print warning message to indicate that the driver code needs updating to use Dev PM OPS interfaces. This will help serve as a way to track drivers that still use Legacy PM OPS and fix them.
The Legacy PM OPS check looks for suspend(struct device *, pm_message_t) or resume(struct device *) bus level interfaces.
Signed-off-by Shuah Khan <[email protected]>
---
 drivers/base/bus.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/base/bus.c +0 -0
index 73f6c29..e8753a7 100644

drivers/base/bus.c int bus_register(struct bus_type *bus) · line 921

Quoted 6 lines

drivers/base/bus.c +3 -0

@@ -916,6 +916,9 @@ int bus_register(struct bus_type *bus)
 		goto bus_groups_fail;
 
 	pr_debug("bus: '%s': registered\n", bus->name);
+	if (bus->suspend || bus->resume)
+		pr_warn("bus '%s' needs updating - use pm pointer.\n",
+			bus->name);

Why can't we just sweep the tree for all of these now, fix them up, and then delete these fields and be done with it?

Same for the other ones, putting warnings in the kernel log files doesn't work at all for getting people to fix up their code (see examples of scsi log messages being there for _years_ about obsolete driver interfaces being used.)

thanks,

greg k-h

Shuah Khan

author

2014-01-06 20:30 UTC #3 lore raw

Re: [PATCH v2 1/3] drivers/bus: Add Legacy PM OPS usage check and warning to bus_register()

Quoted 29 lines · Greg KH

Quoted 18 lines · Shuah Khan
Add Legacy PM OPS usage checks to bus_register() function. If Legacy PM OPS usage is found, print warning message to indicate that the driver code needs updating to use Dev PM OPS interfaces. This will help serve as a way to track drivers that still use Legacy PM OPS and fix them.
The Legacy PM OPS check looks for suspend(struct device *, pm_message_t) or resume(struct device *) bus level interfaces.
Signed-off-by Shuah Khan <[email protected]>
---
  drivers/base/bus.c | 3 +++
  1 file changed, 3 insertions(+)
drivers/base/bus.c +0 -0
index 73f6c29..e8753a7 100644
drivers/base/bus.c int bus_register(struct bus_type *bus) · line 921
Quoted 6 lines
drivers/base/bus.c +3 -0
@@ -916,6 +916,9 @@ int bus_register(struct bus_type *bus)
  		goto bus_groups_fail;
 
  	pr_debug("bus: '%s': registered\n", bus->name);
+	if (bus->suspend || bus->resume)
+		pr_warn("bus '%s' needs updating - use pm pointer.\n",
+			bus->name);
Why can't we just sweep the tree for all of these now, fix them up, and then delete these fields and be done with it?

Quoted 4 lines

Same for the other ones, putting warnings in the kernel log files doesn't work at all for getting people to fix up their code (see examples of scsi log messages being there for _years_ about obsolete driver interfaces being used.)

This change is not a simple case of replacing legacy with pm ops. I have been working on changing drivers that use legacy, in some cases it is an easy change, however in some cases it requires adding new interfaces to port over from legacy to pm ops. Legacy suspend/resume is more course grain and pm ops offer fine grain control over suspend, hibernate etc. Also it has been a challenge without the hardware to test.

My thinking is adding warning might get the attention of individual driver owners.

-- Shuah

Signature · 4 lines

Shuah Khan
Senior Linux Kernel Developer - Open Source Group
Samsung Research America(Silicon Valley)
[email protected] | (970) 672-0658

Greg KH

maintainer

2014-01-06 21:29 UTC #4 lore raw

Re: [PATCH v2 1/3] drivers/bus: Add Legacy PM OPS usage check and warning to bus_register()

Quoted 42 lines · Shuah Khan

Quoted 29 lines · Greg KH
Quoted 18 lines · Shuah Khan
Add Legacy PM OPS usage checks to bus_register() function. If Legacy PM OPS usage is found, print warning message to indicate that the driver code needs updating to use Dev PM OPS interfaces. This will help serve as a way to track drivers that still use Legacy PM OPS and fix them.
The Legacy PM OPS check looks for suspend(struct device *, pm_message_t) or resume(struct device *) bus level interfaces.
Signed-off-by Shuah Khan <[email protected]>
---
 drivers/base/bus.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/base/bus.c +0 -0
index 73f6c29..e8753a7 100644
drivers/base/bus.c int bus_register(struct bus_type *bus) · line 921
Quoted 6 lines
drivers/base/bus.c +3 -0
@@ -916,6 +916,9 @@ int bus_register(struct bus_type *bus)
 		goto bus_groups_fail;
 
 	pr_debug("bus: '%s': registered\n", bus->name);
+	if (bus->suspend || bus->resume)
+		pr_warn("bus '%s' needs updating - use pm pointer.\n",
+			bus->name);
Why can't we just sweep the tree for all of these now, fix them up, and then delete these fields and be done with it?
Quoted 4 lines
Same for the other ones, putting warnings in the kernel log files doesn't work at all for getting people to fix up their code (see examples of scsi log messages being there for _years_ about obsolete driver interfaces being used.)
This change is not a simple case of replacing legacy with pm ops. I have been working on changing drivers that use legacy, in some cases it is an easy change, however in some cases it requires adding new interfaces to port over from legacy to pm ops. Legacy suspend/resume is more course grain and pm ops offer fine grain control over suspend, hibernate etc. Also it has been a challenge without the hardware to test.

Quoted 2 lines

My thinking is adding warning might get the attention of individual driver owners.

If it's not a trivial change, and takes a lot of work by someone who knows this type of thing (i.e. you), then I wouldn't start making kernel log warnings about this, it's just going to annoy a bunch of people :)

Again, see the SCSI log warnings for examples of this, people just tune them out, assuming someone else is going to fix the issue, someday. And that someday never comes...

thanks,

greg k-h

Shuah Khan

author

2014-01-06 21:44 UTC #5 lore raw

Re: [PATCH v2 1/3] drivers/bus: Add Legacy PM OPS usage check and warning to bus_register()

Quoted 43 lines · Greg KH

Quoted 42 lines · Shuah Khan
Quoted 29 lines · Greg KH
Quoted 18 lines · Shuah Khan
Add Legacy PM OPS usage checks to bus_register() function. If Legacy PM OPS usage is found, print warning message to indicate that the driver code needs updating to use Dev PM OPS interfaces. This will help serve as a way to track drivers that still use Legacy PM OPS and fix them.
The Legacy PM OPS check looks for suspend(struct device *, pm_message_t) or resume(struct device *) bus level interfaces.
Signed-off-by Shuah Khan <[email protected]>
---
  drivers/base/bus.c | 3 +++
  1 file changed, 3 insertions(+)
drivers/base/bus.c +0 -0
index 73f6c29..e8753a7 100644
drivers/base/bus.c int bus_register(struct bus_type *bus) · line 921
Quoted 6 lines
drivers/base/bus.c +3 -0
@@ -916,6 +916,9 @@ int bus_register(struct bus_type *bus)
  		goto bus_groups_fail;
 
  	pr_debug("bus: '%s': registered\n", bus->name);
+	if (bus->suspend || bus->resume)
+		pr_warn("bus '%s' needs updating - use pm pointer.\n",
+			bus->name);
Why can't we just sweep the tree for all of these now, fix them up, and then delete these fields and be done with it?
Quoted 4 lines
Same for the other ones, putting warnings in the kernel log files doesn't work at all for getting people to fix up their code (see examples of scsi log messages being there for _years_ about obsolete driver interfaces being used.)
This change is not a simple case of replacing legacy with pm ops. I have been working on changing drivers that use legacy, in some cases it is an easy change, however in some cases it requires adding new interfaces to port over from legacy to pm ops. Legacy suspend/resume is more course grain and pm ops offer fine grain control over suspend, hibernate etc. Also it has been a challenge without the hardware to test.

Quoted 6 lines

Quoted 2 lines
My thinking is adding warning might get the attention of individual driver owners.
If it's not a trivial change, and takes a lot of work by someone who knows this type of thing (i.e. you), then I wouldn't start making kernel log warnings about this, it's just going to annoy a bunch of people :)

ok I will continue fixing the drivers. :)

Quoted 5 lines

Again, see the SCSI log warnings for examples of this, people just tune them out, assuming someone else is going to fix the issue, someday. And that someday never comes...

Yeah. Warnings could just get ignored.

-- Shuah

Signature · 4 lines

Shuah Khan
Senior Linux Kernel Developer - Open Source Group
Samsung Research America(Silicon Valley)
[email protected] | (970) 672-0658

Shuah Khan

author

2014-01-06 20:09 UTC #6 lore raw

PATCH v2 2/3 drivers/class: Add Legacy PM OPS usage check and warning to __class_register()

In reply to Shuah Khan

Add Legacy PM OPS usage checks to __class_register() function. If Legacy PM OPS usage is found, print warning message to indicate that the driver code needs updating to use Dev PM OPS interfaces. This will help serve as a way to track drivers that still use Legacy PM OPS and fix them.

The Legacy PM OPS check looks for suspend(struct device *, pm_message_t) or resume(struct device *) class level interfaces.

Signed-off-by Shuah Khan <[email protected]>

---
 drivers/base/class.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/base/class.c +4 -0

index f96f704..2f20be5 100644
@@ -169,6 +169,10 @@ int __class_register(struct class *cls, struct lock_class_key *key)
 
 	pr_debug("device class '%s': registering\n", cls->name);
 
+	if (cls->suspend || cls->resume)
+		pr_warn("device class '%s' needs updating - use pm pointer.\n",
+			cls->name);
+
 	cp = kzalloc(sizeof(*cp), GFP_KERNEL);
 	if (!cp)
 		return -ENOMEM;

Signature · 1 line

1.8.3.2

Shuah Khan

author

2014-01-06 20:09 UTC #7 lore raw

PATCH v2 3/3 driver: Add Legacy PM OPS usage check and warning to driver_register()

In reply to Shuah Khan

Add Legacy PM OPS usage checks to driver_register() function. If Legacy PM OPS usage is found, print warning message to indicate that the driver code needs updating to use Dev PM OPS interfaces. This will help serve as a way to track drivers that still use Legacy PM OPS and fix them.

The Legacy PM OPS check looks for suspend(struct device *, pm_message_t) or resume(struct device *) struct device_driver interfaces.

Signed-off-by Shuah Khan <[email protected]>

---
 drivers/base/driver.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/base/driver.c +4 -0

index 9e29943..7838a8a 100644
@@ -157,6 +157,10 @@ int driver_register(struct device_driver *drv)
 		printk(KERN_WARNING "Driver '%s' needs updating - please use "
 			"bus_type methods\n", drv->name);
 
+	if (drv->suspend || drv->resume)
+		pr_warn("Driver '%s' needs updating - use pm pointer.\n",
+			drv->name);
+
 	other = driver_find(drv->name, drv->bus);
 	if (other) {
 		printk(KERN_ERR "Error: Driver '%s' is already registered, "

Signature · 1 line

1.8.3.2