[v11,5/7] watchdog: Add Nuvoton NCT6694 WDT support

From: Ming Yu <tmyu0@nuvoton.com>

This driver supports Watchdog timer functionality for NCT6694 MFD
device based on USB interface.

Signed-off-by: Ming Yu <tmyu0@nuvoton.com>
---

Changes since version 10:
- Implement IDA to allocate id
- Add module parameters to configure WDT's timeout and pretimeout value 

Changes since version 9:

Changes since version 8:
- Modify the signed-off-by with my work address

Changes since version 7:
- Add error handling for devm_mutex_init()

Changes since version 6:
- Fix warning

Changes since version 5:
- Modify the module name and the driver name consistently

Changes since version 4:
- Modify arguments in read/write function to a pointer to cmd_header
- Modify all callers that call the read/write function

Changes since version 3:
- Modify array buffer to structure
- Fix defines and comments
- Modify mutex_init() to devm_mutex_init()
- Drop watchdog_init_timeout()

Changes since version 2:
- Add MODULE_ALIAS()
- Modify the pretimeout validation procedure

Changes since version 1:
- Add each driver's command structure
- Fix platform driver registration
- Fix warnings
- Drop unnecessary logs
- Modify start() function to setup device

 MAINTAINERS                    |   1 +
 drivers/watchdog/Kconfig       |  11 ++
 drivers/watchdog/Makefile      |   1 +
 drivers/watchdog/nct6694_wdt.c | 320 +++++++++++++++++++++++++++++++++
 4 files changed, 333 insertions(+)
 create mode 100644 drivers/watchdog/nct6694_wdt.c

