pm_register() and pm_send_all()

pm_register() and pm_send_all()

[Andrew Morton]

This:

kernel/power/pm.c:203: warning: 'pm_register' is deprecated (declared at kernel/power/pm.c:62)
kernel/power/pm.c:203: warning: 'pm_register' is deprecated (declared at kernel/power/pm.c:62)
kernel/power/pm.c:204: warning: 'pm_send_all' is deprecated (declared at kernel/power/pm.c:178)
kernel/power/pm.c:204: warning: 'pm_send_all' is deprecated (declared at kernel/power/pm.c:178)

is getting a bit boring.  These were marked deprecated at least three years
ago.


pm_register() no longer has any callers and afacit can be removed
immediately.

pm_send_all() has just a handful of callers:

arch/x86/kernel/apm_32.c
arch/mips/au1000/common/power.c
arch/frv/kernel/pm.c

could we please get a bit of a push on this?  How should these
pm_send_all() calls be replaced?

Thanks.

(I almost have a warning-free x86_64 allmodconfig, but the one in
drivers/message/i2o/i2o_config.c is going to be a problem).

Re: pm_register() and pm_send_all()

[Rafael J. Wysocki]

On Wednesday, 12 of March 2008, Andrew Morton wrote:
> 
> This:
> 
> kernel/power/pm.c:203: warning: 'pm_register' is deprecated (declared at kernel/power/pm.c:62)
> kernel/power/pm.c:203: warning: 'pm_register' is deprecated (declared at kernel/power/pm.c:62)
> kernel/power/pm.c:204: warning: 'pm_send_all' is deprecated (declared at kernel/power/pm.c:178)
> kernel/power/pm.c:204: warning: 'pm_send_all' is deprecated (declared at kernel/power/pm.c:178)
> 
> is getting a bit boring.  These were marked deprecated at least three years
> ago.
> 
> 
> pm_register() no longer has any callers and afacit can be removed
> immediately.
> 
> pm_send_all() has just a handful of callers:
> 
> arch/x86/kernel/apm_32.c
> arch/mips/au1000/common/power.c
> arch/frv/kernel/pm.c
> 
> could we please get a bit of a push on this?  How should these
> pm_send_all() calls be replaced?

Well, I was going to do that some time ago, but I'm not really familiar with
the old code.  Some assistance would be welcome.

Thanks,
Rafael

Re: pm_register() and pm_send_all()

[Pavel Machek]

On Wed 2008-03-12 15:00:42, Andrew Morton wrote:
> 
> This:
> 
> kernel/power/pm.c:203: warning: 'pm_register' is deprecated (declared at kernel/power/pm.c:62)
> kernel/power/pm.c:203: warning: 'pm_register' is deprecated (declared at kernel/power/pm.c:62)
> kernel/power/pm.c:204: warning: 'pm_send_all' is deprecated (declared at kernel/power/pm.c:178)
> kernel/power/pm.c:204: warning: 'pm_send_all' is deprecated (declared at kernel/power/pm.c:178)
> 
> is getting a bit boring.  These were marked deprecated at least three years
> ago.
> 
> 
> pm_register() no longer has any callers and afacit can be removed
> immediately.
> 
> pm_send_all() has just a handful of callers:
> 
> arch/x86/kernel/apm_32.c
> arch/mips/au1000/common/power.c
> arch/frv/kernel/pm.c

AFAICT pm_send_all is a nop when noone uses pm_register...

Hmm.. can we just force CONFIG_PM_LEGACY=n, and see what happens?

Or maybe this is better idea? It may break build somewhere, but it
should be easy to fix... (it builds here, i386 and x86-64).

Signed-off-by: Pavel Machek <pavel@suse.cz>

diff --git a/arch/frv/kernel/pm.c b/arch/frv/kernel/pm.c
index c57ce3f..73f3aee 100644
--- a/arch/frv/kernel/pm.c
+++ b/arch/frv/kernel/pm.c
@@ -163,14 +163,11 @@ static int sysctl_pm_do_suspend(ctl_tabl
 	if ((mode != 1) && (mode != 5))
 		return -EINVAL;
 
-	retval = pm_send_all(PM_SUSPEND, (void *)3);
-
 	if (retval == 0) {
 		if (mode == 5)
 		    retval = pm_do_bus_sleep();
 		else
 		    retval = pm_do_suspend();
-		pm_send_all(PM_RESUME, (void *)0);
 	}
 
 	return retval;
@@ -183,9 +180,6 @@ static int try_set_cmode(int new_cmode)
 	if (!(clock_cmodes_permitted & (1<<new_cmode)))
 		return -EINVAL;
 
-	/* tell all the drivers we're suspending */
-	pm_send_all(PM_SUSPEND, (void *)3);
-
 	/* now change cmode */
 	local_irq_disable();
 	frv_dma_pause_all();
@@ -201,8 +195,6 @@ #endif
 	frv_dma_resume_all();
 	local_irq_enable();
 
-	/* tell all the drivers we're resuming */
-	pm_send_all(PM_RESUME, (void *)0);
 	return 0;
 }
 
