public inbox for linux-hwmon@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] mfd: nct6694: Refactor transport layer and add HIF (eSPI) support
@ 2026-04-08  5:30 a0282524688
  2026-04-08  5:30 ` [PATCH v2 1/2] mfd: nct6694: Switch to devm_mfd_add_devices() and drop IDA a0282524688
  2026-04-08  5:30 ` [PATCH v2 2/2] mfd: Add Host Interface (HIF) support for Nuvoton NCT6694 a0282524688
  0 siblings, 2 replies; 7+ messages in thread
From: a0282524688 @ 2026-04-08  5:30 UTC (permalink / raw)
  To: tmyu0, linusw, brgl, linux, andi.shyti, lee, mkl, mailhol,
	alexandre.belloni, wim
  Cc: linux-kernel, linux-gpio, linux-i2c, linux-can, netdev,
	linux-watchdog, linux-hwmon, linux-rtc, linux-usb, Ming Yu

From: Ming Yu <a0282524688@gmail.com>

The Nuvoton NCT6694 is a peripheral expander that provides GPIO, I2C,
CAN-FD, Watchdog, HWMON, PWM, and RTC sub-devices. Currently, the
driver only supports USB as the host transport interface.

This series refactors the NCT6694 MFD core to support multiple transport
backends and adds a new Host Interface (HIF) transport driver that
communicates over eSPI using Super-I/O shared memory.

Changes since version 1:
- Reworked the Super-I/O access helpers.

Ming Yu (2):
  mfd: nct6694: Switch to devm_mfd_add_devices() and drop IDA
  mfd: Add Host Interface (HIF) support for Nuvoton NCT6694

 MAINTAINERS                         |   1 +
 drivers/gpio/gpio-nct6694.c         |  26 +-
 drivers/hwmon/nct6694-hwmon.c       |  21 -
 drivers/i2c/busses/i2c-nct6694.c    |  26 +-
 drivers/mfd/Kconfig                 |  47 ++-
 drivers/mfd/Makefile                |   3 +-
 drivers/mfd/nct6694-hif.c           | 634 ++++++++++++++++++++++++++++
 drivers/mfd/nct6694.c               | 180 ++++----
 drivers/net/can/usb/nct6694_canfd.c |  18 +-
 drivers/rtc/rtc-nct6694.c           |   7 -
 drivers/watchdog/nct6694_wdt.c      |  27 +-
 include/linux/mfd/nct6694.h         |  57 ++-
 12 files changed, 814 insertions(+), 233 deletions(-)
 create mode 100644 drivers/mfd/nct6694-hif.c

-- 
2.34.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2 1/2] mfd: nct6694: Switch to devm_mfd_add_devices() and drop IDA
  2026-04-08  5:30 [PATCH v2 0/2] mfd: nct6694: Refactor transport layer and add HIF (eSPI) support a0282524688
@ 2026-04-08  5:30 ` a0282524688
  2026-04-08  5:48   ` sashiko-bot
  2026-04-08  7:25   ` Bartosz Golaszewski
  2026-04-08  5:30 ` [PATCH v2 2/2] mfd: Add Host Interface (HIF) support for Nuvoton NCT6694 a0282524688
  1 sibling, 2 replies; 7+ messages in thread
From: a0282524688 @ 2026-04-08  5:30 UTC (permalink / raw)
  To: tmyu0, linusw, brgl, linux, andi.shyti, lee, mkl, mailhol,
	alexandre.belloni, wim
  Cc: linux-kernel, linux-gpio, linux-i2c, linux-can, netdev,
	linux-watchdog, linux-hwmon, linux-rtc, linux-usb, Ming Yu

From: Ming Yu <a0282524688@gmail.com>

Currently, the nct6694 core driver uses mfd_add_hotplug_devices()
and an IDA to manage subdevice IDs.

Switch the core implementation to use the managed
devm_mfd_add_devices() API, which simplifies the error handling and
device lifecycle management. Concurrently, drop the custom IDA
implementation and transition to using pdev->id.

Signed-off-by: Ming Yu <a0282524688@gmail.com>
---
 drivers/gpio/gpio-nct6694.c         | 19 +------
 drivers/i2c/busses/i2c-nct6694.c    | 19 +------
 drivers/mfd/nct6694.c               | 83 ++++++++++++-----------------
 drivers/net/can/usb/nct6694_canfd.c | 12 +----
 drivers/watchdog/nct6694_wdt.c      | 20 +------
 include/linux/mfd/nct6694.h         |  8 +--
 6 files changed, 43 insertions(+), 118 deletions(-)

diff --git a/drivers/gpio/gpio-nct6694.c b/drivers/gpio/gpio-nct6694.c
index a8607f0d9915..3703a61209e6 100644
--- a/drivers/gpio/gpio-nct6694.c
+++ b/drivers/gpio/gpio-nct6694.c
@@ -7,7 +7,6 @@
 
 #include <linux/bits.h>
 #include <linux/gpio/driver.h>
-#include <linux/idr.h>
 #include <linux/interrupt.h>
 #include <linux/mfd/nct6694.h>
 #include <linux/module.h>
@@ -381,14 +380,6 @@ static void nct6694_irq_dispose_mapping(void *d)
 	irq_dispose_mapping(data->irq);
 }
 
-static void nct6694_gpio_ida_free(void *d)
-{
-	struct nct6694_gpio_data *data = d;
-	struct nct6694 *nct6694 = data->nct6694;
-
-	ida_free(&nct6694->gpio_ida, data->group);
-}
-
 static int nct6694_gpio_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -403,15 +394,7 @@ static int nct6694_gpio_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	data->nct6694 = nct6694;
-
-	ret = ida_alloc(&nct6694->gpio_ida, GFP_KERNEL);
-	if (ret < 0)
-		return ret;
-	data->group = ret;
-
-	ret = devm_add_action_or_reset(dev, nct6694_gpio_ida_free, data);
-	if (ret)
-		return ret;
+	data->group = pdev->id;
 
 	names = devm_kcalloc(dev, NCT6694_NR_GPIO, sizeof(char *),
 			     GFP_KERNEL);
diff --git a/drivers/i2c/busses/i2c-nct6694.c b/drivers/i2c/busses/i2c-nct6694.c
index 1413ab6f9462..7d8ad997f6d2 100644
--- a/drivers/i2c/busses/i2c-nct6694.c
+++ b/drivers/i2c/busses/i2c-nct6694.c
@@ -6,7 +6,6 @@
  */
 
 #include <linux/i2c.h>
-#include <linux/idr.h>
 #include <linux/kernel.h>
 #include <linux/mfd/nct6694.h>
 #include <linux/module.h>
@@ -134,14 +133,6 @@ static int nct6694_i2c_set_baudrate(struct nct6694_i2c_data *data)
 	return 0;
 }
 
-static void nct6694_i2c_ida_free(void *d)
-{
-	struct nct6694_i2c_data *data = d;
-	struct nct6694 *nct6694 = data->nct6694;
-
-	ida_free(&nct6694->i2c_ida, data->port);
-}
-
 static int nct6694_i2c_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -155,15 +146,7 @@ static int nct6694_i2c_probe(struct platform_device *pdev)
 
 	data->dev = dev;
 	data->nct6694 = nct6694;
-
-	ret = ida_alloc(&nct6694->i2c_ida, GFP_KERNEL);
-	if (ret < 0)
-		return ret;
-	data->port = ret;
-
-	ret = devm_add_action_or_reset(dev, nct6694_i2c_ida_free, data);
-	if (ret)
-		return ret;
+	data->port = pdev->id;
 
 	ret = nct6694_i2c_set_baudrate(data);
 	if (ret)
diff --git a/drivers/mfd/nct6694.c b/drivers/mfd/nct6694.c
index 308b2fda3055..8ce2c4985aab 100644
--- a/drivers/mfd/nct6694.c
+++ b/drivers/mfd/nct6694.c
@@ -11,7 +11,6 @@
 
 #include <linux/bits.h>
 #include <linux/interrupt.h>
-#include <linux/idr.h>
 #include <linux/irq.h>
 #include <linux/irqdomain.h>
 #include <linux/kernel.h>
@@ -23,35 +22,35 @@
 #include <linux/usb.h>
 
 static const struct mfd_cell nct6694_devs[] = {
-	MFD_CELL_NAME("nct6694-gpio"),
-	MFD_CELL_NAME("nct6694-gpio"),
-	MFD_CELL_NAME("nct6694-gpio"),
-	MFD_CELL_NAME("nct6694-gpio"),
-	MFD_CELL_NAME("nct6694-gpio"),
-	MFD_CELL_NAME("nct6694-gpio"),
-	MFD_CELL_NAME("nct6694-gpio"),
-	MFD_CELL_NAME("nct6694-gpio"),
-	MFD_CELL_NAME("nct6694-gpio"),
-	MFD_CELL_NAME("nct6694-gpio"),
-	MFD_CELL_NAME("nct6694-gpio"),
-	MFD_CELL_NAME("nct6694-gpio"),
-	MFD_CELL_NAME("nct6694-gpio"),
-	MFD_CELL_NAME("nct6694-gpio"),
-	MFD_CELL_NAME("nct6694-gpio"),
-	MFD_CELL_NAME("nct6694-gpio"),
-
-	MFD_CELL_NAME("nct6694-i2c"),
-	MFD_CELL_NAME("nct6694-i2c"),
-	MFD_CELL_NAME("nct6694-i2c"),
-	MFD_CELL_NAME("nct6694-i2c"),
-	MFD_CELL_NAME("nct6694-i2c"),
-	MFD_CELL_NAME("nct6694-i2c"),
-
-	MFD_CELL_NAME("nct6694-canfd"),
-	MFD_CELL_NAME("nct6694-canfd"),
-
-	MFD_CELL_NAME("nct6694-wdt"),
-	MFD_CELL_NAME("nct6694-wdt"),
+	MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 0),
+	MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 1),
+	MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 2),
+	MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 3),
+	MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 4),
+	MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 5),
+	MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 6),
+	MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 7),
+	MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 8),
+	MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 9),
+	MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 10),
+	MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 11),
+	MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 12),
+	MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 13),
+	MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 14),
+	MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 15),
+
+	MFD_CELL_BASIC("nct6694-i2c", NULL, NULL, 0, 0),
+	MFD_CELL_BASIC("nct6694-i2c", NULL, NULL, 0, 1),
+	MFD_CELL_BASIC("nct6694-i2c", NULL, NULL, 0, 2),
+	MFD_CELL_BASIC("nct6694-i2c", NULL, NULL, 0, 3),
+	MFD_CELL_BASIC("nct6694-i2c", NULL, NULL, 0, 4),
+	MFD_CELL_BASIC("nct6694-i2c", NULL, NULL, 0, 5),
+
+	MFD_CELL_BASIC("nct6694-canfd", NULL, NULL, 0, 0),
+	MFD_CELL_BASIC("nct6694-canfd", NULL, NULL, 0, 1),
+
+	MFD_CELL_BASIC("nct6694-wdt", NULL, NULL, 0, 0),
+	MFD_CELL_BASIC("nct6694-wdt", NULL, NULL, 0, 1),
 
 	MFD_CELL_NAME("nct6694-hwmon"),
 
