[PATCH] watchdog: arm_smc_wdt: add watchdog support

Implement a ARM SMCCC based driver that allow to use a secure watchdog on the platform.
Signed-off-by: Lionel Debieve lionel.debieve@foss.st.com ---
drivers/watchdog/Kconfig | 8 +++ drivers/watchdog/Makefile | 1 + drivers/watchdog/arm_smc_wdt.c | 116 +++++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+) create mode 100644 drivers/watchdog/arm_smc_wdt.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index b5ac8f7f50d..3a0341f609d 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -352,6 +352,14 @@ config WDT_TANGIER Intel Tangier SoC. If you're using a board with Intel Tangier SoC, say Y here.
+config WDT_ARM_SMC + bool "ARM SMC watchdog timer support" + depends on WDT && ARM_SMCCC + imply WATCHDOG + help + Select this to enable Arm SMC watchdog timer. This watchdog will manage + a watchdog based on ARM SMCCC communication. + config SPL_WDT bool "Enable driver model for watchdog timer drivers in SPL" depends on SPL_DM diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index 446d961d7d2..a4633c0d2fa 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -18,6 +18,7 @@ obj-$(CONFIG_$(SPL_TPL_)WDT) += wdt-uclass.o obj-$(CONFIG_WDT_SANDBOX) += sandbox_wdt.o obj-$(CONFIG_WDT_ALARM_SANDBOX) += sandbox_alarm-wdt.o obj-$(CONFIG_WDT_APPLE) += apple_wdt.o +obj-$(CONFIG_WDT_ARM_SMC) += arm_smc_wdt.o obj-$(CONFIG_WDT_ARMADA_37XX) += armada-37xx-wdt.o obj-$(CONFIG_WDT_ASPEED) += ast_wdt.o obj-$(CONFIG_WDT_AST2600) += ast2600_wdt.o diff --git a/drivers/watchdog/arm_smc_wdt.c b/drivers/watchdog/arm_smc_wdt.c new file mode 100644 index 00000000000..e2e3c455082 --- /dev/null +++ b/drivers/watchdog/arm_smc_wdt.c @@ -0,0 +1,116 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * ARM Secure Monitor Call watchdog driver + * Copyright (C) 2022, STMicroelectronics - All Rights Reserved + */ + +#include <dm.h> +#include <dm/device_compat.h> +#include <dm/of_access.h> +#include <linux/arm-smccc.h> +#include <linux/psci.h> +#include <wdt.h> + +#define DRV_NAME "arm_smc_wdt" +#define DRV_VERSION "1.0" + +#define WDT_TIMEOUT_SECS(TIMEOUT) ((TIMEOUT) / 1000) + +enum smcwd_call { + SMCWD_INIT = 0, + SMCWD_SET_TIMEOUT = 1, + SMCWD_ENABLE = 2, + SMCWD_PET = 3, + SMCWD_GET_TIMELEFT = 4, +}; + +struct smcwd_priv_data { + u32 smc_id; + unsigned int min_timeout; + unsigned int max_timeout; +}; + +static int smcwd_call(struct udevice *dev, enum smcwd_call call, + unsigned long arg, struct arm_smccc_res *res) +{ + struct arm_smccc_res local_res; + struct smcwd_priv_data *priv = dev_get_priv(dev); + + if (!res) + res = &local_res; + + arm_smccc_smc(priv->smc_id, call, arg, 0, 0, 0, 0, 0, res); + + if (res->a0 == PSCI_RET_NOT_SUPPORTED) + return -ENODEV; + if (res->a0 == PSCI_RET_INVALID_PARAMS) + return -EINVAL; + if (res->a0 != PSCI_RET_SUCCESS) + return -EIO; + + return 0; +} + +static int smcwd_reset(struct udevice *dev) +{ + return smcwd_call(dev, SMCWD_PET, 0, NULL); +} + +static int smcwd_stop(struct udevice *dev) +{ + return smcwd_call(dev, SMCWD_ENABLE, 0, NULL); +} + +static int smcwd_start(struct udevice *dev, u64 timeout_ms, ulong flags) +{ + int err; + u64 timeout_sec = WDT_TIMEOUT_SECS(timeout_ms); + struct smcwd_priv_data *priv = dev_get_priv(dev); + + if (timeout_sec < priv->min_timeout || timeout_sec > priv->max_timeout) + return -EINVAL; + + err = smcwd_call(dev, SMCWD_SET_TIMEOUT, timeout_sec, NULL); + if (err) + return err; + + return smcwd_call(dev, SMCWD_ENABLE, 1, NULL); +} + +static int smcwd_probe(struct udevice *dev) +{ + int err; + struct arm_smccc_res res; + struct smcwd_priv_data *priv = dev_get_priv(dev); + + priv->smc_id = dev_read_u32_default(dev, "arm,smc-id", 0x82003D06); + + err = smcwd_call(dev, SMCWD_INIT, 0, &res); + if (err < 0) + return err; + + priv->min_timeout = res.a1; + priv->max_timeout = res.a2; + + return 0; +} + +static const struct wdt_ops smcwd_ops = { + .start = smcwd_start, + .stop = smcwd_stop, + .reset = smcwd_reset, +}; + +static const struct udevice_id smcwd_dt_ids[] = { + { .compatible = "arm,smc-wdt" }, + {} +}; + +U_BOOT_DRIVER(wdt_sandbox) = { + .name = "smcwd", + .id = UCLASS_WDT, + .of_match = smcwd_dt_ids, + .priv_auto = sizeof(struct smcwd_priv_data), + .probe = smcwd_probe, + .ops = &smcwd_ops, +};