diff --git a/arch/mips/au1000/common/power.c b/arch/mips/au1000/common/power.c
index 54047d6..a9f7f63 100644
--- a/arch/mips/au1000/common/power.c
+++ b/arch/mips/au1000/common/power.c
@@ -258,7 +258,6 @@ #endif
 static int pm_do_sleep(ctl_table * ctl, int write, struct file *file,
 		       void __user *buffer, size_t * len, loff_t *ppos)
 {
-	int retval = 0;
 #ifdef SLEEP_TEST_TIMEOUT
 #define TMPBUFLEN2 16
 	char buf[TMPBUFLEN2], *p;
@@ -278,33 +277,21 @@ #ifdef SLEEP_TEST_TIMEOUT
 		p = buf;
 		sleep_ticks = simple_strtoul(p, &p, 0);
 #endif
-		retval = pm_send_all(PM_SUSPEND, (void *) 2);
-
-		if (retval)
-			return retval;
 
 		au_sleep();
-		retval = pm_send_all(PM_RESUME, (void *) 0);
 	}
-	return retval;
+	return 0;
 }
 
 static int pm_do_suspend(ctl_table * ctl, int write, struct file *file,
 			 void __user *buffer, size_t * len, loff_t *ppos)
 {
-	int retval = 0;
-
 	if (!write) {
 		*len = 0;
 	} else {
-		retval = pm_send_all(PM_SUSPEND, (void *) 2);
-		if (retval)
-			return retval;
 		suspend_mode = 1;
-
-		retval = pm_send_all(PM_RESUME, (void *) 0);
 	}
-	return retval;
+	return 0;
 }
 
 
diff --git a/arch/x86/kernel/apm_32.c b/arch/x86/kernel/apm_32.c
index d4438ef..d7e92bf 100644
--- a/arch/x86/kernel/apm_32.c
+++ b/arch/x86/kernel/apm_32.c
@@ -1189,19 +1189,6 @@ static int suspend(int vetoable)
 	int err;
 	struct apm_user	*as;
 
-	if (pm_send_all(PM_SUSPEND, (void *)3)) {
-		/* Vetoed */
-		if (vetoable) {
-			if (apm_info.connection_version > 0x100)
-				set_system_power_state(APM_STATE_REJECT);
-			err = -EBUSY;
-			ignore_sys_suspend = 0;
-			printk(KERN_WARNING "apm: suspend was vetoed.\n");
-			goto out;
-		}
-		printk(KERN_CRIT "apm: suspend was vetoed, but suspending anyway.\n");
-	}
-
 	device_suspend(PMSG_SUSPEND);
 	local_irq_disable();
 	device_power_down(PMSG_SUSPEND);
@@ -1224,7 +1211,6 @@ static int suspend(int vetoable)
 	device_power_up();
 	local_irq_enable();
 	device_resume();
-	pm_send_all(PM_RESUME, (void *)0);
 	queue_event(APM_NORMAL_RESUME, NULL);
  out:
 	spin_lock(&user_list_lock);
@@ -1337,7 +1323,6 @@ #endif
 			if ((event != APM_NORMAL_RESUME)
 			    || (ignore_normal_resume == 0)) {
 				device_resume();
-				pm_send_all(PM_RESUME, (void *)0);
 				queue_event(event, NULL);
 			}
 			ignore_normal_resume = 0;
diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index e2d7b39..d6d8f8b 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -19,16 +19,6 @@ config PM
 	  will issue the hlt instruction if nothing is to be done, thereby
 	  sending the processor to sleep and saving power.
 
-config PM_LEGACY
-	bool "Legacy Power Management API (DEPRECATED)"
-	depends on PM
-	default n
-	---help---
-	   Support for pm_register() and friends.  This old API is obsoleted
-	   by the driver model.
-
-	   If unsure, say N.
-
 config PM_DEBUG
 	bool "Power Management Debug Support"
 	depends on PM
diff --git a/kernel/power/Makefile b/kernel/power/Makefile
index e5693d6..523abb9 100644
--- a/kernel/power/Makefile
+++ b/kernel/power/Makefile
@@ -4,7 +4,6 @@ EXTRA_CFLAGS	+=	-DDEBUG
 endif
 
 obj-y				:= main.o
-obj-$(CONFIG_PM_LEGACY)		+= pm.o
 obj-$(CONFIG_PM_SLEEP)		+= process.o console.o sleepy.o
 obj-$(CONFIG_HIBERNATION)	+= swsusp.o disk.o snapshot.o swap.o user.o
 
