[U-Boot] [PATCH 0/3] ARM: sunxi: Fix reset support on sun6i/sun8i

Hi,
This series fixes support for the reset command on sun6i/sun8i. The watchdog hardware has changed in sun6i, both the register addresses and definitions. This makes the reset command unusable on sun6i.
Patch 1 moves the watchdog register definitions into a separate file. The rationale behind this is that on sun6i, the watchdog is effectively a separate piece of hardware and not part of the timer.
Patch 2 fixes the register definitions for sun6i, most importantly the watchdog offsets.
Patch 3 adds the sun6i version of reset via watchdog timeout.
I intended to submit this series after the basic sun8i series. It just so happens that I forgot to leave my A23 tablet on at the office, hence I won't be able to test that series until next week.
Since this series has no dependencies nor conflicts, I decided to send this one first.
Cheers ChenYu
Chen-Yu Tsai (3): ARM: sunxi: Move watchdog register definitions to separate file ARM: sunxi: Add sun6i/sun8i timer block register definition ARM: sunxi: Fix reset command on sun6i/sun8i
arch/arm/cpu/armv7/sunxi/board.c | 15 ++++++++++ arch/arm/include/asm/arch-sunxi/timer.h | 23 ++++++---------- arch/arm/include/asm/arch-sunxi/watchdog.h | 44 ++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 14 deletions(-) create mode 100644 arch/arm/include/asm/arch-sunxi/watchdog.h

