Skip to contentSkip to conversation

ARM: ep93xx: use more reliable CPLD watchdog for reset on ts72xx

PATCH Superseded Ryan Mallon 5 participants 22 messages May 31, 2011 – Aug 10, 2011 3 messages not in the archive

Subsystems (3)
Arm (32-bit) Arm sub-architectures Arm/Cirrus logic EP93XX Arm architecture
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

Ryan Mallon

author
#2 lore raw

PATCH 4/4 ARM: ep93xx: use more reliable CPLD watchdog for reset on ts72xx

Quoted 38 lines · Petr ?tetiar

On all ep93xx based boards from Technologic Systems, there's CPLD watchdog available, so use this one to reset the board instead of the soft reset in CPU. I've seen some weird lockups with the soft reset on ep93xx in the past, while the reset via CPLD watchdog seems to be rock solid (tm) and works fine so far.

Cc Hartley Sweeten <[email protected]>
Cc Ryan Mallon <[email protected]>
Signed-off-by Petr ?tetiar <[email protected]>
---
 arch/arm/mach-ep93xx/include/mach/system.h |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)
arch/arm/mach-ep93xx/include/mach/system.h +10 -0
index 6d661fe..4d02ddd 100644
@@ -3,6 +3,10 @@
  */
 
 #include <mach/hardware.h>
+#ifdef CONFIG_MACH_TS72XX
+#include <linux/io.h>
+#include <mach/ts72xx.h>
+#endif
 
 static inline void arch_idle(void)
 {

@@ -13,11 +17,22 @@ static inline void arch_reset(char mode, const char *cmd)
 {
 	local_irq_disable();
 
+#ifdef CONFIG_MACH_TS72XX
+	/*
+	 * It's more reliable to use CPLD watchdog to perform the reset
+	 */
+	if (board_is_ts7200() || board_is_ts7250() || board_is_ts7260() ||
+	    board_is_ts7300() || board_is_ts7400()) {

arch/arm/mach-ep93xx/include/mach/system.h static inline void arch_reset(char mode, const char *cmd) · line 35

Quoted 10 lines
arch/arm/mach-ep93xx/include/mach/system.h +5 -0
+		__raw_writeb(0x5, TS72XX_WDT_FEED_PHYS_BASE);
+		__raw_writeb(0x1, TS72XX_WDT_CONTROL_PHYS_BASE);
+	}
+#else
 	/*
 	 * Set then clear the SWRST bit to initiate a software reset
 	 */
 	ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_SWRST);
 	ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_SWRST);
+#endif

Do we need the ifdef here? Isn't the if (board_is_xxx) test enough?

~Ryan

#3 lore raw

PATCH 4/4 ARM: ep93xx: use more reliable CPLD watchdog for reset on ts72xx

Ryan Mallon <[email protected]> [2011-05-31 18:47:57]:

Quoted 10 lines
Quoted 10 lines
(quoted diff) +10 -0
+#ifdef CONFIG_MACH_TS72XX
+	/*
+	 * It's more reliable to use CPLD watchdog to perform the reset
+	 */
+	if (board_is_ts7200() || board_is_ts7250() || board_is_ts7260() ||
+	    board_is_ts7300() || board_is_ts7400()) {
+		__raw_writeb(0x5, TS72XX_WDT_FEED_PHYS_BASE);
+		__raw_writeb(0x1, TS72XX_WDT_CONTROL_PHYS_BASE);
+	}
+#else
Quoted 8 lines

Review

Quoted 6 lines
(quoted diff) +1 -0
 	/*
 	 * Set then clear the SWRST bit to initiate a software reset
 	 */
 	ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_SWRST);
 	ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_SWRST);
+#endif

Do we need the ifdef here? Isn't the if (board_is_xxx) test enough?

Yes, sure, that the test is enough. The rationale behind that ifdef is, that I was just thinking, that it's not necessary to drag-in more code (althought very small) in the kernel for other ep93xx machines. If it's ok without, I'll make v2.

-- ynezz

#4 lore raw

PATCH 4/4 ARM: ep93xx: use more reliable CPLD watchdog for reset on ts72xx

Quoted 22 lines · Petr ?tetiar

Ryan Mallon <[email protected]> [2011-05-31 18:47:57]:

Quoted 10 lines
Quoted 10 lines
(quoted diff) +10 -0
+#ifdef CONFIG_MACH_TS72XX
+	/*
+	 * It's more reliable to use CPLD watchdog to perform the reset
+	 */
+	if (board_is_ts7200() || board_is_ts7250() || board_is_ts7260() ||
+	    board_is_ts7300() || board_is_ts7400()) {
+		__raw_writeb(0x5, TS72XX_WDT_FEED_PHYS_BASE);
+		__raw_writeb(0x1, TS72XX_WDT_CONTROL_PHYS_BASE);
+	}
+#else
Quoted 8 lines

Review

Quoted 6 lines
(quoted diff) +1 -0
 	/*
 	 * Set then clear the SWRST bit to initiate a software reset
 	 */
 	ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_SWRST);
 	ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_SWRST);
+#endif

Do we need the ifdef here? Isn't the if (board_is_xxx) test enough?

