Re: [PATCH v5 5/5] watchdog: aaeon: Add watchdog driver for SRG-IMX8P MCU

Guenter Roeck
2026-04-10T15:49:11Z

On 4/8/26 10:21, Thomas Perrot (Schneider Electric) wrote:
> Add watchdog driver for the Aaeon SRG-IMX8P embedded controller.
> This driver provides system monitoring and recovery capabilities
> through the MCU's watchdog timer.
>
> The watchdog supports start, stop, and ping operations with a maximum
> hardware heartbeat of 25 seconds and a default timeout of 240 seconds.
>
> Co-developed-by: Jérémie Dautheribes (Schneider Electric) <jeremie.dautheribes@bootlin.com>
> Signed-off-by: Jérémie Dautheribes (Schneider Electric) <jeremie.dautheribes@bootlin.com>
> Signed-off-by: Thomas Perrot (Schneider Electric) <thomas.perrot@bootlin.com>
> ---
> MAINTAINERS | 1 +
> drivers/watchdog/Kconfig | 10 +++
> drivers/watchdog/Makefile | 1 +
> drivers/watchdog/aaeon_mcu_wdt.c | 132 +++++++++++++++++++++++++++++++++++++++
> 4 files changed, 144 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2538f8c4bc14..7b92af42c9fd 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -193,6 +193,7 @@ S: Maintained
> F: Documentation/devicetree/bindings/mfd/aaeon,srg-imx8p-mcu.yaml
> F: drivers/gpio/gpio-aaeon-mcu.c
> F: drivers/mfd/aaeon-mcu.c
> +F: drivers/watchdog/aaeon_mcu_wdt.c
> F: include/linux/mfd/aaeon-mcu.h
>
> AAEON UPBOARD FPGA MFD DRIVER
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index d3b9df7d466b..f67a0b453316 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -420,6 +420,16 @@ config SL28CPLD_WATCHDOG
>
> # ARM Architecture
>
> +config AAEON_MCU_WATCHDOG
> + tristate "Aaeon MCU Watchdog"
> + depends on MFD_AAEON_MCU
> + select WATCHDOG_CORE
> + help
> + Select this option to enable watchdog timer support for the Aaeon
> + SRG-IMX8P onboard microcontroller (MCU). This driver provides
> + watchdog functionality through the MCU, allowing system monitoring
> + and automatic recovery from system hangs.
> +
> config AIROHA_WATCHDOG
> tristate "Airoha EN7581 Watchdog"
> depends on ARCH_AIROHA || COMPILE_TEST
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index ba52099b1253..2deec425d3ea 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -37,6 +37,7 @@ obj-$(CONFIG_USBPCWATCHDOG) += pcwd_usb.o
> # ALPHA Architecture
>
> # ARM Architecture
> +obj-$(CONFIG_AAEON_MCU_WATCHDOG) += aaeon_mcu_wdt.o
> obj-$(CONFIG_ARM_SP805_WATCHDOG) += sp805_wdt.o
> obj-$(CONFIG_ARM_SBSA_WATCHDOG) += sbsa_gwdt.o
> obj-$(CONFIG_ARMADA_37XX_WATCHDOG) += armada_37xx_wdt.o
> diff --git a/drivers/watchdog/aaeon_mcu_wdt.c b/drivers/watchdog/aaeon_mcu_wdt.c
> new file mode 100644
> index 000000000000..949b506d8194
> --- /dev/null
> +++ b/drivers/watchdog/aaeon_mcu_wdt.c
> @@ -0,0 +1,132 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Aaeon MCU Watchdog driver
> + *
> + * Copyright (C) 2026 Bootlin
> + * Author: Jérémie Dautheribes <jeremie.dautheribes@bootlin.com>
> + * Author: Thomas Perrot <thomas.perrot@bootlin.com>
> + */
> +
> +#include <linux/mfd/aaeon-mcu.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/watchdog.h>
> +
> +#define AAEON_MCU_PING_WDT 0x73
> +
> +#define AAEON_MCU_WDT_TIMEOUT 240
> +#define AAEON_MCU_WDT_HEARTBEAT_MS 25000
> +
> +struct aaeon_mcu_wdt {
> + struct watchdog_device wdt;
> + struct regmap *regmap;
> +};
> +
> +static int aaeon_mcu_wdt_cmd(struct aaeon_mcu_wdt *data, u8 opcode, u8 arg)
> +{
> + return regmap_write(data->regmap, AAEON_MCU_REG(opcode, arg), 0);
> +}
> +
> +static int aaeon_mcu_wdt_start(struct watchdog_device *wdt)
> +{
> + struct aaeon_mcu_wdt *data = watchdog_get_drvdata(wdt);
> +
> + return aaeon_mcu_wdt_cmd(data, AAEON_MCU_CONTROL_WDT_OPCODE, 0x01);
> +}
> +
> +static int aaeon_mcu_wdt_status(struct watchdog_device *wdt, bool *enabled)
> +{
> + struct aaeon_mcu_wdt *data = watchdog_get_drvdata(wdt);
> + unsigned int rsp;
> + int ret;
> +
> + ret = regmap_read(data->regmap,
> + AAEON_MCU_REG(AAEON_MCU_CONTROL_WDT_OPCODE, 0x02),
> + &rsp);
> + if (ret)
> + return ret;
> +
> + *enabled = rsp == 0x01;
> + return 0;
> +}
> +
> +static int aaeon_mcu_wdt_stop(struct watchdog_device *wdt)
> +{
> + struct aaeon_mcu_wdt *data = watchdog_get_drvdata(wdt);
> +
> + return aaeon_mcu_wdt_cmd(data, AAEON_MCU_CONTROL_WDT_OPCODE, 0x00);
> +}
> +
> +static int aaeon_mcu_wdt_ping(struct watchdog_device *wdt)
> +{
> + struct aaeon_mcu_wdt *data = watchdog_get_drvdata(wdt);
> +
> + return aaeon_mcu_wdt_cmd(data, AAEON_MCU_PING_WDT, 0x00);
> +}
> +
> +static const struct watchdog_info aaeon_mcu_wdt_info = {
> + .identity = "Aaeon MCU Watchdog",
> + .options = WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE
> +};
> +
> +static const struct watchdog_ops aaeon_mcu_wdt_ops = {
> + .owner = THIS_MODULE,
> + .start = aaeon_mcu_wdt_start,
> + .stop = aaeon_mcu_wdt_stop,
> + .ping = aaeon_mcu_wdt_ping,
> +};
> +
> +static int aaeon_mcu_wdt_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct watchdog_device *wdt;
> + struct aaeon_mcu_wdt *data;
> + bool enabled;
> + int ret;
> +
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->regmap = dev_get_regmap(dev->parent, NULL);
> + if (!data->regmap)
> + return -ENODEV;
> +
> + wdt = &data->wdt;
> + wdt->parent = dev;
> + wdt->info = &aaeon_mcu_wdt_info;
> + wdt->ops = &aaeon_mcu_wdt_ops;
> + /*
> + * The MCU firmware has a fixed hardware timeout of 25 seconds that
> + * cannot be changed. The watchdog core will handle automatic pinging
> + * to support longer timeouts. The software timeout of 240 seconds is
> + * chosen arbitrarily as a reasonable value and is not user-configurable.
> + */

