| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <8ca53787-5968-41f9-a605-03e58a8c4565@roeck-us.net>
Date: Tue, 3 Feb 2026 09:43:41 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: "Thomas Perrot (Schneider Electric)" <thomas.perrot@...tlin.com>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Linus Walleij <linusw@...nel.org>,
Bartosz Golaszewski <brgl@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>,
Jérémie Dautheribes <jeremie.dautheribes@...tlin.com>,
Wim Van Sebroeck <wim@...ux-watchdog.org>, Lee Jones <lee@...nel.org>
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-gpio@...r.kernel.org, imx@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org, linux-watchdog@...r.kernel.org,
Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH v3 5/5] watchdog: aaeon: Add watchdog driver for SRG-IMX8P
MCU
On 2/3/26 08: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.
> The driver assumes the watchdog is already running at probe time, as
> the MCU typically enables it by default.
>
> Co-developed-by: Jérémie Dautheribes (Schneider Electric) <jeremie.dautheribes@...tlin.com>
> Signed-off-by: Jérémie Dautheribes (Schneider Electric) <jeremie.dautheribes@...tlin.com>
> Signed-off-by: Thomas Perrot (Schneider Electric) <thomas.perrot@...tlin.com>
> ---
> MAINTAINERS | 1 +
> drivers/watchdog/Kconfig | 10 +++
> drivers/watchdog/Makefile | 1 +
> drivers/watchdog/aaeon_mcu_wdt.c | 136 +++++++++++++++++++++++++++++++++++++++
> 4 files changed, 148 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2538f8c4bc1482b139e18243a68f0a21b9be3704..7b92af42c9fdc17a69a4e7a2fe50f9e199c8b144 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 d3b9df7d466b0b7215ee87b3040811d44ee53d2a..0835df4c902f059c0d61a6e8d884742dd7d2f741 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -168,6 +168,16 @@ config SOFT_WATCHDOG_PRETIMEOUT
> watchdog. Be aware that governors might affect the watchdog because it
> is purely software, e.g. the panic governor will stall it!
>
> +config AAEON_MCU_WATCHDOG
> + tristate "Aaeon MCU Watchdog"
> + depends on MFD_AAEON_MCU || COMPILE_TEST
> + 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 BD957XMUF_WATCHDOG
> tristate "ROHM BD9576MUF and BD9573MUF PMIC Watchdog"
> depends on MFD_ROHM_BD957XMUF
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index ba52099b125398a32f80dad23317e223cc4af028..2deec425d3eafb6b208e061fda9f216f4baa8ecc 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 0000000000000000000000000000000000000000..416f3035c3226c3889682102d1d2453a9365b5ba
> --- /dev/null
> +++ b/drivers/watchdog/aaeon_mcu_wdt.c
> @@ -0,0 +1,136 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Aaeon MCU Watchdog driver
> + *
> + * Copyright (C) 2025 Bootlin
> + * Author: Jérémie Dautheribes <jeremie.dautheribes@...tlin.com>
> + * Author: Thomas Perrot <thomas.perrot@...tlin.com>
> + */
> +
> +#include <linux/mfd/aaeon-mcu.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/watchdog.h>
> +
> +#define AAEON_MCU_CONTROL_WDT 0x63
> +#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 device *dev;
> +};
> +
> +static int aaeon_mcu_wdt_cmd(struct device *dev, u8 opcode, u8 arg)
> +{
> + u8 cmd[3] = { opcode, arg, 0x00 };
> + u8 rsp;
> +
> + return aaeon_mcu_i2c_xfer(dev, cmd, sizeof(cmd), &rsp, sizeof(rsp));
This warrants a comment explaining why "rsp" is irrelevant.
> +}
> +
> +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->dev, AAEON_MCU_CONTROL_WDT, 0x01);
> +}
> +
> +static int aaeon_mcu_wdt_status(struct watchdog_device *wdt, bool *enabled)
> +{
> + struct aaeon_mcu_wdt *data = watchdog_get_drvdata(wdt);
> + u8 cmd[3], rsp;
Not that it matters much, but for consistency it would be nice to use the
same pattern as above and initialize cmd here.
> + int ret;
> +
> + cmd[0] = AAEON_MCU_CONTROL_WDT;
> + cmd[1] = 0x02;
> + cmd[2] = 0x00;
> +
> + ret = aaeon_mcu_i2c_xfer(data->dev, cmd, sizeof(cmd), &rsp, sizeof(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->dev, AAEON_MCU_CONTROL_WDT, 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->dev, AAEON_MCU_PING_WDT, 0x00);
> +}
> +
> +static const struct watchdog_info aaeon_mcu_wdt_info = {
> + .identity = "Aaeon MCU Watchdog",
> + .options = WDIOF_KEEPALIVEPING
> +};
> +
> +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->dev = dev->parent;
> +
> + 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 default timeout of 240 seconds is
> + * chosen arbitrarily as a reasonable value; users can adjust it via
> + * the standard watchdog interface if needed.
No, they can't, because WDIOF_SETTIMEOUT is not set in .options. Also,
the above implies that the minimum timeout is 25 seconds, which is not set.
Not that it matters, because the timeout can not be changed in the first
place, but still ...
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@...tlin.com>");
> +MODULE_LICENSE("GPL");
>
Powered by blists - more mailing lists