linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v8 0/6] Add Intel LJCA device driver
@ 2023-05-11 17:58 Ye Xiang
  2023-05-11 17:58 ` [PATCH v8 1/6] usb: Add support for Intel LJCA device Ye Xiang
                   ` (6 more replies)
  0 siblings, 7 replies; 22+ messages in thread
From: Ye Xiang @ 2023-05-11 17:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Arnd Bergmann, Matthias Kaehlcke, Lee Jones,
	Wolfram Sang, Tyrone Ting, Mark Brown, Linus Walleij,
	Marc Zyngier, Bartosz Golaszewski, linux-usb, linux-i2c,
	linux-kernel, linux-spi, linux-gpio
  Cc: srinivas.pandruvada, heikki.krogerus, andriy.shevchenko,
	sakari.ailus, zhifeng.wang, wentong.wu, lixu.zhang, Ye Xiang

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

---
v8:
 - ljca: use READ_ONCE to make the value of ibuf consistent.
 - gpio: inplement get_direction callback.
 - gpio: move kconfig item into `USB GPIO expanders` menu.

v7:
 - ljca: remove unused field `udev` in struct ljca_dev.
 - ljca: rename ljca module name to usb-ljca.
 - ljca: use CONFIG_ACPI to enclose acpi related code.
 - ljca/gpio/i2c/spi: align MACRO defination.

v6:
 - ljca: split LJCA USB driver into two commits: USB part and API part.
 - gpio/i2c/spi: use auxiliary bus for sub-module device enumeration instead of MFD.
 - move document patch for LJCA sysfs entry to the 3th patch of this patch series.
 - ljca: fix potential race condition when wait response timeout.
 - ljca: use devm_kzalloc to malloc ljca device struct. 

v5:
 - move ljca.h from drivers/include/mfd to drivers/include/usb.
 - ljca: fix a potential memory leak issue.
 - add a blank line before return to adust to kernel code style.
 - ljca: sysfs: split "cmd" to "ljca_dfu" and "ljca_trace_level".

v4:
 - move ljca.c from drivers/mfd to drivers/usb/misc folder.
 - fix index warning in sysfs-bus-devices-ljca.

v3:
 - spi: make ljca_spi_transfer inline and fix an endian issue.

v2:
 - ljca: remove reset command.
 - gpio/spi/i2c: add `default MFD_LJCA` in Kconfig.
 - gpio: add "select GPIOLIB_IRQCHIP" in Kconfig.

Ye Xiang (6):
  usb: Add support for Intel LJCA device
  usb: ljca: Add transport interfaces for sub-module drivers
  Documentation: Add ABI doc for attributes of LJCA device
  gpio: Add support for Intel LJCA USB GPIO driver
  i2c: Add support for Intel LJCA USB I2C driver
  spi: Add support for Intel LJCA USB SPI driver

 .../ABI/testing/sysfs-bus-usb-devices-ljca    |   36 +
 drivers/gpio/Kconfig                          |   12 +
 drivers/gpio/Makefile                         |    1 +
 drivers/gpio/gpio-ljca.c                      |  479 ++++++++
 drivers/i2c/busses/Kconfig                    |   11 +
 drivers/i2c/busses/Makefile                   |    1 +
 drivers/i2c/busses/i2c-ljca.c                 |  350 ++++++
 drivers/spi/Kconfig                           |   11 +
 drivers/spi/Makefile                          |    1 +
 drivers/spi/spi-ljca.c                        |  290 +++++
 drivers/usb/misc/Kconfig                      |   14 +
 drivers/usb/misc/Makefile                     |    1 +
 drivers/usb/misc/usb-ljca.c                   | 1068 +++++++++++++++++
 include/linux/usb/ljca.h                      |  102 ++
 14 files changed, 2377 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-usb-devices-ljca
 create mode 100644 drivers/gpio/gpio-ljca.c
 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.34.1


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

* [PATCH v8 1/6] usb: Add support for Intel LJCA device
  2023-05-11 17:58 [PATCH v8 0/6] Add Intel LJCA device driver Ye Xiang
