linux-kernel - Re: [PATCH v2 1/3] PM / Suspend: Add support to check if platform's power is off in suspend

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <126e9fd8-1009-47f7-7299-9e5e93ebfd4d@microchip.com>
Date:   Thu, 10 Jan 2019 10:24:14 +0000
From:   <Claudiu.Beznea@...rochip.com>
To:     <pavel@....cz>
CC:     <Nicolas.Ferre@...rochip.com>, <alexandre.belloni@...tlin.com>,
        <Ludovic.Desroches@...rochip.com>, <linux@...linux.org.uk>,
        <lgirdwood@...il.com>, <broonie@...nel.org>, <rjw@...ysocki.net>,
        <len.brown@...el.com>, <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>, <linux-pm@...r.kernel.org>
Subject: Re: [PATCH v2 1/3] PM / Suspend: Add support to check if platform's
 power is off in suspend

On 09.01.2019 16:14, Pavel Machek wrote:
> Hi!
> 
>> Add support to check if platform's power will be cut off in suspend.
>> This will help drivers shared by multiple platforms to take only the
>> necessary actions while suspending/resuming (some platform may not need
>> to save/restore all the registers if platforms remains powered while
>> suspended). In this way suspend/resume time could be improved.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@...rochip.com>
>> ---
>>  include/linux/suspend.h |  6 ++++++
>>  kernel/power/suspend.c  | 13 +++++++++++++
>>  2 files changed, 19 insertions(+)
>>
>> diff --git a/include/linux/suspend.h b/include/linux/suspend.h
>> index 3f529ad9a9d2..21f19b167fe2 100644
>> --- a/include/linux/suspend.h
>> +++ b/include/linux/suspend.h
>> @@ -173,6 +173,9 @@ static inline void dpm_save_failed_step(enum suspend_stat_step step)
>>   *	Called by the PM core if the suspending of devices fails.
>>   *	This callback is optional and should only be implemented by platforms
>>   *	which require special recovery actions in that situation.
>> + *
>> + * @off_in_suspend: Returns wether the platform's power will be cut off at
> 
> wether -- spelling?

Yep, I did it again :), sorry

> 
>> @@ -185,6 +188,7 @@ struct platform_suspend_ops {
>>  	bool (*suspend_again)(void);
>>  	void (*end)(void);
>>  	void (*recover)(void);
>> +	bool (*off_in_suspend)(suspend_state_t state);
>>  };
> 
> Dunno, should it be per-regulator? SoCs commonly have more than one
> power supply, with some of them off during suspend...

Every regulator has a suspend state which could be described via device
tree. In our case, besides other regulators, we have core regulator, which
is turned off in suspend at the end of the suspend procedure, when PMIC
detects a level transition on one of its pins, transition which is
triggered the moment the CPU is shut down (we are doing this from software
at the end of suspend procedure).

One aspect is that we map our proprietary power saving modes to Linux power
saving modes (suspend-to-ram and standby) at the system initialization. So,
we could have our backup mode mapped to any of suspend-to-ram or standby so
that when:
echo mem > /sys/power/state or
echo standby > /sys/power/state
the final power saving mode could be or not this backup mode (depending on
the mapping - which is done at system initialization based on kernel
parameters).

And so, depending on the mapped mode (which we currently know only from
arch/arm/mach-at91/) the real core's regulator could be on or off. We need
both these information to know if core's regulator would be off in suspend.

I chose to have it here, in platform_suspend_ops thinking that it is more
related to suspend procedure and that every platform could take use of it
to do whatever the platform specific things are done in suspend.
Also, I had in mind that regulator framework doesn't care (as far as I
know) what every regulator feeds.
It looked to me more proper to have it per platform since in our case we
have the mapping b/w Linux power saving modes (suspend-to-ram, standby) and
SoC specific power saving modes done only in arch/arm/mach-at91/.

I also though that doing so, and having the platform_off_in_suspend()
wrapper, in drivers we could only include linux/suspend.h and make use of
this wrapper and in case we would have had it per regulator we should have
been taken care in drivers or regulator framework which regulator deals
with feeding the core.

Thank you,
Claudiu Beznea

> 									Pavel
>