[PATCH] watchdog: arm_smc_wdt: add watchdog support

[Stefan Roese]

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 at 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

-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de