Quoted 4 lines

Yes, sure, that the test is enough. The rationale behind that ifdef is, that I was just thinking, that it's not necessary to drag-in more code (althought very small) in the kernel for other ep93xx machines. If it's ok without, I'll make v2.

For the non ts-xxxx machines the code will reduce to..

	if (0 || 0 || 0 || 0 || 0) {
		...
	} else {
		...
	}

The compiler should then toss the first block of code. Net effect, no code increase.

Please both #ifdef CONFIG_MACH_TS72XX and reorder the #include's.

#include <linux/io.h>

#include <mach/hardware.h>
#include <mach/ts72xx.h>

Regards,
Hartley

#5 lore raw

PATCH v2 1/1 ARM: ep93xx: use more reliable CPLD watchdog for reset on ts72xx

On all ep93xx based boards from Technologic Systems, there's CPLD watchdog available, so use this one to reset the board instead of the soft reset in CPU. I've seen some weird lockups with the soft reset on ep93xx in the past, while the reset via CPLD watchdog seems to be rock solid (tm) and works fine so far.

Cc Hartley Sweeten <[email protected]>
Cc Ryan Mallon <[email protected]>
Signed-off-by Petr ?tetiar <[email protected]>
---
 arch/arm/mach-ep93xx/include/mach/system.h |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)
arch/arm/mach-ep93xx/include/mach/system.h +12 -0
index 6d661fe..8d31c4d 100644
@@ -2,7 +2,10 @@
  * arch/arm/mach-ep93xx/include/mach/system.h
  */
 
+#include <linux/io.h>
+
 #include <mach/hardware.h>
+#include <mach/ts72xx.h>
 
 static inline void arch_idle(void)
 {

@@ -14,6 +17,15 @@ static inline void arch_reset(char mode, const char *cmd)
 	local_irq_disable();
 
 	/*
+	 * It's more reliable to use CPLD watchdog to perform the reset
+	 */
+	if (board_is_ts7200() || board_is_ts7250() || board_is_ts7260() ||
+	    board_is_ts7300() || board_is_ts7400()) {
+		__raw_writeb(0x5, TS72XX_WDT_FEED_PHYS_BASE);
+		__raw_writeb(0x1, TS72XX_WDT_CONTROL_PHYS_BASE);
+	}
+
+	/*
 	 * Set then clear the SWRST bit to initiate a software reset
 	 */
 	ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_SWRST);
Signature · 1 line
1.7.1
#6 lore raw

PATCH v2 1/1 ARM: ep93xx: use more reliable CPLD watchdog for reset on ts72xx

Quoted 29 lines · Petr ?tetiar

On all ep93xx based boards from Technologic Systems, there's CPLD watchdog available, so use this one to reset the board instead of the soft reset in CPU. I've seen some weird lockups with the soft reset on ep93xx in the past, while the reset via CPLD watchdog seems to be rock solid (tm) and works fine so far.

Cc Hartley Sweeten <[email protected]>
Cc Ryan Mallon <[email protected]>
Signed-off-by Petr ?tetiar <[email protected]>
---
 arch/arm/mach-ep93xx/include/mach/system.h |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)
arch/arm/mach-ep93xx/include/mach/system.h +3 -0
index 6d661fe..8d31c4d 100644
@@ -2,7 +2,10 @@
  * arch/arm/mach-ep93xx/include/mach/system.h
  */
 
+#include <linux/io.h>
+
 #include <mach/hardware.h>
+#include <mach/ts72xx.h>
 
 static inline void arch_idle(void)
 {

arch/arm/mach-ep93xx/include/mach/system.h static inline void arch_reset(char mode, const char *cmd) · line 26

Quoted 10 lines
arch/arm/mach-ep93xx/include/mach/system.h +7 -0
@@ -14,6 +17,15 @@ static inline void arch_reset(char mode, const char *cmd)
 	local_irq_disable();
 
 	/*
+	 * It's more reliable to use CPLD watchdog to perform the reset
+	 */
+	if (board_is_ts7200() || board_is_ts7250() || board_is_ts7260() ||
+	    board_is_ts7300() || board_is_ts7400()) {
+		__raw_writeb(0x5, TS72XX_WDT_FEED_PHYS_BASE);
+		__raw_writeb(0x1, TS72XX_WDT_CONTROL_PHYS_BASE);
+	}

Your missing the 'else' here. Without it the ts-xxxx boards will fiddle with the CPLD watchdog to do the reset but the SWRST bit might still get toggled if the reset does not occur quick enough. This might result in you still getting the "weird" lockup.

arch/arm/mach-ep93xx/include/mach/system.h static inline void arch_reset(char mode, const char *cmd) · line 31

Quoted 5 lines
arch/arm/mach-ep93xx/include/mach/system.h +2 -0
+
+	/*
 	 * Set then clear the SWRST bit to initiate a software reset
 	 */
 	ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_SWRST);

Regards,
Hartley

#7 lore raw

PATCH v3 1/1 ARM: ep93xx: use more reliable CPLD watchdog for reset on ts72xx

