2009/5/2 Rafael J. Wysocki <rjw@xxxxxxx>:
> [--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;
> ...
> }
I don't think that is much better, but I changed the first patch to
use a loop anyway. The loop is then removed (as before) in the patch
that adds the stats.
>
> [--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. :-)
You have been making other changes to those files which made it a moving target.
> 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?
The stats patch also used it to track wakeup events, but I have added
another call for this to get rid of the sysdev.
--
Arve Hjønnevåg
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
