Linux-Watchdog Archive on lore.kernel.org
 help / color / Atom feed
* Samsung based S3C2440A chipset - watchdog timer issue
@ 2019-09-19  7:40 Suniel Mahesh
  2019-09-19  8:38 ` Krzysztof Kozlowski
  2019-09-19 13:21 ` Guenter Roeck
  0 siblings, 2 replies; 4+ messages in thread
From: Suniel Mahesh @ 2019-09-19  7:40 UTC (permalink / raw)
  To: kgene, krzk, wim, linux, heiko; +Cc: linux-samsung-soc, linux-watchdog

Hi,

I am working on one of the Samsung based S3C2440A chipset based target .

I have couple of questions and I request someone to shed some light on these: (Thank you)

The watchdog driver in linux, looks like, it just configured WDT but didn't start it (code snippet included below).

May i know the reasson why WDT is not started ? Is it because u-boot already started WDT, implies it is not required to do the
same once we jump into linux ? or is there any specific reason ?

drivers/watchdog/s3c2410_wdt.c (line 53 and lines 616 - 625)

#define S3C2410_WATCHDOG_ATBOOT         (0)
....
static int tmr_atboot   = S3C2410_WATCHDOG_ATBOOT;
...
...
if (tmr_atboot && started == 0) {
                dev_info(dev, "starting watchdog timer\n");
                s3c2410wdt_start(&wdt->wdt_device);
        } else if (!tmr_atboot) {
                /* if we're not enabling the watchdog, then ensure it is
                 * disabled if it has been left running from the bootloader
                 * or other source */

                s3c2410wdt_stop(&wdt->wdt_device);
        }
...
...

Tried to start WDT in linux by assigning value 1 to S3C2410_WATCHDOG_ATBOOT. The target resets.

please comment.

Thanks-- 
Suniel Mahesh

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Samsung based S3C2440A chipset - watchdog timer issue
  2019-09-19  7:40 Samsung based S3C2440A chipset - watchdog timer issue Suniel Mahesh
@ 2019-09-19  8:38 ` Krzysztof Kozlowski
  2019-09-19 13:26   ` Guenter Roeck
  2019-09-19 13:21 ` Guenter Roeck
  1 sibling, 1 reply; 4+ messages in thread
From: Krzysztof Kozlowski @ 2019-09-19  8:38 UTC (permalink / raw)
  To: Suniel Mahesh; +Cc: kgene, wim, linux, heiko, linux-samsung-soc, linux-watchdog

On Thu, 19 Sep 2019 at 09:40, Suniel Mahesh <sunil.m@techveda.org> wrote:
>
> Hi,
>
> I am working on one of the Samsung based S3C2440A chipset based target .
>
> I have couple of questions and I request someone to shed some light on these: (Thank you)
>
> The watchdog driver in linux, looks like, it just configured WDT but didn't start it (code snippet included below).
>
> May i know the reasson why WDT is not started ? Is it because u-boot already started WDT, implies it is not required to do the
> same once we jump into linux ? or is there any specific reason ?
>
> drivers/watchdog/s3c2410_wdt.c (line 53 and lines 616 - 625)
>
> #define S3C2410_WATCHDOG_ATBOOT         (0)
> ....
> static int tmr_atboot   = S3C2410_WATCHDOG_ATBOOT;
> ...
> ...
> if (tmr_atboot && started == 0) {
>                 dev_info(dev, "starting watchdog timer\n");
>                 s3c2410wdt_start(&wdt->wdt_device);
>         } else if (!tmr_atboot) {
>                 /* if we're not enabling the watchdog, then ensure it is
>                  * disabled if it has been left running from the bootloader
>                  * or other source */
>
>                 s3c2410wdt_stop(&wdt->wdt_device);
>         }
> ...
> ...
>
> Tried to start WDT in linux by assigning value 1 to S3C2410_WATCHDOG_ATBOOT. The target resets.
>
> please comment.

I think watchdog should not start during boot before user-space is
brought up. Otherwise who will ping it? Usually watchdog is started by
opening the watchdog device by user-space. If you need it to be
running without user-space, there are special flags for this (see
WDOG_HW_RUNNING and others).

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Samsung based S3C2440A chipset - watchdog timer issue
  2019-09-19  7:40 Samsung based S3C2440A chipset - watchdog timer issue Suniel Mahesh
  2019-09-19  8:38 ` Krzysztof Kozlowski
