On Friday, March 11, 2011, Greg KH wrote:
> On Thu, Mar 10, 2011 at 12:30:45PM +0100, Rafael J. Wysocki wrote:
> > On Thursday, March 10, 2011, Alan Stern wrote:
> > > On Thu, 10 Mar 2011, Rafael J. Wysocki wrote:
> > >
> > > > Some subsystems need to carry out suspend/resume and shutdown
> > > > operations with one CPU on-line and interrupts disabled. The only
> > > > way to register such operations is to define a sysdev class and
> > > > a sysdev specifically for this purpose which is cumbersome and
> > > > inefficient. Moreover, the arguments taken by sysdev suspend,
> > > > resume and shutdown callbacks are practically never necessary.
> > > >
> > > > For this reason, introduce a simpler interface allowing subsystems
> > > > to register operations to be executed very late during system suspend
> > > > and shutdown and very early during resume in the form of
> > > > strcut syscore_ops objects.
> > >
> > > ...
> > >
> > > > Index: linux-2.6/drivers/base/syscore.c
> > > > ===================================================================
> > > > --- /dev/null
> > > > +++ linux-2.6/drivers/base/syscore.c
> > >
> > > It's true that the existing sys.c file lies in drivers/base; this is
> > > presumably because it handles a bunch of class-related registration
> > > stuff. Now you're getting rid of all that, leaving just the
> > > power-related operations, so doesn't it make more sense to put this
> > > file in drivers/base/power?
> > >
> > > > +/**
> > > > + * syscore_suspend - Execute all the registered system core suspend callbacks.
> > > > + *
> > > > + * This function is executed with one CPU on-line and disabled interrupts.
> > > > + */
> > > > +int syscore_suspend(void)
> > > > +{
> > > > + struct syscore_ops *ops;
> > > > +
> > > > + list_for_each_entry_reverse(ops, &syscore_ops_list, node)
> > > > + if (ops->suspend) {
> > > > + int ret = ops->suspend();
> > > > + if (ret) {
> > > > + pr_err("PM: System core suspend callback "
> > > > + "%pF failed.\n", ops->suspend);
> > > > + return ret;
> > >
> > > If an error occurs, you need to go back and resume all the things that
> > > were suspended. At least, that's what the code in sysdev_suspend does.
> > >
> > > > + }
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> >
> > Below is a new version of the patch. I've taken your comment on the failing
> > suspend into account, fix the list traversal direction in syscore_shutdown()
> > and added some debug statements.
> >
> > Thanks,
> > Rafael
> >
> > ---
> > Some subsystems need to carry out suspend/resume and shutdown
> > operations with one CPU on-line and interrupts disabled. The only
> > way to register such operations is to define a sysdev class and
> > a sysdev specifically for this purpose which is cumbersome and
> > inefficient. Moreover, the arguments taken by sysdev suspend,
> > resume and shutdown callbacks are practically never necessary.
> >
> > For this reason, introduce a simpler interface allowing subsystems
> > to register operations to be executed very late during system suspend
> > and shutdown and very early during resume in the form of
> > strcut syscore_ops objects.
> >
> > ---
> > drivers/base/Makefile | 2
> > drivers/base/syscore.c | 107 ++++++++++++++++++++++++++++++++++++++++++++
> > include/linux/syscore_ops.h | 29 +++++++++++
> > kernel/power/hibernate.c | 9 +++
> > kernel/power/suspend.c | 4 +
> > kernel/sys.c | 4 +
> > 6 files changed, 154 insertions(+), 1 deletion(-)
> >
> > Index: linux-2.6/include/linux/syscore_ops.h
> > ===================================================================
> > --- /dev/null
> > +++ linux-2.6/include/linux/syscore_ops.h
> > @@ -0,0 +1,29 @@
> > +/*
> > + * syscore_ops.h - System core operations.
> > + *
> > + * Copyright (C) 2011 Rafael J. Wysocki <rjw@xxxxxxx>, Novell Inc.
> > + *
> > + * This file is released under the GPLv2.
> > + */
> > +
> > +#ifndef _LINUX_SYSCORE_OPS_H
> > +#define _LINUX_SYSCORE_OPS_H
> > +
> > +#include <linux/list.h>
> > +
> > +struct syscore_ops {
> > + struct list_head node;
> > + int (*suspend)(void);
> > + void (*resume)(void);
> > + void (*shutdown)(void);
> > +};
> > +
> > +extern void register_syscore_ops(struct syscore_ops *ops);
> > +extern void unregister_syscore_ops(struct syscore_ops *ops);
> > +#ifdef CONFIG_PM_SLEEP
> > +extern int syscore_suspend(void);
> > +extern void syscore_resume(void);
> > +#endif
>
> Minor nit, provide inline functions for these when CONFIG_PM_SLEEP is
> not defined so the code still builds?
The code using them depends on CONFIG_PM_SLEEP and they are nobody else's
business. :-)
I could avoid using the #ifdef here, but I thought I'd make it clear that
these things were only available when CONFIG_PM_SLEEP was set.
> Other than that, this looks great to me, thanks for doing this.
No problem. :-)
> Do you want me to take it through my tree, or yours?
I can handle it if you give me an ack.
Do you think I should push [1/2] alone for 2.6.39 or wait for the patches
converting subsystems to use this stuff to be ready? I think it'll take
some time to prepare them, especialy for things in the ARM tree that use
sysdevs in some interesting ways ...
Thanks,
Rafael
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