On all ep93xx based boards from Technologic Systems, there's CPLD watchdog available, so use this one to reset the board instead of the soft reset in CPU. I've seen some weird lockups with the soft reset on ep93xx in the past, while the reset via CPLD watchdog seems to be rock solid (tm) and works fine so far.

Cc Hartley Sweeten <[email protected]>
Cc Ryan Mallon <[email protected]>
Signed-off-by Petr ?tetiar <[email protected]>
---
 arch/arm/mach-ep93xx/include/mach/system.h |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)
arch/arm/mach-ep93xx/include/mach/system.h +13 -5
index 6d661fe..2969786 100644
@@ -2,7 +2,10 @@
  * arch/arm/mach-ep93xx/include/mach/system.h
  */
 
+#include <linux/io.h>
+
 #include <mach/hardware.h>
+#include <mach/ts72xx.h>
 
 static inline void arch_idle(void)
 {

@@ -13,11 +16,16 @@ static inline void arch_reset(char mode, const char *cmd)
 {
 	local_irq_disable();
 
-	/*
-	 * Set then clear the SWRST bit to initiate a software reset
-	 */
-	ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_SWRST);
-	ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_SWRST);
+	/* It's more reliable to use CPLD watchdog to perform the reset */
+	if (board_is_ts7200() || board_is_ts7250() || board_is_ts7260() ||
+	    board_is_ts7300() || board_is_ts7400()) {
+		__raw_writeb(0x5, TS72XX_WDT_FEED_PHYS_BASE);
+		__raw_writeb(0x1, TS72XX_WDT_CONTROL_PHYS_BASE);
+	} else {
+		/* Set then clear the SWRST bit to initiate a software reset */
+		ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_SWRST);
+		ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_SWRST);
+	}
 
 	while (1)
 		;
Signature · 1 line
1.7.1
#8 lore raw

PATCH v3 1/1 ARM: ep93xx: use more reliable CPLD watchdog for reset on ts72xx

Quoted 40 lines · Petr ?tetiar

On all ep93xx based boards from Technologic Systems, there's CPLD watchdog available, so use this one to reset the board instead of the soft reset in CPU. I've seen some weird lockups with the soft reset on ep93xx in the past, while the reset via CPLD watchdog seems to be rock solid (tm) and works fine so far.

Cc Hartley Sweeten <[email protected]>
Cc Ryan Mallon <[email protected]>
Signed-off-by Petr ?tetiar <[email protected]>
---
 arch/arm/mach-ep93xx/include/mach/system.h |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)
arch/arm/mach-ep93xx/include/mach/system.h +6 -5
index 6d661fe..2969786 100644
@@ -2,7 +2,10 @@
  * arch/arm/mach-ep93xx/include/mach/system.h
  */
 
+#include <linux/io.h>
+
 #include <mach/hardware.h>
+#include <mach/ts72xx.h>
 
 static inline void arch_idle(void)
 {

@@ -13,11 +16,16 @@ static inline void arch_reset(char mode, const char *cmd)
 {
 	local_irq_disable();
 
-	/*
-	 * Set then clear the SWRST bit to initiate a software reset
-	 */
-	ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_SWRST);
-	ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_SWRST);
+	/* It's more reliable to use CPLD watchdog to perform the reset */
+	if (board_is_ts7200() || board_is_ts7250() || board_is_ts7260() ||
+	    board_is_ts7300() || board_is_ts7400()) {

arch/arm/mach-ep93xx/include/mach/system.h static inline void arch_reset(char mode, const char *cmd) · line 31

Quoted 10 lines
arch/arm/mach-ep93xx/include/mach/system.h +7 -0
+		__raw_writeb(0x5, TS72XX_WDT_FEED_PHYS_BASE);
+		__raw_writeb(0x1, TS72XX_WDT_CONTROL_PHYS_BASE);
+	} else {
+		/* Set then clear the SWRST bit to initiate a software reset */
+		ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_SWRST);
+		ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_SWRST);
+	}
 
 	while (1)
 		;

That looks better.

I pulled out the ts-7200 manual to verify the watchdog reset. Your feeding the watchdog then changing the timeout period to 250ms. Without the "else" the syscon would have tried to reset the board when the SWRST bit is toggled.

I have no way of testing this patch (no ts boards) but as long as Ryan has no objections you have my:

Acked-by H Hartley Sweeten <[email protected]>
#9 lore raw

PATCH v3 1/1 ARM: ep93xx: use more reliable CPLD watchdog for reset on ts72xx

H Hartley Sweeten <[email protected]> [2011-05-31 14:24:04]:

Quoted 3 lines

I pulled out the ts-7200 manual to verify the watchdog reset. Your feeding the watchdog then changing the timeout period to 250ms. Without the "else" the syscon would have tried to reset the board when the SWRST bit is toggled.

Yep, that missing "else" was a good catch, thanks.

This feed first then set behaviour is "logic", but it's correct (250ms is the smallest possible value), from that manual:

In order to load the WDT Control register, the WDT must first be ?fed?, and then within 30 uS, the WDT control register must be written. Writes to this register without first doing a ?WDT feed?, have no affect.

