* [PATCH 0/2] GPIO WDT "start-at-boot" property @ 2021-04-21 16:26 Francesco Zanella 2021-04-21 16:26 ` [PATCH 1/2] dt-bindings: watchdog: gpio-wdt: add "start-at-boot" Francesco Zanella 2021-04-21 16:26 ` [PATCH 2/2] watchdog: gpio_wdt: add "start-at-boot" feature Francesco Zanella 0 siblings, 2 replies; 11+ messages in thread From: Francesco Zanella @ 2021-04-21 16:26 UTC (permalink / raw) To: linux-watchdog, wim, linux Cc: devicetree, robh+dt, linux-kernel, Francesco Zanella I propose to add a new property to GPIO WDT device tree binding "start-at-boot". If this property is present, start pinging hw watchdog at probe, in order to take advantage of kernel configs: - WATCHDOG_HANDLE_BOOT_ENABLED: Avoid possible reboot if hw watchdog was been enabled before the kernel (by uboot for example) and userspace doesn't take control of /dev/watchdog in time; - WATCHDOG_OPEN_TIMEOUT: Reboot if userspace doesn't take control of /dev/watchdog within the timeout. Francesco Zanella (2): dt-bindings: watchdog: gpio-wdt: add "start-at-boot" watchdog: gpio_wdt: add "start-at-boot" feature Documentation/devicetree/bindings/watchdog/gpio-wdt.txt | 7 +++++++ drivers/watchdog/gpio_wdt.c | 6 +++++- 2 files changed, 12 insertions(+), 1 deletion(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] dt-bindings: watchdog: gpio-wdt: add "start-at-boot" 2021-04-21 16:26 [PATCH 0/2] GPIO WDT "start-at-boot" property Francesco Zanella @ 2021-04-21 16:26 ` Francesco Zanella 2021-04-21 16:37 ` Guenter Roeck 2021-04-21 16:26 ` [PATCH 2/2] watchdog: gpio_wdt: add "start-at-boot" feature Francesco Zanella 1 sibling, 1 reply; 11+ messages in thread From: Francesco Zanella @ 2021-04-21 16:26 UTC (permalink / raw) To: linux-watchdog, wim, linux Cc: devicetree, robh+dt, linux-kernel, Francesco Zanella Documentation for new device tree property "start-at-boot". Signed-off-by: Francesco Zanella <francesco.zanella@vimar.com> --- Documentation/devicetree/bindings/watchdog/gpio-wdt.txt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt b/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt index 198794963786..cdaf7f0602e8 100644 --- a/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt +++ b/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt @@ -17,6 +17,13 @@ Optional Properties: - always-running: If the watchdog timer cannot be disabled, add this flag to have the driver keep toggling the signal without a client. It will only cease to toggle the signal when the device is open and the timeout elapsed. +- start-at-boot: Start pinging hw watchdog at probe, in order to take advantage + of kernel configs: + - WATCHDOG_HANDLE_BOOT_ENABLED: Avoid possible reboot if hw watchdog was been + enabled before the kernel (by uboot for example) and userspace doesn't take + control of /dev/watchdog in time; + - WATCHDOG_OPEN_TIMEOUT: Reboot if userspace doesn't take control of + /dev/watchdog within the timeout. Example: watchdog: watchdog { -- 2.17.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dt-bindings: watchdog: gpio-wdt: add "start-at-boot" 2021-04-21 16:26 ` [PATCH 1/2] dt-bindings: watchdog: gpio-wdt: add "start-at-boot" Francesco Zanella @ 2021-04-21 16:37 ` Guenter Roeck 2021-04-22 16:28 ` Francesco Zanella 0 siblings, 1 reply; 11+ messages in thread From: Guenter Roeck @ 2021-04-21 16:37 UTC (permalink / raw) To: Francesco Zanella; +Cc: linux-watchdog, wim, devicetree, robh+dt, linux-kernel On Wed, Apr 21, 2021 at 06:26:20PM +0200, Francesco Zanella wrote: > Documentation for new device tree property "start-at-boot". > > Signed-off-by: Francesco Zanella <francesco.zanella@vimar.com> > --- > Documentation/devicetree/bindings/watchdog/gpio-wdt.txt | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt b/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt > index 198794963786..cdaf7f0602e8 100644 > --- a/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt > +++ b/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt > @@ -17,6 +17,13 @@ Optional Properties: > - always-running: If the watchdog timer cannot be disabled, add this flag to > have the driver keep toggling the signal without a client. It will only cease > to toggle the signal when the device is open and the timeout elapsed. > +- start-at-boot: Start pinging hw watchdog at probe, in order to take advantage > + of kernel configs: > + - WATCHDOG_HANDLE_BOOT_ENABLED: Avoid possible reboot if hw watchdog was been > + enabled before the kernel (by uboot for example) and userspace doesn't take > + control of /dev/watchdog in time; > + - WATCHDOG_OPEN_TIMEOUT: Reboot if userspace doesn't take control of > + /dev/watchdog within the timeout. You are not supposed to refer to Linux kernel details in devicetree bindings documents. Guenter > > Example: > watchdog: watchdog { > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dt-bindings: watchdog: gpio-wdt: add "start-at-boot" 2021-04-21 16:37 ` Guenter Roeck @ 2021-04-22 16:28 ` Francesco Zanella 0 siblings, 0 replies; 11+ messages in thread From: Francesco Zanella @ 2021-04-22 16:28 UTC (permalink / raw) To: Guenter Roeck; +Cc: linux-watchdog, wim, devicetree, robh+dt, linux-kernel On 21/04/21 18:37, Guenter Roeck wrote: > On Wed, Apr 21, 2021 at 06:26:20PM +0200, Francesco Zanella wrote: >> Documentation for new device tree property "start-at-boot". >> >> Signed-off-by: Francesco Zanella <francesco.zanella@vimar.com> >> --- >> Documentation/devicetree/bindings/watchdog/gpio-wdt.txt | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt b/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt >> index 198794963786..cdaf7f0602e8 100644 >> --- a/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt >> +++ b/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt >> @@ -17,6 +17,13 @@ Optional Properties: >> - always-running: If the watchdog timer cannot be disabled, add this flag to >> have the driver keep toggling the signal without a client. It will only cease >> to toggle the signal when the device is open and the timeout elapsed. >> +- start-at-boot: Start pinging hw watchdog at probe, in order to take advantage >> + of kernel configs: >> + - WATCHDOG_HANDLE_BOOT_ENABLED: Avoid possible reboot if hw watchdog was been >> + enabled before the kernel (by uboot for example) and userspace doesn't take >> + control of /dev/watchdog in time; >> + - WATCHDOG_OPEN_TIMEOUT: Reboot if userspace doesn't take control of >> + /dev/watchdog within the timeout. > > You are not supposed to refer to Linux kernel details in devicetree > bindings documents. > > Guenter > >> >> Example: >> watchdog: watchdog { >> -- >> 2.17.1 >> OK, I'm sorry. I will resend the patch series without kernel configs references in documents. Francesco ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] watchdog: gpio_wdt: add "start-at-boot" feature 2021-04-21 16:26 [PATCH 0/2] GPIO WDT "start-at-boot" property Francesco Zanella 2021-04-21 16:26 ` [PATCH 1/2] dt-bindings: watchdog: gpio-wdt: add "start-at-boot" Francesco Zanella @ 2021-04-21 16:26 ` Francesco Zanella 2021-04-21 16:42 ` Guenter Roeck 2021-04-23 11:36 ` Rasmus Villemoes 1 sibling, 2 replies; 11+ messages in thread From: Francesco Zanella @ 2021-04-21 16:26 UTC (permalink / raw) To: linux-watchdog, wim, linux Cc: devicetree, robh+dt, linux-kernel, Francesco Zanella If "start-at-boot" property is present in the device tree, start pinging hw watchdog at probe, in order to take advantage of kernel configs: - WATCHDOG_HANDLE_BOOT_ENABLED: Avoid possible reboot if hw watchdog was been enabled before the kernel (by uboot for example) and userspace doesn't take control of /dev/watchdog in time; - WATCHDOG_OPEN_TIMEOUT: Reboot if userspace doesn't take control of /dev/watchdog within the timeout. Signed-off-by: Francesco Zanella <francesco.zanella@vimar.com> --- drivers/watchdog/gpio_wdt.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c index 0923201ce874..1e6f0322ab7a 100644 --- a/drivers/watchdog/gpio_wdt.c +++ b/drivers/watchdog/gpio_wdt.c @@ -31,6 +31,7 @@ struct gpio_wdt_priv { struct gpio_desc *gpiod; bool state; bool always_running; + bool start_at_boot; unsigned int hw_algo; struct watchdog_device wdd; }; @@ -147,6 +148,9 @@ static int gpio_wdt_probe(struct platform_device *pdev) priv->always_running = of_property_read_bool(np, "always-running"); + priv->start_at_boot = of_property_read_bool(np, + "start-at-boot"); + watchdog_set_drvdata(&priv->wdd, priv); priv->wdd.info = &gpio_wdt_ident; @@ -161,7 +165,7 @@ static int gpio_wdt_probe(struct platform_device *pdev) watchdog_stop_on_reboot(&priv->wdd); - if (priv->always_running) + if (priv->always_running || priv->start_at_boot) gpio_wdt_start(&priv->wdd); return devm_watchdog_register_device(dev, &priv->wdd); -- 2.17.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] watchdog: gpio_wdt: add "start-at-boot" feature 2021-04-21 16:26 ` [PATCH 2/2] watchdog: gpio_wdt: add "start-at-boot" feature Francesco Zanella @ 2021-04-21 16:42 ` Guenter Roeck 2021-04-22 16:28 ` Francesco Zanella 2021-04-23 11:36 ` Rasmus Villemoes 1 sibling, 1 reply; 11+ messages in thread From: Guenter Roeck @ 2021-04-21 16:42 UTC (permalink / raw) To: Francesco Zanella; +Cc: linux-watchdog, wim, devicetree, robh+dt, linux-kernel On Wed, Apr 21, 2021 at 06:26:21PM +0200, Francesco Zanella wrote: > If "start-at-boot" property is present in the device tree, start pinging > hw watchdog at probe, in order to take advantage of kernel configs: > - WATCHDOG_HANDLE_BOOT_ENABLED: Avoid possible reboot if hw watchdog was > been enabled before the kernel (by uboot for example) and userspace > doesn't take control of /dev/watchdog in time; > - WATCHDOG_OPEN_TIMEOUT: Reboot if userspace doesn't take control of > /dev/watchdog within the timeout. > > Signed-off-by: Francesco Zanella <francesco.zanella@vimar.com> > --- > drivers/watchdog/gpio_wdt.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c > index 0923201ce874..1e6f0322ab7a 100644 > --- a/drivers/watchdog/gpio_wdt.c > +++ b/drivers/watchdog/gpio_wdt.c > @@ -31,6 +31,7 @@ struct gpio_wdt_priv { > struct gpio_desc *gpiod; > bool state; > bool always_running; > + bool start_at_boot; > unsigned int hw_algo; > struct watchdog_device wdd; > }; > @@ -147,6 +148,9 @@ static int gpio_wdt_probe(struct platform_device *pdev) > priv->always_running = of_property_read_bool(np, > "always-running"); > > + priv->start_at_boot = of_property_read_bool(np, > + "start-at-boot"); > + > watchdog_set_drvdata(&priv->wdd, priv); > > priv->wdd.info = &gpio_wdt_ident; > @@ -161,7 +165,7 @@ static int gpio_wdt_probe(struct platform_device *pdev) > > watchdog_stop_on_reboot(&priv->wdd); > > - if (priv->always_running) > + if (priv->always_running || priv->start_at_boot) > gpio_wdt_start(&priv->wdd); So the only real difference to always_running is that always_running doesn't stop the watchdog on close but keeps it running. Does that really warrant another property ? Why not just use always-running ? The special use case of being able to stop the watchdog doesn't seem to be worth the trouble. Please explain your use case. Guenter ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] watchdog: gpio_wdt: add "start-at-boot" feature 2021-04-21 16:42 ` Guenter Roeck @ 2021-04-22 16:28 ` Francesco Zanella 2021-04-22 18:05 ` Guenter Roeck 0 siblings, 1 reply; 11+ messages in thread From: Francesco Zanella @ 2021-04-22 16:28 UTC (permalink / raw) To: Guenter Roeck; +Cc: linux-watchdog, wim, devicetree, robh+dt, linux-kernel On 21/04/21 18:42, Guenter Roeck wrote: > On Wed, Apr 21, 2021 at 06:26:21PM +0200, Francesco Zanella wrote: >> If "start-at-boot" property is present in the device tree, start pinging >> hw watchdog at probe, in order to take advantage of kernel configs: >> - WATCHDOG_HANDLE_BOOT_ENABLED: Avoid possible reboot if hw watchdog was >> been enabled before the kernel (by uboot for example) and userspace >> doesn't take control of /dev/watchdog in time; >> - WATCHDOG_OPEN_TIMEOUT: Reboot if userspace doesn't take control of >> /dev/watchdog within the timeout. >> >> Signed-off-by: Francesco Zanella <francesco.zanella@vimar.com> >> --- >> drivers/watchdog/gpio_wdt.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c >> index 0923201ce874..1e6f0322ab7a 100644 >> --- a/drivers/watchdog/gpio_wdt.c >> +++ b/drivers/watchdog/gpio_wdt.c >> @@ -31,6 +31,7 @@ struct gpio_wdt_priv { >> struct gpio_desc *gpiod; >> bool state; >> bool always_running; >> + bool start_at_boot; >> unsigned int hw_algo; >> struct watchdog_device wdd; >> }; >> @@ -147,6 +148,9 @@ static int gpio_wdt_probe(struct platform_device *pdev) >> priv->always_running = of_property_read_bool(np, >> "always-running"); >> >> + priv->start_at_boot = of_property_read_bool(np, >> + "start-at-boot"); >> + >> watchdog_set_drvdata(&priv->wdd, priv); >> >> priv->wdd.info = &gpio_wdt_ident; >> @@ -161,7 +165,7 @@ static int gpio_wdt_probe(struct platform_device *pdev) >> >> watchdog_stop_on_reboot(&priv->wdd); >> >> - if (priv->always_running) >> + if (priv->always_running || priv->start_at_boot) >> gpio_wdt_start(&priv->wdd); > > So the only real difference to always_running is that always_running > doesn't stop the watchdog on close but keeps it running. > > Does that really warrant another property ? Why not just use > always-running ? > > The special use case of being able to stop the watchdog doesn't seem > to be worth the trouble. Please explain your use case. > > Guenter > You got the point. I would like to be able to stop the watchdog when I want. From my point of view always_running is used in the very special case when the wdt chip can't be stopped. I want a normal wdt that can be stopped (for example during a system update), but I want it to start at boot for the 2 reasons that I explained before: - I need continuity in hw wdt pinging because I start in uboot, so I take advantage of WATCHDOG_HANDLE_BOOT_ENABLED that makes the kernel to do that job; but it is triggered only if it is started at probe, if I'm not wrong. - I would like to monitor with the wdt also the kernel at boot, and more importantly handle the case when the userspace app doesn't take control of /dev/watchdog for whatever reason within the timeout set with WATCHDOG_OPEN_TIMEOUT. Hope that this explain my target. Thanks for your attention, Francesco ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] watchdog: gpio_wdt: add "start-at-boot" feature 2021-04-22 16:28 ` Francesco Zanella @ 2021-04-22 18:05 ` Guenter Roeck 2021-04-23 9:47 ` Francesco Zanella 0 siblings, 1 reply; 11+ messages in thread From: Guenter Roeck @ 2021-04-22 18:05 UTC (permalink / raw) To: Francesco Zanella; +Cc: linux-watchdog, wim, devicetree, robh+dt, linux-kernel On Thu, Apr 22, 2021 at 06:28:40PM +0200, Francesco Zanella wrote: > > > On 21/04/21 18:42, Guenter Roeck wrote: > > On Wed, Apr 21, 2021 at 06:26:21PM +0200, Francesco Zanella wrote: > >> If "start-at-boot" property is present in the device tree, start pinging > >> hw watchdog at probe, in order to take advantage of kernel configs: > >> - WATCHDOG_HANDLE_BOOT_ENABLED: Avoid possible reboot if hw watchdog was > >> been enabled before the kernel (by uboot for example) and userspace > >> doesn't take control of /dev/watchdog in time; > >> - WATCHDOG_OPEN_TIMEOUT: Reboot if userspace doesn't take control of > >> /dev/watchdog within the timeout. > >> > >> Signed-off-by: Francesco Zanella <francesco.zanella@vimar.com> > >> --- > >> drivers/watchdog/gpio_wdt.c | 6 +++++- > >> 1 file changed, 5 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c > >> index 0923201ce874..1e6f0322ab7a 100644 > >> --- a/drivers/watchdog/gpio_wdt.c > >> +++ b/drivers/watchdog/gpio_wdt.c > >> @@ -31,6 +31,7 @@ struct gpio_wdt_priv { > >> struct gpio_desc *gpiod; > >> bool state; > >> bool always_running; > >> + bool start_at_boot; > >> unsigned int hw_algo; > >> struct watchdog_device wdd; > >> }; > >> @@ -147,6 +148,9 @@ static int gpio_wdt_probe(struct platform_device *pdev) > >> priv->always_running = of_property_read_bool(np, > >> "always-running"); > >> > >> + priv->start_at_boot = of_property_read_bool(np, > >> + "start-at-boot"); > >> + > >> watchdog_set_drvdata(&priv->wdd, priv); > >> > >> priv->wdd.info = &gpio_wdt_ident; > >> @@ -161,7 +165,7 @@ static int gpio_wdt_probe(struct platform_device *pdev) > >> > >> watchdog_stop_on_reboot(&priv->wdd); > >> > >> - if (priv->always_running) > >> + if (priv->always_running || priv->start_at_boot) > >> gpio_wdt_start(&priv->wdd); > > > > So the only real difference to always_running is that always_running > > doesn't stop the watchdog on close but keeps it running. > > > > Does that really warrant another property ? Why not just use > > always-running ? > > > > The special use case of being able to stop the watchdog doesn't seem > > to be worth the trouble. Please explain your use case. > > > > Guenter > > > > You got the point. > I would like to be able to stop the watchdog when I want. > From my point of view always_running is used in the very special > case when the wdt chip can't be stopped. > I want a normal wdt that can be stopped (for example during a system > update), but I want it to start at boot for the 2 reasons that I > explained before: > - I need continuity in hw wdt pinging because I start in uboot, > so I take advantage of WATCHDOG_HANDLE_BOOT_ENABLED that makes > the kernel to do that job; but it is triggered only if it is > started at probe, if I'm not wrong. That depends. If the driver can read the current state (ie if it can detect if the watchdog is running) it can report it to the watchdog core accordingly. That would be the preferred mechanism. Everything else is just a workaround for bad hardware (bad as in "it doesn't report its state"). Guenter > - I would like to monitor with the wdt also the kernel at boot, > and more importantly handle the case when the userspace app > doesn't take control of /dev/watchdog for whatever reason > within the timeout set with WATCHDOG_OPEN_TIMEOUT. > > Hope that this explain my target. > Thanks for your attention, > > Francesco ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] watchdog: gpio_wdt: add "start-at-boot" feature 2021-04-22 18:05 ` Guenter Roeck @ 2021-04-23 9:47 ` Francesco Zanella 0 siblings, 0 replies; 11+ messages in thread From: Francesco Zanella @ 2021-04-23 9:47 UTC (permalink / raw) To: Guenter Roeck Cc: linux-watchdog, wim, devicetree, robh+dt, linux-kernel, francesco.zanella On 22/04/21 20:05, Guenter Roeck wrote: > On Thu, Apr 22, 2021 at 06:28:40PM +0200, Francesco Zanella wrote: >> >> >> On 21/04/21 18:42, Guenter Roeck wrote: >>> On Wed, Apr 21, 2021 at 06:26:21PM +0200, Francesco Zanella wrote: >>>> If "start-at-boot" property is present in the device tree, start pinging >>>> hw watchdog at probe, in order to take advantage of kernel configs: >>>> - WATCHDOG_HANDLE_BOOT_ENABLED: Avoid possible reboot if hw watchdog was >>>> been enabled before the kernel (by uboot for example) and userspace >>>> doesn't take control of /dev/watchdog in time; >>>> - WATCHDOG_OPEN_TIMEOUT: Reboot if userspace doesn't take control of >>>> /dev/watchdog within the timeout. >>>> >>>> Signed-off-by: Francesco Zanella <francesco.zanella@vimar.com> >>>> --- >>>> drivers/watchdog/gpio_wdt.c | 6 +++++- >>>> 1 file changed, 5 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c >>>> index 0923201ce874..1e6f0322ab7a 100644 >>>> --- a/drivers/watchdog/gpio_wdt.c >>>> +++ b/drivers/watchdog/gpio_wdt.c >>>> @@ -31,6 +31,7 @@ struct gpio_wdt_priv { >>>> struct gpio_desc *gpiod; >>>> bool state; >>>> bool always_running; >>>> + bool start_at_boot; >>>> unsigned int hw_algo; >>>> struct watchdog_device wdd; >>>> }; >>>> @@ -147,6 +148,9 @@ static int gpio_wdt_probe(struct platform_device *pdev) >>>> priv->always_running = of_property_read_bool(np, >>>> "always-running"); >>>> >>>> + priv->start_at_boot = of_property_read_bool(np, >>>> + "start-at-boot"); >>>> + >>>> watchdog_set_drvdata(&priv->wdd, priv); >>>> >>>> priv->wdd.info = &gpio_wdt_ident; >>>> @@ -161,7 +165,7 @@ static int gpio_wdt_probe(struct platform_device *pdev) >>>> >>>> watchdog_stop_on_reboot(&priv->wdd); >>>> >>>> - if (priv->always_running) >>>> + if (priv->always_running || priv->start_at_boot) >>>> gpio_wdt_start(&priv->wdd); >>> >>> So the only real difference to always_running is that always_running >>> doesn't stop the watchdog on close but keeps it running. >>> >>> Does that really warrant another property ? Why not just use >>> always-running ? >>> >>> The special use case of being able to stop the watchdog doesn't seem >>> to be worth the trouble. Please explain your use case. >>> >>> Guenter >>> >> >> You got the point. >> I would like to be able to stop the watchdog when I want. >> From my point of view always_running is used in the very special >> case when the wdt chip can't be stopped. >> I want a normal wdt that can be stopped (for example during a system >> update), but I want it to start at boot for the 2 reasons that I >> explained before: >> - I need continuity in hw wdt pinging because I start in uboot, >> so I take advantage of WATCHDOG_HANDLE_BOOT_ENABLED that makes >> the kernel to do that job; but it is triggered only if it is >> started at probe, if I'm not wrong. > > That depends. If the driver can read the current state (ie if > it can detect if the watchdog is running) it can report it > to the watchdog core accordingly. That would be the preferred > mechanism. Everything else is just a workaround for bad hardware > (bad as in "it doesn't report its state"). > > Guenter > >> - I would like to monitor with the wdt also the kernel at boot, >> and more importantly handle the case when the userspace app >> doesn't take control of /dev/watchdog for whatever reason >> within the timeout set with WATCHDOG_OPEN_TIMEOUT. >> >> Hope that this explain my target. >> Thanks for your attention, >> >> Francesco Right, I agree with you that's the optimal way, but we are talking about a low cost, simple GPIO WDT, that has only an input GPIO as an interface with the system... how can it report its state if the only way to enable/disable it is a particular state of the GPIO that we are going to pilot? I think that this driver was born for that kind of low cost chips (I'm using a SGM706 for example), why can't we let it take advantage of a useful kernel mechanism? Thank you, Francesco ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] watchdog: gpio_wdt: add "start-at-boot" feature 2021-04-21 16:26 ` [PATCH 2/2] watchdog: gpio_wdt: add "start-at-boot" feature Francesco Zanella 2021-04-21 16:42 ` Guenter Roeck @ 2021-04-23 11:36 ` Rasmus Villemoes 2021-04-23 14:24 ` Francesco Zanella 1 sibling, 1 reply; 11+ messages in thread From: Rasmus Villemoes @ 2021-04-23 11:36 UTC (permalink / raw) To: Francesco Zanella, linux-watchdog, wim, linux Cc: devicetree, robh+dt, linux-kernel On 21/04/2021 18.26, Francesco Zanella wrote: > If "start-at-boot" property is present in the device tree, start pinging > hw watchdog at probe, in order to take advantage of kernel configs: (1) Are you aware of the recent proposal to add a similar feature on watchdog core level: https://lore.kernel.org/lkml/?q=start_enable (2) If you set always-running but not nowayout you essentially have what you want now: If userspace opens the device [within the limit set by OPEN_TIMEOUT if that is in effect], but then does a graceful close (i.e. writes 'V' immediately before close()), the kernel will assume responsibility for pinging the device. So the device isn't stopped as such, but if you can't trust the kernel thread/timer to keep it alive, the system is already mostly unusable. [Also, how reliable is that 'the timer is stopped if the gpio is set to be an input' anyway]. Rasmus ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] watchdog: gpio_wdt: add "start-at-boot" feature 2021-04-23 11:36 ` Rasmus Villemoes @ 2021-04-23 14:24 ` Francesco Zanella 0 siblings, 0 replies; 11+ messages in thread From: Francesco Zanella @ 2021-04-23 14:24 UTC (permalink / raw) To: Rasmus Villemoes, linux-watchdog, wim, linux Cc: devicetree, robh+dt, linux-kernel On 23/04/21 13:36, Rasmus Villemoes wrote: > On 21/04/2021 18.26, Francesco Zanella wrote: >> If "start-at-boot" property is present in the device tree, start pinging >> hw watchdog at probe, in order to take advantage of kernel configs: > > (1) Are you aware of the recent proposal to add a similar feature on > watchdog core level: > > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F%3Fq%3Dstart_enable&data=04%7C01%7Cfrancesco.zanella%40vimar.com%7Cde549dd02adb45669ff208d9064c0739%7Ca1f008bcd59b4c668f8760fd9af15c7f%7C1%7C0%7C637547745887915290%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=pcqWkd%2B4m6RSS4KwmjgIbLpaa0XSCAOQorwI%2BIle5uY%3D&reserved=0 > Oh good! Happy to know that, I missed it, sorry, it's quite new. That kind of work would have been my next proposal if this had been accepted. Hope that it will be mainlined. > (2) If you set always-running but not nowayout you essentially have what > you want now: If userspace opens the device [within the limit set by > OPEN_TIMEOUT if that is in effect], but then does a graceful close (i.e. > writes 'V' immediately before close()), the kernel will assume > responsibility for pinging the device. So the device isn't stopped as > such, but if you can't trust the kernel thread/timer to keep it alive, > the system is already mostly unusable. [Also, how reliable is that 'the > timer is stopped if the gpio is set to be an input' anyway]. > > Rasmus > No I would like to be able to totally disable it with stop, not that the kernel will keep it pinged. However, glad to know the news, I will follow the evolution. Thanks, regards, Francesco Zanella ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-04-23 14:24 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-21 16:26 [PATCH 0/2] GPIO WDT "start-at-boot" property Francesco Zanella 2021-04-21 16:26 ` [PATCH 1/2] dt-bindings: watchdog: gpio-wdt: add "start-at-boot" Francesco Zanella 2021-04-21 16:37 ` Guenter Roeck 2021-04-22 16:28 ` Francesco Zanella 2021-04-21 16:26 ` [PATCH 2/2] watchdog: gpio_wdt: add "start-at-boot" feature Francesco Zanella 2021-04-21 16:42 ` Guenter Roeck 2021-04-22 16:28 ` Francesco Zanella 2021-04-22 18:05 ` Guenter Roeck 2021-04-23 9:47 ` Francesco Zanella 2021-04-23 11:36 ` Rasmus Villemoes 2021-04-23 14:24 ` Francesco Zanella
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).