linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v14 0/4] Add Intel LJCA device driver
@ 2023-09-04  5:44 Wentong Wu
  2023-09-04  5:44 ` [PATCH v14 1/4] usb: Add support for Intel LJCA device Wentong Wu
                   ` (5 more replies)
  0 siblings, 6 replies; 19+ messages in thread
From: Wentong Wu @ 2023-09-04  5:44 UTC (permalink / raw)
  To: gregkh, arnd, mka, oneukum, lee, wsa, kfting, broonie,
	linus.walleij, hdegoede, maz, brgl, linux-usb, linux-i2c,
	linux-spi, linux-gpio, andriy.shevchenko, heikki.krogerus,
	andi.shyti, sakari.ailus, bartosz.golaszewski,
	srinivas.pandruvada
  Cc: zhifeng.wang, Wentong Wu

Add driver for Intel La Jolla Cove Adapter (LJCA) device. This
IO-expander expands additional functions to the host system
such as GPIO, I2C and SPI with USB host interface. We add 4
drivers to support this device: a USB driver, a GPIO chip driver,
a I2C controller driver and a SPI controller driver.

---
v14:
 - fix build error: implicit declaration of function 'acpi_dev_clear_dependencies'

v13:
 - make ljca-usb more robust with the help of Hans de Goede
 - call acpi_dev_clear_dependencies() to mark _DEP ACPI dependencies on the I2C controller as satisfied, and patch is from Hans de Goede

v12:
 - switch dev_err to dev_dbg for i2c-ljca driver
 - avoid err printing because of calling usb_kill_urb when attempts to resubmit the rx urb

v11:
 - switch dev_err to dev_dbg for i2c-ljca driver
 - remove message length check because of defined quirk structure
 - remove I2C_FUNC_SMBUS_EMUL support

v10:
 - remove ljca_i2c_format_slave_addr
 - remove memset before write write w_packet
 - make ljca_i2c_stop void and print err message in case failure
 - use dev_err_probe in ljca_i2c_probe function

v9:
 - overhaul usb-ljca driver to make it more structured and easy understand
 - 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      | 246 +++++++------
 drivers/i2c/busses/Kconfig    |  11 +
 drivers/i2c/busses/Makefile   |   1 +
 drivers/i2c/busses/i2c-ljca.c | 334 +++++++++++++++++
 drivers/spi/Kconfig           |  11 +
 drivers/spi/Makefile          |   1 +
 drivers/spi/spi-ljca.c        | 297 +++++++++++++++
 drivers/usb/misc/Kconfig      |  14 +
 drivers/usb/misc/Makefile     |   1 +
 drivers/usb/misc/usb-ljca.c   | 834 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/ljca.h      | 113 ++++++
 12 files changed, 1762 insertions(+), 105 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


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

* [PATCH v14 1/4] usb: Add support for Intel LJCA device
  2023-09-04  5:44 [PATCH v14 0/4] Add Intel LJCA device driver Wentong Wu
@ 2023-09-04  5:44 ` Wentong Wu
  2023-09-04  7:45   ` Hans de Goede
  2023-09-04 10:33   ` Oliver Neukum
  2023-09-04  5:44 ` [PATCH v14 2/4] i2c: Add support for Intel LJCA USB I2C driver Wentong Wu
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 19+ messages in thread
From: Wentong Wu @ 2023-09-04  5:44 UTC (permalink / raw)
  To: gregkh, arnd, mka, oneukum, lee, wsa, kfting, broonie,
	linus.walleij, hdegoede, maz, brgl, linux-usb, linux-i2c,
	linux-spi, linux-gpio, andriy.shevchenko, heikki.krogerus,
	andi.shyti, sakari.ailus, bartosz.golaszewski,
	srinivas.pandruvada
  Cc: zhifeng.wang, 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>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Tested-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/usb/misc/Kconfig    |  14 +
 drivers/usb/misc/Makefile   |   1 +
 drivers/usb/misc/usb-ljca.c | 834 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/ljca.h    | 113 ++++++
 4 files changed, 962 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..942e850
--- /dev/null
+++ b/drivers/usb/misc/usb-ljca.c
@@ -0,0 +1,834 @@
+// 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 && ret != -EPERM)
+		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 ljca_match_ids_walk_data {
+	const struct acpi_device_id *ids;
+	const char *uid;
+	struct acpi_device *adev;
+};
+
+static const struct acpi_device_id ljca_gpio_hids[] = {
+	{ "INTC1074" },
+	{ "INTC1096" },
+	{ "INTC100B" },
+	{ "INTC10D1" },
+	{},
+};
+
+static const struct acpi_device_id ljca_i2c_hids[] = {
+	{ "INTC1075" },
+	{ "INTC1097" },
+	{ "INTC100C" },
+	{ "INTC10D2" },
+	{},
+};
+
+static const struct acpi_device_id ljca_spi_hids[] = {
+	{ "INTC1091" },
+	{ "INTC1098" },
+	{ "INTC100D" },
+	{ "INTC10D3" },
+	{},
+};
+
+static int ljca_match_device_ids(struct acpi_device *adev, void *data)
+{
+	struct ljca_match_ids_walk_data *wd = data;
+	const char *uid = acpi_device_uid(adev);
+
+	if (acpi_match_device_ids(adev, wd->ids))
+		return 0;
+
+	if (!wd->uid)
+		goto match;
+
+	if (!uid)
+		uid = "0";
+	else
+		uid = memchr(uid, wd->uid[0], strlen(uid));
+
+	if (!uid || strcmp(uid, wd->uid))
+		return 0;
+
+match:
+	wd->adev = adev;
+
+	return 1;
+}
+
+/* bind auxiliary device to acpi device */
+static void ljca_auxdev_acpi_bind(struct ljca_adapter *adap,
+				  struct auxiliary_device *auxdev,
+				  u64 adr, u8 id)
+{
+	struct ljca_match_ids_walk_data wd = { 0 };
+	struct acpi_device *parent, *adev;
+	struct device *dev = adap->dev;
+	char uid[4];
+
+	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 = ljca_gpio_hids;
+		break;
+	case LJCA_I2C1_ACPI_ADR:
+	case LJCA_I2C2_ACPI_ADR:
+		snprintf(uid, sizeof(uid), "%d", id);
+		wd.uid = uid;
+		wd.ids = ljca_i2c_hids;
+		break;
+	case LJCA_SPI1_ACPI_ADR:
+	case LJCA_SPI2_ACPI_ADR:
+		wd.ids = ljca_spi_hids;
+		break;
+	default:
+		dev_warn(dev, "unsupported _ADR\n");
+		return;
+	}
+
+	acpi_dev_for_each_child(parent, ljca_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, ljca_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, u8 id)
+{
+}
+#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, id);
+
+	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++;
+
+	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


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

