[PATCH 1/1] sysreset: watchdog: watchdog cannot power off
The watchdog system reset driver can reboot the device but it cannot power it off. If power off is requested, the driver should not reset the system but leave powering off to one of the other system reset drivers.
As power cycling is typically not a feature of a watchdog driver the reset types SYSRESET_POWER and SYSRESET_POWER_OFF shall both be excluded.
Fixes: 17a0c14164dc ("dm: sysreset: add watchdog-reboot driver") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- drivers/sysreset/sysreset_watchdog.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/sysreset/sysreset_watchdog.c b/drivers/sysreset/sysreset_watchdog.c index 35efcac59d..8a659ee9b9 100644 --- a/drivers/sysreset/sysreset_watchdog.c +++ b/drivers/sysreset/sysreset_watchdog.c @@ -20,9 +20,16 @@ static int wdt_reboot_request(struct udevice *dev, enum sysreset_t type) struct wdt_reboot_plat *plat = dev_get_plat(dev); int ret;
- ret = wdt_expire_now(plat->wdt, 0); - if (ret) - return ret; + switch (type) { + case SYSRESET_COLD: + case SYSRESET_WARM: + ret = wdt_expire_now(plat->wdt, 0); + if (ret) + return ret; + break; + default: + return -ENOSYS; + }
return -EINPROGRESS; }
Hi Heinrich,
On Thu, 4 Nov 2021 at 10:31, Heinrich Schuchardt < heinrich.schuchardt@canonical.com> wrote:
The watchdog system reset driver can reboot the device but it cannot power it off. If power off is requested, the driver should not reset the system but leave powering off to one of the other system reset drivers.
As power cycling is typically not a feature of a watchdog driver the reset types SYSRESET_POWER and SYSRESET_POWER_OFF shall both be excluded.
A candid question here. I know that IPMI can powercycle a platform. How would we handle that should there be a way to make the power cycle happen? (for instance we could define a PSCI extension of SYSTEM_RESET2 to have a power cycle effect)
Fixes: 17a0c14164dc ("dm: sysreset: add watchdog-reboot driver") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
drivers/sysreset/sysreset_watchdog.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/sysreset/sysreset_watchdog.c b/drivers/sysreset/sysreset_watchdog.c index 35efcac59d..8a659ee9b9 100644 --- a/drivers/sysreset/sysreset_watchdog.c +++ b/drivers/sysreset/sysreset_watchdog.c @@ -20,9 +20,16 @@ static int wdt_reboot_request(struct udevice *dev, enum sysreset_t type) struct wdt_reboot_plat *plat = dev_get_plat(dev); int ret;
ret = wdt_expire_now(plat->wdt, 0);if (ret)return ret;
switch (type) {case SYSRESET_COLD:case SYSRESET_WARM:ret = wdt_expire_now(plat->wdt, 0);if (ret)return ret;break;default:return -ENOSYS;} return -EINPROGRESS;}
2.32.0
On 11/4/21 10:55, François Ozog wrote:
Hi Heinrich,
On Thu, 4 Nov 2021 at 10:31, Heinrich Schuchardt <heinrich.schuchardt@canonical.com mailto:heinrich.schuchardt@canonical.com> wrote:
The watchdog system reset driver can reboot the device but it cannot power it off. If power off is requested, the driver should not reset the system but leave powering off to one of the other system reset drivers. As power cycling is typically not a feature of a watchdog driver the reset types SYSRESET_POWER and SYSRESET_POWER_OFF shall both be excluded.A candid question here. I know that IPMI can powercycle a platform. How would we handle that should there be a way to make the power cycle happen? (for instance we could define a PSCI extension of SYSTEM_RESET2 to have a power cycle effect)
Your comment seems not to relate to the validity of the current patch.
For PSCI we have a separate sysreset driver drivers/sysreset/sysreset_psci.c. That driver currently does not support SYSRESET_POWER.
Once such an extension is offered by PSCI you should define a device-tree node property for it in Linux' Documentation/devicetree/bindings/arm/psci.yaml. Next implement a probe function in the driver code to detect that property. In psci_sysreset_request(), if type=SYSRESET_POWER, call the appropriate PSCI method if the extension is enabled or return -ENOSYS if the extension is not available.
Currently in U-Boot I see no code invoking a reset with SYSRESET_POWER. You would have to change function do_reset() in drivers/sysreset/sysreset-uclass.c to accept a new argument to select this reset type.
Best regards
Heinrich
Hi Heinrich,
Le jeu. 4 nov. 2021 à 12:45, Heinrich Schuchardt < heinrich.schuchardt@canonical.com> a écrit :
On 11/4/21 10:55, François Ozog wrote:
Hi Heinrich,
On Thu, 4 Nov 2021 at 10:31, Heinrich Schuchardt <heinrich.schuchardt@canonical.com mailto:heinrich.schuchardt@canonical.com> wrote:
The watchdog system reset driver can reboot the device but it cannot power it off. If power off is requested, the driver should not reset the system but leave powering off to one of the other system reset drivers. As power cycling is typically not a feature of a watchdog driver the reset types SYSRESET_POWER and SYSRESET_POWER_OFF shall both be excluded.A candid question here. I know that IPMI can powercycle a platform. How would we handle that should there be a way to make the power cycle happen? (for instance we could define a PSCI extension of SYSTEM_RESET2 to have a power cycle effect)
Your comment seems not to relate to the validity of the current patch.
yes. It was a related question to understand more about the topic.
For PSCI we have a separate sysreset driver drivers/sysreset/sysreset_psci.c. That driver currently does not support SYSRESET_POWER.
Once such an extension is offered by PSCI you should define a device-tree node property for it in Linux' Documentation/devicetree/bindings/arm/psci.yaml. Next implement a probe function in the driver code to detect that property. In psci_sysreset_request(), if type=SYSRESET_POWER, call the appropriate PSCI method if the extension is enabled or return -ENOSYS if the extension is not available.
Currently in U-Boot I see no code invoking a reset with SYSRESET_POWER. You would have to change function do_reset() in drivers/sysreset/sysreset-uclass.c to accept a new argument to select this reset type.
Best regards
Heinrich
On 04.11.21 10:31, Heinrich Schuchardt wrote:
The watchdog system reset driver can reboot the device but it cannot power it off. If power off is requested, the driver should not reset the system but leave powering off to one of the other system reset drivers.
As power cycling is typically not a feature of a watchdog driver the reset types SYSRESET_POWER and SYSRESET_POWER_OFF shall both be excluded.
Fixes: 17a0c14164dc ("dm: sysreset: add watchdog-reboot driver") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
drivers/sysreset/sysreset_watchdog.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/sysreset/sysreset_watchdog.c b/drivers/sysreset/sysreset_watchdog.c index 35efcac59d..8a659ee9b9 100644 --- a/drivers/sysreset/sysreset_watchdog.c +++ b/drivers/sysreset/sysreset_watchdog.c @@ -20,9 +20,16 @@ static int wdt_reboot_request(struct udevice *dev, enum sysreset_t type) struct wdt_reboot_plat *plat = dev_get_plat(dev); int ret;
- ret = wdt_expire_now(plat->wdt, 0);
- if (ret)
return ret;
switch (type) {
case SYSRESET_COLD:
case SYSRESET_WARM:
ret = wdt_expire_now(plat->wdt, 0);if (ret)return ret;break;default:
return -ENOSYS;}
return -EINPROGRESS; }
Viele Grüße, Stefan
participants (3)
-
François Ozog -
Heinrich Schuchardt -
Stefan Roese