(Please don't laugh at that 30us constraint).

Quoted 2 lines

I have no way of testing this patch (no ts boards) but as long as Ryan has no objections you have my:

I'm using it[1] since 2.6.19-rc6-git10 (maybe earlier). Funny, that I didn't forget to add that "else" in this old patch.

And as I see it, you and few other guys really deserve some free TS boards for sure (at least the damn board as a thank you), but Lennert could probably tell you more about their "we only leech the work from the comunity, but don't give anything back" attitude...

1. http://ynezz.ibawizard.net/ts72xx/ts72xx_cpld_wdt_reset.diff.gz

Quoted 1 line
Acked-by H Hartley Sweeten <[email protected]>

Thanks.

-- ynezz

#10 lore raw

PATCH v3 1/1 ARM: ep93xx: use more reliable CPLD watchdog for reset on ts72xx

Quoted 8 lines · Petr ?tetiar

H Hartley Sweeten <[email protected]> [2011-05-31 14:24:04]:

Quoted 3 lines

I pulled out the ts-7200 manual to verify the watchdog reset. Your feeding the watchdog then changing the timeout period to 250ms. Without the "else" the syscon would have tried to reset the board when the SWRST bit is toggled.

Yep, that missing "else" was a good catch, thanks.

Quoted 8 lines

This feed first then set behaviour is "logic", but it's correct (250ms is the smallest possible value), from that manual:

In order to load the WDT Control register, the WDT must first be ?fed?, and then within 30 uS, the WDT control register must be written. Writes to this register without first doing a ?WDT feed?, have no affect.

(Please don't laugh at that 30us constraint).

;-)

Quoted 10 lines
Quoted 2 lines

I have no way of testing this patch (no ts boards) but as long as Ryan has no objections you have my:

I'm using it[1] since 2.6.19-rc6-git10 (maybe earlier). Funny, that I didn't forget to add that "else" in this old patch.

And as I see it, you and few other guys really deserve some free TS boards for sure (at least the damn board as a thank you), but Lennert could probably tell you more about their "we only leech the work from the comunity, but don't give anything back" attitude...

Yah, the ep93xx port has some bad history. It's old history, lets move on...

If they want to give me one I'll be happy to take it. I can even go pick it up, they are only 44 miles away from my on the other side of the valley....

Regards,
Hartley

author
#11 lore raw

PATCH v3 1/1 ARM: ep93xx: use more reliable CPLD watchdog for reset on ts72xx

In reply to H Hartley Sweeten
Quoted 54 lines · H Hartley Sweeten
Quoted 40 lines · Petr ?tetiar

On all ep93xx based boards from Technologic Systems, there's CPLD watchdog available, so use this one to reset the board instead of the soft reset in CPU. I've seen some weird lockups with the soft reset on ep93xx in the past, while the reset via CPLD watchdog seems to be rock solid (tm) and works fine so far.

Cc Hartley Sweeten <[email protected]>
Cc Ryan Mallon <[email protected]>
Signed-off-by Petr ?tetiar <[email protected]>
---
 arch/arm/mach-ep93xx/include/mach/system.h |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)
arch/arm/mach-ep93xx/include/mach/system.h +6 -5
index 6d661fe..2969786 100644
@@ -2,7 +2,10 @@
  * arch/arm/mach-ep93xx/include/mach/system.h
  */
 
+#include <linux/io.h>
+
 #include <mach/hardware.h>
+#include <mach/ts72xx.h>
 
 static inline void arch_idle(void)
 {

@@ -13,11 +16,16 @@ static inline void arch_reset(char mode, const char *cmd)
 {
 	local_irq_disable();
 
-	/*
-	 * Set then clear the SWRST bit to initiate a software reset
-	 */
-	ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_SWRST);
-	ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_SWRST);
+	/* It's more reliable to use CPLD watchdog to perform the reset */
+	if (board_is_ts7200() || board_is_ts7250() || board_is_ts7260() ||
+	    board_is_ts7300() || board_is_ts7400()) {

arch/arm/mach-ep93xx/include/mach/system.h static inline void arch_reset(char mode, const char *cmd) · line 31

Quoted 10 lines
arch/arm/mach-ep93xx/include/mach/system.h +7 -0
+		__raw_writeb(0x5, TS72XX_WDT_FEED_PHYS_BASE);
+		__raw_writeb(0x1, TS72XX_WDT_CONTROL_PHYS_BASE);
+	} else {
+		/* Set then clear the SWRST bit to initiate a software reset */
+		ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_SWRST);
+		ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_SWRST);
+	}
 
 	while (1)
 		;

That looks better.

Quoted 6 lines

I pulled out the ts-7200 manual to verify the watchdog reset. Your feeding the watchdog then changing the timeout period to 250ms. Without the "else" the syscon would have tried to reset the board when the SWRST bit is toggled.

I have no way of testing this patch (no ts boards) but as long as Ryan has no objections you have my:

My only (nitpicky) complaint is to move the CPLD comment inside the if block so that it is more clear that it relates to the ts7xxx boards. Otherwise the patch is fine.

Quoted 2 lines
Acked-by H Hartley Sweeten <[email protected]>
#12 lore raw

