-----Original Message----- From: Simon Kagstrom [mailto:simon.kagstrom@netinsight.net] Sent: Tuesday, September 29, 2009 1:59 PM To: Prafulla Wadaskar Cc: u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH] arm:kirkwood: Add hardware watchdog support for Marvell Kirkwood boards
On Mon, 28 Sep 2009 19:16:16 -0700 Prafulla Wadaskar prafulla@marvell.com wrote:
But do you really need Watchdog support for u-boot?
Paranoia really has no limits :-). The main objective for me personally is to have the watchdog on when Linux starts, but if there is a risk (for whatever reason) that U-boot hangs, it would also help there.
Its good to have watchdog, it will be very useful for some applications. But I don't think we should do it at u-boot level. Secondly If it is supported on Kirkwood platforms in Linux, then the same can be triggered from OS too.
In u-boot I didn't find much watchdog implementation for other arm architectures.
Because if you want to use the watchdog, you will need to
keep it running in
Entire code.
That's fine, the code is already sprinkled with WATCHDOG_RESET() in many places (which calls hw_watchdog_reset()). Also note that this patch doesn't actually turn it on, you'll have to call kw_watchdog_init() first to do that.
newly added code for Kirkwood may not, we need to check and add
Secondly Pls have a look at drivers/watchdog/at91sam9_wdt.c
and its implementation,
This will be a good way of implementation.
Well, I did look at that, and I believe the implementation is fairly similar.
I think you should follow the same method to keep it as "add Kirkwood watchdog driver"
What I wonder about in that context is the use of hw_watchdog_init(). I first thought this was generic, but it's not exported via watchdog.h (like hw_watchdog_reset()). I think it would be nice to have a generic interface which exports
void hw_watchdog_init(unsigned long timeout_ms);
to initialize the watchdog and timeout. The timeout would be a bit crude since hardware have limits to how long the timeouts would be, but anyway.
Another good feature would be a command-line interface to turn it on and configure it, i.e., something like
watchdog on 5000 # Set timeout to 5000 ms watchdog off # Turn off (if possible)
These are good enhancement, you may post these generic patches too :-)
Regards.. Prafulla . .
// Simon