clk: vf610: don't stop enabled clocks in suspend

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 protected]>

---
 drivers/clk/imx/clk-gate2.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

drivers/clk/imx/clk-gate2.c +3 -2

index 8935bff..ac35d75 100644
@@ -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;

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

Quoted 40 lines · Sergei Miroshnichenko

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 protected]>

---
 drivers/clk/imx/clk-gate2.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

drivers/clk/imx/clk-gate2.c +3 -2

index 8935bff..ac35d75 100644
@@ -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;

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

Quoted 67 lines · Stefan Agner

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

Quoted 40 lines · Sergei Miroshnichenko

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 protected]>

---
 drivers/clk/imx/clk-gate2.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

drivers/clk/imx/clk-gate2.c +3 -2

index 8935bff..ac35d75 100644
@@ -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;

Hi Sergei,

Quoted 4 lines · Sergei Miroshnichenko

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.

Quoted 15 lines

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().

Quoted 4 lines

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.

Quoted 2 lines

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

Quoted 72 lines

Best regards,
Sergei

Quoted 67 lines · Stefan Agner

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

Quoted 40 lines · Sergei Miroshnichenko

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 protected]>

---
 drivers/clk/imx/clk-gate2.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

drivers/clk/imx/clk-gate2.c +3 -2

index 8935bff..ac35d75 100644
@@ -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;