PATCH v3 1/1 ARM: ep93xx: use more reliable CPLD watchdog for reset on ts72xx

Quoted 59 lines · Ryan Mallon
Quoted 58 lines · H Hartley Sweeten
Quoted 40 lines · Petr ?tetiar

On all ep93xx based boards from Technologic Systems, there's CPLD watchdog available, so use this one to reset the board instead of the soft reset in CPU. I've seen some weird lockups with the soft reset on ep93xx in the past, while the reset via CPLD watchdog seems to be rock solid (tm) and works fine so far.

Cc Hartley Sweeten <[email protected]>
Cc Ryan Mallon <[email protected]>
Signed-off-by Petr ?tetiar <[email protected]>
---
 arch/arm/mach-ep93xx/include/mach/system.h |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)
arch/arm/mach-ep93xx/include/mach/system.h +6 -5
index 6d661fe..2969786 100644
@@ -2,7 +2,10 @@
  * arch/arm/mach-ep93xx/include/mach/system.h
  */
 
+#include <linux/io.h>
+
 #include <mach/hardware.h>
+#include <mach/ts72xx.h>
 
 static inline void arch_idle(void)
 {

@@ -13,11 +16,16 @@ static inline void arch_reset(char mode, const char *cmd)
 {
 	local_irq_disable();
 
-	/*
-	 * Set then clear the SWRST bit to initiate a software reset
-	 */
-	ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_SWRST);
-	ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_SWRST);
+	/* It's more reliable to use CPLD watchdog to perform the reset */
+	if (board_is_ts7200() || board_is_ts7250() || board_is_ts7260() ||
+	    board_is_ts7300() || board_is_ts7400()) {

arch/arm/mach-ep93xx/include/mach/system.h static inline void arch_reset(char mode, const char *cmd) · line 31

Quoted 10 lines
arch/arm/mach-ep93xx/include/mach/system.h +7 -0
+		__raw_writeb(0x5, TS72XX_WDT_FEED_PHYS_BASE);
+		__raw_writeb(0x1, TS72XX_WDT_CONTROL_PHYS_BASE);
+	} else {
+		/* Set then clear the SWRST bit to initiate a software reset */
+		ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_SWRST);
+		ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_SWRST);
+	}
 
 	while (1)
 		;

That looks better.

I pulled out the ts-7200 manual to verify the watchdog reset. Your feeding the watchdog then changing the timeout period to 250ms. Without the "else" the syscon would have tried to reset the board when the SWRST bit is toggled.

Quoted 6 lines
Quoted 2 lines

I have no way of testing this patch (no ts boards) but as long as Ryan has no objections you have my:

My only (nitpicky) complaint is to move the CPLD comment inside the if block so that it is more clear that it relates to the ts7xxx boards. Otherwise the patch is fine.

Valid point. The CPLD watchdog reset is a ts-xxxx specific thing.

Petr, please move the comment as Ryan suggests.

Regards,
Hartley

#13 lore raw

PATCH v4 1/1 ARM: ep93xx: use more reliable CPLD watchdog for reset on ts72xx

On all ep93xx based boards from Technologic Systems, there's CPLD watchdog available, so use this one to reset the board instead of the soft reset in CPU. I've seen some weird lockups with the soft reset on ep93xx in the past, while the reset via CPLD watchdog seems to be rock solid (tm) and works fine so far.

Cc Ryan Mallon <[email protected]>
Acked-by H Hartley Sweeten <[email protected]>
Signed-off-by Petr ?tetiar <[email protected]>
---
 arch/arm/mach-ep93xx/include/mach/system.h |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)
arch/arm/mach-ep93xx/include/mach/system.h +13 -5
index 6d661fe..67ec430 100644
@@ -2,7 +2,10 @@
  * arch/arm/mach-ep93xx/include/mach/system.h
  */
 
+#include <linux/io.h>
+
 #include <mach/hardware.h>
+#include <mach/ts72xx.h>
 
 static inline void arch_idle(void)
 {

@@ -13,11 +16,16 @@ static inline void arch_reset(char mode, const char *cmd)
 {
 	local_irq_disable();
 
-	/*
-	 * Set then clear the SWRST bit to initiate a software reset
-	 */
-	ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_SWRST);
-	ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_SWRST);
+	if (board_is_ts7200() || board_is_ts7250() || board_is_ts7260() ||
+	    board_is_ts7300() || board_is_ts7400()) {
+		/* We use more reliable CPLD watchdog to perform the reset */
+		__raw_writeb(0x5, TS72XX_WDT_FEED_PHYS_BASE);
+		__raw_writeb(0x1, TS72XX_WDT_CONTROL_PHYS_BASE);
+	} else {
+		/* Set then clear the SWRST bit to initiate a software reset */
+		ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_SWRST);
+		ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_SWRST);
+	}
 
 	while (1)
 		;
Signature · 1 line
1.7.1
#14 lore raw

PATCH v4 1/1 ARM: ep93xx: use more reliable CPLD watchdog for reset on ts72xx

Hi,

Quoted 37 lines · Petr ?tetiar

