[PATCH v2] watchdog: arm_smc_wdt: add watchdog support

[Lionel DEBIEVE]

Hi Stephan,


On 4/11/23 11:29, Stefan Roese wrote:
> Hi Lionel,
>
> On 4/5/23 11:53, 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>
>> Reviewed-by: Patrick Delaunay <patrick.delaunay at foss.st.com>
>> Tested-by: Patrick Delaunay <patrick.delaunay at foss.st.com>
>> ---
>>
>> (no changes since v1)
>
> Hmm, this does not seem to be correct. Could you please include the list
> of changed stuff here to make reviewing easier?

Thanks, sorry fir the change list, issue on my side with the patman 
options ;)

I'll just add a minor change regarding log_err in the probe and send a v3.

Thanks,

Br,

Lionel

>
> Other than this:
>
> Reviewed-by: Stefan Roese <sr at denx.de>
>
> Thanks,
> Stefan
>
>>   drivers/watchdog/Kconfig       |   8 +++
>>   drivers/watchdog/Makefile      |   1 +
>>   drivers/watchdog/arm_smc_wdt.c | 121 +++++++++++++++++++++++++++++++++
>>   3 files changed, 130 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..70ab99bdbfb
>> --- /dev/null
>> +++ b/drivers/watchdog/arm_smc_wdt.c
>> @@ -0,0 +1,121 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * 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
>> + */
>> +
>> +#define LOG_CATEGORY UCLASS_WDT
>> +
>> +#include <dm.h>
>> +#include <dm/device_compat.h>
>> +#include <linux/arm-smccc.h>
>> +#include <linux/psci.h>
>> +#include <wdt.h>
>> +
>> +#define DRV_NAME        "arm_smc_wdt"
>> +
>> +#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 smcwd_priv_data *priv = dev_get_priv(dev);
>> +    struct arm_smccc_res local_res;
>> +
>> +    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)
>> +{
>> +    struct smcwd_priv_data *priv = dev_get_priv(dev);
>> +    u64 timeout_sec = WDT_TIMEOUT_SECS(timeout_ms);
>> +    int err;
>> +
>> +    if (timeout_sec < priv->min_timeout || timeout_sec > 
>> priv->max_timeout)    {
>> +        dev_err(dev, "Timeout value not supported\n");
>> +        return -EINVAL;
>> +    }
>> +
>> +    err = smcwd_call(dev, SMCWD_SET_TIMEOUT, timeout_sec, NULL);
>> +    if (err) {
>> +        dev_err(dev, "Timeout out configuration failed\n");
>> +        return err;
>> +    }
>> +
>> +    return smcwd_call(dev, SMCWD_ENABLE, 1, NULL);
>> +}
>> +
>> +static int smcwd_probe(struct udevice *dev)
>> +{
>> +    struct smcwd_priv_data *priv = dev_get_priv(dev);
>> +    struct arm_smccc_res res;
>> +    int err;
>> +
>> +    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
>