[PATCH v9 0/4] Add Intel LJCA device driver

[PATCH v9 0/4] Add Intel LJCA device driver

[Wentong Wu]

Add driver for Intel La Jolla Cove Adapter (LJCA) device.
This is a USB-GPIO, USB-I2C and USB-SPI device. We add 4
drivers to support this device: a USB driver, a GPIO chip
driver, a I2C controller driver and a SPI controller driver.

---
v9:
 - overhaul usb-ljca driver to make it more structured and easy understanding
 - fix memory leak issue for usb-ljca driver
 - add spinlock to protect tx_buf and ex_buf
 - change exported APIs for usb-ljca driver
 - unify prefix for structures and functions for i2c-ljca driver
 - unify prefix for structures and functions for spi-ljca driver
 - unify prefix for structures and functions for gpio-ljca driver
 - update gpio-ljca, i2c-ljca and spi-ljca drivers according to usb-ljca's changes

Wentong Wu (4):
  usb: Add support for Intel LJCA device
  i2c: Add support for Intel LJCA USB I2C driver
  spi: Add support for Intel LJCA USB SPI driver
  gpio: update Intel LJCA USB GPIO driver

 drivers/gpio/Kconfig          |   4 +-
 drivers/gpio/gpio-ljca.c      | 242 +++++++------
 drivers/i2c/busses/Kconfig    |  11 +
 drivers/i2c/busses/Makefile   |   1 +
 drivers/i2c/busses/i2c-ljca.c | 353 ++++++++++++++++++
 drivers/spi/Kconfig           |  11 +
 drivers/spi/Makefile          |   1 +
 drivers/spi/spi-ljca.c        | 299 ++++++++++++++++
 drivers/usb/misc/Kconfig      |  14 +
 drivers/usb/misc/Makefile     |   1 +
 drivers/usb/misc/usb-ljca.c   | 817 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/ljca.h      | 113 ++++++
 12 files changed, 1764 insertions(+), 103 deletions(-)
 create mode 100644 drivers/i2c/busses/i2c-ljca.c
 create mode 100644 drivers/spi/spi-ljca.c
 create mode 100644 drivers/usb/misc/usb-ljca.c
 create mode 100644 include/linux/usb/ljca.h

-- 
2.7.4

[PATCH v9 1/4] usb: Add support for Intel LJCA device

[Wentong Wu]

Implements the USB part of Intel USB-I2C/GPIO/SPI adapter device
named "La Jolla Cove Adapter" (LJCA).

The communication between the various LJCA module drivers and the
hardware will be muxed/demuxed by this driver. Three modules (
I2C, GPIO, and SPI) are supported currently.

Each sub-module of LJCA device is identified by type field within
the LJCA message header.

The sub-modules of LJCA can use ljca_transfer() to issue a transfer
between host and hardware. And ljca_register_event_cb is exported
to LJCA sub-module drivers for hardware event subscription.

The minimum code in ASL that covers this board is
Scope (\_SB.PCI0.DWC3.RHUB.HS01)
    {
        Device (GPIO)
        {
            Name (_ADR, Zero)
            Name (_STA, 0x0F)
        }

        Device (I2C)
        {
            Name (_ADR, One)
            Name (_STA, 0x0F)
        }

        Device (SPI)
        {
            Name (_ADR, 0x02)
            Name (_STA, 0x0F)
        }
    }

Signed-off-by: Wentong Wu <wentong.wu@intel.com>
Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/usb/misc/Kconfig    |  14 +
 drivers/usb/misc/Makefile   |   1 +
 drivers/usb/misc/usb-ljca.c | 817 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/ljca.h    | 113 ++++++
 4 files changed, 945 insertions(+)
 create mode 100644 drivers/usb/misc/usb-ljca.c
 create mode 100644 include/linux/usb/ljca.h

diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
index 99b15b7..999193e 100644
--- a/drivers/usb/misc/Kconfig
+++ b/drivers/usb/misc/Kconfig
@@ -165,6 +165,20 @@ config APPLE_MFI_FASTCHARGE
 
 	  It is safe to say M here.
 
+config USB_LJCA
+	tristate "Intel La Jolla Cove Adapter support"
+	select AUXILIARY_BUS
+	depends on USB
+	depends on ACPI || COMPILE_TEST
+	help
+	  This adds support for Intel La Jolla Cove USB-I2C/SPI/GPIO
+	  Master Adapter (LJCA). Additional drivers such as I2C_LJCA,
+	  GPIO_LJCA and SPI_LJCA must be enabled in order to use the
+	  functionality of the device.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called usb-ljca.
+
 source "drivers/usb/misc/sisusbvga/Kconfig"
 
 config USB_LD
diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
index 1992cc2..0bc732bc 100644
--- a/drivers/usb/misc/Makefile
+++ b/drivers/usb/misc/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_USB_EMI26)			+= emi26.o
 obj-$(CONFIG_USB_EMI62)			+= emi62.o
 obj-$(CONFIG_USB_EZUSB_FX2)		+= ezusb.o
 obj-$(CONFIG_APPLE_MFI_FASTCHARGE)	+= apple-mfi-fastcharge.o
+obj-$(CONFIG_USB_LJCA)			+= usb-ljca.o
 obj-$(CONFIG_USB_IDMOUSE)		+= idmouse.o
 obj-$(CONFIG_USB_IOWARRIOR)		+= iowarrior.o
 obj-$(CONFIG_USB_ISIGHTFW)		+= isight_firmware.o