On all ep93xx based boards from Technologic Systems, there's CPLD watchdog available, so use this one to reset the board instead of the soft reset in CPU. ?I've seen some weird lockups with the soft reset on ep93xx in the past, while the reset via CPLD watchdog seems to be rock solid (tm) and works fine so far.

Cc Ryan Mallon <[email protected]>
Acked-by H Hartley Sweeten <[email protected]>
Signed-off-by Petr ?tetiar <[email protected]>

---
?arch/arm/mach-ep93xx/include/mach/system.h | ? 18 +++++++++++++----- ?1 files changed, 13 insertions(+), 5 deletions(-)

arch/arm/mach-ep93xx/include/mach/system.h +0 -0
index 6d661fe..67ec430 100644
@@ -2,7 +2,10 @@

?* arch/arm/mach-ep93xx/include/mach/system.h
?*/

arch/arm/mach-ep93xx/include/mach/system.h +2 -0
+#include <linux/io.h>
+

?#include <mach/hardware.h>
+#include <mach/ts72xx.h>

?static inline void arch_idle(void)
?{

arch/arm/mach-ep93xx/include/mach/system.h +0 -5
@@ -13,11 +16,16 @@ static inline void arch_reset(char mode, const char *cmd)
 ?{
 ? ? ? ?local_irq_disable();
 
- ? ? ? /*
- ? ? ? ?* Set then clear the SWRST bit to initiate a software reset
- ? ? ? ?*/
- ? ? ? ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_SWRST);
- ? ? ? ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_SWRST);

arch/arm/mach-ep93xx/include/mach/system.h static inline void arch_reset(char mode, const char *cmd) · line 28

Quoted 10 lines
arch/arm/mach-ep93xx/include/mach/system.h +10 -0
+ ? ? ? if (board_is_ts7200() || board_is_ts7250() || board_is_ts7260() ||
+ ? ? ? ? ? board_is_ts7300() || board_is_ts7400()) {
+ ? ? ? ? ? ? ? /* We use more reliable CPLD watchdog to perform the reset */
+ ? ? ? ? ? ? ? __raw_writeb(0x5, TS72XX_WDT_FEED_PHYS_BASE);
+ ? ? ? ? ? ? ? __raw_writeb(0x1, TS72XX_WDT_CONTROL_PHYS_BASE);
+ ? ? ? } else {
+ ? ? ? ? ? ? ? /* Set then clear the SWRST bit to initiate a software reset */
+ ? ? ? ? ? ? ? ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_SWRST);
+ ? ? ? ? ? ? ? ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_SWRST);
+ ? ? ? }

It would be nicer to introduce a function pointer here, that's filled in by the ts72xx.c machine_init function but NULL by default:

extern void (*ep93xx_reset)(void);
...

arch_reset():

   local_irq_disable();
   if (ep93xx_reset) {
      ep93xx_reset();

} else {
... current SWRST code ...
}
...

Otherwise, other boards would add another if statement, any new ts72xx board would need to modify the soc-common header, etc.

-Olof

#15 lore raw

PATCH v4 1/1 ARM: ep93xx: use more reliable CPLD watchdog for reset on ts72xx

Olof Johansson <[email protected]> [2011-06-03 11:43:27]:

Hi,

Quoted 2 lines

It would be nicer to introduce a function pointer here, that's filled in by the ts72xx.c machine_init function but NULL by default:

yes, I didn't liked the modification of the shared machine code either and I personally dislike the ifdefs also, but

Quoted 2 lines

Otherwise, other boards would add another if statement, any new ts72xx board would need to modify the soc-common header, etc.

this is very unlikely to happen. The TS's ep93xx based products seems to be EOL, they'll just sell them until they've parts/SOC available.

I'll leave the decision up to the maintainers.

-- ynezz

#16 lore raw

PATCH v4 1/1 ARM: ep93xx: use more reliable CPLD watchdog for reset on ts72xx

