Dear Lee,

Thank you for reviewing.
I will fix the code which you mentioned.

Additionally, I will revert the mfd_cell changes to match version 10. (i.e.,
static const struct mfd_cell nct6694_dev[] = {
    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),
    ...
}; ), since I found that using devm_mfd_add_devices() with id field
set to 0 meets my needs. This allows me to use pdev->id in the
sub-drivers.


Best regards,
Ming

Lee Jones [off-list ref] 於 2025年5月23日 週五 下午5:02寫道：

On Tue, 20 May 2025, a0282524688@gmail.com wrote:

quoted

From: Ming Yu <tmyu0@nuvoton.com>

The Nuvoton NCT6694 provides an USB interface to the host to
access its features.

Sub-devices can use the USB functions nct6694_read_msg() and
nct6694_write_msg() to issue a command. They can also request
interrupt that will be called when the USB device receives its
interrupt pipe.

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

Changes since version 10:
- Add change log for the patch
- Fix mfd_cell to MFD_CELL_NAME()
- Remove unnecessary blank line

Changes since version 9:
- Add KernelDoc to exported functions

Changes since version 8:
- Modify the signed-off-by with my work address
- Rename all MFD cell names to "nct6694-xxx"
- Fix some comments in nct6694.c and in nct6694.h

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

Changes since version 6:

Changes since version 5:
- Fix mfd_cell to MFD_CELL_NAME() and MFD_CELL_BASIC()
- Drop unnecessary macros

Changes since version 4:
- Modify arguments in read/write function to a pointer to cmd_header

Changes since version 3:
- Modify array buffer to structure
- Fix defines and comments
- Add header <linux/bits.h> and use BIT macro
- Modify mutex_init() to devm_mutex_init()

Changes since version 2:

Changes since version 1:
- Implement IRQ domain to handle IRQ demux
- Modify USB_DEVICE to USB_DEVICE_AND_INTERFACE_INFO API
- Add command structure
- Fix USB functions
- Sort each driver's header files alphabetically

 MAINTAINERS                 |   6 +
 drivers/mfd/Kconfig         |  15 ++
 drivers/mfd/Makefile        |   2 +
 drivers/mfd/nct6694.c       | 387 ++++++++++++++++++++++++++++++++++++
 include/linux/mfd/nct6694.h |  98 +++++++++
 5 files changed, 508 insertions(+)
 create mode 100644 drivers/mfd/nct6694.c
 create mode 100644 include/linux/mfd/nct6694.h

[...]

I was just going to fix this up for you when I applied the set, but
seeing as it looks like you have to re-submit anyway ...

quoted

+static const struct usb_device_id nct6694_ids[] = {
+     { USB_DEVICE_AND_INTERFACE_INFO(NCT6694_VENDOR_ID, NCT6694_PRODUCT_ID, 0xFF, 0x00, 0x00)},

You need a space before the '}'.

quoted

+     {}
+};
+MODULE_DEVICE_TABLE(usb, nct6694_ids);
+
+static struct usb_driver nct6694_usb_driver = {
+     .name           = "nct6694",
+     .id_table       = nct6694_ids,
+     .probe          = nct6694_usb_probe,
+     .disconnect     = nct6694_usb_disconnect,
+};
+module_usb_driver(nct6694_usb_driver);
+
+MODULE_DESCRIPTION("Nuvoton NCT6694 core driver");
+MODULE_AUTHOR("Ming Yu [off-list ref]");
+MODULE_LICENSE("GPL");

diff --git a/include/linux/mfd/nct6694.h b/include/linux/mfd/nct6694.h
new file mode 100644
index 000000000000..5e172609be3f
--- /dev/null
+++ b/include/linux/mfd/nct6694.h

@@ -0,0 +1,98 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2025 Nuvoton Technology Corp.
+ *
+ * Nuvoton NCT6694 USB transaction and data structure.
+ */
+
+#ifndef __MFD_NCT6694_H
+#define __MFD_NCT6694_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
+
+enum nct6694_irq_id {
+     NCT6694_IRQ_GPIO0 = 0,
+     NCT6694_IRQ_GPIO1,
+     NCT6694_IRQ_GPIO2,
+     NCT6694_IRQ_GPIO3,
+     NCT6694_IRQ_GPIO4,
+     NCT6694_IRQ_GPIO5,
+     NCT6694_IRQ_GPIO6,
+     NCT6694_IRQ_GPIO7,
+     NCT6694_IRQ_GPIO8,
+     NCT6694_IRQ_GPIO9,
+     NCT6694_IRQ_GPIOA,
+     NCT6694_IRQ_GPIOB,
+     NCT6694_IRQ_GPIOC,
+     NCT6694_IRQ_GPIOD,
+     NCT6694_IRQ_GPIOE,
+     NCT6694_IRQ_GPIOF,
+     NCT6694_IRQ_CAN0,
+     NCT6694_IRQ_CAN1,
+     NCT6694_IRQ_RTC,
+     NCT6694_NR_IRQS,
+};
+
+enum nct6694_response_err_status {
+     NCT6694_NO_ERROR = 0,
+     NCT6694_FORMAT_ERROR,
+     NCT6694_RESERVED1,
+     NCT6694_RESERVED2,
+     NCT6694_NOT_SUPPORT_ERROR,
+     NCT6694_NO_RESPONSE_ERROR,
+     NCT6694_TIMEOUT_ERROR,
+     NCT6694_PENDING,
+};
+
+struct __packed nct6694_cmd_header {
+     u8 rsv1;
+     u8 mod;
+     union __packed {
+             __le16 offset;
+             struct __packed {
+                     u8 cmd;
+                     u8 sel;
+             };
+     };
+     u8 hctrl;
+     u8 rsv2;
+     __le16 len;
+};
+
+struct __packed nct6694_response_header {
+     u8 sequence_id;
+     u8 sts;
+     u8 reserved[4];
+     __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;
+     struct mutex irq_lock;
+     struct urb *int_in_urb;
+     struct usb_device *udev;
+     union nct6694_usb_msg *usb_msg;
+     unsigned char *int_buffer;
+     unsigned int irq_enable;
+};
+
+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);
+
+#endif
--

2.34.1

--
Lee Jones [李琼斯]