On 5/19/25 19:03, a0282524688@gmail.com wrote:
> From: Ming Yu <tmyu0@nuvoton.com>
> 
> This driver supports Watchdog timer functionality for NCT6694 MFD
> device based on USB interface.
> 
> Signed-off-by: Ming Yu <tmyu0@nuvoton.com>
> ---
> 
> Changes since version 10:
> - Implement IDA to allocate id
> - Add module parameters to configure WDT's timeout and pretimeout value
> 
> Changes since version 9:
> 
> Changes since version 8:
> - Modify the signed-off-by with my work address
> 
> Changes since version 7:
> - Add error handling for devm_mutex_init()
> 
> Changes since version 6:
> - Fix warning
> 
> Changes since version 5:
> - Modify the module name and the driver name consistently
> 
> Changes since version 4:
> - Modify arguments in read/write function to a pointer to cmd_header
> - Modify all callers that call the read/write function
> 
> Changes since version 3:
> - Modify array buffer to structure
> - Fix defines and comments
> - Modify mutex_init() to devm_mutex_init()
> - Drop watchdog_init_timeout()
> 
> Changes since version 2:
> - Add MODULE_ALIAS()
> - Modify the pretimeout validation procedure
> 
> Changes since version 1:
> - Add each driver's command structure
> - Fix platform driver registration
> - Fix warnings
> - Drop unnecessary logs
> - Modify start() function to setup device
> 
>   MAINTAINERS                    |   1 +
>   drivers/watchdog/Kconfig       |  11 ++
>   drivers/watchdog/Makefile      |   1 +
>   drivers/watchdog/nct6694_wdt.c | 320 +++++++++++++++++++++++++++++++++
>   4 files changed, 333 insertions(+)
>   create mode 100644 drivers/watchdog/nct6694_wdt.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 57b95c21a626..681e0cfb4a4b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -17458,6 +17458,7 @@ F:	drivers/gpio/gpio-nct6694.c
>   F:	drivers/i2c/busses/i2c-nct6694.c
>   F:	drivers/mfd/nct6694.c
>   F:	drivers/net/can/usb/nct6694_canfd.c
> +F:	drivers/watchdog/nct6694_wdt.c
>   F:	include/linux/mfd/nct6694.h
>   
>   NVIDIA (rivafb and nvidiafb) FRAMEBUFFER DRIVER
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 0d8d37f712e8..6d84a509501e 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -760,6 +760,17 @@ config MAX77620_WATCHDOG
>   	  MAX77620 chips. To compile this driver as a module,
>   	  choose M here: the module will be called max77620_wdt.
>   
> +config NCT6694_WATCHDOG
> +	tristate "Nuvoton NCT6694 watchdog support"
> +	depends on MFD_NCT6694
> +	select WATCHDOG_CORE
> +	help
> +	  Say Y here to support Nuvoton NCT6694 watchdog timer
> +	  functionality.
> +
> +	  This driver can also be built as a module. If so, the module
> +	  will be called nct6694_wdt.
> +
>   config IMX2_WDT
>   	tristate "IMX2+ Watchdog"
>   	depends on ARCH_MXC || ARCH_LAYERSCAPE || COMPILE_TEST
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index c9482904bf87..7fe51bb06060 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -233,6 +233,7 @@ obj-$(CONFIG_WM831X_WATCHDOG) += wm831x_wdt.o
>   obj-$(CONFIG_WM8350_WATCHDOG) += wm8350_wdt.o
>   obj-$(CONFIG_MAX63XX_WATCHDOG) += max63xx_wdt.o
>   obj-$(CONFIG_MAX77620_WATCHDOG) += max77620_wdt.o
> +obj-$(CONFIG_NCT6694_WATCHDOG) += nct6694_wdt.o
>   obj-$(CONFIG_ZIIRAVE_WATCHDOG) += ziirave_wdt.o
>   obj-$(CONFIG_SOFT_WATCHDOG) += softdog.o
>   obj-$(CONFIG_MENF21BMC_WATCHDOG) += menf21bmc_wdt.o
> diff --git a/drivers/watchdog/nct6694_wdt.c b/drivers/watchdog/nct6694_wdt.c
> new file mode 100644
> index 000000000000..195bcbc0f156
> --- /dev/null
> +++ b/drivers/watchdog/nct6694_wdt.c
> @@ -0,0 +1,320 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Nuvoton NCT6694 WDT driver based on USB interface.
> + *
> + * Copyright (C) 2025 Nuvoton Technology Corp.
> + */
> +
> +#include <linux/idr.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/nct6694.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/watchdog.h>
> +
> +#define DEVICE_NAME "nct6694-wdt"
> +
> +#define NCT6694_DEFAULT_TIMEOUT		10
> +#define NCT6694_DEFAULT_PRETIMEOUT	0
> +
> +#define NCT6694_WDT_MAX_DEVS		2
> +
> +/*
> + * USB command module type for NCT6694 WDT controller.
> + * This defines the module type used for communication with the NCT6694
> + * WDT controller over the USB interface.
> + */
> +#define NCT6694_WDT_MOD			0x07
> +
> +/* Command 00h - WDT Setup */
> +#define NCT6694_WDT_SETUP		0x00
> +#define NCT6694_WDT_SETUP_SEL(idx)	(idx ? 0x01 : 0x00)
> +
> +/* Command 01h - WDT Command */
> +#define NCT6694_WDT_COMMAND		0x01
> +#define NCT6694_WDT_COMMAND_SEL(idx)	(idx ? 0x01 : 0x00)
> +
> +static unsigned int timeout[NCT6694_WDT_MAX_DEVS] = {
> +	[0 ... (NCT6694_WDT_MAX_DEVS - 1)] = NCT6694_DEFAULT_TIMEOUT
> +};
> +module_param_array(timeout, int, NULL, 0644);
> +MODULE_PARM_DESC(timeout, "Watchdog timeout in seconds");
> +
> +static unsigned int pretimeout[NCT6694_WDT_MAX_DEVS] = {
> +	[0 ... (NCT6694_WDT_MAX_DEVS - 1)] = NCT6694_DEFAULT_PRETIMEOUT
> +};
> +module_param_array(pretimeout, int, NULL, 0644);
> +MODULE_PARM_DESC(pretimeout, "Watchdog pre-timeout in seconds");
> +