On later Allwinner SoCs, the watchdog hardware is by all means a separate hardware block, with its own address range and interrupt line.
Move the register definitions to a separate file to facilitate supporting newer SoCs.
Signed-off-by: Chen-Yu Tsai wens@csie.org --- arch/arm/include/asm/arch-sunxi/timer.h | 17 +++-------------- arch/arm/include/asm/arch-sunxi/watchdog.h | 24 ++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 14 deletions(-) create mode 100644 arch/arm/include/asm/arch-sunxi/watchdog.h
diff --git a/arch/arm/include/asm/arch-sunxi/timer.h b/arch/arm/include/asm/arch-sunxi/timer.h index 58e14fd..8ed383e 100644 --- a/arch/arm/include/asm/arch-sunxi/timer.h +++ b/arch/arm/include/asm/arch-sunxi/timer.h @@ -11,14 +11,10 @@ #ifndef _SUNXI_TIMER_H_ #define _SUNXI_TIMER_H_
-#define WDT_CTRL_RESTART (0x1 << 0) -#define WDT_CTRL_KEY (0x0a57 << 1) -#define WDT_MODE_EN (0x1 << 0) -#define WDT_MODE_RESET_EN (0x1 << 1) - #ifndef __ASSEMBLY__
#include <linux/types.h> +#include <asm/arch/watchdog.h>
/* General purpose timer */ struct sunxi_timer { @@ -43,12 +39,6 @@ struct sunxi_64cnt { u32 hi; /* 0xa8 */ };
-/* Watchdog */ -struct sunxi_wdog { - u32 ctl; /* 0x90 */ - u32 mode; /* 0x94 */ -}; - /* Rtc */ struct sunxi_rtc { u32 ctl; /* 0x100 */ @@ -77,9 +67,8 @@ struct sunxi_timer_reg { struct sunxi_timer timer[6]; /* We have 6 timers */ u8 res2[16]; struct sunxi_avs avs; - struct sunxi_wdog wdog; - u8 res3[8]; - struct sunxi_64cnt cnt64; + struct sunxi_wdog wdog; /* 0x90 */ + struct sunxi_64cnt cnt64; /* 0xa0 */ u8 res4[0x58]; struct sunxi_rtc rtc; struct sunxi_alarm alarm; diff --git a/arch/arm/include/asm/arch-sunxi/watchdog.h b/arch/arm/include/asm/arch-sunxi/watchdog.h new file mode 100644 index 0000000..5b755e3 --- /dev/null +++ b/arch/arm/include/asm/arch-sunxi/watchdog.h @@ -0,0 +1,24 @@ +/* + * (C) Copyright 2014 + * Chen-Yu Tsai wens@csie.org + * + * Watchdog register definitions + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#ifndef _SUNXI_WATCHDOG_H_ +#define _SUNXI_WATCHDOG_H_ + +#define WDT_CTRL_RESTART (0x1 << 0) +#define WDT_CTRL_KEY (0x0a57 << 1) +#define WDT_MODE_EN (0x1 << 0) +#define WDT_MODE_RESET_EN (0x1 << 1) + +struct sunxi_wdog { + u32 ctl; /* 0x00 */ + u32 mode; /* 0x04 */ + u32 res[2]; +}; + +#endif /* _SUNXI_WATCHDOG_H_ */

On Sat, 2014-10-04 at 20:37 +0800, Chen-Yu Tsai wrote:
On later Allwinner SoCs, the watchdog hardware is by all means a separate hardware block, with its own address range and interrupt line.
Move the register definitions to a separate file to facilitate supporting newer SoCs.
Signed-off-by: Chen-Yu Tsai wens@csie.org
Acked-by: Ian Campbell ijc@hellion.org.uk
(nb: this isn't pure code motion, please mention that in the commit log in the future).

The RTC hardware has been moved out of the timer block on sun6i/sun8i. In addition, there are more watchdogs available.
Also note that the timer block definition is not completely accurate for sun5i/sun7i. Various blocks are missing or have been moved out.
Signed-off-by: Chen-Yu Tsai wens@csie.org --- arch/arm/include/asm/arch-sunxi/timer.h | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/arch/arm/include/asm/arch-sunxi/timer.h b/arch/arm/include/asm/arch-sunxi/timer.h index 8ed383e..03a0684 100644 --- a/arch/arm/include/asm/arch-sunxi/timer.h +++ b/arch/arm/include/asm/arch-sunxi/timer.h @@ -67,7 +67,9 @@ struct sunxi_timer_reg { struct sunxi_timer timer[6]; /* We have 6 timers */ u8 res2[16]; struct sunxi_avs avs; +#if defined(CONFIG_SUN4I) || defined(CONFIG_SUN5I) || defined(CONFIG_SUN7I) struct sunxi_wdog wdog; /* 0x90 */ + /* XXX the following is not accurate for sun5i/sun7i */ struct sunxi_64cnt cnt64; /* 0xa0 */ u8 res4[0x58]; struct sunxi_rtc rtc; @@ -75,6 +77,10 @@ struct sunxi_timer_reg { struct sunxi_tgp tgp[4]; u8 res5[8]; u32 cpu_cfg; +#else /* CONFIG_SUN6I || CONFIG_SUN8I || ... */ + u8 res3[16]; + struct sunxi_wdog wdog[5]; /* We have 5 watchdogs */ +#endif };
#endif /* __ASSEMBLY__ */

On Sat, 2014-10-04 at 20:37 +0800, Chen-Yu Tsai wrote:
The RTC hardware has been moved out of the timer block on sun6i/sun8i. In addition, there are more watchdogs available.
Also note that the timer block definition is not completely accurate for sun5i/sun7i. Various blocks are missing or have been moved out.
Signed-off-by: Chen-Yu Tsai wens@csie.org
Acked-by: Ian Campbell ijc@hellion.org.uk
arch/arm/include/asm/arch-sunxi/timer.h | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/arch/arm/include/asm/arch-sunxi/timer.h b/arch/arm/include/asm/arch-sunxi/timer.h index 8ed383e..03a0684 100644 --- a/arch/arm/include/asm/arch-sunxi/timer.h +++ b/arch/arm/include/asm/arch-sunxi/timer.h @@ -67,7 +67,9 @@ struct sunxi_timer_reg { struct sunxi_timer timer[6]; /* We have 6 timers */ u8 res2[16]; struct sunxi_avs avs; +#if defined(CONFIG_SUN4I) || defined(CONFIG_SUN5I) || defined(CONFIG_SUN7I) struct sunxi_wdog wdog; /* 0x90 */
- /* XXX the following is not accurate for sun5i/sun7i */ struct sunxi_64cnt cnt64; /* 0xa0 */ u8 res4[0x58]; struct sunxi_rtc rtc;
@@ -75,6 +77,10 @@ struct sunxi_timer_reg { struct sunxi_tgp tgp[4]; u8 res5[8]; u32 cpu_cfg; +#else /* CONFIG_SUN6I || CONFIG_SUN8I || ... */
- u8 res3[16];
- struct sunxi_wdog wdog[5]; /* We have 5 watchdogs */
+#endif };
#endif /* __ASSEMBLY__ */

The watchdog on sun6i/sun8i has a different layout.
Add the new layout and fix up the setup functions so that reset works.
Signed-off-by: Chen-Yu Tsai wens@csie.org --- arch/arm/cpu/armv7/sunxi/board.c | 15 +++++++++++++++ arch/arm/include/asm/arch-sunxi/watchdog.h | 20 ++++++++++++++++++++ 2 files changed, 35 insertions(+)
diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c index b6d63db..ecf3ff9 100644 --- a/arch/arm/cpu/armv7/sunxi/board.c +++ b/arch/arm/cpu/armv7/sunxi/board.c @@ -75,6 +75,7 @@ int gpio_init(void)
void reset_cpu(ulong addr) { +#if defined(CONFIG_SUN4I) || defined(CONFIG_SUN5I) || defined(CONFIG_SUN7I) static const struct sunxi_wdog *wdog = &((struct sunxi_timer_reg *)SUNXI_TIMER_BASE)->wdog;
@@ -86,6 +87,20 @@ void reset_cpu(ulong addr) /* sun5i sometimes gets stuck without this */ writel(WDT_MODE_RESET_EN | WDT_MODE_EN, &wdog->mode); } +#else /* CONFIG_SUN6I || CONFIG_SUN8I || .. */ + static const struct sunxi_wdog *wdog = + ((struct sunxi_timer_reg *)SUNXI_TIMER_BASE)->wdog; + + /* Set the watchdog for its shortest interval (.5s) and wait */ + writel(WDT_CFG_RESET, &wdog->cfg); + writel(WDT_MODE_EN, &wdog->mode); + writel(WDT_CTRL_KEY | WDT_CTRL_RESTART, &wdog->ctl); + + while (1) { + /* sun5i sometimes gets stuck without this */ + writel(WDT_MODE_EN, &wdog->mode); + } +#endif }
/* do some early init */ diff --git a/arch/arm/include/asm/arch-sunxi/watchdog.h b/arch/arm/include/asm/arch-sunxi/watchdog.h index 5b755e3..ccc8fa3 100644 --- a/arch/arm/include/asm/arch-sunxi/watchdog.h +++ b/arch/arm/include/asm/arch-sunxi/watchdog.h @@ -12,6 +12,9 @@
#define WDT_CTRL_RESTART (0x1 << 0) #define WDT_CTRL_KEY (0x0a57 << 1) + +#if defined(CONFIG_SUN4I) || defined(CONFIG_SUN5I) || defined(CONFIG_SUN7I) + #define WDT_MODE_EN (0x1 << 0) #define WDT_MODE_RESET_EN (0x1 << 1)
@@ -21,4 +24,21 @@ struct sunxi_wdog { u32 res[2]; };
+#else + +#define WDT_CFG_RESET (0x1) +#define WDT_MODE_EN (0x1) + +struct sunxi_wdog { + u32 irq_en; /* 0x00 */ + u32 irq_sta; /* 0x04 */ + u32 res1[2]; + u32 ctl; /* 0x10 */ + u32 cfg; /* 0x14 */ + u32 mode; /* 0x18 */ + u32 res2; +}; + +#endif + #endif /* _SUNXI_WATCHDOG_H_ */

On Sat, 2014-10-04 at 20:37 +0800, Chen-Yu Tsai wrote:
+#else /* CONFIG_SUN6I || CONFIG_SUN8I || .. */
- static const struct sunxi_wdog *wdog =
((struct sunxi_timer_reg *)SUNXI_TIMER_BASE)->wdog;
- /* Set the watchdog for its shortest interval (.5s) and wait */
- writel(WDT_CFG_RESET, &wdog->cfg);
- writel(WDT_MODE_EN, &wdog->mode);
- writel(WDT_CTRL_KEY | WDT_CTRL_RESTART, &wdog->ctl);
That's annoyingly close to the 4/5/7i version, but not quite close enough to allow any meaningful sharing :-/
- while (1) {
/* sun5i sometimes gets stuck without this */
Is this not therefore unnecessary on 6i/8i?
writel(WDT_MODE_EN, &wdog->mode);
- }
+#endif }
/* do some early init */ diff --git a/arch/arm/include/asm/arch-sunxi/watchdog.h b/arch/arm/include/asm/arch-sunxi/watchdog.h index 5b755e3..ccc8fa3 100644 --- a/arch/arm/include/asm/arch-sunxi/watchdog.h +++ b/arch/arm/include/asm/arch-sunxi/watchdog.h @@ -12,6 +12,9 @@
#define WDT_CTRL_RESTART (0x1 << 0) #define WDT_CTRL_KEY (0x0a57 << 1)
+#if defined(CONFIG_SUN4I) || defined(CONFIG_SUN5I) || defined(CONFIG_SUN7I)
#define WDT_MODE_EN (0x1 << 0) #define WDT_MODE_RESET_EN (0x1 << 1)
@@ -21,4 +24,21 @@ struct sunxi_wdog { u32 res[2]; };
+#else
+#define WDT_CFG_RESET (0x1) +#define WDT_MODE_EN (0x1)
+struct sunxi_wdog {
- u32 irq_en; /* 0x00 */
- u32 irq_sta; /* 0x04 */
- u32 res1[2];
- u32 ctl; /* 0x10 */
- u32 cfg; /* 0x14 */
- u32 mode; /* 0x18 */
- u32 res2;
+};
+#endif
#endif /* _SUNXI_WATCHDOG_H_ */

On Sat, Oct 11, 2014 at 11:33 PM, Ian Campbell ijc@hellion.org.uk wrote:
On Sat, 2014-10-04 at 20:37 +0800, Chen-Yu Tsai wrote:
+#else /* CONFIG_SUN6I || CONFIG_SUN8I || .. */
static const struct sunxi_wdog *wdog =
((struct sunxi_timer_reg *)SUNXI_TIMER_BASE)->wdog;
/* Set the watchdog for its shortest interval (.5s) and wait */
writel(WDT_CFG_RESET, &wdog->cfg);
writel(WDT_MODE_EN, &wdog->mode);
writel(WDT_CTRL_KEY | WDT_CTRL_RESTART, &wdog->ctl);
That's annoyingly close to the 4/5/7i version, but not quite close enough to allow any meaningful sharing :-/
I suppose it is possible to share using clrsetbits...
while (1) {
/* sun5i sometimes gets stuck without this */
Is this not therefore unnecessary on 6i/8i?
The kernel restart driver uses this. But Allwinner's original code does not.
ChenYu
writel(WDT_MODE_EN, &wdog->mode);
}
+#endif }
/* do some early init */ diff --git a/arch/arm/include/asm/arch-sunxi/watchdog.h b/arch/arm/include/asm/arch-sunxi/watchdog.h index 5b755e3..ccc8fa3 100644 --- a/arch/arm/include/asm/arch-sunxi/watchdog.h +++ b/arch/arm/include/asm/arch-sunxi/watchdog.h @@ -12,6 +12,9 @@
#define WDT_CTRL_RESTART (0x1 << 0) #define WDT_CTRL_KEY (0x0a57 << 1)
+#if defined(CONFIG_SUN4I) || defined(CONFIG_SUN5I) || defined(CONFIG_SUN7I)
#define WDT_MODE_EN (0x1 << 0) #define WDT_MODE_RESET_EN (0x1 << 1)
@@ -21,4 +24,21 @@ struct sunxi_wdog { u32 res[2]; };
+#else
+#define WDT_CFG_RESET (0x1) +#define WDT_MODE_EN (0x1)
+struct sunxi_wdog {
u32 irq_en; /* 0x00 */
u32 irq_sta; /* 0x04 */
u32 res1[2];
u32 ctl; /* 0x10 */
u32 cfg; /* 0x14 */
u32 mode; /* 0x18 */
u32 res2;
+};
+#endif
#endif /* _SUNXI_WATCHDOG_H_ */

On Sun, 2014-10-12 at 10:40 +0800, Chen-Yu Tsai wrote:
On Sat, Oct 11, 2014 at 11:33 PM, Ian Campbell ijc@hellion.org.uk wrote:
On Sat, 2014-10-04 at 20:37 +0800, Chen-Yu Tsai wrote:
+#else /* CONFIG_SUN6I || CONFIG_SUN8I || .. */
static const struct sunxi_wdog *wdog =
((struct sunxi_timer_reg *)SUNXI_TIMER_BASE)->wdog;
/* Set the watchdog for its shortest interval (.5s) and wait */
writel(WDT_CFG_RESET, &wdog->cfg);
writel(WDT_MODE_EN, &wdog->mode);
writel(WDT_CTRL_KEY | WDT_CTRL_RESTART, &wdog->ctl);
That's annoyingly close to the 4/5/7i version, but not quite close enough to allow any meaningful sharing :-/
I suppose it is possible to share using clrsetbits...
I think it probably isn't worth it.
while (1) {
/* sun5i sometimes gets stuck without this */
Is this not therefore unnecessary on 6i/8i?
The kernel restart driver uses this. But Allwinner's original code does not.
Unless it is needed in practice lets just leave it out.
Ian.

Hi,
On Sat, Oct 4, 2014 at 10:37 PM, Chen-Yu Tsai wens@csie.org wrote:
Hi,
This series fixes support for the reset command on sun6i/sun8i. The watchdog hardware has changed in sun6i, both the register addresses and definitions. This makes the reset command unusable on sun6i.
Doesn't this break building multi-sub-arch kernels that support both sun4i/sun5i/sun7i and sun6i/sun8i?
Would it be possible to just add a new driver for the modified watchdog or put all the bits that changed into the device tree?
Thanks,

Hi,
On Sat, Oct 4, 2014 at 11:02 PM, Julian Calaby julian.calaby@gmail.com wrote:
Hi,
On Sat, Oct 4, 2014 at 10:37 PM, Chen-Yu Tsai wens@csie.org wrote:
Hi,
This series fixes support for the reset command on sun6i/sun8i. The watchdog hardware has changed in sun6i, both the register addresses and definitions. This makes the reset command unusable on sun6i.
Doesn't this break building multi-sub-arch kernels that support both sun4i/sun5i/sun7i and sun6i/sun8i?
Last time I checked, U-boot doesn't do multi-arch kernels.
Would it be possible to just add a new driver for the modified watchdog or put all the bits that changed into the device tree?
That is pretty much what the "kernel" version of this does. See: http://www.spinics.net/lists/arm-kernel/msg362853.html
Cheers ChenYu

Hi,
On Sun, Oct 5, 2014 at 1:12 AM, Chen-Yu Tsai wens@csie.org wrote:
Hi,
On Sat, Oct 4, 2014 at 11:02 PM, Julian Calaby julian.calaby@gmail.com wrote:
Hi,
On Sat, Oct 4, 2014 at 10:37 PM, Chen-Yu Tsai wens@csie.org wrote:
Hi,
This series fixes support for the reset command on sun6i/sun8i. The watchdog hardware has changed in sun6i, both the register addresses and definitions. This makes the reset command unusable on sun6i.
Doesn't this break building multi-sub-arch kernels that support both sun4i/sun5i/sun7i and sun6i/sun8i?
Last time I checked, U-boot doesn't do multi-arch kernels.
Looking at the paths, I could have sworn this was kernel code.
In that case, sorry for the noise.
Thanks,
participants (3)
-
Chen-Yu Tsai
-
Ian Campbell
-
Julian Calaby