[U-Boot] [PATCH V3] imx25: Fix reset

[Matthias Weisser]

This patch fixes the reset command on imx25. The watchdog registers are 16
bits in size and not 32. This patch also adds the service register codes as
constants.

Signed-off-by: Matthias Weisser <weisserm at arcor.de>
---

Changes since V2
- Using 16 bit constants

Changes since V1
- Corrected whitespace
- Leaving reset sequence as it originally was

arch/arm/cpu/arm926ejs/mx25/reset.c | 8 ++++----
arch/arm/include/asm/arch-mx25/imx-regs.h | 14 ++++++++------
2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/arch/arm/cpu/arm926ejs/mx25/reset.c b/arch/arm/cpu/arm926ejs/mx25/reset.c
index 1e33150..1a43683 100644
--- a/arch/arm/cpu/arm926ejs/mx25/reset.c
+++ b/arch/arm/cpu/arm926ejs/mx25/reset.c
@@ -43,14 +43,14 @@ void reset_cpu (ulong ignored)
{
struct wdog_regs *regs = (struct wdog_regs *)IMX_WDT_BASE;
/* Disable watchdog and set Time-Out field to 0 */
- writel (0x00000000, &regs->wcr);
+ writew(0, &regs->wcr);

/* Write Service Sequence */
- writel (0x00005555, &regs->wsr);
- writel (0x0000AAAA, &regs->wsr);
+ writew(WSR_UNLOCK1, &regs->wsr);
+ writew(WSR_UNLOCK2, &regs->wsr);

/* Enable watchdog */
- writel (WCR_WDE, &regs->wcr);
+ writew(WCR_WDE, &regs->wcr);

while (1) ;
}
diff --git a/arch/arm/include/asm/arch-mx25/imx-regs.h b/arch/arm/include/asm/arch-mx25/imx-regs.h
index f709bd8..f5a2929 100644
--- a/arch/arm/include/asm/arch-mx25/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx25/imx-regs.h
@@ -108,11 +108,11 @@ struct gpt_regs {

/* Watchdog Timer (WDOG) registers */
struct wdog_regs {
- u32 wcr; /* Control */
- u32 wsr; /* Service */
- u32 wrsr; /* Reset Status */
- u32 wicr; /* Interrupt Control */
- u32 wmcr; /* Misc Control */
+ u16 wcr; /* Control */
+ u16 wsr; /* Service */
+ u16 wrsr; /* Reset Status */
+ u16 wicr; /* Interrupt Control */
+ u16 wmcr; /* Misc Control */
};

/* IIM control registers */
@@ -308,7 +308,9 @@ struct iim_regs {
#define GPT_CTRL_TEN 1 /* Timer enable */

/* WDOG enable */
-#define WCR_WDE 0x04
+#define WCR_WDE 0x04
+#define WSR_UNLOCK1 0x5555
+#define WSR_UNLOCK2 0xAAAA

/* FUSE bank offsets */
#define IIM0_MAC 0x1a

[Reinhard Meyer]

Dear Matthias Weisser,

Post by Matthias Weisser
struct wdog_regs {
- u32 wcr; /* Control */
- u32 wsr; /* Service */
- u32 wrsr; /* Reset Status */
- u32 wicr; /* Interrupt Control */
- u32 wmcr; /* Misc Control */
+ u16 wcr; /* Control */
+ u16 wsr; /* Service */
+ u16 wrsr; /* Reset Status */
+ u16 wicr; /* Interrupt Control */
+ u16 wmcr; /* Misc Control */
};

What catches my ARM-aware eye:

I do not know the hardware, but are those registers really
arranged on 2 byte boundaries (00, 02, 04, 06, 08, 0a, .. offsets)?
Probably yes, assuming your code is tested ;)

Or should there be a u16 filler between each register?

Best Regards,
Reinhard

[Stefano Babic]

Post by Reinhard Meyer
Dear Matthias Weisser,

Post by Matthias Weisser
struct wdog_regs {
- u32 wcr; /* Control */
- u32 wsr; /* Service */
- u32 wrsr; /* Reset Status */
- u32 wicr; /* Interrupt Control */
- u32 wmcr; /* Misc Control */
+ u16 wcr; /* Control */
+ u16 wsr; /* Service */
+ u16 wrsr; /* Reset Status */
+ u16 wicr; /* Interrupt Control */
+ u16 wmcr; /* Misc Control */
};

I do not know the hardware, but are those registers really
arranged on 2 byte boundaries (00, 02, 04, 06, 08, 0a, .. offsets)?
Probably yes, assuming your code is tested ;)

Yes, they are. It sounds strange, but they are 16-bit wide.

Stefano

[Matthias Weißer]

Hello Reinhard

Post by Reinhard Meyer
Dear Matthias Weisser,

Post by Matthias Weisser
struct wdog_regs {
- u32 wcr; /* Control */
- u32 wsr; /* Service */
- u32 wrsr; /* Reset Status */
- u32 wicr; /* Interrupt Control */
- u32 wmcr; /* Misc Control */
+ u16 wcr; /* Control */
+ u16 wsr; /* Service */
+ u16 wrsr; /* Reset Status */
+ u16 wicr; /* Interrupt Control */
+ u16 wmcr; /* Misc Control */
};

I do not know the hardware, but are those registers really
arranged on 2 byte boundaries (00, 02, 04, 06, 08, 0a, .. offsets)?
Probably yes, assuming your code is tested ;)

Yes, they are. I looked into the datasheet multiple times to confirm
that. Not typical for an ARM system, but how knows which IP Freescale
recycled here ;-)

If you want to convince yourself take a look at page 1988 of
http://cache.freescale.com/files/dsp/doc/ref_manual/IMX25RM.pdf?fpsp=1


Matthias

[Stefano Babic]

Post by Matthias Weisser
This patch fixes the reset command on imx25. The watchdog registers are 16
bits in size and not 32. This patch also adds the service register codes as
constants.
Signed-off-by: Matthias Weisser <weisserm at arcor.de>
---
Changes since V2
- Using 16 bit constants
Changes since V1
- Corrected whitespace
- Leaving reset sequence as it originally was
arch/arm/cpu/arm926ejs/mx25/reset.c | 8 ++++----
arch/arm/include/asm/arch-mx25/imx-regs.h | 14 ++++++++------
2 files changed, 12 insertions(+), 10 deletions(-)

Applied to u-boot-imx, thanks.

Best regards,
Stefano Babic