* [PATCH v14 2/4] i2c: Add support for Intel LJCA USB I2C driver
  2023-09-04  5:44 [PATCH v14 0/4] Add Intel LJCA device driver Wentong Wu
  2023-09-04  5:44 ` [PATCH v14 1/4] usb: Add support for Intel LJCA device Wentong Wu
@ 2023-09-04  5:44 ` Wentong Wu
  2023-09-04  5:44 ` [PATCH v14 3/4] spi: Add support for Intel LJCA USB SPI driver Wentong Wu
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: Wentong Wu @ 2023-09-04  5:44 UTC (permalink / raw)
  To: gregkh, arnd, mka, oneukum, lee, wsa, kfting, broonie,
	linus.walleij, hdegoede, maz, brgl, linux-usb, linux-i2c,
	linux-spi, linux-gpio, andriy.shevchenko, heikki.krogerus,
	andi.shyti, sakari.ailus, bartosz.golaszewski,
	srinivas.pandruvada
  Cc: zhifeng.wang, 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>
Tested-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/i2c/busses/Kconfig    |  11 ++
 drivers/i2c/busses/Makefile   |   1 +
 drivers/i2c/busses/i2c-ljca.c | 334 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 346 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..679a4e0
--- /dev/null
+++ b/drivers/i2c/busses/i2c-ljca.c
@@ -0,0 +1,334 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Intel La Jolla Cove Adapter USB-I2C driver
+ *
+ * Copyright (c) 2023, Intel Corporation.
+ */
+
+#include <linux/acpi.h>
+#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 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;
+
+	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);
+
+	return ret < 0 ? ret : 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;
+
+	w_packet->id = ljca_i2c->i2c_info->id;
+	w_packet->len = cpu_to_le16(sizeof(*w_packet->data));
+	w_packet->data[0] = (slave_addr << 1) | 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 || ret < sizeof(*r_packet))
+		return ret < 0 ? ret : -EIO;
+
+	rp_len = le16_to_cpu(r_packet->len);
+	if (rp_len < 0 || r_packet->id != w_packet->id) {
+		dev_dbg(&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 void 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;
+
+	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 || ret < sizeof(*r_packet)) {
+		dev_dbg(&ljca_i2c->adap.dev,
+			"i2c stop failed ret: %d id: %d\n",
+			ret, w_packet->id);
+		return;
+	}
+
+	rp_len = le16_to_cpu(r_packet->len);
+	if (rp_len < 0 || r_packet->id != w_packet->id)
+		dev_dbg(&ljca_i2c->adap.dev,
+			"i2c stop failed len: %d id: %d %d\n",
+			rp_len, r_packet->id, w_packet->id);
+}
+
+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;
+
+	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 || ret < sizeof(*r_packet))
+		return ret < 0 ? ret : -EIO;
+
+	rp_len = le16_to_cpu(r_packet->len);
+	if (rp_len != len || r_packet->id != w_packet->id) {
+		dev_dbg(&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;
+
+	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 || ret < sizeof(*r_packet))
+		return ret < 0 ? ret : -EIO;
+
+	rplen = le16_to_cpu(r_packet->len);
+	if (rplen != len || r_packet->id != w_packet->id) {
+		dev_dbg(&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;
+}
+
+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)
+		return dev_err_probe(&auxdev->dev, -EIO,
+				     "i2c init failed id: %d\n",
+				     ljca_i2c->i2c_info->id);
+
+	ret = devm_i2c_add_adapter(&auxdev->dev, &ljca_i2c->adap);
+	if (ret)
+		return ret;
+
+	if (has_acpi_companion(&ljca_i2c->adap.dev))
+		acpi_dev_clear_dependencies(ACPI_COMPANION(&ljca_i2c->adap.dev));
+
+	return 0;
+}
+
+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


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

* [PATCH v14 3/4] spi: Add support for Intel LJCA USB SPI driver
  2023-09-04  5:44 [PATCH v14 0/4] Add Intel LJCA device driver Wentong Wu
  2023-09-04  5:44 ` [PATCH v14 1/4] usb: Add support for Intel LJCA device Wentong Wu
  2023-09-04  5:44 ` [PATCH v14 2/4] i2c: Add support for Intel LJCA USB I2C driver Wentong Wu
@ 2023-09-04  5:44 ` Wentong Wu
  2023-09-04  5:44 ` [PATCH v14 4/4] gpio: update Intel LJCA USB GPIO driver Wentong Wu
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: Wentong Wu @ 2023-09-04  5:44 UTC (permalink / raw)
  To: gregkh, arnd, mka, oneukum, lee, wsa, kfting, broonie,
	linus.walleij, hdegoede, maz, brgl, linux-usb, linux-i2c,
	linux-spi, linux-gpio, andriy.shevchenko, heikki.krogerus,
	andi.shyti, sakari.ailus, bartosz.golaszewski,
	srinivas.pandruvada
  Cc: zhifeng.wang, 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>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Tested-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/spi/Kconfig    |  11 ++
 drivers/spi/Makefile   |   1 +
 drivers/spi/spi-ljca.c | 297 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 309 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..6a9ca39
--- /dev/null
+++ b/drivers/spi/spi-ljca.c
@@ -0,0 +1,297 @@
+// 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);
+
+	return ret < 0 ? ret : 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


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

* [PATCH v14 4/4] gpio: update Intel LJCA USB GPIO driver
  2023-09-04  5:44 [PATCH v14 0/4] Add Intel LJCA device driver Wentong Wu
                   ` (2 preceding siblings ...)
  2023-09-04  5:44 ` [PATCH v14 3/4] spi: Add support for Intel LJCA USB SPI driver Wentong Wu
@ 2023-09-04  5:44 ` Wentong Wu
  2023-09-04  7:38 ` [PATCH v14 0/4] Add Intel LJCA device driver Bartosz Golaszewski
  2023-09-04  7:44 ` Hans de Goede
  5 siblings, 0 replies; 19+ messages in thread
