| Message ID | 1306868578-3883-1-git-send-email-ynezz@true.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tuesday, May 31, 2011 12:03 PM, Petr Štetiar wrote: > 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 <hsweeten@visionengravers.com> > Cc: Ryan Mallon <ryan@bluewatersys.com> > Signed-off-by: Petr Štetiar <ynezz@true.cz> > --- > arch/arm/mach-ep93xx/include/mach/system.h | 18 +++++++++++++----- > 1 files changed, 13 insertions(+), 5 deletions(-) > > diff --git a/arch/arm/mach-ep93xx/include/mach/system.h b/arch/arm/mach-ep93xx/include/mach/system.h > index 6d661fe..2969786 100644 > --- a/arch/arm/mach-ep93xx/include/mach/system.h > +++ b/arch/arm/mach-ep93xx/include/mach/system.h > @@ -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) > ; 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 <hsweeten@visionengravers.com>
H Hartley Sweeten <hartleys@visionengravers.com> [2011-05-31 14:24:04]: > 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). > 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 > Acked-by: H Hartley Sweeten <hsweeten@visionengravers.com> Thanks. -- ynezz
On Tuesday, May 31, 2011 12:55 PM, Petr Štetiar wrote: > H Hartley Sweeten <hartleys@visionengravers.com> [2011-05-31 14:24:04]: > >> 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). ;-) >> 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
On 01/06/11 05:24, H Hartley Sweeten wrote: > On Tuesday, May 31, 2011 12:03 PM, Petr Štetiar wrote: >> 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 <hsweeten@visionengravers.com> >> Cc: Ryan Mallon <ryan@bluewatersys.com> >> Signed-off-by: Petr Štetiar <ynezz@true.cz> >> --- >> arch/arm/mach-ep93xx/include/mach/system.h | 18 +++++++++++++----- >> 1 files changed, 13 insertions(+), 5 deletions(-) >> >> diff --git a/arch/arm/mach-ep93xx/include/mach/system.h b/arch/arm/mach-ep93xx/include/mach/system.h >> index 6d661fe..2969786 100644 >> --- a/arch/arm/mach-ep93xx/include/mach/system.h >> +++ b/arch/arm/mach-ep93xx/include/mach/system.h >> @@ -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) >> ; > > 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: 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. > > Acked-by: H Hartley Sweeten <hsweeten@visionengravers.com>
On Tuesday, May 31, 2011 2:43 PM, Ryan Mallon wrote: > On 01/06/11 05:24, H Hartley Sweeten wrote: >> On Tuesday, May 31, 2011 12:03 PM, Petr Štetiar wrote: >>> 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 <hsweeten@visionengravers.com> >>> Cc: Ryan Mallon <ryan@bluewatersys.com> >>> Signed-off-by: Petr Štetiar <ynezz@true.cz> >>> --- >>> arch/arm/mach-ep93xx/include/mach/system.h | 18 +++++++++++++----- >>> 1 files changed, 13 insertions(+), 5 deletions(-) >>> >>> diff --git a/arch/arm/mach-ep93xx/include/mach/system.h b/arch/arm/mach-ep93xx/include/mach/system.h >>> index 6d661fe..2969786 100644 >>> --- a/arch/arm/mach-ep93xx/include/mach/system.h >>> +++ b/arch/arm/mach-ep93xx/include/mach/system.h >>> @@ -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) >>> ; >> >> 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: > > 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
On Tue, May 31, 2011 at 09:02:58PM +0200, Petr Štetiar wrote: > 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 :-(
Mika Westerberg <mika.westerberg@iki.fi> [2011-06-02 12:54:55]: > 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
Mika Westerberg <mika.westerberg@iki.fi> [2011-06-02 12:54:55]: > On Tue, May 31, 2011 at 09:02:58PM +0200, Petr Štetiar wrote: > > 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
diff --git a/arch/arm/mach-ep93xx/include/mach/system.h b/arch/arm/mach-ep93xx/include/mach/system.h index 6d661fe..2969786 100644 --- a/arch/arm/mach-ep93xx/include/mach/system.h +++ b/arch/arm/mach-ep93xx/include/mach/system.h @@ -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) ;
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 <hsweeten@visionengravers.com> Cc: Ryan Mallon <ryan@bluewatersys.com> Signed-off-by: Petr Štetiar <ynezz@true.cz> --- arch/arm/mach-ep93xx/include/mach/system.h | 18 +++++++++++++----- 1 files changed, 13 insertions(+), 5 deletions(-)