Re: [PATCH] Add watchdog driver for w90p910

[Wan ZongShun]

Dear wim,


2009/8/5 Wim Van Sebroeck <wim@xxxxxxxxx>:
> Hi Wan,
>
>> ÂI'm much obliged to you for helping me.
>
> With pleasure :-).
>
>> >> +static irqreturn_t w90p910_wdt_irq(int irq, void *dev_id)
>> >> +{
>> >> + Â Â w90p910_wdt_keepalive();
>> >> + Â Â return IRQ_HANDLED;
>> >> +}
>> >
>> > How does the interrupt work? what does the watchdog do when it times out?
>> > (Is there a datasheet available somewhere?)
>> >
>>
>> The mechanism of watchdog of w90p910 is that the interrupt will occur
>> periodicity every a given time interval.
>>
>> Hmm, there are only two choices to confirm the system running, one to
>> clear this WTIF and ÂWTR bits in handler of interrupt, the other to
>> reset them in user application before interrupt occurs.
>
> Hmm, I'm confused now. how does the watchdog then reboot your system?
> The way we implement watchdog's in linux is that you start the watchdog and that userspace needs to trigger/ping/keep-the-watchdog-alive before the timeout period is finished/over. If the userspace daemon can keep doing this (or with other words: the system is healthy enough to operate normally without to much delay) your system doesn't get rebooted. If it doesn't manage to do it, then the watchdog knows that your system is not stable/healthy enough and will reboot your system.

Thanks a lot for your reviewing again.

Good, above what you said is just apply to my watchdog.


> Here I think that the watchdog is started. Then userspace start pinging the watchdog and everything is OK. If userspace doesn't ping the watchdog in time an interrupt is being generated and instead of rebooting the system, the watchdog is being triggered... If this is indeed what it does then this is wrong.

If so, I can not do handling  the interrupt and only provide a api for
userspace to trigger/ping/keep-the-watchdog-alive.

>
>> > The read-write cyclus to REG_WTCR should also be guarded with a spinlock.
>> > Can you incorporate the call to w90p910_wdt_start() in the ioctl code into this function?
>> >
>>
>> Do you mean that I should merge the "w90p910_wdt_settimeout" to
>> "w90p910_wdt_start()"? or contrary?
>
> I mean the contrary.
> If your watchdog needs to be restarted when you set a new timeout, then it is best to have this in the settimeout function.
> It is also better to do a ping to the watchdog once the new timeout was set. It might however be that a ping is enough and that a restart is not needed.
> So something like the following diff could be it:
> --- w90p910_wdt.c    2009-08-05 06:44:14.286477908 +0000
> +++ w90p910_wdt.c    2009-08-05 06:46:04.496353102 +0000
> @@ -144,6 +144,9 @@
> Â Â Â Â__raw_writel(val, w90p910_wdt->wdt_base + REG_WTCR);
>
> Â Â Â Âwdt_time = new_time_level;
> +
> + Â Â Â w90p910_wdt_start();
> +
> Â Â Â Âreturn 0;
> Â}
>
> @@ -182,9 +185,7 @@
> Â Â Â Â Â Â Â Â Â Â Â Âreturn -EFAULT;
> Â Â Â Â Â Â Â Âif (w90p910_wdt_settimeout(new_value))
> Â Â Â Â Â Â Â Â Â Â Â Âreturn -EINVAL;
> -
> - Â Â Â Â Â Â Â w90p910_wdt_start();
> -
> + Â Â Â Â Â Â Â w90p910_wdt_keepalive();
> Â Â Â Â Â Â Â Âreturn put_user(wdt_time, p);
> Â Â Â Âcase WDIOC_GETTIMEOUT:
> Â Â Â Â Â Â Â Âreturn put_user(wdt_time, p);
> ---
>

Okay, got it.

> I have an additional question: which timeout values can you set? is it 0, 1, 2 and 3 seconds?

The 0, 1, 2 and 3 is not seconds, rather than 'level' , it means my
watchdog controller can only set four grades,
When WTIS bit of REG_WTCR was set one value of  the 0, 1, 2 and 3, the
 real time interval can be calculated via following formula .

WTIS                 real time interval (formula)
0x00                   ((2^ 14 ) * ((external crystal freq) / 256))seconds
0x01                   ((2^ 16 ) * ((external crystal freq) / 256))seconds
0x02                   ((2^ 18 ) * ((external crystal freq) / 256))seconds
0x03                   ((2^ 20 ) * ((external crystal freq) / 256))seconds

The external crystal freq is 15Mhz in w90p910 evaluation board. so I
set the '0x00' as a default value, it means the real time interval is
0.28 second approximately.



>
> Kind regards,
> Wim.
>
>



-- 
Wan z.s
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/