In reply to Petr Štetiar
Quoted 4 lines · Petr ?tetiar
arch/arm/mach-ep93xx/include/mach/system.h +0 -0
@@ -13,11 +16,16 @@ static inline void arch_reset(char mode, const char *cmd)
 {
 	local_irq_disable();

arch/arm/mach-ep93xx/include/mach/system.h static inline void arch_reset(char mode, const char *cmd) · line 23

Quoted 10 lines
arch/arm/mach-ep93xx/include/mach/system.h +5 -5
-	/*
-	 * Set then clear the SWRST bit to initiate a software reset
-	 */
-	ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_SWRST);
-	ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_SWRST);
+	if (board_is_ts7200() || board_is_ts7250() || board_is_ts7260() ||
+	    board_is_ts7300() || board_is_ts7400()) {
+		/* We use more reliable CPLD watchdog to perform the reset */
+		__raw_writeb(0x5, TS72XX_WDT_FEED_PHYS_BASE);
+		__raw_writeb(0x1, TS72XX_WDT_CONTROL_PHYS_BASE);

I just noticed that you are accessing the registers via *physical* address. It currently works because arm_machine_restart() sets up 1:1 mappings in place of userspace before arch_reset() gets called.

This might cause some problems as the register accesses are cached, or does it make a difference in ARM920?

#17 lore raw

PATCH v4 1/1 ARM: ep93xx: use more reliable CPLD watchdog for reset on ts72xx

Mika Westerberg <[email protected]> [2011-06-05 12:54:50]:

Quoted 9 lines

arch/arm/mach-ep93xx/include/mach/system.h static inline void arch_reset(char mode, const char *cmd) · line 23

Quoted 5 lines
arch/arm/mach-ep93xx/include/mach/system.h +5 -0
+	if (board_is_ts7200() || board_is_ts7250() || board_is_ts7260() ||
+	    board_is_ts7300() || board_is_ts7400()) {
+		/* We use more reliable CPLD watchdog to perform the reset */
+		__raw_writeb(0x5, TS72XX_WDT_FEED_PHYS_BASE);
+		__raw_writeb(0x1, TS72XX_WDT_CONTROL_PHYS_BASE);

I just noticed that you are accessing the registers via *physical* address. It currently works because arm_machine_restart() sets up 1:1 mappings in place of userspace before arch_reset() gets called.

Setups the 1:1 mappings, clean+invalidate cache, turns off caching and flush the cache.

Quoted 2 lines

This might cause some problems as the register accesses are cached, or does it make a difference in ARM920?

Caching is disabled by the caller, isn't it?

-- ynezz

#18 lore raw

PATCH v4 1/1 ARM: ep93xx: use more reliable CPLD watchdog for reset on ts72xx

Quoted 12 lines · Petr ?tetiar

Mika Westerberg <[email protected]> [2011-06-05 12:54:50]:

Quoted 9 lines

arch/arm/mach-ep93xx/include/mach/system.h static inline void arch_reset(char mode, const char *cmd) · line 23

Quoted 5 lines
arch/arm/mach-ep93xx/include/mach/system.h +5 -0
+	if (board_is_ts7200() || board_is_ts7250() || board_is_ts7260() ||
+	    board_is_ts7300() || board_is_ts7400()) {
+		/* We use more reliable CPLD watchdog to perform the reset */
+		__raw_writeb(0x5, TS72XX_WDT_FEED_PHYS_BASE);
+		__raw_writeb(0x1, TS72XX_WDT_CONTROL_PHYS_BASE);

I just noticed that you are accessing the registers via *physical* address. It currently works because arm_machine_restart() sets up 1:1 mappings in place of userspace before arch_reset() gets called.

Quoted 7 lines

Setups the 1:1 mappings, clean+invalidate cache, turns off caching and flush the cache.

Quoted 2 lines

This might cause some problems as the register accesses are cached, or does it make a difference in ARM920?

Caching is disabled by the caller, isn't it?

Yes.

I'm just wondering whether this should be done via valid mapping? Note also that __raw_writeb() takes void __iomem * which is different that the value you are passing.

Anyway, I tried the patch on my TS-7260 and I'm still able to reset the board so you can add my

Tested-by Mika Westerberg <[email protected]>

if you like.

#19 lore raw

PATCH v4 1/1 ARM: ep93xx: use more reliable CPLD watchdog for reset on ts72xx

In reply to Petr Štetiar
Quoted 41 lines · Petr ?tetiar

On all ep93xx based boards from Technologic Systems, there's CPLD watchdog available, so use this one to reset the board instead of the soft reset in CPU. I've seen some weird lockups with the soft reset on ep93xx in the past, while the reset via CPLD watchdog seems to be rock solid (tm) and works fine so far.

Cc Ryan Mallon <[email protected]>
Acked-by H Hartley Sweeten <[email protected]>
Signed-off-by Petr ?tetiar <[email protected]>
---
 arch/arm/mach-ep93xx/include/mach/system.h |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)
arch/arm/mach-ep93xx/include/mach/system.h +6 -5
index 6d661fe..67ec430 100644
@@ -2,7 +2,10 @@
  * arch/arm/mach-ep93xx/include/mach/system.h
  */
 
+#include <linux/io.h>
+
 #include <mach/hardware.h>
+#include <mach/ts72xx.h>
 
 static inline void arch_idle(void)
 {

@@ -13,11 +16,16 @@ static inline void arch_reset(char mode, const char *cmd)
 {
 	local_irq_disable();
 
-	/*
-	 * Set then clear the SWRST bit to initiate a software reset
-	 */
-	ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_SWRST);
-	ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_SWRST);
+	if (board_is_ts7200() || board_is_ts7250() || board_is_ts7260() ||
+	    board_is_ts7300() || board_is_ts7400()) {
+		/* We use more reliable CPLD watchdog to perform the reset */

arch/arm/mach-ep93xx/include/mach/system.h static inline void arch_reset(char mode, const char *cmd) · line 31

Quoted 10 lines
arch/arm/mach-ep93xx/include/mach/system.h +7 -0
+		__raw_writeb(0x5, TS72XX_WDT_FEED_PHYS_BASE);
+		__raw_writeb(0x1, TS72XX_WDT_CONTROL_PHYS_BASE);
+	} else {
+		/* Set then clear the SWRST bit to initiate a software reset */
+		ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_SWRST);
+		ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_SWRST);
+	}
 
 	while (1)
 		;