How is this supposed to work if there are multiple NCT6694 in the system ?

> +static bool nowayout = WATCHDOG_NOWAYOUT;
> +module_param(nowayout, bool, 0);
> +MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
> +			   __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> +
> +enum {
> +	NCT6694_ACTION_NONE = 0,
> +	NCT6694_ACTION_SIRQ,
> +	NCT6694_ACTION_GPO,
> +};
> +
> +struct __packed nct6694_wdt_setup {
> +	__le32 pretimeout;
> +	__le32 timeout;
> +	u8 owner;
> +	u8 scratch;
> +	u8 control;
> +	u8 status;
> +	__le32 countdown;
> +};
> +
> +struct __packed nct6694_wdt_cmd {
> +	__le32 wdt_cmd;
> +	__le32 reserved;
> +};
> +
> +union __packed nct6694_wdt_msg {
> +	struct nct6694_wdt_setup setup;
> +	struct nct6694_wdt_cmd cmd;
> +};
> +
> +struct nct6694_wdt_data {
> +	struct watchdog_device wdev;
> +	struct device *dev;
> +	struct nct6694 *nct6694;
> +	struct mutex lock;
> +	union nct6694_wdt_msg *msg;
> +	unsigned char wdev_idx;
> +};
> +
> +static DEFINE_IDA(nct6694_wdt_ida);
> +
> +static int nct6694_wdt_setting(struct watchdog_device *wdev,
> +			       u32 timeout_val, u8 timeout_act,
> +			       u32 pretimeout_val, u8 pretimeout_act)
> +{
> +	struct nct6694_wdt_data *data = watchdog_get_drvdata(wdev);
> +	struct nct6694_wdt_setup *setup = &data->msg->setup;
> +	const struct nct6694_cmd_header cmd_hd = {
> +		.mod = NCT6694_WDT_MOD,
> +		.cmd = NCT6694_WDT_SETUP,
> +		.sel = NCT6694_WDT_SETUP_SEL(data->wdev_idx),
> +		.len = cpu_to_le16(sizeof(*setup))
> +	};
> +	unsigned int timeout_fmt, pretimeout_fmt;
> +
> +	guard(mutex)(&data->lock);
> +

Watchdog drivers are already mutex protected in the watchdog core.

> +	if (pretimeout_val == 0)
> +		pretimeout_act = NCT6694_ACTION_NONE;
> +
> +	timeout_fmt = (timeout_val * 1000) | (timeout_act << 24);
> +	pretimeout_fmt = (pretimeout_val * 1000) | (pretimeout_act << 24);
> +
> +	memset(setup, 0, sizeof(*setup));
> +	setup->timeout = cpu_to_le32(timeout_fmt);
> +	setup->pretimeout = cpu_to_le32(pretimeout_fmt);
> +
> +	return nct6694_write_msg(data->nct6694, &cmd_hd, setup);
> +}
> +
> +static int nct6694_wdt_start(struct watchdog_device *wdev)
> +{
> +	struct nct6694_wdt_data *data = watchdog_get_drvdata(wdev);
> +	int ret;
> +
> +	ret = nct6694_wdt_setting(wdev, wdev->timeout, NCT6694_ACTION_GPO,
> +				  wdev->pretimeout, NCT6694_ACTION_GPO);
> +	if (ret)
> +		return ret;
> +
> +	dev_dbg(data->dev, "Setting WDT(%d): timeout = %d, pretimeout = %d\n",
> +		data->wdev_idx, wdev->timeout, wdev->pretimeout);
> +
> +	return ret;
> +}
> +
> +static int nct6694_wdt_stop(struct watchdog_device *wdev)
> +{
> +	struct nct6694_wdt_data *data = watchdog_get_drvdata(wdev);
> +	struct nct6694_wdt_cmd *cmd = &data->msg->cmd;
> +	const struct nct6694_cmd_header cmd_hd = {
> +		.mod = NCT6694_WDT_MOD,
> +		.cmd = NCT6694_WDT_COMMAND,
> +		.sel = NCT6694_WDT_COMMAND_SEL(data->wdev_idx),
> +		.len = cpu_to_le16(sizeof(*cmd))
> +	};
> +
> +	guard(mutex)(&data->lock);
> +
> +	memcpy(&cmd->wdt_cmd, "WDTC", 4);
> +	cmd->reserved = 0;
> +
> +	return nct6694_write_msg(data->nct6694, &cmd_hd, cmd);
> +}
> +
> +static int nct6694_wdt_ping(struct watchdog_device *wdev)
> +{
> +	struct nct6694_wdt_data *data = watchdog_get_drvdata(wdev);
> +	struct nct6694_wdt_cmd *cmd = &data->msg->cmd;
> +	const struct nct6694_cmd_header cmd_hd = {
> +		.mod = NCT6694_WDT_MOD,
> +		.cmd = NCT6694_WDT_COMMAND,
> +		.sel = NCT6694_WDT_COMMAND_SEL(data->wdev_idx),
> +		.len = cpu_to_le16(sizeof(*cmd))
> +	};
> +
> +	guard(mutex)(&data->lock);
> +	memcpy(&cmd->wdt_cmd, "WDTS", 4);
> +	cmd->reserved = 0;
> +
> +	return nct6694_write_msg(data->nct6694, &cmd_hd, cmd);
> +}
> +
> +static int nct6694_wdt_set_timeout(struct watchdog_device *wdev,
> +				   unsigned int new_timeout)
> +{
> +	int ret;
> +
> +	ret = nct6694_wdt_setting(wdev, new_timeout, NCT6694_ACTION_GPO,
> +				  wdev->pretimeout, NCT6694_ACTION_GPO);
> +	if (ret)
> +		return ret;
> +
> +	wdev->timeout = new_timeout;
> +
> +	return 0;
> +}
> +
> +static int nct6694_wdt_set_pretimeout(struct watchdog_device *wdev,
> +				      unsigned int new_pretimeout)
> +{
> +	int ret;
> +
> +	ret = nct6694_wdt_setting(wdev, wdev->timeout, NCT6694_ACTION_GPO,
> +				  new_pretimeout, NCT6694_ACTION_GPO);
> +	if (ret)
> +		return ret;
> +
> +	wdev->pretimeout = new_pretimeout;
> +
> +	return 0;
> +}
> +
> +static unsigned int nct6694_wdt_get_time(struct watchdog_device *wdev)
> +{
> +	struct nct6694_wdt_data *data = watchdog_get_drvdata(wdev);
> +	struct nct6694_wdt_setup *setup = &data->msg->setup;
> +	const struct nct6694_cmd_header cmd_hd = {
> +		.mod = NCT6694_WDT_MOD,
> +		.cmd = NCT6694_WDT_SETUP,
> +		.sel = NCT6694_WDT_SETUP_SEL(data->wdev_idx),
> +		.len = cpu_to_le16(sizeof(*setup))
> +	};
> +	unsigned int timeleft_ms;
> +	int ret;
> +
> +	guard(mutex)(&data->lock);
> +
> +	ret = nct6694_read_msg(data->nct6694, &cmd_hd, setup);
> +	if (ret)
> +		return 0;
> +
> +	timeleft_ms = le32_to_cpu(setup->countdown);
> +
> +	return timeleft_ms / 1000;
> +}
> +
> +static const struct watchdog_info nct6694_wdt_info = {
> +	.options = WDIOF_SETTIMEOUT	|
> +		   WDIOF_KEEPALIVEPING	|
> +		   WDIOF_MAGICCLOSE	|
> +		   WDIOF_PRETIMEOUT,
> +	.identity = DEVICE_NAME,
> +};
> +
> +static const struct watchdog_ops nct6694_wdt_ops = {
> +	.owner = THIS_MODULE,
> +	.start = nct6694_wdt_start,
> +	.stop = nct6694_wdt_stop,
> +	.set_timeout = nct6694_wdt_set_timeout,
> +	.set_pretimeout = nct6694_wdt_set_pretimeout,
> +	.get_timeleft = nct6694_wdt_get_time,
> +	.ping = nct6694_wdt_ping,
> +};
> +
> +static void nct6694_wdt_ida_remove(void *d)
> +{
> +	struct nct6694_wdt_data *data = d;
> +
> +	ida_free(&nct6694_wdt_ida, data->wdev_idx);
> +}
> +
> +static int nct6694_wdt_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct nct6694 *nct6694 = dev_get_drvdata(pdev->dev.parent);
> +	struct nct6694_wdt_data *data;
> +	struct watchdog_device *wdev;
> +	int ret, wdev_idx;
> +
> +	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	data->msg = devm_kzalloc(dev, sizeof(union nct6694_wdt_msg),
> +				 GFP_KERNEL);
> +	if (!data->msg)
> +		return -ENOMEM;
> +
> +	wdev_idx = ida_alloc(&nct6694_wdt_ida, GFP_KERNEL);
> +	if (wdev_idx < 0)
> +		return wdev_idx;
> +