@@ -307,23 +306,18 @@ static int nct6694_usb_probe(struct usb_interface *iface,
 	nct6694->dev = dev;
 	nct6694->udev = udev;
 
-	ida_init(&nct6694->gpio_ida);
-	ida_init(&nct6694->i2c_ida);
-	ida_init(&nct6694->canfd_ida);
-	ida_init(&nct6694->wdt_ida);
-
 	spin_lock_init(&nct6694->irq_lock);
 
 	ret = devm_mutex_init(dev, &nct6694->access_lock);
 	if (ret)
-		goto err_ida;
+		goto err_irq_domain;
 
 	interface = iface->cur_altsetting;
 
 	int_endpoint = &interface->endpoint[0].desc;
 	if (!usb_endpoint_is_int_in(int_endpoint)) {
 		ret = -ENODEV;
-		goto err_ida;
+		goto err_irq_domain;
 	}
 
 	usb_fill_int_urb(nct6694->int_in_urb, udev, usb_rcvintpipe(udev, NCT6694_INT_IN_EP),
@@ -332,11 +326,11 @@ static int nct6694_usb_probe(struct usb_interface *iface,
 
 	ret = usb_submit_urb(nct6694->int_in_urb, GFP_KERNEL);
 	if (ret)
-		goto err_ida;
+		goto err_irq_domain;
 
 	usb_set_intfdata(iface, nct6694);
 
-	ret = mfd_add_hotplug_devices(dev, nct6694_devs, ARRAY_SIZE(nct6694_devs));
+	ret = devm_mfd_add_devices(dev, 0, nct6694_devs, ARRAY_SIZE(nct6694_devs), NULL, 0, NULL);
 	if (ret)
 		goto err_mfd;
 
@@ -344,11 +338,7 @@ static int nct6694_usb_probe(struct usb_interface *iface,
 
 err_mfd:
 	usb_kill_urb(nct6694->int_in_urb);
-err_ida:
-	ida_destroy(&nct6694->wdt_ida);
-	ida_destroy(&nct6694->canfd_ida);
-	ida_destroy(&nct6694->i2c_ida);
-	ida_destroy(&nct6694->gpio_ida);
+err_irq_domain:
 	irq_domain_remove(nct6694->domain);
 err_urb:
 	usb_free_urb(nct6694->int_in_urb);
@@ -359,12 +349,7 @@ static void nct6694_usb_disconnect(struct usb_interface *iface)
 {
 	struct nct6694 *nct6694 = usb_get_intfdata(iface);
 
-	mfd_remove_devices(nct6694->dev);
 	usb_kill_urb(nct6694->int_in_urb);
-	ida_destroy(&nct6694->wdt_ida);
-	ida_destroy(&nct6694->canfd_ida);
-	ida_destroy(&nct6694->i2c_ida);
-	ida_destroy(&nct6694->gpio_ida);
 	irq_domain_remove(nct6694->domain);
 	usb_free_urb(nct6694->int_in_urb);
 }
diff --git a/drivers/net/can/usb/nct6694_canfd.c b/drivers/net/can/usb/nct6694_canfd.c
index e5f7f8849a73..29282c56430f 100644
--- a/drivers/net/can/usb/nct6694_canfd.c
+++ b/drivers/net/can/usb/nct6694_canfd.c
@@ -8,7 +8,6 @@
 #include <linux/can/dev.h>
 #include <linux/can/rx-offload.h>
 #include <linux/ethtool.h>
-#include <linux/idr.h>
 #include <linux/irqdomain.h>
 #include <linux/kernel.h>
 #include <linux/mfd/nct6694.h>
@@ -725,15 +724,13 @@ static int nct6694_canfd_probe(struct platform_device *pdev)
 	struct net_device *ndev;
 	int port, irq, ret, can_clk;
 
-	port = ida_alloc(&nct6694->canfd_ida, GFP_KERNEL);
-	if (port < 0)
-		return port;
+	port = pdev->id;
 
 	irq = irq_create_mapping(nct6694->domain,
 				 NCT6694_IRQ_CAN0 + port);
 	if (!irq) {
 		ret = -EINVAL;
-		goto free_ida;
+		return ret;
 	}
 
 	ndev = alloc_candev(sizeof(struct nct6694_canfd_priv), 1);
@@ -796,24 +793,19 @@ static int nct6694_canfd_probe(struct platform_device *pdev)
 	free_candev(ndev);
 dispose_irq:
 	irq_dispose_mapping(irq);
-free_ida:
-	ida_free(&nct6694->canfd_ida, port);
 	return ret;
 }
 
 static void nct6694_canfd_remove(struct platform_device *pdev)
 {
 	struct nct6694_canfd_priv *priv = platform_get_drvdata(pdev);
-	struct nct6694 *nct6694 = priv->nct6694;
 	struct net_device *ndev = priv->ndev;
-	int port = ndev->dev_port;
 	int irq = ndev->irq;
 
 	unregister_candev(ndev);
 	can_rx_offload_del(&priv->offload);
 	free_candev(ndev);
 	irq_dispose_mapping(irq);
-	ida_free(&nct6694->canfd_ida, port);
 }
 
 static struct platform_driver nct6694_canfd_driver = {
diff --git a/drivers/watchdog/nct6694_wdt.c b/drivers/watchdog/nct6694_wdt.c
index bc3689bd4b6b..2b4b804a1739 100644
--- a/drivers/watchdog/nct6694_wdt.c
+++ b/drivers/watchdog/nct6694_wdt.c
@@ -5,7 +5,6 @@
  * Copyright (C) 2025 Nuvoton Technology Corp.
  */
 
-#include <linux/idr.h>
 #include <linux/kernel.h>
 #include <linux/mfd/nct6694.h>
 #include <linux/module.h>
@@ -233,21 +232,12 @@ static const struct watchdog_ops nct6694_wdt_ops = {
 	.ping = nct6694_wdt_ping,
 };
 
-static void nct6694_wdt_ida_free(void *d)
-{
-	struct nct6694_wdt_data *data = d;
-	struct nct6694 *nct6694 = data->nct6694;
-
-	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(dev->parent);
 	struct nct6694_wdt_data *data;
 	struct watchdog_device *wdev;
-	int ret;
 
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
@@ -260,15 +250,7 @@ static int nct6694_wdt_probe(struct platform_device *pdev)
 
 	data->dev = dev;
 	data->nct6694 = nct6694;
-
-	ret = ida_alloc(&nct6694->wdt_ida, GFP_KERNEL);
-	if (ret < 0)
-		return ret;
-	data->wdev_idx = ret;
-
-	ret = devm_add_action_or_reset(dev, nct6694_wdt_ida_free, data);
-	if (ret)
-		return ret;
+	data->wdev_idx = pdev->id;
 
 	wdev = &data->wdev;
 	wdev->info = &nct6694_wdt_info;
diff --git a/include/linux/mfd/nct6694.h b/include/linux/mfd/nct6694.h
index 6eb9be2cd4a0..496da72949d9 100644
--- a/include/linux/mfd/nct6694.h
+++ b/include/linux/mfd/nct6694.h
@@ -8,6 +8,10 @@
 #ifndef __MFD_NCT6694_H
 #define __MFD_NCT6694_H
 
+#include <linux/mutex.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
 #define NCT6694_VENDOR_ID	0x0416
 #define NCT6694_PRODUCT_ID	0x200B
 #define NCT6694_INT_IN_EP	0x81
@@ -82,10 +86,6 @@ union __packed nct6694_usb_msg {
 
 struct nct6694 {
 	struct device *dev;
-	struct ida gpio_ida;
-	struct ida i2c_ida;
-	struct ida canfd_ida;
-	struct ida wdt_ida;
 	struct irq_domain *domain;
 	struct mutex access_lock;
 	spinlock_t irq_lock;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 2/2] mfd: Add Host Interface (HIF) support for Nuvoton NCT6694
  2026-04-08  5:30 [PATCH v2 0/2] mfd: nct6694: Refactor transport layer and add HIF (eSPI) support a0282524688
  2026-04-08  5:30 ` [PATCH v2 1/2] mfd: nct6694: Switch to devm_mfd_add_devices() and drop IDA a0282524688
@ 2026-04-08  5:30 ` a0282524688
  2026-04-08  6:30   ` sashiko-bot
  1 sibling, 1 reply; 7+ messages in thread
From: a0282524688 @ 2026-04-08  5:30 UTC (permalink / raw)
  To: tmyu0, linusw, brgl, linux, andi.shyti, lee, mkl, mailhol,
	alexandre.belloni, wim
  Cc: linux-kernel, linux-gpio, linux-i2c, linux-can, netdev,
	linux-watchdog, linux-hwmon, linux-rtc, linux-usb, Ming Yu

From: Ming Yu <a0282524688@gmail.com>

The Nuvoton NCT6694 also provides a Host Interface (HIF) via eSPI
to the host to access its features.

Sub-devices can use the common functions nct6694_read_msg() and
nct6694_write_msg() to issue a command. They can also request
interrupts that will be called when the HIF device triggers a
shared memory interrupt.

To support multiple transports, the driver configuration is
updated to allow selecting between the USB and HIF interfaces.

Signed-off-by: Ming Yu <a0282524688@gmail.com>
---
Changes since version 1:
- Drop function pointers from Super-I/O access and use static inline
  helpers with proper types.

 MAINTAINERS                         |   1 +
 drivers/gpio/gpio-nct6694.c         |   7 -
 drivers/hwmon/nct6694-hwmon.c       |  21 -
 drivers/i2c/busses/i2c-nct6694.c    |   7 -
 drivers/mfd/Kconfig                 |  47 ++-
 drivers/mfd/Makefile                |   3 +-
 drivers/mfd/nct6694-hif.c           | 634 ++++++++++++++++++++++++++++
 drivers/mfd/nct6694.c               |  97 +++--
 drivers/net/can/usb/nct6694_canfd.c |   6 -
 drivers/rtc/rtc-nct6694.c           |   7 -
 drivers/watchdog/nct6694_wdt.c      |   7 -
 include/linux/mfd/nct6694.h         |  51 ++-
 12 files changed, 772 insertions(+), 116 deletions(-)
 create mode 100644 drivers/mfd/nct6694-hif.c

diff --git a/MAINTAINERS b/MAINTAINERS
index c3fe46d7c4bc..7b6241faa6df 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18899,6 +18899,7 @@ S:	Supported
 F:	drivers/gpio/gpio-nct6694.c
 F:	drivers/hwmon/nct6694-hwmon.c
 F:	drivers/i2c/busses/i2c-nct6694.c
+F:	drivers/mfd/nct6694-hif.c
 F:	drivers/mfd/nct6694.c
 F:	drivers/net/can/usb/nct6694_canfd.c
 F:	drivers/rtc/rtc-nct6694.c
diff --git a/drivers/gpio/gpio-nct6694.c b/drivers/gpio/gpio-nct6694.c
index 3703a61209e6..a279510ece89 100644
--- a/drivers/gpio/gpio-nct6694.c
+++ b/drivers/gpio/gpio-nct6694.c
@@ -12,13 +12,6 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 
-/*
- * USB command module type for NCT6694 GPIO controller.
- * This defines the module type used for communication with the NCT6694
- * GPIO controller over the USB interface.
- */
-#define NCT6694_GPIO_MOD	0xFF
-
 #define NCT6694_GPIO_VER	0x90
 #define NCT6694_GPIO_VALID	0x110
 #define NCT6694_GPI_DATA	0x120
diff --git a/drivers/hwmon/nct6694-hwmon.c b/drivers/hwmon/nct6694-hwmon.c
index 6dcf22ca5018..581451875f2c 100644
--- a/drivers/hwmon/nct6694-hwmon.c
+++ b/drivers/hwmon/nct6694-hwmon.c
@@ -15,13 +15,6 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 
-/*
- * USB command module type for NCT6694 report channel
- * This defines the module type used for communication with the NCT6694
- * report channel over the USB interface.
- */
-#define NCT6694_RPT_MOD			0xFF
-
 /* Report channel */
 /*
  * The report channel is used to report the status of the hardware monitor
@@ -38,13 +31,6 @@
 #define NCT6694_TIN_STS(x)		(0x6A + (x))
 #define NCT6694_FIN_STS(x)		(0x6E + (x))
 
-/*
- * USB command module type for NCT6694 HWMON controller.
- * This defines the module type used for communication with the NCT6694
- * HWMON controller over the USB interface.
- */
-#define NCT6694_HWMON_MOD		0x00
-
 /* Command 00h - Hardware Monitor Control */
 #define NCT6694_HWMON_CONTROL		0x00
 #define NCT6694_HWMON_CONTROL_SEL	0x00
@@ -53,13 +39,6 @@
 #define NCT6694_HWMON_ALARM		0x02
 #define NCT6694_HWMON_ALARM_SEL		0x00
 
-/*
- * USB command module type for NCT6694 PWM controller.
- * This defines the module type used for communication with the NCT6694
- * PWM controller over the USB interface.
- */
-#define NCT6694_PWM_MOD			0x01
-
 /* PWM Command - Manual Control */
 #define NCT6694_PWM_CONTROL		0x01
 #define NCT6694_PWM_CONTROL_SEL		0x00
diff --git a/drivers/i2c/busses/i2c-nct6694.c b/drivers/i2c/busses/i2c-nct6694.c
index 7d8ad997f6d2..7ee209a04d16 100644
--- a/drivers/i2c/busses/i2c-nct6694.c
+++ b/drivers/i2c/busses/i2c-nct6694.c
@@ -11,13 +11,6 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 
-/*
- * USB command module type for NCT6694 I2C controller.
- * This defines the module type used for communication with the NCT6694
- * I2C controller over the USB interface.
- */
-#define NCT6694_I2C_MOD			0x03
-
 /* Command 00h - I2C Deliver */
 #define NCT6694_I2C_DELIVER		0x00
 #define NCT6694_I2C_DELIVER_SEL		0x00
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 7192c9d1d268..8a715ec2f79f 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1164,19 +1164,46 @@ config MFD_MENF21BMC
 	  will be called menf21bmc.
 
 config MFD_NCT6694
-	tristate "Nuvoton NCT6694 support"
+	tristate
 	select MFD_CORE
+	help
+	  Core MFD support for the Nuvoton NCT6694 peripheral expander.
+	  This provides the common APIs and shared structures used by all
+	  interfaces (USB, HIF) to access the NCT6694 hardware features
+	  such as GPIO, I2C, CAN-FD, Watchdog, ADC, PWM, and RTC.
+
+	  It is selected automatically by the transport interface drivers.
+
+config MFD_NCT6694_HIF
+	tristate "Nuvoton NCT6694 HIF (eSPI) interface support"
+	depends on HAS_IOPORT && ACPI
+	select MFD_NCT6694
+	select REGMAP_MMIO
+	help
+	  This enables support for the Nuvoton NCT6694 peripheral expander
+	  connected via the Host Interface (HIF) using eSPI transport.
+
+	  The transport driver uses Super-I/O mapping and shared memory to
+	  communicate with the NCT6694 firmware. Enable this option if you
+	  are using the NCT6694 over an eSPI interface on an ACPI platform.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called nct6694-hif.
+
+config MFD_NCT6694_USB
+	tristate "Nuvoton NCT6694 USB interface support"
+	select MFD_NCT6694
 	depends on USB
 	help
-	  This enables support for the Nuvoton USB device NCT6694, which shares
-	  peripherals.
-	  The Nuvoton NCT6694 is a peripheral expander with 16 GPIO chips,
-	  6 I2C controllers, 2 CANfd controllers, 2 Watchdog timers, ADC,
-	  PWM, and RTC.
-	  This driver provides core APIs to access the NCT6694 hardware
-	  monitoring and control features.
-	  Additional drivers must be enabled to utilize the specific
-	  functionalities of the device.
+	  This enables support for the Nuvoton NCT6694 peripheral expander
+	  connected via the USB interface.
+
+	  The transport driver uses USB bulk and interrupt transfers to
+	  communicate with the NCT6694 firmware. Enable this option if you
+	  are using the NCT6694 via a USB connection.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called nct6694.
 
 config MFD_OCELOT
 	tristate "Microsemi Ocelot External Control Support"
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index e75e8045c28a..4cee9b74978c 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -124,7 +124,8 @@ obj-$(CONFIG_MFD_MC13XXX_I2C)	+= mc13xxx-i2c.o
 
 obj-$(CONFIG_MFD_PF1550)	+= pf1550.o
 
-obj-$(CONFIG_MFD_NCT6694)	+= nct6694.o
+obj-$(CONFIG_MFD_NCT6694_HIF)	+= nct6694-hif.o
+obj-$(CONFIG_MFD_NCT6694_USB)	+= nct6694.o
 
 obj-$(CONFIG_MFD_CORE)		+= mfd-core.o
 
diff --git a/drivers/mfd/nct6694-hif.c b/drivers/mfd/nct6694-hif.c
new file mode 100644
index 000000000000..7560754e7481
--- /dev/null
+++ b/drivers/mfd/nct6694-hif.c
@@ -0,0 +1,634 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2026 Nuvoton Technology Corp.
+ *
+ * Nuvoton NCT6694 host-interface (eSPI) transport driver.
+ */
+
+#include <linux/acpi.h>
+#include <linux/bits.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/kernel.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/nct6694.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/unaligned.h>
+
+#define DRVNAME "nct6694-hif"
+
+#define NCT6694_POLL_INTERVAL_US	10
+#define NCT6694_POLL_TIMEOUT_US		10000
+
+/*
+ * Super-I/O registers
+ */
+#define SIO_REG_LDSEL		0x07	/* Logical device select */
+#define SIO_REG_DEVID		0x20	/* Device ID (2 bytes) */
+#define SIO_REG_LD_SHM		0x0F	/* Logical device shared memory control */
+
+#define SIO_REG_SHM_ENABLE	0x30	/* Enable shared memory */
+#define SIO_REG_SHM_BASE_ADDR	0x60	/* Shared memory base address (2 bytes) */
+#define SIO_REG_SHM_IRQ_NR	0x70	/* Shared memory interrupt number */
+
+#define SIO_REG_UNLOCK_KEY	0x87	/* Key to enable Super-I/O */
+#define SIO_REG_LOCK_KEY	0xAA	/* Key to disable Super-I/O */
+
+#define SIO_NCT6694B_ID		0xD029
+#define SIO_NCT6694D_ID		0x5832
+
+/*
+ * Super-I/O Shared Memory Logical Device registers
+ */
+#define NCT6694_SHM_COFS_STS			0x2E
+#define NCT6694_SHM_COFS_STS_COFS4W		BIT(7)
+
+#define NCT6694_SHM_COFS_CTL2			0x3B
+#define NCT6694_SHM_COFS_CTL2_COFS4W_IE		BIT(3)
+
+#define NCT6694_SHM_INTR_STATUS			0x9C	/* Interrupt status register (4 bytes) */
+
+enum nct6694_chips {
+	NCT6694B = 0,
+	NCT6694D,
+};
+
+enum nct6694_module_id {
+	NCT6694_GPIO0 = 0,
+	NCT6694_GPIO1,
+	NCT6694_GPIO2,
+	NCT6694_GPIO3,
+	NCT6694_GPIO4,
+	NCT6694_GPIO5,
+	NCT6694_GPIO6,
+	NCT6694_GPIO7,
+	NCT6694_GPIO8,
+	NCT6694_GPIO9,
+	NCT6694_GPIOA,
+	NCT6694_GPIOB,
+	NCT6694_GPIOC,
+	NCT6694_GPIOD,
+	NCT6694_GPIOE,
+	NCT6694_GPIOF,
+	NCT6694_I2C0,
+	NCT6694_I2C1,
+	NCT6694_I2C2,
+	NCT6694_I2C3,
+	NCT6694_I2C4,
+	NCT6694_I2C5,
+	NCT6694_CAN0,
+	NCT6694_CAN1,
+};
+
+struct __packed nct6694_msg {
+	struct nct6694_cmd_header cmd_header;
+	struct nct6694_response_header response_header;
+	unsigned char *data;
+};
+
+struct nct6694_sio_data {
+	enum nct6694_chips chip;
+	int sioreg;	/* Super-I/O index port */
+};
+
+struct nct6694_hif_data {
+	struct regmap *regmap;
+	struct mutex msg_lock;
+	struct nct6694_sio_data *sio_data;
+	void __iomem *msg_base;
+	unsigned int shm_base;
+};
+
+static const char * const nct6694_chip_names[] = {
+	"NCT6694D",
+	"NCT6694B"
+};
+
+/*
+ * Super-I/O functions.
+ */
+static inline int superio_enter(struct nct6694_sio_data *sio_data)
+{
+	int ioreg = sio_data->sioreg;
+
+	/*
+	 * Try to reserve <ioreg> and <ioreg + 1> for exclusive access.
+	 */
+	if (!request_muxed_region(ioreg, 2, DRVNAME))
+		return -EBUSY;
+
+	outb(SIO_REG_UNLOCK_KEY, ioreg);
+	outb(SIO_REG_UNLOCK_KEY, ioreg);
+
+	return 0;
+}
+
+static inline void superio_exit(struct nct6694_sio_data *sio_data)
+{
+	int ioreg = sio_data->sioreg;
+
+	outb(SIO_REG_LOCK_KEY, ioreg);
+
+	release_region(ioreg, 2);
+}
+
+static inline void superio_select(struct nct6694_sio_data *sio_data, int ld)
+{
+	int ioreg = sio_data->sioreg;
+
+	outb(SIO_REG_LDSEL, ioreg);
+	outb(ld, ioreg + 1);
+}
+
+static inline int superio_inb(struct nct6694_sio_data *sio_data, int reg)
+{
+	int ioreg = sio_data->sioreg;
+
+	outb(reg, ioreg);
+	return inb(ioreg + 1);
+}
+
+static inline int superio_inw(struct nct6694_sio_data *sio_data, int reg)
+{
+	int ioreg = sio_data->sioreg;
+	int val;
+
+	outb(reg++, ioreg);
+	val = inb(ioreg + 1) << 8;
+	outb(reg, ioreg);
+	val |= inb(ioreg + 1);
+
+	return val;
+}
+
+static inline void superio_outb(struct nct6694_sio_data *sio_data, int reg, u8 val)
+{
+	int ioreg = sio_data->sioreg;
+
+	outb(reg, ioreg);
+	outb(val, ioreg + 1);
+}
+
+static int nct6694_sio_find(struct nct6694_sio_data *sio_data, u8 sioreg)
+{
+	int ret;
+	u16 devid;
+
+	sio_data->sioreg = sioreg;
+
+	ret = superio_enter(sio_data);
+	if (ret)
+		return ret;
+
+	/* Check Chip ID */
+	devid = superio_inw(sio_data, SIO_REG_DEVID);
+	switch (devid) {
+	case SIO_NCT6694B_ID:
+		sio_data->chip = NCT6694B;
+		break;
+	case SIO_NCT6694D_ID:
+		sio_data->chip = NCT6694D;
+		break;
+	default:
+		pr_err("Unsupported device 0x%04x\n", devid);
+		goto err;
+	}
+
+	pr_info("Found %s at %#x\n", nct6694_chip_names[sio_data->chip], sio_data->sioreg);
+
+	superio_exit(sio_data);
+
+	return 0;
+
+err:
+	superio_exit(sio_data);
+	return -ENODEV;
+}
+
+static const struct mfd_cell_acpi_match nct6694_acpi_match_gpio[] = {
+	{ .adr = NCT6694_GPIO0 },
+	{ .adr = NCT6694_GPIO1 },
+	{ .adr = NCT6694_GPIO2 },
+	{ .adr = NCT6694_GPIO3 },
+	{ .adr = NCT6694_GPIO4 },
+	{ .adr = NCT6694_GPIO5 },
+	{ .adr = NCT6694_GPIO6 },
+	{ .adr = NCT6694_GPIO7 },
+	{ .adr = NCT6694_GPIO8 },
+	{ .adr = NCT6694_GPIO9 },
+	{ .adr = NCT6694_GPIOA },
+	{ .adr = NCT6694_GPIOB },
+	{ .adr = NCT6694_GPIOC },
+	{ .adr = NCT6694_GPIOD },
+	{ .adr = NCT6694_GPIOE },
+	{ .adr = NCT6694_GPIOF },
+};
+
+static const struct mfd_cell_acpi_match nct6694_acpi_match_i2c[] = {
+	{ .adr = NCT6694_I2C0 },
+	{ .adr = NCT6694_I2C1 },
+	{ .adr = NCT6694_I2C2 },
+	{ .adr = NCT6694_I2C3 },
+	{ .adr = NCT6694_I2C4 },
+	{ .adr = NCT6694_I2C5 },
+};
+
+static const struct mfd_cell_acpi_match nct6694_acpi_match_can[] = {
+	{ .adr = NCT6694_CAN0 },
+	{ .adr = NCT6694_CAN1 },
+};
+
+static const struct mfd_cell nct6694_devs[] = {
+	MFD_CELL_ACPI("nct6694-gpio", NULL, NULL, 0, 0, &nct6694_acpi_match_gpio[0]),
+	MFD_CELL_ACPI("nct6694-gpio", NULL, NULL, 0, 1, &nct6694_acpi_match_gpio[1]),
+	MFD_CELL_ACPI("nct6694-gpio", NULL, NULL, 0, 2, &nct6694_acpi_match_gpio[2]),
+	MFD_CELL_ACPI("nct6694-gpio", NULL, NULL, 0, 3, &nct6694_acpi_match_gpio[3]),
+	MFD_CELL_ACPI("nct6694-gpio", NULL, NULL, 0, 4, &nct6694_acpi_match_gpio[4]),
+	MFD_CELL_ACPI("nct6694-gpio", NULL, NULL, 0, 5, &nct6694_acpi_match_gpio[5]),
+	MFD_CELL_ACPI("nct6694-gpio", NULL, NULL, 0, 6, &nct6694_acpi_match_gpio[6]),
+	MFD_CELL_ACPI("nct6694-gpio", NULL, NULL, 0, 7, &nct6694_acpi_match_gpio[7]),
+	MFD_CELL_ACPI("nct6694-gpio", NULL, NULL, 0, 8, &nct6694_acpi_match_gpio[8]),
+	MFD_CELL_ACPI("nct6694-gpio", NULL, NULL, 0, 9, &nct6694_acpi_match_gpio[9]),
+	MFD_CELL_ACPI("nct6694-gpio", NULL, NULL, 0, 10, &nct6694_acpi_match_gpio[10]),
+	MFD_CELL_ACPI("nct6694-gpio", NULL, NULL, 0, 11, &nct6694_acpi_match_gpio[11]),
+	MFD_CELL_ACPI("nct6694-gpio", NULL, NULL, 0, 12, &nct6694_acpi_match_gpio[12]),
+	MFD_CELL_ACPI("nct6694-gpio", NULL, NULL, 0, 13, &nct6694_acpi_match_gpio[13]),
+	MFD_CELL_ACPI("nct6694-gpio", NULL, NULL, 0, 14, &nct6694_acpi_match_gpio[14]),
+	MFD_CELL_ACPI("nct6694-gpio", NULL, NULL, 0, 15, &nct6694_acpi_match_gpio[15]),
+
+	MFD_CELL_ACPI("nct6694-i2c", NULL, NULL, 0, 0, &nct6694_acpi_match_i2c[0]),
+	MFD_CELL_ACPI("nct6694-i2c", NULL, NULL, 0, 1, &nct6694_acpi_match_i2c[1]),
+	MFD_CELL_ACPI("nct6694-i2c", NULL, NULL, 0, 2, &nct6694_acpi_match_i2c[2]),
+	MFD_CELL_ACPI("nct6694-i2c", NULL, NULL, 0, 3, &nct6694_acpi_match_i2c[3]),
+	MFD_CELL_ACPI("nct6694-i2c", NULL, NULL, 0, 4, &nct6694_acpi_match_i2c[4]),
+	MFD_CELL_ACPI("nct6694-i2c", NULL, NULL, 0, 5, &nct6694_acpi_match_i2c[5]),
+
+	MFD_CELL_ACPI("nct6694-canfd", NULL, NULL, 0, 0, &nct6694_acpi_match_can[0]),
+	MFD_CELL_ACPI("nct6694-canfd", NULL, NULL, 0, 1, &nct6694_acpi_match_can[1]),
+};
+
+static int nct6694_response_err_handling(struct nct6694 *nct6694, unsigned char err_status)
+{
+	switch (err_status) {
+	case NCT6694_NO_ERROR:
+		return 0;
+	case NCT6694_NOT_SUPPORT_ERROR:
+		dev_err(nct6694->dev, "Command is not supported!\n");
+		break;
+	case NCT6694_NO_RESPONSE_ERROR:
+		dev_warn(nct6694->dev, "Command received no response!\n");
+		break;
+	case NCT6694_TIMEOUT_ERROR:
+		dev_warn(nct6694->dev, "Command timed out!\n");
+		break;
+	case NCT6694_PENDING:
+		dev_err(nct6694->dev, "Command is pending!\n");
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return -EIO;
+}
+
+static int nct6694_xfer_msg(struct nct6694 *nct6694,
+			    const struct nct6694_cmd_header *cmd_hd,
+			    u8 hctrl, void *buf)
+{
+	struct nct6694_hif_data *hdata = nct6694->priv;
+	void __iomem *hdr = hdata->msg_base + offsetof(struct nct6694_msg, cmd_header);
+	struct nct6694_cmd_header cmd = *cmd_hd;
+	struct nct6694_response_header resp;
+	u16 len = le16_to_cpu(cmd.len);
+	u8 status;
+	int ret;
+
+	guard(mutex)(&hdata->msg_lock);
+
+	/* Wait until the previous command is completed */
+	ret = readb_poll_timeout(hdr + offsetof(struct nct6694_cmd_header, hctrl),
+				 status, status == 0, NCT6694_POLL_INTERVAL_US,
+				 NCT6694_POLL_TIMEOUT_US);
+	if (ret)
+		return ret;
+
+	/*
+	 * Write cmd header fields, but skip hctrl — writing to it triggers
+	 * firmware command processing and must be deferred until data is ready.
+	 */
+	memcpy_toio(hdr, &cmd, offsetof(struct nct6694_cmd_header, hctrl));
+	memcpy_toio(hdr + offsetof(struct nct6694_cmd_header, rsv2), &cmd.rsv2,
+		    sizeof(cmd) - offsetof(struct nct6694_cmd_header, rsv2));
+
+	if (hctrl == NCT6694_HCTRL_SET && len)
+		memcpy_toio(hdata->msg_base + offsetof(struct nct6694_msg, data),
+			    buf, len);
+
+	/* Write hctrl last to trigger command processing */
+	writeb(hctrl, hdr + offsetof(struct nct6694_cmd_header, hctrl));
+
+	ret = readb_poll_timeout(hdr + offsetof(struct nct6694_cmd_header, hctrl),
+				 status, status == 0, NCT6694_POLL_INTERVAL_US,
+				 NCT6694_POLL_TIMEOUT_US);
+	if (ret)
+		return ret;
+
+	memcpy_fromio(&resp, hdata->msg_base + offsetof(struct nct6694_msg, response_header),
+		      sizeof(resp));
+
+	ret = nct6694_response_err_handling(nct6694, resp.sts);
+	if (ret)
+		return ret;
+
+	if (le16_to_cpu(resp.len))
+		memcpy_fromio(buf, hdata->msg_base + offsetof(struct nct6694_msg, data),
+			      min(len, le16_to_cpu(resp.len)));
+
+	return 0;
+}
+
+/**
+ * nct6694_hif_read_msg() - Send a command and read response data via HIF
+ * @nct6694: NCT6694 device data
+ * @cmd_hd: command header
+ * @buf: buffer to store response data
+ *
+ * Return: 0 on success or negative errno on failure.
+ */
+static int nct6694_hif_read_msg(struct nct6694 *nct6694,
+				const struct nct6694_cmd_header *cmd_hd,
+				void *buf)
+{
+	struct nct6694_hif_data *hdata = nct6694->priv;
+
+	if (cmd_hd->mod == NCT6694_RPT_MOD)
+		return regmap_bulk_read(hdata->regmap,
+					le16_to_cpu(cmd_hd->offset),
+					buf, le16_to_cpu(cmd_hd->len));
+	return nct6694_xfer_msg(nct6694, cmd_hd, NCT6694_HCTRL_GET, buf);
+}
+
+/**
+ * nct6694_hif_write_msg() - Send a command with data payload via HIF
+ * @nct6694: NCT6694 device data
+ * @cmd_hd: command header
+ * @buf: buffer containing data to send
+ *
+ * Return: 0 on success or negative errno on failure.
+ */
+static int nct6694_hif_write_msg(struct nct6694 *nct6694,
+				 const struct nct6694_cmd_header *cmd_hd,
+				 void *buf)
+{
+	struct nct6694_hif_data *hdata = nct6694->priv;
+
+	if (cmd_hd->mod == NCT6694_RPT_MOD)
+		return regmap_bulk_write(hdata->regmap,
+					 le16_to_cpu(cmd_hd->offset),
+					 buf, le16_to_cpu(cmd_hd->len));
+	return nct6694_xfer_msg(nct6694, cmd_hd, NCT6694_HCTRL_SET, buf);
+}
+
+static const struct regmap_config nct6694_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.reg_stride = 1,
+};
+
+static irqreturn_t nct6694_irq_handler(int irq, void *data)
+{
+	struct nct6694 *nct6694 = data;
+	struct nct6694_hif_data *hdata = nct6694->priv;
+	u8 reg_data[4];
+	u32 intr_status;
+	int ret;
+
+	/* Check interrupt status is set */
+	if (!(inb(hdata->shm_base + NCT6694_SHM_COFS_STS) & NCT6694_SHM_COFS_STS_COFS4W))
+		return IRQ_NONE;
+
+	/* Clear interrupt status */
+	outb(NCT6694_SHM_COFS_STS_COFS4W, hdata->shm_base + NCT6694_SHM_COFS_STS);
+
+	ret = regmap_bulk_read(hdata->regmap, NCT6694_SHM_INTR_STATUS,
+			       reg_data, ARRAY_SIZE(reg_data));
+	if (ret)
+		return IRQ_NONE;
+
+	intr_status = get_unaligned_le32(reg_data);
+
+	while (intr_status) {
+		int irq = __ffs(intr_status);
+
+		generic_handle_irq_safe(irq_find_mapping(nct6694->domain, irq));
+		intr_status &= ~BIT(irq);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static void nct6694_irq_release(void *data)
+{
+	struct nct6694 *nct6694 = data;
+	struct nct6694_hif_data *hdata = nct6694->priv;
+	unsigned char cofs_ctl2;
+
+	/* Disable SIRQ interrupt */
+	cofs_ctl2 = inb(hdata->shm_base + NCT6694_SHM_COFS_CTL2);
+	cofs_ctl2 &= ~NCT6694_SHM_COFS_CTL2_COFS4W_IE;
+	outb(cofs_ctl2, hdata->shm_base + NCT6694_SHM_COFS_CTL2);
+}
+
+static int nct6694_irq_init(struct nct6694 *nct6694, int irq)
+{
+	struct nct6694_hif_data *hdata = nct6694->priv;
+	struct nct6694_sio_data *sio_data = hdata->sio_data;
+	unsigned char cofs_ctl2;
+
+	/* Set SIRQ number */
+	superio_enter(sio_data);
+	superio_select(sio_data, SIO_REG_LD_SHM);
+	if (!superio_inb(sio_data, SIO_REG_SHM_ENABLE)) {
+		superio_exit(sio_data);
+		return -EIO;
+	}
+	hdata->shm_base = superio_inw(sio_data, SIO_REG_SHM_BASE_ADDR);
+
+	superio_outb(sio_data, SIO_REG_SHM_IRQ_NR, irq);
+
+	superio_exit(sio_data);
+
+	/* Enable SIRQ interrupt */
+	cofs_ctl2 = inb(hdata->shm_base + NCT6694_SHM_COFS_CTL2);
+	cofs_ctl2 |= NCT6694_SHM_COFS_CTL2_COFS4W_IE;
+	outb(cofs_ctl2, hdata->shm_base + NCT6694_SHM_COFS_CTL2);
+
+	return 0;
+}
+
+static void nct6694_irq_enable(struct irq_data *data)
+{
+	struct nct6694 *nct6694 = irq_data_get_irq_chip_data(data);
+	irq_hw_number_t hwirq = irqd_to_hwirq(data);
+
+	guard(spinlock_irqsave)(&nct6694->irq_lock);
+
+	nct6694->irq_enable |= BIT(hwirq);
+}
+
+static void nct6694_irq_disable(struct irq_data *data)
+{
+	struct nct6694 *nct6694 = irq_data_get_irq_chip_data(data);
+	irq_hw_number_t hwirq = irqd_to_hwirq(data);
+
+	guard(spinlock_irqsave)(&nct6694->irq_lock);
+
+	nct6694->irq_enable &= ~BIT(hwirq);
+}
+
+static const struct irq_chip nct6694_irq_chip = {
+	.name = "nct6694-irq",
+	.flags = IRQCHIP_SKIP_SET_WAKE,
+	.irq_enable = nct6694_irq_enable,
+	.irq_disable = nct6694_irq_disable,
+};
+
+static void nct6694_irq_domain_remove(void *data)
+{
+	struct nct6694 *nct6694 = data;
+
+	irq_domain_remove(nct6694->domain);
+}
+
+static int nct6694_irq_domain_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
+{
+	struct nct6694 *nct6694 = d->host_data;
+
+	irq_set_chip_data(irq, nct6694);
+	irq_set_chip_and_handler(irq, &nct6694_irq_chip, handle_simple_irq);
+
+	return 0;
+}
+
+static void nct6694_irq_domain_unmap(struct irq_domain *d, unsigned int irq)
+{
+	irq_set_chip_and_handler(irq, NULL, NULL);
+	irq_set_chip_data(irq, NULL);
+}
+
+static const struct irq_domain_ops nct6694_irq_domain_ops = {
+	.map	= nct6694_irq_domain_map,
+	.unmap	= nct6694_irq_domain_unmap,
+};
+
+static const u8 sio_addrs[] = { 0x2e, 0x4e };
+
+static int nct6694_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct nct6694_sio_data *sio_data;
+	struct nct6694_hif_data *hdata;
+	struct nct6694 *data;
+	void __iomem *rpt_base, *msg_base;
+	int ret, i, irq;
+
+	sio_data = devm_kzalloc(dev, sizeof(*sio_data), GFP_KERNEL);
+	if (!sio_data)
+		return -ENOMEM;
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	hdata = devm_kzalloc(dev, sizeof(*hdata), GFP_KERNEL);
+	if (!hdata)
+		return -ENOMEM;
+
+	rpt_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(rpt_base))
+		return PTR_ERR(rpt_base);
+	msg_base = devm_platform_ioremap_resource(pdev, 1);
+	if (IS_ERR(msg_base))
+		return PTR_ERR(msg_base);
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return irq;
+
+	for (i = 0; i < ARRAY_SIZE(sio_addrs); i++) {
+		ret = nct6694_sio_find(sio_data, sio_addrs[i]);
+		if (!ret)
+			break;
+	}
+	if (ret)
+		return ret;
+
+	hdata->sio_data = sio_data;
+	hdata->msg_base = msg_base;
+	hdata->regmap = devm_regmap_init_mmio(dev, rpt_base,
+					      &nct6694_regmap_config);
+	if (IS_ERR(hdata->regmap))
+		return PTR_ERR(hdata->regmap);
+
+	data->dev = dev;
+	data->priv = hdata;
+	data->read_msg = nct6694_hif_read_msg;
+	data->write_msg = nct6694_hif_write_msg;
+
+	spin_lock_init(&data->irq_lock);
+
+	data->domain = irq_domain_create_simple(NULL, NCT6694_NR_IRQS, 0,
+						&nct6694_irq_domain_ops,
+						data);
+	if (!data->domain)
+		return -ENODEV;
+
+	ret = devm_add_action_or_reset(dev, nct6694_irq_domain_remove, data);
+	if (ret)
+		return ret;
+
+	ret = nct6694_irq_init(data, irq);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(dev, nct6694_irq_release, data);
+	if (ret)
+		return ret;
+
+	ret = devm_request_threaded_irq(dev, irq, NULL, nct6694_irq_handler,
+					IRQF_ONESHOT | IRQF_SHARED,
+					dev_name(dev), data);
+	if (ret)
+		return ret;
+
+	ret = devm_mutex_init(dev, &hdata->msg_lock);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, data);
+
+	return devm_mfd_add_devices(dev, 0, nct6694_devs, ARRAY_SIZE(nct6694_devs), NULL, 0, NULL);
+}
+
+static const struct acpi_device_id nct6694_acpi_ids[] = {
+	{ "NTN0538", 0 },
+	{}
+};
+
+static struct platform_driver nct6694_driver = {
+	.driver = {
+		.name = DRVNAME,
+		.acpi_match_table = nct6694_acpi_ids,
+	},
+	.probe	= nct6694_probe,
+};
+module_platform_driver(nct6694_driver);
+
+MODULE_DESCRIPTION("Nuvoton NCT6694 host-interface transport driver");
+MODULE_AUTHOR("Ming Yu <tmyu0@nuvoton.com>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/mfd/nct6694.c b/drivers/mfd/nct6694.c
index 8ce2c4985aab..903a0a7f0694 100644
--- a/drivers/mfd/nct6694.c
+++ b/drivers/mfd/nct6694.c
@@ -21,6 +21,27 @@
 #include <linux/spinlock.h>
 #include <linux/usb.h>
 
+#define NCT6694_VENDOR_ID	0x0416
+#define NCT6694_PRODUCT_ID	0x200B
+#define NCT6694_INT_IN_EP	0x81
+#define NCT6694_BULK_IN_EP	0x02
+#define NCT6694_BULK_OUT_EP	0x03
+
+#define NCT6694_URB_TIMEOUT	1000
+
+union __packed nct6694_usb_msg {
+	struct nct6694_cmd_header cmd_header;
+	struct nct6694_response_header response_header;
+};
+
+struct nct6694_usb_data {
+	struct mutex access_lock;
+	struct urb *int_in_urb;
+	struct usb_device *udev;
+	union nct6694_usb_msg *usb_msg;
+	__le32 *int_buffer;
+};
+
 static const struct mfd_cell nct6694_devs[] = {
 	MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 0),
 	MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 1),
@@ -57,7 +78,8 @@ static const struct mfd_cell nct6694_devs[] = {
 	MFD_CELL_NAME("nct6694-rtc"),
 };
 
-static int nct6694_response_err_handling(struct nct6694 *nct6694, unsigned char err_status)
+static int nct6694_usb_err_handling(struct nct6694 *nct6694,
+				    unsigned char err_status)
 {
 	switch (err_status) {
 	case NCT6694_NO_ERROR:
@@ -82,7 +104,7 @@ static int nct6694_response_err_handling(struct nct6694 *nct6694, unsigned char
 }
 
 /**
- * nct6694_read_msg() - Read message from NCT6694 device
+ * nct6694_usb_read_msg() - Read message from NCT6694 device via USB
  * @nct6694: NCT6694 device pointer
  * @cmd_hd: command header structure
  * @buf: buffer to store the response data
@@ -93,13 +115,16 @@ static int nct6694_response_err_handling(struct nct6694 *nct6694, unsigned char
  *
  * Return: Negative value on error or 0 on success.
  */
-int nct6694_read_msg(struct nct6694 *nct6694, const struct nct6694_cmd_header *cmd_hd, void *buf)
+static int nct6694_usb_read_msg(struct nct6694 *nct6694,
+				const struct nct6694_cmd_header *cmd_hd,
+				void *buf)
 {
-	union nct6694_usb_msg *msg = nct6694->usb_msg;
-	struct usb_device *udev = nct6694->udev;
+	struct nct6694_usb_data *udata = nct6694->priv;
+	union nct6694_usb_msg *msg = udata->usb_msg;
+	struct usb_device *udev = udata->udev;
 	int tx_len, rx_len, ret;
 
-	guard(mutex)(&nct6694->access_lock);
+	guard(mutex)(&udata->access_lock);
 
 	memcpy(&msg->cmd_header, cmd_hd, sizeof(*cmd_hd));
 	msg->cmd_header.hctrl = NCT6694_HCTRL_GET;
@@ -128,12 +153,11 @@ int nct6694_read_msg(struct nct6694 *nct6694, const struct nct6694_cmd_header *c
 		return -EIO;
 	}
 
-	return nct6694_response_err_handling(nct6694, msg->response_header.sts);
+	return nct6694_usb_err_handling(nct6694, msg->response_header.sts);
 }
-EXPORT_SYMBOL_GPL(nct6694_read_msg);
 
 /**
- * nct6694_write_msg() - Write message to NCT6694 device
+ * nct6694_usb_write_msg() - Write message to NCT6694 device via USB
  * @nct6694: NCT6694 device pointer
  * @cmd_hd: command header structure
  * @buf: buffer containing the data to be sent
@@ -143,13 +167,16 @@ EXPORT_SYMBOL_GPL(nct6694_read_msg);
  *
  * Return: Negative value on error or 0 on success.
  */
-int nct6694_write_msg(struct nct6694 *nct6694, const struct nct6694_cmd_header *cmd_hd, void *buf)
+static int nct6694_usb_write_msg(struct nct6694 *nct6694,
+				 const struct nct6694_cmd_header *cmd_hd,
+				 void *buf)
 {
-	union nct6694_usb_msg *msg = nct6694->usb_msg;
-	struct usb_device *udev = nct6694->udev;
+	struct nct6694_usb_data *udata = nct6694->priv;
+	union nct6694_usb_msg *msg = udata->usb_msg;
+	struct usb_device *udev = udata->udev;
 	int tx_len, rx_len, ret;
 
-	guard(mutex)(&nct6694->access_lock);
+	guard(mutex)(&udata->access_lock);
 
 	memcpy(&msg->cmd_header, cmd_hd, sizeof(*cmd_hd));
 	msg->cmd_header.hctrl = NCT6694_HCTRL_SET;
@@ -184,9 +211,8 @@ int nct6694_write_msg(struct nct6694 *nct6694, const struct nct6694_cmd_header *
 		return -EIO;
 	}
 
-	return nct6694_response_err_handling(nct6694, msg->response_header.sts);
+	return nct6694_usb_err_handling(nct6694, msg->response_header.sts);
 }
-EXPORT_SYMBOL_GPL(nct6694_write_msg);
 
 static void usb_int_callback(struct urb *urb)
 {
@@ -276,6 +302,7 @@ static int nct6694_usb_probe(struct usb_interface *iface,
 	struct usb_endpoint_descriptor *int_endpoint;
 	struct usb_host_interface *interface;
 	struct device *dev = &iface->dev;
+	struct nct6694_usb_data *udata;
 	struct nct6694 *nct6694;
 	int ret;
 
@@ -283,18 +310,28 @@ static int nct6694_usb_probe(struct usb_interface *iface,
 	if (!nct6694)
 		return -ENOMEM;
 
-	nct6694->usb_msg = devm_kzalloc(dev, sizeof(union nct6694_usb_msg), GFP_KERNEL);
-	if (!nct6694->usb_msg)
+	udata = devm_kzalloc(dev, sizeof(*udata), GFP_KERNEL);
+	if (!udata)
+		return -ENOMEM;
+
+	udata->usb_msg = devm_kzalloc(dev, sizeof(*udata->usb_msg), GFP_KERNEL);
+	if (!udata->usb_msg)
 		return -ENOMEM;
 
-	nct6694->int_buffer = devm_kzalloc(dev, sizeof(*nct6694->int_buffer), GFP_KERNEL);
-	if (!nct6694->int_buffer)
+	udata->int_buffer = devm_kzalloc(dev, sizeof(*udata->int_buffer), GFP_KERNEL);
+	if (!udata->int_buffer)
 		return -ENOMEM;
 
-	nct6694->int_in_urb = usb_alloc_urb(0, GFP_KERNEL);
-	if (!nct6694->int_in_urb)
+	udata->int_in_urb = usb_alloc_urb(0, GFP_KERNEL);
+	if (!udata->int_in_urb)
 		return -ENOMEM;
 
+	udata->udev = udev;
+
+	nct6694->priv = udata;
+	nct6694->read_msg = nct6694_usb_read_msg;
+	nct6694->write_msg = nct6694_usb_write_msg;
+
 	nct6694->domain = irq_domain_create_simple(NULL, NCT6694_NR_IRQS, 0,
 						   &nct6694_irq_domain_ops,
 						   nct6694);
@@ -304,11 +341,10 @@ static int nct6694_usb_probe(struct usb_interface *iface,
 	}
 
 	nct6694->dev = dev;
-	nct6694->udev = udev;
 
 	spin_lock_init(&nct6694->irq_lock);
 
-	ret = devm_mutex_init(dev, &nct6694->access_lock);
+	ret = devm_mutex_init(dev, &udata->access_lock);
 	if (ret)
 		goto err_irq_domain;
 
@@ -320,11 +356,11 @@ static int nct6694_usb_probe(struct usb_interface *iface,
 		goto err_irq_domain;
 	}
 
-	usb_fill_int_urb(nct6694->int_in_urb, udev, usb_rcvintpipe(udev, NCT6694_INT_IN_EP),
-			 nct6694->int_buffer, sizeof(*nct6694->int_buffer), usb_int_callback,
+	usb_fill_int_urb(udata->int_in_urb, udev, usb_rcvintpipe(udev, NCT6694_INT_IN_EP),
+			 udata->int_buffer, sizeof(*udata->int_buffer), usb_int_callback,
 			 nct6694, int_endpoint->bInterval);
 
-	ret = usb_submit_urb(nct6694->int_in_urb, GFP_KERNEL);
+	ret = usb_submit_urb(udata->int_in_urb, GFP_KERNEL);
 	if (ret)
 		goto err_irq_domain;
 
@@ -337,21 +373,22 @@ static int nct6694_usb_probe(struct usb_interface *iface,
 	return 0;
 
 err_mfd:
-	usb_kill_urb(nct6694->int_in_urb);
+	usb_kill_urb(udata->int_in_urb);
 err_irq_domain:
 	irq_domain_remove(nct6694->domain);
 err_urb:
-	usb_free_urb(nct6694->int_in_urb);
+	usb_free_urb(udata->int_in_urb);
 	return ret;
 }
 
 static void nct6694_usb_disconnect(struct usb_interface *iface)
 {
 	struct nct6694 *nct6694 = usb_get_intfdata(iface);
+	struct nct6694_usb_data *udata = nct6694->priv;
 
-	usb_kill_urb(nct6694->int_in_urb);
+	usb_kill_urb(udata->int_in_urb);
 	irq_domain_remove(nct6694->domain);
-	usb_free_urb(nct6694->int_in_urb);
+	usb_free_urb(udata->int_in_urb);
 }
 
 static const struct usb_device_id nct6694_ids[] = {
diff --git a/drivers/net/can/usb/nct6694_canfd.c b/drivers/net/can/usb/nct6694_canfd.c
index 29282c56430f..05db00455f63 100644
--- a/drivers/net/can/usb/nct6694_canfd.c
+++ b/drivers/net/can/usb/nct6694_canfd.c
@@ -17,12 +17,6 @@
 
 #define DEVICE_NAME "nct6694-canfd"
 
-/* USB command module type for NCT6694 CANfd controller.
- * This defines the module type used for communication with the NCT6694
- * CANfd controller over the USB interface.
- */
-#define NCT6694_CANFD_MOD			0x05
-
 /* Command 00h - CAN Setting and Initialization */
 #define NCT6694_CANFD_SETTING			0x00
 #define NCT6694_CANFD_SETTING_ACTIVE_CTRL1	BIT(0)
diff --git a/drivers/rtc/rtc-nct6694.c b/drivers/rtc/rtc-nct6694.c
index 35401a0d9cf5..c06902f150c9 100644
--- a/drivers/rtc/rtc-nct6694.c
+++ b/drivers/rtc/rtc-nct6694.c
@@ -14,13 +14,6 @@
 #include <linux/rtc.h>
 #include <linux/slab.h>
 
-/*
- * USB command module type for NCT6694 RTC controller.
- * This defines the module type used for communication with the NCT6694
- * RTC controller over the USB interface.
- */
-#define NCT6694_RTC_MOD		0x08
-
 /* Command 00h - RTC Time */
 #define NCT6694_RTC_TIME	0x0000
 #define NCT6694_RTC_TIME_SEL	0x00
diff --git a/drivers/watchdog/nct6694_wdt.c b/drivers/watchdog/nct6694_wdt.c
index 2b4b804a1739..847d8f1d1830 100644
--- a/drivers/watchdog/nct6694_wdt.c
+++ b/drivers/watchdog/nct6694_wdt.c
@@ -19,13 +19,6 @@
 
 #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)
diff --git a/include/linux/mfd/nct6694.h b/include/linux/mfd/nct6694.h
index 496da72949d9..ff0814dc82d4 100644
--- a/include/linux/mfd/nct6694.h
+++ b/include/linux/mfd/nct6694.h
@@ -2,7 +2,8 @@
 /*
  * Copyright (C) 2025 Nuvoton Technology Corp.
  *
- * Nuvoton NCT6694 USB transaction and data structure.
+ * Nuvoton NCT6694 core definitions shared by all transport drivers
+ * and sub-device drivers.
  */
 
 #ifndef __MFD_NCT6694_H
@@ -12,16 +13,17 @@
 #include <linux/spinlock.h>
 #include <linux/types.h>
 
-#define NCT6694_VENDOR_ID	0x0416
-#define NCT6694_PRODUCT_ID	0x200B
-#define NCT6694_INT_IN_EP	0x81
-#define NCT6694_BULK_IN_EP	0x02
-#define NCT6694_BULK_OUT_EP	0x03
-
 #define NCT6694_HCTRL_SET	0x40
 #define NCT6694_HCTRL_GET	0x80
 
-#define NCT6694_URB_TIMEOUT	1000
+#define NCT6694_HWMON_MOD	0x00
+#define NCT6694_PWM_MOD		0x01
+#define NCT6694_I2C_MOD		0x03
+#define NCT6694_CANFD_MOD	0x05
+#define NCT6694_WDT_MOD		0x07
+#define NCT6694_RTC_MOD		0x08
+#define NCT6694_RPT_MOD		0xFF
+#define NCT6694_GPIO_MOD	NCT6694_RPT_MOD
 
 enum nct6694_irq_id {
 	NCT6694_IRQ_GPIO0 = 0,
@@ -79,24 +81,33 @@ struct __packed nct6694_response_header {
 	__le16 len;
 };
 
-union __packed nct6694_usb_msg {
-	struct nct6694_cmd_header cmd_header;
-	struct nct6694_response_header response_header;
-};
-
 struct nct6694 {
 	struct device *dev;
 	struct irq_domain *domain;
-	struct mutex access_lock;
 	spinlock_t irq_lock;
-	struct urb *int_in_urb;
-	struct usb_device *udev;
-	union nct6694_usb_msg *usb_msg;
-	__le32 *int_buffer;
 	unsigned int irq_enable;
+
+	void *priv;
+	int (*read_msg)(struct nct6694 *nct6694,
+			const struct nct6694_cmd_header *cmd_hd,
+			void *buf);
+	int (*write_msg)(struct nct6694 *nct6694,
+			 const struct nct6694_cmd_header *cmd_hd,
+			 void *buf);
 };
 
-int nct6694_read_msg(struct nct6694 *nct6694, const struct nct6694_cmd_header *cmd_hd, void *buf);
-int nct6694_write_msg(struct nct6694 *nct6694, const struct nct6694_cmd_header *cmd_hd, void *buf);
+static inline int nct6694_read_msg(struct nct6694 *nct6694,
+				   const struct nct6694_cmd_header *cmd_hd,
+				   void *buf)
+{
+	return nct6694->read_msg(nct6694, cmd_hd, buf);
+}
+
+static inline int nct6694_write_msg(struct nct6694 *nct6694,
+				    const struct nct6694_cmd_header *cmd_hd,
+				    void *buf)
+{
+	return nct6694->write_msg(nct6694, cmd_hd, buf);
+}
 
 #endif
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/2] mfd: nct6694: Switch to devm_mfd_add_devices() and drop IDA
  2026-04-08  5:30 ` [PATCH v2 1/2] mfd: nct6694: Switch to devm_mfd_add_devices() and drop IDA a0282524688
@ 2026-04-08  5:48   ` sashiko-bot
  2026-04-08  7:25   ` Bartosz Golaszewski
  1 sibling, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-04-08  5:48 UTC (permalink / raw)
  To: Unknown; +Cc: linux-hwmon

Sashiko AI review found 2 potential issue(s):
- [High] Use-After-Free and WARN_ON due to Teardown Inversion on USB disconnect and probe failure
- [High] Sysfs Namespace Collision breaking support for multiple USB devices
--

commit 35c3701128ee68ba67a073bb4beea066282a9dba
Author: Ming Yu <a0282524688@gmail.com>

mfd: nct6694: Switch to devm_mfd_add_devices() and drop IDA
    
This commit switches the nct6694 core driver to use devm_mfd_add_devices()
for simplified device lifecycle management. It also drops the custom IDA
implementation and transitions to using pdev->id.

> diff --git a/drivers/mfd/nct6694.c b/drivers/mfd/nct6694.c
> index 308b2fda3055c..8ce2c4985aaba 100644
> --- a/drivers/mfd/nct6694.c
> +++ b/drivers/mfd/nct6694.c

[ ... ]

> @@ -332,11 +326,11 @@ static int nct6694_usb_probe(struct usb_interface *iface,
>  
>  	usb_set_intfdata(iface, nct6694);
>  
> -	ret = mfd_add_hotplug_devices(dev, nct6694_devs, ARRAY_SIZE(nct6694_devs));
> +	ret = devm_mfd_add_devices(dev, 0, nct6694_devs, ARRAY_SIZE(nct6694_devs), NULL, 0, NULL);

The previous mfd_add_hotplug_devices() call automatically used
PLATFORM_DEVID_AUTO, which assigns globally unique IDs to prevent sysfs
name collisions.

By switching to devm_mfd_add_devices() with an ID argument of 0, does this
bind pdev->id to the static cell->id (0, 1, 2...)?

If a user connects a second identical NCT6694 USB device, it seems the MFD
core might attempt to register devices with the exact same names (e.g.,
nct6694-gpio.0) on the global platform bus. Will this cause the platform bus
to reject the second device with a -EEXIST error, breaking hotplug support
for multiple devices?

[ ... ]

> @@ -344,11 +338,7 @@ static int nct6694_usb_probe(struct usb_interface *iface,
>  
>  err_mfd:
>  	usb_kill_urb(nct6694->int_in_urb);
> -err_ida:
> -	ida_destroy(&nct6694->wdt_ida);
> -	ida_destroy(&nct6694->canfd_ida);
> -	ida_destroy(&nct6694->i2c_ida);
> -	ida_destroy(&nct6694->gpio_ida);
> +err_irq_domain:
>  	irq_domain_remove(nct6694->domain);
>  err_urb:
>  	usb_free_urb(nct6694->int_in_urb);
> @@ -359,12 +349,7 @@ static void nct6694_usb_disconnect(struct usb_interface *iface)
>  {
>  	struct nct6694 *nct6694 = usb_get_intfdata(iface);
>  
> -	mfd_remove_devices(nct6694->dev);
>  	usb_kill_urb(nct6694->int_in_urb);
> -	ida_destroy(&nct6694->wdt_ida);
> -	ida_destroy(&nct6694->canfd_ida);
> -	ida_destroy(&nct6694->i2c_ida);
> -	ida_destroy(&nct6694->gpio_ida);
>  	irq_domain_remove(nct6694->domain);

By relying on devres for MFD device removal, the child devices will now be
unregistered during the devres cleanup phase, which happens after
nct6694_usb_disconnect() returns.

However, irq_domain_remove() is still explicitly called here. Will this
destroy and free the domain memory while the child devices are still
registered and holding active IRQ mappings?

If the domain is removed early, it looks like it would trigger a WARN_ON in
irq_domain_remove() due to active mappings. Furthermore, when devres later
unregisters the child devices, their teardown paths (such as
nct6694_irq_dispose_mapping() and nct6694_canfd_remove()) call
irq_dispose_mapping() and access the already-freed IRQ domain. Can this
lead to a use-after-free?

A similar ordering issue might also happen in the probe error path if
devm_mfd_add_devices() partially fails, as the error path frees the IRQ
domain before devres cleans up any successfully added child devices.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260408053037.1867092-1-a0282524688@gmail.com?part=1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] mfd: Add Host Interface (HIF) support for Nuvoton NCT6694
  2026-04-08  5:30 ` [PATCH v2 2/2] mfd: Add Host Interface (HIF) support for Nuvoton NCT6694 a0282524688
@ 2026-04-08  6:30   ` sashiko-bot
  0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-04-08  6:30 UTC (permalink / raw)
  To: Unknown; +Cc: linux-hwmon

Sashiko AI review found 8 potential issue(s):
- [High] devm_request_threaded_irq() with IRQF_SHARED and a NULL primary handler unconditionally fails.
- [High] Writing a Linux virtual IRQ number to the SIO hardware register corrupts interrupt routing.
- [High] Use-After-Free during USB disconnect due to incorrect teardown ordering of the IRQ domain.
- [High] Direct DMA to stack variables causes kernel panics.
- [High] Missing endpoint count check allows out-of-bounds array access.
- [Medium] Unbalanced release_region() and unsynchronized hardware access due to ignored superio_enter() return value.
- [Medium] Devres teardown ordering creates a window for an unhandled interrupt storm.
- [Medium] Unmapped hardware interrupts invoke the handler for Linux IRQ 0, causing system instability.
--

commit 60375817e31ea4cd313c9492ef71c36123f2e1d1
Author: Ming Yu <a0282524688@gmail.com>

mfd: Add Host Interface (HIF) support for Nuvoton NCT6694

This commit adds Host Interface (HIF) via eSPI for the Nuvoton NCT6694,
introducing a new transport driver while reorganizing the core and USB
driver. It provides common read/write functions and shared memory
interrupt handling.

> diff --git a/drivers/mfd/nct6694-hif.c b/drivers/mfd/nct6694-hif.c
> new file mode 100644
> index 0000000000000..7560754e74810
> --- /dev/null
> +++ b/drivers/mfd/nct6694-hif.c
[ ... ]
> +static irqreturn_t nct6694_irq_handler(int irq, void *data)
> +{
[ ... ]
> +	while (intr_status) {
> +		int irq = __ffs(intr_status);
> +
> +		generic_handle_irq_safe(irq_find_mapping(nct6694->domain, irq));
> +		intr_status &= ~BIT(irq);
> +	}
> +
> +	return IRQ_HANDLED;
> +}

If the hardware sets an unexpected interrupt bit that hasn't been mapped,
irq_find_mapping() returns 0. Does passing 0 directly into
generic_handle_irq_safe() inadvertently invoke the handler for Linux IRQ 0,
causing system instability? This applies to both this function and the USB
interrupt handler.

[ ... ]
> +static int nct6694_irq_init(struct nct6694 *nct6694, int irq)
> +{
> +	struct nct6694_hif_data *hdata = nct6694->priv;
> +	struct nct6694_sio_data *sio_data = hdata->sio_data;
> +	unsigned char cofs_ctl2;
> +
> +	/* Set SIRQ number */
> +	superio_enter(sio_data);

What happens if the request_muxed_region() inside superio_enter() fails and
returns -EBUSY? If the return value is ignored, the driver might proceed
with unsynchronized hardware access and later call superio_exit(),
releasing an I/O region it never successfully acquired.

> +	superio_select(sio_data, SIO_REG_LD_SHM);
> +	if (!superio_inb(sio_data, SIO_REG_SHM_ENABLE)) {
> +		superio_exit(sio_data);
> +		return -EIO;
> +	}
> +	hdata->shm_base = superio_inw(sio_data, SIO_REG_SHM_BASE_ADDR);
> +
> +	superio_outb(sio_data, SIO_REG_SHM_IRQ_NR, irq);

Could writing the Linux virtual IRQ number here misconfigure the hardware?
The irq parameter comes from platform_get_irq(), which returns a
dynamically allocated virtual IRQ number rather than the physical ISA or
eSPI IRQ line expected by the Super I/O chip.

[ ... ]
> +static int nct6694_probe(struct platform_device *pdev)
> +{
[ ... ]
> +	ret = nct6694_irq_init(data, irq);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_add_action_or_reset(dev, nct6694_irq_release, data);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_request_threaded_irq(dev, irq, NULL, nct6694_irq_handler,
> +					IRQF_ONESHOT | IRQF_SHARED,
> +					dev_name(dev), data);

The IRQ subsystem explicitly rejects shared interrupts without a primary
handler, as the default primary handler unconditionally masks the shared
line and starves other devices. Will this devm_request_threaded_irq() call
unconditionally return -EINVAL and fail to initialize?

Also, devres actions execute in reverse order during removal. Since the
hardware interrupt is enabled via nct6694_irq_init() before the IRQ handler
is requested, the handler will be freed before the hardware interrupt is
disabled during teardown. Could this create a window for an unhandled
interrupt storm?

> diff --git a/drivers/mfd/nct6694.c b/drivers/mfd/nct6694.c
> index 8ce2c4985aaba..903a0a7f0694c 100644
> --- a/drivers/mfd/nct6694.c
> +++ b/drivers/mfd/nct6694.c
[ ... ]
> +static int nct6694_usb_read_msg(struct nct6694 *nct6694,
> +				const struct nct6694_cmd_header *cmd_hd,
> +				void *buf)
> +{
[ ... ]
> +	/* Receive data packet from USB device */
> +	ret = usb_bulk_msg(udev, usb_rcvbulkpipe(udev, NCT6694_BULK_IN_EP), buf,
> +			   le16_to_cpu(cmd_hd->len), &rx_len, NCT6694_URB_TIMEOUT);

Child drivers often pass pointers to local stack variables as the buf
argument. When passed directly to usb_bulk_msg(), does this cause a kernel
panic if CONFIG_VMAP_STACK is enabled, due to performing DMA on a stack
variable? Would it be safer to allocate a bounce buffer for the payload?
This applies to nct6694_usb_write_msg() as well.

[ ... ]
>  static int nct6694_usb_probe(struct usb_interface *iface,
>  			     const struct usb_device_id *id)
>  {
[ ... ]
> +	interface = iface->cur_altsetting;
> +
> +	int_endpoint = &interface->endpoint[0].desc;
> +	if (!usb_endpoint_is_int_in(int_endpoint)) {
> +		ret = -ENODEV;
> +		goto err_irq_domain;
> +	}

If a malformed or malicious USB device presents an interface with zero
endpoints, will this direct access to interface->endpoint[0] result in an
out-of-bounds read?

[ ... ]
>  static void nct6694_usb_disconnect(struct usb_interface *iface)
>  {
>  	struct nct6694 *nct6694 = usb_get_intfdata(iface);
> +	struct nct6694_usb_data *udata = nct6694->priv;
>  
> -	usb_kill_urb(nct6694->int_in_urb);
> +	usb_kill_urb(udata->int_in_urb);
>  	irq_domain_remove(nct6694->domain);
> -	usb_free_urb(nct6694->int_in_urb);
> +	usb_free_urb(udata->int_in_urb);
>  }

The child MFD devices are registered using devm_mfd_add_devices(), which
means they will be unbound by devres after this disconnect function returns.
If irq_domain_remove() is called here, will the child drivers attempt to
access the already-freed IRQ domain during their devres cleanup routines,
leading to a use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260408053037.1867092-1-a0282524688@gmail.com?part=2

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/2] mfd: nct6694: Switch to devm_mfd_add_devices() and drop IDA
  2026-04-08  5:30 ` [PATCH v2 1/2] mfd: nct6694: Switch to devm_mfd_add_devices() and drop IDA a0282524688
  2026-04-08  5:48   ` sashiko-bot
@ 2026-04-08  7:25   ` Bartosz Golaszewski
  2026-04-10  9:59     ` Ming Yu
  1 sibling, 1 reply; 7+ messages in thread
From: Bartosz Golaszewski @ 2026-04-08  7:25 UTC (permalink / raw)
  To: a0282524688
  Cc: tmyu0, linusw, linux, andi.shyti, lee, mkl, mailhol,
	alexandre.belloni, wim, linux-kernel, linux-gpio, linux-i2c,
	linux-can, netdev, linux-watchdog, linux-hwmon, linux-rtc,
	linux-usb

On Wed, Apr 8, 2026 at 7:31 AM <a0282524688@gmail.com> wrote:
>
> From: Ming Yu <a0282524688@gmail.com>
>
> Currently, the nct6694 core driver uses mfd_add_hotplug_devices()
> and an IDA to manage subdevice IDs.
>
> Switch the core implementation to use the managed
> devm_mfd_add_devices() API, which simplifies the error handling and
> device lifecycle management. Concurrently, drop the custom IDA
> implementation and transition to using pdev->id.
>
> Signed-off-by: Ming Yu <a0282524688@gmail.com>
> ---

This does result in a nice code shrink but I'd split this commit into
two: one switching to using MFD_CELL_BASIC() with hard-coded devices
IDs and one completing the transition to devres.

Bart

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/2] mfd: nct6694: Switch to devm_mfd_add_devices() and drop IDA
  2026-04-08  7:25   ` Bartosz Golaszewski
@ 2026-04-10  9:59     ` Ming Yu
  0 siblings, 0 replies; 7+ messages in thread
From: Ming Yu @ 2026-04-10  9:59 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: tmyu0, linusw, linux, andi.shyti, lee, mkl, mailhol,
	alexandre.belloni, wim, linux-kernel, linux-gpio, linux-i2c,
	linux-can, netdev, linux-watchdog, linux-hwmon, linux-rtc,
	linux-usb

Hi Bart, all,

Thanks for the review.

Bartosz Golaszewski <brgl@kernel.org> 於 2026年4月8日週三 下午3:25寫道:
>
> On Wed, Apr 8, 2026 at 7:31 AM <a0282524688@gmail.com> wrote:
> >
> > From: Ming Yu <a0282524688@gmail.com>
> >
> > Currently, the nct6694 core driver uses mfd_add_hotplug_devices()
> > and an IDA to manage subdevice IDs.
> >
> > Switch the core implementation to use the managed
> > devm_mfd_add_devices() API, which simplifies the error handling and
> > device lifecycle management. Concurrently, drop the custom IDA
> > implementation and transition to using pdev->id.
> >
> > Signed-off-by: Ming Yu <a0282524688@gmail.com>
> > ---
>
> This does result in a nice code shrink but I'd split this commit into
> two: one switching to using MFD_CELL_BASIC() with hard-coded devices
> IDs and one completing the transition to devres.
>


You are right that this change is trying to do too much at once, and
splitting it as you suggested would make the series much cleaner.

After looking more closely at the ID handling and hotplug
implications, I realized that switching to devm_mfd_add_devices() and
dropping the IDA is not a good fit for this driver. The current
mfd_add_hotplug_devices() path uses PLATFORM_DEVID_AUTO, which gives
globally unique device IDs and avoids sysfs name collisions. If we
switch to devm_mfd_add_devices() with fixed IDs, multiple identical
NCT6694 devices can end up registering subdevices with the same
platform device names, which would break hotplug support when more
than one device is present.

So I think it is better not to pursue this direction further.

For the next revision, I will drop this part of the change and keep
the existing MFD core logic, including the IDA usage. The series will
focus on adding the nct6694-hif MFD driver only, and I will add the
IDA initialization there as needed.

Thanks again for the suggestion and review.


Best regards,
Ming

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-04-10  9:59 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-08  5:30 [PATCH v2 0/2] mfd: nct6694: Refactor transport layer and add HIF (eSPI) support a0282524688
2026-04-08  5:30 ` [PATCH v2 1/2] mfd: nct6694: Switch to devm_mfd_add_devices() and drop IDA a0282524688
2026-04-08  5:48   ` sashiko-bot
2026-04-08  7:25   ` Bartosz Golaszewski
2026-04-10  9:59     ` Ming Yu
2026-04-08  5:30 ` [PATCH v2 2/2] mfd: Add Host Interface (HIF) support for Nuvoton NCT6694 a0282524688
2026-04-08  6:30   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox