| Message ID | 20210305213659.31025-3-pali@kernel.org |
|---|---|
| State | Superseded |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | [1/4] watchdog: Set/unset GD_FLG_WDT_READY flag in wdt_start()/wdt_stop() | expand |
On 05.03.21 22:36, Pali Rohár wrote: > In some cases it is useful to compile support for U-Boot command 'wdt' > without starting HW watchdog in early U-Boot phase. For example when user Nitpicking: when "the" user > want to start watchdog only on demand by some boot script. > > This change adds a new compile option WATCHDOG_AUTOSTART to control whether > U-Boot should automatically start watchdog during init phase or not. start "the" watchdog > > This option is enabled by default as it was was default behavior prior as it was the default > introducing this new change. When compiling U-Boot users can decide to turn > this option off. > > Signed-off-by: Pali Rohár <pali@kernel.org> > --- > drivers/watchdog/Kconfig | 13 +++++++++++++ > drivers/watchdog/wdt-uclass.c | 7 +++++++ > 2 files changed, 20 insertions(+) > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > index 602ccbe41c00..aa76a8f2d239 100644 > --- a/drivers/watchdog/Kconfig > +++ b/drivers/watchdog/Kconfig > @@ -9,6 +9,19 @@ config WATCHDOG > this option if you want to service enabled watchdog by U-Boot. Disable > this option if you want U-Boot to start watchdog but never service it. > > +config WATCHDOG_AUTOSTART > + bool "Automatically start watchdog timer" > + depends on WDT > + default y > + help > + Automatically start watchdog timer and start servicing it during > + init phase. Enabled by default. Disable this option if you want > + to compile U-Boot with CONFIG_WDT support but do not want to > + activate watchdog, like when CONFIG_WDT option is disabled. You > + would be able to start watchdog manually by 'wdt' command. Useful > + when you want to have support for 'wdt' command but do not want > + to have watchdog enabled by default. > + > config WATCHDOG_TIMEOUT_MSECS > int "Watchdog timeout in msec" > default 128000 if ARCH_MX25 || ARCH_MX31 || ARCH_MX5 || ARCH_MX6 > diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c > index 7500b3ed90e3..00a408bf5cc5 100644 > --- a/drivers/watchdog/wdt-uclass.c > +++ b/drivers/watchdog/wdt-uclass.c > @@ -27,7 +27,9 @@ static ulong reset_period = 1000; > int initr_watchdog(void) > { > u32 timeout = WATCHDOG_TIMEOUT_SECS; > +#ifdef CONFIG_WATCHDOG_AUTOSTART > int ret; > +#endif Please don't add more #ifdef's if possible, see below... > > /* > * Init watchdog: This will call the probe function of the > @@ -51,6 +53,10 @@ int initr_watchdog(void) > 4 * reset_period) / 4; > } > > +#ifndef CONFIG_WATCHDOG_AUTOSTART > + printf("WDT: Not starting\n"); > + return 0; > +#else > ret = wdt_start(gd->watchdog_dev, timeout * 1000, 0); > if (ret != 0) { > printf("WDT: Failed to start\n"); > @@ -61,6 +67,7 @@ int initr_watchdog(void) > IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", timeout); > > return 0; > +#endif Please use this instead here: if (CONFIG_IS_ENABLED(WATCHDOG_AUTOSTART)) { ... Thanks, Stefan > } > > int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags) > Viele Grüße, Stefan
On Tuesday 09 March 2021 12:33:24 Stefan Roese wrote: > On 05.03.21 22:36, Pali Rohár wrote: > > In some cases it is useful to compile support for U-Boot command 'wdt' > > without starting HW watchdog in early U-Boot phase. For example when user > > Nitpicking: > when "the" user > > > want to start watchdog only on demand by some boot script. > > > > This change adds a new compile option WATCHDOG_AUTOSTART to control whether > > U-Boot should automatically start watchdog during init phase or not. > > start "the" watchdog > > > > > This option is enabled by default as it was was default behavior prior > > as it was the default > > > introducing this new change. When compiling U-Boot users can decide to turn > > this option off. > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > --- > > drivers/watchdog/Kconfig | 13 +++++++++++++ > > drivers/watchdog/wdt-uclass.c | 7 +++++++ > > 2 files changed, 20 insertions(+) > > > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > > index 602ccbe41c00..aa76a8f2d239 100644 > > --- a/drivers/watchdog/Kconfig > > +++ b/drivers/watchdog/Kconfig > > @@ -9,6 +9,19 @@ config WATCHDOG > > this option if you want to service enabled watchdog by U-Boot. Disable > > this option if you want U-Boot to start watchdog but never service it. > > +config WATCHDOG_AUTOSTART > > + bool "Automatically start watchdog timer" > > + depends on WDT > > + default y > > + help > > + Automatically start watchdog timer and start servicing it during > > + init phase. Enabled by default. Disable this option if you want > > + to compile U-Boot with CONFIG_WDT support but do not want to > > + activate watchdog, like when CONFIG_WDT option is disabled. You > > + would be able to start watchdog manually by 'wdt' command. Useful > > + when you want to have support for 'wdt' command but do not want > > + to have watchdog enabled by default. > > + > > config WATCHDOG_TIMEOUT_MSECS > > int "Watchdog timeout in msec" > > default 128000 if ARCH_MX25 || ARCH_MX31 || ARCH_MX5 || ARCH_MX6 > > diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c > > index 7500b3ed90e3..00a408bf5cc5 100644 > > --- a/drivers/watchdog/wdt-uclass.c > > +++ b/drivers/watchdog/wdt-uclass.c > > @@ -27,7 +27,9 @@ static ulong reset_period = 1000; > > int initr_watchdog(void) > > { > > u32 timeout = WATCHDOG_TIMEOUT_SECS; > > +#ifdef CONFIG_WATCHDOG_AUTOSTART > > int ret; > > +#endif > > Please don't add more #ifdef's if possible, see below... > > > /* > > * Init watchdog: This will call the probe function of the > > @@ -51,6 +53,10 @@ int initr_watchdog(void) > > 4 * reset_period) / 4; > > } > > +#ifndef CONFIG_WATCHDOG_AUTOSTART > > + printf("WDT: Not starting\n"); > > + return 0; > > +#else > > ret = wdt_start(gd->watchdog_dev, timeout * 1000, 0); > > if (ret != 0) { > > printf("WDT: Failed to start\n"); > > @@ -61,6 +67,7 @@ int initr_watchdog(void) > > IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", timeout); > > return 0; > > +#endif > > Please use this instead here: > > if (CONFIG_IS_ENABLED(WATCHDOG_AUTOSTART)) { > ... > > Thanks, > Stefan > > > } > > int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags) > > > > > Viele Grüße, > Stefan I have fixed these issues in V2.
On 09.03.21 14:27, Pali Rohár wrote: > On Tuesday 09 March 2021 12:33:24 Stefan Roese wrote: >> On 05.03.21 22:36, Pali Rohár wrote: >>> In some cases it is useful to compile support for U-Boot command 'wdt' >>> without starting HW watchdog in early U-Boot phase. For example when user >> >> Nitpicking: >> when "the" user >> >>> want to start watchdog only on demand by some boot script. >>> >>> This change adds a new compile option WATCHDOG_AUTOSTART to control whether >>> U-Boot should automatically start watchdog during init phase or not. >> >> start "the" watchdog >> >>> >>> This option is enabled by default as it was was default behavior prior >> >> as it was the default >> >>> introducing this new change. When compiling U-Boot users can decide to turn >>> this option off. >>> >>> Signed-off-by: Pali Rohár <pali@kernel.org> >>> --- >>> drivers/watchdog/Kconfig | 13 +++++++++++++ >>> drivers/watchdog/wdt-uclass.c | 7 +++++++ >>> 2 files changed, 20 insertions(+) >>> >>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig >>> index 602ccbe41c00..aa76a8f2d239 100644 >>> --- a/drivers/watchdog/Kconfig >>> +++ b/drivers/watchdog/Kconfig >>> @@ -9,6 +9,19 @@ config WATCHDOG >>> this option if you want to service enabled watchdog by U-Boot. Disable >>> this option if you want U-Boot to start watchdog but never service it. >>> +config WATCHDOG_AUTOSTART >>> + bool "Automatically start watchdog timer" >>> + depends on WDT >>> + default y >>> + help >>> + Automatically start watchdog timer and start servicing it during >>> + init phase. Enabled by default. Disable this option if you want >>> + to compile U-Boot with CONFIG_WDT support but do not want to >>> + activate watchdog, like when CONFIG_WDT option is disabled. You >>> + would be able to start watchdog manually by 'wdt' command. Useful >>> + when you want to have support for 'wdt' command but do not want >>> + to have watchdog enabled by default. >>> + >>> config WATCHDOG_TIMEOUT_MSECS >>> int "Watchdog timeout in msec" >>> default 128000 if ARCH_MX25 || ARCH_MX31 || ARCH_MX5 || ARCH_MX6 >>> diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c >>> index 7500b3ed90e3..00a408bf5cc5 100644 >>> --- a/drivers/watchdog/wdt-uclass.c >>> +++ b/drivers/watchdog/wdt-uclass.c >>> @@ -27,7 +27,9 @@ static ulong reset_period = 1000; >>> int initr_watchdog(void) >>> { >>> u32 timeout = WATCHDOG_TIMEOUT_SECS; >>> +#ifdef CONFIG_WATCHDOG_AUTOSTART >>> int ret; >>> +#endif >> >> Please don't add more #ifdef's if possible, see below... >> >>> /* >>> * Init watchdog: This will call the probe function of the >>> @@ -51,6 +53,10 @@ int initr_watchdog(void) >>> 4 * reset_period) / 4; >>> } >>> +#ifndef CONFIG_WATCHDOG_AUTOSTART >>> + printf("WDT: Not starting\n"); >>> + return 0; >>> +#else >>> ret = wdt_start(gd->watchdog_dev, timeout * 1000, 0); >>> if (ret != 0) { >>> printf("WDT: Failed to start\n"); >>> @@ -61,6 +67,7 @@ int initr_watchdog(void) >>> IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", timeout); >>> return 0; >>> +#endif >> >> Please use this instead here: >> >> if (CONFIG_IS_ENABLED(WATCHDOG_AUTOSTART)) { >> ... >> >> Thanks, >> Stefan >> >>> } >>> int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags) >>> >> >> >> Viele Grüße, >> Stefan > > I have fixed these issues in V2. Thanks. Could you please next time when sending an updated patch series / version include the tags, like my RB tag? As patchwork collects these automatically and now I need to re-send them again - which I will do this time. ;) Thanks, Stefan
On Tuesday 09 March 2021 17:12:40 Stefan Roese wrote: > On 09.03.21 14:27, Pali Rohár wrote: > > On Tuesday 09 March 2021 12:33:24 Stefan Roese wrote: > > > On 05.03.21 22:36, Pali Rohár wrote: > > > > In some cases it is useful to compile support for U-Boot command 'wdt' > > > > without starting HW watchdog in early U-Boot phase. For example when user > > > > > > Nitpicking: > > > when "the" user > > > > > > > want to start watchdog only on demand by some boot script. > > > > > > > > This change adds a new compile option WATCHDOG_AUTOSTART to control whether > > > > U-Boot should automatically start watchdog during init phase or not. > > > > > > start "the" watchdog > > > > > > > > > > > This option is enabled by default as it was was default behavior prior > > > > > > as it was the default > > > > > > > introducing this new change. When compiling U-Boot users can decide to turn > > > > this option off. > > > > > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > > > --- > > > > drivers/watchdog/Kconfig | 13 +++++++++++++ > > > > drivers/watchdog/wdt-uclass.c | 7 +++++++ > > > > 2 files changed, 20 insertions(+) > > > > > > > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > > > > index 602ccbe41c00..aa76a8f2d239 100644 > > > > --- a/drivers/watchdog/Kconfig > > > > +++ b/drivers/watchdog/Kconfig > > > > @@ -9,6 +9,19 @@ config WATCHDOG > > > > this option if you want to service enabled watchdog by U-Boot. Disable > > > > this option if you want U-Boot to start watchdog but never service it. > > > > +config WATCHDOG_AUTOSTART > > > > + bool "Automatically start watchdog timer" > > > > + depends on WDT > > > > + default y > > > > + help > > > > + Automatically start watchdog timer and start servicing it during > > > > + init phase. Enabled by default. Disable this option if you want > > > > + to compile U-Boot with CONFIG_WDT support but do not want to > > > > + activate watchdog, like when CONFIG_WDT option is disabled. You > > > > + would be able to start watchdog manually by 'wdt' command. Useful > > > > + when you want to have support for 'wdt' command but do not want > > > > + to have watchdog enabled by default. > > > > + > > > > config WATCHDOG_TIMEOUT_MSECS > > > > int "Watchdog timeout in msec" > > > > default 128000 if ARCH_MX25 || ARCH_MX31 || ARCH_MX5 || ARCH_MX6 > > > > diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c > > > > index 7500b3ed90e3..00a408bf5cc5 100644 > > > > --- a/drivers/watchdog/wdt-uclass.c > > > > +++ b/drivers/watchdog/wdt-uclass.c > > > > @@ -27,7 +27,9 @@ static ulong reset_period = 1000; > > > > int initr_watchdog(void) > > > > { > > > > u32 timeout = WATCHDOG_TIMEOUT_SECS; > > > > +#ifdef CONFIG_WATCHDOG_AUTOSTART > > > > int ret; > > > > +#endif > > > > > > Please don't add more #ifdef's if possible, see below... > > > > > > > /* > > > > * Init watchdog: This will call the probe function of the > > > > @@ -51,6 +53,10 @@ int initr_watchdog(void) > > > > 4 * reset_period) / 4; > > > > } > > > > +#ifndef CONFIG_WATCHDOG_AUTOSTART > > > > + printf("WDT: Not starting\n"); > > > > + return 0; > > > > +#else > > > > ret = wdt_start(gd->watchdog_dev, timeout * 1000, 0); > > > > if (ret != 0) { > > > > printf("WDT: Failed to start\n"); > > > > @@ -61,6 +67,7 @@ int initr_watchdog(void) > > > > IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", timeout); > > > > return 0; > > > > +#endif > > > > > > Please use this instead here: > > > > > > if (CONFIG_IS_ENABLED(WATCHDOG_AUTOSTART)) { > > > ... > > > > > > Thanks, > > > Stefan > > > > > > > } > > > > int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags) > > > > > > > > > > > > > Viele Grüße, > > > Stefan > > > > I have fixed these issues in V2. > > Thanks. > > Could you please next time when sending an updated patch series / > version include the tags, like my RB tag? As patchwork collects > these automatically and now I need to re-send them again - which I > will do this time. ;) > > Thanks, > Stefan Ok, no problem!
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index 602ccbe41c00..aa76a8f2d239 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -9,6 +9,19 @@ config WATCHDOG this option if you want to service enabled watchdog by U-Boot. Disable this option if you want U-Boot to start watchdog but never service it. +config WATCHDOG_AUTOSTART + bool "Automatically start watchdog timer" + depends on WDT + default y + help + Automatically start watchdog timer and start servicing it during + init phase. Enabled by default. Disable this option if you want + to compile U-Boot with CONFIG_WDT support but do not want to + activate watchdog, like when CONFIG_WDT option is disabled. You + would be able to start watchdog manually by 'wdt' command. Useful + when you want to have support for 'wdt' command but do not want + to have watchdog enabled by default. + config WATCHDOG_TIMEOUT_MSECS int "Watchdog timeout in msec" default 128000 if ARCH_MX25 || ARCH_MX31 || ARCH_MX5 || ARCH_MX6 diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c index 7500b3ed90e3..00a408bf5cc5 100644 --- a/drivers/watchdog/wdt-uclass.c +++ b/drivers/watchdog/wdt-uclass.c @@ -27,7 +27,9 @@ static ulong reset_period = 1000; int initr_watchdog(void) { u32 timeout = WATCHDOG_TIMEOUT_SECS; +#ifdef CONFIG_WATCHDOG_AUTOSTART int ret; +#endif /* * Init watchdog: This will call the probe function of the @@ -51,6 +53,10 @@ int initr_watchdog(void) 4 * reset_period) / 4; } +#ifndef CONFIG_WATCHDOG_AUTOSTART + printf("WDT: Not starting\n"); + return 0; +#else ret = wdt_start(gd->watchdog_dev, timeout * 1000, 0); if (ret != 0) { printf("WDT: Failed to start\n"); @@ -61,6 +67,7 @@ int initr_watchdog(void) IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", timeout); return 0; +#endif } int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
In some cases it is useful to compile support for U-Boot command 'wdt' without starting HW watchdog in early U-Boot phase. For example when user want to start watchdog only on demand by some boot script. This change adds a new compile option WATCHDOG_AUTOSTART to control whether U-Boot should automatically start watchdog during init phase or not. This option is enabled by default as it was was default behavior prior introducing this new change. When compiling U-Boot users can decide to turn this option off. Signed-off-by: Pali Rohár <pali@kernel.org> --- drivers/watchdog/Kconfig | 13 +++++++++++++ drivers/watchdog/wdt-uclass.c | 7 +++++++ 2 files changed, 20 insertions(+)