diff --git a/kernel/power/pm.c b/kernel/power/pm.c
deleted file mode 100644
index 60c73fa..0000000
--- a/kernel/power/pm.c
+++ /dev/null
@@ -1,205 +0,0 @@
-/*
- *  pm.c - Power management interface
- *
- *  Copyright (C) 2000 Andrew Henroid
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation; either version 2 of the License, or
- *  (at your option) any later version.
- *
- *  This program is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with this program; if not, write to the Free Software
- *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
- */
-#include <linux/init.h>
-#include <linux/module.h>
-#include <linux/spinlock.h>
-#include <linux/mm.h>
-#include <linux/slab.h>
-#include <linux/pm.h>
-#include <linux/pm_legacy.h>
-#include <linux/interrupt.h>
-#include <linux/mutex.h>
-
-/*
- *	Locking notes:
- *		pm_devs_lock can be a semaphore providing pm ops are not called
- *	from an interrupt handler (already a bad idea so no change here). Each
- *	change must be protected so that an unlink of an entry doesn't clash
- *	with a pm send - which is permitted to sleep in the current architecture
- *
- *	Module unloads clashing with pm events now work out safely, the module 
- *	unload path will block until the event has been sent. It may well block
- *	until a resume but that will be fine.
- */
- 
-static DEFINE_MUTEX(pm_devs_lock);
-static LIST_HEAD(pm_devs);
-
-/**
- *	pm_register - register a device with power management
- *	@type: device type 
- *	@id: device ID
- *	@callback: callback function
- *
- *	Add a device to the list of devices that wish to be notified about
- *	power management events. A &pm_dev structure is returned on success,
- *	on failure the return is %NULL.
- *
- *      The callback function will be called in process context and
- *      it may sleep.
- */
- 
-struct pm_dev *pm_register(pm_dev_t type,
-			   unsigned long id,
-			   pm_callback callback)
-{
-	struct pm_dev *dev = kzalloc(sizeof(struct pm_dev), GFP_KERNEL);
-	if (dev) {
-		dev->type = type;
-		dev->id = id;
-		dev->callback = callback;
-
-		mutex_lock(&pm_devs_lock);
-		list_add(&dev->entry, &pm_devs);
-		mutex_unlock(&pm_devs_lock);
-	}
-	return dev;
-}
-
-/**
- *	pm_send - send request to a single device
- *	@dev: device to send to
- *	@rqst: power management request
- *	@data: data for the callback
- *
- *	Issue a power management request to a given device. The 
- *	%PM_SUSPEND and %PM_RESUME events are handled specially. The
- *	data field must hold the intended next state. No call is made
- *	if the state matches.
- *
- *	BUGS: what stops two power management requests occurring in parallel
- *	and conflicting.
- *
- *	WARNING: Calling pm_send directly is not generally recommended, in
- *	particular there is no locking against the pm_dev going away. The
- *	caller must maintain all needed locking or have 'inside knowledge'
- *	on the safety. Also remember that this function is not locked against
- *	pm_unregister. This means that you must handle SMP races on callback
- *	execution and unload yourself.
- */
- 
-static int pm_send(struct pm_dev *dev, pm_request_t rqst, void *data)
-{
-	int status = 0;
-	unsigned long prev_state, next_state;
-
-	if (in_interrupt())
-		BUG();
-
-	switch (rqst) {
-	case PM_SUSPEND:
-	case PM_RESUME:
-		prev_state = dev->state;
-		next_state = (unsigned long) data;
-		if (prev_state != next_state) {
-			if (dev->callback)
-				status = (*dev->callback)(dev, rqst, data);
-			if (!status) {
-				dev->state = next_state;
-				dev->prev_state = prev_state;
-			}
-		}
-		else {
-			dev->prev_state = prev_state;
-		}
-		break;
-	default:
-		if (dev->callback)
-			status = (*dev->callback)(dev, rqst, data);
-		break;
-	}
-	return status;
-}
-
-/*
- * Undo incomplete request
- */
-static void pm_undo_all(struct pm_dev *last)
-{
-	struct list_head *entry = last->entry.prev;
-	while (entry != &pm_devs) {
-		struct pm_dev *dev = list_entry(entry, struct pm_dev, entry);
-		if (dev->state != dev->prev_state) {
-			/* previous state was zero (running) resume or
-			 * previous state was non-zero (suspended) suspend
-			 */
-			pm_request_t undo = (dev->prev_state
-					     ? PM_SUSPEND:PM_RESUME);
-			pm_send(dev, undo, (void*) dev->prev_state);
-		}
-		entry = entry->prev;
-	}
-}
-
-/**
- *	pm_send_all - send request to all managed devices
- *	@rqst: power management request
- *	@data: data for the callback
- *
- *	Issue a power management request to a all devices. The 
- *	%PM_SUSPEND events are handled specially. Any device is 
- *	permitted to fail a suspend by returning a non zero (error)
- *	value from its callback function. If any device vetoes a 
- *	suspend request then all other devices that have suspended 
- *	during the processing of this request are restored to their
- *	previous state.
- *
- *	WARNING:  This function takes the pm_devs_lock. The lock is not dropped until
- *	the callbacks have completed. This prevents races against pm locking
- *	functions, races against module unload pm_unregister code. It does
- *	mean however that you must not issue pm_ functions within the callback
- *	or you will deadlock and users will hate you.
- *
- *	Zero is returned on success. If a suspend fails then the status
- *	from the device that vetoes the suspend is returned.
- *
- *	BUGS: what stops two power management requests occurring in parallel
- *	and conflicting.
- */
- 
-int pm_send_all(pm_request_t rqst, void *data)
-{
-	struct list_head *entry;
-	
-	mutex_lock(&pm_devs_lock);
-	entry = pm_devs.next;
-	while (entry != &pm_devs) {
-		struct pm_dev *dev = list_entry(entry, struct pm_dev, entry);
-		if (dev->callback) {
-			int status = pm_send(dev, rqst, data);
-			if (status) {
-				/* return devices to previous state on
-				 * failed suspend request
-				 */
-				if (rqst == PM_SUSPEND)
-					pm_undo_all(dev);
-				mutex_unlock(&pm_devs_lock);
-				return status;
-			}
-		}
-		entry = entry->next;
-	}
-	mutex_unlock(&pm_devs_lock);
-	return 0;
-}
-
-EXPORT_SYMBOL(pm_register);
-EXPORT_SYMBOL(pm_send_all);
-


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: pm_register() and pm_send_all()

