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

From: [email-masked] (Sergei Miroshnichenko)
Date: Mon, 19 Oct 2015 08:53:05 +0300

Hi Stefan,

Thanks for the review!

Our goal is dynamic run-time configuration of wakeup sources in
userspace using standard sysfs API:

echo enabled/disabled > /sys/class/<deviceX>/power/wakeup

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.

What do you think about this?

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 <[email-masked]>
>> ---
>>  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;
>