From: Wentong Wu @ 2023-09-04  5:44 UTC (permalink / raw)
  To: gregkh, arnd, mka, oneukum, lee, wsa, kfting, broonie,
	linus.walleij, hdegoede, maz, brgl, linux-usb, linux-i2c,
	linux-spi, linux-gpio, andriy.shevchenko, heikki.krogerus,
	andi.shyti, sakari.ailus, bartosz.golaszewski,
	srinivas.pandruvada
  Cc: zhifeng.wang, 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>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Tested-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpio/Kconfig     |   4 +-
 drivers/gpio/gpio-ljca.c | 246 +++++++++++++++++++++++++++--------------------
 2 files changed, 145 insertions(+), 105 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..8965677 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,34 +462,33 @@ 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("Wang Zhifeng <zhifeng.wang@intel.com>");
-MODULE_AUTHOR("Zhang Lixu <lixu.zhang@intel.com>");
+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-GPIO driver");
 MODULE_LICENSE("GPL");
 MODULE_IMPORT_NS(LJCA);
-- 
2.7.4


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

* Re: [PATCH v14 0/4] Add Intel LJCA device driver
  2023-09-04  5:44 [PATCH v14 0/4] Add Intel LJCA device driver Wentong Wu
                   ` (3 preceding siblings ...)
  2023-09-04  5:44 ` [PATCH v14 4/4] gpio: update Intel LJCA USB GPIO driver Wentong Wu
@ 2023-09-04  7:38 ` Bartosz Golaszewski
  2023-09-04  8:25   ` Wu, Wentong
  2023-09-04  7:44 ` Hans de Goede
  5 siblings, 1 reply; 19+ messages in thread
From: Bartosz Golaszewski @ 2023-09-04  7:38 UTC (permalink / raw)
  To: Wentong Wu
  Cc: gregkh, arnd, mka, oneukum, lee, wsa, kfting, broonie,
	linus.walleij, hdegoede, maz, linux-usb, linux-i2c, linux-spi,
	linux-gpio, andriy.shevchenko, heikki.krogerus, andi.shyti,
	sakari.ailus, bartosz.golaszewski, srinivas.pandruvada,
	zhifeng.wang

On Mon, Sep 4, 2023 at 7:44 AM Wentong Wu <wentong.wu@intel.com> wrote:
>
> Add driver for Intel La Jolla Cove Adapter (LJCA) device. This
> IO-expander expands additional functions to the host system
> such as GPIO, I2C and SPI with USB host interface. We add 4
> drivers to support this device: a USB driver, a GPIO chip driver,
> a I2C controller driver and a SPI controller driver.
>

Please stop spamming the list with resends. Give reviewers at least a
couple days to respond.

Bartosz

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

* Re: [PATCH v14 0/4] Add Intel LJCA device driver
  2023-09-04  5:44 [PATCH v14 0/4] Add Intel LJCA device driver Wentong Wu
                   ` (4 preceding siblings ...)
  2023-09-04  7:38 ` [PATCH v14 0/4] Add Intel LJCA device driver Bartosz Golaszewski
@ 2023-09-04  7:44 ` Hans de Goede
  2023-09-04  7:56   ` Wu, Wentong
  5 siblings, 1 reply; 19+ messages in thread
From: Hans de Goede @ 2023-09-04  7:44 UTC (permalink / raw)
  To: Wentong Wu, gregkh, arnd, mka, oneukum, lee, wsa, kfting,
	broonie, linus.walleij, maz, brgl, linux-usb, linux-i2c,
	linux-spi, linux-gpio, andriy.shevchenko, heikki.krogerus,
	andi.shyti, sakari.ailus, bartosz.golaszewski,
	srinivas.pandruvada
  Cc: zhifeng.wang

Hi,

On 9/4/23 07:44, Wentong Wu wrote:
> Add driver for Intel La Jolla Cove Adapter (LJCA) device. This
> IO-expander expands additional functions to the host system
> such as GPIO, I2C and SPI with USB host interface. We add 4
> drivers to support this device: a USB driver, a GPIO chip driver,
> a I2C controller driver and a SPI controller driver.
> 
> ---
> v14:
>  - fix build error: implicit declaration of function 'acpi_dev_clear_dependencies'
> 
> v13:
>  - make ljca-usb more robust with the help of Hans de Goede
>  - call acpi_dev_clear_dependencies() to mark _DEP ACPI dependencies on the I2C controller as satisfied, and patch is from Hans de Goede

Thank you I can confirm that v14 works well on my ThinkPad x1 yoga gen 8:

Tested-by: Hans de Goede <hdegoede@redhat.com>

Note I still have one small remark for patch 1/4, I'll reply to
the patch itself with the remark.

Regards,

Hans

> 
> v12:
>  - switch dev_err to dev_dbg for i2c-ljca driver
>  - avoid err printing because of calling usb_kill_urb when attempts to resubmit the rx urb
> 
> v11:
>  - switch dev_err to dev_dbg for i2c-ljca driver
>  - remove message length check because of defined quirk structure
>  - remove I2C_FUNC_SMBUS_EMUL support
> 
> v10:
>  - remove ljca_i2c_format_slave_addr
>  - remove memset before write write w_packet
>  - make ljca_i2c_stop void and print err message in case failure
>  - use dev_err_probe in ljca_i2c_probe function
> 
> v9:
>  - overhaul usb-ljca driver to make it more structured and easy understand
>  - 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      | 246 +++++++------
>  drivers/i2c/busses/Kconfig    |  11 +
>  drivers/i2c/busses/Makefile   |   1 +
>  drivers/i2c/busses/i2c-ljca.c | 334 +++++++++++++++++
>  drivers/spi/Kconfig           |  11 +
>  drivers/spi/Makefile          |   1 +
>  drivers/spi/spi-ljca.c        | 297 +++++++++++++++
>  drivers/usb/misc/Kconfig      |  14 +
>  drivers/usb/misc/Makefile     |   1 +
>  drivers/usb/misc/usb-ljca.c   | 834 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/usb/ljca.h      | 113 ++++++
>  12 files changed, 1762 insertions(+), 105 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
> 


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

* Re: [PATCH v14 1/4] usb: Add support for Intel LJCA device
  2023-09-04  5:44 ` [PATCH v14 1/4] usb: Add support for Intel LJCA device Wentong Wu
