* [PATCH 0/2] aspeed: watchdog: fix system reset and read alt-boot option @ 2018-03-09 21:58 Eddie James 2018-03-09 21:58 ` [PATCH 1/2] watchdog: aspeed: Fix translation of reset mode to ctrl register Eddie James 2018-03-09 21:58 ` [PATCH 2/2] watchdog: aspeed: Allow configuring for alternate boot Eddie James 0 siblings, 2 replies; 8+ messages in thread From: Eddie James @ 2018-03-09 21:58 UTC (permalink / raw) To: linux-watchdog; +Cc: linux-kernel, joel, wim, linux, Eddie James This series provides a fix to correctly set the reset mode of the control register. Previously, configuring anything other than a full chip reset would not reset anything when the watchdog timer expires. The series also provides a patch to read in the existing aspeed,alt-boot boolean option from the device-tree and set the appropriate bit in the control register. Milton Miller (2): watchdog: aspeed: Fix translation of reset mode to ctrl register watchdog: aspeed: Allow configuring for alternate boot drivers/watchdog/aspeed_wdt.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] watchdog: aspeed: Fix translation of reset mode to ctrl register 2018-03-09 21:58 [PATCH 0/2] aspeed: watchdog: fix system reset and read alt-boot option Eddie James @ 2018-03-09 21:58 ` Eddie James 2018-03-09 22:03 ` Guenter Roeck 2018-03-09 21:58 ` [PATCH 2/2] watchdog: aspeed: Allow configuring for alternate boot Eddie James 1 sibling, 1 reply; 8+ messages in thread From: Eddie James @ 2018-03-09 21:58 UTC (permalink / raw) To: linux-watchdog; +Cc: linux-kernel, joel, wim, linux, Milton Miller, Eddie James From: Milton Miller <miltonm@us.ibm.com> Assert RESET_SYSTEM bit for any reset and set MODE field from reset type. The watchdog control register has a RESET_SYSTEM bit that is really closer to activate a reset, and RESET_SYSTEM_MODE field that chooses how much to reset. Before this patch, a node without these optional property would do a SOC reset, but a node with properties requesting a cpu or SOC reset would do nothing and a node requesting a system reset would do a SOC reset. Fixes: b7f0b8ad25f3 ("drivers/watchdog: ASPEED reference dev tree properties for config") Signed-off-by: Milton Miller <miltonm@us.ibm.com> Signed-off-by: Eddie James <eajames@linux.vnet.ibm.com> --- drivers/watchdog/aspeed_wdt.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c index ca5b91e..d1987d6 100644 --- a/drivers/watchdog/aspeed_wdt.c +++ b/drivers/watchdog/aspeed_wdt.c @@ -232,11 +232,14 @@ static int aspeed_wdt_probe(struct platform_device *pdev) wdt->ctrl |= WDT_CTRL_RESET_MODE_SOC | WDT_CTRL_RESET_SYSTEM; } else { if (!strcmp(reset_type, "cpu")) - wdt->ctrl |= WDT_CTRL_RESET_MODE_ARM_CPU; + wdt->ctrl |= WDT_CTRL_RESET_MODE_ARM_CPU | + WDT_CTRL_RESET_SYSTEM; else if (!strcmp(reset_type, "soc")) - wdt->ctrl |= WDT_CTRL_RESET_MODE_SOC; + wdt->ctrl |= WDT_CTRL_RESET_MODE_SOC | + WDT_CTRL_RESET_SYSTEM; else if (!strcmp(reset_type, "system")) - wdt->ctrl |= WDT_CTRL_RESET_SYSTEM; + wdt->ctrl |= WDT_CTRL_RESET_MODE_FULL_CHIP | + WDT_CTRL_RESET_SYSTEM; else if (strcmp(reset_type, "none")) return -EINVAL; } -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] watchdog: aspeed: Fix translation of reset mode to ctrl register 2018-03-09 21:58 ` [PATCH 1/2] watchdog: aspeed: Fix translation of reset mode to ctrl register Eddie James @ 2018-03-09 22:03 ` Guenter Roeck 0 siblings, 0 replies; 8+ messages in thread From: Guenter Roeck @ 2018-03-09 22:03 UTC (permalink / raw) To: Eddie James; +Cc: linux-watchdog, linux-kernel, joel, wim, Milton Miller On Fri, Mar 09, 2018 at 03:58:19PM -0600, Eddie James wrote: > From: Milton Miller <miltonm@us.ibm.com> > > Assert RESET_SYSTEM bit for any reset and set MODE field from reset > type. > > The watchdog control register has a RESET_SYSTEM bit that is really > closer to activate a reset, and RESET_SYSTEM_MODE field that chooses > how much to reset. > > Before this patch, a node without these optional property would do a > SOC reset, but a node with properties requesting a cpu or SOC reset > would do nothing and a node requesting a system reset would do a > SOC reset. > > Fixes: b7f0b8ad25f3 ("drivers/watchdog: ASPEED reference dev tree properties for config") > Signed-off-by: Milton Miller <miltonm@us.ibm.com> > Signed-off-by: Eddie James <eajames@linux.vnet.ibm.com> Reviewed-by: Guenter Roeck <linux@roeck-us.net> > --- > drivers/watchdog/aspeed_wdt.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c > index ca5b91e..d1987d6 100644 > --- a/drivers/watchdog/aspeed_wdt.c > +++ b/drivers/watchdog/aspeed_wdt.c > @@ -232,11 +232,14 @@ static int aspeed_wdt_probe(struct platform_device *pdev) > wdt->ctrl |= WDT_CTRL_RESET_MODE_SOC | WDT_CTRL_RESET_SYSTEM; > } else { > if (!strcmp(reset_type, "cpu")) > - wdt->ctrl |= WDT_CTRL_RESET_MODE_ARM_CPU; > + wdt->ctrl |= WDT_CTRL_RESET_MODE_ARM_CPU | > + WDT_CTRL_RESET_SYSTEM; > else if (!strcmp(reset_type, "soc")) > - wdt->ctrl |= WDT_CTRL_RESET_MODE_SOC; > + wdt->ctrl |= WDT_CTRL_RESET_MODE_SOC | > + WDT_CTRL_RESET_SYSTEM; > else if (!strcmp(reset_type, "system")) > - wdt->ctrl |= WDT_CTRL_RESET_SYSTEM; > + wdt->ctrl |= WDT_CTRL_RESET_MODE_FULL_CHIP | > + WDT_CTRL_RESET_SYSTEM; > else if (strcmp(reset_type, "none")) > return -EINVAL; > } > -- > 1.8.3.1 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] watchdog: aspeed: Allow configuring for alternate boot 2018-03-09 21:58 [PATCH 0/2] aspeed: watchdog: fix system reset and read alt-boot option Eddie James 2018-03-09 21:58 ` [PATCH 1/2] watchdog: aspeed: Fix translation of reset mode to ctrl register Eddie James @ 2018-03-09 21:58 ` Eddie James 2018-03-09 22:06 ` Guenter Roeck 2018-03-13 4:29 ` Joel Stanley 1 sibling, 2 replies; 8+ messages in thread From: Eddie James @ 2018-03-09 21:58 UTC (permalink / raw) To: linux-watchdog; +Cc: linux-kernel, joel, wim, linux, Milton Miller, Eddie James From: Milton Miller <miltonm@us.ibm.com> Allow the device tree to specify a watchdog to fallover to the alternate boot source. The aspeeed watchdog can set a latch directing flash chip select 0 to chip select 1, allowing boot from an alternate media if the watchdog is not reset in time. On the ast2400 bank 1 also goes to flash bank 1, while on the ast2500 the chip selects are swapped. Signed-off-by: Milton Miller <miltonm@us.ibm.com> Signed-off-by: Eddie James <eajames@linux.vnet.ibm.com> --- drivers/watchdog/aspeed_wdt.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c index d1987d6..f41d246 100644 --- a/drivers/watchdog/aspeed_wdt.c +++ b/drivers/watchdog/aspeed_wdt.c @@ -46,6 +46,7 @@ struct aspeed_wdt_config { #define WDT_RELOAD_VALUE 0x04 #define WDT_RESTART 0x08 #define WDT_CTRL 0x0C +#define WDT_CTRL_BOOT_SECONDARY BIT(7) #define WDT_CTRL_RESET_MODE_SOC (0x00 << 5) #define WDT_CTRL_RESET_MODE_FULL_CHIP (0x01 << 5) #define WDT_CTRL_RESET_MODE_ARM_CPU (0x10 << 5) @@ -245,6 +246,8 @@ static int aspeed_wdt_probe(struct platform_device *pdev) } if (of_property_read_bool(np, "aspeed,external-signal")) wdt->ctrl |= WDT_CTRL_WDT_EXT; + if (of_property_read_bool(np, "aspeed,alt-boot")) + wdt->ctrl |= WDT_CTRL_BOOT_SECONDARY; if (readl(wdt->base + WDT_CTRL) & WDT_CTRL_ENABLE) { /* -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] watchdog: aspeed: Allow configuring for alternate boot 2018-03-09 21:58 ` [PATCH 2/2] watchdog: aspeed: Allow configuring for alternate boot Eddie James @ 2018-03-09 22:06 ` Guenter Roeck 2018-03-14 19:53 ` Eddie James 2018-03-13 4:29 ` Joel Stanley 1 sibling, 1 reply; 8+ messages in thread From: Guenter Roeck @ 2018-03-09 22:06 UTC (permalink / raw) To: Eddie James; +Cc: linux-watchdog, linux-kernel, joel, wim, Milton Miller On Fri, Mar 09, 2018 at 03:58:20PM -0600, Eddie James wrote: > From: Milton Miller <miltonm@us.ibm.com> > > Allow the device tree to specify a watchdog to fallover to > the alternate boot source. > > The aspeeed watchdog can set a latch directing flash chip select 0 to > chip select 1, allowing boot from an alternate media if the watchdog > is not reset in time. On the ast2400 bank 1 also goes to flash bank 1, > while on the ast2500 the chip selects are swapped. > > Signed-off-by: Milton Miller <miltonm@us.ibm.com> > Signed-off-by: Eddie James <eajames@linux.vnet.ibm.com> This is already documented in the bindings document, so the property should be ok. Reviewed-by: Guenter Roeck <linux@roeck-us.net> > --- > drivers/watchdog/aspeed_wdt.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c > index d1987d6..f41d246 100644 > --- a/drivers/watchdog/aspeed_wdt.c > +++ b/drivers/watchdog/aspeed_wdt.c > @@ -46,6 +46,7 @@ struct aspeed_wdt_config { > #define WDT_RELOAD_VALUE 0x04 > #define WDT_RESTART 0x08 > #define WDT_CTRL 0x0C > +#define WDT_CTRL_BOOT_SECONDARY BIT(7) > #define WDT_CTRL_RESET_MODE_SOC (0x00 << 5) > #define WDT_CTRL_RESET_MODE_FULL_CHIP (0x01 << 5) > #define WDT_CTRL_RESET_MODE_ARM_CPU (0x10 << 5) > @@ -245,6 +246,8 @@ static int aspeed_wdt_probe(struct platform_device *pdev) > } > if (of_property_read_bool(np, "aspeed,external-signal")) > wdt->ctrl |= WDT_CTRL_WDT_EXT; > + if (of_property_read_bool(np, "aspeed,alt-boot")) > + wdt->ctrl |= WDT_CTRL_BOOT_SECONDARY; > > if (readl(wdt->base + WDT_CTRL) & WDT_CTRL_ENABLE) { > /* > -- > 1.8.3.1 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] watchdog: aspeed: Allow configuring for alternate boot 2018-03-09 22:06 ` Guenter Roeck @ 2018-03-14 19:53 ` Eddie James 2018-03-16 22:24 ` Guenter Roeck 0 siblings, 1 reply; 8+ messages in thread From: Eddie James @ 2018-03-14 19:53 UTC (permalink / raw) To: Guenter Roeck; +Cc: linux-watchdog, linux-kernel, joel, wim, Milton Miller On 03/09/2018 04:06 PM, Guenter Roeck wrote: > On Fri, Mar 09, 2018 at 03:58:20PM -0600, Eddie James wrote: >> From: Milton Miller <miltonm@us.ibm.com> >> >> Allow the device tree to specify a watchdog to fallover to >> the alternate boot source. >> >> The aspeeed watchdog can set a latch directing flash chip select 0 to >> chip select 1, allowing boot from an alternate media if the watchdog >> is not reset in time. On the ast2400 bank 1 also goes to flash bank 1, >> while on the ast2500 the chip selects are swapped. >> >> Signed-off-by: Milton Miller <miltonm@us.ibm.com> >> Signed-off-by: Eddie James <eajames@linux.vnet.ibm.com> > This is already documented in the bindings document, so the property > should be ok. > > Reviewed-by: Guenter Roeck <linux@roeck-us.net> Thanks. On a related note, our system has need of a way to determine if it's currently booted from the alternate chip or not. It seems the only way to tell is to look at the ast2400/2500 watchdog timeout status register. We can add some debugfs to the aspeed driver I guess, but I wonder if this is a common enough situation that the watchdog core could use some debugfs or sysfs for providing that info? Thanks, Eddie > >> --- >> drivers/watchdog/aspeed_wdt.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c >> index d1987d6..f41d246 100644 >> --- a/drivers/watchdog/aspeed_wdt.c >> +++ b/drivers/watchdog/aspeed_wdt.c >> @@ -46,6 +46,7 @@ struct aspeed_wdt_config { >> #define WDT_RELOAD_VALUE 0x04 >> #define WDT_RESTART 0x08 >> #define WDT_CTRL 0x0C >> +#define WDT_CTRL_BOOT_SECONDARY BIT(7) >> #define WDT_CTRL_RESET_MODE_SOC (0x00 << 5) >> #define WDT_CTRL_RESET_MODE_FULL_CHIP (0x01 << 5) >> #define WDT_CTRL_RESET_MODE_ARM_CPU (0x10 << 5) >> @@ -245,6 +246,8 @@ static int aspeed_wdt_probe(struct platform_device *pdev) >> } >> if (of_property_read_bool(np, "aspeed,external-signal")) >> wdt->ctrl |= WDT_CTRL_WDT_EXT; >> + if (of_property_read_bool(np, "aspeed,alt-boot")) >> + wdt->ctrl |= WDT_CTRL_BOOT_SECONDARY; >> >> if (readl(wdt->base + WDT_CTRL) & WDT_CTRL_ENABLE) { >> /* >> -- >> 1.8.3.1 >> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] watchdog: aspeed: Allow configuring for alternate boot 2018-03-14 19:53 ` Eddie James @ 2018-03-16 22:24 ` Guenter Roeck 0 siblings, 0 replies; 8+ messages in thread From: Guenter Roeck @ 2018-03-16 22:24 UTC (permalink / raw) To: Eddie James; +Cc: linux-watchdog, linux-kernel, joel, wim, Milton Miller On Wed, Mar 14, 2018 at 02:53:13PM -0500, Eddie James wrote: > > > On 03/09/2018 04:06 PM, Guenter Roeck wrote: > >On Fri, Mar 09, 2018 at 03:58:20PM -0600, Eddie James wrote: > >>From: Milton Miller <miltonm@us.ibm.com> > >> > >>Allow the device tree to specify a watchdog to fallover to > >>the alternate boot source. > >> > >>The aspeeed watchdog can set a latch directing flash chip select 0 to > >>chip select 1, allowing boot from an alternate media if the watchdog > >>is not reset in time. On the ast2400 bank 1 also goes to flash bank 1, > >>while on the ast2500 the chip selects are swapped. > >> > >>Signed-off-by: Milton Miller <miltonm@us.ibm.com> > >>Signed-off-by: Eddie James <eajames@linux.vnet.ibm.com> > >This is already documented in the bindings document, so the property > >should be ok. > > > >Reviewed-by: Guenter Roeck <linux@roeck-us.net> > > Thanks. On a related note, our system has need of a way to determine if it's > currently booted from the alternate chip or not. It seems the only way to > tell is to look at the ast2400/2500 watchdog timeout status register. We can > add some debugfs to the aspeed driver I guess, but I wonder if this is a > common enough situation that the watchdog core could use some debugfs or > sysfs for providing that info? > If you have the means to determine that the watchdog timed out in the previous boot, can you just set the WDIOF_CARDRESET status bit ? Guenter > Thanks, > Eddie > > > > >>--- > >> drivers/watchdog/aspeed_wdt.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >>diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c > >>index d1987d6..f41d246 100644 > >>--- a/drivers/watchdog/aspeed_wdt.c > >>+++ b/drivers/watchdog/aspeed_wdt.c > >>@@ -46,6 +46,7 @@ struct aspeed_wdt_config { > >> #define WDT_RELOAD_VALUE 0x04 > >> #define WDT_RESTART 0x08 > >> #define WDT_CTRL 0x0C > >>+#define WDT_CTRL_BOOT_SECONDARY BIT(7) > >> #define WDT_CTRL_RESET_MODE_SOC (0x00 << 5) > >> #define WDT_CTRL_RESET_MODE_FULL_CHIP (0x01 << 5) > >> #define WDT_CTRL_RESET_MODE_ARM_CPU (0x10 << 5) > >>@@ -245,6 +246,8 @@ static int aspeed_wdt_probe(struct platform_device *pdev) > >> } > >> if (of_property_read_bool(np, "aspeed,external-signal")) > >> wdt->ctrl |= WDT_CTRL_WDT_EXT; > >>+ if (of_property_read_bool(np, "aspeed,alt-boot")) > >>+ wdt->ctrl |= WDT_CTRL_BOOT_SECONDARY; > >> if (readl(wdt->base + WDT_CTRL) & WDT_CTRL_ENABLE) { > >> /* > >>-- > >>1.8.3.1 > >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] watchdog: aspeed: Allow configuring for alternate boot 2018-03-09 21:58 ` [PATCH 2/2] watchdog: aspeed: Allow configuring for alternate boot Eddie James 2018-03-09 22:06 ` Guenter Roeck @ 2018-03-13 4:29 ` Joel Stanley 1 sibling, 0 replies; 8+ messages in thread From: Joel Stanley @ 2018-03-13 4:29 UTC (permalink / raw) To: Eddie James Cc: LINUXWATCHDOG, Linux Kernel Mailing List, Wim Van Sebroeck, Guenter Roeck, Milton Miller, Andrew Jeffery On Sat, Mar 10, 2018 at 8:28 AM, Eddie James <eajames@linux.vnet.ibm.com> wrote: > From: Milton Miller <miltonm@us.ibm.com> > > Allow the device tree to specify a watchdog to fallover to > the alternate boot source. > > The aspeeed watchdog can set a latch directing flash chip select 0 to > chip select 1, allowing boot from an alternate media if the watchdog > is not reset in time. On the ast2400 bank 1 also goes to flash bank 1, > while on the ast2500 the chip selects are swapped. > > Signed-off-by: Milton Miller <miltonm@us.ibm.com> > Signed-off-by: Eddie James <eajames@linux.vnet.ibm.com> > --- > drivers/watchdog/aspeed_wdt.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c > index d1987d6..f41d246 100644 > --- a/drivers/watchdog/aspeed_wdt.c > +++ b/drivers/watchdog/aspeed_wdt.c > @@ -46,6 +46,7 @@ struct aspeed_wdt_config { > #define WDT_RELOAD_VALUE 0x04 > #define WDT_RESTART 0x08 > #define WDT_CTRL 0x0C > +#define WDT_CTRL_BOOT_SECONDARY BIT(7) > #define WDT_CTRL_RESET_MODE_SOC (0x00 << 5) > #define WDT_CTRL_RESET_MODE_FULL_CHIP (0x01 << 5) > #define WDT_CTRL_RESET_MODE_ARM_CPU (0x10 << 5) > @@ -245,6 +246,8 @@ static int aspeed_wdt_probe(struct platform_device *pdev) > } > if (of_property_read_bool(np, "aspeed,external-signal")) > wdt->ctrl |= WDT_CTRL_WDT_EXT; > + if (of_property_read_bool(np, "aspeed,alt-boot")) > + wdt->ctrl |= WDT_CTRL_BOOT_SECONDARY; If a user sets this property on the only watchdog in the system, then they will trigger this behaviour when doing a normal 'reboot'. That would not be desirable. We could mitigate this by: - not registering the watchdog as a reboot device if this property is enabled - clearing the WDT_CTRL_BOOT_SECONDARY bit in the aspeed_wdt_restart path The second option is neater as it allows the watchdog to behave normally when only one is enabled. The first would be confusing if a system didn't have more than one watchdog enabled. Cheers, Joel ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-03-16 22:24 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-03-09 21:58 [PATCH 0/2] aspeed: watchdog: fix system reset and read alt-boot option Eddie James 2018-03-09 21:58 ` [PATCH 1/2] watchdog: aspeed: Fix translation of reset mode to ctrl register Eddie James 2018-03-09 22:03 ` Guenter Roeck 2018-03-09 21:58 ` [PATCH 2/2] watchdog: aspeed: Allow configuring for alternate boot Eddie James 2018-03-09 22:06 ` Guenter Roeck 2018-03-14 19:53 ` Eddie James 2018-03-16 22:24 ` Guenter Roeck 2018-03-13 4:29 ` Joel Stanley
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).