[U-Boot] [PATCH] arm:kirkwood: Add hardware watchdog support for Marvell Kirkwood boards

[Prafulla Wadaskar]

> -----Original Message-----
> From: Simon Kagstrom [mailto:simon.kagstrom at netinsight.net] 
> Sent: Wednesday, October 28, 2009 6:14 PM
> To: Prafulla Wadaskar
> Cc: u-boot at lists.denx.de
> Subject: Re: [PATCH] arm:kirkwood: Add hardware watchdog 
> support for Marvell Kirkwood boards
> 
> On Wed, 28 Oct 2009 04:34:10 -0700
> Prafulla Wadaskar <prafulla at marvell.com> wrote:
> 
> > >    #define CNTMR_VAL_REG(tmrnum)		
> > > &kwtmr_regs->tmr[tmrnum].val
> > > 
> > > and used for the regular timer support. I'm not sure I 
> like that, but
> > > at least the file should be internally consistent.
> > 
> > You can update the structure to use WDT timer in the same 
> way as other timers,
> > there is no sense putting additional names in structure.
> 
> But I'm not - the WDT timer is used in the same way as the 
> other timer.

So the kwtmr_register structure clean up can be a separate patch.

> The only difference is the added WATCHDOG_TMR define which specifies
> which Kirkwood timer to use as a watchdog.

Ack

> 
> > > But then it's unconditionally turned on as soon as the first
> > > WATCHDOG_RESET() is called, which might not be what you want.
> > > 
> > > In the long run, we should probably add command line support for
> > > enabling the watchdog (some might want to do it just 
> before starting
> > > Linux for example).
> > 
> > You can even call WATCHDOG_RESET() from wherever from your 
> code to enable it
> 
> Sure, but WATCHDOG_RESET() will be called anyway (and probably before
> my code), so it will be enabled anyhow in that case. My point is that
> sometimes you don't want the watchdog to get started directly, hence
> the function to enable it.

That is also valid point,
This will be the generic need for all architectures.
Lets introduce WATCHDOG_INIT() as new generic interface.

What do you think?

Regards..
Prafulla . .

> 
> // Simon
>