@ 2023-09-04  7:45   ` Hans de Goede
  2023-09-04  7:52     ` Wu, Wentong
  2023-09-04 10:33   ` Oliver Neukum
  1 sibling, 1 reply; 19+ messages in thread
From: Hans de Goede @ 2023-09-04  7:45 UTC (permalink / raw)
  To: Wentong Wu, gregkh, arnd, mka, oneukum, lee, wsa, kfting,
	broonie, linus.walleij, maz, brgl, linux-usb, linux-i2c,
	linux-spi, linux-gpio, andriy.shevchenko, heikki.krogerus,
	andi.shyti, sakari.ailus, bartosz.golaszewski,
	srinivas.pandruvada
  Cc: zhifeng.wang

Hi,

On 9/4/23 07:44, Wentong Wu wrote:
> 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>
> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
> Tested-by: Hans de Goede <hdegoede@redhat.com>
> ---

<snip>

> +#ifdef CONFIG_ACPI
> +struct ljca_match_ids_walk_data {
> +	const struct acpi_device_id *ids;
> +	const char *uid;
> +	struct acpi_device *adev;
> +};
> +
> +static const struct acpi_device_id ljca_gpio_hids[] = {
> +	{ "INTC1074" },
> +	{ "INTC1096" },
> +	{ "INTC100B" },
> +	{ "INTC10D1" },
> +	{},
> +};
> +
> +static const struct acpi_device_id ljca_i2c_hids[] = {
> +	{ "INTC1075" },
> +	{ "INTC1097" },
> +	{ "INTC100C" },
> +	{ "INTC10D2" },
> +	{},
> +};
> +
> +static const struct acpi_device_id ljca_spi_hids[] = {
> +	{ "INTC1091" },
> +	{ "INTC1098" },
> +	{ "INTC100D" },
> +	{ "INTC10D3" },
> +	{},
> +};
> +
> +static int ljca_match_device_ids(struct acpi_device *adev, void *data)
> +{
> +	struct ljca_match_ids_walk_data *wd = data;
> +	const char *uid = acpi_device_uid(adev);
> +
> +	if (acpi_match_device_ids(adev, wd->ids))
> +		return 0;
> +
> +	if (!wd->uid)
> +		goto match;
> +
> +	if (!uid)
> +		uid = "0";
> +	else
> +		uid = memchr(uid, wd->uid[0], strlen(uid));

Note this line can be simplified to:

		uid = strchr(uid, wd->uid[0]);

Regards,

Hans


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

* RE: [PATCH v14 1/4] usb: Add support for Intel LJCA device
  2023-09-04  7:45   ` Hans de Goede
@ 2023-09-04  7:52     ` Wu, Wentong
  0 siblings, 0 replies; 19+ messages in thread
From: Wu, Wentong @ 2023-09-04  7:52 UTC (permalink / raw)
  To: Hans de Goede, gregkh, arnd, mka, oneukum, lee, wsa, kfting,
	broonie, linus.walleij, maz, brgl, linux-usb, linux-i2c,
	linux-spi, linux-gpio, andriy.shevchenko, heikki.krogerus,
	andi.shyti, sakari.ailus, bartosz.golaszewski, Pandruvada,
	Srinivas
  Cc: Wang, Zhifeng

> From: Hans de Goede <hdegoede@redhat.com>
> 
> Hi,
> 
> On 9/4/23 07:44, Wentong Wu wrote:
> > 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>
> > Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
> > Tested-by: Hans de Goede <hdegoede@redhat.com>
> > ---
> > +
> > +static int ljca_match_device_ids(struct acpi_device *adev, void
> > +*data) {
> > +	struct ljca_match_ids_walk_data *wd = data;
> > +	const char *uid = acpi_device_uid(adev);
> > +
> > +	if (acpi_match_device_ids(adev, wd->ids))
> > +		return 0;
> > +
> > +	if (!wd->uid)
> > +		goto match;
> > +
> > +	if (!uid)
> > +		uid = "0";
> > +	else
> > +		uid = memchr(uid, wd->uid[0], strlen(uid));
> 
> Note this line can be simplified to:
> 
> 		uid = strchr(uid, wd->uid[0]);

Ack, this will be included in next version, thanks.

BR,
Wentong 
> 
> Regards,
> 
> Hans


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

* RE: [PATCH v14 0/4] Add Intel LJCA device driver
  2023-09-04  7:44 ` Hans de Goede