@ 2023-05-11 17:58 ` Ye Xiang
  2023-05-22  9:21   ` Oliver Neukum
  2023-05-11 17:58 ` [PATCH v8 2/6] usb: ljca: Add transport interfaces for sub-module drivers Ye Xiang
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 22+ messages in thread
From: Ye Xiang @ 2023-05-11 17:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Arnd Bergmann, Matthias Kaehlcke, Lee Jones,
	Wolfram Sang, Tyrone Ting, Mark Brown, Linus Walleij,
	Marc Zyngier, Bartosz Golaszewski, linux-usb, linux-i2c,
	linux-kernel, linux-spi, linux-gpio
  Cc: srinivas.pandruvada, heikki.krogerus, andriy.shevchenko,
	sakari.ailus, zhifeng.wang, wentong.wu, lixu.zhang, Ye Xiang

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 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: Ye Xiang <xiang.ye@intel.com>
---
 drivers/usb/misc/Kconfig    |  14 +
 drivers/usb/misc/Makefile   |   1 +
 drivers/usb/misc/usb-ljca.c | 977 ++++++++++++++++++++++++++++++++++++
 include/linux/usb/ljca.h    |  50 ++
 4 files changed, 1042 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 a5f7652db7da..3434dbcd94f5 100644
--- a/drivers/usb/misc/Kconfig
+++ b/drivers/usb/misc/Kconfig
@@ -273,6 +273,20 @@ config USB_LINK_LAYER_TEST
 	  Layer Test Device. Say Y only when you want to conduct USB Super Speed
 	  Link Layer Test for host controllers.
 
+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.
+
 config USB_CHAOSKEY
 	tristate "ChaosKey random number generator driver support"
 	depends on HW_RANDOM
diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
index 93581baec3a8..a41d02cc4f3e 100644
--- a/drivers/usb/misc/Makefile
+++ b/drivers/usb/misc/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_USB_HUB_USB251XB)		+= usb251xb.o
 obj-$(CONFIG_USB_HSIC_USB3503)		+= usb3503.o
 obj-$(CONFIG_USB_HSIC_USB4604)		+= usb4604.o
 obj-$(CONFIG_USB_CHAOSKEY)		+= chaoskey.o
+obj-$(CONFIG_USB_LJCA)			+= usb-ljca.o
 
 obj-$(CONFIG_USB_SISUSBVGA)		+= sisusbvga/
 obj-$(CONFIG_USB_LINK_LAYER_TEST)	+= lvstest.o
diff --git a/drivers/usb/misc/usb-ljca.c b/drivers/usb/misc/usb-ljca.c
new file mode 100644
index 000000000000..4f76f73fdcac
--- /dev/null
+++ b/drivers/usb/misc/usb-ljca.c
@@ -0,0 +1,977 @@
+// 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>
+
+enum ljca_acpi_match_adr {
+	LJCA_ACPI_MATCH_GPIO,
+	LJCA_ACPI_MATCH_I2C1,
+	LJCA_ACPI_MATCH_I2C2,
+	LJCA_ACPI_MATCH_SPI1,
+	LJCA_ACPI_MATCH_SPI2,
+	LJCA_ACPI_MATCH_MAX,
+};
+
+struct ljca_msg {
+	u8 type;
+	u8 cmd;
+	u8 flags;
+	u8 len;
+	u8 data[];
+} __packed;
+
+struct fw_version {
+	u8 major;
+	u8 minor;
+	__le16 patch;
+	__le16 build;
+} __packed;
+
+/**
+ * enum ljca_stub_type - Stub type supported by LJCA.
+ * @LJCA_MNG_STUB: Provides Management messages.
+ * @LJCA_DIAG_STUB: provides Diagnose messages.
+ * @LJCA_GPIO_STUB: provides GPIO functionality.
+ * @LJCA_I2C_STUB: provides I2C functionality.
+ * @LJCA_SPI_STUB: provides SPI functionality.
+ */
+enum ljca_stub_type {
+	LJCA_MNG_STUB = 1,
+	LJCA_DIAG_STUB,
+	LJCA_GPIO_STUB,
+	LJCA_I2C_STUB,
+	LJCA_SPI_STUB,
+};
+
+/* command Flags */
+#define LJCA_ACK_FLAG	BIT(0)
+#define LJCA_RESP_FLAG	BIT(1)
+#define LJCA_CMPL_FLAG	BIT(2)
+
+/* MNG stub commands */
+enum ljca_mng_cmd {
+	LJCA_MNG_GET_VERSION = 1,
+	LJCA_MNG_RESET_NOTIFY,
+	LJCA_MNG_RESET,
+	LJCA_MNG_ENUM_GPIO,
+	LJCA_MNG_ENUM_I2C,
+	LJCA_MNG_POWER_STATE_CHANGE,
+	LJCA_MNG_SET_DFU_MODE,
+	LJCA_MNG_ENUM_SPI,
+};
+
+/* DIAG commands */
+enum ljca_diag_cmd {
+	LJCA_DIAG_GET_STATE = 1,
+	LJCA_DIAG_GET_STATISTIC,
+	LJCA_DIAG_SET_TRACE_LEVEL,
+	LJCA_DIAG_SET_ECHO_MODE,
+	LJCA_DIAG_GET_FW_LOG,
+	LJCA_DIAG_GET_FW_COREDUMP,
+	LJCA_DIAG_TRIGGER_WDT,
+	LJCA_DIAG_TRIGGER_FAULT,
+	LJCA_DIAG_FEED_WDT,
+	LJCA_DIAG_GET_SECURE_STATE,
+};
+
+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;
+
+#define LJCA_MAX_PACKET_SIZE		64
+#define LJCA_MAX_PAYLOAD_SIZE		(LJCA_MAX_PACKET_SIZE - sizeof(struct ljca_msg))
+#define LJCA_USB_WRITE_TIMEOUT_MS	200
+#define LJCA_USB_WRITE_ACK_TIMEOUT_MS	500
+#define LJCA_USB_ENUM_STUB_TIMEOUT_MS	20
+
+struct ljca_event_cb_entry {
+	struct list_head list;
+	unsigned id;
+	ljca_event_cb_t notify;
+	void *context;
+};
+
+struct ljca_dev {
+	struct usb_interface *intf;
+	u8 in_ep; /* the address of the bulk in endpoint */
+	u8 out_ep; /* the address of the bulk out endpoint */
+
+	/* the urb/buffer for read */
+	struct urb *in_urb;
+	unsigned char *ibuf;
+	size_t ibuf_len;
+
+	/* mutex to protect all fields in this structure */
+	struct mutex mutex;
+	bool started;
+	struct list_head stubs_list;
+
+	/* to wait for an ongoing write ack */
+	wait_queue_head_t ack_wq;
+
+	struct ljca *ljcas[LJCA_ACPI_MATCH_MAX];
+	int ljca_count;
+};
+
+struct ljca_stub_packet {
+	unsigned int *ibuf_len;
+	u8 *ibuf;
+};
+
+struct ljca_stub {
+	struct list_head list;
+	struct usb_interface *intf;
+	struct ljca_stub_packet ipacket;
+	u8 type;
+
+	/* for identify ack */
+	bool acked;
+	int cur_cmd;
+
+	struct list_head event_entrys;
+	/* lock to protect event_entry */
+	spinlock_t event_cb_lock;
+
+	unsigned long priv[];
+};
+
+static inline void *ljca_priv(struct ljca_stub *stub)
+{
+	return stub->priv;
+}
+
+static bool ljca_validate(struct ljca_msg *header, u32 data_len)
+{
+	return header->len + sizeof(*header) == data_len;
+}
+
+static struct ljca_stub *ljca_stub_alloc(struct ljca_dev *dev, u8 type, int priv_size)
+{
+	struct ljca_stub *stub;
+
+	stub = kzalloc(struct_size(stub, priv, priv_size), GFP_KERNEL);
+	if (!stub)
+		return ERR_PTR(-ENOMEM);
+
+	stub->type = type;
+	stub->intf = usb_get_intf(dev->intf);
+	INIT_LIST_HEAD(&stub->event_entrys);
+	spin_lock_init(&stub->event_cb_lock);
+	list_add_tail(&stub->list, &dev->stubs_list);
+
+	return stub;
+}
+
+static void ljca_stub_destroy(struct ljca_stub *stub)
+{
+	usb_put_intf(stub->intf);
+	list_del_init(&stub->list);
+	kfree(stub);
+}
+
+static void ljca_stub_cleanup(struct ljca_dev *dev)
+{
+	struct ljca_stub *stub, *next;
+
+	list_for_each_entry_safe(stub, next, &dev->stubs_list, list) {
+		ljca_stub_destroy(stub);
+	}
+}
+
+static struct ljca_stub *ljca_stub_find(struct ljca_dev *dev, u8 type)
+{
+	struct ljca_stub *stub;
+
+	list_for_each_entry(stub, &dev->stubs_list, list) {
+		if (stub->type == type)
+			return stub;
+	}
+
+	dev_err(&dev->intf->dev, "USB stub not found, type:%d\n", type);
+
+	return ERR_PTR(-ENODEV);
+}
+
+static void ljca_stub_notify(struct ljca_stub *stub, u8 cmd, const void *evt_data, int len)
+{
+	struct ljca_event_cb_entry *event_entry;
+	unsigned long flags;
+
+	spin_lock_irqsave(&stub->event_cb_lock, flags);
+	list_for_each_entry(event_entry, &stub->event_entrys, list) {
+		if (event_entry->notify)
+			event_entry->notify(event_entry->context, cmd, evt_data, len);
+	}
+	spin_unlock_irqrestore(&stub->event_cb_lock, flags);
+}
+
+static int ljca_parse(struct ljca_dev *dev, struct ljca_msg *header)
+{
+	struct ljca_stub *stub;
+	unsigned int *ibuf_len;
+	u8 *ibuf;
+
+	stub = ljca_stub_find(dev, header->type);
+	if (IS_ERR(stub))
+		return PTR_ERR(stub);
+
+	if (!(header->flags & LJCA_ACK_FLAG)) {
+		ljca_stub_notify(stub, header->cmd, header->data, header->len);
+		return 0;
+	}
+
+	if (stub->cur_cmd != header->cmd) {
+		dev_err(&dev->intf->dev, "header and stub current command mismatch (%x vs %x)\n",
+			header->cmd, stub->cur_cmd);
+		return -EINVAL;
+	}
+
+	ibuf_len = READ_ONCE(stub->ipacket.ibuf_len);
+	ibuf = READ_ONCE(stub->ipacket.ibuf);
+
+	if (ibuf && ibuf_len) {
+		unsigned int newlen;
+
+		newlen = min_t(unsigned int, header->len, *ibuf_len);
+
+		*ibuf_len = newlen;
+		memcpy(ibuf, header->data, newlen);
+	}
+
+	stub->acked = true;
+	wake_up(&dev->ack_wq);
+
+	return 0;
+}
+
+static int ljca_stub_write(struct ljca_stub *stub, u8 cmd, const void *obuf, unsigned int obuf_len,
+			   void *ibuf, unsigned int *ibuf_len, bool wait_ack, unsigned long timeout)
+{
+	struct ljca_dev *dev = usb_get_intfdata(stub->intf);
+	u8 flags = LJCA_CMPL_FLAG;
+	struct ljca_msg *header;
+	unsigned int msg_len = sizeof(*header) + obuf_len;
+	int actual;
+	int ret;
+
+	if (msg_len > LJCA_MAX_PACKET_SIZE)
+		return -EINVAL;
+
+	if (wait_ack)
+		flags |= LJCA_ACK_FLAG;
+
+	header = kmalloc(msg_len, GFP_KERNEL);
+	if (!header)
+		return -ENOMEM;
+
+	header->type = stub->type;
+	header->cmd = cmd;
+	header->flags = flags;
+	header->len = obuf_len;
+
+	if (obuf)
+		memcpy(header->data, obuf, obuf_len);
+
+	dev_dbg(&dev->intf->dev, "send: type:%d cmd:%d flags:%d len:%d\n", header->type,
+		header->cmd, header->flags, header->len);
+
+	usb_autopm_get_interface(dev->intf);
+	if (!dev->started) {
+		kfree(header);
+		ret = -ENODEV;
+		goto error_put;
+	}
+
+	mutex_lock(&dev->mutex);
+	stub->cur_cmd = cmd;
+	stub->ipacket.ibuf = ibuf;
+	stub->ipacket.ibuf_len = ibuf_len;
+	stub->acked = false;
+	ret = usb_bulk_msg(interface_to_usbdev(dev->intf),
+			   usb_sndbulkpipe(interface_to_usbdev(dev->intf), dev->out_ep), header,
+			   msg_len, &actual, LJCA_USB_WRITE_TIMEOUT_MS);
+	kfree(header);
+	if (ret) {
+		dev_err(&dev->intf->dev, "bridge write failed ret:%d\n", ret);
+		goto error_unlock;
+	}
+
+	if (actual != msg_len) {
+		dev_err(&dev->intf->dev, "bridge write length mismatch (%d vs %d)\n", msg_len,
+			actual);
+		ret = -EINVAL;
+		goto error_unlock;
+	}
+
+	if (wait_ack) {
+		ret = wait_event_timeout(dev->ack_wq, stub->acked, msecs_to_jiffies(timeout));
+		if (!ret) {
+			dev_err(&dev->intf->dev, "acked wait timeout\n");
+			ret = -ETIMEDOUT;
+			goto error_unlock;
+		}
+	}
+
+	ret = 0;
+error_unlock:
+	stub->ipacket.ibuf = NULL;
+	stub->ipacket.ibuf_len = NULL;
+	mutex_unlock(&dev->mutex);
+error_put:
+	usb_autopm_put_interface(dev->intf);
+
+	return ret;
+}
+
+static void ljca_read_complete(struct urb *urb)
+{
+	struct ljca_msg *header = urb->transfer_buffer;
+	struct ljca_dev *dev = urb->context;
+	int len = urb->actual_length;
+	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(&dev->intf->dev, "read bulk urb transfer failed: %d\n", urb->status);
+		goto resubmit;
+	}
+
+	dev_dbg(&dev->intf->dev, "receive: type:%d cmd:%d flags:%d len:%d\n", header->type,
+		header->cmd, header->flags, header->len);
+
+	if (!ljca_validate(header, len)) {
+		dev_err(&dev->intf->dev, "data not correct header->len:%d payload_len:%d\n ",
+			header->len, len);
+		goto resubmit;
+	}
+
+	ret = ljca_parse(dev, header);
+	if (ret)
+		dev_err(&dev->intf->dev, "failed to parse data: ret:%d type:%d len:%d\n", ret,
+			header->type, header->len);
+
+resubmit:
+	ret = usb_submit_urb(urb, GFP_ATOMIC);
+	if (ret)
+		dev_err(&dev->intf->dev, "failed submitting read urb, error %d\n", ret);
+}
+
+static int ljca_start(struct ljca_dev *dev)
+{
+	int ret;
+
+	usb_fill_bulk_urb(dev->in_urb, interface_to_usbdev(dev->intf),
+			  usb_rcvbulkpipe(interface_to_usbdev(dev->intf), dev->in_ep), dev->ibuf,
+			  dev->ibuf_len, ljca_read_complete, dev);
+
+	ret = usb_submit_urb(dev->in_urb, GFP_KERNEL);
+	if (ret) {
+		dev_err(&dev->intf->dev, "failed submitting read urb, error %d\n", ret);
+		return ret;
+	}
+
+	mutex_lock(&dev->mutex);
+	dev->started = true;
+	mutex_unlock(&dev->mutex);
+
+	return 0;
+}
+
+static int ljca_mng_reset_handshake(struct ljca_stub *stub)
+{
+	u32 *reset_id_priv = ljca_priv(stub);
+	__le32 reset_id;
+	__le32 reset_id_ret = 0;
+	unsigned int ilen = sizeof(__le32);
+	int ret;
+
+	reset_id = cpu_to_le32((*reset_id_priv)++);
+	ret = ljca_stub_write(stub, LJCA_MNG_RESET_NOTIFY, &reset_id, sizeof(reset_id),
+			      &reset_id_ret, &ilen, true, LJCA_USB_WRITE_ACK_TIMEOUT_MS);
+	if (ret)
+		return ret;
+
+	if (ilen != sizeof(reset_id_ret) || reset_id_ret != reset_id) {
+		dev_err(&stub->intf->dev, "mng reset notify failed reset_id:%u/%u\n",
+			le32_to_cpu(reset_id_ret), le32_to_cpu(reset_id));
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void ljca_aux_release(struct device *dev)
+{
+	struct auxiliary_device *auxdev = to_auxiliary_dev(dev);
+	struct ljca *ljca = auxiliary_dev_to_ljca(auxdev);
+
+	kfree(auxdev->dev.platform_data);
+	kfree(ljca);
+}
+
+#ifdef CONFIG_ACPI
+static void ljca_aux_dev_acpi_bind(struct ljca_dev *dev, struct auxiliary_device *auxdev,
+				   unsigned int adr)
+{
+	struct acpi_device *parent;
+	struct acpi_device *adev = NULL;
+
+	/* new auxiliary device bind to acpi device */
+	parent = ACPI_COMPANION(&dev->intf->dev);
+	if (!parent)
+		return;
+
+	adev = acpi_find_child_device(parent, adr, false);
+	ACPI_COMPANION_SET(&auxdev->dev, adev ?: parent);
+}
+#else
+static void ljca_aux_dev_acpi_bind(struct ljca_dev *dev, struct auxiliary_device *auxdev,
+				   unsigned int adr)
+{
+}
+#endif
+
+static int ljca_add_aux_dev(struct ljca_dev *dev, char *name, u8 type, u8 id, u8 adr, void *data,
+			    unsigned int len)
+{
+	struct auxiliary_device *auxdev;
+	struct ljca *ljca;
+	int ret;
+
+	if (dev->ljca_count >= ARRAY_SIZE(dev->ljcas))
+		return -EINVAL;
+
+	ljca = kzalloc(sizeof(*ljca), GFP_KERNEL);
+	if (!ljca)
+		return -ENOMEM;
+
+	ljca->type = type;
+	ljca->id = id;
+	ljca->dev = dev;
+
+	auxdev = &ljca->auxdev;
+	auxdev->name = name;
+	auxdev->id = id;
+	auxdev->dev.platform_data = kmemdup(data, len, GFP_KERNEL);
+	if (!auxdev->dev.platform_data)
+		return -ENOMEM;
+
+	auxdev->dev.parent = &dev->intf->dev;
+	auxdev->dev.release = ljca_aux_release;
+
+	ret = auxiliary_device_init(auxdev);
+	if (ret) {
+		kfree(auxdev->dev.platform_data);
+		kfree(auxdev);
+		return ret;
+	}
+
+	ljca_aux_dev_acpi_bind(dev, auxdev, adr);
+
+	ret = auxiliary_device_add(auxdev);
+	if (ret) {
+		dev_err(&dev->intf->dev, "failed to add auxiliary dev %s.%d\n", name, id);
+		auxiliary_device_uninit(auxdev);
+		return ret;
+	}
+
+	dev->ljcas[dev->ljca_count++] = ljca;
+
+	return 0;
+}
+
+static void ljca_cleanup_aux_dev(struct ljca_dev *dev)
+{
+	int i;
+
+	for (i = 0; i < dev->ljca_count; i++) {
+		auxiliary_device_delete(&dev->ljcas[i]->auxdev);
+		auxiliary_device_uninit(&dev->ljcas[i]->auxdev);
+	}
+}
+
+static int ljca_gpio_stub_init(struct ljca_dev *dev, struct ljca_gpio_descriptor *desc)
+{
+	u32 valid_pin[LJCA_MAX_GPIO_NUM / BITS_PER_TYPE(u32)];
+	struct ljca_gpio_info gpio_info = {};
+	struct ljca_stub *stub;
+	int gpio_num;
+	int i;
+	int ret;
+
+	gpio_num = desc->pins_per_bank * desc->bank_num;
+	if (gpio_num > LJCA_MAX_GPIO_NUM)
+		return -EINVAL;
+
+	stub = ljca_stub_alloc(dev, LJCA_GPIO_STUB, desc->bank_num * sizeof(struct ljca *));
+	if (IS_ERR(stub))
+		return PTR_ERR(stub);
+
+	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_add_aux_dev(dev, "ljca-gpio", stub->type, 0, LJCA_ACPI_MATCH_GPIO, &gpio_info,
+			       sizeof(gpio_info));
+	if (ret)
+		ljca_stub_destroy(stub);
+
+	return ret;
+}
+
+static int ljca_mng_enum_gpio(struct ljca_stub *stub)
+{
+	struct ljca_dev *dev = usb_get_intfdata(stub->intf);
+	char buf[LJCA_MAX_PAYLOAD_SIZE];
+	struct ljca_gpio_descriptor *desc;
+	unsigned int len = LJCA_MAX_PAYLOAD_SIZE;
+	int ret;
+
+	ret = ljca_stub_write(stub, LJCA_MNG_ENUM_GPIO, NULL, 0, buf, &len, true,
+			      LJCA_USB_ENUM_STUB_TIMEOUT_MS);
+	if (ret)
+		return ret;
+
+	desc = (struct ljca_gpio_descriptor *)buf;
+	if (len != struct_size(desc, bank_desc, desc->bank_num)) {
+		dev_err(&stub->intf->dev, "GPIO enumeration failed, len:%u bank_num:%u\n", len,
+			desc->bank_num);
+		return -EINVAL;
+	}
+
+	return ljca_gpio_stub_init(dev, desc);
+}
+
+static int ljca_i2c_stub_init(struct ljca_dev *dev, struct ljca_i2c_descriptor *desc)
+{
+	struct ljca_stub *stub;
+	int ret;
+	int i;
+
+	stub = ljca_stub_alloc(dev, LJCA_I2C_STUB, 0);
+	if (IS_ERR(stub))
+		return PTR_ERR(stub);
+
+	for (i = 0; i < desc->num; i++) {
+		struct ljca_i2c_info i2c_info = {};
+
+		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_add_aux_dev(dev, "ljca-i2c", stub->type, i, LJCA_ACPI_MATCH_I2C1 + i,
+				       &i2c_info, sizeof(i2c_info));
+		if (ret) {
+			ljca_stub_destroy(stub);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int ljca_mng_enum_i2c(struct ljca_stub *stub)
+{
+	struct ljca_dev *dev = usb_get_intfdata(stub->intf);
+	char buf[LJCA_MAX_PAYLOAD_SIZE];
+	struct ljca_i2c_descriptor *desc;
+	unsigned int len = LJCA_MAX_PAYLOAD_SIZE;
+	int ret;
+
+	ret = ljca_stub_write(stub, LJCA_MNG_ENUM_I2C, NULL, 0, buf, &len, true,
+			      LJCA_USB_ENUM_STUB_TIMEOUT_MS);
+	if (ret) {
+		dev_err(&stub->intf->dev, "I2C enumeration failed, ret:%d len:%u\n", ret, len);
+		return ret;
+	}
+
+	desc = (struct ljca_i2c_descriptor *)buf;
+
+	return ljca_i2c_stub_init(dev, desc);
+}
+
+static int ljca_spi_stub_init(struct ljca_dev *dev, struct ljca_spi_descriptor *desc)
+{
+	struct ljca_stub *stub;
+	int i;
+	int ret;
+
+	stub = ljca_stub_alloc(dev, LJCA_SPI_STUB, 0);
+	if (IS_ERR(stub))
+		return PTR_ERR(stub);
+
+	for (i = 0; i < desc->num; i++) {
+		struct ljca_spi_info spi_info = {};
+
+		spi_info.id = desc->info[i].id;
+		spi_info.capacity = desc->info[i].capacity;
+
+		ret = ljca_add_aux_dev(dev, "ljca-spi", stub->type, i, LJCA_ACPI_MATCH_SPI1 + i,
+				       &spi_info, sizeof(spi_info));
+		if (ret) {
+			ljca_stub_destroy(stub);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int ljca_mng_enum_spi(struct ljca_stub *stub)
+{
+	struct ljca_dev *dev = usb_get_intfdata(stub->intf);
+	char buf[LJCA_MAX_PAYLOAD_SIZE];
+	struct ljca_spi_descriptor *desc;
+	unsigned int len = LJCA_MAX_PAYLOAD_SIZE;
+	int ret;
+
+	ret = ljca_stub_write(stub, LJCA_MNG_ENUM_SPI, NULL, 0, buf, &len, true,
+			      LJCA_USB_ENUM_STUB_TIMEOUT_MS);
+	if (ret) {
+		dev_err(&stub->intf->dev, "SPI enumeration failed, ret:%d len:%d\n", ret, len);
+		return ret;
+	}
+
+	desc = (struct ljca_spi_descriptor *)buf;
+
+	return ljca_spi_stub_init(dev, desc);
+}
+
+static int ljca_mng_get_version(struct ljca_stub *stub, char *buf)
+{
+	struct fw_version version = {};
+	unsigned int len = sizeof(version);
+	int ret;
+
+	ret = ljca_stub_write(stub, LJCA_MNG_GET_VERSION, NULL, 0, &version, &len, true,
+			      LJCA_USB_WRITE_ACK_TIMEOUT_MS);
+	if (ret)
+		return ret;
+
+	if (len != sizeof(version)) {
+		dev_err(&stub->intf->dev, "get version failed, len:%d\n", len);
+		return -EINVAL;
+	}
+
+	return sysfs_emit(buf, "%d.%d.%d.%d\n", version.major, version.minor,
+			  le16_to_cpu(version.patch), le16_to_cpu(version.build));
+}
+
+static inline int ljca_mng_set_dfu_mode(struct ljca_stub *stub)
+{
+	return ljca_stub_write(stub, LJCA_MNG_SET_DFU_MODE, NULL, 0, NULL, NULL, true,
+			       LJCA_USB_WRITE_ACK_TIMEOUT_MS);
+}
+
+static int ljca_mng_link(struct ljca_dev *dev, struct ljca_stub *stub)
+{
+	int ret;
+
+	ret = ljca_mng_reset_handshake(stub);
+	if (ret)
+		return ret;
+
+	/* try to enum all the stubs */
+	ret = ljca_mng_enum_gpio(stub);
+	if (ret)
+		dev_warn(&dev->intf->dev, "GPIO function does not support yet\n");
+
+	ret = ljca_mng_enum_i2c(stub);
+	if (ret)
+		dev_warn(&dev->intf->dev, "I2C function does not support yet\n");
+
+	ret = ljca_mng_enum_spi(stub);
+	if (ret)
+		dev_warn(&dev->intf->dev, "SPI function does not support yet\n");
+
+	return 0;
+}
+
+static int ljca_mng_init(struct ljca_dev *dev)
+{
+	struct ljca_stub *stub;
+	int ret;
+
+	stub = ljca_stub_alloc(dev, LJCA_MNG_STUB, sizeof(u32));
+	if (IS_ERR(stub))
+		return PTR_ERR(stub);
+
+	ret = ljca_mng_link(dev, stub);
+	if (ret)
+		dev_err(&dev->intf->dev, "mng stub link failed, ret:%d\n", ret);
+
+	return ret;
+}
+
+static inline int ljca_diag_set_trace_level(struct ljca_stub *stub, u8 level)
+{
+	return ljca_stub_write(stub, LJCA_DIAG_SET_TRACE_LEVEL, &level, sizeof(level), NULL, NULL,
+			       true, LJCA_USB_WRITE_ACK_TIMEOUT_MS);
+}
+
+static int ljca_diag_init(struct ljca_dev *dev)
+{
+	struct ljca_stub *stub;
+
+	stub = ljca_stub_alloc(dev, LJCA_DIAG_STUB, 0);
+
+	return PTR_ERR_OR_ZERO(stub);
+}
+
+static void ljca_delete(struct ljca_dev *dev)
+{
+	mutex_destroy(&dev->mutex);
+	usb_free_urb(dev->in_urb);
+	usb_put_intf(dev->intf);
+}
+
+static int ljca_init(struct ljca_dev *dev)
+{
+	mutex_init(&dev->mutex);
+	init_waitqueue_head(&dev->ack_wq);
+	INIT_LIST_HEAD(&dev->stubs_list);
+
+	return 0;
+}
+
+static void ljca_stop(struct ljca_dev *dev)
+{
+	mutex_lock(&dev->mutex);
+	dev->started = false;
+	mutex_unlock(&dev->mutex);
+	usb_kill_urb(dev->in_urb);
+}
+
+static ssize_t ljca_enable_dfu_store(struct device *dev, struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	struct usb_interface *intf = to_usb_interface(dev);
+	struct ljca_dev *ljca_dev = usb_get_intfdata(intf);
+	struct ljca_stub *mng_stub = ljca_stub_find(ljca_dev, LJCA_MNG_STUB);
+	bool enable;
+	int ret;
+
+	ret = kstrtobool(buf, &enable);
+	if (ret)
+		return ret;
+
+	if (enable) {
+		ret = ljca_mng_set_dfu_mode(mng_stub);
+		if (ret)
+			return ret;
+	}
+
+	return count;
+}
+static DEVICE_ATTR_WO(ljca_enable_dfu);
+
+static ssize_t ljca_trace_level_store(struct device *dev, struct device_attribute *attr,
+				      const char *buf, size_t count)
+{
+	struct usb_interface *intf = to_usb_interface(dev);
+	struct ljca_dev *ljca_dev = usb_get_intfdata(intf);
+	struct ljca_stub *diag_stub = ljca_stub_find(ljca_dev, LJCA_DIAG_STUB);
+	u8 level;
+	int ret;
+
+	if (kstrtou8(buf, 0, &level))
+		return -EINVAL;
+
+	ret = ljca_diag_set_trace_level(diag_stub, level);
+	if (ret)
+		return ret;
+
+	return count;
+}
+static DEVICE_ATTR_WO(ljca_trace_level);
+
+static ssize_t ljca_version_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct usb_interface *intf = to_usb_interface(dev);
+	struct ljca_dev *ljca_dev = usb_get_intfdata(intf);
+	struct ljca_stub *stub = ljca_stub_find(ljca_dev, LJCA_MNG_STUB);
+
+	return ljca_mng_get_version(stub, buf);
+}
+static DEVICE_ATTR_RO(ljca_version);
+
+static struct attribute *ljca_attrs[] = {
+	&dev_attr_ljca_version.attr,
+	&dev_attr_ljca_enable_dfu.attr,
+	&dev_attr_ljca_trace_level.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(ljca);
+
+static int ljca_probe(struct usb_interface *intf, const struct usb_device_id *id)
+{
+	struct ljca_dev *dev;
+	struct usb_endpoint_descriptor *bulk_in, *bulk_out;
+	int ret;
+
+	/* allocate memory for our device state and initialize it */
+	dev = devm_kzalloc(&intf->dev, sizeof(*dev), GFP_KERNEL);
+	if (!dev)
+		return -ENOMEM;
+
+	ljca_init(dev);
+	dev->intf = usb_get_intf(intf);
+
+	/* set up the endpoint information use only the first bulk-in and bulk-out endpoints */
+	ret = usb_find_common_endpoints(intf->cur_altsetting, &bulk_in, &bulk_out, NULL, NULL);
+	if (ret) {
+		dev_err(&intf->dev, "could not find both bulk-in and bulk-out endpoints\n");
+		goto error;
+	}
+
+	dev->ibuf_len = usb_endpoint_maxp(bulk_in);
+	dev->in_ep = bulk_in->bEndpointAddress;
+	dev->ibuf = devm_kzalloc(&intf->dev, dev->ibuf_len, GFP_KERNEL);
+	if (!dev->ibuf) {
+		ret = -ENOMEM;
+		goto error;
+	}
+
+	dev->in_urb = usb_alloc_urb(0, GFP_KERNEL);
+	if (!dev->in_urb) {
+		ret = -ENOMEM;
+		goto error;
+	}
+
+	dev->out_ep = bulk_out->bEndpointAddress;
+	/* save our data pointer in this intf device */
+	usb_set_intfdata(intf, dev);
+	ret = ljca_start(dev);
+	if (ret) {
+		dev_err(&intf->dev, "bridge read start failed ret %d\n", ret);
+		goto error;
+	}
+
+	ret = ljca_mng_init(dev);
+	if (ret) {
+		dev_err(&intf->dev, "register mng stub failed ret %d\n", ret);
+		goto error_stop;
+	}
+
+	ret = ljca_diag_init(dev);
+	if (ret) {
+		dev_err(&intf->dev, "register diag stub failed ret %d\n", ret);
+		goto error_stop;
+	}
+
+	usb_enable_autosuspend(interface_to_usbdev(dev->intf));
+
+	return 0;
+
+error_stop:
+	ljca_stop(dev);
+	ljca_cleanup_aux_dev(dev);
+	ljca_stub_cleanup(dev);
+error:
+	dev_err(&intf->dev, "LJCA USB device init failed\n");
+	ljca_delete(dev);
+
+	return ret;
+}
+
+static void ljca_disconnect(struct usb_interface *intf)
+{
+	struct ljca_dev *dev = usb_get_intfdata(intf);
+
+	ljca_stop(dev);
+	ljca_cleanup_aux_dev(dev);
+	ljca_stub_cleanup(dev);
+	ljca_delete(dev);
+}
+
+static int ljca_suspend(struct usb_interface *intf, pm_message_t message)
+{
+	struct ljca_dev *dev = usb_get_intfdata(intf);
+
+	ljca_stop(dev);
+
+	return 0;
+}
+
+static int ljca_resume(struct usb_interface *intf)
+{
+	struct ljca_dev *dev = usb_get_intfdata(intf);
+
+	return ljca_start(dev);
+}
+
+static const struct usb_device_id ljca_table[] = {
+	{USB_DEVICE(0x8086, 0x0b63)},
+	{}
+};
+MODULE_DEVICE_TABLE(usb, ljca_table);
+
+static struct usb_driver ljca_driver = {
+	.name = "ljca",
+	.probe = ljca_probe,
+	.disconnect = ljca_disconnect,
+	.suspend = ljca_suspend,
+	.resume = ljca_resume,
+	.id_table = ljca_table,
+	.dev_groups = ljca_groups,
+	.supports_autosuspend = 1,
+};
+module_usb_driver(ljca_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_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 000000000000..f209fa3ba54a
--- /dev/null
+++ b/include/linux/usb/ljca.h
@@ -0,0 +1,50 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _LINUX_USB_LJCA_H_
+#define _LINUX_USB_LJCA_H_
+
+#include <linux/types.h>
+
+struct ljca_dev;
+struct auxiliary_device;
+
+struct ljca {
+	struct auxiliary_device auxdev;
+	u8 type;
+	u8 id;
+	struct ljca_dev *dev;
+};
+
+#define auxiliary_dev_to_ljca(auxiliary_dev) container_of(auxiliary_dev, struct ljca, auxdev)
+
+#define LJCA_MAX_GPIO_NUM 64
+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;
+};
+
+/**
+ * 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);
+
+#endif
-- 
2.34.1


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

* [PATCH v8 2/6] usb: ljca: Add transport interfaces for sub-module drivers
  2023-05-11 17:58 [PATCH v8 0/6] Add Intel LJCA device driver Ye Xiang
  2023-05-11 17:58 ` [PATCH v8 1/6] usb: Add support for Intel LJCA device Ye Xiang