[Ralf Baechle]

On Thu, Mar 13, 2008 at 10:28:13AM +0100, Pavel Machek wrote:

> AFAICT pm_send_all is a nop when noone uses pm_register...
> 
> Hmm.. can we just force CONFIG_PM_LEGACY=n, and see what happens?
> 
> Or maybe this is better idea? It may break build somewhere, but it
> should be easy to fix... (it builds here, i386 and x86-64).

Looks correct to me, thus ACK.

Spinning that a little further I wonder how useful the rest of the
Alchemy code - which seems to only deal with devices - is.  If at all.
I'm going to check with some Alchemy users if that code actually works or
can be removed.  I think the latter.  Also there is the APM emulation
available which provides similar functionality.

  Ralf

Re: pm_register() and pm_send_all()

[Ralf Baechle]

On Thu, Mar 13, 2008 at 12:28:23PM +0000, Ralf Baechle wrote:

> Looks correct to me, thus ACK.
> 
> Spinning that a little further I wonder how useful the rest of the
> Alchemy code - which seems to only deal with devices - is.  If at all.
> I'm going to check with some Alchemy users if that code actually works or
> can be removed.  I think the latter.  Also there is the APM emulation
> available which provides similar functionality.

So here a patch do cleanup a part.  Probably more to come

From: Ralf Baechle <ralf@linux-mips.org>

[MIPS] Alchey: Crapectomy after removal of pm_send_all calls.

Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

 arch/mips/au1000/common/power.c |   20 --------------------
 1 file changed, 20 deletions(-)

Index: linux-mips/arch/mips/au1000/common/power.c
===================================================================
--- linux-mips.orig/arch/mips/au1000/common/power.c
+++ linux-mips/arch/mips/au1000/common/power.c
@@ -283,18 +283,6 @@ static int pm_do_sleep(ctl_table * ctl, 
 	return 0;
 }
 
