| 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: <f8af59e1-29a0-4b32-929a-b30580dad62a@roeck-us.net>
Date: Fri, 23 Jan 2026 08:11:36 -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 v2 5/5] watchdog: aaeon: Add watchdog driver for
SRG-IMX8PL MCU
On 1/23/26 01:54, Thomas Perrot (Schneider Electric) wrote:
> Add watchdog driver for the Aaeon SRG-IMX8PL 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 | 110 +++++++++++++++++++++++++++++++++++++++
> 4 files changed, 122 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 28dd964cdf69bdcaec3eb82d6df851a2bad47415..66cc4a5dcbe0977e6baee6d3b5e6023dcc06847b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -193,6 +193,7 @@ S: Maintained
> F: Documentation/devicetree/bindings/mfd/aaeon,srg-imx8pl-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..1bd4a7bee303e5e2508f540dc2c16e9e19ed18b0 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
> + select WATCHDOG_CORE
> + help
> + Select this option to enable watchdog timer support for the Aaeon
> + SRG-IMX8PL 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..99be3ce327f44063fce6ba77a7d4efcba34361df
> --- /dev/null
> +++ b/drivers/watchdog/aaeon_mcu_wdt.c
> @@ -0,0 +1,110 @@
> +// 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));
> +}
> +
> +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_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;
> + 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;
> + wdt->timeout = AAEON_MCU_WDT_TIMEOUT;
> + wdt->max_hw_heartbeat_ms = AAEON_MCU_WDT_HEARTBEAT_MS;
If this is the only supported timeout, and the timeout can not be updated,
that should be explained in the code. That explanation will need to include
the reason why the timeout is being fixed to 240 seconds and can not be
updated, even in software. Without that, it just does not make sense
to have a fixed hardware timeout of 25 seconds and a fixed software timeout
of 240 seconds without means to change the latter.
> +
> + watchdog_set_drvdata(wdt, data);
> + platform_set_drvdata(pdev, data);
> +
platform_set_drvdata() seems unnecessary; I don't see where it is used.
> + ret = aaeon_mcu_wdt_start(wdt);
> + if (ret)
> + return ret;
> +
> + set_bit(WDOG_HW_RUNNING, &wdt->status);
> +
This is only acceptable if there is no means to detect if the watchdog is
running. If so, that warrants a comment in the code.
Thanks,
Guenter
> + 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