@ 2023-05-11 17:58 ` Ye Xiang
  2023-05-22  9:36   ` Oliver Neukum
  2023-05-11 17:58 ` [PATCH v8 3/6] Documentation: Add ABI doc for attributes of LJCA device Ye Xiang
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 22+ messages in thread
From: Ye Xiang @ 2023-05-11 17:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Arnd Bergmann, Matthias Kaehlcke, Lee Jones,
	Wolfram Sang, Tyrone Ting, Mark Brown, Linus Walleij,
	Marc Zyngier, Bartosz Golaszewski, linux-usb, linux-i2c,
	linux-kernel, linux-spi, linux-gpio
  Cc: srinivas.pandruvada, heikki.krogerus, andriy.shevchenko,
	sakari.ailus, zhifeng.wang, wentong.wu, lixu.zhang, Ye Xiang

Adds the transport interfaces for various LJCA sub-module drivers
to communicate with LJCA hardware. 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.

Signed-off-by: Ye Xiang <xiang.ye@intel.com>
---
 drivers/usb/misc/usb-ljca.c | 91 +++++++++++++++++++++++++++++++++++++
 include/linux/usb/ljca.h    | 52 +++++++++++++++++++++
 2 files changed, 143 insertions(+)

diff --git a/drivers/usb/misc/usb-ljca.c b/drivers/usb/misc/usb-ljca.c
index 4f76f73fdcac..3b68c9e472fc 100644
--- a/drivers/usb/misc/usb-ljca.c
+++ b/drivers/usb/misc/usb-ljca.c
@@ -368,6 +368,97 @@ static int ljca_stub_write(struct ljca_stub *stub, u8 cmd, const void *obuf, uns
 	return ret;
 }
 
+static int ljca_transfer_internal(struct ljca *ljca, u8 cmd, const void *obuf,
+				  unsigned int obuf_len, void *ibuf, unsigned int *ibuf_len,
+				  bool wait_ack)
+{
+	struct ljca_stub *stub;
+
+	stub = ljca_stub_find(ljca->dev, ljca->type);
+	if (IS_ERR(stub))
+		return PTR_ERR(stub);
+
+	return ljca_stub_write(stub, cmd, obuf, obuf_len, ibuf, ibuf_len, wait_ack,
+			       LJCA_USB_WRITE_ACK_TIMEOUT_MS);
+}
+
+int ljca_transfer(struct ljca *ljca, u8 cmd, const void *obuf, unsigned int obuf_len, void *ibuf,
+		  unsigned int *ibuf_len)
+{
+	return ljca_transfer_internal(ljca, cmd, obuf, obuf_len, ibuf, ibuf_len, true);
+}
+EXPORT_SYMBOL_NS_GPL(ljca_transfer, LJCA);
+
+int ljca_transfer_noack(struct ljca *ljca, u8 cmd, const void *obuf, unsigned int obuf_len)
+{
+	return ljca_transfer_internal(ljca, cmd, obuf, obuf_len, NULL, NULL, false);
+}
+EXPORT_SYMBOL_NS_GPL(ljca_transfer_noack, LJCA);
+
+int ljca_register_event_cb(struct ljca *ljca, ljca_event_cb_t event_cb, void *context)
+{
+	struct ljca_event_cb_entry *entry, *iter;
+	struct ljca_stub *stub;
+	unsigned long flags;
+	int ret;
+
+	stub = ljca_stub_find(ljca->dev, ljca->type);
+	if (IS_ERR(stub))
+		return PTR_ERR(stub);
+
+	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry)
+		return -ENOMEM;
+
+	entry->notify = event_cb;
+	entry->context = context;
+	entry->id = ljca->id;
+
+	spin_lock_irqsave(&stub->event_cb_lock, flags);
+
+	list_for_each_entry(iter, &stub->event_entrys, list) {
+		if (iter->id == ljca->id) {
+			ret = -EBUSY;
+			break;
+		}
+	}
+
+	if (!ret)
+		list_add_tail(&entry->list, &stub->event_entrys);
+
+	spin_unlock_irqrestore(&stub->event_cb_lock, flags);
+
+	if (ret)
+		kfree(entry);
+
+	return ret;
+}
+EXPORT_SYMBOL_NS_GPL(ljca_register_event_cb, LJCA);
+
+void ljca_unregister_event_cb(struct ljca *ljca)
+{
+	struct ljca_stub *stub;
+	struct ljca_event_cb_entry *iter;
+	unsigned long flags;
+
+	stub = ljca_stub_find(ljca->dev, ljca->type);
+	if (IS_ERR(stub))
+		return;
+
+	spin_lock_irqsave(&stub->event_cb_lock, flags);
+
+	list_for_each_entry(iter, &stub->event_entrys, list) {
+		if (iter->id == ljca->id) {
+			list_del_init(&iter->list);
+			kfree(iter);
+			break;
+		}
+	}
+
+	spin_unlock_irqrestore(&stub->event_cb_lock, flags);
+}
+EXPORT_SYMBOL_NS_GPL(ljca_unregister_event_cb, LJCA);
+
 static void ljca_read_complete(struct urb *urb)
 {
 	struct ljca_msg *header = urb->transfer_buffer;
diff --git a/include/linux/usb/ljca.h b/include/linux/usb/ljca.h
index f209fa3ba54a..74e9729d05ec 100644
--- a/include/linux/usb/ljca.h
+++ b/include/linux/usb/ljca.h
@@ -47,4 +47,56 @@ struct ljca_spi_info {
  */
 typedef void (*ljca_event_cb_t)(void *context, u8 cmd, const void *evt_data, int len);
 
+/**
+ * ljca_register_event_cb - register a callback function to receive events
+ *
+ * @ljca: ljca device handle
+ * @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 *ljca, ljca_event_cb_t event_cb, void *context);
+
+/**
+ * ljca_unregister_event_cb - unregister the callback function for an event
+ *
+ * @ljca: ljca device handle
+ */
+void ljca_unregister_event_cb(struct ljca *ljca);
+
+/**
+ * ljca_transfer - issue a LJCA command and wait for a response and the
+ * associated data
+ *
+ * @ljca: ljca device handle
+ * @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; it will be modified
+ *	to indicate the actual data transferred; it shouldn't be NULL as well
+ *	when ibuf isn't NULL
+ *
+ * Return: 0 for success, negative value for errors
+ */
+int ljca_transfer(struct ljca *ljca, 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
+ *
+ * @ljca: ljca device handle
+ * @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 *ljca, u8 cmd, const void *obuf, unsigned int obuf_len);
+
 #endif
-- 
2.34.1


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

* [PATCH v8 3/6] Documentation: Add ABI doc for attributes of LJCA device
  2023-05-11 17:58 [PATCH v8 0/6] Add Intel LJCA device driver Ye Xiang
  2023-05-11 17:58 ` [PATCH v8 1/6] usb: Add support for Intel LJCA device Ye Xiang
  2023-05-11 17:58 ` [PATCH v8 2/6] usb: ljca: Add transport interfaces for sub-module drivers Ye Xiang
@ 2023-05-11 17:58 ` Ye Xiang
  2023-05-11 17:58 ` [PATCH v8 4/6] gpio: Add support for Intel LJCA USB GPIO driver Ye Xiang
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 22+ messages in thread
From: Ye Xiang @ 2023-05-11 17:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Arnd Bergmann, Matthias Kaehlcke, Lee Jones,
	Wolfram Sang, Tyrone Ting, Mark Brown, Linus Walleij,
	Marc Zyngier, Bartosz Golaszewski, linux-usb, linux-i2c,
	linux-kernel, linux-spi, linux-gpio
  Cc: srinivas.pandruvada, heikki.krogerus, andriy.shevchenko,
	sakari.ailus, zhifeng.wang, wentong.wu, lixu.zhang, Ye Xiang

Add sysfs attributes Documentation entries for LJCA device

Signed-off-by: Ye Xiang <xiang.ye@intel.com>
---
 .../ABI/testing/sysfs-bus-usb-devices-ljca    | 36 +++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-usb-devices-ljca

diff --git a/Documentation/ABI/testing/sysfs-bus-usb-devices-ljca b/Documentation/ABI/testing/sysfs-bus-usb-devices-ljca
new file mode 100644
index 000000000000..16eecaf870e2
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-usb-devices-ljca
@@ -0,0 +1,36 @@
+What:		/sys/bus/usb/.../ljca_version
+Date:		July 2023
+KernelVersion:	6.4
+Contact:	Ye Xiang <xiang.ye@intel.com>
+Description:
+		Provides the current firmware version of LJCA device.
+		The format is Major.Minor.Patch.Build, where
+		Major, Minor, Patch, and Build are decimal numbers.
+		For example: 1.0.0.256
+
+What:		/sys/bus/usb/.../ljca_enable_dfu
+Date:		July 2023
+KernelVersion:	6.4
+Contact:	Ye Xiang <xiang.ye@intel.com>
+Description:
+		Writing 1 to this file to force the LJCA device into DFU
+		mode so the firmware can be updated. After firmware
+		updating has been done, the device will back to normal
+		working mode.
+
+What:		/sys/bus/usb/.../ljca_trace_level
+Date:		July 2023
+KernelVersion:	6.4
+Contact:	Ye Xiang <xiang.ye@intel.com>
+Description:
+		Writing N to this file to set firmware log level of LJCA
+		device. The log can be printed to another computer through
+		UART ports in LJCA device. Valid values:
+
+		  ==	==========
+		   0	LEVEL_ERROR
+		   1	LEVEL_WARNING
+		   2	LEVEL_INFO
+		   3	LEVEL_DEBUG
+		   4	LEVEL_OFF
+		  ==	==========
-- 
2.34.1


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

* [PATCH v8 4/6] gpio: Add support for Intel LJCA USB GPIO driver
  2023-05-11 17:58 [PATCH v8 0/6] Add Intel LJCA device driver Ye Xiang
                   ` (2 preceding siblings ...)
  2023-05-11 17:58 ` [PATCH v8 3/6] Documentation: Add ABI doc for attributes of LJCA device Ye Xiang
@ 2023-05-11 17:58 ` Ye Xiang
  2023-05-11 21:07   ` Linus Walleij
                     ` (2 more replies)
  2023-05-11 17:58 ` [PATCH v8 5/6] i2c: Add support for Intel LJCA USB I2C driver Ye Xiang
                   ` (2 subsequent siblings)
  6 siblings, 3 replies; 22+ messages in thread
From: Ye Xiang @ 2023-05-11 17:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Arnd Bergmann, Matthias Kaehlcke, Lee Jones,
	Wolfram Sang, Tyrone Ting, Mark Brown, Linus Walleij,
	Marc Zyngier, Bartosz Golaszewski, linux-usb, linux-i2c,
	linux-kernel, linux-spi, linux-gpio
  Cc: srinivas.pandruvada, heikki.krogerus, andriy.shevchenko,
	sakari.ailus, zhifeng.wang, wentong.wu, lixu.zhang, Ye Xiang

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

Signed-off-by: Ye Xiang <xiang.ye@intel.com>
---
 drivers/gpio/Kconfig     |  12 +
 drivers/gpio/Makefile    |   1 +
 drivers/gpio/gpio-ljca.c | 479 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 492 insertions(+)
 create mode 100644 drivers/gpio/gpio-ljca.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 13be729710f2..bbf00e157dcd 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1658,6 +1658,18 @@ config GPIO_VIPERBOARD
 	  River Tech's viperboard.h for detailed meaning
 	  of the module parameters.
 
+config GPIO_LJCA
+	tristate "INTEL La Jolla Cove Adapter GPIO support"
+	depends on USB_LJCA
+	select GPIOLIB_IRQCHIP
+	default USB_LJCA
+	help
+	  Select this option to enable GPIO 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 gpio-ljca.
+
 endmenu
 
 menu "Virtual GPIO drivers"
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index c048ba003367..eb59524d18c0 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -77,6 +77,7 @@ obj-$(CONFIG_GPIO_IXP4XX)		+= gpio-ixp4xx.o
 obj-$(CONFIG_GPIO_JANZ_TTL)		+= gpio-janz-ttl.o
 obj-$(CONFIG_GPIO_KEMPLD)		+= gpio-kempld.o
 obj-$(CONFIG_GPIO_LATCH)		+= gpio-latch.o
+obj-$(CONFIG_GPIO_LJCA) 		+= gpio-ljca.o
 obj-$(CONFIG_GPIO_LOGICVC)		+= gpio-logicvc.o
 obj-$(CONFIG_GPIO_LOONGSON1)		+= gpio-loongson1.o
 obj-$(CONFIG_GPIO_LOONGSON)		+= gpio-loongson.o
