Re: [PATCH v11 1/7] mfd: Add core driver for Nuvoton NCT6694

Lee Jones
2025-05-23T09:02:15Z

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

> 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 ...

> +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 '}'.

> + {}
> +};
> +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 <tmyu0@nuvoton.com>");
> +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 [李琼斯]