Re: [PATCH v3 1/6] PM / core: Add LEAVE_SUSPENDED driver flag

[Rafael J. Wysocki]

On Fri, Nov 17, 2017 at 2:49 PM, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
> [...]
>
>>>
>>>>> Second, have you considered setting the default value of
>>>>> dev->power.may_skip_resume to true?
>>>>
>>>> Yes.
>>>>
>>>>> That would means the subsystem
>>>>> instead need to implement an opt-out method. I am thinking that it may
>>>>> not be an issue, since we anyway at this point, don't have drivers
>>>>> using the LEAVE_SUSPENDED flag.
>>>>
>>>> Opt-out doesn't work because of the need to invoke the "noirq" callbacks.
>>>
>>> I am not sure I follow that.
>>>
>>> Whatever needs to be fixed on the subsystem level, that could be done
>>> before the driver starts using the LEAVE_SUSPENDED flag. No?
>>
>> That requires a bit of explanation, sorry for being overly concise.
>>
>> The core calls ->resume_noirq from the middle layer regardless of
>> whether or not the device will be left suspended, so the
>> ->resume_noirq cannot do arbitrary things to it.  Setting
>> may_skip_resume by the middle layer tells the core that the middle
>> layer is ready for that and is going to cooperate.  If may_skip_resume
>> had been set by default, that piece of information would have been
>> missing.
>
> Huh, I still don't get that. Sorry.
>
> If the "may_skip_resume" is default set to true by the PM core,
> wouldn't that just mean that the middle-layer needs to implement an
> opt-out method, rather than opt-in. In principle to opt-out the
> middle-layer needs to set may_skip_resume to false in suspend_noirq
> phase, no?

Yes, but if the middle-layer doesn't clear it, that may mean two
things.  First, the middle layer is ready and so on.  Good.  Second,
the middle layer is not aware of the whole thing.  Not good.  The core
cannot tell.

In the opt-in case, however, all is clear. :-)

> Then we only need to make sure drivers don't starts use
> LEAVE_SUSPENDED, before we make sure the middle layers is adopted. But
> that should not be a problem.
>
> The benefit would be that those middle layers that can cope with
> LEAVE_SUSPENDED as of today don't need to change.

I'm not sure if that's the case.

The middle layer has to evaluate dev_pm_may_skip_resume() in
->resume_noirq() to check if the device can be left in suspend, as it
cannot determine that in ->suspend_noirq() yet.

Thanks,
Rafael