[PATCH] MIPS: JZ4740: reset: Initialize hibernate wakeup counters.

[PATCH] MIPS: JZ4740: reset: Initialize hibernate wakeup counters.

[Maarten ter Huurne]

In hibernation mode only the wakeup logic and the RTC are left running,
so this is what users perceive as power down.

If the counters are not initialized, the corresponding pin (typically
connected to the power button) has to be asserted for two seconds
before the device wakes up. Most users expect a shorter wakeup time.

I took the timing values of 100 ms and 60 ms from BouKiCHi's patch for
the Dingoo A320 kernel.

Signed-off-by: Maarten ter Huurne <maarten@treewalker.org>
Acked-by: Lars-Peter Clausen <lars@metafoo.de>
---
 arch/mips/jz4740/reset.c |   46 ++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/arch/mips/jz4740/reset.c b/arch/mips/jz4740/reset.c
index 5f1fb95..e6d1d7b 100644
--- a/arch/mips/jz4740/reset.c
+++ b/arch/mips/jz4740/reset.c
@@ -21,6 +21,9 @@
 #include <asm/mach-jz4740/base.h>
 #include <asm/mach-jz4740/timer.h>
 
+#include "reset.h"
+#include "clock.h"
+
 static void jz4740_halt(void)
 {
 	while (1) {
@@ -53,21 +56,52 @@ static void jz4740_restart(char *command)
 	jz4740_halt();
 }
 
-#define JZ_REG_RTC_CTRL		0x00
-#define JZ_REG_RTC_HIBERNATE	0x20
+#define JZ_REG_RTC_CTRL			0x00
+#define JZ_REG_RTC_HIBERNATE		0x20
+#define JZ_REG_RTC_WAKEUP_FILTER	0x24
+#define JZ_REG_RTC_RESET_COUNTER	0x28
 
-#define JZ_RTC_CTRL_WRDY	BIT(7)
+#define JZ_RTC_CTRL_WRDY		BIT(7)
+#define JZ_RTC_WAKEUP_FILTER_MASK	0x0000FFE0
+#define JZ_RTC_RESET_COUNTER_MASK	0x00000FE0
 
-static void jz4740_power_off(void)
+static inline void jz4740_rtc_wait_ready(void __iomem *rtc_base)
 {
-	void __iomem *rtc_base = ioremap(JZ4740_RTC_BASE_ADDR, 0x24);
 	uint32_t ctrl;
-
 	do {
 		ctrl = readl(rtc_base + JZ_REG_RTC_CTRL);
 	} while (!(ctrl & JZ_RTC_CTRL_WRDY));
+}
+
+static void jz4740_power_off(void)
+{
+	void __iomem *rtc_base = ioremap(JZ4740_RTC_BASE_ADDR, 0x38);
+	unsigned long long wakeup_filter_ticks;
+	unsigned long long reset_counter_ticks;
+
+	/* Set minimum wakeup pin assertion time: 100 ms.
+	   Range is 0 to 2 sec if RTC is clocked at 32 kHz. */
+	wakeup_filter_ticks = (100 * jz4740_clock_bdata.rtc_rate) / 1000;
+	if (wakeup_filter_ticks < JZ_RTC_WAKEUP_FILTER_MASK)
+		wakeup_filter_ticks &= JZ_RTC_WAKEUP_FILTER_MASK;
+	else
+		wakeup_filter_ticks = JZ_RTC_WAKEUP_FILTER_MASK;
+	jz4740_rtc_wait_ready(rtc_base);
+	writel(wakeup_filter_ticks, rtc_base + JZ_REG_RTC_WAKEUP_FILTER);
 
+	/* Set reset pin low-level assertion time after wakeup: 60 ms.
+	   Range is 0 to 125 ms if RTC is clocked at 32 kHz. */
+	reset_counter_ticks = (60 * jz4740_clock_bdata.rtc_rate) / 1000;
+	if (reset_counter_ticks < JZ_RTC_RESET_COUNTER_MASK)
+		reset_counter_ticks &= JZ_RTC_RESET_COUNTER_MASK;
+	else
+		reset_counter_ticks = JZ_RTC_RESET_COUNTER_MASK;
+	jz4740_rtc_wait_ready(rtc_base);
+	writel(reset_counter_ticks, rtc_base + JZ_REG_RTC_RESET_COUNTER);
+
+	jz4740_rtc_wait_ready(rtc_base);
 	writel(1, rtc_base + JZ_REG_RTC_HIBERNATE);
+
 	jz4740_halt();
 }
 
-- 
1.7.7

Re: [PATCH] MIPS: JZ4740: reset: Initialize hibernate wakeup counters.

[Sergei Shtylyov]

Hello.

On 29-03-2012 20:19, Maarten ter Huurne wrote:

> In hibernation mode only the wakeup logic and the RTC are left running,
> so this is what users perceive as power down.

> If the counters are not initialized, the corresponding pin (typically
> connected to the power button) has to be asserted for two seconds
> before the device wakes up. Most users expect a shorter wakeup time.

> I took the timing values of 100 ms and 60 ms from BouKiCHi's patch for
> the Dingoo A320 kernel.

> Signed-off-by: Maarten ter Huurne<maarten@treewalker.org>
> Acked-by: Lars-Peter Clausen<lars@metafoo.de>
> ---
>   arch/mips/jz4740/reset.c |   46 ++++++++++++++++++++++++++++++++++++++++------
>   1 files changed, 40 insertions(+), 6 deletions(-)

> diff --git a/arch/mips/jz4740/reset.c b/arch/mips/jz4740/reset.c
> index 5f1fb95..e6d1d7b 100644
> --- a/arch/mips/jz4740/reset.c
> +++ b/arch/mips/jz4740/reset.c
[...]
> @@ -53,21 +56,52 @@ static void jz4740_restart(char *command)
[...]
> -static void jz4740_power_off(void)
> +static inline void jz4740_rtc_wait_ready(void __iomem *rtc_base)
>   {
> -	void __iomem *rtc_base = ioremap(JZ4740_RTC_BASE_ADDR, 0x24);
>   	uint32_t ctrl;
> -

    Do not remove the empty line here.

>   	do {
>   		ctrl = readl(rtc_base + JZ_REG_RTC_CTRL);
>   	} while (!(ctrl&  JZ_RTC_CTRL_WRDY));
> +}
> +
> +static void jz4740_power_off(void)
> +{
> +	void __iomem *rtc_base = ioremap(JZ4740_RTC_BASE_ADDR, 0x38);
> +	unsigned long long wakeup_filter_ticks;
> +	unsigned long long reset_counter_ticks;
> +
> +	/* Set minimum wakeup pin assertion time: 100 ms.
> +	   Range is 0 to 2 sec if RTC is clocked at 32 kHz. */
> +	wakeup_filter_ticks = (100 * jz4740_clock_bdata.rtc_rate) / 1000;
> +	if (wakeup_filter_ticks<  JZ_RTC_WAKEUP_FILTER_MASK)
> +		wakeup_filter_ticks&= JZ_RTC_WAKEUP_FILTER_MASK;
> +	else
> +		wakeup_filter_ticks = JZ_RTC_WAKEUP_FILTER_MASK;
> +	jz4740_rtc_wait_ready(rtc_base);
> +	writel(wakeup_filter_ticks, rtc_base + JZ_REG_RTC_WAKEUP_FILTER);

    Writing 64-bit variable to a 32-bit register?

> +	/* Set reset pin low-level assertion time after wakeup: 60 ms.
> +	   Range is 0 to 125 ms if RTC is clocked at 32 kHz. */

    The preferred style for multi-line comments is this:

/*
  * bla
  * bla
  */

> +	reset_counter_ticks = (60 * jz4740_clock_bdata.rtc_rate) / 1000;
> +	if (reset_counter_ticks<  JZ_RTC_RESET_COUNTER_MASK)
> +		reset_counter_ticks&= JZ_RTC_RESET_COUNTER_MASK;
> +	else
> +		reset_counter_ticks = JZ_RTC_RESET_COUNTER_MASK;
> +	jz4740_rtc_wait_ready(rtc_base);
> +	writel(reset_counter_ticks, rtc_base + JZ_REG_RTC_RESET_COUNTER);

    Writing 64-bit variable to a 32-bit register?

WBR, Sergei

Re: [PATCH] MIPS: JZ4740: reset: Initialize hibernate wakeup counters.

[Maarten ter Huurne]

On Friday 30 March 2012 16:23:31 Sergei Shtylyov wrote:

[...]

> > +static void jz4740_power_off(void)
> > +{
> > +	void __iomem *rtc_base = ioremap(JZ4740_RTC_BASE_ADDR, 0x38);
> > +	unsigned long long wakeup_filter_ticks;
> > +	unsigned long long reset_counter_ticks;
> > +
> > +	/* Set minimum wakeup pin assertion time: 100 ms.
> > +	   Range is 0 to 2 sec if RTC is clocked at 32 kHz. */
> > +	wakeup_filter_ticks = (100 * jz4740_clock_bdata.rtc_rate) / 1000;
> > +	if (wakeup_filter_ticks<  JZ_RTC_WAKEUP_FILTER_MASK)
> > +		wakeup_filter_ticks&= JZ_RTC_WAKEUP_FILTER_MASK;
> > +	else
> > +		wakeup_filter_ticks = JZ_RTC_WAKEUP_FILTER_MASK;
> > +	jz4740_rtc_wait_ready(rtc_base);
> > +	writel(wakeup_filter_ticks, rtc_base + JZ_REG_RTC_WAKEUP_FILTER);
> 
>     Writing 64-bit variable to a 32-bit register?

Actually the variable can be 32-bit: rtc_rate is in kHz, so the computation 
would wrap around for an RTC clock of over 40 GHz and I'm sure that is far 
more than the hardware supports.

I'll prepare a new patch fixing this and the other issues you mentioned. 
Thanks for reviewing.

Bye,
		Maarten

[PATCH] MIPS: JZ4740: reset: Initialize hibernate wakeup counters.

[Maarten ter Huurne]

In hibernation mode only the wakeup logic and the RTC are left running,
so this is what users perceive as power down.

If the counters are not initialized, the corresponding pin (typically
connected to the power button) has to be asserted for two seconds
before the device wakes up. Most users expect a shorter wakeup time.

I took the timing values of 100 ms and 60 ms from BouKiCHi's patch for
the Dingoo A320 kernel.

Signed-off-by: Maarten ter Huurne <maarten@treewalker.org>
---
 arch/mips/jz4740/reset.c |   49 +++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/arch/mips/jz4740/reset.c b/arch/mips/jz4740/reset.c
index 5f1fb95..6c0da5a 100644
--- a/arch/mips/jz4740/reset.c
+++ b/arch/mips/jz4740/reset.c
@@ -21,6 +21,9 @@
 #include <asm/mach-jz4740/base.h>
 #include <asm/mach-jz4740/timer.h>
 
+#include "reset.h"
+#include "clock.h"
+
 static void jz4740_halt(void)
 {
 	while (1) {
@@ -53,21 +56,57 @@ static void jz4740_restart(char *command)
 	jz4740_halt();
 }
 
-#define JZ_REG_RTC_CTRL		0x00
-#define JZ_REG_RTC_HIBERNATE	0x20
+#define JZ_REG_RTC_CTRL			0x00
+#define JZ_REG_RTC_HIBERNATE		0x20
+#define JZ_REG_RTC_WAKEUP_FILTER	0x24
+#define JZ_REG_RTC_RESET_COUNTER	0x28
 
-#define JZ_RTC_CTRL_WRDY	BIT(7)
+#define JZ_RTC_CTRL_WRDY		BIT(7)
+#define JZ_RTC_WAKEUP_FILTER_MASK	0x0000FFE0
+#define JZ_RTC_RESET_COUNTER_MASK	0x00000FE0
 
-static void jz4740_power_off(void)
+static inline void jz4740_rtc_wait_ready(void __iomem *rtc_base)
 {
-	void __iomem *rtc_base = ioremap(JZ4740_RTC_BASE_ADDR, 0x24);
 	uint32_t ctrl;
 
 	do {
 		ctrl = readl(rtc_base + JZ_REG_RTC_CTRL);
 	} while (!(ctrl & JZ_RTC_CTRL_WRDY));
+}
 
+static void jz4740_power_off(void)
+{
+	void __iomem *rtc_base = ioremap(JZ4740_RTC_BASE_ADDR, 0x38);
+	unsigned long wakeup_filter_ticks;
+	unsigned long reset_counter_ticks;
+
+	/*
+	 * Set minimum wakeup pin assertion time: 100 ms.
+	 * Range is 0 to 2 sec if RTC is clocked at 32 kHz.
+	 */
+	wakeup_filter_ticks = (100 * jz4740_clock_bdata.rtc_rate) / 1000;
+	if (wakeup_filter_ticks < JZ_RTC_WAKEUP_FILTER_MASK)
+		wakeup_filter_ticks &= JZ_RTC_WAKEUP_FILTER_MASK;
+	else
+		wakeup_filter_ticks = JZ_RTC_WAKEUP_FILTER_MASK;
+	jz4740_rtc_wait_ready(rtc_base);
+	writel(wakeup_filter_ticks, rtc_base + JZ_REG_RTC_WAKEUP_FILTER);
+
+	/*
+	 * Set reset pin low-level assertion time after wakeup: 60 ms.
+	 * Range is 0 to 125 ms if RTC is clocked at 32 kHz.
+	 */
+	reset_counter_ticks = (60 * jz4740_clock_bdata.rtc_rate) / 1000;
+	if (reset_counter_ticks < JZ_RTC_RESET_COUNTER_MASK)
+		reset_counter_ticks &= JZ_RTC_RESET_COUNTER_MASK;
+	else
+		reset_counter_ticks = JZ_RTC_RESET_COUNTER_MASK;
+	jz4740_rtc_wait_ready(rtc_base);
+	writel(reset_counter_ticks, rtc_base + JZ_REG_RTC_RESET_COUNTER);
+
+	jz4740_rtc_wait_ready(rtc_base);
 	writel(1, rtc_base + JZ_REG_RTC_HIBERNATE);
+
 	jz4740_halt();
 }
 
-- 
1.7.7

Re: [PATCH] MIPS: JZ4740: reset: Initialize hibernate wakeup counters.

[Maarten ter Huurne]

On Friday 30 March 2012 16:07:41, I wrote:

> Actually the variable can be 32-bit: rtc_rate is in kHz, so the
> computation would wrap around for an RTC clock of over 40 GHz and I'm
> sure that is far more than the hardware supports.

Correction: rtc_rate is in Hz, therefore the computation would wrap around 
at 40 MHz, but that's likely still far more than the hardware supports. The 
typical RTC clock value is 32 kHz.

Bye,
		Maarten