On Thursday 30 April 2009, Arve Hjønnevåg wrote:
> 2009/4/29 Rafael J. Wysocki <rjw@xxxxxxx>:
> > On Wednesday 15 April 2009, Arve Hjønnevåg wrote:
> >> diff --git a/include/linux/suspend_block.h b/include/linux/suspend_block.h
> >> new file mode 100755
> >> index 0000000..7820c60
> >> --- /dev/null
> >> +++ b/include/linux/suspend_block.h
> >
> > suspend_blockers.h perhaps?
>
> suspend_blocker.h?
Fine by me.
[--snip--]
> >> +config SUSPEND_BLOCK
> >> + bool "Suspend block"
> >> + depends on PM
> >> + select RTC_LIB
> >
> > depends on RTC_LIB
> >
> > select doesn't really work and I don't think it ever will.
>
> Nothing depends on RTC_LIB, it is only selected.
If nothing in your code needs anything depending on RTC_LIB, why select it?
[--snip--]
> >
> >> +
> >> +retry:
> >> + if (suspend_is_blocked()) {
> >> + if (debug_mask & DEBUG_SUSPEND)
> >> + pr_info("suspend: abort suspend\n");
> >> + goto abort;
> >
> > If that were in a loop you could just use 'break' here.
>
> do you think this is better:
> while(1) {
> if (exception1) {
> ...
> break;
> }
> ...
> if (exception2) {
> ...
> continue;
> }
> break;
> }
I rather thought of
for (;;) {
if (exception1) {
...
break;
}
...
if (!exception2)
break;
...
}
[--snip--]
> >> +static int suspend_block_suspend(struct sys_device *dev, pm_message_t state)
> >> +{
> >> + int ret = suspend_is_blocked() ? -EAGAIN : 0;
> >> + if (debug_mask & DEBUG_SUSPEND)
> >> + pr_info("suspend_block_suspend return %d\n", ret);
> >> + return ret;
> >> +}
> >> +
> >> +static struct sysdev_class suspend_block_sysclass = {
> >> + .name = "suspend_block",
> >> + .suspend = suspend_block_suspend,
> >> +};
> >> +static struct sys_device suspend_block_sysdev = {
> >> + .cls = &suspend_block_sysclass,
> >> +};
> >> +
> >
> > Hmm. Perhaps add the suspend_is_blocked() check at the beginning of
> > sysdev_suspend() instead of this? Surely you don't want to suspend
> > any sysdevs with any suspend blockers active, right?
>
> You could make the same argument for any device. Using a sysdev makes
> the patch easier to apply.
You've lost me here. :-)
AFAICT, the only purpose of the sysdev class above is to abort suspend if
suspend_is_blocked() returns 'true'. If that is correct, then IMO you could
achieve the same goal by calling suspend_is_blocked() directly from
sysdev_suspend(), right before check_wakeup_irqs(). Or am I missing anything?
Thanks,
Rafael
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