Petr,

This patch is still in Russell's patch tracker. I just realized a possible problem with it.

If any other ep93xx machine is selected along with MACH_TS72XX and the kernel is booted on a non-ts72xx machine, the static mapping for the ts72xx CPLD will not be done. I believe this will cause a problem when doing the boad_is_* calls due to the:

	__raw_readb(TS72XX_MODEL_VIRT_BASE)

I think the best solution is to remove the #include <mach/ts72xx.h> from this patch and change this:

+	if (board_is_ts7200() || board_is_ts7250() || board_is_ts7260() ||
+	    board_is_ts7300() || board_is_ts7400()) {

To:

+ if (machine_is_ts72xx())

This will correctly evaluate is the kernel is booted on a MACH_TYPE_TS72XX system.

Can you please update and test the patch?

Regards,
Hartley

#20 lore raw

PATCH v3 1/1 ARM: ep93xx: use more reliable CPLD watchdog for reset on ts72xx

In reply to Petr Štetiar
Quoted 5 lines · Petr ?tetiar

On all ep93xx based boards from Technologic Systems, there's CPLD watchdog available, so use this one to reset the board instead of the soft reset in CPU. I've seen some weird lockups with the soft reset on ep93xx in the past, while the reset via CPLD watchdog seems to be rock solid (tm) and works fine so far.

Where can I find the whole series? I was going to test this on my TS-7260 but I only found this patch from my inbox and it doesn't apply without the previous one :-(

#21 lore raw

PATCH v3 1/1 ARM: ep93xx: use more reliable CPLD watchdog for reset on ts72xx

Mika Westerberg <[email protected]> [2011-06-02 12:54:55]:

Quoted 3 lines

Where can I find the whole series? I was going to test this on my TS-7260 but I only found this patch from my inbox and it doesn't apply without the previous one :-(

I'll push that wip branch on github later today, and will give you the link. Thanks.

-- ynezz

#22 lore raw

PATCH v3 1/1 ARM: ep93xx: use more reliable CPLD watchdog for reset on ts72xx

In reply to Mika Westerberg

Mika Westerberg <[email protected]> [2011-06-02 12:54:55]:

Quoted 10 lines · Petr ?tetiar
Quoted 5 lines

On all ep93xx based boards from Technologic Systems, there's CPLD watchdog available, so use this one to reset the board instead of the soft reset in CPU. I've seen some weird lockups with the soft reset on ep93xx in the past, while the reset via CPLD watchdog seems to be rock solid (tm) and works fine so far.

Where can I find the whole series? I was going to test this on my TS-7260 but I only found this patch from my inbox and it doesn't apply without the previous one :-(

Please clone git://github.com/ynezz/linux-2.6.git, there's branch ts72xx-wip and you need to cherry-pick this two commits:

d3c3c086b21b11063efc27c30acb08c03b9fbe23
cd23b7132d441a0bc23e74ef04ad74c4616e13c9

-- ynezz

A message that is not in the archive (sent off-list or not yet received): [email protected]

#24 lore raw

PATCH 3/4 ARM: ep93xx: add model detection for ts-7300 and ts-7400 boards

Quoted 29 lines · Petr ?tetiar

Subject: [PATCH 3/4] ARM: ep93xx: add model detection for ts-7300 and ts-7400 boards

Cc Ryan Mallon <[email protected]>
Cc Hartley Sweeten <[email protected]>
Signed-off-by Petr ?tetiar <[email protected]>
---
 arch/arm/mach-ep93xx/include/mach/ts72xx.h |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)
arch/arm/mach-ep93xx/include/mach/ts72xx.h +5 -0
index 0eabec6..ee7f875 100644
@@ -20,6 +20,8 @@
 #define TS72XX_MODEL_TS7200		0x00
 #define TS72XX_MODEL_TS7250		0x01
 #define TS72XX_MODEL_TS7260		0x02
+#define TS72XX_MODEL_TS7300		0x03
+#define TS72XX_MODEL_TS7400		0x04
 
 
 #define TS72XX_OPTIONS_PHYS_BASE	0x22400000

@@ -66,6 +68,16 @@ static inline int board_is_ts7260(void)
 	return __raw_readb(TS72XX_MODEL_VIRT_BASE) == TS72XX_MODEL_TS7260;
 }
 
+static inline int board_is_ts7300(void)
+{
+	return __raw_readb(TS72XX_MODEL_VIRT_BASE) == TS72XX_MODEL_TS7300;

arch/arm/mach-ep93xx/include/mach/ts72xx.h static inline int board_is_ts7260(void) · line 83

Quoted 10 lines
arch/arm/mach-ep93xx/include/mach/ts72xx.h +7 -0
+}
+
+static inline int board_is_ts7400(void)
+{
+	return __raw_readb(TS72XX_MODEL_VIRT_BASE) == TS72XX_MODEL_TS7400;
+}
+
 static inline int is_max197_installed(void)
 {
 	return !!(__raw_readb(TS72XX_OPTIONS_VIRT_BASE) &
Acked-by H Hartley Sweeten <[email protected]>