diff mbox

[U-Boot,AT91] Add watchdog reset to the at91samx SOC family

Message ID 20110315112717.GA24904@gandalf.sssup.it
State Superseded
Delegated to: Reinhard Meyer
Headers show

Commit Message

michael March 15, 2011, 11:27 a.m. UTC 
This patch add the watchdog reset function to the atmel atsam9x architectures.

Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
Cc: Reinhard Meyer <u-boot@emk-elektronik.de>

---

Comments

Reinhard Meyer March 15, 2011, 12:01 p.m. UTC | #1 
Dear Michael Trimarchi,
> This patch add the watchdog reset function to the atmel atsam9x architectures.

NAK.

Watchdog already exists at drivers/watchdog/at91sam9_wdt.c :)

Maybe its a bit of an obscure location... But it is already invented ;)

> +	int re_enable = disable_interrupts();
> +
> +	writel(AT91_WDT_CR_WDRSTT | AT91_WDT_CR_KEY, &wdt->cr);
> +
> +	if (re_enable)
> +		enable_interrupts();

Besides... why would we need to disable IRQs here, writel() is using one instruction
to write to the address?

Best Regards,

Reinhard
michael March 15, 2011, 12:09 p.m. UTC | #2 
HI

On 03/15/2011 01:01 PM, Reinhard Meyer wrote:
> Dear Michael Trimarchi,
>> This patch add the watchdog reset function to the atmel atsam9x architectures.
> 
> NAK.
> 
> Watchdog already exists at drivers/watchdog/at91sam9_wdt.c :)
> 
> Maybe its a bit of an obscure location... But it is already invented ;)
> 
>> +	int re_enable = disable_interrupts();
>> +
>> +	writel(AT91_WDT_CR_WDRSTT | AT91_WDT_CR_KEY, &wdt->cr);
>> +
>> +	if (re_enable)
>> +		enable_interrupts();
> 

Yes was obscured, and it is the only one :) there

Michael

> Besides... why would we need to disable IRQs here, writel() is using one instruction
> to write to the address?
> 
> Best Regards,
> 
> Reinhard
>
michael March 15, 2011, 12:50 p.m. UTC | #3 
Hi

On 03/15/2011 01:01 PM, Reinhard Meyer wrote:
> Dear Michael Trimarchi,
>> This patch add the watchdog reset function to the atmel atsam9x architectures.
> 
> NAK.
> 
> Watchdog already exists at drivers/watchdog/at91sam9_wdt.c :)
> 

I'm getting confused because I have seen other implementation of the watchdog reset function and I was thinking
that was the correct location of the function. Some board initialize it in the sturtup.

> Maybe its a bit of an obscure location... But it is already invented ;)
> 
>> +	int re_enable = disable_interrupts();
>> +
>> +	writel(AT91_WDT_CR_WDRSTT | AT91_WDT_CR_KEY, &wdt->cr);
>> +
>> +	if (re_enable)
>> +		enable_interrupts();
> 
> Besides... why would we need to disable IRQs here, writel() is using one instruction
> to write to the address?
> 

:(, Ok, I'm porting a new board and I have done it too fase without checking to much the code
but the final result. Anyway I have seen the driver but I don't understand how the HW_WATCHDOG interface match the hw_watchdog_init.


Michael Trimarchi


> Best Regards,
> 
> Reinhard
>
diff mbox

Patch

diff --git a/arch/arm/cpu/arm926ejs/at91/timer.c b/arch/arm/cpu/arm926ejs/at91/timer.c
index 82b8d7e..2e4ff02 100644
--- a/arch/arm/cpu/arm926ejs/at91/timer.c
+++ b/arch/arm/cpu/arm926ejs/at91/timer.c
@@ -26,6 +26,7 @@ 
 #include <asm/arch/hardware.h>
 #include <asm/arch/at91_pit.h>
 #include <asm/arch/at91_pmc.h>
+#include <asm/arch/at91_wdt.h>
 #include <asm/arch/clk.h>
 #include <asm/arch/io.h>
 #include <div64.h>
@@ -139,3 +140,18 @@  ulong get_tbclk(void)
 {
 	return gd->timer_rate_hz;
 }
+
+#if defined(CONFIG_WATCHDOG)
+
+void watchdog_reset(void)
+{
+	at91_wdt_t *wdt = (at91_wdt_t *) AT91_WDT_BASE;
+	int re_enable = disable_interrupts();
+
+	writel(AT91_WDT_CR_WDRSTT | AT91_WDT_CR_KEY, &wdt->cr);
+
+	if (re_enable)
+		enable_interrupts();
+}
+
+#endif