On Tue, Nov 29, 2016 at 2:27 AM, Chen, Yu C <[email protected]> wrote:
> Hi,
>> -----Original Message-----
>> From: [email protected] [mailto:[email protected]] On Behalf Of
>> Rafael J. Wysocki
>> Sent: Wednesday, November 23, 2016 7:03 AM
[cut]
>>
>> > + work_on_cpu(cpu, tstate_pm_fn, &save);
>>
>> Does work_on_cpu() wait for the work to complete?
>>
> Yes, it might increase the suspend/resume time, a 'queue_work_on' might be
> better?
Not really, because you'd need to wait before doing the
put_online_cpus() anyway.
>> > + put_online_cpus();
>> > +}
>> > +
>> > +static int tstate_suspend(struct notifier_block *nb,
>> > + unsigned long mode, void *_unused) {
>> > + tstate_check(mode, true);
>> > + return 0;
>> > +}
>> > +
>> > +static int tstate_resume(struct notifier_block *nb,
>> > + unsigned long mode, void *_unused) {
>> > + tstate_check(mode, false);
>> > + return 0;
>> > +}
>> > +
>> > +static int __init tstate_pm_init(void) {
>> > + /*
>> > + * tstate_suspend should save tstate after
>> > + * thermal zone's update in thermal_pm_notify,
>> > + * vice versa tstate_resume restore tstate before
>> > + * thermal_pm_notify, thus the thermal framework
>> > + * has a chance to re-adjust tstate according to the
>> > + * temperature trend.
>> > + */
>> > + pm_notifier(tstate_suspend, -1);
>> > + pm_notifier(tstate_resume, 1);
>>
>> I don't think this is going to do what you really want.
>>
>> Each of these notifiers is going to be invoked during both suspend and
>> resume,
> Yes,
>
>> so I guess you only need one notifier?
>
> Here's my original thought: tstate_suspend needs to be invoked after
> thermal_pm_notify, which has a priority of '0',
> so the notifier of tstate_suspend should be lower than '0',
> thus '-1'. And the same for tstate_resume,
> it should be invoked before thermal_pm_notify,
> thus priority is '1' ?
If there's a dependency like that, it needs to be explicit. That is,
thermal_pm_notify() needs to invoke your new code in the right order
with respect to what it does already.
Thanks,
Rafael