diff --git a/drivers/gpio/gpio-ljca.c b/drivers/gpio/gpio-ljca.c
new file mode 100644
index 000000000000..81835a21e8c0
--- /dev/null
+++ b/drivers/gpio/gpio-ljca.c
@@ -0,0 +1,479 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Intel La Jolla Cove Adapter USB-GPIO driver
+ *
+ * Copyright (c) 2023, Intel Corporation.
+ */
+
+#include <linux/acpi.h>
+#include <linux/auxiliary_bus.h>
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/dev_printk.h>
+#include <linux/gpio/driver.h>
+#include <linux/irq.h>
+#include <linux/kernel.h>
+#include <linux/kref.h>
+#include <linux/module.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_CONF_DISABLE		BIT(0)
+#define LJCA_GPIO_CONF_INPUT		BIT(1)
+#define LJCA_GPIO_CONF_OUTPUT		BIT(2)
+#define LJCA_GPIO_CONF_PULLUP		BIT(3)
+#define LJCA_GPIO_CONF_PULLDOWN		BIT(4)
+#define LJCA_GPIO_CONF_DEFAULT		BIT(5)
+#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)
+
+/* Intentional overlap with PULLUP / PULLDOWN */
+#define LJCA_GPIO_CONF_SET		BIT(3)
+#define LJCA_GPIO_CONF_CLR		BIT(4)
+
+struct gpio_op {
+	u8 index;
+	u8 value;
+} __packed;
+
+struct gpio_packet {
+	u8 num;
+	struct gpio_op item[];
+} __packed;
+
+#define LJCA_GPIO_BUF_SIZE 60
+struct ljca_gpio_dev {
+	struct ljca *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 */
+	struct mutex irq_lock;
+	struct work_struct work;
+	/* lock to 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)
+{
+	struct gpio_packet *packet = (struct gpio_packet *)ljca_gpio->obuf;
+	int ret;
+
+	mutex_lock(&ljca_gpio->trans_lock);
+	packet->item[0].index = gpio_id;
+	packet->item[0].value = config | ljca_gpio->connect_mode[gpio_id];
+	packet->num = 1;
+
+	ret = ljca_transfer(ljca_gpio->ljca, LJCA_GPIO_CONFIG, packet,
+			    struct_size(packet, item, packet->num), NULL, NULL);
+	mutex_unlock(&ljca_gpio->trans_lock);
+
+	return ret;
+}
+
+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;
+	int ret;
+
+	mutex_lock(&ljca_gpio->trans_lock);
+	packet->num = 1;
+	packet->item[0].index = gpio_id;
+	ret = ljca_transfer(ljca_gpio->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->ljca->auxdev.dev,
+			"read package error, gpio_id:%u num:%u ibuf_len:%d\n", gpio_id,
+			ack_packet->num, ibuf_len);
+		ret = -EIO;
+	}
+
+out_unlock:
+	mutex_unlock(&ljca_gpio->trans_lock);
+	if (ret)
+		return ret;
+
+	return ack_packet->item[0].value > 0;
+}
+
+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;
+	int ret;
+
+	mutex_lock(&ljca_gpio->trans_lock);
+	packet->num = 1;
+	packet->item[0].index = gpio_id;
+	packet->item[0].value = value & 1;
+
+	ret = ljca_transfer(ljca_gpio->ljca, LJCA_GPIO_WRITE, packet,
+			    struct_size(packet, item, packet->num), NULL, NULL);
+	mutex_unlock(&ljca_gpio->trans_lock);
+
+	return ret;
+}
+
+static int ljca_gpio_get_value(struct gpio_chip *chip, unsigned int offset)
+{
+	struct ljca_gpio_dev *ljca_gpio = gpiochip_get_data(chip);
+
+	return ljca_gpio_read(ljca_gpio, offset);
+}
+
+static void ljca_gpio_set_value(struct gpio_chip *chip, unsigned int offset, int val)
+{
+	struct ljca_gpio_dev *ljca_gpio = gpiochip_get_data(chip);
+	int ret;
+
+	ret = ljca_gpio_write(ljca_gpio, offset, val);
+	if (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)
+{
+	struct ljca_gpio_dev *ljca_gpio = gpiochip_get_data(chip);
+	u8 config = LJCA_GPIO_CONF_INPUT | LJCA_GPIO_CONF_CLR;
+	int ret;
+
+	ret = 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, unsigned int offset, int val)
+{
+	struct ljca_gpio_dev *ljca_gpio = gpiochip_get_data(chip);
+	u8 config = LJCA_GPIO_CONF_OUTPUT | LJCA_GPIO_CONF_CLR;
+	int ret;
+
+	ret = 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)
+{
+	struct ljca_gpio_dev *ljca_gpio = gpiochip_get_data(chip);
+
+	ljca_gpio->connect_mode[offset] = 0;
+	switch (pinconf_to_config_param(config)) {
+	case PIN_CONFIG_BIAS_PULL_UP:
+		ljca_gpio->connect_mode[offset] |= LJCA_GPIO_CONF_PULLUP;
+		break;
+	case PIN_CONFIG_BIAS_PULL_DOWN:
+		ljca_gpio->connect_mode[offset] |= LJCA_GPIO_CONF_PULLDOWN;
+		break;
+	case PIN_CONFIG_DRIVE_PUSH_PULL:
+	case PIN_CONFIG_PERSIST_STATE:
+		break;
+	default:
+		return -ENOTSUPP;
+	}
+
+	return 0;
+}
+
+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);
+
+	WARN_ON_ONCE(ngpios != ljca_gpio->gpio_info->num);
+	bitmap_copy(valid_mask, ljca_gpio->gpio_info->valid_pin_map, ngpios);
+
+	return 0;
+}
+
+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)
+{
+	struct gpio_packet *packet = (struct gpio_packet *)ljca_gpio->obuf;
+	int ret;
+
+	mutex_lock(&ljca_gpio->trans_lock);
+	packet->num = 1;
+	packet->item[0].index = gpio_id;
+	packet->item[0].value = 0;
+
+	ret = ljca_transfer(ljca_gpio->ljca, enable ? LJCA_GPIO_INT_UNMASK : LJCA_GPIO_INT_MASK,
+			    packet, struct_size(packet, item, packet->num), NULL, NULL);
+	mutex_unlock(&ljca_gpio->trans_lock);
+
+	return ret;
+}
+
+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;
+
+	for_each_set_bit(gpio_id, ljca_gpio->reenable_irqs, ljca_gpio->gc.ngpio) {
+		clear_bit(gpio_id, ljca_gpio->reenable_irqs);
+		unmasked = test_bit(gpio_id, ljca_gpio->unmasked_irqs);
+		if (unmasked)
+			ljca_enable_irq(ljca_gpio, gpio_id, true);
+	}
+}
+
+static void ljca_gpio_event_cb(void *context, u8 cmd, const void *evt_data, int len)
+{
+	const struct gpio_packet *packet = evt_data;
+	struct ljca_gpio_dev *ljca_gpio = context;
+	int i;
+	int 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);
+		if (!irq) {
+			dev_err(ljca_gpio->gc.parent, "gpio_id %u does not mapped to IRQ yet\n",
+				packet->item[i].index);
+			return;
+		}
+
+		generic_handle_domain_irq(ljca_gpio->gc.irq.domain, irq);
+		set_bit(packet->item[i].index, ljca_gpio->reenable_irqs);
+	}
+
+	schedule_work(&ljca_gpio->work);
+}
+
+static void ljca_irq_unmask(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);
+
+	gpiochip_enable_irq(gc, gpio_id);
+	set_bit(gpio_id, ljca_gpio->unmasked_irqs);
+}
+
+static void ljca_irq_mask(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);
+
+	clear_bit(gpio_id, ljca_gpio->unmasked_irqs);
+	gpiochip_disable_irq(gc, gpio_id);
+}
+
+static int ljca_irq_set_type(struct irq_data *irqd, unsigned int type)
+{
+	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);
+
+	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);
+		break;
+	case IRQ_TYPE_LEVEL_LOW:
+		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);
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		ljca_gpio->connect_mode[gpio_id] |= (LJCA_GPIO_CONF_EDGE | LJCA_GPIO_CONF_PULLDOWN);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void ljca_irq_bus_lock(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);
+
+	mutex_lock(&ljca_gpio->irq_lock);
+}
+
+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;
+
+	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_enable_irq(ljca_gpio, gpio_id, true);
+			set_bit(gpio_id, ljca_gpio->enabled_irqs);
+		} else {
+			ljca_enable_irq(ljca_gpio, gpio_id, false);
+			clear_bit(gpio_id, ljca_gpio->enabled_irqs);
+		}
+	}
+
+	mutex_unlock(&ljca_gpio->irq_lock);
+}
+
+static const struct irq_chip ljca_gpio_irqchip = {
+	.name = "ljca-irq",
+	.irq_mask = ljca_irq_mask,
+	.irq_unmask = ljca_irq_unmask,
+	.irq_set_type = ljca_irq_set_type,
+	.irq_bus_lock = ljca_irq_bus_lock,
+	.irq_bus_sync_unlock = ljca_irq_bus_unlock,
+	.flags = IRQCHIP_IMMUTABLE,
+	GPIOCHIP_IRQ_RESOURCE_HELPERS,
+};
+
+static int ljca_gpio_probe(struct auxiliary_device *auxdev,
+			   const struct auxiliary_device_id *aux_dev_id)
+{
+	struct ljca *ljca = auxiliary_dev_to_ljca(auxdev);
+	struct ljca_gpio_dev *ljca_gpio;
+	struct gpio_irq_chip *girq;
+	int ret;
+
+	ljca_gpio = devm_kzalloc(&auxdev->dev, sizeof(*ljca_gpio), GFP_KERNEL);
+	if (!ljca_gpio)
+		return -ENOMEM;
+
+	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->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 = &auxdev->dev;
+
+	ljca_gpio->gc.base = -1;
+	ljca_gpio->gc.ngpio = ljca_gpio->gpio_info->num;
+	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;
+
+	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);
+	girq->parent_handler = NULL;
+	girq->num_parents = 0;
+	girq->parents = NULL;
+	girq->default_type = IRQ_TYPE_NONE;
+	girq->handler = handle_simple_irq;
+	girq->init_valid_mask = ljca_gpio_irq_init_valid_mask;
+
+	INIT_WORK(&ljca_gpio->work, ljca_gpio_async);
+	ret = gpiochip_add_data(&ljca_gpio->gc, ljca_gpio);
+	if (ret) {
+		ljca_unregister_event_cb(ljca);
+		mutex_destroy(&ljca_gpio->irq_lock);
+		mutex_destroy(&ljca_gpio->trans_lock);
+	}
+
+	return ret;
+}
+
+static void ljca_gpio_remove(struct auxiliary_device *auxdev)
+{
+	struct ljca_gpio_dev *ljca_gpio = auxiliary_get_drvdata(auxdev);
+
+	gpiochip_remove(&ljca_gpio->gc);
+	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);
+}
+
+#define LJCA_GPIO_DRV_NAME "ljca.ljca-gpio"
+static const struct auxiliary_device_id ljca_gpio_id_table[] = {
+	{ LJCA_GPIO_DRV_NAME, 0 },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(auxiliary, ljca_gpio_id_table);
+
+static struct auxiliary_driver ljca_gpio_driver = {
+	.probe = ljca_gpio_probe,
+	.remove = ljca_gpio_remove,
+	.id_table = ljca_gpio_id_table,
+};
+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_DESCRIPTION("Intel La Jolla Cove Adapter USB-GPIO driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(LJCA);
-- 
2.34.1


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

* [PATCH v8 5/6] i2c: Add support for Intel LJCA USB I2C driver
  2023-05-11 17:58 [PATCH v8 0/6] Add Intel LJCA device driver Ye Xiang
                   ` (3 preceding siblings ...)
  2023-05-11 17:58 ` [PATCH v8 4/6] gpio: Add support for Intel LJCA USB GPIO driver Ye Xiang
@ 2023-05-11 17:58 ` Ye Xiang
  2023-05-22  9:50   ` Oliver Neukum
  2023-05-11 17:58 ` [PATCH v8 6/6] spi: Add support for Intel LJCA USB SPI driver Ye Xiang
  2023-05-13  8:50 ` [PATCH v8 0/6] Add Intel LJCA device driver Greg Kroah-Hartman
  6 siblings, 1 reply; 22+ messages in thread
From: Ye Xiang @ 2023-05-11 17:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Arnd Bergmann, Matthias Kaehlcke, Lee Jones,
	Wolfram Sang, Tyrone Ting, Mark Brown, Linus Walleij,
	Marc Zyngier, Bartosz Golaszewski, linux-usb, linux-i2c,
	linux-kernel, linux-spi, linux-gpio
  Cc: srinivas.pandruvada, heikki.krogerus, andriy.shevchenko,
	sakari.ailus, zhifeng.wang, wentong.wu, lixu.zhang, Ye Xiang

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: Ye Xiang <xiang.ye@intel.com>
---
 drivers/i2c/busses/Kconfig    |  11 ++
 drivers/i2c/busses/Makefile   |   1 +
 drivers/i2c/busses/i2c-ljca.c | 350 ++++++++++++++++++++++++++++++++++
 3 files changed, 362 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-ljca.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 25eb4e8fd22f..37fed62797a9 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1261,6 +1261,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 af56fe2c75c0..4af5b06ef288 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 000000000000..a173c4326a28
--- /dev/null
+++ b/drivers/i2c/busses/i2c-ljca.c
@@ -0,0 +1,350 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Intel La Jolla Cove Adapter USB-I2C driver
+ *
+ * Copyright (c) 2023, Intel Corporation.
+ */
+
+#include <linux/auxiliary_bus.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/dev_printk.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/usb/ljca.h>
+
+/* I2C 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 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)
+
+/* I2C Transfer */
+struct i2c_xfer {
+	u8 id;
+	u8 slave;
+	u16 flag; /* speed, 8/16bit addr, addr increase, etc */
+	u16 addr;
+	u16 len;
+	u8 data[];
+} __packed;
+
+/* I2C raw commands: Init/Start/Read/Write/Stop */
+struct i2c_rw_packet {
+	u8 id;
+	__le16 len;
+	u8 data[];
+} __packed;
+
+#define LJCA_I2C_BUF_SIZE	60
+#define LJCA_I2C_MAX_XFER_SIZE	(LJCA_I2C_BUF_SIZE - sizeof(struct i2c_rw_packet))
+
+struct ljca_i2c_dev {
+	struct ljca *ljca;
+	struct ljca_i2c_info *i2c_info;
+	struct i2c_adapter adap;
+
+	u8 obuf[LJCA_I2C_BUF_SIZE];
+	u8 ibuf[LJCA_I2C_BUF_SIZE];
+};
+
+static u8 ljca_i2c_format_slave_addr(u8 slave_addr, u8 type)
+{
+	return (slave_addr << 1) | type;
+}
+
+static int ljca_i2c_init(struct ljca_i2c_dev *ljca_i2c, u8 id)
+{
+	struct i2c_rw_packet *w_packet = (struct i2c_rw_packet *)ljca_i2c->obuf;
+
+	memset(w_packet, 0, sizeof(*w_packet));
+	w_packet->id = id;
+	w_packet->data[0] = LJCA_I2C_INIT_FLAG_FREQ_400K;
+	w_packet->len = cpu_to_le16(sizeof(*w_packet->data));
+
+	return ljca_transfer(ljca_i2c->ljca, LJCA_I2C_INIT, w_packet,
+			     struct_size(w_packet, data, 1), NULL, NULL);
+}
+
+static int ljca_i2c_start(struct ljca_i2c_dev *ljca_i2c, u8 slave_addr, enum ljca_xfer_type type)
+{
+	struct i2c_rw_packet *w_packet = (struct i2c_rw_packet *)ljca_i2c->obuf;
+	struct i2c_rw_packet *r_packet = (struct i2c_rw_packet *)ljca_i2c->ibuf;
+	unsigned int ibuf_len = LJCA_I2C_BUF_SIZE;
+	int ret;
+	s16 rp_len;
+
+	memset(w_packet, 0, sizeof(*w_packet));
+	w_packet->id = ljca_i2c->i2c_info->id;
+	w_packet->data[0] = ljca_i2c_format_slave_addr(slave_addr, type);
+	w_packet->len = cpu_to_le16(sizeof(*w_packet->data));
+
+	ret = ljca_transfer(ljca_i2c->ljca, LJCA_I2C_START, w_packet,
+			    struct_size(w_packet, data, 1), r_packet, &ibuf_len);
+	if (ret)
+		return ret;
+
+	if (ibuf_len < sizeof(*r_packet))
+		return -EIO;
+
+	rp_len = le16_to_cpu(r_packet->len);
+	if (rp_len < 0 || r_packet->id != w_packet->id) {
+		dev_err(&ljca_i2c->adap.dev, "i2c start failed len:%d id:%d %d\n", rp_len,
+			r_packet->id, w_packet->id);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int ljca_i2c_stop(struct ljca_i2c_dev *ljca_i2c, u8 slave_addr)
+{
+	struct i2c_rw_packet *w_packet = (struct i2c_rw_packet *)ljca_i2c->obuf;
+	struct i2c_rw_packet *r_packet = (struct i2c_rw_packet *)ljca_i2c->ibuf;
+	unsigned int ibuf_len = LJCA_I2C_BUF_SIZE;
+	int ret;
+	s16 rp_len;
+
+	memset(w_packet, 0, sizeof(*w_packet));
+	w_packet->id = ljca_i2c->i2c_info->id;
+	w_packet->data[0] = 0;
+	w_packet->len = cpu_to_le16(sizeof(*w_packet->data));
+
+	ret = ljca_transfer(ljca_i2c->ljca, LJCA_I2C_STOP, w_packet,
+			    struct_size(w_packet, data, 1), r_packet, &ibuf_len);
+	if (ret)
+		return ret;
+
+	if (ibuf_len < sizeof(*r_packet))
+		return -EIO;
+
+	rp_len = le16_to_cpu(r_packet->len);
+	if (rp_len < 0 || r_packet->id != w_packet->id) {
+		dev_err(&ljca_i2c->adap.dev, "i2c stop failed len:%d id:%d %d\n", rp_len,
+			r_packet->id, w_packet->id);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int ljca_i2c_pure_read(struct ljca_i2c_dev *ljca_i2c, u8 *data, u8 len)
+{
+	struct i2c_rw_packet *w_packet = (struct i2c_rw_packet *)ljca_i2c->obuf;
+	struct i2c_rw_packet *r_packet = (struct i2c_rw_packet *)ljca_i2c->ibuf;
+	unsigned int ibuf_len = LJCA_I2C_BUF_SIZE;
+	int ret;
+	s16 rp_len;
+
+	if (len > LJCA_I2C_MAX_XFER_SIZE)
+		return -EINVAL;
+
+	memset(w_packet, 0, sizeof(*w_packet));
+	w_packet->id = ljca_i2c->i2c_info->id;
+	w_packet->len = cpu_to_le16(len);
+	w_packet->data[0] = 0;
+
+	ret = ljca_transfer(ljca_i2c->ljca, LJCA_I2C_READ, w_packet,
+			    struct_size(w_packet, data, 1), r_packet, &ibuf_len);
+	if (ret)
+		return ret;
+
+	if (ibuf_len < sizeof(*r_packet))
+		return -EIO;
+
+	rp_len = le16_to_cpu(r_packet->len);
+	if (rp_len != len || r_packet->id != w_packet->id) {
+		dev_err(&ljca_i2c->adap.dev, "i2c raw read failed len:%d id:%d %d\n", rp_len,
+			r_packet->id, w_packet->id);
+		return -EIO;
+	}
+
+	memcpy(data, r_packet->data, len);
+
+	return 0;
+}
+
+static int ljca_i2c_read(struct ljca_i2c_dev *ljca_i2c, u8 slave_addr, u8 *data,
+			 u8 len)
+{
+	int ret;
+
+	ret = ljca_i2c_start(ljca_i2c, slave_addr, LJCA_I2C_READ_XFER_TYPE);
+	if (ret)
+		goto out_stop;
+
+	ret = ljca_i2c_pure_read(ljca_i2c, data, len);
+
+out_stop:
+	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 i2c_rw_packet *w_packet = (struct i2c_rw_packet *)ljca_i2c->obuf;
+	struct i2c_rw_packet *r_packet = (struct i2c_rw_packet *)ljca_i2c->ibuf;
+	unsigned int ibuf_len = LJCA_I2C_BUF_SIZE;
+	s16 rplen;
+	int ret;
+
+	if (len > LJCA_I2C_MAX_XFER_SIZE)
+		return -EINVAL;
+
+	memset(w_packet, 0, sizeof(*w_packet));
+	w_packet->id = ljca_i2c->i2c_info->id;
+	w_packet->len = cpu_to_le16(len);
+	memcpy(w_packet->data, data, len);
+
+	ret = ljca_transfer(ljca_i2c->ljca, LJCA_I2C_WRITE, w_packet,
+			    struct_size(w_packet, data, len), r_packet, &ibuf_len);
+	if (ret)
+		return ret;
+
+	if (ibuf_len < sizeof(*r_packet))
+		return -EIO;
+
+	rplen = le16_to_cpu(r_packet->len);
+	if (rplen != len || r_packet->id != w_packet->id) {
+		dev_err(&ljca_i2c->adap.dev, "i2c write failed len:%d id:%d/%d\n", rplen,
+			r_packet->id, w_packet->id);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int ljca_i2c_write(struct ljca_i2c_dev *ljca_i2c, u8 slave_addr, u8 *data, u8 len)
+{
+	int ret;
+
+	if (!data)
+		return -EINVAL;
+
+	ret = ljca_i2c_start(ljca_i2c, slave_addr, LJCA_I2C_WRITE_XFER_TYPE);
+	if (ret)
+		goto out_stop;
+
+	ret = ljca_i2c_pure_write(ljca_i2c, data, len);
+
+out_stop:
+	ljca_i2c_stop(ljca_i2c, slave_addr);
+
+	return ret;
+}
+
+static int ljca_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msg, int num)
+{
+	struct ljca_i2c_dev *ljca_i2c;
+	struct i2c_msg *cur_msg;
+	int i, ret;
+
+	ljca_i2c = i2c_get_adapdata(adapter);
+	if (!ljca_i2c)
+		return -EINVAL;
+
+	for (i = 0; i < num; i++) {
+		cur_msg = &msg[i];
+		if (cur_msg->flags & I2C_M_RD)
+			ret = ljca_i2c_read(ljca_i2c, cur_msg->addr, cur_msg->buf, cur_msg->len);
+		else
+			ret = ljca_i2c_write(ljca_i2c, cur_msg->addr, cur_msg->buf, cur_msg->len);
+
+		if (ret)
+			return ret;
+	}
+
+	return num;
+}
+
+static u32 ljca_i2c_func(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static const struct i2c_adapter_quirks ljca_i2c_quirks = {
+	.max_read_len = LJCA_I2C_MAX_XFER_SIZE,
+	.max_write_len = LJCA_I2C_MAX_XFER_SIZE,
+};
+
+static const struct i2c_algorithm ljca_i2c_algo = {
+	.master_xfer = ljca_i2c_xfer,
+	.functionality = ljca_i2c_func,
+};
+
+static int ljca_i2c_probe(struct auxiliary_device *auxdev,
+			  const struct auxiliary_device_id *aux_dev_id)
+{
+	struct ljca *ljca = auxiliary_dev_to_ljca(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;
+	device_set_node(&ljca_i2c->adap.dev, dev_fwnode(&auxdev->dev));
+	i2c_set_adapdata(&ljca_i2c->adap, ljca_i2c);
+	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);
+
+	auxiliary_set_drvdata(auxdev, ljca_i2c);
+
+	ret = ljca_i2c_init(ljca_i2c, ljca_i2c->i2c_info->id);
+	if (ret) {
+		dev_err(&auxdev->dev, "i2c init failed id:%d\n", ljca_i2c->i2c_info->id);
+		return -EIO;
+	}
+
+	return devm_i2c_add_adapter(&auxdev->dev, &ljca_i2c->adap);
+}
+
+#define LJCA_I2C_DRV_NAME "ljca.ljca-i2c"
+static const struct auxiliary_device_id ljca_i2c_id_table[] = {
+	{ LJCA_I2C_DRV_NAME, 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("Ye Xiang <xiang.ye@intel.com>");
+MODULE_AUTHOR("Wang Zhifeng <zhifeng.wang@intel.com>");
+MODULE_AUTHOR("Zhang Lixu <lixu.zhang@intel.com>");
+MODULE_DESCRIPTION("Intel La Jolla Cove Adapter USB-I2C driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(LJCA);
-- 
2.34.1


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

* [PATCH v8 6/6] spi: Add support for Intel LJCA USB SPI driver
  2023-05-11 17:58 [PATCH v8 0/6] Add Intel LJCA device driver Ye Xiang
                   ` (4 preceding siblings ...)
  2023-05-11 17:58 ` [PATCH v8 5/6] i2c: Add support for Intel LJCA USB I2C driver Ye Xiang
@ 2023-05-11 17:58 ` Ye Xiang
  2023-05-12  4:18   ` Mark Brown
  2023-05-13  8:50 ` [PATCH v8 0/6] Add Intel LJCA device driver Greg Kroah-Hartman
  6 siblings, 1 reply; 22+ messages in thread
From: Ye Xiang @ 2023-05-11 17:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Arnd Bergmann, Matthias Kaehlcke, Lee Jones,
	Wolfram Sang, Tyrone Ting, Mark Brown, Linus Walleij,
	Marc Zyngier, Bartosz Golaszewski, linux-usb, linux-i2c,
	linux-kernel, linux-spi, linux-gpio
  Cc: srinivas.pandruvada, heikki.krogerus, andriy.shevchenko,
	sakari.ailus, zhifeng.wang, wentong.wu, lixu.zhang, Ye Xiang

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: Ye Xiang <xiang.ye@intel.com>
---
 drivers/spi/Kconfig    |  11 ++
 drivers/spi/Makefile   |   1 +
 drivers/spi/spi-ljca.c | 290 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 302 insertions(+)
 create mode 100644 drivers/spi/spi-ljca.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 47bbba04fe3a..c3de4e20531f 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -404,6 +404,17 @@ config SPI_HISI_SFC_V3XX
 	  This enables support for HiSilicon v3xx SPI NOR flash controller
 	  found in hi16xx chipsets.
 
+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_NXP_FLEXSPI
 	tristate "NXP Flex SPI controller"
 	depends on ARCH_LAYERSCAPE || HAS_IOMEM
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index d87cf75bee6a..0d0cc1b0fb9b 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -69,6 +69,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 000000000000..7a784f509f68
--- /dev/null
+++ b/drivers/spi/spi-ljca.c
@@ -0,0 +1,290 @@
+// 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>
+
+/* SPI commands */
+enum ljca_spi_cmd {
+	LJCA_SPI_INIT = 1,
+	LJCA_SPI_READ,
+	LJCA_SPI_WRITE,
+	LJCA_SPI_WRITEREAD,
+	LJCA_SPI_DEINIT,
+};
+
+#define LJCA_SPI_BUS_MAX_HZ 48000000
+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,
+};
+
+#define LJCA_SPI_BUF_SIZE		60
+#define LJCA_SPI_MAX_XFER_SIZE		(LJCA_SPI_BUF_SIZE - sizeof(struct 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)
+
+struct spi_init_packet {
+	u8 index;
+	u8 speed;
+	u8 mode;
+} __packed;
+
+struct spi_xfer_packet {
+	u8 indicator;
+	s8 len;
+	u8 data[];
+} __packed;
+
+struct ljca_spi_dev {
+	struct ljca *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 spi_xfer_packet *w_packet = (struct spi_xfer_packet *)ljca_spi->obuf;
+	struct spi_xfer_packet *r_packet = (struct spi_xfer_packet *)ljca_spi->ibuf;
+	unsigned int ibuf_len = LJCA_SPI_BUF_SIZE;
+	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, &ibuf_len);
+	if (ret)
+		return ret;
+
+	if (ibuf_len < 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 spi_init_packet w_packet = {};
+	int ret;
+
+	if (ljca_spi->mode == mode && ljca_spi->speed == div)
+		return 0;
+
+	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);
+
+	w_packet.index = ljca_spi->spi_info->id;
+	w_packet.speed = div;
+	ret = ljca_transfer(ljca_spi->ljca, LJCA_SPI_INIT, &w_packet,
+			    sizeof(w_packet), NULL, NULL);
+	if (ret)
+		return ret;
+
+	ljca_spi->mode = mode;
+	ljca_spi->speed = div;
+
+	return 0;
+}
+
+static int ljca_spi_deinit(struct ljca_spi_dev *ljca_spi)
+{
+	struct spi_init_packet w_packet = {};
+
+	w_packet.index = ljca_spi->spi_info->id;
+	return ljca_transfer(ljca_spi->ljca, LJCA_SPI_DEINIT, &w_packet, sizeof(w_packet),
+			     NULL, NULL);
+}
+
+static inline int ljca_spi_transfer(struct ljca_spi_dev *ljca_spi, const u8 *tx_data, u8 *rx_data,
+				    u16 len)
+{
+	int remaining = len;
+	int offset = 0;
+	int cur_len;
+	int complete;
+	int i;
+	int cmd;
+	int ret;
+
+	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)
+{
+	struct ljca_spi_dev *ljca_spi = spi_controller_get_devdata(controller);
+	int ret;
+	u8 div;
+
+	div = min_t(u8, LJCA_SPI_BUS_SPEED_MIN,
+		    DIV_ROUND_UP(controller->max_speed_hz, xfer->speed_hz) / 2 - 1);
+	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 *ljca = auxiliary_dev_to_ljca(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;
+
+	auxiliary_set_drvdata(auxdev, controller);
+	ljca_spi = spi_controller_get_devdata(controller);
+
+	ljca_spi->ljca = ljca;
+	ljca_spi->spi_info = dev_get_platdata(&auxdev->dev);
+	ljca_spi->controller = controller;
+	device_set_node(&ljca_spi->controller->dev, dev_fwnode(&auxdev->dev));
+
+	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;
+
+	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)
+};
+
+#define LJCA_SPI_DRV_NAME "ljca.ljca-spi"
+static const struct auxiliary_device_id ljca_spi_id_table[] = {
+	{ LJCA_SPI_DRV_NAME, 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("Ye Xiang <xiang.ye@intel.com>");
+MODULE_AUTHOR("Wang Zhifeng <zhifeng.wang@intel.com>");
+MODULE_AUTHOR("Zhang Lixu <lixu.zhang@intel.com>");
+MODULE_DESCRIPTION("Intel La Jolla Cove Adapter USB-SPI driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(LJCA);
-- 
2.34.1


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

* Re: [PATCH v8 4/6] gpio: Add support for Intel LJCA USB GPIO driver
  2023-05-11 17:58 ` [PATCH v8 4/6] gpio: Add support for Intel LJCA USB GPIO driver Ye Xiang
@ 2023-05-11 21:07   ` Linus Walleij
  2023-06-20  7:00     ` Wu, Wentong
  2023-05-15 15:49   ` Bartosz Golaszewski
  2023-07-03  1:11   ` Wu, Wentong
  2 siblings, 1 reply; 22+ messages in thread
From: Linus Walleij @ 2023-05-11 21:07 UTC (permalink / raw)
  To: Ye Xiang
  Cc: Greg Kroah-Hartman, Arnd Bergmann, Matthias Kaehlcke, Lee Jones,
	Wolfram Sang, Tyrone Ting, Mark Brown, Marc Zyngier,
	Bartosz Golaszewski, linux-usb, linux-i2c, linux-kernel,
	linux-spi, linux-gpio, srinivas.pandruvada, heikki.krogerus,
	andriy.shevchenko, sakari.ailus, zhifeng.wang, wentong.wu,
	lixu.zhang

On Thu, May 11, 2023 at 8:01 PM Ye Xiang <xiang.ye@intel.com> wrote:

> Implements the GPIO function of Intel USB-I2C/GPIO/SPI adapter
> device named "La Jolla Cove Adapter" (LJCA). It communicate with
> LJCA GPIO module with specific protocol through interfaces exported
> by LJCA USB driver.
>
> Signed-off-by: Ye Xiang <xiang.ye@intel.com>

Overall this looks very nice.

> +static int gpio_config(struct ljca_gpio_dev *ljca_gpio, u8 gpio_id, u8 config)

All other functions are prefixed ljca_* except this one, so I think you
should do that on this one too.

With that fixes:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

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

* Re: [PATCH v8 6/6] spi: Add support for Intel LJCA USB SPI driver
  2023-05-11 17:58 ` [PATCH v8 6/6] spi: Add support for Intel LJCA USB SPI driver Ye Xiang
@ 2023-05-12  4:18   ` Mark Brown
  2023-06-20  6:47     ` Wu, Wentong
  0 siblings, 1 reply; 22+ messages in thread
From: Mark Brown @ 2023-05-12  4:18 UTC (permalink / raw)
  To: Ye Xiang
  Cc: Greg Kroah-Hartman, Arnd Bergmann, Matthias Kaehlcke, Lee Jones,
	Wolfram Sang, Tyrone Ting, Linus Walleij, Marc Zyngier,
	Bartosz Golaszewski, linux-usb, linux-i2c, linux-kernel,
	linux-spi, linux-gpio, srinivas.pandruvada, heikki.krogerus,
	andriy.shevchenko, sakari.ailus, zhifeng.wang, wentong.wu,
	lixu.zhang

[-- Attachment #1: Type: text/plain, Size: 522 bytes --]

On Fri, May 12, 2023 at 01:58:44AM +0800, Ye Xiang wrote:

> +++ b/drivers/spi/spi-ljca.c
> @@ -0,0 +1,290 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Intel La Jolla Cove Adapter USB-SPI driver

Please make the entire comment a C++ one so things look more
intentional.

> +struct spi_init_packet {
> +	u8 index;
> +	u8 speed;
> +	u8 mode;
> +} __packed;
> +
> +struct spi_xfer_packet {

These should be namespaced, especially since they look likely to collide
with other things.  Otherwise this looks fine.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH v8 0/6] Add Intel LJCA device driver
  2023-05-11 17:58 [PATCH v8 0/6] Add Intel LJCA device driver Ye Xiang
                   ` (5 preceding siblings ...)
  2023-05-11 17:58 ` [PATCH v8 6/6] spi: Add support for Intel LJCA USB SPI driver Ye Xiang
@ 2023-05-13  8:50 ` Greg Kroah-Hartman
  2023-06-20  8:10   ` Wu, Wentong
  6 siblings, 1 reply; 22+ messages in thread
From: Greg Kroah-Hartman @ 2023-05-13  8:50 UTC (permalink / raw)
  To: Ye Xiang
  Cc: Arnd Bergmann, Matthias Kaehlcke, Lee Jones, Wolfram Sang,
	Tyrone Ting, Mark Brown, Linus Walleij, Marc Zyngier,
	Bartosz Golaszewski, linux-usb, linux-i2c, linux-kernel,
	linux-spi, linux-gpio, srinivas.pandruvada, heikki.krogerus,
	andriy.shevchenko, sakari.ailus, zhifeng.wang, wentong.wu,
	lixu.zhang

On Fri, May 12, 2023 at 01:58:38AM +0800, Ye Xiang wrote:
> Add driver for Intel La Jolla Cove Adapter (LJCA) device.
> This is a USB-GPIO, USB-I2C and USB-SPI device. We add 4
> drivers to support this device: a USB driver, a GPIO chip
> driver, a I2C controller driver and a SPI controller driver.

I am sorry, but you have not followed the required Intel-specific
requirements for submitting code like this.  Please work with the Linux
Intel developer group to resolve this issue and do it properly for your
next patch submission as I can not take this one for this obvious
reason.

thanks,

greg k-h

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

* Re: [PATCH v8 4/6] gpio: Add support for Intel LJCA USB GPIO driver
  2023-05-11 17:58 ` [PATCH v8 4/6] gpio: Add support for Intel LJCA USB GPIO driver Ye Xiang
  2023-05-11 21:07   ` Linus Walleij
@ 2023-05-15 15:49   ` Bartosz Golaszewski
  2023-07-03  1:11   ` Wu, Wentong
  2 siblings, 0 replies; 22+ messages in thread
From: Bartosz Golaszewski @ 2023-05-15 15:49 UTC (permalink / raw)
  To: Ye Xiang
  Cc: Greg Kroah-Hartman, Arnd Bergmann, Matthias Kaehlcke, Lee Jones,
	Wolfram Sang, Tyrone Ting, Mark Brown, Linus Walleij,
	Marc Zyngier, linux-usb, linux-i2c, linux-kernel, linux-spi,
	linux-gpio, srinivas.pandruvada, heikki.krogerus,
	andriy.shevchenko, sakari.ailus, zhifeng.wang, wentong.wu,
	lixu.zhang

On Thu, May 11, 2023 at 8:01 PM Ye Xiang <xiang.ye@intel.com> wrote:
>
> Implements the GPIO function of Intel USB-I2C/GPIO/SPI adapter
> device named "La Jolla Cove Adapter" (LJCA). It communicate with
> LJCA GPIO module with specific protocol through interfaces exported
> by LJCA USB driver.
>
> Signed-off-by: Ye Xiang <xiang.ye@intel.com>
> ---

With Linus' comment addressed, you can add:

Acked-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

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

* Re: [PATCH v8 1/6] usb: Add support for Intel LJCA device
  2023-05-11 17:58 ` [PATCH v8 1/6] usb: Add support for Intel LJCA device Ye Xiang
@ 2023-05-22  9:21   ` Oliver Neukum
  2023-06-20  7:39     ` Wu, Wentong
  0 siblings, 1 reply; 22+ messages in thread
From: Oliver Neukum @ 2023-05-22  9:21 UTC (permalink / raw)
  To: Ye Xiang, Greg Kroah-Hartman, Arnd Bergmann, Matthias Kaehlcke,
	Lee Jones, Wolfram Sang, Tyrone Ting, Mark Brown, Linus Walleij,
	Marc Zyngier, Bartosz Golaszewski, linux-usb, linux-i2c,
	linux-kernel, linux-spi, linux-gpio
  Cc: srinivas.pandruvada, heikki.krogerus, andriy.shevchenko,
	sakari.ailus, zhifeng.wang, wentong.wu, lixu.zhang

On 11.05.23 19:58, Ye Xiang 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.
> 

Hi,

I am terribly sorry to come up with issues after so many iterations,
but I am really afraid there are issues left.

> 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: Ye Xiang <xiang.ye@intel.com>
> ---

> +
> +/* MNG stub commands */
> +enum ljca_mng_cmd {
> +	LJCA_MNG_GET_VERSION = 1,
> +	LJCA_MNG_RESET_NOTIFY,
> +	LJCA_MNG_RESET,
> +	LJCA_MNG_ENUM_GPIO,
> +	LJCA_MNG_ENUM_I2C,
> +	LJCA_MNG_POWER_STATE_CHANGE,
> +	LJCA_MNG_SET_DFU_MODE,
> +	LJCA_MNG_ENUM_SPI,
> +};
> +
> +/* DIAG commands */
> +enum ljca_diag_cmd {
> +	LJCA_DIAG_GET_STATE = 1,
> +	LJCA_DIAG_GET_STATISTIC,
> +	LJCA_DIAG_SET_TRACE_LEVEL,
> +	LJCA_DIAG_SET_ECHO_MODE,
> +	LJCA_DIAG_GET_FW_LOG,
> +	LJCA_DIAG_GET_FW_COREDUMP,
> +	LJCA_DIAG_TRIGGER_WDT,
> +	LJCA_DIAG_TRIGGER_FAULT,
> +	LJCA_DIAG_FEED_WDT,
> +	LJCA_DIAG_GET_SECURE_STATE,
> +};

Should those really be enum? They just happen
to be nummerically dense.


> +static int ljca_parse(struct ljca_dev *dev, struct ljca_msg *header)
> +{
> +	struct ljca_stub *stub;
> +	unsigned int *ibuf_len;
> +	u8 *ibuf;
> +
> +	stub = ljca_stub_find(dev, header->type);
> +	if (IS_ERR(stub))
> +		return PTR_ERR(stub);
> +
> +	if (!(header->flags & LJCA_ACK_FLAG)) {
> +		ljca_stub_notify(stub, header->cmd, header->data, header->len);
> +		return 0;
> +	}

First you ack ...

> +
> +	if (stub->cur_cmd != header->cmd) {
> +		dev_err(&dev->intf->dev, "header and stub current command mismatch (%x vs %x)\n",
> +			header->cmd, stub->cur_cmd);
> +		return -EINVAL;
> +	}

... then you check whether this is for the correct command?

> +
> +	ibuf_len = READ_ONCE(stub->ipacket.ibuf_len);
> +	ibuf = READ_ONCE(stub->ipacket.ibuf);

This races against stub_write(). Yes, every value now is consistent
within this function. But that does not solve the issue. The pair needs
to be consistent. You need to rule out that you are reading a different
length and buffer location. I am afraid this needs a spinlock.

> +
> +	if (ibuf && ibuf_len) {
> +		unsigned int newlen;
> +
> +		newlen = min_t(unsigned int, header->len, *ibuf_len);
> +
> +		*ibuf_len = newlen;
> +		memcpy(ibuf, header->data, newlen);
> +	}
> +
> +	stub->acked = true;
> +	wake_up(&dev->ack_wq);
> +
> +	return 0;
> +}
> +
> +static int ljca_stub_write(struct ljca_stub *stub, u8 cmd, const void *obuf, unsigned int obuf_len,
> +			   void *ibuf, unsigned int *ibuf_len, bool wait_ack, unsigned long timeout)
> +{
> +	struct ljca_dev *dev = usb_get_intfdata(stub->intf);
> +	u8 flags = LJCA_CMPL_FLAG;
> +	struct ljca_msg *header;
> +	unsigned int msg_len = sizeof(*header) + obuf_len;
> +	int actual;
> +	int ret;
> +
> +	if (msg_len > LJCA_MAX_PACKET_SIZE)
> +		return -EINVAL;
> +
> +	if (wait_ack)
> +		flags |= LJCA_ACK_FLAG;
> +
> +	header = kmalloc(msg_len, GFP_KERNEL);
> +	if (!header)
> +		return -ENOMEM;

Do you really want to first set a flag, then error out?

> +	header->type = stub->type;
> +	header->cmd = cmd;
> +	header->flags = flags;
> +	header->len = obuf_len;
> +
> +	if (obuf)
> +		memcpy(header->data, obuf, obuf_len);
> +
> +	dev_dbg(&dev->intf->dev, "send: type:%d cmd:%d flags:%d len:%d\n", header->type,
> +		header->cmd, header->flags, header->len);
> +
> +	usb_autopm_get_interface(dev->intf);
> +	if (!dev->started) {
> +		kfree(header);
> +		ret = -ENODEV;

Again, the flag remains set.

> +		goto error_put;
> +	}
> +
> +	mutex_lock(&dev->mutex);
> +	stub->cur_cmd = cmd;
> +	stub->ipacket.ibuf = ibuf;
> +	stub->ipacket.ibuf_len = ibuf_len;
> +	stub->acked = false;
> +	ret = usb_bulk_msg(interface_to_usbdev(dev->intf),
> +			   usb_sndbulkpipe(interface_to_usbdev(dev->intf), dev->out_ep), header,
> +			   msg_len, &actual, LJCA_USB_WRITE_TIMEOUT_MS);
> +	kfree(header);
> +	if (ret) {
> +		dev_err(&dev->intf->dev, "bridge write failed ret:%d\n", ret);
> +		goto error_unlock;
> +	}
> +
> +	if (actual != msg_len) {
> +		dev_err(&dev->intf->dev, "bridge write length mismatch (%d vs %d)\n", msg_len,
> +			actual);
> +		ret = -EINVAL;
> +		goto error_unlock;
> +	}
> +
> +	if (wait_ack) {
> +		ret = wait_event_timeout(dev->ack_wq, stub->acked, msecs_to_jiffies(timeout));
> +		if (!ret) {
> +			dev_err(&dev->intf->dev, "acked wait timeout\n");
> +			ret = -ETIMEDOUT;
> +			goto error_unlock;
> +		}
> +	}
> +
> +	ret = 0;
> +error_unlock:
> +	stub->ipacket.ibuf = NULL;
> +	stub->ipacket.ibuf_len = NULL;
> +	mutex_unlock(&dev->mutex);
> +error_put:
> +	usb_autopm_put_interface(dev->intf);
> +
> +	return ret;
> +}
> +

[..]
> +static int ljca_start(struct ljca_dev *dev)
> +{
> +	int ret;
> +
> +	usb_fill_bulk_urb(dev->in_urb, interface_to_usbdev(dev->intf),
> +			  usb_rcvbulkpipe(interface_to_usbdev(dev->intf), dev->in_ep), dev->ibuf,
> +			  dev->ibuf_len, ljca_read_complete, dev);
> +
> +	ret = usb_submit_urb(dev->in_urb, GFP_KERNEL);
> +	if (ret) {
> +		dev_err(&dev->intf->dev, "failed submitting read urb, error %d\n", ret);
> +		return ret;
> +	}
> +
> +	mutex_lock(&dev->mutex);
> +	dev->started = true;
> +	mutex_unlock(&dev->mutex);

Why do you take a mutex here? Either this function cannot race with anything else,
or you set the flag too late.

> +
> +	return 0;
> +}
> +

> +#ifdef CONFIG_ACPI
> +static void ljca_aux_dev_acpi_bind(struct ljca_dev *dev, struct auxiliary_device *auxdev,
> +				   unsigned int adr)
> +{
> +	struct acpi_device *parent;
> +	struct acpi_device *adev = NULL;
> +
> +	/* new auxiliary device bind to acpi device */
> +	parent = ACPI_COMPANION(&dev->intf->dev);
> +	if (!parent)
> +		return;
> +
> +	adev = acpi_find_child_device(parent, adr, false);
> +	ACPI_COMPANION_SET(&auxdev->dev, adev ?: parent);
> +}
> +#else
> +static void ljca_aux_dev_acpi_bind(struct ljca_dev *dev, struct auxiliary_device *auxdev,
> +				   unsigned int adr)
> +{
> +}
> +#endif
> +
> +static int ljca_add_aux_dev(struct ljca_dev *dev, char *name, u8 type, u8 id, u8 adr, void *data,
> +			    unsigned int len)
> +{
> +	struct auxiliary_device *auxdev;
> +	struct ljca *ljca;
> +	int ret;
> +
> +	if (dev->ljca_count >= ARRAY_SIZE(dev->ljcas))
> +		return -EINVAL;
> +
> +	ljca = kzalloc(sizeof(*ljca), GFP_KERNEL);
> +	if (!ljca)
> +		return -ENOMEM;
> +
> +	ljca->type = type;
> +	ljca->id = id;
> +	ljca->dev = dev;
> +
> +	auxdev = &ljca->auxdev;
> +	auxdev->name = name;
> +	auxdev->id = id;
> +	auxdev->dev.platform_data = kmemdup(data, len, GFP_KERNEL);
> +	if (!auxdev->dev.platform_data)
> +		return -ENOMEM;

memory leak of struct ljca *ljca

> +
> +	auxdev->dev.parent = &dev->intf->dev;
> +	auxdev->dev.release = ljca_aux_release;

	Regards
		Oliver


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

* Re: [PATCH v8 2/6] usb: ljca: Add transport interfaces for sub-module drivers
  2023-05-11 17:58 ` [PATCH v8 2/6] usb: ljca: Add transport interfaces for sub-module drivers Ye Xiang
@ 2023-05-22  9:36   ` Oliver Neukum
  0 siblings, 0 replies; 22+ messages in thread
From: Oliver Neukum @ 2023-05-22  9:36 UTC (permalink / raw)
  To: Ye Xiang, Greg Kroah-Hartman, Arnd Bergmann, Matthias Kaehlcke,
	Lee Jones, Wolfram Sang, Tyrone Ting, Mark Brown, Linus Walleij,
	Marc Zyngier, Bartosz Golaszewski, linux-usb, linux-i2c,
	linux-kernel, linux-spi, linux-gpio
  Cc: srinivas.pandruvada, heikki.krogerus, andriy.shevchenko,
	sakari.ailus, zhifeng.wang, wentong.wu, lixu.zhang



On 11.05.23 19:58, Ye Xiang wrote:
> Adds the transport interfaces for various LJCA sub-module drivers
> to communicate with LJCA hardware. 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.
> 
> Signed-off-by: Ye Xiang <xiang.ye@intel.com>
> ---

> +
> +int ljca_register_event_cb(struct ljca *ljca, ljca_event_cb_t event_cb, void *context)
> +{
> +	struct ljca_event_cb_entry *entry, *iter;
> +	struct ljca_stub *stub;
> +	unsigned long flags;
> +	int ret;

Uninitialized variable

> +
> +	stub = ljca_stub_find(ljca->dev, ljca->type);
> +	if (IS_ERR(stub))
> +		return PTR_ERR(stub);
> +
> +	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> +	if (!entry)
> +		return -ENOMEM;
> +
> +	entry->notify = event_cb;
> +	entry->context = context;
> +	entry->id = ljca->id;
> +
> +	spin_lock_irqsave(&stub->event_cb_lock, flags);

You are using GFP_KERNEL a few lines earlier. No need for irqsave

> +
> +	list_for_each_entry(iter, &stub->event_entrys, list) {
> +		if (iter->id == ljca->id) {
> +			ret = -EBUSY;
> +			break;
> +		}
> +	}
> +
> +	if (!ret)
> +		list_add_tail(&entry->list, &stub->event_entrys);
> +
> +	spin_unlock_irqrestore(&stub->event_cb_lock, flags);
> +
> +	if (ret)
> +		kfree(entry);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_NS_GPL(ljca_register_event_cb, LJCA);

	Regards
		Oliver


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

* Re: [PATCH v8 5/6] i2c: Add support for Intel LJCA USB I2C driver
  2023-05-11 17:58 ` [PATCH v8 5/6] i2c: Add support for Intel LJCA USB I2C driver Ye Xiang
@ 2023-05-22  9:50   ` Oliver Neukum
  2023-06-20  6:57     ` Wu, Wentong
  0 siblings, 1 reply; 22+ messages in thread
From: Oliver Neukum @ 2023-05-22  9:50 UTC (permalink / raw)
  To: Ye Xiang, Greg Kroah-Hartman, Arnd Bergmann, Matthias Kaehlcke,
	Lee Jones, Wolfram Sang, Tyrone Ting, Mark Brown, Linus Walleij,
	Marc Zyngier, Bartosz Golaszewski, linux-usb, linux-i2c,
	linux-kernel, linux-spi, linux-gpio
  Cc: srinivas.pandruvada, heikki.krogerus, andriy.shevchenko,
	sakari.ailus, zhifeng.wang, wentong.wu, lixu.zhang



On 11.05.23 19:58, Ye Xiang wrote:
> 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.

> +/* I2C Transfer */
> +struct i2c_xfer {
> +	u8 id;
> +	u8 slave;
> +	u16 flag; /* speed, 8/16bit addr, addr increase, etc */
> +	u16 addr;
> +	u16 len;
> +	u8 data[];
> +} __packed;

Where is this needed?
Why is it defined __packed, yet the internal endianness
is not declared?

	Regards
		Oliver

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

* RE: [PATCH v8 6/6] spi: Add support for Intel LJCA USB SPI driver
  2023-05-12  4:18   ` Mark Brown
@ 2023-06-20  6:47     ` Wu, Wentong
  2023-06-20 11:39       ` Mark Brown
  0 siblings, 1 reply; 22+ messages in thread
From: Wu, Wentong @ 2023-06-20  6:47 UTC (permalink / raw)
  To: Mark Brown, Ye, Xiang
  Cc: Greg Kroah-Hartman, Arnd Bergmann, Matthias Kaehlcke, Lee Jones,
	Wolfram Sang, Tyrone Ting, Linus Walleij, Marc Zyngier,
	Bartosz Golaszewski, linux-usb, linux-i2c, linux-kernel,
	linux-spi, linux-gpio, Pandruvada, Srinivas, heikki.krogerus,
	andriy.shevchenko, sakari.ailus, Wang, Zhifeng, Zhang, Lixu

Hi Brown,

Thanks for your review

> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: Friday, May 12, 2023 12:18 PM
> 
> On Fri, May 12, 2023 at 01:58:44AM +0800, Ye Xiang wrote:
> 
> > +++ b/drivers/spi/spi-ljca.c
> > @@ -0,0 +1,290 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Intel La Jolla Cove Adapter USB-SPI driver
> 
> Please make the entire comment a C++ one so things look more intentional.
> 

I see lots of drivers are commenting like current one. 
But sorry, you mean the entire comment start with /* and end with */ ? Thanks

BR,
Wentong

> > +struct spi_init_packet {
> > +	u8 index;
> > +	u8 speed;
> > +	u8 mode;
> > +} __packed;
> > +
> > +struct spi_xfer_packet {
> 
> These should be namespaced, especially since they look likely to collide with
> other things.  Otherwise this looks fine.

Ack, these two structs will be like below in next version, thanks
struct ljca_spi_init_packet {
	u8 index;
	u8 index;
	u8 mode;
} __packed;

struct ljca_spi_xfer_packet {
	u8 indicator;
	s8 len;
	u8 data[];
} __packed;

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

* RE: [PATCH v8 5/6] i2c: Add support for Intel LJCA USB I2C driver
  2023-05-22  9:50   ` Oliver Neukum
@ 2023-06-20  6:57     ` Wu, Wentong
  0 siblings, 0 replies; 22+ messages in thread
From: Wu, Wentong @ 2023-06-20  6:57 UTC (permalink / raw)
  To: Oliver Neukum, Ye, Xiang
  Cc: Arnd Bergmann, Matthias Kaehlcke, Lee Jones, Wolfram Sang,
	Tyrone Ting, Mark Brown, Linus Walleij, Greg Kroah-Hartman,
	Marc Zyngier, Bartosz Golaszewski, linux-usb, linux-i2c,
	linux-kernel, linux-spi, linux-gpio, Pandruvada, Srinivas,
	heikki.krogerus, andriy.shevchenko, sakari.ailus, Wang, Zhifeng,
	Zhang, Lixu

Hi Oliver,

Thanks for your review.

> -----Original Message-----
> From: Oliver Neukum <oneukum@suse.com>
> Sent: Monday, May 22, 2023 5:50 PM
> 
> On 11.05.23 19:58, Ye Xiang wrote:
> > 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.
> 
> > +/* I2C Transfer */
> > +struct i2c_xfer {
> > +	u8 id;
> > +	u8 slave;
> > +	u16 flag; /* speed, 8/16bit addr, addr increase, etc */
> > +	u16 addr;
> > +	u16 len;
> > +	u8 data[];
> > +} __packed;
> 
> Where is this needed?

Yes, you're right, we don’t need this structure. Thanks 

> Why is it defined __packed, yet the internal endianness is not declared?
> 
> 	Regards
> 		Oliver

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

* RE: [PATCH v8 4/6] gpio: Add support for Intel LJCA USB GPIO driver
  2023-05-11 21:07   ` Linus Walleij
@ 2023-06-20  7:00     ` Wu, Wentong
  0 siblings, 0 replies; 22+ messages in thread
From: Wu, Wentong @ 2023-06-20  7:00 UTC (permalink / raw)
  To: Linus Walleij, Ye, Xiang
  Cc: Greg Kroah-Hartman, Arnd Bergmann, Matthias Kaehlcke, Lee Jones,
	Wolfram Sang, Tyrone Ting, Mark Brown, Marc Zyngier,
	Bartosz Golaszewski, linux-usb, linux-i2c, linux-kernel,
	linux-spi, linux-gpio, Pandruvada, Srinivas, heikki.krogerus,
	andriy.shevchenko, sakari.ailus, Wang, Zhifeng, Zhang, Lixu



> -----Original Message-----
> From: Linus Walleij <linus.walleij@linaro.org>
> Sent: Friday, May 12, 2023 5:08 AM
> 
> > +static int gpio_config(struct ljca_gpio_dev *ljca_gpio, u8 gpio_id,
> > +u8 config)
> 
> All other functions are prefixed ljca_* except this one, so I think you should do
> that on this one too.

Thank you, Linus, this will be fixed in next version, Thanks

> 
> With that fixes:
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> 
> Yours,
> Linus Walleij

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

* RE: [PATCH v8 1/6] usb: Add support for Intel LJCA device
  2023-05-22  9:21   ` Oliver Neukum
@ 2023-06-20  7:39     ` Wu, Wentong
  2023-06-20 12:11       ` Oliver Neukum
  0 siblings, 1 reply; 22+ messages in thread
From: Wu, Wentong @ 2023-06-20  7:39 UTC (permalink / raw)
  To: Oliver Neukum, Ye, Xiang
  Cc: Greg Kroah-Hartman, Arnd Bergmann, Matthias Kaehlcke, Lee Jones,
	Wolfram Sang, Tyrone Ting, Mark Brown, Linus Walleij,
	Marc Zyngier, Bartosz Golaszewski, linux-usb, linux-i2c,
	linux-kernel, linux-spi, linux-gpio, Pandruvada, Srinivas,
	heikki.krogerus, andriy.shevchenko, sakari.ailus, Wang, Zhifeng,
	Zhang, Lixu

Hi Oliver,

Thanks for your review

> -----Original Message-----
> From: Oliver Neukum <oneukum@suse.com>
> Sent: Monday, May 22, 2023 5:22 PM
> 
> On 11.05.23 19:58, Ye Xiang 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.
> >
> 
> Hi,
> 
> I am terribly sorry to come up with issues after so many iterations, but I am
> really afraid there are issues left.
> 
> > 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: Ye Xiang <xiang.ye@intel.com>
> > ---
> 
> > +
> > +/* MNG stub commands */
> > +enum ljca_mng_cmd {
> > +	LJCA_MNG_GET_VERSION = 1,
> > +	LJCA_MNG_RESET_NOTIFY,
> > +	LJCA_MNG_RESET,
> > +	LJCA_MNG_ENUM_GPIO,
> > +	LJCA_MNG_ENUM_I2C,
> > +	LJCA_MNG_POWER_STATE_CHANGE,
> > +	LJCA_MNG_SET_DFU_MODE,
> > +	LJCA_MNG_ENUM_SPI,
> > +};
> > +
> > +/* DIAG commands */
> > +enum ljca_diag_cmd {
> > +	LJCA_DIAG_GET_STATE = 1,
> > +	LJCA_DIAG_GET_STATISTIC,
> > +	LJCA_DIAG_SET_TRACE_LEVEL,
> > +	LJCA_DIAG_SET_ECHO_MODE,
> > +	LJCA_DIAG_GET_FW_LOG,
> > +	LJCA_DIAG_GET_FW_COREDUMP,
> > +	LJCA_DIAG_TRIGGER_WDT,
> > +	LJCA_DIAG_TRIGGER_FAULT,
> > +	LJCA_DIAG_FEED_WDT,
> > +	LJCA_DIAG_GET_SECURE_STATE,
> > +};
> 
> Should those really be enum? They just happen to be nummerically dense.
> 
> 
> > +static int ljca_parse(struct ljca_dev *dev, struct ljca_msg *header)
> > +{
> > +	struct ljca_stub *stub;
> > +	unsigned int *ibuf_len;
> > +	u8 *ibuf;
> > +
> > +	stub = ljca_stub_find(dev, header->type);
> > +	if (IS_ERR(stub))
> > +		return PTR_ERR(stub);
> > +
> > +	if (!(header->flags & LJCA_ACK_FLAG)) {
> > +		ljca_stub_notify(stub, header->cmd, header->data, header-
> >len);
> > +		return 0;
> > +	}
> 
> First you ack ...

This is checking whether the message is cmd ack or event, if not cmd ack, it will
be event.

> 
> > +
> > +	if (stub->cur_cmd != header->cmd) {
> > +		dev_err(&dev->intf->dev, "header and stub current command
> mismatch (%x vs %x)\n",
> > +			header->cmd, stub->cur_cmd);
> > +		return -EINVAL;
> > +	}
> 
> ... then you check whether this is for the correct command?

If it's cmd ack, we check if it's correct command.

> 
> > +
> > +	ibuf_len = READ_ONCE(stub->ipacket.ibuf_len);
> > +	ibuf = READ_ONCE(stub->ipacket.ibuf);
> 
> This races against stub_write(). Yes, every value now is consistent within this
> function. But that does not solve the issue. The pair needs to be consistent. You
> need to rule out that you are reading a different length and buffer location. I am
> afraid this needs a spinlock.

Sorry, based on my understanding, the stub's ibuf_len and ibuf won't be changed by
stub_write until firmware ack the current command.
This memory barrier here is for potential cache consistency issue in some arches though
most likely this driver will only be run on X86 platform.

> 
> > +
> > +	if (ibuf && ibuf_len) {
> > +		unsigned int newlen;
> > +
> > +		newlen = min_t(unsigned int, header->len, *ibuf_len);
> > +
> > +		*ibuf_len = newlen;
> > +		memcpy(ibuf, header->data, newlen);
> > +	}
> > +
> > +	stub->acked = true;
> > +	wake_up(&dev->ack_wq);
> > +
> > +	return 0;
> > +}
> > +
> > +static int ljca_stub_write(struct ljca_stub *stub, u8 cmd, const void *obuf,
> unsigned int obuf_len,
> > +			   void *ibuf, unsigned int *ibuf_len, bool wait_ack,
> unsigned
> > +long timeout) {
> > +	struct ljca_dev *dev = usb_get_intfdata(stub->intf);
> > +	u8 flags = LJCA_CMPL_FLAG;
> > +	struct ljca_msg *header;
> > +	unsigned int msg_len = sizeof(*header) + obuf_len;
> > +	int actual;
> > +	int ret;
> > +
> > +	if (msg_len > LJCA_MAX_PACKET_SIZE)
> > +		return -EINVAL;
> > +
> > +	if (wait_ack)
> > +		flags |= LJCA_ACK_FLAG;

Will be removed in next version

> > +
> > +	header = kmalloc(msg_len, GFP_KERNEL);
> > +	if (!header)
> > +		return -ENOMEM;
> 
> Do you really want to first set a flag, then error out?

Thanks, in next version, it will be like blow:
header->flags = wait_ack ? flags | LJCA_ACK_FLAG : flags;

> 
> > +	header->type = stub->type;
> > +	header->cmd = cmd;
> > +	header->flags = flags;
> > +	header->len = obuf_len;
> > +
> > +	if (obuf)
> > +		memcpy(header->data, obuf, obuf_len);
> > +
> > +	dev_dbg(&dev->intf->dev, "send: type:%d cmd:%d flags:%d len:%d\n",
> header->type,
> > +		header->cmd, header->flags, header->len);
> > +
> > +	usb_autopm_get_interface(dev->intf);
> > +	if (!dev->started) {
> > +		kfree(header);
> > +		ret = -ENODEV;
> 
> Again, the flag remains set.

The flag is local variable, and the header has been freed as well, I don't think we have
to take care the flag here.

> 
> > +		goto error_put;
> > +	}
> > +
> > +	mutex_lock(&dev->mutex);
> > +	stub->cur_cmd = cmd;
> > +	stub->ipacket.ibuf = ibuf;
> > +	stub->ipacket.ibuf_len = ibuf_len;
> > +	stub->acked = false;
> > +	ret = usb_bulk_msg(interface_to_usbdev(dev->intf),
> > +			   usb_sndbulkpipe(interface_to_usbdev(dev->intf), dev-
> >out_ep), header,
> > +			   msg_len, &actual, LJCA_USB_WRITE_TIMEOUT_MS);
> > +	kfree(header);
> > +	if (ret) {
> > +		dev_err(&dev->intf->dev, "bridge write failed ret:%d\n", ret);
> > +		goto error_unlock;
> > +	}
> > +
> > +	if (actual != msg_len) {
> > +		dev_err(&dev->intf->dev, "bridge write length mismatch (%d
> vs %d)\n", msg_len,
> > +			actual);
> > +		ret = -EINVAL;
> > +		goto error_unlock;
> > +	}
> > +
> > +	if (wait_ack) {
> > +		ret = wait_event_timeout(dev->ack_wq, stub->acked,
> msecs_to_jiffies(timeout));
> > +		if (!ret) {
> > +			dev_err(&dev->intf->dev, "acked wait timeout\n");
> > +			ret = -ETIMEDOUT;
> > +			goto error_unlock;
> > +		}
> > +	}
> > +
> > +	ret = 0;
> > +error_unlock:
> > +	stub->ipacket.ibuf = NULL;
> > +	stub->ipacket.ibuf_len = NULL;
> > +	mutex_unlock(&dev->mutex);
> > +error_put:
> > +	usb_autopm_put_interface(dev->intf);
> > +
> > +	return ret;
> > +}
> > +
> 
> [..]
> > +static int ljca_start(struct ljca_dev *dev) {
> > +	int ret;
> > +
> > +	usb_fill_bulk_urb(dev->in_urb, interface_to_usbdev(dev->intf),
> > +			  usb_rcvbulkpipe(interface_to_usbdev(dev->intf), dev-
> >in_ep), dev->ibuf,
> > +			  dev->ibuf_len, ljca_read_complete, dev);
> > +
> > +	ret = usb_submit_urb(dev->in_urb, GFP_KERNEL);
> > +	if (ret) {
> > +		dev_err(&dev->intf->dev, "failed submitting read urb,
> error %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	mutex_lock(&dev->mutex);
> > +	dev->started = true;
> > +	mutex_unlock(&dev->mutex);
> 
> Why do you take a mutex here? Either this function cannot race with anything
> else, or you set the flag too late.

Thanks, I will consider the 'started' again.

> 
> > +
> > +	return 0;
> > +}
> > +
> 
> > +#ifdef CONFIG_ACPI
> > +static void ljca_aux_dev_acpi_bind(struct ljca_dev *dev, struct
> auxiliary_device *auxdev,
> > +				   unsigned int adr)
> > +{
> > +	struct acpi_device *parent;
> > +	struct acpi_device *adev = NULL;
> > +
> > +	/* new auxiliary device bind to acpi device */
> > +	parent = ACPI_COMPANION(&dev->intf->dev);
> > +	if (!parent)
> > +		return;
> > +
> > +	adev = acpi_find_child_device(parent, adr, false);
> > +	ACPI_COMPANION_SET(&auxdev->dev, adev ?: parent); } #else static
> > +void ljca_aux_dev_acpi_bind(struct ljca_dev *dev, struct auxiliary_device
> *auxdev,
> > +				   unsigned int adr)
> > +{
> > +}
> > +#endif
> > +
> > +static int ljca_add_aux_dev(struct ljca_dev *dev, char *name, u8 type, u8 id,
> u8 adr, void *data,
> > +			    unsigned int len)
> > +{
> > +	struct auxiliary_device *auxdev;
> > +	struct ljca *ljca;
> > +	int ret;
> > +
> > +	if (dev->ljca_count >= ARRAY_SIZE(dev->ljcas))
> > +		return -EINVAL;
> > +
> > +	ljca = kzalloc(sizeof(*ljca), GFP_KERNEL);
> > +	if (!ljca)
> > +		return -ENOMEM;
> > +
> > +	ljca->type = type;
> > +	ljca->id = id;
> > +	ljca->dev = dev;
> > +
> > +	auxdev = &ljca->auxdev;
> > +	auxdev->name = name;
> > +	auxdev->id = id;
> > +	auxdev->dev.platform_data = kmemdup(data, len, GFP_KERNEL);
> > +	if (!auxdev->dev.platform_data)
> > +		return -ENOMEM;
> 
> memory leak of struct ljca *ljca

ack, this will be fixed in next version, thanks

> 
> > +
> > +	auxdev->dev.parent = &dev->intf->dev;
> > +	auxdev->dev.release = ljca_aux_release;
> 
> 	Regards
> 		Oliver


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

* RE: [PATCH v8 0/6] Add Intel LJCA device driver
  2023-05-13  8:50 ` [PATCH v8 0/6] Add Intel LJCA device driver Greg Kroah-Hartman
@ 2023-06-20  8:10   ` Wu, Wentong
  0 siblings, 0 replies; 22+ messages in thread
From: Wu, Wentong @ 2023-06-20  8:10 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Ye, Xiang, Mark Brown, Bartosz Golaszewski,
	Linus Walleij, Oliver Neukum
  Cc: Arnd Bergmann, Matthias Kaehlcke, Lee Jones, Wolfram Sang,
	Tyrone Ting, Marc Zyngier, linux-usb, linux-i2c, linux-kernel,
	linux-spi, linux-gpio, Pandruvada, Srinivas, heikki.krogerus,
	andriy.shevchenko, sakari.ailus, Wang, Zhifeng, Zhang, Lixu

Hi Greg, 

Thanks for your review.

BTW, somehow I will take care this patch set.

Thanks
Wentong

> -----Original Message-----
> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Sent: Saturday, May 13, 2023 4:50 PM
> 
> On Fri, May 12, 2023 at 01:58:38AM +0800, Ye Xiang wrote:
> > Add driver for Intel La Jolla Cove Adapter (LJCA) device.
> > This is a USB-GPIO, USB-I2C and USB-SPI device. We add 4 drivers to
> > support this device: a USB driver, a GPIO chip driver, a I2C
> > controller driver and a SPI controller driver.
> 
> I am sorry, but you have not followed the required Intel-specific requirements
> for submitting code like this.  Please work with the Linux Intel developer group
> to resolve this issue and do it properly for your next patch submission as I can
> not take this one for this obvious reason.
> 
> thanks,
> 
> greg k-h

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

* Re: [PATCH v8 6/6] spi: Add support for Intel LJCA USB SPI driver
  2023-06-20  6:47     ` Wu, Wentong
@ 2023-06-20 11:39       ` Mark Brown
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2023-06-20 11:39 UTC (permalink / raw)
  To: Wu, Wentong
  Cc: Ye, Xiang, Greg Kroah-Hartman, Arnd Bergmann, Matthias Kaehlcke,
	Lee Jones, Wolfram Sang, Tyrone Ting, Linus Walleij,
	Marc Zyngier, Bartosz Golaszewski, linux-usb, linux-i2c,
	linux-kernel, linux-spi, linux-gpio, Pandruvada, Srinivas,
	heikki.krogerus, andriy.shevchenko, sakari.ailus, Wang, Zhifeng,
	Zhang, Lixu

[-- Attachment #1: Type: text/plain, Size: 477 bytes --]

On Tue, Jun 20, 2023 at 06:47:11AM +0000, Wu, Wentong wrote:

> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * Intel La Jolla Cove Adapter USB-SPI driver

> > Please make the entire comment a C++ one so things look more intentional.

> I see lots of drivers are commenting like current one. 
> But sorry, you mean the entire comment start with /* and end with */ ? Thanks

Other way around, use // for all the lines in this comment block please.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH v8 1/6] usb: Add support for Intel LJCA device
  2023-06-20  7:39     ` Wu, Wentong
@ 2023-06-20 12:11       ` Oliver Neukum
  0 siblings, 0 replies; 22+ messages in thread
From: Oliver Neukum @ 2023-06-20 12:11 UTC (permalink / raw)
  To: Wu, Wentong, Oliver Neukum, Ye, Xiang
  Cc: Greg Kroah-Hartman, Arnd Bergmann, Matthias Kaehlcke, Lee Jones,
	Wolfram Sang, Tyrone Ting, Mark Brown, Linus Walleij,
	Marc Zyngier, Bartosz Golaszewski, linux-usb, linux-i2c,
	linux-kernel, linux-spi, linux-gpio, Pandruvada, Srinivas,
	heikki.krogerus, andriy.shevchenko, sakari.ailus, Wang, Zhifeng,
	Zhang, Lixu



On 20.06.23 09:39, Wu, Wentong wrote:

Hi,

>>> +
>>> +	ibuf_len = READ_ONCE(stub->ipacket.ibuf_len);
>>> +	ibuf = READ_ONCE(stub->ipacket.ibuf);
>>
>> This races against stub_write(). Yes, every value now is consistent within this
>> function. But that does not solve the issue. The pair needs to be consistent. You
>> need to rule out that you are reading a different length and buffer location. I am
>> afraid this needs a spinlock.
> 
> Sorry, based on my understanding, the stub's ibuf_len and ibuf won't be changed by
> stub_write until firmware ack the current command.

Or a timeout.
There are two places in the driver ibuf_len and ibuf are touched.
Both are in ljca_stub_write(), but in case of a timeout they are set
to NULL. And you are checking for that case. That is good, but not sufficient,
as READ_ONCE does not guarantee ordering, nor do you use WRITE_ONCE to set them.

> This memory barrier here is for potential cache consistency issue in some arches though
> most likely this driver will only be run on X86 platform.

Well, you can move them to architecture dependent directories. But if not, they
will have to work generically.

A correct version would be something like:

+static int ljca_parse(struct ljca_dev *dev, struct ljca_msg *header)
+{
+	struct ljca_stub *stub;
+	unsigned int *ibuf_len;
+	u8 *ibuf;
+
+	stub = ljca_stub_find(dev, header->type);
+	if (IS_ERR(stub))
+		return PTR_ERR(stub);
+
+	if (!(header->flags & LJCA_ACK_FLAG)) {
+		ljca_stub_notify(stub, header->cmd, header->data, header->len);
+		return 0;
+	}
+
+	if (stub->cur_cmd != header->cmd) {
+		dev_err(&dev->intf->dev, "header and stub current command mismatch (%x vs %x)\n",
+			header->cmd, stub->cur_cmd);
+		return -EINVAL;
+	}
+

smp_rmb();

+	ibuf_len = READ_ONCE(stub->ipacket.ibuf_len);
+	ibuf = READ_ONCE(stub->ipacket.ibuf);
+
+	if (ibuf && ibuf_len) {
+		unsigned int newlen;
+
+		newlen = min_t(unsigned int, header->len, *ibuf_len);
+
+		*ibuf_len = newlen;
+		memcpy(ibuf, header->data, newlen);
+	}
+
+	stub->acked = true;
+	wake_up(&dev->ack_wq);
+
+	return 0;
+}
+
+static int ljca_stub_write(struct ljca_stub *stub, u8 cmd, const void *obuf, unsigned int obuf_len,
+			   void *ibuf, unsigned int *ibuf_len, bool wait_ack, unsigned long timeout)
+{
+	struct ljca_dev *dev = usb_get_intfdata(stub->intf);
+	u8 flags = LJCA_CMPL_FLAG;
+	struct ljca_msg *header;
+	unsigned int msg_len = sizeof(*header) + obuf_len;
+	int actual;
+	int ret;
+
+	if (msg_len > LJCA_MAX_PACKET_SIZE)
+		return -EINVAL;
+
+	if (wait_ack)
+		flags |= LJCA_ACK_FLAG;
+
+	header = kmalloc(msg_len, GFP_KERNEL);
+	if (!header)
+		return -ENOMEM;
+
+	header->type = stub->type;
+	header->cmd = cmd;
+	header->flags = flags;
+	header->len = obuf_len;
+
+	if (obuf)
+		memcpy(header->data, obuf, obuf_len);
+
+	dev_dbg(&dev->intf->dev, "send: type:%d cmd:%d flags:%d len:%d\n", header->type,
+		header->cmd, header->flags, header->len);
+
+	usb_autopm_get_interface(dev->intf);
+	if (!dev->started) {
+		kfree(header);
+		ret = -ENODEV;
+		goto error_put;
+	}
+
+	mutex_lock(&dev->mutex);
+	stub->cur_cmd = cmd;

WRITE_ONCE(stub->ipacket.ibuf, ibuf);
WRITE_ONCE(stub->ipacket.ibuf_len, ibuf_len);

+	stub->acked = false;

smp_wmb();

[..]

+error_unlock:

WRITE_ONCE(stub->ipacket.ibuf, NULL);
WRITE_ONCE(stub->ipacket.ibuf_len, NULL);
smp_wmb();

+	mutex_unlock(&dev->mutex);

	Regards
		Oliver


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

* RE: [PATCH v8 4/6] gpio: Add support for Intel LJCA USB GPIO driver
  2023-05-11 17:58 ` [PATCH v8 4/6] gpio: Add support for Intel LJCA USB GPIO driver Ye Xiang
  2023-05-11 21:07   ` Linus Walleij
  2023-05-15 15:49   ` Bartosz Golaszewski
@ 2023-07-03  1:11   ` Wu, Wentong
  2 siblings, 0 replies; 22+ messages in thread
From: Wu, Wentong @ 2023-07-03  1:11 UTC (permalink / raw)
  To: Ye, Xiang
  Cc: Pandruvada, Srinivas, heikki.krogerus, andriy.shevchenko,
	sakari.ailus, Wang, Zhifeng, Zhang, Lixu, Greg Kroah-Hartman,
	Arnd Bergmann, Matthias Kaehlcke, Lee Jones, Wolfram Sang,
	Tyrone Ting, Mark Brown, Linus Walleij, Marc Zyngier,
	Bartosz Golaszewski, linux-usb, linux-i2c, linux-kernel,
	linux-spi, linux-gpio



> -----Original Message-----
> From: Ye, Xiang <xiang.ye@intel.com>
> Sent: Friday, May 12, 2023 1:59 AM
> 
> Implements the GPIO function of Intel USB-I2C/GPIO/SPI adapter device named
> "La Jolla Cove Adapter" (LJCA). It communicate with LJCA GPIO module with
> specific protocol through interfaces exported by LJCA USB driver.
> 
> Signed-off-by: Ye Xiang <xiang.ye@intel.com>
> ---
>  drivers/gpio/Kconfig     |  12 +
>  drivers/gpio/Makefile    |   1 +
>  drivers/gpio/gpio-ljca.c | 479 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 492 insertions(+)
>  create mode 100644 drivers/gpio/gpio-ljca.c
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index
> 13be729710f2..bbf00e157dcd 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1658,6 +1658,18 @@ config GPIO_VIPERBOARD
>  	  River Tech's viperboard.h for detailed meaning
>  	  of the module parameters.
> 
> +config GPIO_LJCA
> +	tristate "INTEL La Jolla Cove Adapter GPIO support"
> +	depends on USB_LJCA
> +	select GPIOLIB_IRQCHIP
> +	default USB_LJCA
> +	help
> +	  Select this option to enable GPIO 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 gpio-ljca.
> +
>  endmenu
> 
>  menu "Virtual GPIO drivers"
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index
> c048ba003367..eb59524d18c0 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -77,6 +77,7 @@ obj-$(CONFIG_GPIO_IXP4XX)		+= gpio-
> ixp4xx.o
>  obj-$(CONFIG_GPIO_JANZ_TTL)		+= gpio-janz-ttl.o
>  obj-$(CONFIG_GPIO_KEMPLD)		+= gpio-kempld.o
>  obj-$(CONFIG_GPIO_LATCH)		+= gpio-latch.o
> +obj-$(CONFIG_GPIO_LJCA) 		+= gpio-ljca.o
>  obj-$(CONFIG_GPIO_LOGICVC)		+= gpio-logicvc.o
>  obj-$(CONFIG_GPIO_LOONGSON1)		+= gpio-loongson1.o
>  obj-$(CONFIG_GPIO_LOONGSON)		+= gpio-loongson.o
> diff --git a/drivers/gpio/gpio-ljca.c b/drivers/gpio/gpio-ljca.c new file mode
> 100644 index 000000000000..81835a21e8c0
> --- /dev/null
> +++ b/drivers/gpio/gpio-ljca.c
> @@ -0,0 +1,479 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Intel La Jolla Cove Adapter USB-GPIO driver
> + *
> + * Copyright (c) 2023, Intel Corporation.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/auxiliary_bus.h>
> +#include <linux/bitfield.h>
> +#include <linux/bitops.h>
> +#include <linux/dev_printk.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/irq.h>
> +#include <linux/kernel.h>
> +#include <linux/kref.h>
> +#include <linux/module.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_CONF_DISABLE		BIT(0)
> +#define LJCA_GPIO_CONF_INPUT		BIT(1)
> +#define LJCA_GPIO_CONF_OUTPUT		BIT(2)
> +#define LJCA_GPIO_CONF_PULLUP		BIT(3)
> +#define LJCA_GPIO_CONF_PULLDOWN		BIT(4)
> +#define LJCA_GPIO_CONF_DEFAULT		BIT(5)
> +#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)
> +
> +/* Intentional overlap with PULLUP / PULLDOWN */
> +#define LJCA_GPIO_CONF_SET		BIT(3)
> +#define LJCA_GPIO_CONF_CLR		BIT(4)
> +
> +struct gpio_op {
> +	u8 index;
> +	u8 value;
> +} __packed;
> +
> +struct gpio_packet {
> +	u8 num;
> +	struct gpio_op item[];
> +} __packed;
> +
> +#define LJCA_GPIO_BUF_SIZE 60
> +struct ljca_gpio_dev {
> +	struct ljca *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 */
> +	struct mutex irq_lock;
> +	struct work_struct work;
> +	/* lock to 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) {
> +	struct gpio_packet *packet = (struct gpio_packet *)ljca_gpio->obuf;
> +	int ret;
> +
> +	mutex_lock(&ljca_gpio->trans_lock);
> +	packet->item[0].index = gpio_id;
> +	packet->item[0].value = config | ljca_gpio->connect_mode[gpio_id];
> +	packet->num = 1;
> +
> +	ret = ljca_transfer(ljca_gpio->ljca, LJCA_GPIO_CONFIG, packet,
> +			    struct_size(packet, item, packet->num), NULL, NULL);
> +	mutex_unlock(&ljca_gpio->trans_lock);
> +
> +	return ret;
> +}
> +
> +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;
> +	int ret;
> +
> +	mutex_lock(&ljca_gpio->trans_lock);
> +	packet->num = 1;
> +	packet->item[0].index = gpio_id;
> +	ret = ljca_transfer(ljca_gpio->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->ljca->auxdev.dev,
> +			"read package error, gpio_id:%u num:%u
> ibuf_len:%d\n", gpio_id,
> +			ack_packet->num, ibuf_len);
> +		ret = -EIO;
> +	}
> +
> +out_unlock:
> +	mutex_unlock(&ljca_gpio->trans_lock);
> +	if (ret)
> +		return ret;
> +
> +	return ack_packet->item[0].value > 0;
> +}
> +
> +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;
> +	int ret;
> +
> +	mutex_lock(&ljca_gpio->trans_lock);
> +	packet->num = 1;
> +	packet->item[0].index = gpio_id;
> +	packet->item[0].value = value & 1;
> +
> +	ret = ljca_transfer(ljca_gpio->ljca, LJCA_GPIO_WRITE, packet,
> +			    struct_size(packet, item, packet->num), NULL, NULL);
> +	mutex_unlock(&ljca_gpio->trans_lock);
> +
> +	return ret;
> +}
> +
> +static int ljca_gpio_get_value(struct gpio_chip *chip, unsigned int
> +offset) {
> +	struct ljca_gpio_dev *ljca_gpio = gpiochip_get_data(chip);
> +
> +	return ljca_gpio_read(ljca_gpio, offset); }
> +
> +static void ljca_gpio_set_value(struct gpio_chip *chip, unsigned int
> +offset, int val) {
> +	struct ljca_gpio_dev *ljca_gpio = gpiochip_get_data(chip);
> +	int ret;
> +
> +	ret = ljca_gpio_write(ljca_gpio, offset, val);
> +	if (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) {
> +	struct ljca_gpio_dev *ljca_gpio = gpiochip_get_data(chip);
> +	u8 config = LJCA_GPIO_CONF_INPUT | LJCA_GPIO_CONF_CLR;
> +	int ret;
> +
> +	ret = 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, unsigned
> +int offset, int val) {
> +	struct ljca_gpio_dev *ljca_gpio = gpiochip_get_data(chip);
> +	u8 config = LJCA_GPIO_CONF_OUTPUT | LJCA_GPIO_CONF_CLR;
> +	int ret;
> +
> +	ret = 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)
> +{
> +	struct ljca_gpio_dev *ljca_gpio = gpiochip_get_data(chip);
> +
> +	ljca_gpio->connect_mode[offset] = 0;
> +	switch (pinconf_to_config_param(config)) {
> +	case PIN_CONFIG_BIAS_PULL_UP:
> +		ljca_gpio->connect_mode[offset] |=
> LJCA_GPIO_CONF_PULLUP;
> +		break;
> +	case PIN_CONFIG_BIAS_PULL_DOWN:
> +		ljca_gpio->connect_mode[offset] |=
> LJCA_GPIO_CONF_PULLDOWN;
> +		break;
> +	case PIN_CONFIG_DRIVE_PUSH_PULL:
> +	case PIN_CONFIG_PERSIST_STATE:
> +		break;
> +	default:
> +		return -ENOTSUPP;
> +	}
> +
> +	return 0;
> +}
> +
> +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);
> +
> +	WARN_ON_ONCE(ngpios != ljca_gpio->gpio_info->num);
> +	bitmap_copy(valid_mask, ljca_gpio->gpio_info->valid_pin_map, ngpios);
> +
> +	return 0;
> +}
> +
> +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) {
> +	struct gpio_packet *packet = (struct gpio_packet *)ljca_gpio->obuf;
> +	int ret;
> +
> +	mutex_lock(&ljca_gpio->trans_lock);
> +	packet->num = 1;
> +	packet->item[0].index = gpio_id;
> +	packet->item[0].value = 0;
> +
> +	ret = ljca_transfer(ljca_gpio->ljca, enable ? LJCA_GPIO_INT_UNMASK :
> LJCA_GPIO_INT_MASK,
> +			    packet, struct_size(packet, item, packet->num), NULL,
> NULL);
> +	mutex_unlock(&ljca_gpio->trans_lock);
> +
> +	return ret;
> +}
> +
> +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;
> +
> +	for_each_set_bit(gpio_id, ljca_gpio->reenable_irqs, ljca_gpio->gc.ngpio)
> {
> +		clear_bit(gpio_id, ljca_gpio->reenable_irqs);
> +		unmasked = test_bit(gpio_id, ljca_gpio->unmasked_irqs);
> +		if (unmasked)
> +			ljca_enable_irq(ljca_gpio, gpio_id, true);
> +	}
> +}
> +
> +static void ljca_gpio_event_cb(void *context, u8 cmd, const void
> +*evt_data, int len) {
> +	const struct gpio_packet *packet = evt_data;
> +	struct ljca_gpio_dev *ljca_gpio = context;
> +	int i;
> +	int 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);
> +		if (!irq) {
> +			dev_err(ljca_gpio->gc.parent, "gpio_id %u does not
> mapped to IRQ yet\n",
> +				packet->item[i].index);
> +			return;
> +		}
> +
> +		generic_handle_domain_irq(ljca_gpio->gc.irq.domain, irq);
> +		set_bit(packet->item[i].index, ljca_gpio->reenable_irqs);
> +	}
> +
> +	schedule_work(&ljca_gpio->work);
> +}
> +
> +static void ljca_irq_unmask(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);
> +
> +	gpiochip_enable_irq(gc, gpio_id);
> +	set_bit(gpio_id, ljca_gpio->unmasked_irqs); }
> +
> +static void ljca_irq_mask(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);
> +
> +	clear_bit(gpio_id, ljca_gpio->unmasked_irqs);
> +	gpiochip_disable_irq(gc, gpio_id);
> +}
> +
> +static int ljca_irq_set_type(struct irq_data *irqd, unsigned int type)
> +{
> +	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);
> +
> +	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);
> +		break;
> +	case IRQ_TYPE_LEVEL_LOW:
> +		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);
> +		break;
> +	case IRQ_TYPE_EDGE_FALLING:
> +		ljca_gpio->connect_mode[gpio_id] |= (LJCA_GPIO_CONF_EDGE
> | LJCA_GPIO_CONF_PULLDOWN);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static void ljca_irq_bus_lock(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);
> +
> +	mutex_lock(&ljca_gpio->irq_lock);
> +}
> +
> +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;
> +
> +	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_enable_irq(ljca_gpio, gpio_id, true);
> +			set_bit(gpio_id, ljca_gpio->enabled_irqs);
> +		} else {
> +			ljca_enable_irq(ljca_gpio, gpio_id, false);
> +			clear_bit(gpio_id, ljca_gpio->enabled_irqs);
> +		}
> +	}
> +
> +	mutex_unlock(&ljca_gpio->irq_lock);
> +}
> +
> +static const struct irq_chip ljca_gpio_irqchip = {
> +	.name = "ljca-irq",
> +	.irq_mask = ljca_irq_mask,
> +	.irq_unmask = ljca_irq_unmask,
> +	.irq_set_type = ljca_irq_set_type,
> +	.irq_bus_lock = ljca_irq_bus_lock,
> +	.irq_bus_sync_unlock = ljca_irq_bus_unlock,
> +	.flags = IRQCHIP_IMMUTABLE,
> +	GPIOCHIP_IRQ_RESOURCE_HELPERS,
> +};
> +
> +static int ljca_gpio_probe(struct auxiliary_device *auxdev,
> +			   const struct auxiliary_device_id *aux_dev_id) {
> +	struct ljca *ljca = auxiliary_dev_to_ljca(auxdev);
> +	struct ljca_gpio_dev *ljca_gpio;
> +	struct gpio_irq_chip *girq;
> +	int ret;
> +
> +	ljca_gpio = devm_kzalloc(&auxdev->dev, sizeof(*ljca_gpio),
> GFP_KERNEL);
> +	if (!ljca_gpio)
> +		return -ENOMEM;
> +
> +	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->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 = &auxdev->dev;
> +
> +	ljca_gpio->gc.base = -1;
> +	ljca_gpio->gc.ngpio = ljca_gpio->gpio_info->num;
> +	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;
> +
> +	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);
> +	girq->parent_handler = NULL;
> +	girq->num_parents = 0;
> +	girq->parents = NULL;
> +	girq->default_type = IRQ_TYPE_NONE;
> +	girq->handler = handle_simple_irq;
> +	girq->init_valid_mask = ljca_gpio_irq_init_valid_mask;
> +
> +	INIT_WORK(&ljca_gpio->work, ljca_gpio_async);
> +	ret = gpiochip_add_data(&ljca_gpio->gc, ljca_gpio);
> +	if (ret) {
> +		ljca_unregister_event_cb(ljca);
> +		mutex_destroy(&ljca_gpio->irq_lock);
> +		mutex_destroy(&ljca_gpio->trans_lock);
> +	}
> +
> +	return ret;
> +}
> +
> +static void ljca_gpio_remove(struct auxiliary_device *auxdev) {
> +	struct ljca_gpio_dev *ljca_gpio = auxiliary_get_drvdata(auxdev);
> +
> +	gpiochip_remove(&ljca_gpio->gc);
> +	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);
> +}
> +
> +#define LJCA_GPIO_DRV_NAME "ljca.ljca-gpio"

The wrong name causes the probe match failed. The name should be
"usb_ljca.ljca-gpio", the same as spi and i2c.

BR,
Wentong
 
> +static const struct auxiliary_device_id ljca_gpio_id_table[] = {
> +	{ LJCA_GPIO_DRV_NAME, 0 },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(auxiliary, ljca_gpio_id_table);
> +
> +static struct auxiliary_driver ljca_gpio_driver = {
> +	.probe = ljca_gpio_probe,
> +	.remove = ljca_gpio_remove,
> +	.id_table = ljca_gpio_id_table,
> +};
> +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_DESCRIPTION("Intel La Jolla Cove
> +Adapter USB-GPIO driver"); MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS(LJCA);
> --
> 2.34.1


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

end of thread, other threads:[~2023-07-03  1:12 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-11 17:58 [PATCH v8 0/6] Add Intel LJCA device driver Ye Xiang
2023-05-11 17:58 ` [PATCH v8 1/6] usb: Add support for Intel LJCA device Ye Xiang
2023-05-22  9:21   ` Oliver Neukum
2023-06-20  7:39     ` Wu, Wentong
2023-06-20 12:11       ` Oliver Neukum
2023-05-11 17:58 ` [PATCH v8 2/6] usb: ljca: Add transport interfaces for sub-module drivers Ye Xiang
2023-05-22  9:36   ` Oliver Neukum
2023-05-11 17:58 ` [PATCH v8 3/6] Documentation: Add ABI doc for attributes of LJCA device Ye Xiang
2023-05-11 17:58 ` [PATCH v8 4/6] gpio: Add support for Intel LJCA USB GPIO driver Ye Xiang
2023-05-11 21:07   ` Linus Walleij
2023-06-20  7:00     ` Wu, Wentong
2023-05-15 15:49   ` Bartosz Golaszewski
2023-07-03  1:11   ` Wu, Wentong
2023-05-11 17:58 ` [PATCH v8 5/6] i2c: Add support for Intel LJCA USB I2C driver Ye Xiang
2023-05-22  9:50   ` Oliver Neukum
2023-06-20  6:57     ` Wu, Wentong
2023-05-11 17:58 ` [PATCH v8 6/6] spi: Add support for Intel LJCA USB SPI driver Ye Xiang
2023-05-12  4:18   ` Mark Brown
2023-06-20  6:47     ` Wu, Wentong
2023-06-20 11:39       ` Mark Brown
2023-05-13  8:50 ` [PATCH v8 0/6] Add Intel LJCA device driver Greg Kroah-Hartman
2023-06-20  8:10   ` 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).