Sorry, I fail to understand why this is needed or even makes sense.
That ID is global, so if there is more than one chip they all get more or
less random IDs assigned. Why would that be valuable or necessary ?
If it is just necessary to give the watchdog different IDs, and the
values don't matter, why not just use pdev->id ?
I guess that won't work either because it is used as array index below.
You'll have to find a different means to identify which of the two watchdogs
is handled by this instance of the driver.

This warrants a detailed explanation why there need to be globally unique IDs
across multiple chips.

> +	ret = devm_add_action_or_reset(dev, nct6694_wdt_ida_remove, data);
> +	if (ret)
> +		return ret;
> +
> +	data->dev = dev;
> +	data->nct6694 = nct6694;
> +	data->wdev_idx = wdev_idx;
> +
> +	wdev = &data->wdev;
> +	wdev->info = &nct6694_wdt_info;
> +	wdev->ops = &nct6694_wdt_ops;
> +	wdev->timeout = timeout[wdev_idx];
> +	wdev->pretimeout = pretimeout[wdev_idx];
> +	if (timeout[wdev_idx] < pretimeout[wdev_idx]) {

Maybe I am missing it, but I don't see where wdev_idx is bound to [0,1].
It is global. There could be a dozen of those chips connected through
USB.

> +		dev_warn(data->dev, "pretimeout < timeout. Setting to zero\n");
> +		wdev->pretimeout = 0;
> +	}
> +
> +	wdev->min_timeout = 1;
> +	wdev->max_timeout = 255;
> +
> +	ret = devm_mutex_init(dev, &data->lock);
> +	if (ret)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, data);
> +
> +	watchdog_set_drvdata(&data->wdev, data);
> +	watchdog_set_nowayout(&data->wdev, nowayout);
> +	watchdog_stop_on_reboot(&data->wdev);
> +
> +	return devm_watchdog_register_device(dev, &data->wdev);
> +}
> +
> +static struct platform_driver nct6694_wdt_driver = {
> +	.driver = {
> +		.name	= DEVICE_NAME,
> +	},
> +	.probe		= nct6694_wdt_probe,
> +};
> +
> +module_platform_driver(nct6694_wdt_driver);
> +
> +MODULE_DESCRIPTION("USB-WDT driver for NCT6694");
> +MODULE_AUTHOR("Ming Yu <tmyu0@nuvoton.com>");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:nct6694-wdt");