Hi Lionel,
On 3/31/23 09:14, Lionel Debieve wrote:
Implement a ARM SMCCC based driver that allow to use a secure watchdog on the platform.
Signed-off-by: Lionel Debieve lionel.debieve@foss.st.com
drivers/watchdog/Kconfig | 8 +++ drivers/watchdog/Makefile | 1 + drivers/watchdog/arm_smc_wdt.c | 116 +++++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+) create mode 100644 drivers/watchdog/arm_smc_wdt.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index b5ac8f7f50d..3a0341f609d 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -352,6 +352,14 @@ config WDT_TANGIER Intel Tangier SoC. If you're using a board with Intel Tangier SoC, say Y here.
+config WDT_ARM_SMC
- bool "ARM SMC watchdog timer support"
- depends on WDT && ARM_SMCCC
- imply WATCHDOG
- help
Select this to enable Arm SMC watchdog timer. This watchdog will manage
a watchdog based on ARM SMCCC communication.
- config SPL_WDT bool "Enable driver model for watchdog timer drivers in SPL" depends on SPL_DM
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index 446d961d7d2..a4633c0d2fa 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -18,6 +18,7 @@ obj-$(CONFIG_$(SPL_TPL_)WDT) += wdt-uclass.o obj-$(CONFIG_WDT_SANDBOX) += sandbox_wdt.o obj-$(CONFIG_WDT_ALARM_SANDBOX) += sandbox_alarm-wdt.o obj-$(CONFIG_WDT_APPLE) += apple_wdt.o +obj-$(CONFIG_WDT_ARM_SMC) += arm_smc_wdt.o obj-$(CONFIG_WDT_ARMADA_37XX) += armada-37xx-wdt.o obj-$(CONFIG_WDT_ASPEED) += ast_wdt.o obj-$(CONFIG_WDT_AST2600) += ast2600_wdt.o diff --git a/drivers/watchdog/arm_smc_wdt.c b/drivers/watchdog/arm_smc_wdt.c new file mode 100644 index 00000000000..e2e3c455082 --- /dev/null +++ b/drivers/watchdog/arm_smc_wdt.c @@ -0,0 +1,116 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- ARM Secure Monitor Call watchdog driver
- Copyright (C) 2022, STMicroelectronics - All Rights Reserved
- */
+#include <dm.h> +#include <dm/device_compat.h> +#include <dm/of_access.h> +#include <linux/arm-smccc.h> +#include <linux/psci.h> +#include <wdt.h>
+#define DRV_NAME "arm_smc_wdt" +#define DRV_VERSION "1.0"
+#define WDT_TIMEOUT_SECS(TIMEOUT) ((TIMEOUT) / 1000)
+enum smcwd_call {
- SMCWD_INIT = 0,
- SMCWD_SET_TIMEOUT = 1,
- SMCWD_ENABLE = 2,
- SMCWD_PET = 3,
- SMCWD_GET_TIMELEFT = 4,
+};
+struct smcwd_priv_data {
- u32 smc_id;
- unsigned int min_timeout;
- unsigned int max_timeout;
+};
+static int smcwd_call(struct udevice *dev, enum smcwd_call call,
unsigned long arg, struct arm_smccc_res *res)
+{
- struct arm_smccc_res local_res;
- struct smcwd_priv_data *priv = dev_get_priv(dev);
- if (!res)
res = &local_res;
- arm_smccc_smc(priv->smc_id, call, arg, 0, 0, 0, 0, 0, res);
- if (res->a0 == PSCI_RET_NOT_SUPPORTED)
return -ENODEV;
- if (res->a0 == PSCI_RET_INVALID_PARAMS)
return -EINVAL;
- if (res->a0 != PSCI_RET_SUCCESS)
return -EIO;
- return 0;
+}
+static int smcwd_reset(struct udevice *dev) +{
- return smcwd_call(dev, SMCWD_PET, 0, NULL);
+}
+static int smcwd_stop(struct udevice *dev) +{
- return smcwd_call(dev, SMCWD_ENABLE, 0, NULL);
+}
+static int smcwd_start(struct udevice *dev, u64 timeout_ms, ulong flags) +{
- int err;
- u64 timeout_sec = WDT_TIMEOUT_SECS(timeout_ms);
- struct smcwd_priv_data *priv = dev_get_priv(dev);
Nitpicking: It's generally more common to use the reverse xmas tree style for variable declaration ordering. On other places in this code as well.
- if (timeout_sec < priv->min_timeout || timeout_sec > priv->max_timeout)
return -EINVAL;
An error message would be good here. Or at least some debug level message.
- err = smcwd_call(dev, SMCWD_SET_TIMEOUT, timeout_sec, NULL);
- if (err)
return err;
Again, please enhance the driver with some error or at least debug level logging.
Thanks, Stefan
- return smcwd_call(dev, SMCWD_ENABLE, 1, NULL);
+}
+static int smcwd_probe(struct udevice *dev) +{
- int err;
- struct arm_smccc_res res;
- struct smcwd_priv_data *priv = dev_get_priv(dev);
- priv->smc_id = dev_read_u32_default(dev, "arm,smc-id", 0x82003D06);
- err = smcwd_call(dev, SMCWD_INIT, 0, &res);
- if (err < 0)
return err;
- priv->min_timeout = res.a1;
- priv->max_timeout = res.a2;
- return 0;
+}
+static const struct wdt_ops smcwd_ops = {
- .start = smcwd_start,
- .stop = smcwd_stop,
- .reset = smcwd_reset,
+};
+static const struct udevice_id smcwd_dt_ids[] = {
- { .compatible = "arm,smc-wdt" },
- {}
+};
+U_BOOT_DRIVER(wdt_sandbox) = {
- .name = "smcwd",
- .id = UCLASS_WDT,
- .of_match = smcwd_dt_ids,
- .priv_auto = sizeof(struct smcwd_priv_data),
- .probe = smcwd_probe,
- .ops = &smcwd_ops,
+};
Viele Grüße, Stefan Roese