Odd, unusual, unnecessary, I would argue that most people would consider a fixed
timeout of 240s as anything but reasonable, and as the comment says arbitrary.
Since I am sure that I pointed this out before, you still insist, and I am
tired of arguing: Your funeral, so

Acked-by: Guenter Roeck <linux@roeck-us.net>

Guenter

> + wdt->timeout = AAEON_MCU_WDT_TIMEOUT;
> + wdt->max_hw_heartbeat_ms = AAEON_MCU_WDT_HEARTBEAT_MS;
> +
> + watchdog_set_drvdata(wdt, data);
> +
> + ret = aaeon_mcu_wdt_status(wdt, &enabled);
> + if (ret)
> + return ret;
> +
> + if (enabled)
> + set_bit(WDOG_HW_RUNNING, &wdt->status);
> +
> + return devm_watchdog_register_device(dev, wdt);
> +}
> +
> +static struct platform_driver aaeon_mcu_wdt_driver = {
> + .driver = {
> + .name = "aaeon-mcu-wdt",
> + },
> + .probe = aaeon_mcu_wdt_probe,
> +};
> +
> +module_platform_driver(aaeon_mcu_wdt_driver);
> +
> +MODULE_DESCRIPTION("Aaeon MCU Watchdog Driver");
> +MODULE_AUTHOR("Jérémie Dautheribes <jeremie.dautheribes@bootlin.com>");
> +MODULE_LICENSE("GPL");
>