@ 2023-09-04  7:56   ` Wu, Wentong
  0 siblings, 0 replies; 19+ messages in thread
From: Wu, Wentong @ 2023-09-04  7:56 UTC (permalink / raw)
  To: Hans de Goede, gregkh, arnd, mka, oneukum, lee, wsa, kfting,
	broonie, linus.walleij, maz, brgl, linux-usb, linux-i2c,
	linux-spi, linux-gpio, andriy.shevchenko, heikki.krogerus,
	andi.shyti, sakari.ailus, bartosz.golaszewski, Pandruvada,
	Srinivas
  Cc: Wang, Zhifeng

> From: Hans de Goede <hdegoede@redhat.com>
> 
> Hi,
> 
> On 9/4/23 07:44, Wentong Wu wrote:
> > Add driver for Intel La Jolla Cove Adapter (LJCA) device. This
> > IO-expander expands additional functions to the host system such as
> > GPIO, I2C and SPI with USB host interface. We add 4 drivers to support
> > this device: a USB driver, a GPIO chip driver, a I2C controller driver
> > and a SPI controller driver.
> >
> > ---
> > v14:
> >  - fix build error: implicit declaration of function
> 'acpi_dev_clear_dependencies'
> >
> > v13:
> >  - make ljca-usb more robust with the help of Hans de Goede
> >  - call acpi_dev_clear_dependencies() to mark _DEP ACPI dependencies
> > on the I2C controller as satisfied, and patch is from Hans de Goede
> 
> Thank you I can confirm that v14 works well on my ThinkPad x1 yoga gen 8:
> 
> Tested-by: Hans de Goede <hdegoede@redhat.com>

Thanks, v14 also works well on my setup(RPL platform).

BR,
Wentong
> 
> Note I still have one small remark for patch 1/4, I'll reply to the patch itself with
> the remark.
> 
> Regards,
> 
> Hans
> 
> >
> > v12:
> >  - switch dev_err to dev_dbg for i2c-ljca driver
> >  - avoid err printing because of calling usb_kill_urb when attempts to
> > resubmit the rx urb
> >
> > v11:
> >  - switch dev_err to dev_dbg for i2c-ljca driver
> >  - remove message length check because of defined quirk structure
> >  - remove I2C_FUNC_SMBUS_EMUL support
> >
> > v10:
> >  - remove ljca_i2c_format_slave_addr
> >  - remove memset before write write w_packet
> >  - make ljca_i2c_stop void and print err message in case failure
> >  - use dev_err_probe in ljca_i2c_probe function
> >
> > v9:
> >  - overhaul usb-ljca driver to make it more structured and easy
> > understand
> >  - 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      | 246 +++++++------
> >  drivers/i2c/busses/Kconfig    |  11 +
> >  drivers/i2c/busses/Makefile   |   1 +
> >  drivers/i2c/busses/i2c-ljca.c | 334 +++++++++++++++++
> >  drivers/spi/Kconfig           |  11 +
> >  drivers/spi/Makefile          |   1 +
> >  drivers/spi/spi-ljca.c        | 297 +++++++++++++++
> >  drivers/usb/misc/Kconfig      |  14 +
> >  drivers/usb/misc/Makefile     |   1 +
> >  drivers/usb/misc/usb-ljca.c   | 834
> ++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/usb/ljca.h      | 113 ++++++
> >  12 files changed, 1762 insertions(+), 105 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

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

* RE: [PATCH v14 0/4] Add Intel LJCA device driver
  2023-09-04  7:38 ` [PATCH v14 0/4] Add Intel LJCA device driver Bartosz Golaszewski
@ 2023-09-04  8:25   ` Wu, Wentong
  0 siblings, 0 replies; 19+ messages in thread
From: Wu, Wentong @ 2023-09-04  8:25 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: gregkh, arnd, mka, oneukum, lee, wsa, kfting, broonie,
	linus.walleij, hdegoede, maz, linux-usb, linux-i2c, linux-spi,
	linux-gpio, andriy.shevchenko, heikki.krogerus, andi.shyti,
	sakari.ailus, bartosz.golaszewski, Pandruvada, Srinivas, Wang,
	Zhifeng

> From: Bartosz Golaszewski <brgl@bgdev.pl>
> 
> On Mon, Sep 4, 2023 at 7:44 AM Wentong Wu <wentong.wu@intel.com> wrote:
> >
> > Add driver for Intel La Jolla Cove Adapter (LJCA) device. This
> > IO-expander expands additional functions to the host system such as
> > GPIO, I2C and SPI with USB host interface. We add 4 drivers to support
> > this device: a USB driver, a GPIO chip driver, a I2C controller driver
> > and a SPI controller driver.
> >
> 
> Please stop spamming the list with resends. Give reviewers at least a couple days
> to respond.

Understood, thanks

BR,
Wentong
> 
> Bartosz

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

* Re: [PATCH v14 1/4] usb: Add support for Intel LJCA device
  2023-09-04  5:44 ` [PATCH v14 1/4] usb: Add support for Intel LJCA device Wentong Wu
  2023-09-04  7:45   ` Hans de Goede
@ 2023-09-04 10:33   ` Oliver Neukum
  2023-09-04 13:52     ` Wu, Wentong
  1 sibling, 1 reply; 19+ messages in thread
From: Oliver Neukum @ 2023-09-04 10:33 UTC (permalink / raw)
  To: Wentong Wu, gregkh, arnd, mka, oneukum, lee, wsa, kfting,
	broonie, linus.walleij, hdegoede, maz, brgl, linux-usb,
	linux-i2c, linux-spi, linux-gpio, andriy.shevchenko,
	heikki.krogerus, andi.shyti, sakari.ailus, bartosz.golaszewski,
	srinivas.pandruvada
  Cc: zhifeng.wang



On 04.09.23 07:44, Wentong Wu wrote:

> +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);

Let's suppose we are here, when a device is unplugged.
timeout is arbitrary as it is passed down to us.

> +		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;
> +}
> +

[..]

> +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);
> +}

And we execute this. rx_urb is killed and does nothing.
I see nothing that terminates the waiting if you hit the wrong window.
It seems like this needs to complete the waiting.

	Regards
		Oliver

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

* RE: [PATCH v14 1/4] usb: Add support for Intel LJCA device
  2023-09-04 10:33   ` Oliver Neukum
