| Message ID | 20241210104524.2466586-2-tmyu0@nuvoton.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add Nuvoton NCT6694 MFD drivers | expand |
On 12/10/2024 11:45 AM, Ming Yu wrote: > 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 implements USB device functionality and shares the > chip's peripherals as a child device. > > Each child device 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> > --- > MAINTAINERS | 7 + > drivers/mfd/Kconfig | 10 + > drivers/mfd/Makefile | 2 + > drivers/mfd/nct6694.c | 382 ++++++++++++++++++++++++++++++++++++ > include/linux/mfd/nct6694.h | 139 +++++++++++++ > 5 files changed, 540 insertions(+) > create mode 100644 drivers/mfd/nct6694.c > create mode 100644 include/linux/mfd/nct6694.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index 67d2159406c2..9c9a94ff8f98 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -16541,6 +16541,13 @@ F: drivers/nubus/ > F: include/linux/nubus.h > F: include/uapi/linux/nubus.h > > +NUVOTON NCT6694 MFD DRIVER > +M: Ming Yu <tmyu0@nuvoton.com> > +L: linux-kernel@vger.kernel.org > +S: Supported > +F: drivers/mfd/nct6694.c > +F: include/linux/mfd/nct6694.h > + > NVIDIA (rivafb and nvidiafb) FRAMEBUFFER DRIVER > M: Antonino Daplas <adaplas@gmail.com> > L: linux-fbdev@vger.kernel.org > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > index 03c1e4e3eea4..07ccc32f9985 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -558,6 +558,16 @@ config MFD_MX25_TSADC > i.MX25 processors. They consist of a conversion queue for general > purpose ADC and a queue for Touchscreens. > > +config MFD_NCT6694 > + tristate "Nuvoton NCT6694 support" > + select MFD_CORE > + depends on USB > + help > + This adds support for Nuvoton USB device NCT6694 sharing peripherals > + This includes the USB devcie driver and core APIs. > + Additional drivers must be enabled in order to use the functionality > + of the device. > + > config MFD_HI6421_PMIC > tristate "HiSilicon Hi6421 PMU/Codec IC" > depends on OF > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > index e057d6d6faef..9d0365ba6a26 100644 > --- a/drivers/mfd/Makefile > +++ b/drivers/mfd/Makefile > @@ -117,6 +117,8 @@ obj-$(CONFIG_TWL6040_CORE) += twl6040.o > > obj-$(CONFIG_MFD_MX25_TSADC) += fsl-imx25-tsadc.o > > +obj-$(CONFIG_MFD_NCT6694) += nct6694.o > + > obj-$(CONFIG_MFD_MC13XXX) += mc13xxx-core.o > obj-$(CONFIG_MFD_MC13XXX_SPI) += mc13xxx-spi.o > obj-$(CONFIG_MFD_MC13XXX_I2C) += mc13xxx-i2c.o > diff --git a/drivers/mfd/nct6694.c b/drivers/mfd/nct6694.c > new file mode 100644 > index 000000000000..071333d13eae > --- /dev/null > +++ b/drivers/mfd/nct6694.c > @@ -0,0 +1,382 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Nuvoton NCT6694 MFD driver based on USB interface. > + * > + * Copyright (C) 2024 Nuvoton Technology Corp. > + */ > + > +#include <linux/interrupt.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/slab.h> > +#include <linux/usb.h> > + > +#define MFD_DEV_SIMPLE(_name) \ > +{ \ > + .name = NCT6694_DEV_##_name, \ > +} \ > + > +#define MFD_DEV_WITH_ID(_name, _id) \ > +{ \ > + .name = NCT6694_DEV_##_name, \ > + .id = _id, \ > +} > + > +/* MFD device resources */ > +static const struct mfd_cell nct6694_dev[] = { > + MFD_DEV_WITH_ID(GPIO, 0x0), > + MFD_DEV_WITH_ID(GPIO, 0x1), > + MFD_DEV_WITH_ID(GPIO, 0x2), > + MFD_DEV_WITH_ID(GPIO, 0x3), > + MFD_DEV_WITH_ID(GPIO, 0x4), > + MFD_DEV_WITH_ID(GPIO, 0x5), > + MFD_DEV_WITH_ID(GPIO, 0x6), > + MFD_DEV_WITH_ID(GPIO, 0x7), > + MFD_DEV_WITH_ID(GPIO, 0x8), > + MFD_DEV_WITH_ID(GPIO, 0x9), > + MFD_DEV_WITH_ID(GPIO, 0xA), > + MFD_DEV_WITH_ID(GPIO, 0xB), > + MFD_DEV_WITH_ID(GPIO, 0xC), > + MFD_DEV_WITH_ID(GPIO, 0xD), > + MFD_DEV_WITH_ID(GPIO, 0xE), > + MFD_DEV_WITH_ID(GPIO, 0xF), > + > + MFD_DEV_WITH_ID(I2C, 0x0), > + MFD_DEV_WITH_ID(I2C, 0x1), > + MFD_DEV_WITH_ID(I2C, 0x2), > + MFD_DEV_WITH_ID(I2C, 0x3), > + MFD_DEV_WITH_ID(I2C, 0x4), > + MFD_DEV_WITH_ID(I2C, 0x5), > + > + MFD_DEV_WITH_ID(CAN, 0x0), > + MFD_DEV_WITH_ID(CAN, 0x1), > + > + MFD_DEV_WITH_ID(WDT, 0x0), > + MFD_DEV_WITH_ID(WDT, 0x1), > + > + MFD_DEV_SIMPLE(HWMON), > + MFD_DEV_SIMPLE(RTC), > +}; > + > +static int nct6694_response_err_handling(struct nct6694 *nct6694, > + unsigned char err_status) > +{ > + struct device *dev = &nct6694->udev->dev; > + > + switch (err_status) { > + case NCT6694_NO_ERROR: > + return err_status; > + case NCT6694_NOT_SUPPORT_ERROR: > + dev_dbg(dev, "%s: Command is not support!\n", __func__); > + break; > + case NCT6694_NO_RESPONSE_ERROR: > + dev_dbg(dev, "%s: Command is no response!\n", __func__); > + break; > + case NCT6694_TIMEOUT_ERROR: > + dev_dbg(dev, "%s: Command is timeout!\n", __func__); > + break; > + case NCT6694_PENDING: > + dev_dbg(dev, "%s: Command is pending!\n", __func__); > + break; > + default: > + return -EINVAL; > + } > + > + return -EIO; > +} > + > +int nct6694_read_msg(struct nct6694 *nct6694, u8 mod, u16 offset, > + u16 length, void *buf) > +{ > + struct nct6694_cmd_header *cmd_header = nct6694->cmd_header; > + struct nct6694_response_header *response_header = nct6694->response_header; RCT violation > + struct usb_device *udev = nct6694->udev; > + int tx_len, rx_len, ret; > + > + guard(mutex)(&nct6694->access_lock); > + > + /* Send command packet to USB device */ > + cmd_header->mod = mod; > + cmd_header->cmd = offset & 0xFF; > + cmd_header->sel = (offset >> 8) & 0xFF; > + cmd_header->hctrl = NCT6694_HCTRL_GET; > + cmd_header->len = length; > + > + ret = usb_bulk_msg(udev, usb_sndbulkpipe(udev, NCT6694_BULK_OUT_EP), > + cmd_header, NCT6694_CMD_PACKET_SZ, &tx_len, > + nct6694->timeout); > + if (ret) > + return ret; > + > + /* Receive response packet from USB device */ > + ret = usb_bulk_msg(udev, usb_rcvbulkpipe(udev, NCT6694_BULK_IN_EP), > + response_header, NCT6694_CMD_PACKET_SZ, &rx_len, > + nct6694->timeout); > + if (ret) > + return ret; > + > + ret = usb_bulk_msg(udev, usb_rcvbulkpipe(udev, NCT6694_BULK_IN_EP), > + buf, NCT6694_MAX_PACKET_SZ, &rx_len, nct6694->timeout); > + if (ret) > + return ret; > + > + return nct6694_response_err_handling(nct6694, response_header->sts); > +} > +EXPORT_SYMBOL(nct6694_read_msg); > + > +int nct6694_write_msg(struct nct6694 *nct6694, u8 mod, u16 offset, > + u16 length, void *buf) > +{ > + struct nct6694_cmd_header *cmd_header = nct6694->cmd_header; > + struct nct6694_response_header *response_header = nct6694->response_header; Same as above > + struct usb_device *udev = nct6694->udev; > + int tx_len, rx_len, ret; > + > + guard(mutex)(&nct6694->access_lock); > + > + /* Send command packet to USB device */ > + cmd_header->mod = mod; > + cmd_header->cmd = offset & 0xFF; > + cmd_header->sel = (offset >> 8) & 0xFF; > + cmd_header->hctrl = NCT6694_HCTRL_SET; > + cmd_header->len = length; > + > + ret = usb_bulk_msg(udev, usb_sndbulkpipe(udev, NCT6694_BULK_OUT_EP), > + cmd_header, NCT6694_CMD_PACKET_SZ, &tx_len, > + nct6694->timeout); > + if (ret) > + return ret; > + > + ret = usb_bulk_msg(udev, usb_sndbulkpipe(udev, NCT6694_BULK_OUT_EP), > + buf, length, &tx_len, nct6694->timeout); > + if (ret) > + return ret; > + > + /* Receive response packet from USB device */ > + ret = usb_bulk_msg(udev, usb_rcvbulkpipe(udev, NCT6694_BULK_IN_EP), > + response_header, NCT6694_CMD_PACKET_SZ, &rx_len, > + nct6694->timeout); > + if (ret) > + return ret; > + > + ret = usb_bulk_msg(udev, usb_rcvbulkpipe(udev, NCT6694_BULK_IN_EP), > + buf, NCT6694_MAX_PACKET_SZ, &rx_len, nct6694->timeout); > + if (ret) > + return ret; > + > + return nct6694_response_err_handling(nct6694, response_header->sts); > +} > +EXPORT_SYMBOL(nct6694_write_msg); > + > +static void usb_int_callback(struct urb *urb) > +{ > + struct nct6694 *nct6694 = urb->context; > + struct device *dev = &nct6694->udev->dev; > + unsigned int *int_status = urb->transfer_buffer; Again RCT violation > + int ret; > + > + switch (urb->status) { > + case 0: > + break; > + case -ECONNRESET: > + case -ENOENT: > + case -ESHUTDOWN: > + return; > + default: > + goto resubmit; > + } > + > + while (*int_status) { > + int irq = __ffs(*int_status); > + > + if (*int_status & (1 << irq)) > + generic_handle_irq_safe(irq_find_mapping(nct6694->domain, irq)); > + > + *int_status &= ~(1 << irq); > + } > + > +resubmit: > + ret = usb_submit_urb(urb, GFP_ATOMIC); > + if (ret) > + dev_dbg(dev, "%s: Failed to resubmit urb, status %d", > + __func__, ret); > +} > + > +static void nct6694_irq_lock(struct irq_data *data) > +{ > + struct nct6694 *nct6694 = irq_data_get_irq_chip_data(data); > + > + mutex_lock(&nct6694->irq_lock); > +} > + > +static void nct6694_irq_sync_unlock(struct irq_data *data) > +{ > + struct nct6694 *nct6694 = irq_data_get_irq_chip_data(data); > + > + mutex_unlock(&nct6694->irq_lock); > +} > + > +static void nct6694_irq_enable(struct irq_data *data) > +{ > + struct nct6694 *nct6694 = irq_data_get_irq_chip_data(data); > + unsigned int irq = data->hwirq; > + > + nct6694->irq_enable |= (1 << irq); I wonder if this is necessary to create local variable irq if this is used only once. Maybe instead: nct6694->irq_enable |= (1 << data->hwirq); Not big thing though > +} > + > +static void nct6694_irq_disable(struct irq_data *data) > +{ > + struct nct6694 *nct6694 = irq_data_get_irq_chip_data(data); > + unsigned int irq = data->hwirq; > + > + nct6694->irq_enable &= ~(1 << irq); > +} > + > +static struct irq_chip nct6694_irq_chip = { > + .name = "nct6694-irq", > + .flags = IRQCHIP_SKIP_SET_WAKE, > + .irq_bus_lock = nct6694_irq_lock, > + .irq_bus_sync_unlock = nct6694_irq_sync_unlock, > + .irq_enable = nct6694_irq_enable, > + .irq_disable = nct6694_irq_disable, > +}; > + > +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 int nct6694_usb_probe(struct usb_interface *iface, > + const struct usb_device_id *id) > +{ > + struct usb_device *udev = interface_to_usbdev(iface); > + struct device *dev = &udev->dev; > + struct usb_host_interface *interface; > + struct usb_endpoint_descriptor *int_endpoint; > + struct nct6694 *nct6694; > + struct nct6694_cmd_header *cmd_header; > + struct nct6694_response_header *response_header; > + int pipe, maxp; > + int ret; Please follow RCT rule > + > + interface = iface->cur_altsetting; > + > + int_endpoint = &interface->endpoint[0].desc; > + if (!usb_endpoint_is_int_in(int_endpoint)) > + return -ENODEV; > + > + nct6694 = devm_kzalloc(dev, sizeof(*nct6694), GFP_KERNEL); > + if (!nct6694) > + return -ENOMEM; > + > + pipe = usb_rcvintpipe(udev, NCT6694_INT_IN_EP); > + maxp = usb_maxpacket(udev, pipe); > + > + cmd_header = devm_kzalloc(dev, sizeof(*cmd_header), > + GFP_KERNEL); > + if (!cmd_header) > + return -ENOMEM; > + > + response_header = devm_kzalloc(dev, sizeof(*response_header), > + GFP_KERNEL); > + if (!response_header) > + return -ENOMEM; > + > + nct6694->int_buffer = devm_kcalloc(dev, NCT6694_MAX_PACKET_SZ, > + sizeof(unsigned char), GFP_KERNEL); > + if (!nct6694->int_buffer) > + return -ENOMEM; > + > + nct6694->int_in_urb = usb_alloc_urb(0, GFP_KERNEL); > + if (!nct6694->int_in_urb) > + return -ENOMEM;> + > + nct6694->domain = irq_domain_add_simple(NULL, NCT6694_NR_IRQS, 0, > + &nct6694_irq_domain_ops, > + nct6694); > + if (!nct6694->domain) > + return -ENODEV; > + > + nct6694->udev = udev; > + nct6694->timeout = NCT6694_URB_TIMEOUT; /* Wait until urb complete */ > + nct6694->cmd_header = cmd_header; > + nct6694->response_header = response_header; > + > + mutex_init(&nct6694->access_lock); > + mutex_init(&nct6694->irq_lock); > + > + usb_fill_int_urb(nct6694->int_in_urb, udev, pipe, > + nct6694->int_buffer, maxp, usb_int_callback, > + nct6694, int_endpoint->bInterval); > + ret = usb_submit_urb(nct6694->int_in_urb, GFP_KERNEL); > + if (ret) > + goto err_urb; > + > + dev_set_drvdata(dev, nct6694); > + usb_set_intfdata(iface, nct6694); > + > + ret = mfd_add_hotplug_devices(dev, nct6694_dev, ARRAY_SIZE(nct6694_dev)); > + if (ret) > + goto err_mfd; > + > + dev_info(dev, "Probed device: (%04X:%04X)\n", id->idVendor, id->idProduct); > + return 0; > + > +err_mfd: > + usb_kill_urb(nct6694->int_in_urb); > +err_urb: > + usb_free_urb(nct6694->int_in_urb); > + return dev_err_probe(dev, ret, "Probe failed\n"); > +} > + > +static void nct6694_usb_disconnect(struct usb_interface *iface) > +{ > + struct usb_device *udev = interface_to_usbdev(iface); > + struct nct6694 *nct6694 = usb_get_intfdata(iface); > + > + mfd_remove_devices(&udev->dev); > + usb_kill_urb(nct6694->int_in_urb); > + usb_free_urb(nct6694->int_in_urb); > +} > + > +static const struct usb_device_id nct6694_ids[] = { > + { USB_DEVICE_AND_INTERFACE_INFO(NCT6694_VENDOR_ID, > + NCT6694_PRODUCT_ID, > + 0xFF, 0x00, 0x00)}, > + {} > +}; > +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("USB-MFD driver for NCT6694"); > +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..9733d47f5a01 > --- /dev/null > +++ b/include/linux/mfd/nct6694.h > @@ -0,0 +1,139 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Nuvoton NCT6694 USB transaction and data structure. > + * > + * Copyright (C) 2024 Nuvoton Technology Corp. > + */ > + > +#ifndef __MFD_NCT6694_H > +#define __MFD_NCT6694_H > + > +#define NCT6694_DEV_GPIO "nct6694-gpio" > +#define NCT6694_DEV_I2C "nct6694-i2c" > +#define NCT6694_DEV_CAN "nct6694-can" > +#define NCT6694_DEV_WDT "nct6694-wdt" > +#define NCT6694_DEV_HWMON "nct6694-hwmon" > +#define NCT6694_DEV_RTC "nct6694-rtc" > + > +#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_MAX_PACKET_SZ 0x200 > + > +#define NCT6694_CMD_PACKET_SZ 0x8 > +#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_CAN1, > + NCT6694_IRQ_CAN2, > + 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 nct6694 { > + struct usb_device *udev; > + struct urb *int_in_urb; > + struct irq_domain *domain; > + struct nct6694_cmd_header *cmd_header; > + struct nct6694_response_header *response_header; > + struct mutex access_lock; > + struct mutex irq_lock; > + unsigned char *int_buffer; > + unsigned int irq_enable; > + /* time in msec to wait for the urb to the complete */ > + long timeout; > +}; > + > +struct nct6694_cmd_header { > + unsigned char rsv1; > + unsigned char mod; > + unsigned char cmd; > + unsigned char sel; > + unsigned char hctrl; > + unsigned char rsv2; > + unsigned short len; > +} __packed; > + > +struct nct6694_response_header { > + unsigned char sequence_id; > + unsigned char sts; > + unsigned int rsv; > + unsigned short len; > +} __packed; > + > +/* > + * nct6694_read_msg - Receive data from NCT6694 USB device > + * > + * @nct6694 - Nuvoton NCT6694 structure > + * @mod - Module byte > + * @offset - Offset byte or (Select byte | Command byte) > + * @length - Length byte > + * @buf - Read data from rx buffer > + * > + * USB Transaction format: > + * > + * OUT |RSV|MOD|CMD|SEL|HCTL|RSV|LEN_L|LEN_H| > + * OUT |SEQ|STS|RSV|RSV|RSV|RSV||LEN_L|LEN_H| > + * IN |-------D------A------D------A-------| > + * IN ...... > + * IN |-------D------A------D------A-------| > + */ > +int nct6694_read_msg(struct nct6694 *nct6694, u8 mod, u16 offset, > + u16 length, void *buf); > + > +/* > + * nct6694_read_msg - Transmit data to NCT6694 USB device > + * > + * @nct6694 - Nuvoton NCT6694 structure > + * @mod - Module byte > + * @offset - Offset byte or (Select byte | Command byte) > + * @length - Length byte > + * @buf - Write data to tx buffer > + * > + * USB Transaction format: > + * > + * OUT |RSV|MOD|CMD|SEL|HCTL|RSV|LEN_L|LEN_H| > + * OUT |-------D------A------D------A-------| > + * OUT ...... > + * OUT |-------D------A------D------A-------| > + * IN |SEQ|STS|RSV|RSV|RSV|RSV||LEN_L|LEN_H| > + * IN |-------D------A------D------A-------| > + * IN ...... > + * IN |-------D------A------D------A-------| > + */ > +int nct6694_write_msg(struct nct6694 *nct6694, u8 mod, u16 offset, > + u16 length, void *buf); > + > +#endif
On Tue, 10 Dec 2024 11:57:41 +0100 Mateusz Polchlopek wrote: > > +int nct6694_read_msg(struct nct6694 *nct6694, u8 mod, u16 offset, > > + u16 length, void *buf) > > +{ > > + struct nct6694_cmd_header *cmd_header = nct6694->cmd_header; > > + struct nct6694_response_header *response_header = nct6694->response_header; > > RCT violation This code is not under net not drivers/net As a general rule please focus on functional review, formatting and process issues are harder to judge unless you read all of the mailing list traffic.
On 12/11/2024 2:56 AM, Jakub Kicinski wrote: > On Tue, 10 Dec 2024 11:57:41 +0100 Mateusz Polchlopek wrote: >>> +int nct6694_read_msg(struct nct6694 *nct6694, u8 mod, u16 offset, >>> + u16 length, void *buf) >>> +{ >>> + struct nct6694_cmd_header *cmd_header = nct6694->cmd_header; >>> + struct nct6694_response_header *response_header = nct6694->response_header; >> >> RCT violation > > This code is not under net not drivers/net > As a general rule please focus on functional review, formatting and > process issues are harder to judge unless you read all of the mailing > list traffic. Okay, my bad. Thanks Kuba for explanation and I will focus on code next time
Le 10/12/2024 à 11:45, Ming Yu a écrit : > 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 implements USB device functionality and shares the > chip's peripherals as a child device. > > Each child device 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. ... > +int nct6694_read_msg(struct nct6694 *nct6694, u8 mod, u16 offset, > + u16 length, void *buf) > +{ > + struct nct6694_cmd_header *cmd_header = nct6694->cmd_header; > + struct nct6694_response_header *response_header = nct6694->response_header; > + struct usb_device *udev = nct6694->udev; > + int tx_len, rx_len, ret; > + > + guard(mutex)(&nct6694->access_lock); Nitpick: This could be moved a few lines below, should it still comply with your coding style. > + > + /* Send command packet to USB device */ > + cmd_header->mod = mod; > + cmd_header->cmd = offset & 0xFF; > + cmd_header->sel = (offset >> 8) & 0xFF; > + cmd_header->hctrl = NCT6694_HCTRL_GET; > + cmd_header->len = length; > + > + ret = usb_bulk_msg(udev, usb_sndbulkpipe(udev, NCT6694_BULK_OUT_EP), > + cmd_header, NCT6694_CMD_PACKET_SZ, &tx_len, > + nct6694->timeout); > + if (ret) > + return ret; > + > + /* Receive response packet from USB device */ > + ret = usb_bulk_msg(udev, usb_rcvbulkpipe(udev, NCT6694_BULK_IN_EP), > + response_header, NCT6694_CMD_PACKET_SZ, &rx_len, > + nct6694->timeout); > + if (ret) > + return ret; > + > + ret = usb_bulk_msg(udev, usb_rcvbulkpipe(udev, NCT6694_BULK_IN_EP), > + buf, NCT6694_MAX_PACKET_SZ, &rx_len, nct6694->timeout); > + if (ret) > + return ret; > + > + return nct6694_response_err_handling(nct6694, response_header->sts); > +} > +EXPORT_SYMBOL(nct6694_read_msg); > + > +int nct6694_write_msg(struct nct6694 *nct6694, u8 mod, u16 offset, > + u16 length, void *buf) > +{ > + struct nct6694_cmd_header *cmd_header = nct6694->cmd_header; > + struct nct6694_response_header *response_header = nct6694->response_header; > + struct usb_device *udev = nct6694->udev; > + int tx_len, rx_len, ret; > + > + guard(mutex)(&nct6694->access_lock); Nitpick: This could be moved a few lines below, should it still comply with your coding style. > + > + /* Send command packet to USB device */ Nitpick: double space before */ > + cmd_header->mod = mod; > + cmd_header->cmd = offset & 0xFF; > + cmd_header->sel = (offset >> 8) & 0xFF; > + cmd_header->hctrl = NCT6694_HCTRL_SET; > + cmd_header->len = length; ... > +static struct irq_chip nct6694_irq_chip = { const? > + .name = "nct6694-irq", > + .flags = IRQCHIP_SKIP_SET_WAKE, > + .irq_bus_lock = nct6694_irq_lock, > + .irq_bus_sync_unlock = nct6694_irq_sync_unlock, > + .irq_enable = nct6694_irq_enable, > + .irq_disable = nct6694_irq_disable, > +}; ... > +static int nct6694_usb_probe(struct usb_interface *iface, > + const struct usb_device_id *id) > +{ > + struct usb_device *udev = interface_to_usbdev(iface); > + struct device *dev = &udev->dev; > + struct usb_host_interface *interface; > + struct usb_endpoint_descriptor *int_endpoint; > + struct nct6694 *nct6694; > + struct nct6694_cmd_header *cmd_header; > + struct nct6694_response_header *response_header; > + int pipe, maxp; > + int ret; > + > + interface = iface->cur_altsetting; > + > + int_endpoint = &interface->endpoint[0].desc; > + if (!usb_endpoint_is_int_in(int_endpoint)) > + return -ENODEV; > + > + nct6694 = devm_kzalloc(dev, sizeof(*nct6694), GFP_KERNEL); > + if (!nct6694) > + return -ENOMEM; > + > + pipe = usb_rcvintpipe(udev, NCT6694_INT_IN_EP); > + maxp = usb_maxpacket(udev, pipe); > + > + cmd_header = devm_kzalloc(dev, sizeof(*cmd_header), > + GFP_KERNEL); > + if (!cmd_header) > + return -ENOMEM; > + > + response_header = devm_kzalloc(dev, sizeof(*response_header), > + GFP_KERNEL); > + if (!response_header) > + return -ENOMEM; > + > + nct6694->int_buffer = devm_kcalloc(dev, NCT6694_MAX_PACKET_SZ, > + sizeof(unsigned char), GFP_KERNEL); Why for cmd_header and response_header we use a temp variable, while here we update directly nct6694->int_buffer? It would save a few LoC do remove this temp var. > + if (!nct6694->int_buffer) > + return -ENOMEM; > + > + nct6694->int_in_urb = usb_alloc_urb(0, GFP_KERNEL); > + if (!nct6694->int_in_urb) > + return -ENOMEM; ... CJ
Dear Krzysztof, Thank you for your comments, Krzysztof Kozlowski <krzk@kernel.org> 於 2024年12月10日 週二 下午10:38寫道： > > > + > > + dev_set_drvdata(dev, nct6694); > > + usb_set_intfdata(iface, nct6694); > > + > > + ret = mfd_add_hotplug_devices(dev, nct6694_dev, ARRAY_SIZE(nct6694_dev)); > > + if (ret) > > + goto err_mfd; > > + > > + dev_info(dev, "Probed device: (%04X:%04X)\n", id->idVendor, id->idProduct); > > Drop. Duplicating existing messages and interfaces. Your driver is > supposed to be silent on success. > Okay, I will drop it in v4. > > + return 0; > > + > > +err_mfd: > > + usb_kill_urb(nct6694->int_in_urb); > > +err_urb: > > + usb_free_urb(nct6694->int_in_urb); > > + return dev_err_probe(dev, ret, "Probe failed\n"); > > No, this should go to individual call causing errors so this will be > informative. Above is not informative at all and kernel already reports > this, so drop. > Okay, I will drop it in v4. > > +} > > + > > +static void nct6694_usb_disconnect(struct usb_interface *iface) > > +{ > > + struct usb_device *udev = interface_to_usbdev(iface); > > + struct nct6694 *nct6694 = usb_get_intfdata(iface); > Best regards, Ming
Dear Christophe, Thank you for your comments, Christophe JAILLET <christophe.jaillet@wanadoo.fr> 於 2024年12月12日 週四 上午12:44寫道： > > > +int nct6694_read_msg(struct nct6694 *nct6694, u8 mod, u16 offset, > > + u16 length, void *buf) > > +{ > > + struct nct6694_cmd_header *cmd_header = nct6694->cmd_header; > > + struct nct6694_response_header *response_header = nct6694->response_header; > > + struct usb_device *udev = nct6694->udev; > > + int tx_len, rx_len, ret; > > + > > + guard(mutex)(&nct6694->access_lock); > > Nitpick: This could be moved a few lines below, should it still comply > with your coding style. > I think the lock should be placed here to prevent the cmd_header from being overwritten by another caller. Could you share your perspective on this? > > + > > + /* Send command packet to USB device */ > > + cmd_header->mod = mod; > > + cmd_header->cmd = offset & 0xFF; > > + cmd_header->sel = (offset >> 8) & 0xFF; > > + cmd_header->hctrl = NCT6694_HCTRL_GET; > > + cmd_header->len = length; > > + > > + ret = usb_bulk_msg(udev, usb_sndbulkpipe(udev, NCT6694_BULK_OUT_EP), > > + cmd_header, NCT6694_CMD_PACKET_SZ, &tx_len, > > + nct6694->timeout); > > + if (ret) > > + return ret; > > + > > + /* Receive response packet from USB device */ > > + ret = usb_bulk_msg(udev, usb_rcvbulkpipe(udev, NCT6694_BULK_IN_EP), > > + response_header, NCT6694_CMD_PACKET_SZ, &rx_len, > > + nct6694->timeout); > > + if (ret) > > + return ret; > > + > > + ret = usb_bulk_msg(udev, usb_rcvbulkpipe(udev, NCT6694_BULK_IN_EP), > > + buf, NCT6694_MAX_PACKET_SZ, &rx_len, nct6694->timeout); > > + if (ret) > > + return ret; > > + > > + return nct6694_response_err_handling(nct6694, response_header->sts); > > +} > > +EXPORT_SYMBOL(nct6694_read_msg); > > + > > +int nct6694_write_msg(struct nct6694 *nct6694, u8 mod, u16 offset, > > + u16 length, void *buf) > > +{ > > + struct nct6694_cmd_header *cmd_header = nct6694->cmd_header; > > + struct nct6694_response_header *response_header = nct6694->response_header; > > + struct usb_device *udev = nct6694->udev; > > + int tx_len, rx_len, ret; > > + > > + guard(mutex)(&nct6694->access_lock); > > Nitpick: This could be moved a few lines below, should it still comply > with your coding style. > I think the lock should be placed here to prevent the cmd_header from being overwritten by another caller. Could you share your perspective on this? > > + > > + /* Send command packet to USB device */ > > Nitpick: double space before */ > Fix it in v4. > > + cmd_header->mod = mod; > > + cmd_header->cmd = offset & 0xFF; > > + cmd_header->sel = (offset >> 8) & 0xFF; > > + cmd_header->hctrl = NCT6694_HCTRL_SET; > > + cmd_header->len = length; > > ... > > > +static struct irq_chip nct6694_irq_chip = { > > const? > Fix it in v4. > > + .name = "nct6694-irq", > > + .flags = IRQCHIP_SKIP_SET_WAKE, > > + .irq_bus_lock = nct6694_irq_lock, > > + .irq_bus_sync_unlock = nct6694_irq_sync_unlock, > > + .irq_enable = nct6694_irq_enable, > > + .irq_disable = nct6694_irq_disable, > > +}; > > ... > > > +static int nct6694_usb_probe(struct usb_interface *iface, > > + const struct usb_device_id *id) > > +{ > > + struct usb_device *udev = interface_to_usbdev(iface); > > + struct device *dev = &udev->dev; > > + struct usb_host_interface *interface; > > + struct usb_endpoint_descriptor *int_endpoint; > > + struct nct6694 *nct6694; > > + struct nct6694_cmd_header *cmd_header; > > + struct nct6694_response_header *response_header; > > + int pipe, maxp; > > + int ret; > > + > > + interface = iface->cur_altsetting; > > + > > + int_endpoint = &interface->endpoint[0].desc; > > + if (!usb_endpoint_is_int_in(int_endpoint)) > > + return -ENODEV; > > + > > + nct6694 = devm_kzalloc(dev, sizeof(*nct6694), GFP_KERNEL); > > + if (!nct6694) > > + return -ENOMEM; > > + > > + pipe = usb_rcvintpipe(udev, NCT6694_INT_IN_EP); > > + maxp = usb_maxpacket(udev, pipe); > > + > > + cmd_header = devm_kzalloc(dev, sizeof(*cmd_header), > > + GFP_KERNEL); > > + if (!cmd_header) > > + return -ENOMEM; > > + > > + response_header = devm_kzalloc(dev, sizeof(*response_header), > > + GFP_KERNEL); > > + if (!response_header) > > + return -ENOMEM; > > + > > + nct6694->int_buffer = devm_kcalloc(dev, NCT6694_MAX_PACKET_SZ, > > + sizeof(unsigned char), GFP_KERNEL); > > Why for cmd_header and response_header we use a temp variable, while > here we update directly nct6694->int_buffer? > > It would save a few LoC do remove this temp var. > Fix it in v4. > > + if (!nct6694->int_buffer) > > + return -ENOMEM; > > + > > + nct6694->int_in_urb = usb_alloc_urb(0, GFP_KERNEL); > > + if (!nct6694->int_in_urb) > > + return -ENOMEM; > > ... > Best regards, Ming
Le 12/12/2024 à 08:01, Ming Yu a écrit : > Dear Christophe, > > Thank you for your comments, > > Christophe JAILLET <christophe.jaillet-39ZsbGIQGT5GWvitb5QawA@public.gmane.org> 於 2024年12月12日 週四 上午12:44寫道： >> >>> +int nct6694_read_msg(struct nct6694 *nct6694, u8 mod, u16 offset, >>> + u16 length, void *buf) >>> +{ >>> + struct nct6694_cmd_header *cmd_header = nct6694->cmd_header; >>> + struct nct6694_response_header *response_header = nct6694->response_header; >>> + struct usb_device *udev = nct6694->udev; >>> + int tx_len, rx_len, ret; >>> + >>> + guard(mutex)(&nct6694->access_lock); >> >> Nitpick: This could be moved a few lines below, should it still comply >> with your coding style. >> > > I think the lock should be placed here to prevent the cmd_header from > being overwritten by another caller. > Could you share your perspective on this? You are right, I misread the code :( (I though cmd_header was a local structure) > >>> + >>> + /* Send command packet to USB device */ >>> + cmd_header->mod = mod; >>> + cmd_header->cmd = offset & 0xFF; >>> + cmd_header->sel = (offset >> 8) & 0xFF; >>> + cmd_header->hctrl = NCT6694_HCTRL_GET; >>> + cmd_header->len = length; CJ
Hi Ming, thanks for your patch! On Tue, Dec 10, 2024 at 11:45 AM Ming Yu <a0282524688@gmail.com> wrote: > 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 implements USB device functionality and shares the > chip's peripherals as a child device. > > Each child device 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> (...) > + while (*int_status) { > + int irq = __ffs(*int_status); > + > + if (*int_status & (1 << irq)) > + generic_handle_irq_safe(irq_find_mapping(nct6694->domain, irq)); > + > + *int_status &= ~(1 << irq); What about doing what you do in the GPIO driver and #include <linux/bits.h> And search and replace "(1 << irq)" with BIT(irq)? PS the main reason we do this is because int a = (1 << 31); becomes a negative number on 32bit machines, and can lead to confusing side effects. BIT() always work on unsigned. Yours, Linus Walleij
Dear Linus, Thank you for your reply, Linus Walleij <linus.walleij@linaro.org> 於 2024年12月20日 週五 下午8:45寫道： > > Hi Ming, > > thanks for your patch! > ... > > + while (*int_status) { > > + int irq = __ffs(*int_status); > > + > > + if (*int_status & (1 << irq)) > > + generic_handle_irq_safe(irq_find_mapping(nct6694->domain, irq)); > > + > > + *int_status &= ~(1 << irq); > > What about doing what you do in the GPIO driver and > #include <linux/bits.h> > > And search and replace "(1 << irq)" with BIT(irq)? > > PS the main reason we do this is because > > int a = (1 << 31); > > becomes a negative number on 32bit machines, and > can lead to confusing side effects. BIT() always work > on unsigned. > Understood! I will make the modifications in the v4. Best regards, Ming
diff --git a/MAINTAINERS b/MAINTAINERS index 67d2159406c2..9c9a94ff8f98 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -16541,6 +16541,13 @@ F: drivers/nubus/ F: include/linux/nubus.h F: include/uapi/linux/nubus.h +NUVOTON NCT6694 MFD DRIVER +M: Ming Yu <tmyu0@nuvoton.com> +L: linux-kernel@vger.kernel.org +S: Supported +F: drivers/mfd/nct6694.c +F: include/linux/mfd/nct6694.h + NVIDIA (rivafb and nvidiafb) FRAMEBUFFER DRIVER M: Antonino Daplas <adaplas@gmail.com> L: linux-fbdev@vger.kernel.org diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index 03c1e4e3eea4..07ccc32f9985 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -558,6 +558,16 @@ config MFD_MX25_TSADC i.MX25 processors. They consist of a conversion queue for general purpose ADC and a queue for Touchscreens. +config MFD_NCT6694 + tristate "Nuvoton NCT6694 support" + select MFD_CORE + depends on USB + help + This adds support for Nuvoton USB device NCT6694 sharing peripherals + This includes the USB devcie driver and core APIs. + Additional drivers must be enabled in order to use the functionality + of the device. + config MFD_HI6421_PMIC tristate "HiSilicon Hi6421 PMU/Codec IC" depends on OF diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index e057d6d6faef..9d0365ba6a26 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -117,6 +117,8 @@ obj-$(CONFIG_TWL6040_CORE) += twl6040.o obj-$(CONFIG_MFD_MX25_TSADC) += fsl-imx25-tsadc.o +obj-$(CONFIG_MFD_NCT6694) += nct6694.o + obj-$(CONFIG_MFD_MC13XXX) += mc13xxx-core.o obj-$(CONFIG_MFD_MC13XXX_SPI) += mc13xxx-spi.o obj-$(CONFIG_MFD_MC13XXX_I2C) += mc13xxx-i2c.o diff --git a/drivers/mfd/nct6694.c b/drivers/mfd/nct6694.c new file mode 100644 index 000000000000..071333d13eae --- /dev/null +++ b/drivers/mfd/nct6694.c @@ -0,0 +1,382 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Nuvoton NCT6694 MFD driver based on USB interface. + * + * Copyright (C) 2024 Nuvoton Technology Corp. + */ + +#include <linux/interrupt.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/slab.h> +#include <linux/usb.h> + +#define MFD_DEV_SIMPLE(_name) \ +{ \ + .name = NCT6694_DEV_##_name, \ +} \ + +#define MFD_DEV_WITH_ID(_name, _id) \ +{ \ + .name = NCT6694_DEV_##_name, \ + .id = _id, \ +} + +/* MFD device resources */ +static const struct mfd_cell nct6694_dev[] = { + MFD_DEV_WITH_ID(GPIO, 0x0), + MFD_DEV_WITH_ID(GPIO, 0x1), + MFD_DEV_WITH_ID(GPIO, 0x2), + MFD_DEV_WITH_ID(GPIO, 0x3), + MFD_DEV_WITH_ID(GPIO, 0x4), + MFD_DEV_WITH_ID(GPIO, 0x5), + MFD_DEV_WITH_ID(GPIO, 0x6), + MFD_DEV_WITH_ID(GPIO, 0x7), + MFD_DEV_WITH_ID(GPIO, 0x8), + MFD_DEV_WITH_ID(GPIO, 0x9), + MFD_DEV_WITH_ID(GPIO, 0xA), + MFD_DEV_WITH_ID(GPIO, 0xB), + MFD_DEV_WITH_ID(GPIO, 0xC), + MFD_DEV_WITH_ID(GPIO, 0xD), + MFD_DEV_WITH_ID(GPIO, 0xE), + MFD_DEV_WITH_ID(GPIO, 0xF), + + MFD_DEV_WITH_ID(I2C, 0x0), + MFD_DEV_WITH_ID(I2C, 0x1), + MFD_DEV_WITH_ID(I2C, 0x2), + MFD_DEV_WITH_ID(I2C, 0x3), + MFD_DEV_WITH_ID(I2C, 0x4), + MFD_DEV_WITH_ID(I2C, 0x5), + + MFD_DEV_WITH_ID(CAN, 0x0), + MFD_DEV_WITH_ID(CAN, 0x1), + + MFD_DEV_WITH_ID(WDT, 0x0), + MFD_DEV_WITH_ID(WDT, 0x1), + + MFD_DEV_SIMPLE(HWMON), + MFD_DEV_SIMPLE(RTC), +}; + +static int nct6694_response_err_handling(struct nct6694 *nct6694, + unsigned char err_status) +{ + struct device *dev = &nct6694->udev->dev; + + switch (err_status) { + case NCT6694_NO_ERROR: + return err_status; + case NCT6694_NOT_SUPPORT_ERROR: + dev_dbg(dev, "%s: Command is not support!\n", __func__); + break; + case NCT6694_NO_RESPONSE_ERROR: + dev_dbg(dev, "%s: Command is no response!\n", __func__); + break; + case NCT6694_TIMEOUT_ERROR: + dev_dbg(dev, "%s: Command is timeout!\n", __func__); + break; + case NCT6694_PENDING: + dev_dbg(dev, "%s: Command is pending!\n", __func__); + break; + default: + return -EINVAL; + } + + return -EIO; +} + +int nct6694_read_msg(struct nct6694 *nct6694, u8 mod, u16 offset, + u16 length, void *buf) +{ + struct nct6694_cmd_header *cmd_header = nct6694->cmd_header; + struct nct6694_response_header *response_header = nct6694->response_header; + struct usb_device *udev = nct6694->udev; + int tx_len, rx_len, ret; + + guard(mutex)(&nct6694->access_lock); + + /* Send command packet to USB device */ + cmd_header->mod = mod; + cmd_header->cmd = offset & 0xFF; + cmd_header->sel = (offset >> 8) & 0xFF; + cmd_header->hctrl = NCT6694_HCTRL_GET; + cmd_header->len = length; + + ret = usb_bulk_msg(udev, usb_sndbulkpipe(udev, NCT6694_BULK_OUT_EP), + cmd_header, NCT6694_CMD_PACKET_SZ, &tx_len, + nct6694->timeout); + if (ret) + return ret; + + /* Receive response packet from USB device */ + ret = usb_bulk_msg(udev, usb_rcvbulkpipe(udev, NCT6694_BULK_IN_EP), + response_header, NCT6694_CMD_PACKET_SZ, &rx_len, + nct6694->timeout); + if (ret) + return ret; + + ret = usb_bulk_msg(udev, usb_rcvbulkpipe(udev, NCT6694_BULK_IN_EP), + buf, NCT6694_MAX_PACKET_SZ, &rx_len, nct6694->timeout); + if (ret) + return ret; + + return nct6694_response_err_handling(nct6694, response_header->sts); +} +EXPORT_SYMBOL(nct6694_read_msg); + +int nct6694_write_msg(struct nct6694 *nct6694, u8 mod, u16 offset, + u16 length, void *buf) +{ + struct nct6694_cmd_header *cmd_header = nct6694->cmd_header; + struct nct6694_response_header *response_header = nct6694->response_header; + struct usb_device *udev = nct6694->udev; + int tx_len, rx_len, ret; + + guard(mutex)(&nct6694->access_lock); + + /* Send command packet to USB device */ + cmd_header->mod = mod; + cmd_header->cmd = offset & 0xFF; + cmd_header->sel = (offset >> 8) & 0xFF; + cmd_header->hctrl = NCT6694_HCTRL_SET; + cmd_header->len = length; + + ret = usb_bulk_msg(udev, usb_sndbulkpipe(udev, NCT6694_BULK_OUT_EP), + cmd_header, NCT6694_CMD_PACKET_SZ, &tx_len, + nct6694->timeout); + if (ret) + return ret; + + ret = usb_bulk_msg(udev, usb_sndbulkpipe(udev, NCT6694_BULK_OUT_EP), + buf, length, &tx_len, nct6694->timeout); + if (ret) + return ret; + + /* Receive response packet from USB device */ + ret = usb_bulk_msg(udev, usb_rcvbulkpipe(udev, NCT6694_BULK_IN_EP), + response_header, NCT6694_CMD_PACKET_SZ, &rx_len, + nct6694->timeout); + if (ret) + return ret; + + ret = usb_bulk_msg(udev, usb_rcvbulkpipe(udev, NCT6694_BULK_IN_EP), + buf, NCT6694_MAX_PACKET_SZ, &rx_len, nct6694->timeout); + if (ret) + return ret; + + return nct6694_response_err_handling(nct6694, response_header->sts); +} +EXPORT_SYMBOL(nct6694_write_msg); + +static void usb_int_callback(struct urb *urb) +{ + struct nct6694 *nct6694 = urb->context; + struct device *dev = &nct6694->udev->dev; + unsigned int *int_status = urb->transfer_buffer; + int ret; + + switch (urb->status) { + case 0: + break; + case -ECONNRESET: + case -ENOENT: + case -ESHUTDOWN: + return; + default: + goto resubmit; + } + + while (*int_status) { + int irq = __ffs(*int_status); + + if (*int_status & (1 << irq)) + generic_handle_irq_safe(irq_find_mapping(nct6694->domain, irq)); + + *int_status &= ~(1 << irq); + } + +resubmit: + ret = usb_submit_urb(urb, GFP_ATOMIC); + if (ret) + dev_dbg(dev, "%s: Failed to resubmit urb, status %d", + __func__, ret); +} + +static void nct6694_irq_lock(struct irq_data *data) +{ + struct nct6694 *nct6694 = irq_data_get_irq_chip_data(data); + + mutex_lock(&nct6694->irq_lock); +} + +static void nct6694_irq_sync_unlock(struct irq_data *data) +{ + struct nct6694 *nct6694 = irq_data_get_irq_chip_data(data); + + mutex_unlock(&nct6694->irq_lock); +} + +static void nct6694_irq_enable(struct irq_data *data) +{ + struct nct6694 *nct6694 = irq_data_get_irq_chip_data(data); + unsigned int irq = data->hwirq; + + nct6694->irq_enable |= (1 << irq); +} + +static void nct6694_irq_disable(struct irq_data *data) +{ + struct nct6694 *nct6694 = irq_data_get_irq_chip_data(data); + unsigned int irq = data->hwirq; + + nct6694->irq_enable &= ~(1 << irq); +} + +static struct irq_chip nct6694_irq_chip = { + .name = "nct6694-irq", + .flags = IRQCHIP_SKIP_SET_WAKE, + .irq_bus_lock = nct6694_irq_lock, + .irq_bus_sync_unlock = nct6694_irq_sync_unlock, + .irq_enable = nct6694_irq_enable, + .irq_disable = nct6694_irq_disable, +}; + +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 int nct6694_usb_probe(struct usb_interface *iface, + const struct usb_device_id *id) +{ + struct usb_device *udev = interface_to_usbdev(iface); + struct device *dev = &udev->dev; + struct usb_host_interface *interface; + struct usb_endpoint_descriptor *int_endpoint; + struct nct6694 *nct6694; + struct nct6694_cmd_header *cmd_header; + struct nct6694_response_header *response_header; + int pipe, maxp; + int ret; + + interface = iface->cur_altsetting; + + int_endpoint = &interface->endpoint[0].desc; + if (!usb_endpoint_is_int_in(int_endpoint)) + return -ENODEV; + + nct6694 = devm_kzalloc(dev, sizeof(*nct6694), GFP_KERNEL); + if (!nct6694) + return -ENOMEM; + + pipe = usb_rcvintpipe(udev, NCT6694_INT_IN_EP); + maxp = usb_maxpacket(udev, pipe); + + cmd_header = devm_kzalloc(dev, sizeof(*cmd_header), + GFP_KERNEL); + if (!cmd_header) + return -ENOMEM; + + response_header = devm_kzalloc(dev, sizeof(*response_header), + GFP_KERNEL); + if (!response_header) + return -ENOMEM; + + nct6694->int_buffer = devm_kcalloc(dev, NCT6694_MAX_PACKET_SZ, + sizeof(unsigned char), GFP_KERNEL); + if (!nct6694->int_buffer) + return -ENOMEM; + + nct6694->int_in_urb = usb_alloc_urb(0, GFP_KERNEL); + if (!nct6694->int_in_urb) + return -ENOMEM; + + nct6694->domain = irq_domain_add_simple(NULL, NCT6694_NR_IRQS, 0, + &nct6694_irq_domain_ops, + nct6694); + if (!nct6694->domain) + return -ENODEV; + + nct6694->udev = udev; + nct6694->timeout = NCT6694_URB_TIMEOUT; /* Wait until urb complete */ + nct6694->cmd_header = cmd_header; + nct6694->response_header = response_header; + + mutex_init(&nct6694->access_lock); + mutex_init(&nct6694->irq_lock); + + usb_fill_int_urb(nct6694->int_in_urb, udev, pipe, + nct6694->int_buffer, maxp, usb_int_callback, + nct6694, int_endpoint->bInterval); + ret = usb_submit_urb(nct6694->int_in_urb, GFP_KERNEL); + if (ret) + goto err_urb; + + dev_set_drvdata(dev, nct6694); + usb_set_intfdata(iface, nct6694); + + ret = mfd_add_hotplug_devices(dev, nct6694_dev, ARRAY_SIZE(nct6694_dev)); + if (ret) + goto err_mfd; + + dev_info(dev, "Probed device: (%04X:%04X)\n", id->idVendor, id->idProduct); + return 0; + +err_mfd: + usb_kill_urb(nct6694->int_in_urb); +err_urb: + usb_free_urb(nct6694->int_in_urb); + return dev_err_probe(dev, ret, "Probe failed\n"); +} + +static void nct6694_usb_disconnect(struct usb_interface *iface) +{ + struct usb_device *udev = interface_to_usbdev(iface); + struct nct6694 *nct6694 = usb_get_intfdata(iface); + + mfd_remove_devices(&udev->dev); + usb_kill_urb(nct6694->int_in_urb); + usb_free_urb(nct6694->int_in_urb); +} + +static const struct usb_device_id nct6694_ids[] = { + { USB_DEVICE_AND_INTERFACE_INFO(NCT6694_VENDOR_ID, + NCT6694_PRODUCT_ID, + 0xFF, 0x00, 0x00)}, + {} +}; +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("USB-MFD driver for NCT6694"); +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..9733d47f5a01 --- /dev/null +++ b/include/linux/mfd/nct6694.h @@ -0,0 +1,139 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Nuvoton NCT6694 USB transaction and data structure. + * + * Copyright (C) 2024 Nuvoton Technology Corp. + */ + +#ifndef __MFD_NCT6694_H +#define __MFD_NCT6694_H + +#define NCT6694_DEV_GPIO "nct6694-gpio" +#define NCT6694_DEV_I2C "nct6694-i2c" +#define NCT6694_DEV_CAN "nct6694-can" +#define NCT6694_DEV_WDT "nct6694-wdt" +#define NCT6694_DEV_HWMON "nct6694-hwmon" +#define NCT6694_DEV_RTC "nct6694-rtc" + +#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_MAX_PACKET_SZ 0x200 + +#define NCT6694_CMD_PACKET_SZ 0x8 +#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_CAN1, + NCT6694_IRQ_CAN2, + 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 nct6694 { + struct usb_device *udev; + struct urb *int_in_urb; + struct irq_domain *domain; + struct nct6694_cmd_header *cmd_header; + struct nct6694_response_header *response_header; + struct mutex access_lock; + struct mutex irq_lock; + unsigned char *int_buffer; + unsigned int irq_enable; + /* time in msec to wait for the urb to the complete */ + long timeout; +}; + +struct nct6694_cmd_header { + unsigned char rsv1; + unsigned char mod; + unsigned char cmd; + unsigned char sel; + unsigned char hctrl; + unsigned char rsv2; + unsigned short len; +} __packed; + +struct nct6694_response_header { + unsigned char sequence_id; + unsigned char sts; + unsigned int rsv; + unsigned short len; +} __packed; + +/* + * nct6694_read_msg - Receive data from NCT6694 USB device + * + * @nct6694 - Nuvoton NCT6694 structure + * @mod - Module byte + * @offset - Offset byte or (Select byte | Command byte) + * @length - Length byte + * @buf - Read data from rx buffer + * + * USB Transaction format: + * + * OUT |RSV|MOD|CMD|SEL|HCTL|RSV|LEN_L|LEN_H| + * OUT |SEQ|STS|RSV|RSV|RSV|RSV||LEN_L|LEN_H| + * IN |-------D------A------D------A-------| + * IN ...... + * IN |-------D------A------D------A-------| + */ +int nct6694_read_msg(struct nct6694 *nct6694, u8 mod, u16 offset, + u16 length, void *buf); + +/* + * nct6694_read_msg - Transmit data to NCT6694 USB device + * + * @nct6694 - Nuvoton NCT6694 structure + * @mod - Module byte + * @offset - Offset byte or (Select byte | Command byte) + * @length - Length byte + * @buf - Write data to tx buffer + * + * USB Transaction format: + * + * OUT |RSV|MOD|CMD|SEL|HCTL|RSV|LEN_L|LEN_H| + * OUT |-------D------A------D------A-------| + * OUT ...... + * OUT |-------D------A------D------A-------| + * IN |SEQ|STS|RSV|RSV|RSV|RSV||LEN_L|LEN_H| + * IN |-------D------A------D------A-------| + * IN ...... + * IN |-------D------A------D------A-------| + */ +int nct6694_write_msg(struct nct6694 *nct6694, u8 mod, u16 offset, + u16 length, void *buf); + +#endif
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 implements USB device functionality and shares the chip's peripherals as a child device. Each child device 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> --- MAINTAINERS | 7 + drivers/mfd/Kconfig | 10 + drivers/mfd/Makefile | 2 + drivers/mfd/nct6694.c | 382 ++++++++++++++++++++++++++++++++++++ include/linux/mfd/nct6694.h | 139 +++++++++++++ 5 files changed, 540 insertions(+) create mode 100644 drivers/mfd/nct6694.c create mode 100644 include/linux/mfd/nct6694.h