Hi,
On 3/31/23 09:59, Stefan Roese wrote:
Hi Lionel,
On 3/31/23 09:14, Lionel Debieve wrote:
Implement a ARM SMCCC based driver that allow to use a secure watchdog on the platform.
Signed-off-by: Lionel Debieve lionel.debieve@foss.st.com
drivers/watchdog/Kconfig | 8 +++ drivers/watchdog/Makefile | 1 + drivers/watchdog/arm_smc_wdt.c | 116 +++++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+) create mode 100644 drivers/watchdog/arm_smc_wdt.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index b5ac8f7f50d..3a0341f609d 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -352,6 +352,14 @@ config WDT_TANGIER Intel Tangier SoC. If you're using a board with Intel Tangier SoC, say Y here. +config WDT_ARM_SMC + bool "ARM SMC watchdog timer support" + depends on WDT && ARM_SMCCC + imply WATCHDOG + help + Select this to enable Arm SMC watchdog timer. This watchdog will manage + a watchdog based on ARM SMCCC communication.
config SPL_WDT bool "Enable driver model for watchdog timer drivers in SPL" depends on SPL_DM diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index 446d961d7d2..a4633c0d2fa 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -18,6 +18,7 @@ obj-$(CONFIG_$(SPL_TPL_)WDT) += wdt-uclass.o obj-$(CONFIG_WDT_SANDBOX) += sandbox_wdt.o obj-$(CONFIG_WDT_ALARM_SANDBOX) += sandbox_alarm-wdt.o obj-$(CONFIG_WDT_APPLE) += apple_wdt.o +obj-$(CONFIG_WDT_ARM_SMC) += arm_smc_wdt.o obj-$(CONFIG_WDT_ARMADA_37XX) += armada-37xx-wdt.o obj-$(CONFIG_WDT_ASPEED) += ast_wdt.o obj-$(CONFIG_WDT_AST2600) += ast2600_wdt.o diff --git a/drivers/watchdog/arm_smc_wdt.c b/drivers/watchdog/arm_smc_wdt.c new file mode 100644 index 00000000000..e2e3c455082 --- /dev/null +++ b/drivers/watchdog/arm_smc_wdt.c @@ -0,0 +1,116 @@ +// SPDX-License-Identifier: GPL-2.0+
Please replace with correct SPDX identifier for new file => GPL-2.0-or-later
GPL-2.0+ identifier is deprecated as indicated in https://spdx.org/licenses/
+/*
- ARM Secure Monitor Call watchdog driver
- Copyright (C) 2022, STMicroelectronics - All Rights Reserved
+ * This file is based on Linux driver drivers/watchdog/arm_smc_wdt.c
?
- */
please add the LOG define for this new driver:
#define LOG_CATEGORY UCLASS_WDT
+#include <dm.h> +#include <dm/device_compat.h>
dm/device_compat.h is needed ?
no call for dev debug function (printk / dev_err() /...)
+#include <dm/of_access.h>
of_access.h is really needed ?
no of_XXX() function call...
+#include <linux/arm-smccc.h> +#include <linux/psci.h> +#include <wdt.h>
+#define DRV_NAME "arm_smc_wdt" +#define DRV_VERSION "1.0"
NITS: DRV_VERSION is not used,
its is only for alignment with kernel driver ?
+#define WDT_TIMEOUT_SECS(TIMEOUT) ((TIMEOUT) / 1000)
+enum smcwd_call { + SMCWD_INIT = 0, + SMCWD_SET_TIMEOUT = 1, + SMCWD_ENABLE = 2, + SMCWD_PET = 3, + SMCWD_GET_TIMELEFT = 4, +};
+struct smcwd_priv_data { + u32 smc_id; + unsigned int min_timeout; + unsigned int max_timeout; +};
+static int smcwd_call(struct udevice *dev, enum smcwd_call call, + unsigned long arg, struct arm_smccc_res *res) +{ + struct arm_smccc_res local_res; + struct smcwd_priv_data *priv = dev_get_priv(dev);
+ if (!res) + res = &local_res;
+ arm_smccc_smc(priv->smc_id, call, arg, 0, 0, 0, 0, 0, res);
+ if (res->a0 == PSCI_RET_NOT_SUPPORTED) + return -ENODEV; + if (res->a0 == PSCI_RET_INVALID_PARAMS) + return -EINVAL; + if (res->a0 != PSCI_RET_SUCCESS) + return -EIO;
+ return 0; +}
+static int smcwd_reset(struct udevice *dev) +{ + return smcwd_call(dev, SMCWD_PET, 0, NULL); +}
+static int smcwd_stop(struct udevice *dev) +{ + return smcwd_call(dev, SMCWD_ENABLE, 0, NULL); +}
+static int smcwd_start(struct udevice *dev, u64 timeout_ms, ulong flags) +{ + int err; + u64 timeout_sec = WDT_TIMEOUT_SECS(timeout_ms); + struct smcwd_priv_data *priv = dev_get_priv(dev);
Nitpicking: It's generally more common to use the reverse xmas tree style for variable declaration ordering. On other places in this code as well.
+ if (timeout_sec < priv->min_timeout || timeout_sec > priv->max_timeout) + return -EINVAL;
An error message would be good here. Or at least some debug level message.
+ err = smcwd_call(dev, SMCWD_SET_TIMEOUT, timeout_sec, NULL); + if (err) + return err;
Again, please enhance the driver with some error or at least debug level logging.
Thanks, Stefan
+ return smcwd_call(dev, SMCWD_ENABLE, 1, NULL); +}
+static int smcwd_probe(struct udevice *dev) +{ + int err; + struct arm_smccc_res res; + struct smcwd_priv_data *priv = dev_get_priv(dev);
+ priv->smc_id = dev_read_u32_default(dev, "arm,smc-id", 0x82003D06);
+ err = smcwd_call(dev, SMCWD_INIT, 0, &res); + if (err < 0) + return err;
+ priv->min_timeout = res.a1; + priv->max_timeout = res.a2;
+ return 0; +}
+static const struct wdt_ops smcwd_ops = { + .start = smcwd_start, + .stop = smcwd_stop, + .reset = smcwd_reset, +};
+static const struct udevice_id smcwd_dt_ids[] = { + { .compatible = "arm,smc-wdt" }, + {} +};
+U_BOOT_DRIVER(wdt_sandbox) = { + .name = "smcwd", + .id = UCLASS_WDT, + .of_match = smcwd_dt_ids, + .priv_auto = sizeof(struct smcwd_priv_data), + .probe = smcwd_probe, + .ops = &smcwd_ops, +};
Viele Grüße, Stefan Roese
even with few NIT
Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com
Tested-by: Patrick Delaunay patrick.delaunay@foss.st.com
On STM32MP13x platform with watchdog managed in OP-TEE.
Thanks Patrick
participants (3)
-
Lionel Debieve
-
Patrick DELAUNAY
-
Stefan Roese