diff --git a/drivers/usb/misc/usb-ljca.c b/drivers/usb/misc/usb-ljca.c
new file mode 100644
index 0000000..8e024a3
--- /dev/null
+++ b/drivers/usb/misc/usb-ljca.c
@@ -0,0 +1,817 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Intel La Jolla Cove Adapter USB driver
+ *
+ * Copyright (c) 2023, Intel Corporation.
+ */
+
+#include <linux/acpi.h>
+#include <linux/auxiliary_bus.h>
+#include <linux/dev_printk.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/usb.h>
+#include <linux/usb/ljca.h>
+
+/* command flags */
+#define LJCA_ACK_FLAG			BIT(0)
+#define LJCA_RESP_FLAG			BIT(1)
+#define LJCA_CMPL_FLAG			BIT(2)
+
+#define LJCA_MAX_PACKET_SIZE		64u
+#define LJCA_MAX_PAYLOAD_SIZE		\
+		(LJCA_MAX_PACKET_SIZE - sizeof(struct ljca_msg))
+
+#define LJCA_WRITE_TIMEOUT_MS		200
+#define LJCA_WRITE_ACK_TIMEOUT_MS	500
+#define LJCA_ENUM_CLIENT_TIMEOUT_MS	20
+
+/* ljca client type */
+enum ljca_client_type {
+	LJCA_CLIENT_MNG = 1,
+	LJCA_CLIENT_GPIO = 3,
+	LJCA_CLIENT_I2C = 4,
+	LJCA_CLIENT_SPI = 5,
+};
+
+/* MNG client commands */
+enum ljca_mng_cmd {
+	LJCA_MNG_RESET = 2,
+	LJCA_MNG_ENUM_GPIO = 4,
+	LJCA_MNG_ENUM_I2C = 5,
+	LJCA_MNG_ENUM_SPI = 8,
+};
+
+/* ljca client acpi _ADR */
+enum ljca_client_acpi_adr {
+	LJCA_GPIO_ACPI_ADR,
+	LJCA_I2C1_ACPI_ADR,
+	LJCA_I2C2_ACPI_ADR,
+	LJCA_SPI1_ACPI_ADR,
+	LJCA_SPI2_ACPI_ADR,
+	LJCA_CLIENT_ACPI_ADR_MAX,
+};
+
+/* ljca cmd message structure */
+struct ljca_msg {
+	u8 type;
+	u8 cmd;
+	u8 flags;
+	u8 len;
+	u8 data[];
+} __packed;
+
+struct ljca_i2c_ctr_info {
+	u8 id;
+	u8 capacity;
+	u8 intr_pin;
+} __packed;
+
+struct ljca_i2c_descriptor {
+	u8 num;
+	struct ljca_i2c_ctr_info info[];
+} __packed;
+
+struct ljca_spi_ctr_info {
+	u8 id;
+	u8 capacity;
+} __packed;
+
+struct ljca_spi_descriptor {
+	u8 num;
+	struct ljca_spi_ctr_info info[];
+} __packed;
+
+struct ljca_bank_descriptor {
+	u8 bank_id;
+	u8 pin_num;
+
+	/* 1 bit for each gpio, 1 means valid */
+	u32 valid_pins;
+} __packed;
+
+struct ljca_gpio_descriptor {
+	u8 pins_per_bank;
+	u8 bank_num;
+	struct ljca_bank_descriptor bank_desc[];
+} __packed;
+
+struct ljca_adapter {
+	struct usb_interface *intf;
+	struct usb_device *usb_dev;
+	struct device *dev;
+
+	unsigned int rx_pipe;
+	unsigned int tx_pipe;
+
+	/* urb for recv */
+	struct urb *rx_urb;
+	/* buffer for recv */
+	void *rx_buf;
+	unsigned int rx_len;
+
+	/* external buffer for recv */
+	void *ex_buf;
+	unsigned int ex_buf_len;
+	/* actual data length copied to external buffer */
+	unsigned int actual_length;
+
+	/* buffer for send */
+	void *tx_buf;
+	unsigned int tx_buf_len;
+
+	/* lock to protect tx_buf and ex_buf */
+	spinlock_t lock;
+
+	struct completion cmd_completion;
+
+	/* mutex to protect command download */
+	struct mutex mutex;
+
+	/* client device list */
+	struct list_head client_list;
+
+	/* used to reset firmware */
+	u32 reset_id;
+};
+
+static void ljca_handle_event(struct ljca_adapter *adap,
+			      struct ljca_msg *header)
+{
+	struct ljca_client *client;
+
+	list_for_each_entry(client, &adap->client_list, link) {
+		/*
+		 * FIXME: currently only GPIO register event callback.
+		 * firmware message structure should include id when
+		 * multiple same type clients register event callback.
+		 */
+		if (client->type == header->type) {
+			unsigned long flags;
+
+			spin_lock_irqsave(&client->event_cb_lock, flags);
+			client->event_cb(client->context, header->cmd,
+					 header->data, header->len);
+			spin_unlock_irqrestore(&client->event_cb_lock, flags);
+
+			break;
+		}
+	}
+}
+
+/* process command ack */
+static void ljca_handle_cmd_ack(struct ljca_adapter *adap,
+				struct ljca_msg *header)
+{
+	struct ljca_msg *tx_header = adap->tx_buf;
+	unsigned int actual_len = 0;
+	unsigned int ibuf_len;
+	unsigned long flags;
+	void *ibuf;
+
+	spin_lock_irqsave(&adap->lock, flags);
+
+	if (tx_header->type != header->type || tx_header->cmd != header->cmd) {
+		spin_unlock_irqrestore(&adap->lock, flags);
+
+		return;
+	}
+
+	ibuf_len = adap->ex_buf_len;
+	ibuf = adap->ex_buf;
+
+	if (ibuf && ibuf_len) {
+		actual_len = min_t(unsigned int, header->len, ibuf_len);
+
+		/* copy received data to external buffer */
+		memcpy(ibuf, header->data, actual_len);
+	}
+	/* update copied data length */
+	adap->actual_length = actual_len;
+
+	spin_unlock_irqrestore(&adap->lock, flags);
+
+	complete(&adap->cmd_completion);
+}
+
+static void ljca_recv(struct urb *urb)
+{
+	struct ljca_msg *header = urb->transfer_buffer;
+	struct ljca_adapter *adap = urb->context;
+	int ret;
+
+	if (urb->status) {
+		/* sync/async unlink faults aren't errors */
+		if (urb->status == -ENOENT || urb->status == -ECONNRESET ||
+		    urb->status == -ESHUTDOWN)
+			return;
+
+		dev_err(adap->dev, "recv urb error: %d\n", urb->status);
+		goto resubmit;
+	}
+
+	if (header->len + sizeof(*header) != urb->actual_length)
+		goto resubmit;
+
+	if (header->flags & LJCA_ACK_FLAG)
+		ljca_handle_cmd_ack(adap, header);
+	else
+		ljca_handle_event(adap, header);
+
+resubmit:
+	ret = usb_submit_urb(urb, GFP_ATOMIC);
+	if (ret)
+		dev_err(adap->dev, "resubmit recv urb error %d\n", ret);
+}
+
+static int ljca_send(struct ljca_adapter *adap, u8 type, u8 cmd,
+		     const void *obuf, unsigned int obuf_len, void *ibuf,
+		     unsigned int ibuf_len, bool ack, unsigned long timeout)
+{
+	unsigned int msg_len = sizeof(struct ljca_msg) + obuf_len;
+	struct ljca_msg *header = adap->tx_buf;
+	unsigned long flags;
+	unsigned int actual;
+	int ret = 0;
+
+	if (msg_len > adap->tx_buf_len)
+		return -EINVAL;
+
+	mutex_lock(&adap->mutex);
+
+	spin_lock_irqsave(&adap->lock, flags);
+
+	header->type = type;
+	header->cmd = cmd;
+	header->len = obuf_len;
+	if (obuf)
+		memcpy(header->data, obuf, obuf_len);
+
+	header->flags = LJCA_CMPL_FLAG | (ack ? LJCA_ACK_FLAG : 0);
+
+	adap->ex_buf = ibuf;
+	adap->ex_buf_len = ibuf_len;
+	adap->actual_length = 0;
+
+	spin_unlock_irqrestore(&adap->lock, flags);
+
+	reinit_completion(&adap->cmd_completion);
+
+	usb_autopm_get_interface(adap->intf);
+
+	ret = usb_bulk_msg(adap->usb_dev, adap->tx_pipe, header,
+			   msg_len, &actual, LJCA_WRITE_TIMEOUT_MS);
+	if (!ret && ack) {
+		ret = wait_for_completion_timeout(&adap->cmd_completion,
+						  timeout);
+		if (ret < 0) {
+			goto out;
+		} if (!ret) {
+			ret = -ETIMEDOUT;
+			goto out;
+		}
+	}
+	ret = adap->actual_length;
+
+out:
+	spin_lock_irqsave(&adap->lock, flags);
+	adap->ex_buf = NULL;
+	adap->ex_buf_len = 0;
+
+	memset(header, 0, sizeof(*header));
+	spin_unlock_irqrestore(&adap->lock, flags);
+
+	usb_autopm_put_interface(adap->intf);
+
+	mutex_unlock(&adap->mutex);
+
+	return ret;
+}
+
+int ljca_transfer(struct ljca_client *client, u8 cmd, const void *obuf,
+		  unsigned int obuf_len, void *ibuf, unsigned int ibuf_len)
+{
+	return ljca_send(client->adapter, client->type, cmd,
+			 obuf, obuf_len, ibuf, ibuf_len, true,
+			 LJCA_WRITE_ACK_TIMEOUT_MS);
+}
+EXPORT_SYMBOL_NS_GPL(ljca_transfer, LJCA);
+
+int ljca_transfer_noack(struct ljca_client *client, u8 cmd, const void *obuf,
+			unsigned int obuf_len)
+{
+	return ljca_send(client->adapter, client->type, cmd, obuf,
+			 obuf_len, NULL, 0, false, LJCA_WRITE_ACK_TIMEOUT_MS);
+}
+EXPORT_SYMBOL_NS_GPL(ljca_transfer_noack, LJCA);
+
+int ljca_register_event_cb(struct ljca_client *client, ljca_event_cb_t event_cb,
+			   void *context)
+{
+	unsigned long flags;
+
+	if (!event_cb)
+		return -EINVAL;
+
+	spin_lock_irqsave(&client->event_cb_lock, flags);
+
+	if (client->event_cb) {
+		spin_unlock_irqrestore(&client->event_cb_lock, flags);
+		return -EALREADY;
+	}
+
+	client->event_cb = event_cb;
+	client->context = context;
+
+	spin_unlock_irqrestore(&client->event_cb_lock, flags);
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(ljca_register_event_cb, LJCA);
+
+void ljca_unregister_event_cb(struct ljca_client *client)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&client->event_cb_lock, flags);
+
+	client->event_cb = NULL;
+	client->context = NULL;
+
+	spin_unlock_irqrestore(&client->event_cb_lock, flags);
+}
+EXPORT_SYMBOL_NS_GPL(ljca_unregister_event_cb, LJCA);
+
+#ifdef CONFIG_ACPI
+struct match_ids_walk_data {
+	const struct acpi_device_id *ids;
+	struct acpi_device *adev;
+};
+
+static const struct acpi_device_id gpio_hids[] = {
+	{ "INTC1074" },
+	{ "INTC1096" },
+	{ "INTC100B" },
+	{ "INTC10D1" },
+	{},
+};
+
+static const struct acpi_device_id i2c_hids[] = {
+	{ "INTC1075" },
+	{ "INTC1097" },
+	{ "INTC100C" },
+	{ "INTC10D2" },
+	{},
+};
+
+static const struct acpi_device_id spi_hids[] = {
+	{ "INTC1091" },
+	{ "INTC1098" },
+	{ "INTC100D" },
+	{ "INTC10D3" },
+	{},
+};
+
+static int match_device_ids(struct acpi_device *adev, void *data)
+{
+	struct match_ids_walk_data *wd = data;
+
+	if (!acpi_match_device_ids(adev, wd->ids)) {
+		wd->adev = adev;
+		return 1;
+	}
+
+	return 0;
+}
+
+/* bind auxiliary device to acpi device */
+static void ljca_auxdev_acpi_bind(struct ljca_adapter *adap,
+				  struct auxiliary_device *auxdev,
+				  u64 adr)
+{
+	struct match_ids_walk_data wd = { 0 };
+	struct acpi_device *parent, *adev;
+	struct device *dev = adap->dev;
+
+	parent = ACPI_COMPANION(dev);
+	if (!parent)
+		return;
+
+	/*
+	 * get auxdev ACPI handle from the ACPI device directly
+	 * under the parent that matches _ADR.
+	 */
+	adev = acpi_find_child_device(parent, adr, false);
+	if (adev) {
+		ACPI_COMPANION_SET(&auxdev->dev, adev);
+		return;
+	}
+
+	/*
+	 * _ADR is a grey area in the ACPI specification, some
+	 * platforms use _HID to distinguish children devices.
+	 */
+	switch (adr) {
+	case LJCA_GPIO_ACPI_ADR:
+		wd.ids = gpio_hids;
+		break;
+	case LJCA_I2C1_ACPI_ADR:
+	case LJCA_I2C2_ACPI_ADR:
+		wd.ids = i2c_hids;
+		break;
+	case LJCA_SPI1_ACPI_ADR:
+	case LJCA_SPI2_ACPI_ADR:
+		wd.ids = spi_hids;
+		break;
+	default:
+		dev_warn(dev, "unsupported _ADR\n");
+		return;
+	}
+
+	acpi_dev_for_each_child(parent, match_device_ids, &wd);
+	if (wd.adev) {
+		ACPI_COMPANION_SET(&auxdev->dev, wd.adev);
+		return;
+	}
+
+	parent = ACPI_COMPANION(dev->parent->parent);
+	if (!parent)
+		return;
+
+	acpi_dev_for_each_child(parent, match_device_ids, &wd);
+	if (wd.adev)
+		ACPI_COMPANION_SET(&auxdev->dev, wd.adev);
+}
+#else
+static void ljca_auxdev_acpi_bind(struct ljca_adapter *adap,
+				  struct auxiliary_device *auxdev,
+				  u64 adr)
+{
+}
+#endif
+
+static void ljca_auxdev_release(struct device *dev)
+{
+	struct auxiliary_device *auxdev = to_auxiliary_dev(dev);
+
+	kfree(auxdev->dev.platform_data);
+}
+
+static int ljca_new_client_device(struct ljca_adapter *adap, u8 type, u8 id,
+				  char *name, void *data, u64 adr)
+{
+	struct auxiliary_device *auxdev;
+	struct ljca_client *client;
+	int ret;
+
+	client = kzalloc(sizeof *client, GFP_KERNEL);
+	if (!client)
+		return -ENOMEM;
+
+	client->type = type;
+	client->id = id;
+	client->adapter = adap;
+	spin_lock_init(&client->event_cb_lock);
+
+	auxdev = &client->auxdev;
+	auxdev->name = name;
+	auxdev->id = id;
+
+	auxdev->dev.parent = adap->dev;
+	auxdev->dev.platform_data = data;
+	auxdev->dev.release = ljca_auxdev_release;
+
+	ret = auxiliary_device_init(auxdev);
+	if (ret)
+		goto err_free;
+
+	ljca_auxdev_acpi_bind(adap, auxdev, adr);
+
+	ret = auxiliary_device_add(auxdev);
+	if (ret)
+		goto err_uninit;
+
+	list_add_tail(&client->link, &adap->client_list);
+
+	return 0;
+
+err_uninit:
+	auxiliary_device_uninit(auxdev);
+
+err_free:
+	kfree(client);
+
+	return ret;
+}
+
+static int ljca_enumerate_gpio(struct ljca_adapter *adap)
+{
+	u32 valid_pin[LJCA_MAX_GPIO_NUM / BITS_PER_TYPE(u32)];
+	struct ljca_gpio_descriptor *desc;
+	struct ljca_gpio_info *gpio_info;
+	u8 buf[LJCA_MAX_PAYLOAD_SIZE];
+	int ret, gpio_num;
+	unsigned int i;
+
+	ret = ljca_send(adap, LJCA_CLIENT_MNG, LJCA_MNG_ENUM_GPIO, NULL, 0, buf,
+			sizeof(buf), true, LJCA_ENUM_CLIENT_TIMEOUT_MS);
+	if (ret < 0)
+		return ret;
+
+	/* check firmware response */
+	desc = (struct ljca_gpio_descriptor *)buf;
+	if (ret != struct_size(desc, bank_desc, desc->bank_num))
+		return -EINVAL;
+
+	gpio_num = desc->pins_per_bank * desc->bank_num;
+	if (gpio_num > LJCA_MAX_GPIO_NUM)
+		return -EINVAL;
+
+	/* construct platform data */
+	gpio_info = kzalloc(sizeof *gpio_info, GFP_KERNEL);
+	if (!gpio_info)
+		return -ENOMEM;
+	gpio_info->num = gpio_num;
+
+	for (i = 0; i < desc->bank_num; i++)
+		valid_pin[i] = desc->bank_desc[i].valid_pins;
+	bitmap_from_arr32(gpio_info->valid_pin_map, valid_pin, gpio_num);
+
+	ret = ljca_new_client_device(adap, LJCA_CLIENT_GPIO, 0, "ljca-gpio",
+				     gpio_info, LJCA_GPIO_ACPI_ADR);
+	if (ret)
+		kfree(gpio_info);
+
+	return ret;
+}
+
+static int ljca_enumerate_i2c(struct ljca_adapter *adap)
+{
+	struct ljca_i2c_descriptor *desc;
+	struct ljca_i2c_info *i2c_info;
+	u8 buf[LJCA_MAX_PAYLOAD_SIZE];
+	unsigned int i;
+	int ret;
+
+	ret = ljca_send(adap, LJCA_CLIENT_MNG, LJCA_MNG_ENUM_I2C, NULL, 0, buf,
+			sizeof(buf), true, LJCA_ENUM_CLIENT_TIMEOUT_MS);
+	if (ret < 0)
+		return ret;
+
+	/* check firmware response */
+	desc = (struct ljca_i2c_descriptor *)buf;
+	if (ret != struct_size(desc, info, desc->num))
+		return -EINVAL;
+
+	for (i = 0; i < desc->num; i++) {
+		/* construct platform data */
+		i2c_info = kzalloc(sizeof *i2c_info, GFP_KERNEL);
+		if (!i2c_info)
+			return -ENOMEM;
+
+		i2c_info->id = desc->info[i].id;
+		i2c_info->capacity = desc->info[i].capacity;
+		i2c_info->intr_pin = desc->info[i].intr_pin;
+
+		ret = ljca_new_client_device(adap, LJCA_CLIENT_I2C, i,
+					     "ljca-i2c", i2c_info,
+					     LJCA_I2C1_ACPI_ADR + i);
+		if (ret) {
+			kfree(i2c_info);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int ljca_enumerate_spi(struct ljca_adapter *adap)
+{
+	struct ljca_spi_descriptor *desc;
+	struct ljca_spi_info *spi_info;
+	u8 buf[LJCA_MAX_PAYLOAD_SIZE];
+	unsigned int i;
+	int ret;
+
+	ret = ljca_send(adap, LJCA_CLIENT_MNG, LJCA_MNG_ENUM_SPI, NULL, 0, buf,
+			sizeof(buf), true, LJCA_ENUM_CLIENT_TIMEOUT_MS);
+	if (ret < 0)
+		return ret;
+
+	desc = (struct ljca_spi_descriptor *)buf;
+	for (i = 0; i < desc->num; i++) {
+		/* construct platform data */
+		spi_info = kzalloc(sizeof *spi_info, GFP_KERNEL);
+		if (!spi_info)
+			return -ENOMEM;
+
+		spi_info->id = desc->info[i].id;
+		spi_info->capacity = desc->info[i].capacity;
+
+		ret = ljca_new_client_device(adap, LJCA_CLIENT_SPI, i,
+					     "ljca-spi", spi_info,
+					     LJCA_SPI1_ACPI_ADR + i);
+		if (ret) {
+			kfree(spi_info);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int ljca_reset_handshake(struct ljca_adapter *adap)
+{
+	__le32 reset_id = cpu_to_le32(adap->reset_id);
+	__le32 reset_id_ret = 0;
+	int ret;
+
+	adap->reset_id += 1;
+
+	ret = ljca_send(adap, LJCA_CLIENT_MNG, LJCA_MNG_RESET, &reset_id,
+			sizeof(__le32), &reset_id_ret, sizeof(__le32), true,
+			LJCA_WRITE_ACK_TIMEOUT_MS);
+	if (ret < 0)
+		return ret;
+
+	if (reset_id_ret != reset_id)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int ljca_enumerate_clients(struct ljca_adapter *adap)
+{
+	int ret;
+
+	ret = ljca_reset_handshake(adap);
+	if (ret)
+		return ret;
+
+	ret = ljca_enumerate_gpio(adap);
+	if (ret)
+		dev_warn(adap->dev, "enumerate GPIO error\n");
+
+	ret = ljca_enumerate_i2c(adap);
+	if (ret)
+		dev_warn(adap->dev, "enumerate I2C error\n");
+
+	ret = ljca_enumerate_spi(adap);
+	if (ret)
+		dev_warn(adap->dev, "enumerate SPI error\n");
+
+	return 0;
+}
+
+static int ljca_probe(struct usb_interface *interface,
+		      const struct usb_device_id *id)
+{
+	struct usb_device *usb_dev = interface_to_usbdev(interface);
+	struct usb_host_interface *alt = interface->cur_altsetting;
+	struct usb_endpoint_descriptor *ep_in, *ep_out;
+	struct device *dev = &interface->dev;
+	struct ljca_adapter *adap;
+	int ret;
+
+	adap = devm_kzalloc(dev, sizeof(*adap), GFP_KERNEL);
+	if (!adap)
+		return -ENOMEM;
+
+	/* separate tx buffer allocation for alignment */
+	adap->tx_buf = devm_kzalloc(dev, LJCA_MAX_PACKET_SIZE, GFP_KERNEL);
+	if (!adap->tx_buf)
+		return -ENOMEM;
+	adap->tx_buf_len = LJCA_MAX_PACKET_SIZE;
+
+	mutex_init(&adap->mutex);
+	spin_lock_init(&adap->lock);
+	init_completion(&adap->cmd_completion);
+	INIT_LIST_HEAD(&adap->client_list);
+
+	adap->intf = usb_get_intf(interface);
+	adap->usb_dev = usb_dev;
+	adap->dev = dev;
+
+	/*
+	 * find the first bulk in and out endpoints.
+	 * ignore any others.
+	 */
+	ret = usb_find_common_endpoints(alt, &ep_in, &ep_out, NULL, NULL);
+	if (ret) {
+		dev_err(dev, "bulk endpoints not found\n");
+		goto err;
+	}
+	adap->rx_pipe = usb_rcvbulkpipe(usb_dev, usb_endpoint_num(ep_in));
+	adap->tx_pipe = usb_sndbulkpipe(usb_dev, usb_endpoint_num(ep_out));
+
+	/* setup rx buffer */
+	adap->rx_len = usb_endpoint_maxp(ep_in);
+	adap->rx_buf = devm_kzalloc(dev, adap->rx_len, GFP_KERNEL);
+	if (!adap->rx_buf) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	/* alloc rx urb */
+	adap->rx_urb = usb_alloc_urb(0, GFP_KERNEL);
+	if (!adap->rx_urb) {
+		ret = -ENOMEM;
+		goto err;
+	}
+	usb_fill_bulk_urb(adap->rx_urb, usb_dev, adap->rx_pipe,
+			  adap->rx_buf, adap->rx_len, ljca_recv, adap);
+
+	usb_set_intfdata(interface, adap);
+
+	/* submit rx urb before enumerate clients */
+	ret = usb_submit_urb(adap->rx_urb, GFP_KERNEL);
+	if (ret) {
+		dev_err(dev, "submit rx urb failed: %d\n", ret);
+		goto err_free;
+	}
+
+	ret = ljca_enumerate_clients(adap);
+	if (ret)
+		goto err_kill;
+
+	usb_enable_autosuspend(usb_dev);
+
+	return 0;
+
+err_kill:
+	usb_kill_urb(adap->rx_urb);
+
+err_free:
+	usb_free_urb(adap->rx_urb);
+
+err:
+	usb_put_intf(adap->intf);
+	mutex_destroy(&adap->mutex);
+
+	return ret;
+}
+
+static void ljca_disconnect(struct usb_interface *interface)
+{
+	struct ljca_adapter *adap = usb_get_intfdata(interface);
+	struct ljca_client *client, *next;
+
+	usb_kill_urb(adap->rx_urb);
+
+	list_for_each_entry_safe(client, next, &adap->client_list, link) {
+		auxiliary_device_delete(&client->auxdev);
+		auxiliary_device_uninit(&client->auxdev);
+
+		list_del_init(&client->link);
+		kfree(client);
+	}
+
+	usb_free_urb(adap->rx_urb);
+
+	usb_put_intf(adap->intf);
+
+	mutex_destroy(&adap->mutex);
+}
+
+static int ljca_suspend(struct usb_interface *interface, pm_message_t message)
+{
+	struct ljca_adapter *adap = usb_get_intfdata(interface);
+
+	usb_kill_urb(adap->rx_urb);
+
+	return 0;
+}
+
+static int ljca_resume(struct usb_interface *interface)
+{
+	struct ljca_adapter *adap = usb_get_intfdata(interface);
+
+	return usb_submit_urb(adap->rx_urb, GFP_KERNEL);
+}
+
+static const struct usb_device_id ljca_table[] = {
+	{ USB_DEVICE(0x8086, 0x0b63) },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(usb, ljca_table);
+
+static struct usb_driver ljca_driver = {
+	.name = "ljca",
+	.id_table = ljca_table,
+	.probe = ljca_probe,
+	.disconnect = ljca_disconnect,
+	.suspend = ljca_suspend,
+	.resume = ljca_resume,
+	.supports_autosuspend = 1,
+};
+module_usb_driver(ljca_driver);
+
+MODULE_AUTHOR("Wentong Wu <wentong.wu@intel.com>");
+MODULE_AUTHOR("Zhifeng Wang <zhifeng.wang@intel.com>");
+MODULE_AUTHOR("Lixu Zhang <lixu.zhang@intel.com>");
+MODULE_DESCRIPTION("Intel La Jolla Cove Adapter USB driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/usb/ljca.h b/include/linux/usb/ljca.h
new file mode 100644
index 0000000..6eadd43
--- /dev/null
+++ b/include/linux/usb/ljca.h
@@ -0,0 +1,113 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2023, Intel Corporation. All rights reserved.
+ */
+#ifndef _LINUX_USB_LJCA_H_
+#define _LINUX_USB_LJCA_H_
+
+#include <linux/auxiliary_bus.h>
+#include <linux/list.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+#define LJCA_MAX_GPIO_NUM 64
+
+#define auxiliary_dev_to_ljca_client(auxiliary_dev)			\
+		container_of(auxiliary_dev, struct ljca_client, auxdev)
+
+struct ljca_adapter;
+
+/**
+ * typedef ljca_event_cb_t - event callback function signature
+ *
+ * @context: the execution context of who registered this callback
+ * @cmd: the command from device for this event
+ * @evt_data: the event data payload
+ * @len: the event data payload length
+ *
+ * The callback function is called in interrupt context and the data payload is
+ * only valid during the call. If the user needs later access of the data, it
+ * must copy it.
+ */
+typedef void (*ljca_event_cb_t)(void *context, u8 cmd, const void *evt_data, int len);
+
+struct ljca_client {
+	u8 type;
+	u8 id;
+	struct list_head link;
+	struct auxiliary_device auxdev;
+	struct ljca_adapter *adapter;
+
+	void *context;
+	ljca_event_cb_t event_cb;
+	/* lock to protect event_cb */
+	spinlock_t event_cb_lock;
+};
+
+struct ljca_gpio_info {
+	unsigned int num;
+	DECLARE_BITMAP(valid_pin_map, LJCA_MAX_GPIO_NUM);
+};
+
+struct ljca_i2c_info {
+	u8 id;
+	u8 capacity;
+	u8 intr_pin;
+};
+
+struct ljca_spi_info {
+	u8 id;
+	u8 capacity;
+};
+
+/**
+ * ljca_register_event_cb - register a callback function to receive events
+ *
+ * @client: ljca client device
+ * @event_cb: callback function
+ * @context: execution context of event callback
+ *
+ * Return: 0 in case of success, negative value in case of error
+ */
+int ljca_register_event_cb(struct ljca_client *client, ljca_event_cb_t event_cb, void *context);
+
+/**
+ * ljca_unregister_event_cb - unregister the callback function for an event
+ *
+ * @client: ljca client device
+ */
+void ljca_unregister_event_cb(struct ljca_client *client);
+
+/**
+ * ljca_transfer - issue a LJCA command and wait for a response
+ *
+ * @client: ljca client device
+ * @cmd: the command to be sent to the device
+ * @obuf: the buffer to be sent to the device; it can be NULL if the user
+ *	doesn't need to transmit data with this command
+ * @obuf_len: the size of the buffer to be sent to the device; it should
+ *	be 0 when obuf is NULL
+ * @ibuf: any data associated with the response will be copied here; it can be
+ *	NULL if the user doesn't need the response data
+ * @ibuf_len: must be initialized to the input buffer size
+ *
+ * Return: the actual length data transferred for success, negative value for errors
+ */
+int ljca_transfer(struct ljca_client *client, u8 cmd, const void *obuf,
+		  unsigned int obuf_len, void *ibuf, unsigned int ibuf_len);
+
+/**
+ * ljca_transfer_noack - issue a LJCA command without a response
+ *
+ * @client: ljca client device
+ * @cmd: the command to be sent to the device
+ * @obuf: the buffer to be sent to the device; it can be NULL if the user
+ *	doesn't need to transmit data with this command
+ * @obuf_len: the size of the buffer to be sent to the device
+ *
+ * Return: 0 for success, negative value for errors
+ */
+int ljca_transfer_noack(struct ljca_client *client, u8 cmd, const void *obuf,
+			unsigned int obuf_len);
+
+#endif
-- 
2.7.4

[PATCH v9 2/4] i2c: Add support for Intel LJCA USB I2C driver

[Wentong Wu]

Implements the I2C function of Intel USB-I2C/GPIO/SPI adapter device
named "La Jolla Cove Adapter" (LJCA). It communicate with LJCA I2C
module with specific protocol through interfaces exported by LJCA
USB driver.

Signed-off-by: Wentong Wu <wentong.wu@intel.com>
Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
---
 drivers/i2c/busses/Kconfig    |  11 ++
 drivers/i2c/busses/Makefile   |   1 +
 drivers/i2c/busses/i2c-ljca.c | 353 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 365 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-ljca.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 9cfe8fc..7e2c52d 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1264,6 +1264,17 @@ config I2C_DLN2
 	 This driver can also be built as a module.  If so, the module
 	 will be called i2c-dln2.
 
+config I2C_LJCA
+	tristate "I2C functionality of Intel La Jolla Cove Adapter"
+	depends on USB_LJCA
+	default USB_LJCA
+	help
+	  If you say yes to this option, I2C functionality support of Intel
+	  La Jolla Cove Adapter (LJCA) will be included.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-ljca.
+
 config I2C_CP2615
 	tristate "Silicon Labs CP2615 USB sound card and I2C adapter"
 	depends on USB
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index af56fe2..3757b93 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -133,6 +133,7 @@ obj-$(CONFIG_I2C_GXP)		+= i2c-gxp.o
 # External I2C/SMBus adapter drivers
 obj-$(CONFIG_I2C_DIOLAN_U2C)	+= i2c-diolan-u2c.o
 obj-$(CONFIG_I2C_DLN2)		+= i2c-dln2.o
+obj-$(CONFIG_I2C_LJCA)		+= i2c-ljca.o
 obj-$(CONFIG_I2C_CP2615) += i2c-cp2615.o
 obj-$(CONFIG_I2C_PARPORT)	+= i2c-parport.o
 obj-$(CONFIG_I2C_PCI1XXXX)	+= i2c-mchp-pci1xxxx.o
diff --git a/drivers/i2c/busses/i2c-ljca.c b/drivers/i2c/busses/i2c-ljca.c
new file mode 100644
index 0000000..2af8c32
--- /dev/null
+++ b/drivers/i2c/busses/i2c-ljca.c
@@ -0,0 +1,353 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Intel La Jolla Cove Adapter USB-I2C driver
+ *
+ * Copyright (c) 2023, Intel Corporation.
+ */
+
+#include <linux/auxiliary_bus.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/dev_printk.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/usb/ljca.h>
+
+/* I2C init flags */
+#define LJCA_I2C_INIT_FLAG_MODE			BIT(0)
+#define LJCA_I2C_INIT_FLAG_MODE_POLLING		FIELD_PREP(LJCA_I2C_INIT_FLAG_MODE, 0)
+#define LJCA_I2C_INIT_FLAG_MODE_INTERRUPT	FIELD_PREP(LJCA_I2C_INIT_FLAG_MODE, 1)
+
+#define LJCA_I2C_INIT_FLAG_ADDR_16BIT		BIT(0)
+
+#define LJCA_I2C_INIT_FLAG_FREQ			GENMASK(2, 1)
+#define LJCA_I2C_INIT_FLAG_FREQ_100K		FIELD_PREP(LJCA_I2C_INIT_FLAG_FREQ, 0)
+#define LJCA_I2C_INIT_FLAG_FREQ_400K		FIELD_PREP(LJCA_I2C_INIT_FLAG_FREQ, 1)
+#define LJCA_I2C_INIT_FLAG_FREQ_1M		FIELD_PREP(LJCA_I2C_INIT_FLAG_FREQ, 2)
+
+#define LJCA_I2C_BUF_SIZE			60u
+#define LJCA_I2C_MAX_XFER_SIZE			(LJCA_I2C_BUF_SIZE - sizeof(struct ljca_i2c_rw_packet))
+
+/* I2C commands */
+enum ljca_i2c_cmd {
+	LJCA_I2C_INIT = 1,
+	LJCA_I2C_XFER,
+	LJCA_I2C_START,
+	LJCA_I2C_STOP,
+	LJCA_I2C_READ,
+	LJCA_I2C_WRITE,
+};
+
+enum ljca_xfer_type {
+	LJCA_I2C_WRITE_XFER_TYPE,
+	LJCA_I2C_READ_XFER_TYPE,
+};
+
+/* I2C raw commands: Init/Start/Read/Write/Stop */
+struct ljca_i2c_rw_packet {
+	u8 id;
+	__le16 len;
+	u8 data[];
+} __packed;
+
+struct ljca_i2c_dev {
+	struct ljca_client *ljca;
+	struct ljca_i2c_info *i2c_info;
+	struct i2c_adapter adap;
+
+	u8 obuf[LJCA_I2C_BUF_SIZE];
+	u8 ibuf[LJCA_I2C_BUF_SIZE];
+};
+
+static u8 ljca_i2c_format_slave_addr(u8 slave_addr, u8 type)
+{
+	return (slave_addr << 1) | type;
+}
+
+static int ljca_i2c_init(struct ljca_i2c_dev *ljca_i2c, u8 id)
+{
+	struct ljca_i2c_rw_packet *w_packet =
+			(struct ljca_i2c_rw_packet *)ljca_i2c->obuf;
+	int ret;
+
+	memset(w_packet, 0, sizeof(*w_packet));
+	w_packet->id = id;
+	w_packet->len = cpu_to_le16(sizeof(*w_packet->data));
+	w_packet->data[0] = LJCA_I2C_INIT_FLAG_FREQ_400K;
+
+	ret = ljca_transfer(ljca_i2c->ljca, LJCA_I2C_INIT, w_packet,
+			    struct_size(w_packet, data, 1), NULL, 0);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int ljca_i2c_start(struct ljca_i2c_dev *ljca_i2c, u8 slave_addr,
+			  enum ljca_xfer_type type)
+{
+	struct ljca_i2c_rw_packet *w_packet =
+			(struct ljca_i2c_rw_packet *)ljca_i2c->obuf;
+	struct ljca_i2c_rw_packet *r_packet =
+			(struct ljca_i2c_rw_packet *)ljca_i2c->ibuf;
+	s16 rp_len;
+	int ret;
+
+	memset(w_packet, 0, sizeof(*w_packet));
+	w_packet->id = ljca_i2c->i2c_info->id;
+	w_packet->len = cpu_to_le16(sizeof(*w_packet->data));
+	w_packet->data[0] = ljca_i2c_format_slave_addr(slave_addr, type);
+
+	ret = ljca_transfer(ljca_i2c->ljca, LJCA_I2C_START, w_packet,
+			    struct_size(w_packet, data, 1), r_packet,
+			    LJCA_I2C_BUF_SIZE);
+	if (ret < 0)
+		return ret;
+	else if (ret < sizeof(*r_packet))
+		return -EIO;
+
+	rp_len = le16_to_cpu(r_packet->len);
+	if (rp_len < 0 || r_packet->id != w_packet->id) {
+		dev_err(&ljca_i2c->adap.dev,
+			"i2c start failed len: %d id: %d %d\n",
+			rp_len, r_packet->id, w_packet->id);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int ljca_i2c_stop(struct ljca_i2c_dev *ljca_i2c, u8 slave_addr)
+{
+	struct ljca_i2c_rw_packet *w_packet =
+			(struct ljca_i2c_rw_packet *)ljca_i2c->obuf;
+	struct ljca_i2c_rw_packet *r_packet =
+			(struct ljca_i2c_rw_packet *)ljca_i2c->ibuf;
+	s16 rp_len;
+	int ret;
+
+	memset(w_packet, 0, sizeof(*w_packet));
+	w_packet->id = ljca_i2c->i2c_info->id;
+	w_packet->len = cpu_to_le16(sizeof(*w_packet->data));
+	w_packet->data[0] = 0;
+
+	ret = ljca_transfer(ljca_i2c->ljca, LJCA_I2C_STOP, w_packet,
+			    struct_size(w_packet, data, 1), r_packet,
+			    LJCA_I2C_BUF_SIZE);
+	if (ret < 0)
+		return ret;
+	else if (ret < sizeof(*r_packet))
+		return -EIO;
+
+	rp_len = le16_to_cpu(r_packet->len);
+	if (rp_len < 0 || r_packet->id != w_packet->id) {
+		dev_err(&ljca_i2c->adap.dev,
+			"i2c stop failed len: %d id: %d %d\n",
+			rp_len, r_packet->id, w_packet->id);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int ljca_i2c_pure_read(struct ljca_i2c_dev *ljca_i2c, u8 *data, u8 len)
+{
+	struct ljca_i2c_rw_packet *w_packet =
+			(struct ljca_i2c_rw_packet *)ljca_i2c->obuf;
+	struct ljca_i2c_rw_packet *r_packet =
+			(struct ljca_i2c_rw_packet *)ljca_i2c->ibuf;
+	s16 rp_len;
+	int ret;
+
+	if (len > LJCA_I2C_MAX_XFER_SIZE)
+		return -EINVAL;
+
+	memset(w_packet, 0, sizeof(*w_packet));
+	w_packet->id = ljca_i2c->i2c_info->id;
+	w_packet->len = cpu_to_le16(len);
+	w_packet->data[0] = 0;
+
+	ret = ljca_transfer(ljca_i2c->ljca, LJCA_I2C_READ, w_packet,
+			    struct_size(w_packet, data, 1), r_packet,
+			    LJCA_I2C_BUF_SIZE);
+	if (ret < 0)
+		return ret;
+	else if (ret < sizeof(*r_packet))
+		return -EIO;
+
+	rp_len = le16_to_cpu(r_packet->len);
+	if (rp_len != len || r_packet->id != w_packet->id) {
+		dev_err(&ljca_i2c->adap.dev,
+			"i2c raw read failed len: %d id: %d %d\n",
+			rp_len, r_packet->id, w_packet->id);
+		return -EIO;
+	}
+
+	memcpy(data, r_packet->data, len);
+
+	return 0;
+}
+
+static int ljca_i2c_read(struct ljca_i2c_dev *ljca_i2c, u8 slave_addr, u8 *data,
+			 u8 len)
+{
+	int ret;
+
+	ret = ljca_i2c_start(ljca_i2c, slave_addr, LJCA_I2C_READ_XFER_TYPE);
+	if (!ret)
+		ret = ljca_i2c_pure_read(ljca_i2c, data, len);
+
+	ljca_i2c_stop(ljca_i2c, slave_addr);
+
+	return ret;
+}
+
+static int ljca_i2c_pure_write(struct ljca_i2c_dev *ljca_i2c, u8 *data, u8 len)
+{
+	struct ljca_i2c_rw_packet *w_packet =
+			(struct ljca_i2c_rw_packet *)ljca_i2c->obuf;
+	struct ljca_i2c_rw_packet *r_packet =
+			(struct ljca_i2c_rw_packet *)ljca_i2c->ibuf;
+	s16 rplen;
+	int ret;
+
+	if (len > LJCA_I2C_MAX_XFER_SIZE)
+		return -EINVAL;
+
+	memset(w_packet, 0, sizeof(*w_packet));
+	w_packet->id = ljca_i2c->i2c_info->id;
+	w_packet->len = cpu_to_le16(len);
+	memcpy(w_packet->data, data, len);
+
+	ret = ljca_transfer(ljca_i2c->ljca, LJCA_I2C_WRITE, w_packet,
+			    struct_size(w_packet, data, len), r_packet,
+			    LJCA_I2C_BUF_SIZE);
+	if (ret < 0)
+		return ret;
+	else if (ret < sizeof(*r_packet))
+		return -EIO;
+
+	rplen = le16_to_cpu(r_packet->len);
+	if (rplen != len || r_packet->id != w_packet->id) {
+		dev_err(&ljca_i2c->adap.dev,
+			"i2c write failed len: %d id: %d/%d\n",
+			rplen, r_packet->id, w_packet->id);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int ljca_i2c_write(struct ljca_i2c_dev *ljca_i2c, u8 slave_addr,
+			  u8 *data, u8 len)
+{
+	int ret;
+
+	ret = ljca_i2c_start(ljca_i2c, slave_addr, LJCA_I2C_WRITE_XFER_TYPE);
+	if (!ret)
+		ret = ljca_i2c_pure_write(ljca_i2c, data, len);
+
+	ljca_i2c_stop(ljca_i2c, slave_addr);
+
+	return ret;
+}
+
+static int ljca_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msg,
+			 int num)
+{
+	struct ljca_i2c_dev *ljca_i2c;
+	struct i2c_msg *cur_msg;
+	int i, ret;
+
+	ljca_i2c = i2c_get_adapdata(adapter);
+	if (!ljca_i2c)
+		return -EINVAL;
+
+	for (i = 0; i < num; i++) {
+		cur_msg = &msg[i];
+		if (cur_msg->flags & I2C_M_RD)
+			ret = ljca_i2c_read(ljca_i2c, cur_msg->addr,
+					    cur_msg->buf, cur_msg->len);
+		else
+			ret = ljca_i2c_write(ljca_i2c, cur_msg->addr,
+					     cur_msg->buf, cur_msg->len);
+
+		if (ret)
+			return ret;
+	}
+
+	return num;
+}
+
+static u32 ljca_i2c_func(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static const struct i2c_adapter_quirks ljca_i2c_quirks = {
+	.max_read_len = LJCA_I2C_MAX_XFER_SIZE,
+	.max_write_len = LJCA_I2C_MAX_XFER_SIZE,
+};
+
+static const struct i2c_algorithm ljca_i2c_algo = {
+	.master_xfer = ljca_i2c_xfer,
+	.functionality = ljca_i2c_func,
+};
+
+static int ljca_i2c_probe(struct auxiliary_device *auxdev,
+			  const struct auxiliary_device_id *aux_dev_id)
+{
+	struct ljca_client *ljca = auxiliary_dev_to_ljca_client(auxdev);
+	struct ljca_i2c_dev *ljca_i2c;
+	int ret;
+
+	ljca_i2c = devm_kzalloc(&auxdev->dev, sizeof(*ljca_i2c), GFP_KERNEL);
+	if (!ljca_i2c)
+		return -ENOMEM;
+
+	ljca_i2c->ljca = ljca;
+	ljca_i2c->i2c_info = dev_get_platdata(&auxdev->dev);
+
+	ljca_i2c->adap.owner = THIS_MODULE;
+	ljca_i2c->adap.class = I2C_CLASS_HWMON;
+	ljca_i2c->adap.algo = &ljca_i2c_algo;
+	ljca_i2c->adap.quirks = &ljca_i2c_quirks;
+	ljca_i2c->adap.dev.parent = &auxdev->dev;
+
+	snprintf(ljca_i2c->adap.name, sizeof(ljca_i2c->adap.name), "%s-%s-%d",
+		 dev_name(&auxdev->dev), dev_name(auxdev->dev.parent),
+		 ljca_i2c->i2c_info->id);
+
+	device_set_node(&ljca_i2c->adap.dev, dev_fwnode(&auxdev->dev));
+
+	i2c_set_adapdata(&ljca_i2c->adap, ljca_i2c);
+	auxiliary_set_drvdata(auxdev, ljca_i2c);
+
+	ret = ljca_i2c_init(ljca_i2c, ljca_i2c->i2c_info->id);
+	if (ret) {
+		dev_err(&auxdev->dev, "i2c init failed id: %d\n",
+			ljca_i2c->i2c_info->id);
+		return -EIO;
+	}
+
+	return devm_i2c_add_adapter(&auxdev->dev, &ljca_i2c->adap);
+}
+
+static const struct auxiliary_device_id ljca_i2c_id_table[] = {
+	{ "usb_ljca.ljca-i2c", 0 },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(auxiliary, ljca_i2c_id_table);
+
+static struct auxiliary_driver ljca_i2c_driver = {
+	.probe = ljca_i2c_probe,
+	.id_table = ljca_i2c_id_table,
+};
+module_auxiliary_driver(ljca_i2c_driver);
+
+MODULE_AUTHOR("Wentong Wu <wentong.wu@intel.com>");
+MODULE_AUTHOR("Zhifeng Wang <zhifeng.wang@intel.com>");
+MODULE_AUTHOR("Lixu Zhang <lixu.zhang@intel.com>");
+MODULE_DESCRIPTION("Intel La Jolla Cove Adapter USB-I2C driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(LJCA);
-- 
2.7.4

[PATCH v9 3/4] spi: Add support for Intel LJCA USB SPI driver

[Wentong Wu]

Implements the SPI function of Intel USB-I2C/GPIO/SPI adapter device
named "La Jolla Cove Adapter" (LJCA). It communicate with LJCA SPI
module with specific protocol through interfaces exported by LJCA USB
driver.

Signed-off-by: Wentong Wu <wentong.wu@intel.com>
Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/spi/Kconfig    |  11 ++
 drivers/spi/Makefile   |   1 +
 drivers/spi/spi-ljca.c | 299 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 311 insertions(+)
 create mode 100644 drivers/spi/spi-ljca.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 8962b25..ad18864 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -583,6 +583,17 @@ config SPI_FSL_ESPI
 	  From MPC8536, 85xx platform uses the controller, and all P10xx,
 	  P20xx, P30xx,P40xx, P50xx uses this controller.
 
+config SPI_LJCA
+	tristate "Intel La Jolla Cove Adapter SPI support"
+	depends on USB_LJCA
+	default USB_LJCA
+	help
+	  Select this option to enable SPI driver for the Intel
+	  La Jolla Cove Adapter (LJCA) board.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called spi-ljca.
+
 config SPI_MESON_SPICC
 	tristate "Amlogic Meson SPICC controller"
 	depends on COMMON_CLK
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 080c2c1..0847962 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -70,6 +70,7 @@ obj-$(CONFIG_SPI_INTEL_PCI)		+= spi-intel-pci.o
 obj-$(CONFIG_SPI_INTEL_PLATFORM)	+= spi-intel-platform.o
 obj-$(CONFIG_SPI_LANTIQ_SSC)		+= spi-lantiq-ssc.o
 obj-$(CONFIG_SPI_JCORE)			+= spi-jcore.o
+obj-$(CONFIG_SPI_LJCA)			+= spi-ljca.o
 obj-$(CONFIG_SPI_LM70_LLP)		+= spi-lm70llp.o
 obj-$(CONFIG_SPI_LP8841_RTC)		+= spi-lp8841-rtc.o
 obj-$(CONFIG_SPI_MESON_SPICC)		+= spi-meson-spicc.o
diff --git a/drivers/spi/spi-ljca.c b/drivers/spi/spi-ljca.c
new file mode 100644
index 0000000..6b5ecb0
--- /dev/null
+++ b/drivers/spi/spi-ljca.c
@@ -0,0 +1,299 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Intel La Jolla Cove Adapter USB-SPI driver
+ *
+ * Copyright (c) 2023, Intel Corporation.
+ */
+
+#include <linux/auxiliary_bus.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/dev_printk.h>
+#include <linux/module.h>
+#include <linux/spi/spi.h>
+#include <linux/usb/ljca.h>
+
+#define LJCA_SPI_BUS_MAX_HZ		48000000
+
+#define LJCA_SPI_BUF_SIZE		60u
+#define LJCA_SPI_MAX_XFER_SIZE		\
+	(LJCA_SPI_BUF_SIZE - sizeof(struct ljca_spi_xfer_packet))
+
+#define LJCA_SPI_CLK_MODE_POLARITY	BIT(0)
+#define LJCA_SPI_CLK_MODE_PHASE		BIT(1)
+
+#define LJCA_SPI_XFER_INDICATOR_ID	GENMASK(5, 0)
+#define LJCA_SPI_XFER_INDICATOR_CMPL	BIT(6)
+#define LJCA_SPI_XFER_INDICATOR_INDEX	BIT(7)
+
+/* SPI commands */
+enum ljca_spi_cmd {
+	LJCA_SPI_INIT = 1,
+	LJCA_SPI_READ,
+	LJCA_SPI_WRITE,
+	LJCA_SPI_WRITEREAD,
+	LJCA_SPI_DEINIT,
+};
+
+enum {
+	LJCA_SPI_BUS_SPEED_24M,
+	LJCA_SPI_BUS_SPEED_12M,
+	LJCA_SPI_BUS_SPEED_8M,
+	LJCA_SPI_BUS_SPEED_6M,
+	LJCA_SPI_BUS_SPEED_4_8M, /*4.8MHz*/
+	LJCA_SPI_BUS_SPEED_MIN = LJCA_SPI_BUS_SPEED_4_8M,
+};
+
+enum {
+	LJCA_SPI_CLOCK_LOW_POLARITY,
+	LJCA_SPI_CLOCK_HIGH_POLARITY,
+};
+
+enum {
+	LJCA_SPI_CLOCK_FIRST_PHASE,
+	LJCA_SPI_CLOCK_SECOND_PHASE,
+};
+
+struct ljca_spi_init_packet {
+	u8 index;
+	u8 speed;
+	u8 mode;
+} __packed;
+
+struct ljca_spi_xfer_packet {
+	u8 indicator;
+	u8 len;
+	u8 data[];
+} __packed;
+
+struct ljca_spi_dev {
+	struct ljca_client *ljca;
+	struct spi_controller *controller;
+	struct ljca_spi_info *spi_info;
+	u8 speed;
+	u8 mode;
+
+	u8 obuf[LJCA_SPI_BUF_SIZE];
+	u8 ibuf[LJCA_SPI_BUF_SIZE];
+};
+
+static int ljca_spi_read_write(struct ljca_spi_dev *ljca_spi, const u8 *w_data,
+			       u8 *r_data, int len, int id, int complete,
+			       int cmd)
+{
+	struct ljca_spi_xfer_packet *w_packet =
+			(struct ljca_spi_xfer_packet *)ljca_spi->obuf;
+	struct ljca_spi_xfer_packet *r_packet =
+			(struct ljca_spi_xfer_packet *)ljca_spi->ibuf;
+	int ret;
+
+	w_packet->indicator = FIELD_PREP(LJCA_SPI_XFER_INDICATOR_ID, id) |
+			      FIELD_PREP(LJCA_SPI_XFER_INDICATOR_CMPL, complete) |
+			      FIELD_PREP(LJCA_SPI_XFER_INDICATOR_INDEX,
+					 ljca_spi->spi_info->id);
+
+	if (cmd == LJCA_SPI_READ) {
+		w_packet->len = sizeof(u16);
+		*(__le16 *)&w_packet->data[0] = cpu_to_le16(len);
+	} else {
+		w_packet->len = len;
+		memcpy(w_packet->data, w_data, len);
+	}
+
+	ret = ljca_transfer(ljca_spi->ljca, cmd, w_packet,
+			    struct_size(w_packet, data, w_packet->len),
+			    r_packet, LJCA_SPI_BUF_SIZE);
+	if (ret < 0)
+		return ret;
+	else if (ret < sizeof(*r_packet) || r_packet->len <= 0)
+		return -EIO;
+
+	if (r_data)
+		memcpy(r_data, r_packet->data, r_packet->len);
+
+	return 0;
+}
+
+static int ljca_spi_init(struct ljca_spi_dev *ljca_spi, u8 div, u8 mode)
+{
+	struct ljca_spi_init_packet w_packet = {};
+	int ret;
+
+	if (ljca_spi->mode == mode && ljca_spi->speed == div)
+		return 0;
+
+	w_packet.index = ljca_spi->spi_info->id;
+	w_packet.speed = div;
+	w_packet.mode = FIELD_PREP(LJCA_SPI_CLK_MODE_POLARITY,
+				   (mode & SPI_CPOL) ? LJCA_SPI_CLOCK_HIGH_POLARITY :
+						       LJCA_SPI_CLOCK_LOW_POLARITY) |
+			FIELD_PREP(LJCA_SPI_CLK_MODE_PHASE,
+				   (mode & SPI_CPHA) ? LJCA_SPI_CLOCK_SECOND_PHASE :
+						       LJCA_SPI_CLOCK_FIRST_PHASE);
+
+	ret = ljca_transfer(ljca_spi->ljca, LJCA_SPI_INIT, &w_packet,
+			    sizeof(w_packet), NULL, 0);
+	if (ret < 0)
+		return ret;
+
+	ljca_spi->mode = mode;
+	ljca_spi->speed = div;
+
+	return 0;
+}
+
+static int ljca_spi_deinit(struct ljca_spi_dev *ljca_spi)
+{
+	struct ljca_spi_init_packet w_packet = {};
+	int ret;
+
+	w_packet.index = ljca_spi->spi_info->id;
+
+	ret = ljca_transfer(ljca_spi->ljca, LJCA_SPI_DEINIT, &w_packet,
+			    sizeof(w_packet), NULL, 0);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static inline int ljca_spi_transfer(struct ljca_spi_dev *ljca_spi,
+				    const u8 *tx_data, u8 *rx_data, u16 len)
+{
+	int complete, cur_len;
+	int remaining = len;
+	int cmd, ret, i;
+	int offset = 0;
+
+	if (tx_data && rx_data)
+		cmd = LJCA_SPI_WRITEREAD;
+	else if (tx_data)
+		cmd = LJCA_SPI_WRITE;
+	else if (rx_data)
+		cmd = LJCA_SPI_READ;
+	else
+		return -EINVAL;
+
+	for (i = 0; remaining > 0; i++) {
+		cur_len = min_t(unsigned int, remaining, LJCA_SPI_MAX_XFER_SIZE);
+		complete = (cur_len == remaining);
+
+		ret = ljca_spi_read_write(ljca_spi,
+					  tx_data ? tx_data + offset : NULL,
+					  rx_data ? rx_data + offset : NULL,
+					  cur_len, i, complete, cmd);
+		if (ret)
+			return ret;
+
+		offset += cur_len;
+		remaining -= cur_len;
+	}
+
+	return 0;
+}
+
+static int ljca_spi_transfer_one(struct spi_controller *controller,
+				 struct spi_device *spi,
+				 struct spi_transfer *xfer)
+{
+	u8 div = DIV_ROUND_UP(controller->max_speed_hz, xfer->speed_hz) / 2 - 1;
+	struct ljca_spi_dev *ljca_spi = spi_controller_get_devdata(controller);
+	int ret;
+
+	div = min_t(u8, LJCA_SPI_BUS_SPEED_MIN, div);
+
+	ret = ljca_spi_init(ljca_spi, div, spi->mode);
+	if (ret) {
+		dev_err(&ljca_spi->ljca->auxdev.dev,
+			"cannot initialize transfer ret %d\n", ret);
+		return ret;
+	}
+
+	ret = ljca_spi_transfer(ljca_spi, xfer->tx_buf, xfer->rx_buf, xfer->len);
+	if (ret)
+		dev_err(&ljca_spi->ljca->auxdev.dev,
+			"transfer failed len: %d\n", xfer->len);
+
+	return ret;
+}
+
+static int ljca_spi_probe(struct auxiliary_device *auxdev,
+			  const struct auxiliary_device_id *aux_dev_id)
+{
+	struct ljca_client *ljca = auxiliary_dev_to_ljca_client(auxdev);
+	struct spi_controller *controller;
+	struct ljca_spi_dev *ljca_spi;
+	int ret;
+
+	controller = devm_spi_alloc_master(&auxdev->dev, sizeof(*ljca_spi));
+	if (!controller)
+		return -ENOMEM;
+
+	ljca_spi = spi_controller_get_devdata(controller);
+	ljca_spi->ljca = ljca;
+	ljca_spi->spi_info = dev_get_platdata(&auxdev->dev);
+	ljca_spi->controller = controller;
+
+	controller->bus_num = -1;
+	controller->mode_bits = SPI_CPHA | SPI_CPOL;
+	controller->transfer_one = ljca_spi_transfer_one;
+	controller->auto_runtime_pm = false;
+	controller->max_speed_hz = LJCA_SPI_BUS_MAX_HZ;
+
+	device_set_node(&ljca_spi->controller->dev, dev_fwnode(&auxdev->dev));
+	auxiliary_set_drvdata(auxdev, controller);
+
+	ret = spi_register_controller(controller);
+	if (ret)
+		dev_err(&auxdev->dev, "Failed to register controller\n");
+
+	return ret;
+}
+
+static void ljca_spi_dev_remove(struct auxiliary_device *auxdev)
+{
+	struct spi_controller *controller = auxiliary_get_drvdata(auxdev);
+	struct ljca_spi_dev *ljca_spi = spi_controller_get_devdata(controller);
+
+	spi_unregister_controller(controller);
+	ljca_spi_deinit(ljca_spi);
+}
+
+static int ljca_spi_dev_suspend(struct device *dev)
+{
+	struct spi_controller *controller = dev_get_drvdata(dev);
+
+	return spi_controller_suspend(controller);
+}
+
+static int ljca_spi_dev_resume(struct device *dev)
+{
+	struct spi_controller *controller = dev_get_drvdata(dev);
+
+	return spi_controller_resume(controller);
+}
+
+static const struct dev_pm_ops ljca_spi_pm = {
+	SYSTEM_SLEEP_PM_OPS(ljca_spi_dev_suspend, ljca_spi_dev_resume)
+};
+
+static const struct auxiliary_device_id ljca_spi_id_table[] = {
+	{ "usb_ljca.ljca-spi", 0 },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(auxiliary, ljca_spi_id_table);
+
+static struct auxiliary_driver ljca_spi_driver = {
+	.driver.pm	= &ljca_spi_pm,
+	.probe		= ljca_spi_probe,
+	.remove		= ljca_spi_dev_remove,
+	.id_table	= ljca_spi_id_table,
+};
+module_auxiliary_driver(ljca_spi_driver);
+
+MODULE_AUTHOR("Wentong Wu <wentong.wu@intel.com>");
+MODULE_AUTHOR("Zhifeng Wang <zhifeng.wang@intel.com>");
+MODULE_AUTHOR("Lixu Zhang <lixu.zhang@intel.com>");
+MODULE_DESCRIPTION("Intel La Jolla Cove Adapter USB-SPI driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(LJCA);
-- 
2.7.4

[PATCH v9 4/4] gpio: update Intel LJCA USB GPIO driver

[Wentong Wu]

This driver communicate with LJCA GPIO module with specific
protocol through interfaces exported by LJCA USB driver.
Update the driver according to LJCA USB driver's changes.

Signed-off-by: Wentong Wu <wentong.wu@intel.com>
Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/gpio/Kconfig     |   4 +-
 drivers/gpio/gpio-ljca.c | 242 +++++++++++++++++++++++++++--------------------
 2 files changed, 143 insertions(+), 103 deletions(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index e382dfe..c85843b 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1294,9 +1294,9 @@ config GPIO_KEMPLD
 
 config GPIO_LJCA
 	tristate "INTEL La Jolla Cove Adapter GPIO support"
-	depends on MFD_LJCA
+	depends on USB_LJCA
 	select GPIOLIB_IRQCHIP
-	default MFD_LJCA
+	default USB_LJCA
 	help
 	  Select this option to enable GPIO driver for the INTEL
 	  La Jolla Cove Adapter (LJCA) board.
diff --git a/drivers/gpio/gpio-ljca.c b/drivers/gpio/gpio-ljca.c
index 87863f0..f603095 100644
--- a/drivers/gpio/gpio-ljca.c
+++ b/drivers/gpio/gpio-ljca.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/auxiliary_bus.h>
 #include <linux/bitfield.h>
 #include <linux/bitops.h>
 #include <linux/dev_printk.h>
@@ -13,19 +14,18 @@
 #include <linux/irq.h>
 #include <linux/kernel.h>
 #include <linux/kref.h>
-#include <linux/mfd/ljca.h>
 #include <linux/module.h>
-#include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/types.h>
+#include <linux/usb/ljca.h>
 
 /* GPIO commands */
-#define LJCA_GPIO_CONFIG	1
-#define LJCA_GPIO_READ		2
-#define LJCA_GPIO_WRITE		3
-#define LJCA_GPIO_INT_EVENT	4
-#define LJCA_GPIO_INT_MASK	5
-#define LJCA_GPIO_INT_UNMASK	6
+#define LJCA_GPIO_CONFIG		1
+#define LJCA_GPIO_READ			2
+#define LJCA_GPIO_WRITE			3
+#define LJCA_GPIO_INT_EVENT		4
+#define LJCA_GPIO_INT_MASK		5
+#define LJCA_GPIO_INT_UNMASK		6
 
 #define LJCA_GPIO_CONF_DISABLE		BIT(0)
 #define LJCA_GPIO_CONF_INPUT		BIT(1)
@@ -36,45 +36,49 @@
 #define LJCA_GPIO_CONF_INTERRUPT	BIT(6)
 #define LJCA_GPIO_INT_TYPE		BIT(7)
 
-#define LJCA_GPIO_CONF_EDGE	FIELD_PREP(LJCA_GPIO_INT_TYPE, 1)
-#define LJCA_GPIO_CONF_LEVEL	FIELD_PREP(LJCA_GPIO_INT_TYPE, 0)
+#define LJCA_GPIO_CONF_EDGE		FIELD_PREP(LJCA_GPIO_INT_TYPE, 1)
+#define LJCA_GPIO_CONF_LEVEL		FIELD_PREP(LJCA_GPIO_INT_TYPE, 0)
 
 /* Intentional overlap with PULLUP / PULLDOWN */
-#define LJCA_GPIO_CONF_SET	BIT(3)
-#define LJCA_GPIO_CONF_CLR	BIT(4)
+#define LJCA_GPIO_CONF_SET		BIT(3)
+#define LJCA_GPIO_CONF_CLR		BIT(4)
 
-struct gpio_op {
+#define LJCA_GPIO_BUF_SIZE		60u
+
+struct ljca_gpio_op {
 	u8 index;
 	u8 value;
 } __packed;
 
-struct gpio_packet {
+struct ljca_gpio_packet {
 	u8 num;
-	struct gpio_op item[];
+	struct ljca_gpio_op item[];
 } __packed;
 
-#define LJCA_GPIO_BUF_SIZE 60
 struct ljca_gpio_dev {
-	struct platform_device *pdev;
+	struct ljca_client *ljca;
 	struct gpio_chip gc;
 	struct ljca_gpio_info *gpio_info;
 	DECLARE_BITMAP(unmasked_irqs, LJCA_MAX_GPIO_NUM);
 	DECLARE_BITMAP(enabled_irqs, LJCA_MAX_GPIO_NUM);
 	DECLARE_BITMAP(reenable_irqs, LJCA_MAX_GPIO_NUM);
+	DECLARE_BITMAP(output_enabled, LJCA_MAX_GPIO_NUM);
 	u8 *connect_mode;
-	/* mutex to protect irq bus */
+	/* protect irq bus */
 	struct mutex irq_lock;
 	struct work_struct work;
-	/* lock to protect package transfer to Hardware */
+	/* protect package transfer to hardware */
 	struct mutex trans_lock;
 
 	u8 obuf[LJCA_GPIO_BUF_SIZE];
 	u8 ibuf[LJCA_GPIO_BUF_SIZE];
 };
 
-static int gpio_config(struct ljca_gpio_dev *ljca_gpio, u8 gpio_id, u8 config)
+static int ljca_gpio_config(struct ljca_gpio_dev *ljca_gpio, u8 gpio_id,
+			    u8 config)
 {
-	struct gpio_packet *packet = (struct gpio_packet *)ljca_gpio->obuf;
+	struct ljca_gpio_packet *packet =
+				(struct ljca_gpio_packet *)ljca_gpio->obuf;
 	int ret;
 
 	mutex_lock(&ljca_gpio->trans_lock);
@@ -82,43 +86,43 @@ static int gpio_config(struct ljca_gpio_dev *ljca_gpio, u8 gpio_id, u8 config)
 	packet->item[0].value = config | ljca_gpio->connect_mode[gpio_id];
 	packet->num = 1;
 
-	ret = ljca_transfer(ljca_gpio->gpio_info->ljca, LJCA_GPIO_CONFIG, packet,
-			    struct_size(packet, item, packet->num), NULL, NULL);
+	ret = ljca_transfer(ljca_gpio->ljca, LJCA_GPIO_CONFIG, packet,
+			    struct_size(packet, item, packet->num), NULL, 0);
 	mutex_unlock(&ljca_gpio->trans_lock);
-	return ret;
+
+	return ret < 0 ? ret : 0;
 }
 
 static int ljca_gpio_read(struct ljca_gpio_dev *ljca_gpio, u8 gpio_id)
 {
-	struct gpio_packet *packet = (struct gpio_packet *)ljca_gpio->obuf;
-	struct gpio_packet *ack_packet = (struct gpio_packet *)ljca_gpio->ibuf;
-	unsigned int ibuf_len = LJCA_GPIO_BUF_SIZE;
+	struct ljca_gpio_packet *ack_packet =
+				(struct ljca_gpio_packet *)ljca_gpio->ibuf;
+	struct ljca_gpio_packet *packet =
+				(struct ljca_gpio_packet *)ljca_gpio->obuf;
 	int ret;
 
 	mutex_lock(&ljca_gpio->trans_lock);
 	packet->num = 1;
 	packet->item[0].index = gpio_id;
-	ret = ljca_transfer(ljca_gpio->gpio_info->ljca, LJCA_GPIO_READ, packet,
-			    struct_size(packet, item, packet->num), ljca_gpio->ibuf, &ibuf_len);
-	if (ret)
-		goto out_unlock;
-
-	if (!ibuf_len || ack_packet->num != packet->num) {
-		dev_err(&ljca_gpio->pdev->dev, "failed gpio_id:%u %u", gpio_id, ack_packet->num);
-		ret = -EIO;
+	ret = ljca_transfer(ljca_gpio->ljca, LJCA_GPIO_READ, packet,
+			    struct_size(packet, item, packet->num),
+			    ljca_gpio->ibuf, LJCA_GPIO_BUF_SIZE);
+
+	if (ret <= 0 || ack_packet->num != packet->num) {
+		dev_err(&ljca_gpio->ljca->auxdev.dev,
+			"read package error, gpio_id: %u num: %u ret: %d\n",
+			gpio_id, ack_packet->num, ret);
+		ret = ret < 0 ? ret : -EIO;
 	}
-
-out_unlock:
 	mutex_unlock(&ljca_gpio->trans_lock);
-	if (ret)
-		return ret;
-	return ack_packet->item[0].value > 0;
+
+	return ret < 0 ? ret : ack_packet->item[0].value > 0;
 }
 
-static int ljca_gpio_write(struct ljca_gpio_dev *ljca_gpio, u8 gpio_id,
-			   int value)
+static int ljca_gpio_write(struct ljca_gpio_dev *ljca_gpio, u8 gpio_id, int value)
 {
-	struct gpio_packet *packet = (struct gpio_packet *)ljca_gpio->obuf;
+	struct ljca_gpio_packet *packet =
+			(struct ljca_gpio_packet *)ljca_gpio->obuf;
 	int ret;
 
 	mutex_lock(&ljca_gpio->trans_lock);
@@ -126,10 +130,11 @@ static int ljca_gpio_write(struct ljca_gpio_dev *ljca_gpio, u8 gpio_id,
 	packet->item[0].index = gpio_id;
 	packet->item[0].value = value & 1;
 
-	ret = ljca_transfer(ljca_gpio->gpio_info->ljca, LJCA_GPIO_WRITE, packet,
-			    struct_size(packet, item, packet->num), NULL, NULL);
+	ret = ljca_transfer(ljca_gpio->ljca, LJCA_GPIO_WRITE, packet,
+			    struct_size(packet, item, packet->num), NULL, 0);
 	mutex_unlock(&ljca_gpio->trans_lock);
-	return ret;
+
+	return ret < 0 ? ret : 0;
 }
 
 static int ljca_gpio_get_value(struct gpio_chip *chip, unsigned int offset)
@@ -147,16 +152,24 @@ static void ljca_gpio_set_value(struct gpio_chip *chip, unsigned int offset,
 
 	ret = ljca_gpio_write(ljca_gpio, offset, val);
 	if (ret)
-		dev_err(chip->parent, "offset:%u val:%d set value failed %d\n", offset, val, ret);
+		dev_err(chip->parent,
+			"set value failed offset: %u val: %d ret: %d\n",
+			offset, val, ret);
 }
 
-static int ljca_gpio_direction_input(struct gpio_chip *chip,
-				     unsigned int offset)
+static int ljca_gpio_direction_input(struct gpio_chip *chip, unsigned int offset)
 {
 	struct ljca_gpio_dev *ljca_gpio = gpiochip_get_data(chip);
 	u8 config = LJCA_GPIO_CONF_INPUT | LJCA_GPIO_CONF_CLR;
+	int ret;
 
-	return gpio_config(ljca_gpio, offset, config);
+	ret = ljca_gpio_config(ljca_gpio, offset, config);
+	if (ret)
+		return ret;
+
+	clear_bit(offset, ljca_gpio->output_enabled);
+
+	return 0;
 }
 
 static int ljca_gpio_direction_output(struct gpio_chip *chip,
@@ -166,14 +179,26 @@ static int ljca_gpio_direction_output(struct gpio_chip *chip,
 	u8 config = LJCA_GPIO_CONF_OUTPUT | LJCA_GPIO_CONF_CLR;
 	int ret;
 
-	ret = gpio_config(ljca_gpio, offset, config);
+	ret = ljca_gpio_config(ljca_gpio, offset, config);
 	if (ret)
 		return ret;
 
 	ljca_gpio_set_value(chip, offset, val);
+	set_bit(offset, ljca_gpio->output_enabled);
+
 	return 0;
 }
 
+static int ljca_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
+{
+	struct ljca_gpio_dev *ljca_gpio = gpiochip_get_data(chip);
+
+	if (test_bit(offset, ljca_gpio->output_enabled))
+		return GPIO_LINE_DIRECTION_OUT;
+
+	return GPIO_LINE_DIRECTION_IN;
+}
+
 static int ljca_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
 				unsigned long config)
 {
@@ -197,7 +222,8 @@ static int ljca_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
 	return 0;
 }
 
-static int ljca_gpio_init_valid_mask(struct gpio_chip *chip, unsigned long *valid_mask,
+static int ljca_gpio_init_valid_mask(struct gpio_chip *chip,
+				     unsigned long *valid_mask,
 				     unsigned int ngpios)
 {
 	struct ljca_gpio_dev *ljca_gpio = gpiochip_get_data(chip);
@@ -208,15 +234,18 @@ static int ljca_gpio_init_valid_mask(struct gpio_chip *chip, unsigned long *vali
 	return 0;
 }
 
-static void ljca_gpio_irq_init_valid_mask(struct gpio_chip *chip, unsigned long *valid_mask,
+static void ljca_gpio_irq_init_valid_mask(struct gpio_chip *chip,
+					  unsigned long *valid_mask,
 					  unsigned int ngpios)
 {
 	ljca_gpio_init_valid_mask(chip, valid_mask, ngpios);
 }
 
-static int ljca_enable_irq(struct ljca_gpio_dev *ljca_gpio, int gpio_id, bool enable)
+static int ljca_enable_irq(struct ljca_gpio_dev *ljca_gpio, int gpio_id,
+			   bool enable)
 {
-	struct gpio_packet *packet = (struct gpio_packet *)ljca_gpio->obuf;
+	struct ljca_gpio_packet *packet =
+			(struct ljca_gpio_packet *)ljca_gpio->obuf;
 	int ret;
 
 	mutex_lock(&ljca_gpio->trans_lock);
@@ -224,18 +253,20 @@ static int ljca_enable_irq(struct ljca_gpio_dev *ljca_gpio, int gpio_id, bool en
 	packet->item[0].index = gpio_id;
 	packet->item[0].value = 0;
 
-	ret = ljca_transfer(ljca_gpio->gpio_info->ljca,
-			    enable ? LJCA_GPIO_INT_UNMASK : LJCA_GPIO_INT_MASK, packet,
-			    struct_size(packet, item, packet->num), NULL, NULL);
+	ret = ljca_transfer(ljca_gpio->ljca,
+			    enable ? LJCA_GPIO_INT_UNMASK : LJCA_GPIO_INT_MASK,
+			    packet, struct_size(packet, item, packet->num),
+			    NULL, 0);
 	mutex_unlock(&ljca_gpio->trans_lock);
-	return ret;
+
+	return ret < 0 ? ret : 0;
 }
 
 static void ljca_gpio_async(struct work_struct *work)
 {
-	struct ljca_gpio_dev *ljca_gpio = container_of(work, struct ljca_gpio_dev, work);
-	int gpio_id;
-	int unmasked;
+	struct ljca_gpio_dev *ljca_gpio =
+			container_of(work, struct ljca_gpio_dev, work);
+	int gpio_id, unmasked;
 
 	for_each_set_bit(gpio_id, ljca_gpio->reenable_irqs, ljca_gpio->gc.ngpio) {
 		clear_bit(gpio_id, ljca_gpio->reenable_irqs);
@@ -245,20 +276,22 @@ static void ljca_gpio_async(struct work_struct *work)
 	}
 }
 
-static void ljca_gpio_event_cb(void *context, u8 cmd, const void *evt_data, int len)
+static void ljca_gpio_event_cb(void *context, u8 cmd, const void *evt_data,
+			       int len)
 {
-	const struct gpio_packet *packet = evt_data;
+	const struct ljca_gpio_packet *packet = evt_data;
 	struct ljca_gpio_dev *ljca_gpio = context;
-	int i;
-	int irq;
+	int i, irq;
 
 	if (cmd != LJCA_GPIO_INT_EVENT)
 		return;
 
 	for (i = 0; i < packet->num; i++) {
-		irq = irq_find_mapping(ljca_gpio->gc.irq.domain, packet->item[i].index);
+		irq = irq_find_mapping(ljca_gpio->gc.irq.domain,
+				       packet->item[i].index);
 		if (!irq) {
-			dev_err(ljca_gpio->gc.parent, "gpio_id %u does not mapped to IRQ yet\n",
+			dev_err(ljca_gpio->gc.parent,
+				"gpio_id %u does not mapped to IRQ yet\n",
 				packet->item[i].index);
 			return;
 		}
@@ -299,18 +332,22 @@ static int ljca_irq_set_type(struct irq_data *irqd, unsigned int type)
 	ljca_gpio->connect_mode[gpio_id] = LJCA_GPIO_CONF_INTERRUPT;
 	switch (type) {
 	case IRQ_TYPE_LEVEL_HIGH:
-		ljca_gpio->connect_mode[gpio_id] |= (LJCA_GPIO_CONF_LEVEL | LJCA_GPIO_CONF_PULLUP);
+		ljca_gpio->connect_mode[gpio_id] |=
+			(LJCA_GPIO_CONF_LEVEL | LJCA_GPIO_CONF_PULLUP);
 		break;
 	case IRQ_TYPE_LEVEL_LOW:
-		ljca_gpio->connect_mode[gpio_id] |= (LJCA_GPIO_CONF_LEVEL | LJCA_GPIO_CONF_PULLDOWN);
+		ljca_gpio->connect_mode[gpio_id] |=
+			(LJCA_GPIO_CONF_LEVEL | LJCA_GPIO_CONF_PULLDOWN);
 		break;
 	case IRQ_TYPE_EDGE_BOTH:
 		break;
 	case IRQ_TYPE_EDGE_RISING:
-		ljca_gpio->connect_mode[gpio_id] |= (LJCA_GPIO_CONF_EDGE | LJCA_GPIO_CONF_PULLUP);
+		ljca_gpio->connect_mode[gpio_id] |=
+			(LJCA_GPIO_CONF_EDGE | LJCA_GPIO_CONF_PULLUP);
 		break;
 	case IRQ_TYPE_EDGE_FALLING:
-		ljca_gpio->connect_mode[gpio_id] |= (LJCA_GPIO_CONF_EDGE | LJCA_GPIO_CONF_PULLDOWN);
+		ljca_gpio->connect_mode[gpio_id] |=
+			(LJCA_GPIO_CONF_EDGE | LJCA_GPIO_CONF_PULLDOWN);
 		break;
 	default:
 		return -EINVAL;
@@ -332,15 +369,14 @@ static void ljca_irq_bus_unlock(struct irq_data *irqd)
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
 	struct ljca_gpio_dev *ljca_gpio = gpiochip_get_data(gc);
 	int gpio_id = irqd_to_hwirq(irqd);
-	int enabled;
-	int unmasked;
+	int enabled, unmasked;
 
 	enabled = test_bit(gpio_id, ljca_gpio->enabled_irqs);
 	unmasked = test_bit(gpio_id, ljca_gpio->unmasked_irqs);
 
 	if (enabled != unmasked) {
 		if (unmasked) {
-			gpio_config(ljca_gpio, gpio_id, 0);
+			ljca_gpio_config(ljca_gpio, gpio_id, 0);
 			ljca_enable_irq(ljca_gpio, gpio_id, true);
 			set_bit(gpio_id, ljca_gpio->enabled_irqs);
 		} else {
@@ -363,43 +399,48 @@ static const struct irq_chip ljca_gpio_irqchip = {
 	GPIOCHIP_IRQ_RESOURCE_HELPERS,
 };
 
-static int ljca_gpio_probe(struct platform_device *pdev)
+static int ljca_gpio_probe(struct auxiliary_device *auxdev,
+			   const struct auxiliary_device_id *aux_dev_id)
 {
+	struct ljca_client *ljca = auxiliary_dev_to_ljca_client(auxdev);
 	struct ljca_gpio_dev *ljca_gpio;
 	struct gpio_irq_chip *girq;
 	int ret;
 
-	ljca_gpio = devm_kzalloc(&pdev->dev, sizeof(*ljca_gpio), GFP_KERNEL);
+	ljca_gpio = devm_kzalloc(&auxdev->dev, sizeof(*ljca_gpio), GFP_KERNEL);
 	if (!ljca_gpio)
 		return -ENOMEM;
 
-	ljca_gpio->gpio_info = dev_get_platdata(&pdev->dev);
-	ljca_gpio->connect_mode = devm_kcalloc(&pdev->dev, ljca_gpio->gpio_info->num,
-					       sizeof(*ljca_gpio->connect_mode), GFP_KERNEL);
+	ljca_gpio->ljca = ljca;
+	ljca_gpio->gpio_info = dev_get_platdata(&auxdev->dev);
+	ljca_gpio->connect_mode = devm_kcalloc(&auxdev->dev,
+					       ljca_gpio->gpio_info->num,
+					       sizeof(*ljca_gpio->connect_mode),
+					       GFP_KERNEL);
 	if (!ljca_gpio->connect_mode)
 		return -ENOMEM;
 
 	mutex_init(&ljca_gpio->irq_lock);
 	mutex_init(&ljca_gpio->trans_lock);
-	ljca_gpio->pdev = pdev;
 	ljca_gpio->gc.direction_input = ljca_gpio_direction_input;
 	ljca_gpio->gc.direction_output = ljca_gpio_direction_output;
+	ljca_gpio->gc.get_direction = ljca_gpio_get_direction;
 	ljca_gpio->gc.get = ljca_gpio_get_value;
 	ljca_gpio->gc.set = ljca_gpio_set_value;
 	ljca_gpio->gc.set_config = ljca_gpio_set_config;
 	ljca_gpio->gc.init_valid_mask = ljca_gpio_init_valid_mask;
 	ljca_gpio->gc.can_sleep = true;
-	ljca_gpio->gc.parent = &pdev->dev;
+	ljca_gpio->gc.parent = &auxdev->dev;
 
 	ljca_gpio->gc.base = -1;
 	ljca_gpio->gc.ngpio = ljca_gpio->gpio_info->num;
-	ljca_gpio->gc.label = ACPI_COMPANION(&pdev->dev) ?
-			      acpi_dev_name(ACPI_COMPANION(&pdev->dev)) :
-			      dev_name(&pdev->dev);
+	ljca_gpio->gc.label = ACPI_COMPANION(&auxdev->dev) ?
+			      acpi_dev_name(ACPI_COMPANION(&auxdev->dev)) :
+			      dev_name(&auxdev->dev);
 	ljca_gpio->gc.owner = THIS_MODULE;
 
-	platform_set_drvdata(pdev, ljca_gpio);
-	ljca_register_event_cb(ljca_gpio->gpio_info->ljca, ljca_gpio_event_cb, ljca_gpio);
+	auxiliary_set_drvdata(auxdev, ljca_gpio);
+	ljca_register_event_cb(ljca, ljca_gpio_event_cb, ljca_gpio);
 
 	girq = &ljca_gpio->gc.irq;
 	gpio_irq_chip_set_chip(girq, &ljca_gpio_irqchip);
@@ -413,7 +454,7 @@ static int ljca_gpio_probe(struct platform_device *pdev)
 	INIT_WORK(&ljca_gpio->work, ljca_gpio_async);
 	ret = gpiochip_add_data(&ljca_gpio->gc, ljca_gpio);
 	if (ret) {
-		ljca_unregister_event_cb(ljca_gpio->gpio_info->ljca);
+		ljca_unregister_event_cb(ljca);
 		mutex_destroy(&ljca_gpio->irq_lock);
 		mutex_destroy(&ljca_gpio->trans_lock);
 	}
@@ -421,32 +462,31 @@ static int ljca_gpio_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int ljca_gpio_remove(struct platform_device *pdev)
+static void ljca_gpio_remove(struct auxiliary_device *auxdev)
 {
-	struct ljca_gpio_dev *ljca_gpio = platform_get_drvdata(pdev);
+	struct ljca_gpio_dev *ljca_gpio = auxiliary_get_drvdata(auxdev);
 
 	gpiochip_remove(&ljca_gpio->gc);
-	ljca_unregister_event_cb(ljca_gpio->gpio_info->ljca);
+	ljca_unregister_event_cb(ljca_gpio->ljca);
+	cancel_work_sync(&ljca_gpio->work);
 	mutex_destroy(&ljca_gpio->irq_lock);
 	mutex_destroy(&ljca_gpio->trans_lock);
-	return 0;
 }
 
-#define LJCA_GPIO_DRV_NAME "ljca-gpio"
-static const struct platform_device_id ljca_gpio_id[] = {
-	{ LJCA_GPIO_DRV_NAME, 0 },
-	{ /* sentinel */ }
+static const struct auxiliary_device_id ljca_gpio_id_table[] = {
+	{ "usb_ljca.ljca-gpio", 0 },
+	{ /* sentinel */ },
 };
-MODULE_DEVICE_TABLE(platform, ljca_gpio_id);
+MODULE_DEVICE_TABLE(auxiliary, ljca_gpio_id_table);
 
-static struct platform_driver ljca_gpio_driver = {
-	.driver.name = LJCA_GPIO_DRV_NAME,
+static struct auxiliary_driver ljca_gpio_driver = {
 	.probe = ljca_gpio_probe,
 	.remove = ljca_gpio_remove,
+	.id_table = ljca_gpio_id_table,
 };
-module_platform_driver(ljca_gpio_driver);
+module_auxiliary_driver(ljca_gpio_driver);
 
-MODULE_AUTHOR("Ye Xiang <xiang.ye@intel.com>");
+MODULE_AUTHOR("Wu Wentong <wentong.wu@intel.com>");
 MODULE_AUTHOR("Wang Zhifeng <zhifeng.wang@intel.com>");
 MODULE_AUTHOR("Zhang Lixu <lixu.zhang@intel.com>");
 MODULE_DESCRIPTION("Intel La Jolla Cove Adapter USB-GPIO driver");
-- 
2.7.4

Re: [PATCH v9 4/4] gpio: update Intel LJCA USB GPIO driver

[Linus Walleij]

On Thu, Aug 17, 2023 at 12:32 AM Wentong Wu <wentong.wu@intel.com> wrote:

> This driver communicate with LJCA GPIO module with specific
> protocol through interfaces exported by LJCA USB driver.
> Update the driver according to LJCA USB driver's changes.
>
> Signed-off-by: Wentong Wu <wentong.wu@intel.com>
> Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>

This patch does several things at the same time, consider the
"one technical step per patch" approach, for some definition
of a "technical step". The upside is that git bisect gets better
precision when something goes sidewise.

However it's no big deal for me as I'm not hung up on process
and I bet you have tested the result, so:
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

RE: [PATCH v9 4/4] gpio: update Intel LJCA USB GPIO driver

[Wu, Wentong]

> From: Linus Walleij <linus.walleij@linaro.org>
> 
> On Thu, Aug 17, 2023 at 12:32 AM Wentong Wu <wentong.wu@intel.com>
> wrote:
> 
> > This driver communicate with LJCA GPIO module with specific protocol
> > through interfaces exported by LJCA USB driver.
> > Update the driver according to LJCA USB driver's changes.
> >
> > Signed-off-by: Wentong Wu <wentong.wu@intel.com>
> > Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> 
> This patch does several things at the same time, consider the "one technical step
> per patch" approach, for some definition of a "technical step". The upside is that
> git bisect gets better precision when something goes sidewise.

Ack, thanks. I will follow this going forward.

> 
> However it's no big deal for me as I'm not hung up on process and
> I bet you have tested the result, so:

Definitely, I have tested this patch sets dozens of times.

> Acked-by: Linus Walleij <linus.walleij@linaro.org>

Thanks a lot

BR,
Wentong
> 
> Yours,
> Linus Walleij

Re: [PATCH v9 2/4] i2c: Add support for Intel LJCA USB I2C driver

[Andi Shyti]

Hi Weontong,

patch looks good, just few minor comments.

[...]

> +static u8 ljca_i2c_format_slave_addr(u8 slave_addr, u8 type)
> +{
> +	return (slave_addr << 1) | type;
> +}

this is used only once, you could eventually ger rid of it and
use explicitely in the start function.

[...]

> +static int ljca_i2c_start(struct ljca_i2c_dev *ljca_i2c, u8 slave_addr,
> +			  enum ljca_xfer_type type)
> +{
> +	struct ljca_i2c_rw_packet *w_packet =
> +			(struct ljca_i2c_rw_packet *)ljca_i2c->obuf;
> +	struct ljca_i2c_rw_packet *r_packet =
> +			(struct ljca_i2c_rw_packet *)ljca_i2c->ibuf;
> +	s16 rp_len;
> +	int ret;
> +
> +	memset(w_packet, 0, sizeof(*w_packet));

do you really need to set this to '0' as you are initializing the
header

> +	w_packet->id = ljca_i2c->i2c_info->id;
> +	w_packet->len = cpu_to_le16(sizeof(*w_packet->data));
> +	w_packet->data[0] = ljca_i2c_format_slave_addr(slave_addr, type);
> +
> +	ret = ljca_transfer(ljca_i2c->ljca, LJCA_I2C_START, w_packet,
> +			    struct_size(w_packet, data, 1), r_packet,
> +			    LJCA_I2C_BUF_SIZE);
> +	if (ret < 0)
> +		return ret;
> +	else if (ret < sizeof(*r_packet))
> +		return -EIO;
> +
> +	rp_len = le16_to_cpu(r_packet->len);
> +	if (rp_len < 0 || r_packet->id != w_packet->id) {
> +		dev_err(&ljca_i2c->adap.dev,
> +			"i2c start failed len: %d id: %d %d\n",
> +			rp_len, r_packet->id, w_packet->id);
> +		return -EIO;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ljca_i2c_stop(struct ljca_i2c_dev *ljca_i2c, u8 slave_addr)

the return value here is not used anywhere, what about making
this function void and printing a warning in case of failure?

[...]

> +static int ljca_i2c_probe(struct auxiliary_device *auxdev,
> +			  const struct auxiliary_device_id *aux_dev_id)
> +{

[...]

> +	ret = ljca_i2c_init(ljca_i2c, ljca_i2c->i2c_info->id);
> +	if (ret) {
> +		dev_err(&auxdev->dev, "i2c init failed id: %d\n",
> +			ljca_i2c->i2c_info->id);
> +		return -EIO;

please use dev_err_probe();

Andi

RE: [PATCH v9 2/4] i2c: Add support for Intel LJCA USB I2C driver

[Wu, Wentong]

Hi Andi,

Thanks for your review. 

> From: Andi Shyti <andi.shyti@kernel.org>
> 
> Hi Weontong,
> 
> patch looks good, just few minor comments.

Thanks, and below comments will be fixed in next version patch set.

BR,
Wentong

> 
> [...]
> 
> > +static u8 ljca_i2c_format_slave_addr(u8 slave_addr, u8 type) {
> > +	return (slave_addr << 1) | type;
> > +}
> 
> this is used only once, you could eventually ger rid of it and use explicitely in the
> start function.

Ack, thanks.

> 
> [...]
> 
> > +static int ljca_i2c_start(struct ljca_i2c_dev *ljca_i2c, u8 slave_addr,
> > +			  enum ljca_xfer_type type)
> > +{
> > +	struct ljca_i2c_rw_packet *w_packet =
> > +			(struct ljca_i2c_rw_packet *)ljca_i2c->obuf;
> > +	struct ljca_i2c_rw_packet *r_packet =
> > +			(struct ljca_i2c_rw_packet *)ljca_i2c->ibuf;
> > +	s16 rp_len;
> > +	int ret;
> > +
> > +	memset(w_packet, 0, sizeof(*w_packet));
> 
> do you really need to set this to '0' as you are initializing the header

Ack, thanks

> 
> > +	w_packet->id = ljca_i2c->i2c_info->id;
> > +	w_packet->len = cpu_to_le16(sizeof(*w_packet->data));
> > +	w_packet->data[0] = ljca_i2c_format_slave_addr(slave_addr, type);
> > +
> > +	ret = ljca_transfer(ljca_i2c->ljca, LJCA_I2C_START, w_packet,
> > +			    struct_size(w_packet, data, 1), r_packet,
> > +			    LJCA_I2C_BUF_SIZE);
> > +	if (ret < 0)
> > +		return ret;
> > +	else if (ret < sizeof(*r_packet))
> > +		return -EIO;
> > +
> > +	rp_len = le16_to_cpu(r_packet->len);
> > +	if (rp_len < 0 || r_packet->id != w_packet->id) {
> > +		dev_err(&ljca_i2c->adap.dev,
> > +			"i2c start failed len: %d id: %d %d\n",
> > +			rp_len, r_packet->id, w_packet->id);
> > +		return -EIO;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int ljca_i2c_stop(struct ljca_i2c_dev *ljca_i2c, u8
> > +slave_addr)
> 
> the return value here is not used anywhere, what about making this function
> void and printing a warning in case of failure?

Ack, thanks

> 
> [...]
> 
> > +static int ljca_i2c_probe(struct auxiliary_device *auxdev,
> > +			  const struct auxiliary_device_id *aux_dev_id) {
> 
> [...]
> 
> > +	ret = ljca_i2c_init(ljca_i2c, ljca_i2c->i2c_info->id);
> > +	if (ret) {
> > +		dev_err(&auxdev->dev, "i2c init failed id: %d\n",
> > +			ljca_i2c->i2c_info->id);
> > +		return -EIO;
> 
> please use dev_err_probe();

Ack, thanks

> 
> Andi

Re: [PATCH v9 4/4] gpio: update Intel LJCA USB GPIO driver

[sakari.ailus]

Hi Wentong, Linus,

On Thu, Aug 17, 2023 at 07:07:54AM +0000, Wu, Wentong wrote:
> > From: Linus Walleij <linus.walleij@linaro.org>
> > 
> > On Thu, Aug 17, 2023 at 12:32 AM Wentong Wu <wentong.wu@intel.com>
> > wrote:
> > 
> > > This driver communicate with LJCA GPIO module with specific protocol
> > > through interfaces exported by LJCA USB driver.
> > > Update the driver according to LJCA USB driver's changes.
> > >
> > > Signed-off-by: Wentong Wu <wentong.wu@intel.com>
> > > Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > 
> > This patch does several things at the same time, consider the "one technical step
> > per patch" approach, for some definition of a "technical step". The upside is that
> > git bisect gets better precision when something goes sidewise.
> 
> Ack, thanks. I will follow this going forward.

The old LJCA GPIO driver got added without the rest of the LJCA, including
the main driver (now 1st patch of this set). I might have just reverted the
patch that added the old one and put the new one on top.

The old driver was never usable AFAIU and there are many changes as Linus
noted. It would be easier to review as new driver.

I wonder what others think.

-- 
Regards,

Sakari Ailus

RE: [PATCH v9 4/4] gpio: update Intel LJCA USB GPIO driver

[Wu, Wentong]

> From: sakari.ailus@linux.intel.com <sakari.ailus@linux.intel.com>
> 
> Hi Wentong, Linus,
> 
> On Thu, Aug 17, 2023 at 07:07:54AM +0000, Wu, Wentong wrote:
> > > From: Linus Walleij <linus.walleij@linaro.org>
> > >
> > > On Thu, Aug 17, 2023 at 12:32 AM Wentong Wu <wentong.wu@intel.com>
> > > wrote:
> > >
> > > > This driver communicate with LJCA GPIO module with specific
> > > > protocol through interfaces exported by LJCA USB driver.
> > > > Update the driver according to LJCA USB driver's changes.
> > > >
> > > > Signed-off-by: Wentong Wu <wentong.wu@intel.com>
> > > > Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > >
> > > This patch does several things at the same time, consider the "one
> > > technical step per patch" approach, for some definition of a
> > > "technical step". The upside is that git bisect gets better precision when
> something goes sidewise.
> >
> > Ack, thanks. I will follow this going forward.
> 
> The old LJCA GPIO driver got added without the rest of the LJCA, including the
> main driver (now 1st patch of this set). I might have just reverted the patch that
> added the old one and put the new one on top.
> 
> The old driver was never usable AFAIU and there are many changes as Linus
> noted. It would be easier to review as new driver.
> 
> I wonder what others think.

Agree, Thanks

BR,
Wentong
> 
> --
> Regards,
> 
> Sakari Ailus