@ 2023-09-04 13:52     ` Wu, Wentong
  2023-09-04 14:06       ` Oliver Neukum
  0 siblings, 1 reply; 19+ messages in thread
From: Wu, Wentong @ 2023-09-04 13:52 UTC (permalink / raw)
  To: Oliver Neukum, gregkh, arnd, mka, lee, wsa, kfting, broonie,
	linus.walleij, hdegoede, maz, brgl, linux-usb, linux-i2c,
	linux-spi, linux-gpio, andriy.shevchenko, heikki.krogerus,
	andi.shyti, sakari.ailus, bartosz.golaszewski, Pandruvada,
	Srinivas
  Cc: Wang, Zhifeng

> From: Oliver Neukum <oneukum@suse.com>
> 
> On 04.09.23 07:44, Wentong Wu wrote:
> 
> > +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);
> 
> Let's suppose we are here, when a device is unplugged.
> timeout is arbitrary as it is passed down to us.
> 
> > +		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;
> > +}
> > +
> 
> [..]
> 
> > +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);
> > +}
> 
> And we execute this. rx_urb is killed and does nothing.
> I see nothing that terminates the waiting if you hit the wrong window.

I think the auxiliary_device_delete will trigger the remove function of ljca
client in his own sub system, and the delete() or remove() of every subsystem
will not return until the ongoing transfer(probably blocked by above
cmd_completion) complete. And that also makes sure no more transfers
comes to there.

BR,
Wentong
> It seems like this needs to complete the waiting.
> 
> 	Regards
> 		Oliver

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

* Re: [PATCH v14 1/4] usb: Add support for Intel LJCA device
  2023-09-04 13:52     ` Wu, Wentong
@ 2023-09-04 14:06       ` Oliver Neukum
  2023-09-05  2:20         ` Wu, Wentong
  0 siblings, 1 reply; 19+ messages in thread
From: Oliver Neukum @ 2023-09-04 14:06 UTC (permalink / raw)
  To: Wu, Wentong, Oliver Neukum, gregkh, arnd, mka, lee, wsa, kfting,
	broonie, linus.walleij, hdegoede, maz, brgl, linux-usb,
	linux-i2c, linux-spi, linux-gpio, andriy.shevchenko,
	heikki.krogerus, andi.shyti, sakari.ailus, bartosz.golaszewski,
	Pandruvada, Srinivas
  Cc: Wang, Zhifeng



On 04.09.23 15:52, Wu, Wentong wrote:
>> From: Oliver Neukum <oneukum@suse.com>

>> And we execute this. rx_urb is killed and does nothing.
>> I see nothing that terminates the waiting if you hit the wrong window.
> 
> I think the auxiliary_device_delete will trigger the remove function of ljca
> client in his own sub system, and the delete() or remove() of every subsystem
> will not return until the ongoing transfer(probably blocked by above
> cmd_completion) complete. And that also makes sure no more transfers
> comes to there.

Sure, you will not free used memory. But what allows you to be sure that you
make any progress at all? That is that you will hang arbitrarily long in
disconnect?

	Regards
		Oliver


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

* RE: [PATCH v14 1/4] usb: Add support for Intel LJCA device
  2023-09-04 14:06       ` Oliver Neukum
@ 2023-09-05  2:20         ` Wu, Wentong
  2023-09-05  8:46           ` Oliver Neukum
  0 siblings, 1 reply; 19+ messages in thread
From: Wu, Wentong @ 2023-09-05  2:20 UTC (permalink / raw)
  To: Oliver Neukum
  Cc: gregkh, arnd, mka, lee, wsa, kfting, broonie, linus.walleij,
	hdegoede, maz, brgl, linux-usb, linux-i2c, linux-spi, linux-gpio,
	andriy.shevchenko, heikki.krogerus, andi.shyti, sakari.ailus,
	bartosz.golaszewski, Pandruvada, Srinivas, Wang, Zhifeng

Hi Oliver,

Thanks for your review

> From: Oliver Neukum <oneukum@suse.com>
> On 04.09.23 15:52, Wu, Wentong wrote:
> >> From: Oliver Neukum <oneukum@suse.com>
> 
> >> And we execute this. rx_urb is killed and does nothing.
> >> I see nothing that terminates the waiting if you hit the wrong window.
> >
> > I think the auxiliary_device_delete will trigger the remove function
> > of ljca client in his own sub system, and the delete() or remove() of
> > every subsystem will not return until the ongoing transfer(probably
> > blocked by above
> > cmd_completion) complete. And that also makes sure no more transfers
> > comes to there.
> 
> Sure, you will not free used memory. But what allows you to be sure that you make any progress at all? 
>
> That is that you will hang arbitrarily long in disconnect?
This routine isn't called in an interrupt context, and it allows sleep or wait
something before the real shutdown like many drivers' remove() or
disconnect() do.

If we want to speed up the disconnect(), below changes is to complete the
cmd_completion if usb_kill_urb() has been called, but there is still possibility
ljca client init one more transfer before auxiliary_device_delete()

