* [PATCH 0/2] drivers/watchdog ASPEED: Add optional dev tree configs @ 2017-06-13 20:38 Christopher Bostic 2017-06-13 20:38 ` [PATCH 1/2] drivers/watchdog: Document new aspeed optional dev tree properties Christopher Bostic 2017-06-13 20:38 ` [PATCH 2/2] drivers/watchdog: ASPEED reference dev tree properties for config Christopher Bostic 0 siblings, 2 replies; 9+ messages in thread From: Christopher Bostic @ 2017-06-13 20:38 UTC (permalink / raw) To: wim, linux; +Cc: Christopher Bostic, joel, linux-kernel, linux-watchdog, andrew Add optional device tree attribute 'external-signal'. Configure watchdog to drive external signal on timeout if this attribute is present. Add optional device tree attribute 'no-system-reset'. Configure watchdog to not reset system on timeout if this attribute is present. This mode is used when one of the other watchdogs in system is to be responsible for managing system reset. Christopher Bostic (2): drivers/watchdog: Document new aspeed optional dev tree properties. drivers/watchdog: ASPEED reference dev tree properties for config Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt | 11 +++++++++++ drivers/watchdog/aspeed_wdt.c | 13 +++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) -- 1.8.2.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] drivers/watchdog: Document new aspeed optional dev tree properties. 2017-06-13 20:38 [PATCH 0/2] drivers/watchdog ASPEED: Add optional dev tree configs Christopher Bostic @ 2017-06-13 20:38 ` Christopher Bostic 2017-06-24 22:07 ` [1/2] " Guenter Roeck 2017-06-27 2:59 ` [PATCH 1/2] " Joel Stanley 2017-06-13 20:38 ` [PATCH 2/2] drivers/watchdog: ASPEED reference dev tree properties for config Christopher Bostic 1 sibling, 2 replies; 9+ messages in thread From: Christopher Bostic @ 2017-06-13 20:38 UTC (permalink / raw) To: wim, linux; +Cc: Christopher Bostic, joel, linux-kernel, linux-watchdog, andrew Describe new optional property 'external-signal'. When present in the system device tree an exernal signal is generated on watchdog timeout. Describe new optional property 'no-system-reset'. When present in the system device tree no system reset is to occur on watchdog timeout. System reset in this case is managed by one of the other watchogs available. Signed-off-by: Christopher Bostic <cbostic@linux.vnet.ibm.com> --- Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt index c5e74d7..4099ea5 100644 --- a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt +++ b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt @@ -8,9 +8,20 @@ Required properties: - reg: physical base address of the controller and length of memory mapped region +Optional properties: + - external-signal: If the property is present then an external signal is to + be generated on watchdog timeout. If absent, external signal is not + generated. + + - no-system-reset: If the property is present then system will not be reset + on watchdog timeout. In this case one of the other watchdogs will handle + reset. If absent then the watchdog resets the system on timeout. + Example: wdt1: watchdog@1e785000 { compatible = "aspeed,ast2400-wdt"; reg = <0x1e785000 0x1c>; + external-signal; + no-system-reset; }; -- 1.8.2.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [1/2] drivers/watchdog: Document new aspeed optional dev tree properties. 2017-06-13 20:38 ` [PATCH 1/2] drivers/watchdog: Document new aspeed optional dev tree properties Christopher Bostic @ 2017-06-24 22:07 ` Guenter Roeck 2017-06-27 2:16 ` Christopher Bostic 2017-06-27 2:59 ` [PATCH 1/2] " Joel Stanley 1 sibling, 1 reply; 9+ messages in thread From: Guenter Roeck @ 2017-06-24 22:07 UTC (permalink / raw) To: Christopher Bostic; +Cc: wim, joel, linux-kernel, linux-watchdog, andrew On Tue, Jun 13, 2017 at 03:38:17PM -0500, Christopher Bostic wrote: > Describe new optional property 'external-signal'. When present in the > system device tree an exernal signal is generated on watchdog timeout. > > Describe new optional property 'no-system-reset'. When present in the > system device tree no system reset is to occur on watchdog timeout. > System reset in this case is managed by one of the other watchogs > available. > > Signed-off-by: Christopher Bostic <cbostic@linux.vnet.ibm.com> This requires DT maintainer approval, but DT maintainers were not copied. Please resend and use scripts/get_maintainer.pl to determine who to send it to. Guenter > --- > Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt > index c5e74d7..4099ea5 100644 > --- a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt > +++ b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt > @@ -8,9 +8,20 @@ Required properties: > - reg: physical base address of the controller and length of memory mapped > region > > +Optional properties: > + - external-signal: If the property is present then an external signal is to > + be generated on watchdog timeout. If absent, external signal is not > + generated. > + > + - no-system-reset: If the property is present then system will not be reset > + on watchdog timeout. In this case one of the other watchdogs will handle > + reset. If absent then the watchdog resets the system on timeout. > + > Example: > > wdt1: watchdog@1e785000 { > compatible = "aspeed,ast2400-wdt"; > reg = <0x1e785000 0x1c>; > + external-signal; > + no-system-reset; > }; ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [1/2] drivers/watchdog: Document new aspeed optional dev tree properties. 2017-06-24 22:07 ` [1/2] " Guenter Roeck @ 2017-06-27 2:16 ` Christopher Bostic 0 siblings, 0 replies; 9+ messages in thread From: Christopher Bostic @ 2017-06-27 2:16 UTC (permalink / raw) To: Guenter Roeck; +Cc: wim, joel, linux-kernel, linux-watchdog, andrew On 6/24/17 5:07 PM, Guenter Roeck wrote: > On Tue, Jun 13, 2017 at 03:38:17PM -0500, Christopher Bostic wrote: >> Describe new optional property 'external-signal'. When present in the >> system device tree an exernal signal is generated on watchdog timeout. >> >> Describe new optional property 'no-system-reset'. When present in the >> system device tree no system reset is to occur on watchdog timeout. >> System reset in this case is managed by one of the other watchogs >> available. >> >> Signed-off-by: Christopher Bostic <cbostic@linux.vnet.ibm.com> > This requires DT maintainer approval, but DT maintainers were not copied. > Please resend and use scripts/get_maintainer.pl to determine who to send > it to. > > Guenter Hi Guenter, Will resend with maintainers copied. Thanks -Chris >> --- >> Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt >> index c5e74d7..4099ea5 100644 >> --- a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt >> +++ b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt >> @@ -8,9 +8,20 @@ Required properties: >> - reg: physical base address of the controller and length of memory mapped >> region >> >> +Optional properties: >> + - external-signal: If the property is present then an external signal is to >> + be generated on watchdog timeout. If absent, external signal is not >> + generated. >> + >> + - no-system-reset: If the property is present then system will not be reset >> + on watchdog timeout. In this case one of the other watchdogs will handle >> + reset. If absent then the watchdog resets the system on timeout. >> + >> Example: >> >> wdt1: watchdog@1e785000 { >> compatible = "aspeed,ast2400-wdt"; >> reg = <0x1e785000 0x1c>; >> + external-signal; >> + no-system-reset; >> }; ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] drivers/watchdog: Document new aspeed optional dev tree properties. 2017-06-13 20:38 ` [PATCH 1/2] drivers/watchdog: Document new aspeed optional dev tree properties Christopher Bostic 2017-06-24 22:07 ` [1/2] " Guenter Roeck @ 2017-06-27 2:59 ` Joel Stanley 2017-06-27 19:44 ` Christopher Bostic 1 sibling, 1 reply; 9+ messages in thread From: Joel Stanley @ 2017-06-27 2:59 UTC (permalink / raw) To: Christopher Bostic Cc: Wim Van Sebroeck, Guenter Roeck, Linux Kernel Mailing List, linux-watchdog, Andrew Jeffery On Wed, Jun 14, 2017 at 6:08 AM, Christopher Bostic <cbostic@linux.vnet.ibm.com> wrote: > Describe new optional property 'external-signal'. When present in the > system device tree an exernal signal is generated on watchdog timeout. > > Describe new optional property 'no-system-reset'. When present in the > system device tree no system reset is to occur on watchdog timeout. > System reset in this case is managed by one of the other watchogs > available. > > Signed-off-by: Christopher Bostic <cbostic@linux.vnet.ibm.com> > --- > Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt > index c5e74d7..4099ea5 100644 > --- a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt > +++ b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt > @@ -8,9 +8,20 @@ Required properties: > - reg: physical base address of the controller and length of memory mapped > region > > +Optional properties: > + - external-signal: If the property is present then an external signal is to > + be generated on watchdog timeout. If absent, external signal is not > + generated. > + > + - no-system-reset: If the property is present then system will not be reset > + on watchdog timeout. In this case one of the other watchdogs will handle > + reset. If absent then the watchdog resets the system on timeout. I'm not sure that this describes the hardware. The datasheet says: Whenever timeout ocurs, WDT can program to generate 6 types of signals: * ARM reset signal: to reset ARM CPU only * SOC reset signal: to reset SOC part function * System reset signal: to reset full chip * Interrupt signal: to interrupt CPU * Eternal signal: to external reset counter (only WDT1 and WDT2) * Alternate boot signal: to boot from alternate block I think your bindings should describe these modes where possible. Cheers, Joel > + > Example: > > wdt1: watchdog@1e785000 { > compatible = "aspeed,ast2400-wdt"; > reg = <0x1e785000 0x1c>; > + external-signal; > + no-system-reset; > }; > -- > 1.8.2.2 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] drivers/watchdog: Document new aspeed optional dev tree properties. 2017-06-27 2:59 ` [PATCH 1/2] " Joel Stanley @ 2017-06-27 19:44 ` Christopher Bostic 0 siblings, 0 replies; 9+ messages in thread From: Christopher Bostic @ 2017-06-27 19:44 UTC (permalink / raw) To: Joel Stanley Cc: Wim Van Sebroeck, Guenter Roeck, Linux Kernel Mailing List, linux-watchdog, Andrew Jeffery On 6/26/17 9:59 PM, Joel Stanley wrote: > On Wed, Jun 14, 2017 at 6:08 AM, Christopher Bostic > <cbostic@linux.vnet.ibm.com> wrote: >> Describe new optional property 'external-signal'. When present in the >> system device tree an exernal signal is generated on watchdog timeout. >> >> Describe new optional property 'no-system-reset'. When present in the >> system device tree no system reset is to occur on watchdog timeout. >> System reset in this case is managed by one of the other watchogs >> available. >> >> Signed-off-by: Christopher Bostic <cbostic@linux.vnet.ibm.com> >> --- >> Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt >> index c5e74d7..4099ea5 100644 >> --- a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt >> +++ b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt >> @@ -8,9 +8,20 @@ Required properties: >> - reg: physical base address of the controller and length of memory mapped >> region >> >> +Optional properties: >> + - external-signal: If the property is present then an external signal is to >> + be generated on watchdog timeout. If absent, external signal is not >> + generated. >> + >> + - no-system-reset: If the property is present then system will not be reset >> + on watchdog timeout. In this case one of the other watchdogs will handle >> + reset. If absent then the watchdog resets the system on timeout. > I'm not sure that this describes the hardware. The datasheet says: > > Whenever timeout ocurs, WDT can program to generate 6 types of signals: > > * ARM reset signal: to reset ARM CPU only > * SOC reset signal: to reset SOC part function > * System reset signal: to reset full chip > * Interrupt signal: to interrupt CPU > * Eternal signal: to external reset counter (only WDT1 and WDT2) > * Alternate boot signal: to boot from alternate block > > I think your bindings should describe these modes where possible. I'll add the modes you describe. Thanks, Chris > > Cheers, > > Joel > > >> + >> Example: >> >> wdt1: watchdog@1e785000 { >> compatible = "aspeed,ast2400-wdt"; >> reg = <0x1e785000 0x1c>; >> + external-signal; >> + no-system-reset; >> }; >> -- >> 1.8.2.2 >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] drivers/watchdog: ASPEED reference dev tree properties for config 2017-06-13 20:38 [PATCH 0/2] drivers/watchdog ASPEED: Add optional dev tree configs Christopher Bostic 2017-06-13 20:38 ` [PATCH 1/2] drivers/watchdog: Document new aspeed optional dev tree properties Christopher Bostic @ 2017-06-13 20:38 ` Christopher Bostic 2017-06-24 22:17 ` [2/2] " Guenter Roeck 1 sibling, 1 reply; 9+ messages in thread From: Christopher Bostic @ 2017-06-13 20:38 UTC (permalink / raw) To: wim, linux; +Cc: Christopher Bostic, joel, linux-kernel, linux-watchdog, andrew Reference the system device tree when configuring the watchdog. Configure for external signal generation if optional attribute 'external-signal' is present in device tree. Configure for reset system after timeout if the optional attribute 'no-system-reset' is not present. Disabling system reset may be required in situations where one of the other watchdogs in the systems is responsible for this. Signed-off-by: Christopher Bostic <cbostic@linux.vnet.ibm.com> --- drivers/watchdog/aspeed_wdt.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c index 1c65258..9deb4be 100644 --- a/drivers/watchdog/aspeed_wdt.c +++ b/drivers/watchdog/aspeed_wdt.c @@ -140,6 +140,7 @@ static int aspeed_wdt_probe(struct platform_device *pdev) { struct aspeed_wdt *wdt; struct resource *res; + struct device_node *np; int ret; wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL); @@ -170,8 +171,16 @@ static int aspeed_wdt_probe(struct platform_device *pdev) * the SOC and not the full chip */ wdt->ctrl = WDT_CTRL_RESET_MODE_SOC | - WDT_CTRL_1MHZ_CLK | - WDT_CTRL_RESET_SYSTEM; + WDT_CTRL_1MHZ_CLK; + + np = pdev->dev.of_node; + if (np && !of_get_property(np, "no-system-reset", NULL)) + wdt->ctrl |= WDT_CTRL_RESET_SYSTEM; + + if (np && of_get_property(np, "external-signal", NULL)) + wdt->ctrl |= WDT_CTRL_WDT_EXT; + + writel(wdt->ctrl, wdt->base + WDT_CTRL); if (readl(wdt->base + WDT_CTRL) & WDT_CTRL_ENABLE) { aspeed_wdt_start(&wdt->wdd); -- 1.8.2.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [2/2] drivers/watchdog: ASPEED reference dev tree properties for config 2017-06-13 20:38 ` [PATCH 2/2] drivers/watchdog: ASPEED reference dev tree properties for config Christopher Bostic @ 2017-06-24 22:17 ` Guenter Roeck 2017-06-27 2:17 ` Christopher Bostic 0 siblings, 1 reply; 9+ messages in thread From: Guenter Roeck @ 2017-06-24 22:17 UTC (permalink / raw) To: Christopher Bostic; +Cc: wim, joel, linux-kernel, linux-watchdog, andrew On Tue, Jun 13, 2017 at 03:38:18PM -0500, Christopher Bostic wrote: > Reference the system device tree when configuring the watchdog. > Configure for external signal generation if optional attribute > 'external-signal' is present in device tree. Configure for > reset system after timeout if the optional attribute > 'no-system-reset' is not present. Disabling system reset may be > required in situations where one of the other watchdogs in the > systems is responsible for this. > > Signed-off-by: Christopher Bostic <cbostic@linux.vnet.ibm.com> > --- > drivers/watchdog/aspeed_wdt.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c > index 1c65258..9deb4be 100644 > --- a/drivers/watchdog/aspeed_wdt.c > +++ b/drivers/watchdog/aspeed_wdt.c > @@ -140,6 +140,7 @@ static int aspeed_wdt_probe(struct platform_device *pdev) > { > struct aspeed_wdt *wdt; > struct resource *res; > + struct device_node *np; > int ret; > > wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL); > @@ -170,8 +171,16 @@ static int aspeed_wdt_probe(struct platform_device *pdev) > * the SOC and not the full chip > */ > wdt->ctrl = WDT_CTRL_RESET_MODE_SOC | > - WDT_CTRL_1MHZ_CLK | > - WDT_CTRL_RESET_SYSTEM; > + WDT_CTRL_1MHZ_CLK; > + > + np = pdev->dev.of_node; > + if (np && !of_get_property(np, "no-system-reset", NULL)) > + wdt->ctrl |= WDT_CTRL_RESET_SYSTEM; > + Please use of_property_read_bool(). Also, checking np for NULL is unnecessary since the called function does that. Note that the above code as written is wrong - if np is NULL, WDT_CTRL_RESET_SYSTEM will no longer be set. The new properties should probably start with "aspeed," but I'll leave that up to DT maintainers to decide. > + if (np && of_get_property(np, "external-signal", NULL)) > + wdt->ctrl |= WDT_CTRL_WDT_EXT; > + > + writel(wdt->ctrl, wdt->base + WDT_CTRL); > > if (readl(wdt->base + WDT_CTRL) & WDT_CTRL_ENABLE) { > aspeed_wdt_start(&wdt->wdd); ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [2/2] drivers/watchdog: ASPEED reference dev tree properties for config 2017-06-24 22:17 ` [2/2] " Guenter Roeck @ 2017-06-27 2:17 ` Christopher Bostic 0 siblings, 0 replies; 9+ messages in thread From: Christopher Bostic @ 2017-06-27 2:17 UTC (permalink / raw) To: Guenter Roeck; +Cc: wim, joel, linux-kernel, linux-watchdog, andrew On 6/24/17 5:17 PM, Guenter Roeck wrote: > On Tue, Jun 13, 2017 at 03:38:18PM -0500, Christopher Bostic wrote: >> Reference the system device tree when configuring the watchdog. >> Configure for external signal generation if optional attribute >> 'external-signal' is present in device tree. Configure for >> reset system after timeout if the optional attribute >> 'no-system-reset' is not present. Disabling system reset may be >> required in situations where one of the other watchdogs in the >> systems is responsible for this. >> >> Signed-off-by: Christopher Bostic <cbostic@linux.vnet.ibm.com> >> --- >> drivers/watchdog/aspeed_wdt.c | 13 +++++++++++-- >> 1 file changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c >> index 1c65258..9deb4be 100644 >> --- a/drivers/watchdog/aspeed_wdt.c >> +++ b/drivers/watchdog/aspeed_wdt.c >> @@ -140,6 +140,7 @@ static int aspeed_wdt_probe(struct platform_device *pdev) >> { >> struct aspeed_wdt *wdt; >> struct resource *res; >> + struct device_node *np; >> int ret; >> >> wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL); >> @@ -170,8 +171,16 @@ static int aspeed_wdt_probe(struct platform_device *pdev) >> * the SOC and not the full chip >> */ >> wdt->ctrl = WDT_CTRL_RESET_MODE_SOC | >> - WDT_CTRL_1MHZ_CLK | >> - WDT_CTRL_RESET_SYSTEM; >> + WDT_CTRL_1MHZ_CLK; >> + >> + np = pdev->dev.of_node; >> + if (np && !of_get_property(np, "no-system-reset", NULL)) >> + wdt->ctrl |= WDT_CTRL_RESET_SYSTEM; >> + > Please use of_property_read_bool(). Also, checking np for NULL is unnecessary > since the called function does that. Note that the above code as written is > wrong - if np is NULL, WDT_CTRL_RESET_SYSTEM will no longer be set. Will make the suggested changes. Thanks -Chris > The new properties should probably start with "aspeed," but I'll leave that > up to DT maintainers to decide. > >> + if (np && of_get_property(np, "external-signal", NULL)) >> + wdt->ctrl |= WDT_CTRL_WDT_EXT; >> + >> + writel(wdt->ctrl, wdt->base + WDT_CTRL); >> >> if (readl(wdt->base + WDT_CTRL) & WDT_CTRL_ENABLE) { >> aspeed_wdt_start(&wdt->wdd); ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-06-27 19:44 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-06-13 20:38 [PATCH 0/2] drivers/watchdog ASPEED: Add optional dev tree configs Christopher Bostic 2017-06-13 20:38 ` [PATCH 1/2] drivers/watchdog: Document new aspeed optional dev tree properties Christopher Bostic 2017-06-24 22:07 ` [1/2] " Guenter Roeck 2017-06-27 2:16 ` Christopher Bostic 2017-06-27 2:59 ` [PATCH 1/2] " Joel Stanley 2017-06-27 19:44 ` Christopher Bostic 2017-06-13 20:38 ` [PATCH 2/2] drivers/watchdog: ASPEED reference dev tree properties for config Christopher Bostic 2017-06-24 22:17 ` [2/2] " Guenter Roeck 2017-06-27 2:17 ` Christopher Bostic
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).