-static int pm_do_suspend(ctl_table * ctl, int write, struct file *file,
-			 void __user *buffer, size_t * len, loff_t *ppos)
-{
-	if (!write) {
-		*len = 0;
-	} else {
-		suspend_mode = 1;
-	}
-	return 0;
-}
-
-
 static int pm_do_freq(ctl_table * ctl, int write, struct file *file,
 		      void __user *buffer, size_t * len, loff_t *ppos)
 {
@@ -408,14 +396,6 @@ static int pm_do_freq(ctl_table * ctl, i
 
 static struct ctl_table pm_table[] = {
 	{
-		.ctl_name 	= CTL_UNNUMBERED,
-		.procname	= "suspend",
-		.data		= NULL,
-		.maxlen		= 0,
-		.mode		= 0600,
-		.proc_handler	= &pm_do_suspend
-	},
-	{
 		.ctl_name	= CTL_UNNUMBERED,
 		.procname	= "sleep",
 		.data		= NULL,

Re: pm_register() and pm_send_all()

[Pavel Machek]

On Thu 2008-03-13 14:19:17, Ralf Baechle wrote:
> On Thu, Mar 13, 2008 at 12:28:23PM +0000, Ralf Baechle wrote:
> 
> > Looks correct to me, thus ACK.
> > 
> > Spinning that a little further I wonder how useful the rest of the
> > Alchemy code - which seems to only deal with devices - is.  If at all.
> > I'm going to check with some Alchemy users if that code actually works or
> > can be removed.  I think the latter.  Also there is the APM emulation
> > available which provides similar functionality.
> 
> So here a patch do cleanup a part.  Probably more to come
> 
> From: Ralf Baechle <ralf@linux-mips.org>
> 
> [MIPS] Alchey: Crapectomy after removal of pm_send_all calls.
> 
> Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

ACK.

> 
>  arch/mips/au1000/common/power.c |   20 --------------------
>  1 file changed, 20 deletions(-)
> 
> Index: linux-mips/arch/mips/au1000/common/power.c
> ===================================================================
> --- linux-mips.orig/arch/mips/au1000/common/power.c
> +++ linux-mips/arch/mips/au1000/common/power.c
> @@ -283,18 +283,6 @@ static int pm_do_sleep(ctl_table * ctl, 
>  	return 0;
>  }
>  
> -static int pm_do_suspend(ctl_table * ctl, int write, struct file *file,
> -			 void __user *buffer, size_t * len, loff_t *ppos)
> -{
> -	if (!write) {
> -		*len = 0;
> -	} else {
> -		suspend_mode = 1;
> -	}
> -	return 0;
> -}
> -
> -
>  static int pm_do_freq(ctl_table * ctl, int write, struct file *file,
>  		      void __user *buffer, size_t * len, loff_t *ppos)
>  {
> @@ -408,14 +396,6 @@ static int pm_do_freq(ctl_table * ctl, i
>  
>  static struct ctl_table pm_table[] = {
>  	{
> -		.ctl_name 	= CTL_UNNUMBERED,
> -		.procname	= "suspend",
> -		.data		= NULL,
> -		.maxlen		= 0,
> -		.mode		= 0600,
> -		.proc_handler	= &pm_do_suspend
> -	},
> -	{
>  		.ctl_name	= CTL_UNNUMBERED,
>  		.procname	= "sleep",
>  		.data		= NULL,

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
pomozte zachranit klanovicky les:  http://www.ujezdskystrom.info/

Re: pm_register() and pm_send_all()

[Rafael J. Wysocki]

On Thursday, 13 of March 2008, Pavel Machek wrote:
> On Wed 2008-03-12 15:00:42, Andrew Morton wrote:
> > 
> > This:
> > 
> > kernel/power/pm.c:203: warning: 'pm_register' is deprecated (declared at kernel/power/pm.c:62)
> > kernel/power/pm.c:203: warning: 'pm_register' is deprecated (declared at kernel/power/pm.c:62)
> > kernel/power/pm.c:204: warning: 'pm_send_all' is deprecated (declared at kernel/power/pm.c:178)
> > kernel/power/pm.c:204: warning: 'pm_send_all' is deprecated (declared at kernel/power/pm.c:178)
> > 
> > is getting a bit boring.  These were marked deprecated at least three years
> > ago.
> > 
> > 
> > pm_register() no longer has any callers and afacit can be removed
> > immediately.
> > 
> > pm_send_all() has just a handful of callers:
> > 
> > arch/x86/kernel/apm_32.c
> > arch/mips/au1000/common/power.c
> > arch/frv/kernel/pm.c
> 
> AFAICT pm_send_all is a nop when noone uses pm_register...

Apparently on 2.6.24 it's not used anywehere, at least according to LXR.

> Hmm.. can we just force CONFIG_PM_LEGACY=n, and see what happens?

Do you want me to push this patch and the following one from Ralf to Len?

> Or maybe this is better idea? It may break build somewhere, but it
> should be easy to fix... (it builds here, i386 and x86-64).
> 
> Signed-off-by: Pavel Machek <pavel@suse.cz>
> 
> diff --git a/arch/frv/kernel/pm.c b/arch/frv/kernel/pm.c
> index c57ce3f..73f3aee 100644
> --- a/arch/frv/kernel/pm.c
> +++ b/arch/frv/kernel/pm.c
> @@ -163,14 +163,11 @@ static int sysctl_pm_do_suspend(ctl_tabl
>  	if ((mode != 1) && (mode != 5))
>  		return -EINVAL;
>  
> -	retval = pm_send_all(PM_SUSPEND, (void *)3);
> -
>  	if (retval == 0) {
>  		if (mode == 5)
>  		    retval = pm_do_bus_sleep();
>  		else
>  		    retval = pm_do_suspend();
> -		pm_send_all(PM_RESUME, (void *)0);
>  	}
>  
>  	return retval;
> @@ -183,9 +180,6 @@ static int try_set_cmode(int new_cmode)
>  	if (!(clock_cmodes_permitted & (1<<new_cmode)))
>  		return -EINVAL;
>  
> -	/* tell all the drivers we're suspending */
> -	pm_send_all(PM_SUSPEND, (void *)3);
> -
>  	/* now change cmode */
>  	local_irq_disable();
>  	frv_dma_pause_all();
> @@ -201,8 +195,6 @@ #endif
>  	frv_dma_resume_all();
>  	local_irq_enable();
>  
> -	/* tell all the drivers we're resuming */
> -	pm_send_all(PM_RESUME, (void *)0);
>  	return 0;
>  }
>  
> diff --git a/arch/mips/au1000/common/power.c b/arch/mips/au1000/common/power.c
> index 54047d6..a9f7f63 100644
> --- a/arch/mips/au1000/common/power.c
> +++ b/arch/mips/au1000/common/power.c
> @@ -258,7 +258,6 @@ #endif
>  static int pm_do_sleep(ctl_table * ctl, int write, struct file *file,
>  		       void __user *buffer, size_t * len, loff_t *ppos)
>  {
> -	int retval = 0;
>  #ifdef SLEEP_TEST_TIMEOUT
>  #define TMPBUFLEN2 16
>  	char buf[TMPBUFLEN2], *p;
> @@ -278,33 +277,21 @@ #ifdef SLEEP_TEST_TIMEOUT
>  		p = buf;
>  		sleep_ticks = simple_strtoul(p, &p, 0);
>  #endif
> -		retval = pm_send_all(PM_SUSPEND, (void *) 2);
> -
> -		if (retval)
> -			return retval;
>  
>  		au_sleep();
> -		retval = pm_send_all(PM_RESUME, (void *) 0);
>  	}
> -	return retval;
> +	return 0;
>  }
>  
>  static int pm_do_suspend(ctl_table * ctl, int write, struct file *file,
>  			 void __user *buffer, size_t * len, loff_t *ppos)
>  {
> -	int retval = 0;
> -
>  	if (!write) {
>  		*len = 0;
>  	} else {
> -		retval = pm_send_all(PM_SUSPEND, (void *) 2);
> -		if (retval)
> -			return retval;
>  		suspend_mode = 1;
> -
> -		retval = pm_send_all(PM_RESUME, (void *) 0);
>  	}
> -	return retval;
> +	return 0;
>  }
>  
>  
> diff --git a/arch/x86/kernel/apm_32.c b/arch/x86/kernel/apm_32.c
> index d4438ef..d7e92bf 100644
> --- a/arch/x86/kernel/apm_32.c
> +++ b/arch/x86/kernel/apm_32.c
> @@ -1189,19 +1189,6 @@ static int suspend(int vetoable)
>  	int err;
>  	struct apm_user	*as;
>  
> -	if (pm_send_all(PM_SUSPEND, (void *)3)) {
> -		/* Vetoed */
> -		if (vetoable) {
> -			if (apm_info.connection_version > 0x100)
> -				set_system_power_state(APM_STATE_REJECT);
> -			err = -EBUSY;
> -			ignore_sys_suspend = 0;
> -			printk(KERN_WARNING "apm: suspend was vetoed.\n");
> -			goto out;
> -		}
> -		printk(KERN_CRIT "apm: suspend was vetoed, but suspending anyway.\n");
> -	}
> -
>  	device_suspend(PMSG_SUSPEND);
>  	local_irq_disable();
>  	device_power_down(PMSG_SUSPEND);
> @@ -1224,7 +1211,6 @@ static int suspend(int vetoable)
>  	device_power_up();
>  	local_irq_enable();
>  	device_resume();
> -	pm_send_all(PM_RESUME, (void *)0);
>  	queue_event(APM_NORMAL_RESUME, NULL);
>   out:
>  	spin_lock(&user_list_lock);
> @@ -1337,7 +1323,6 @@ #endif
>  			if ((event != APM_NORMAL_RESUME)
>  			    || (ignore_normal_resume == 0)) {
>  				device_resume();
> -				pm_send_all(PM_RESUME, (void *)0);
>  				queue_event(event, NULL);
>  			}
>  			ignore_normal_resume = 0;
> diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> index e2d7b39..d6d8f8b 100644
> --- a/kernel/power/Kconfig
> +++ b/kernel/power/Kconfig
> @@ -19,16 +19,6 @@ config PM
>  	  will issue the hlt instruction if nothing is to be done, thereby
>  	  sending the processor to sleep and saving power.
>  
> -config PM_LEGACY
> -	bool "Legacy Power Management API (DEPRECATED)"
> -	depends on PM
> -	default n
> -	---help---
> -	   Support for pm_register() and friends.  This old API is obsoleted
> -	   by the driver model.
> -
> -	   If unsure, say N.
> -
>  config PM_DEBUG
>  	bool "Power Management Debug Support"
>  	depends on PM
> diff --git a/kernel/power/Makefile b/kernel/power/Makefile
> index e5693d6..523abb9 100644
> --- a/kernel/power/Makefile
> +++ b/kernel/power/Makefile
> @@ -4,7 +4,6 @@ EXTRA_CFLAGS	+=	-DDEBUG
>  endif
>  
>  obj-y				:= main.o
> -obj-$(CONFIG_PM_LEGACY)		+= pm.o
>  obj-$(CONFIG_PM_SLEEP)		+= process.o console.o sleepy.o
>  obj-$(CONFIG_HIBERNATION)	+= swsusp.o disk.o snapshot.o swap.o user.o
>  
> diff --git a/kernel/power/pm.c b/kernel/power/pm.c
> deleted file mode 100644
> index 60c73fa..0000000
> --- a/kernel/power/pm.c
> +++ /dev/null
> @@ -1,205 +0,0 @@
> -/*
> - *  pm.c - Power management interface
> - *
> - *  Copyright (C) 2000 Andrew Henroid
> - *
> - *  This program is free software; you can redistribute it and/or modify
> - *  it under the terms of the GNU General Public License as published by
> - *  the Free Software Foundation; either version 2 of the License, or
> - *  (at your option) any later version.
> - *
> - *  This program is distributed in the hope that it will be useful,
> - *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> - *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - *  GNU General Public License for more details.
> - *
> - *  You should have received a copy of the GNU General Public License
> - *  along with this program; if not, write to the Free Software
> - *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> - */
> -#include <linux/init.h>
> -#include <linux/module.h>
> -#include <linux/spinlock.h>
> -#include <linux/mm.h>
> -#include <linux/slab.h>
> -#include <linux/pm.h>
> -#include <linux/pm_legacy.h>
> -#include <linux/interrupt.h>
> -#include <linux/mutex.h>
> -
> -/*
> - *	Locking notes:
> - *		pm_devs_lock can be a semaphore providing pm ops are not called
> - *	from an interrupt handler (already a bad idea so no change here). Each
> - *	change must be protected so that an unlink of an entry doesn't clash
> - *	with a pm send - which is permitted to sleep in the current architecture
> - *
> - *	Module unloads clashing with pm events now work out safely, the module 
> - *	unload path will block until the event has been sent. It may well block
> - *	until a resume but that will be fine.
> - */
> - 
> -static DEFINE_MUTEX(pm_devs_lock);
> -static LIST_HEAD(pm_devs);
> -
> -/**
> - *	pm_register - register a device with power management
> - *	@type: device type 
> - *	@id: device ID
> - *	@callback: callback function
> - *
> - *	Add a device to the list of devices that wish to be notified about
> - *	power management events. A &pm_dev structure is returned on success,
> - *	on failure the return is %NULL.
> - *
> - *      The callback function will be called in process context and
> - *      it may sleep.
> - */
> - 
> -struct pm_dev *pm_register(pm_dev_t type,
> -			   unsigned long id,
> -			   pm_callback callback)
> -{
> -	struct pm_dev *dev = kzalloc(sizeof(struct pm_dev), GFP_KERNEL);
> -	if (dev) {
> -		dev->type = type;
> -		dev->id = id;
> -		dev->callback = callback;
> -
> -		mutex_lock(&pm_devs_lock);
> -		list_add(&dev->entry, &pm_devs);
> -		mutex_unlock(&pm_devs_lock);
> -	}
> -	return dev;
> -}
> -
> -/**
> - *	pm_send - send request to a single device
> - *	@dev: device to send to
> - *	@rqst: power management request
> - *	@data: data for the callback
> - *
> - *	Issue a power management request to a given device. The 
> - *	%PM_SUSPEND and %PM_RESUME events are handled specially. The
> - *	data field must hold the intended next state. No call is made
> - *	if the state matches.
> - *
> - *	BUGS: what stops two power management requests occurring in parallel
> - *	and conflicting.
> - *
> - *	WARNING: Calling pm_send directly is not generally recommended, in
> - *	particular there is no locking against the pm_dev going away. The
> - *	caller must maintain all needed locking or have 'inside knowledge'
> - *	on the safety. Also remember that this function is not locked against
> - *	pm_unregister. This means that you must handle SMP races on callback
> - *	execution and unload yourself.
> - */
> - 
> -static int pm_send(struct pm_dev *dev, pm_request_t rqst, void *data)
> -{
> -	int status = 0;
> -	unsigned long prev_state, next_state;
> -
> -	if (in_interrupt())
> -		BUG();
> -
> -	switch (rqst) {
> -	case PM_SUSPEND:
> -	case PM_RESUME:
> -		prev_state = dev->state;
> -		next_state = (unsigned long) data;
> -		if (prev_state != next_state) {
> -			if (dev->callback)
> -				status = (*dev->callback)(dev, rqst, data);
> -			if (!status) {
> -				dev->state = next_state;
> -				dev->prev_state = prev_state;
> -			}
> -		}
> -		else {
> -			dev->prev_state = prev_state;
> -		}
> -		break;
> -	default:
> -		if (dev->callback)
> -			status = (*dev->callback)(dev, rqst, data);
> -		break;
> -	}
> -	return status;
> -}
> -
> -/*
> - * Undo incomplete request
> - */
> -static void pm_undo_all(struct pm_dev *last)
> -{
> -	struct list_head *entry = last->entry.prev;
> -	while (entry != &pm_devs) {
> -		struct pm_dev *dev = list_entry(entry, struct pm_dev, entry);
> -		if (dev->state != dev->prev_state) {
> -			/* previous state was zero (running) resume or
> -			 * previous state was non-zero (suspended) suspend
> -			 */
> -			pm_request_t undo = (dev->prev_state
> -					     ? PM_SUSPEND:PM_RESUME);
> -			pm_send(dev, undo, (void*) dev->prev_state);
> -		}
> -		entry = entry->prev;
> -	}
> -}
> -
> -/**
> - *	pm_send_all - send request to all managed devices
> - *	@rqst: power management request
> - *	@data: data for the callback
> - *
> - *	Issue a power management request to a all devices. The 
> - *	%PM_SUSPEND events are handled specially. Any device is 
> - *	permitted to fail a suspend by returning a non zero (error)
> - *	value from its callback function. If any device vetoes a 
> - *	suspend request then all other devices that have suspended 
> - *	during the processing of this request are restored to their
> - *	previous state.
> - *
> - *	WARNING:  This function takes the pm_devs_lock. The lock is not dropped until
> - *	the callbacks have completed. This prevents races against pm locking
> - *	functions, races against module unload pm_unregister code. It does
> - *	mean however that you must not issue pm_ functions within the callback
> - *	or you will deadlock and users will hate you.
> - *
> - *	Zero is returned on success. If a suspend fails then the status
> - *	from the device that vetoes the suspend is returned.
> - *
> - *	BUGS: what stops two power management requests occurring in parallel
> - *	and conflicting.
> - */
> - 
> -int pm_send_all(pm_request_t rqst, void *data)
> -{
> -	struct list_head *entry;
> -	
> -	mutex_lock(&pm_devs_lock);
> -	entry = pm_devs.next;
> -	while (entry != &pm_devs) {
> -		struct pm_dev *dev = list_entry(entry, struct pm_dev, entry);
> -		if (dev->callback) {
> -			int status = pm_send(dev, rqst, data);
> -			if (status) {
> -				/* return devices to previous state on
> -				 * failed suspend request
> -				 */
> -				if (rqst == PM_SUSPEND)
> -					pm_undo_all(dev);
> -				mutex_unlock(&pm_devs_lock);
> -				return status;
> -			}
> -		}
> -		entry = entry->next;
> -	}
> -	mutex_unlock(&pm_devs_lock);
> -	return 0;
> -}
> -
> -EXPORT_SYMBOL(pm_register);
> -EXPORT_SYMBOL(pm_send_all);
> -

Re: pm_register() and pm_send_all()

[Pavel Machek]

On Thu 2008-03-13 21:54:58, Rafael J. Wysocki wrote:
> On Thursday, 13 of March 2008, Pavel Machek wrote:
> > On Wed 2008-03-12 15:00:42, Andrew Morton wrote:
> > > 
> > > This:
> > > 
> > > kernel/power/pm.c:203: warning: 'pm_register' is deprecated (declared at kernel/power/pm.c:62)
> > > kernel/power/pm.c:203: warning: 'pm_register' is deprecated (declared at kernel/power/pm.c:62)
> > > kernel/power/pm.c:204: warning: 'pm_send_all' is deprecated (declared at kernel/power/pm.c:178)
> > > kernel/power/pm.c:204: warning: 'pm_send_all' is deprecated (declared at kernel/power/pm.c:178)
> > > 
> > > is getting a bit boring.  These were marked deprecated at least three years
> > > ago.
> > > 
> > > 
> > > pm_register() no longer has any callers and afacit can be removed
> > > immediately.
> > > 
> > > pm_send_all() has just a handful of callers:
> > > 
> > > arch/x86/kernel/apm_32.c
> > > arch/mips/au1000/common/power.c
> > > arch/frv/kernel/pm.c
> > 
> > AFAICT pm_send_all is a nop when noone uses pm_register...
> 
> Apparently on 2.6.24 it's not used anywehere, at least according to LXR.
> 
> > Hmm.. can we just force CONFIG_PM_LEGACY=n, and see what happens?
> 
> Do you want me to push this patch and the following one from Ralf to
> Len?

Yes... I think we can kill pm_send_all etc...

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
pomozte zachranit klanovicky les:  http://www.ujezdskystrom.info/

Re: pm_register() and pm_send_all()

[Ingo Molnar]

* Pavel Machek <pavel@ucw.cz> wrote:

> --- a/arch/x86/kernel/apm_32.c
> +++ b/arch/x86/kernel/apm_32.c

thanks Pavel for taking care of this - the x86 bits look good. Those 
warnings have annoyed me for ages :-/

Acked-by: Ingo Molnar <mingo@elte.hu>

	Ingo

Re: pm_register() and pm_send_all()

[David Howells]

Andrew Morton <akpm@linux-foundation.org> wrote:

> pm_send_all() has just a handful of callers:

What should be done instead of calling pm_send_all()?

David

Re: pm_register() and pm_send_all()

[Andrew Morton]

On Thu, 13 Mar 2008 12:31:37 +0000 David Howells <dhowells@redhat.com> wrote:

> Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > pm_send_all() has just a handful of callers:
> 
> What should be done instead of calling pm_send_all()?
> 

I don't know, and neither does Rafael.  Hopefully Pavel does?

Re: pm_register() and pm_send_all()

[Pavel Machek]

On Thu 2008-03-13 12:31:37, David Howells wrote:
> Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > pm_send_all() has just a handful of callers:
> 
> What should be done instead of calling pm_send_all()?

pm_send_all is currently nop.

device_suspend/resume are closest equivalents. 

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
pomozte zachranit klanovicky les:  http://www.ujezdskystrom.info/

Re: pm_register() and pm_send_all()

[Andrew Morton]

On Thu, 13 Mar 2008 19:05:58 +0100
Pavel Machek <pavel@ucw.cz> wrote:

> On Thu 2008-03-13 12:31:37, David Howells wrote:
> > Andrew Morton <akpm@linux-foundation.org> wrote:
> > 
> > > pm_send_all() has just a handful of callers:
> > 
> > What should be done instead of calling pm_send_all()?
> 
> pm_send_all is currently nop.

So we could have deleted pm_send_all() and pm_register() ages ago?  And
we've all been needlessly staring at those warnings all this time? 
Grumble.

Could we please have a volunteer to take care of making these things go
away asap?

Re: pm_register() and pm_send_all()

[Pavel Machek]

On Thu 2008-03-13 11:26:14, Andrew Morton wrote:
> On Thu, 13 Mar 2008 19:05:58 +0100
> Pavel Machek <pavel@ucw.cz> wrote:
> 
> > On Thu 2008-03-13 12:31:37, David Howells wrote:
> > > Andrew Morton <akpm@linux-foundation.org> wrote:
> > > 
> > > > pm_send_all() has just a handful of callers:
> > > 
> > > What should be done instead of calling pm_send_all()?
> > 
> > pm_send_all is currently nop.
> 
> So we could have deleted pm_send_all() and pm_register() ages ago?  And
> we've all been needlessly staring at those warnings all this time? 
> Grumble.

we could have deleted pm_send_all when users of pm_register were gone,
yes.... just nobody noticed :-(.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
pomozte zachranit klanovicky les:  http://www.ujezdskystrom.info/