@@ -206,7 +206,11 @@ static void ljca_recv(struct urb *urb)
 
        if (urb->status) {
                /* sync/async unlink faults aren't errors */
-               if (urb->status == -ENOENT || urb->status == -ECONNRESET ||
+               if (urb->status == -ENOENT) {
+                       complete(&adap->cmd_completion);
+                       return;
+               }
+               if (urb->status == -ECONNRESET ||
                    urb->status == -ESHUTDOWN)
                        return;

perhaps we could add one more 'status' field in ljca_adapter to represent
disconnect() happen or not, and it will be checked in the beginning of
ljca_send() to avoid any new transfer.

BR,
Wentong

> 
> 	Regards
> 		Oliver


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

* Re: [PATCH v14 1/4] usb: Add support for Intel LJCA device
  2023-09-05  2:20         ` Wu, Wentong
@ 2023-09-05  8:46           ` Oliver Neukum
  2023-09-05  8:53             ` Wu, Wentong
  0 siblings, 1 reply; 19+ messages in thread
From: Oliver Neukum @ 2023-09-05  8:46 UTC (permalink / raw)
  To: Wu, Wentong, Oliver Neukum
  Cc: gregkh, arnd, mka, lee, wsa, kfting, broonie, linus.walleij,
	hdegoede, maz, brgl, linux-usb, linux-i2c, linux-spi, linux-gpio,
	andriy.shevchenko, heikki.krogerus, andi.shyti, sakari.ailus,
	bartosz.golaszewski, Pandruvada, Srinivas, Wang, Zhifeng



On 05.09.23 04:20, Wu, Wentong wrote:

Hi,

>> That is that you will hang arbitrarily long in disconnect?
> This routine isn't called in an interrupt context, and it allows sleep or wait
> something before the real shutdown like many drivers' remove() or
> disconnect() do.

It is, however, in the context of a kernel thread. We can wait, but not for
arbitrary periods.

> If we want to speed up the disconnect(), below changes is to complete the
> cmd_completion if usb_kill_urb() has been called, but there is still possibility
> ljca client init one more transfer before auxiliary_device_delete()
> 
> @@ -206,7 +206,11 @@ static void ljca_recv(struct urb *urb)
>   
>          if (urb->status) {
>                  /* sync/async unlink faults aren't errors */
> -               if (urb->status == -ENOENT || urb->status == -ECONNRESET ||
> +               if (urb->status == -ENOENT) {
> +                       complete(&adap->cmd_completion);
> +                       return;

I'd say you'd break suspend() by such a change.
You cannot complete in the interrupt handler, unless you can determine
why the URB is killed.
  
> +               }
> +               if (urb->status == -ECONNRESET ||
>                      urb->status == -ESHUTDOWN)
>                          return;
> 
> perhaps we could add one more 'status' field in ljca_adapter to represent
> disconnect() happen or not, and it will be checked in the beginning of
> ljca_send() to avoid any new transfer.

That would be a solution.


	Regards
		Oliver

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

* RE: [PATCH v14 1/4] usb: Add support for Intel LJCA device
  2023-09-05  8:46           ` Oliver Neukum
@ 2023-09-05  8:53             ` Wu, Wentong
  2023-09-05 10:04               ` gregkh
  0 siblings, 1 reply; 19+ messages in thread
From: Wu, Wentong @ 2023-09-05  8:53 UTC (permalink / raw)
  To: Oliver Neukum
  Cc: gregkh, arnd, mka, lee, wsa, kfting, broonie, linus.walleij,
	hdegoede, maz, brgl, linux-usb, linux-i2c, linux-spi, linux-gpio,
	andriy.shevchenko, heikki.krogerus, andi.shyti, sakari.ailus,
	bartosz.golaszewski, Pandruvada, Srinivas, Wang, Zhifeng

> From: Oliver Neukum <oneukum@suse.com>
> 
> On 05.09.23 04:20, Wu, Wentong wrote:
> 
> Hi,
> 
> >> That is that you will hang arbitrarily long in disconnect?
> > This routine isn't called in an interrupt context, and it allows sleep
> > or wait something before the real shutdown like many drivers' remove()
> > or
> > disconnect() do.
> 
> It is, however, in the context of a kernel thread. We can wait, but not for
> arbitrary periods.

AFAIK, this is very common.

> 
> > If we want to speed up the disconnect(), below changes is to complete
> > the cmd_completion if usb_kill_urb() has been called, but there is
> > still possibility ljca client init one more transfer before
> > auxiliary_device_delete()
> >
> > @@ -206,7 +206,11 @@ static void ljca_recv(struct urb *urb)
> >
> >          if (urb->status) {
> >                  /* sync/async unlink faults aren't errors */
> > -               if (urb->status == -ENOENT || urb->status == -ECONNRESET ||
> > +               if (urb->status == -ENOENT) {
> > +                       complete(&adap->cmd_completion);
> > +                       return;
> 
> I'd say you'd break suspend() by such a change.
> You cannot complete in the interrupt handler, unless you can determine why the
> URB is killed.

With below status field in ljca_adapter to determine if it's killed by disconnect().

If this is preferred, I could cook the patch for review.

If this is fixed, could you please help merge this usb-ljca driver so that it won't
block others which depends on this driver?

BR,
Wentong
> 
> > +               }
> > +               if (urb->status == -ECONNRESET ||
> >                      urb->status == -ESHUTDOWN)
> >                          return;
> >
> > perhaps we could add one more 'status' field in ljca_adapter to
> > represent
> > disconnect() happen or not, and it will be checked in the beginning of
> > ljca_send() to avoid any new transfer.
> 
> That would be a solution.
> 
> 
> 	Regards
> 		Oliver

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

* Re: [PATCH v14 1/4] usb: Add support for Intel LJCA device
  2023-09-05  8:53             ` Wu, Wentong
@ 2023-09-05 10:04               ` gregkh
  2023-09-06  3:29                 ` Wu, Wentong
  0 siblings, 1 reply; 19+ messages in thread
From: gregkh @ 2023-09-05 10:04 UTC (permalink / raw)
  To: Wu, Wentong
  Cc: Oliver Neukum, arnd, mka, lee, wsa, kfting, broonie,
	linus.walleij, hdegoede, maz, brgl, linux-usb, linux-i2c,
	linux-spi, linux-gpio, andriy.shevchenko, heikki.krogerus,
	andi.shyti, sakari.ailus, bartosz.golaszewski, Pandruvada,
	Srinivas, Wang, Zhifeng

On Tue, Sep 05, 2023 at 08:53:43AM +0000, Wu, Wentong wrote:
> > From: Oliver Neukum <oneukum@suse.com>
> > 
> > On 05.09.23 04:20, Wu, Wentong wrote:
> > 
> > Hi,
> > 
> > >> That is that you will hang arbitrarily long in disconnect?
> > > This routine isn't called in an interrupt context, and it allows sleep
> > > or wait something before the real shutdown like many drivers' remove()
> > > or
> > > disconnect() do.
> > 
> > It is, however, in the context of a kernel thread. We can wait, but not for
> > arbitrary periods.
> 
> AFAIK, this is very common.
> 
> > 
> > > If we want to speed up the disconnect(), below changes is to complete
> > > the cmd_completion if usb_kill_urb() has been called, but there is
> > > still possibility ljca client init one more transfer before
> > > auxiliary_device_delete()
> > >
> > > @@ -206,7 +206,11 @@ static void ljca_recv(struct urb *urb)
> > >
> > >          if (urb->status) {
> > >                  /* sync/async unlink faults aren't errors */
> > > -               if (urb->status == -ENOENT || urb->status == -ECONNRESET ||
> > > +               if (urb->status == -ENOENT) {
> > > +                       complete(&adap->cmd_completion);
> > > +                       return;
> > 
> > I'd say you'd break suspend() by such a change.
> > You cannot complete in the interrupt handler, unless you can determine why the
> > URB is killed.
> 
> With below status field in ljca_adapter to determine if it's killed by disconnect().
> 
> If this is preferred, I could cook the patch for review.
> 
> If this is fixed, could you please help merge this usb-ljca driver so that it won't
> block others which depends on this driver?

Please relax, we can't do anything until after -rc1 is out, and for me,
that includes reviewing the code.

There is no rush, or deadline, here at all.  It will be merged when it
is acceptable.

thanks,

greg k-h

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

* RE: [PATCH v14 1/4] usb: Add support for Intel LJCA device
  2023-09-05 10:04               ` gregkh
@ 2023-09-06  3:29                 ` Wu, Wentong
  0 siblings, 0 replies; 19+ messages in thread
From: Wu, Wentong @ 2023-09-06  3:29 UTC (permalink / raw)
  To: gregkh, Oliver Neukum
  Cc: arnd, mka, lee, wsa, kfting, broonie, linus.walleij, hdegoede,
	maz, brgl, linux-usb, linux-i2c, linux-spi, linux-gpio,
	andriy.shevchenko, heikki.krogerus, andi.shyti, sakari.ailus,
	bartosz.golaszewski, Pandruvada, Srinivas, Wang, Zhifeng

> From: gregkh@linuxfoundation.org <gregkh@linuxfoundation.org>
> On Tue, Sep 05, 2023 at 08:53:43AM +0000, Wu, Wentong wrote:
> > > From: Oliver Neukum <oneukum@suse.com>
> > >
> > > On 05.09.23 04:20, Wu, Wentong wrote:
> > >
> > > Hi,
> > >
> > > >> That is that you will hang arbitrarily long in disconnect?
> > > > This routine isn't called in an interrupt context, and it allows
> > > > sleep or wait something before the real shutdown like many
> > > > drivers' remove() or
> > > > disconnect() do.
> > >
> > > It is, however, in the context of a kernel thread. We can wait, but
> > > not for arbitrary periods.
> >
> > AFAIK, this is very common.
> >
> > >
> > > > If we want to speed up the disconnect(), below changes is to
> > > > complete the cmd_completion if usb_kill_urb() has been called, but
> > > > there is still possibility ljca client init one more transfer
> > > > before
> > > > auxiliary_device_delete()
> > > >
> > > > @@ -206,7 +206,11 @@ static void ljca_recv(struct urb *urb)
> > > >
> > > >          if (urb->status) {
> > > >                  /* sync/async unlink faults aren't errors */
> > > > -               if (urb->status == -ENOENT || urb->status == -ECONNRESET ||
> > > > +               if (urb->status == -ENOENT) {
> > > > +                       complete(&adap->cmd_completion);
> > > > +                       return;
> > >
> > > I'd say you'd break suspend() by such a change.
> > > You cannot complete in the interrupt handler, unless you can
> > > determine why the URB is killed.
> >
> > With below status field in ljca_adapter to determine if it's killed by disconnect().
> >
> > If this is preferred, I could cook the patch for review.
> >
> > If this is fixed, could you please help merge this usb-ljca driver so
> > that it won't block others which depends on this driver?
> 
> Please relax, we can't do anything until after -rc1 is out, and for me, that
> includes reviewing the code.

Thanks for your review.

> 
> There is no rush, or deadline, here at all.  It will be merged when it is acceptable.

Understood, thanks

BR,
Wentong
> 
> thanks,
> 
> greg k-h

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

end of thread, other threads:[~2023-09-06  3:30 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-04  5:44 [PATCH v14 0/4] Add Intel LJCA device driver Wentong Wu
2023-09-04  5:44 ` [PATCH v14 1/4] usb: Add support for Intel LJCA device Wentong Wu
2023-09-04  7:45   ` Hans de Goede
2023-09-04  7:52     ` Wu, Wentong
2023-09-04 10:33   ` Oliver Neukum
2023-09-04 13:52     ` Wu, Wentong
2023-09-04 14:06       ` Oliver Neukum
2023-09-05  2:20         ` Wu, Wentong
2023-09-05  8:46           ` Oliver Neukum
2023-09-05  8:53             ` Wu, Wentong
2023-09-05 10:04               ` gregkh
2023-09-06  3:29                 ` Wu, Wentong
2023-09-04  5:44 ` [PATCH v14 2/4] i2c: Add support for Intel LJCA USB I2C driver Wentong Wu
2023-09-04  5:44 ` [PATCH v14 3/4] spi: Add support for Intel LJCA USB SPI driver Wentong Wu
2023-09-04  5:44 ` [PATCH v14 4/4] gpio: update Intel LJCA USB GPIO driver Wentong Wu
2023-09-04  7:38 ` [PATCH v14 0/4] Add Intel LJCA device driver Bartosz Golaszewski
2023-09-04  8:25   ` Wu, Wentong
2023-09-04  7:44 ` Hans de Goede
2023-09-04  7:56   ` Wu, Wentong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).