[PATCH] clk: vf610: don't stop enabled clocks in suspend

[Sergei Miroshnichenko]

Hi Stefan,

Ok, in a couple of days I'll prepare a patchset with:
 - device_may_wakeup()-template for drivers we are working with;
 - declaration of the rest of Vybrid clocks;
 - patch from this thread;
 - initial platform code to support "freeze" low-power state for Vybrid.

Best regards,
Sergei

On 10/24/2015 02:53 AM, Stefan Agner wrote:
> Hi Sergei,
> 
> On 2015-10-18 22:53, Sergei Miroshnichenko wrote:
>> Our goal is dynamic run-time configuration of wakeup sources in
>> userspace using standard sysfs API:
>>
>> echo enabled/disabled > /sys/class/<deviceX>/power/wakeup
> 
> Yes, that is exactly what I am talking about. In the tree I linked below
> it is shown how I implemented this API for UART on Vybrid.
> 
>>
>> To implement wakeup support in drivers, the following template can be
>> used in suspend()/resume() handlers:
>>
>> if (device_may_wakeup(...)) {
>> 	enable_irq_wake(...);/disable_irq_wake(...);
>> } else {
>> 	clk_disable_unprepare(...);/clk_prepare_enable(...);
>> }
>>
>> Next step will be declaring the rest of Vybrid clocks in
>> drivers/clk/imx/clk-vf610.c (about 15 more). Those of them that aren't
>> used by any driver, will be automatically turned off by kernel at
>> startup (after all drivers are initialized) via clk_disable_unused().
>>
>> With this approach, we utilize standard mechanism of waking up used all
>> over the kernel, and make it configurable even from shell scripts. We've
>> tested it with uart, gpio, rtc, spi, i2c, lan and mmc (SDIO): these
>> changes are also planned to be sent upstream.
> 
> That is exactly the point: As long as we don't have the template above
> for all drivers upstream, we should still let the hardware take care.
> 
>>
>> What do you think about this?
> 
> My suggestion is to use a "per clock" configuration in clk-vf610.c, as I
> did it with UART with those two changes. This way we can make a slow
> transition:
> http://git.toradex.com/cgit/linux-toradex.git/commit/?h=toradex_vf_4.1-next&id=66e003c1ad2acdaa7d42e1113add7d44479392ab
> http://git.toradex.com/cgit/linux-toradex.git/commit/?h=toradex_vf_4.1-next&id=fc60861b91c6585740e9a502fa4008b2f4694b22
> 
> Or we make sure all drivers really disable clocks correctly on suspend,
> and then introduce your change...
> 
> --
> Stefan
> 
>>
>> Best regards,
>> Sergei
>>
>> On 10/16/2015 10:46 PM, Stefan Agner wrote:
>>> Hi Sergei,
>>>
>>> If all peripherals which do not act as wakeup source disable clocks
>>> explicily, this would be the right approach. However, currently not all
>>> of them do, hence this patch will raise the energy consumption during
>>> sleep modes. The peripherals of course rely on the clock driver to
>>> restore the state (gates) at wake-up time, but I think that is how it
>>> works for most SoCs anyway.
>>>
>>> In the Toradex branch I came up with a solution which allows to
>>> selectively define which clock should be disabled and which should be
>>> left on by the hardware:
>>> http://git.toradex.com/cgit/linux-toradex.git/commit/?h=toradex_vf_4.1-next&id=66e003c1ad2acdaa7d42e1113add7d44479392ab
>>>
>>> I then started to fix some peripherals so that they explicitly disable
>>> clock if wakeup is not enabled. I changed those clocks so they do not
>>> get automatically disabled:
>>> http://git.toradex.com/cgit/linux-toradex.git/commit/?h=toradex_vf_4.1-next&id=fc60861b91c6585740e9a502fa4008b2f4694b22
>>>
>>> I plan to upstream those changes but did not came around to put
>>> toghether a rebased and cleaned up patchset so far.
>>>
>>> --
>>> Stefan
>>>
>>> On 2015-10-16 07:51, Sergei Miroshnichenko wrote:
>>>> Currently Vybrid Clock Gating is configured to turn off all clocks in stop
>>>> mode, while kernel assumes them enabled unless explicitly disabled. This
>>>> behavior prevents waking up from wakeup sources.
>>>>
>>>> Fix this by replacing Clock Gating Register bitmask 3 (Clock is on during
>>>> all modes, except stop mode) with bitmask 2 (Clock is on during all modes,
>>>> even stop mode): refer to Vybrid Reference Manual Chapter 10 "Clock
>>>> Controller Module (CCM)".
>>>>
>>>> Tested on a VF610-based board.
>>>>
>>>> Signed-off-by: Sergei Miroshnichenko <sergeimir at emcraft.com>
>>>> ---
>>>>  drivers/clk/imx/clk-gate2.c | 5 +++--
>>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/clk/imx/clk-gate2.c b/drivers/clk/imx/clk-gate2.c
>>>> index 8935bff..ac35d75 100644
>>>> --- a/drivers/clk/imx/clk-gate2.c
>>>> +++ b/drivers/clk/imx/clk-gate2.c
>>>> @@ -50,7 +50,8 @@ static int clk_gate2_enable(struct clk_hw *hw)
>>>>  		goto out;
>>>>
>>>>  	reg = readl(gate->reg);
>>>> -	reg |= 3 << gate->bit_idx;
>>>> +	reg &= ~(3 << gate->bit_idx);
>>>> +	reg |= 2 << gate->bit_idx;
>>>>  	writel(reg, gate->reg);
>>>>
>>>>  out:
>>>> @@ -86,7 +87,7 @@ static int clk_gate2_reg_is_enabled(void __iomem
>>>> *reg, u8 bit_idx)
>>>>  {
>>>>  	u32 val = readl(reg);
>>>>
>>>> -	if (((val >> bit_idx) & 1) == 1)
>>>> +	if (((val >> bit_idx) & 3) == 2)
>>>>  		return 1;
>>>>
>>>>  	return 0;
>>>