Re: [PATCH 2/2] watchdog: mpcore_wdt Add reload value setting for CNS3xxx hardware

[Anton Vorontsov <cbouatmailru@gmail.com>]

Hello,

thanks for the patches.

On Tue, Jul 05, 2011 at 08:05:48PM +0800, Tommy Lin wrote:
> Original MPcore watchdog setting about load register (offset 0x20) is different
> from CNS3xxx data sheet. The CNS3xxx data sheet says watchdog has following
> features:
> 1. The Watchdog Counter Register (offset 0x24) is a down counter.
> 2. The timer interval is calculated using following equation:
>    (PRESCALER_value+1) X (Load_value+1) X 2 / CPU CLK_frequency
> Thus the watchdog load register control in CNS3xxx way is add to MPcore watchdog
> source. The original control method is also kept if the CPU architecture is not
> CNS3xxx.
> 
> Signed-off-by: Tommy Lin <tommy.lin@caviumnetworks.com>
> ---
>  drivers/watchdog/mpcore_wdt.c |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/watchdog/mpcore_wdt.c b/drivers/watchdog/mpcore_wdt.c
> index 2b4af22..816e0c5 100644
> --- a/drivers/watchdog/mpcore_wdt.c
> +++ b/drivers/watchdog/mpcore_wdt.c
> @@ -42,6 +42,9 @@ struct mpcore_wdt {
>  	int		irq;
>  	unsigned int	perturb;
>  	char		expect_close;
> +#ifdef CONFIG_ARCH_CNS3XXX
> +	unsigned long	reload_unit; /* ticks per second */
> +#endif

Nope, these #ifdefs are not acceptible. Kernel should support multi-arch
images soon.

[...]
> +#ifdef CONFIG_ARCH_CNS3XXX
> +	/* Assume prescale is set to 256 */
> +	wdt->reload_unit = cns3xxx_cpu_clock() * 1000000 / 256 / 2;
> +#endif

You probably want to pass reload_unit via platform_data, and also
check (in runtime) that the watchdog is CNS3xxx. Or if reload_unit
!= 0.

Thanks,

-- 
Anton Vorontsov
Email: cbouatmailru@gmail.com