@ 2019-09-19 13:21 ` Guenter Roeck
  1 sibling, 0 replies; 4+ messages in thread
From: Guenter Roeck @ 2019-09-19 13:21 UTC (permalink / raw)
  To: Suniel Mahesh, kgene, krzk, wim, heiko; +Cc: linux-samsung-soc, linux-watchdog

On 9/19/19 12:40 AM, Suniel Mahesh wrote:
> Hi,
> 
> I am working on one of the Samsung based S3C2440A chipset based target .
> 
> I have couple of questions and I request someone to shed some light on these: (Thank you)
> 
> The watchdog driver in linux, looks like, it just configured WDT but didn't start it (code snippet included below).
> 
> May i know the reasson why WDT is not started ? Is it because u-boot already started WDT, implies it is not required to do the
> same once we jump into linux ? or is there any specific reason ?
> 

The driver does not detect if the watchdog is not running when probed.
What u-boot does is irrelevant. To start the watchdog at boot, you would
have to provide the tmr_atboot=1 module parameter. Otherwise, it is explicitly
disabled, as shown in the code, and opening the watchdog device would start
the watchdog.

Having said that, the code is less than perfect. Initializing the timeout
in the probe function should be handled differently, and the kernel should
be informed that/if the watchdog is started at boot. It should also be
possible to detect if the watchdog is running instead of relying on tmr_atboot,
and inform the kernel accordingly. But those are not bugs, just possible
improvements.

Guenter

> drivers/watchdog/s3c2410_wdt.c (line 53 and lines 616 - 625)
> 
> #define S3C2410_WATCHDOG_ATBOOT         (0)
> ....
> static int tmr_atboot   = S3C2410_WATCHDOG_ATBOOT;
> ...
> ...
> if (tmr_atboot && started == 0) {
>                  dev_info(dev, "starting watchdog timer\n");
>                  s3c2410wdt_start(&wdt->wdt_device);
>          } else if (!tmr_atboot) {
>                  /* if we're not enabling the watchdog, then ensure it is
>                   * disabled if it has been left running from the bootloader
>                   * or other source */
> 
>                  s3c2410wdt_stop(&wdt->wdt_device);
>          }
> ...
> ...
> 
> Tried to start WDT in linux by assigning value 1 to S3C2410_WATCHDOG_ATBOOT. The target resets.
> 
> please comment.
> 
> Thanks--
> Suniel Mahesh
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Samsung based S3C2440A chipset - watchdog timer issue
  2019-09-19  8:38 ` Krzysztof Kozlowski
@ 2019-09-19 13:26   ` Guenter Roeck
  0 siblings, 0 replies; 4+ messages in thread
From: Guenter Roeck @ 2019-09-19 13:26 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Suniel Mahesh
  Cc: kgene, wim, heiko, linux-samsung-soc, linux-watchdog

On 9/19/19 1:38 AM, Krzysztof Kozlowski wrote:
> On Thu, 19 Sep 2019 at 09:40, Suniel Mahesh <sunil.m@techveda.org> wrote:
>>
>> Hi,
>>
>> I am working on one of the Samsung based S3C2440A chipset based target .
>>
>> I have couple of questions and I request someone to shed some light on these: (Thank you)
>>
>> The watchdog driver in linux, looks like, it just configured WDT but didn't start it (code snippet included below).
>>
>> May i know the reasson why WDT is not started ? Is it because u-boot already started WDT, implies it is not required to do the
>> same once we jump into linux ? or is there any specific reason ?
>>
>> drivers/watchdog/s3c2410_wdt.c (line 53 and lines 616 - 625)
>>
>> #define S3C2410_WATCHDOG_ATBOOT         (0)
>> ....
>> static int tmr_atboot   = S3C2410_WATCHDOG_ATBOOT;
>> ...
>> ...
>> if (tmr_atboot && started == 0) {
>>                  dev_info(dev, "starting watchdog timer\n");
>>                  s3c2410wdt_start(&wdt->wdt_device);
>>          } else if (!tmr_atboot) {
>>                  /* if we're not enabling the watchdog, then ensure it is
>>                   * disabled if it has been left running from the bootloader
>>                   * or other source */
>>
>>                  s3c2410wdt_stop(&wdt->wdt_device);
>>          }
>> ...
>> ...
>>
>> Tried to start WDT in linux by assigning value 1 to S3C2410_WATCHDOG_ATBOOT. The target resets.
>>
>> please comment.
> 
> I think watchdog should not start during boot before user-space is
> brought up. Otherwise who will ping it? Usually watchdog is started by
> opening the watchdog device by user-space. If you need it to be
> running without user-space, there are special flags for this (see
> WDOG_HW_RUNNING and others).
> 
That is not entirely correct. There are use cases where the watchdog
is started on purpose and userspace has to open it within a specified
period of time to prevent the system from rebooting. Presumably this is
what the current driver tries to implement. WDOG_HW_RUNNING was added
to the core after the driver was written, so it is not surprising that
the driver does not support it. Patches welcome.

Thanks,
Guenter

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-19  7:40 Samsung based S3C2440A chipset - watchdog timer issue Suniel Mahesh
2019-09-19  8:38 ` Krzysztof Kozlowski
2019-09-19 13:26   ` Guenter Roeck
2019-09-19 13:21 ` Guenter Roeck

Linux-Watchdog Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-watchdog/0 linux-watchdog/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-watchdog linux-watchdog/ https://lore.kernel.org/linux-watchdog \
		linux-watchdog@vger.kernel.org linux-watchdog@archiver.kernel.org
	public-inbox-index linux-watchdog

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-watchdog


AGPL code for this site: git clone https://public-inbox.org/ public-inbox