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.
---
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 (5):
mfd: Add support for Intel 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
Documentation: Add ABI doc for attributes of LJCA device
.../ABI/testing/sysfs-bus-usb-devices-ljca | 22 +
drivers/gpio/Kconfig | 12 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-ljca.c | 454 ++++++++
drivers/i2c/busses/Kconfig | 11 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-ljca.c | 357 +++++++
drivers/spi/Kconfig | 11 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-ljca.c | 291 ++++++
drivers/usb/misc/Kconfig | 13 +
drivers/usb/misc/Makefile | 1 +
drivers/usb/misc/ljca.c | 969 ++++++++++++++++++
include/linux/mfd/ljca.h | 95 ++
14 files changed, 2239 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/ljca.c
create mode 100644 include/linux/mfd/ljca.h
--
2.34.1
This patch 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. The sub-module of
LJCA can use ljca_transfer() to issue a transfer between host
and hardware.
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 <[email protected]>
Reviewed-by: Sakari Ailus <[email protected]>
---
drivers/usb/misc/Kconfig | 13 +
drivers/usb/misc/Makefile | 1 +
drivers/usb/misc/ljca.c | 969 ++++++++++++++++++++++++++++++++++++++
include/linux/mfd/ljca.h | 95 ++++
4 files changed, 1078 insertions(+)
create mode 100644 drivers/usb/misc/ljca.c
create mode 100644 include/linux/mfd/ljca.h
diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
index a5f7652db7da..59ec120c26d4 100644
--- a/drivers/usb/misc/Kconfig
+++ b/drivers/usb/misc/Kconfig
@@ -273,6 +273,19 @@ 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 MFD_CORE
+ depends on USB
+ 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 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..6f6adfbe17e0 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) += ljca.o
obj-$(CONFIG_USB_SISUSBVGA) += sisusbvga/
obj-$(CONFIG_USB_LINK_LAYER_TEST) += lvstest.o
diff --git a/drivers/usb/misc/ljca.c b/drivers/usb/misc/ljca.c
new file mode 100644
index 000000000000..e9520f3d2904
--- /dev/null
+++ b/drivers/usb/misc/ljca.c
@@ -0,0 +1,969 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Intel La Jolla Cove Adapter USB driver
+ *
+ * Copyright (c) 2023, Intel Corporation.
+ */
+
+#include <linux/dev_printk.h>
+#include <linux/kernel.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/ljca.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/usb.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,
+};
+
+static struct mfd_cell_acpi_match ljca_acpi_match_gpio = {
+ .adr = LJCA_ACPI_MATCH_GPIO,
+};
+
+static struct mfd_cell_acpi_match ljca_acpi_match_i2cs[] = {
+ {
+ .adr = LJCA_ACPI_MATCH_I2C1,
+ },
+ {
+ .adr = LJCA_ACPI_MATCH_I2C2,
+ },
+};
+
+static struct mfd_cell_acpi_match ljca_acpi_match_spis[] = {
+ {
+ .adr = LJCA_ACPI_MATCH_SPI1,
+ },
+ {
+ .adr = LJCA_ACPI_MATCH_SPI2,
+ },
+};
+
+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 {
+ ljca_event_cb_t notify;
+ void *context;
+};
+
+struct ljca_dev {
+ struct usb_device *udev;
+ 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;
+
+ bool started;
+ struct list_head stubs_list;
+
+ /* to wait for an ongoing write ack */
+ wait_queue_head_t ack_wq;
+
+ struct mfd_cell *cells;
+ int cell_count;
+ /* mutex to protect package transfer with LJCA device */
+ struct mutex mutex;
+};
+
+struct ljca {
+ u8 type;
+ struct ljca_dev *dev;
+};
+
+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 ljca_event_cb_entry event_entry;
+ /* lock to protect event_entry */
+ spinlock_t event_cb_lock;
+
+ struct ljca ljca;
+ 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 = dev->intf;
+ stub->ljca.dev = dev;
+ stub->ljca.type = stub->type;
+ spin_lock_init(&stub->event_cb_lock);
+ list_add_tail(&stub->list, &dev->stubs_list);
+ return 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)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&stub->event_cb_lock, flags);
+ if (stub->event_entry.notify && stub->event_entry.context)
+ stub->event_entry.notify(stub->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;
+
+ 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;
+ }
+
+ if (stub->ipacket.ibuf && stub->ipacket.ibuf_len) {
+ unsigned int newlen;
+
+ newlen = min_t(unsigned int, header->len, *stub->ipacket.ibuf_len);
+
+ *stub->ipacket.ibuf_len = newlen;
+ memcpy(stub->ipacket.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) {
+ 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(dev->udev, usb_sndbulkpipe(dev->udev, 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;
+ }
+ }
+
+ stub->ipacket.ibuf = NULL;
+ stub->ipacket.ibuf_len = NULL;
+ ret = 0;
+error_unlock:
+ mutex_unlock(&dev->mutex);
+error_put:
+ usb_autopm_put_interface(dev->intf);
+ 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_stub *stub;
+ unsigned long flags;
+
+ stub = ljca_stub_find(ljca->dev, ljca->type);
+ if (IS_ERR(stub))
+ return PTR_ERR(stub);
+
+ spin_lock_irqsave(&stub->event_cb_lock, flags);
+ stub->event_entry.notify = event_cb;
+ stub->event_entry.context = context;
+ spin_unlock_irqrestore(&stub->event_cb_lock, flags);
+
+ return 0;
+}
+EXPORT_SYMBOL_NS_GPL(ljca_register_event_cb, LJCA);
+
+void ljca_unregister_event_cb(struct ljca *ljca)
+{
+ struct ljca_stub *stub;
+ unsigned long flags;
+
+ stub = ljca_stub_find(ljca->dev, ljca->type);
+ if (IS_ERR(stub))
+ return;
+
+ spin_lock_irqsave(&stub->event_cb_lock, flags);
+ stub->event_entry.notify = NULL;
+ stub->event_entry.context = NULL;
+ spin_unlock_irqrestore(&stub->event_cb_lock, flags);
+}
+EXPORT_SYMBOL_NS_GPL(ljca_unregister_event_cb, LJCA);
+
+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) {
+ list_del_init(&stub->list);
+ kfree(stub);
+ }
+}
+
+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;
+
+ WARN_ON_ONCE(!dev);
+ WARN_ON_ONCE(!header);
+
+ 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, dev->udev, usb_rcvbulkpipe(dev->udev, 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);
+
+ dev->started = true;
+ return ret;
+}
+
+struct ljca_mng_priv {
+ u32 reset_id;
+};
+
+static int ljca_mng_reset_handshake(struct ljca_stub *stub)
+{
+ struct ljca_mng_priv *priv;
+ __le32 reset_id;
+ __le32 reset_id_ret = 0;
+ unsigned int ilen = sizeof(__le32);
+ int ret;
+
+ priv = ljca_priv(stub);
+ reset_id = cpu_to_le32(priv->reset_id++);
+ 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 int ljca_add_mfd_cell(struct ljca_dev *dev, struct mfd_cell *cell)
+{
+ struct mfd_cell *new_cells;
+
+ new_cells = krealloc_array(dev->cells, dev->cell_count + 1, sizeof(*new_cells), GFP_KERNEL);
+ if (!new_cells)
+ return -ENOMEM;
+
+ memcpy(&new_cells[dev->cell_count], cell, sizeof(*cell));
+ dev->cells = new_cells;
+ dev->cell_count++;
+
+ return 0;
+}
+
+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 mfd_cell cell = {};
+ struct ljca_stub *stub;
+ int gpio_num;
+ int i;
+
+ 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, sizeof(*gpio_info));
+ if (IS_ERR(stub))
+ return PTR_ERR(stub);
+
+ gpio_info = ljca_priv(stub);
+ gpio_info->ljca = &stub->ljca;
+ 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);
+
+ cell.name = "ljca-gpio";
+ cell.platform_data = gpio_info;
+ cell.pdata_size = sizeof(*gpio_info);
+ cell.acpi_match = &ljca_acpi_match_gpio;
+
+ return ljca_add_mfd_cell(dev, &cell);
+}
+
+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_i2c_info *i2c_info;
+ struct ljca_stub *stub;
+ int ret;
+ int i;
+
+ stub = ljca_stub_alloc(dev, LJCA_I2C_STUB, size_mul(desc->num, sizeof(*i2c_info)));
+ if (IS_ERR(stub))
+ return PTR_ERR(stub);
+
+ i2c_info = ljca_priv(stub);
+
+ for (i = 0; i < desc->num; i++) {
+ struct mfd_cell cell = {};
+
+ i2c_info[i].ljca = &stub->ljca;
+ i2c_info[i].id = desc->info[i].id;
+ i2c_info[i].capacity = desc->info[i].capacity;
+ i2c_info[i].intr_pin = desc->info[i].intr_pin;
+
+ cell.name = "ljca-i2c";
+ cell.platform_data = &i2c_info[i];
+ cell.pdata_size = sizeof(i2c_info[i]);
+
+ if (i < ARRAY_SIZE(ljca_acpi_match_i2cs))
+ cell.acpi_match = &ljca_acpi_match_i2cs[i];
+
+ ret = ljca_add_mfd_cell(dev, &cell);
+ if (ret)
+ 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_spi_info *spi_info;
+ struct ljca_stub *stub;
+ int i;
+ int ret;
+
+ stub = ljca_stub_alloc(dev, LJCA_SPI_STUB, size_mul(desc->num, sizeof(*spi_info)));
+ if (IS_ERR(stub))
+ return PTR_ERR(stub);
+
+ spi_info = ljca_priv(stub);
+ for (i = 0; i < desc->num; i++) {
+ struct mfd_cell cell = {};
+
+ spi_info[i].ljca = &stub->ljca;
+ spi_info[i].id = desc->info[i].id;
+ spi_info[i].capacity = desc->info[i].capacity;
+
+ cell.name = "ljca-spi";
+ cell.platform_data = &spi_info[i];
+ cell.pdata_size = sizeof(spi_info[i]);
+ if (i < ARRAY_SIZE(ljca_acpi_match_spis))
+ cell.acpi_match = &ljca_acpi_match_spis[i];
+
+ ret = ljca_add_mfd_cell(dev, &cell);
+ if (ret)
+ 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 */
+ ljca_mng_enum_gpio(stub);
+ ljca_mng_enum_i2c(stub);
+ ljca_mng_enum_spi(stub);
+
+ return 0;
+}
+
+static int ljca_mng_init(struct ljca_dev *dev)
+{
+ struct ljca_stub *stub;
+ struct ljca_mng_priv *priv;
+ int ret;
+
+ stub = ljca_stub_alloc(dev, LJCA_MNG_STUB, sizeof(*priv));
+ if (IS_ERR(stub))
+ return PTR_ERR(stub);
+
+ priv = ljca_priv(stub);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->reset_id = 0;
+
+ 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);
+ usb_put_dev(dev->udev);
+ kfree(dev->ibuf);
+ kfree(dev->cells);
+ kfree(dev);
+}
+
+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)
+{
+ dev->started = false;
+ usb_kill_urb(dev->in_urb);
+}
+
+static ssize_t cmd_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);
+ struct ljca_stub *diag_stub = ljca_stub_find(ljca_dev, LJCA_DIAG_STUB);
+
+ if (sysfs_streq(buf, "dfu"))
+ ljca_mng_set_dfu_mode(mng_stub);
+ else if (sysfs_streq(buf, "debug"))
+ ljca_diag_set_trace_level(diag_stub, 3);
+
+ return count;
+}
+
+static ssize_t cmd_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "%s\n", "supported cmd: [dfu, debug]");
+}
+static DEVICE_ATTR_RW(cmd);
+
+static ssize_t 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(version);
+
+static struct attribute *ljca_attrs[] = {
+ &dev_attr_version.attr,
+ &dev_attr_cmd.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 = kzalloc(sizeof(*dev), GFP_KERNEL);
+ if (!dev)
+ return -ENOMEM;
+
+ ljca_init(dev);
+ dev->udev = usb_get_dev(interface_to_usbdev(intf));
+ 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 = kzalloc(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;
+ }
+
+ ret = mfd_add_hotplug_devices(&intf->dev, dev->cells, dev->cell_count);
+ if (ret) {
+ dev_err(&intf->dev, "failed to add mfd devices\n");
+ goto error_stop;
+ }
+
+ usb_enable_autosuspend(dev->udev);
+ return 0;
+error_stop:
+ ljca_stop(dev);
+error:
+ dev_err(&intf->dev, "LJCA USB device init failed\n");
+ /* this frees allocated memory */
+ ljca_stub_cleanup(dev);
+ ljca_delete(dev);
+ return ret;
+}
+
+static void ljca_disconnect(struct usb_interface *intf)
+{
+ struct ljca_dev *dev = usb_get_intfdata(intf);
+
+ ljca_stop(dev);
+ mfd_remove_devices(&intf->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 <[email protected]>");
+MODULE_AUTHOR("Wang Zhifeng <[email protected]>");
+MODULE_AUTHOR("Zhang Lixu <[email protected]>");
+MODULE_DESCRIPTION("Intel La Jolla Cove Adapter USB driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/ljca.h b/include/linux/mfd/ljca.h
new file mode 100644
index 000000000000..9ae3ea242294
--- /dev/null
+++ b/include/linux/mfd/ljca.h
@@ -0,0 +1,95 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _LINUX_USB_LJCA_H_
+#define _LINUX_USB_LJCA_H_
+
+#include <linux/types.h>
+
+struct ljca;
+
+#define LJCA_MAX_GPIO_NUM 64
+struct ljca_gpio_info {
+ struct ljca *ljca;
+ unsigned int num;
+ DECLARE_BITMAP(valid_pin_map, LJCA_MAX_GPIO_NUM);
+};
+
+struct ljca_i2c_info {
+ struct ljca *ljca;
+ u8 id;
+ u8 capacity;
+ u8 intr_pin;
+};
+
+struct ljca_spi_info {
+ struct ljca *ljca;
+ 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);
+
+/**
+ * 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
This patch 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 <[email protected]>
Reviewed-by: Sakari Ailus <[email protected]>
Reviewed-by: Linus Walleij <[email protected]>
---
drivers/gpio/Kconfig | 12 ++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-ljca.c | 454 +++++++++++++++++++++++++++++++++++++++
3 files changed, 467 insertions(+)
create mode 100644 drivers/gpio/gpio-ljca.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 13be729710f2..8be697f9f621 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1253,6 +1253,18 @@ config GPIO_KEMPLD
This driver can also be built as a module. If so, the module will be
called gpio-kempld.
+config GPIO_LJCA
+ tristate "INTEL La Jolla Cove Adapter GPIO support"
+ depends on MFD_LJCA
+ select GPIOLIB_IRQCHIP
+ default MFD_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.
+
config GPIO_LP3943
tristate "TI/National Semiconductor LP3943 GPIO expander"
depends on MFD_LP3943
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..87863f0230f5
--- /dev/null
+++ b/drivers/gpio/gpio-ljca.c
@@ -0,0 +1,454 @@
+// 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/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/mfd/ljca.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/types.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 platform_device *pdev;
+ 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);
+ 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->gpio_info->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->gpio_info->ljca, LJCA_GPIO_READ, packet,
+ struct_size(packet, item, packet->num), ljca_gpio->ibuf, &ibuf_len);
+ if (ret)
+ goto out_unlock;
+
+ if (!ibuf_len || ack_packet->num != packet->num) {
+ dev_err(&ljca_gpio->pdev->dev, "failed gpio_id:%u %u", gpio_id, ack_packet->num);
+ ret = -EIO;
+ }
+
+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->gpio_info->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, "offset:%u val:%d set value failed %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;
+
+ return gpio_config(ljca_gpio, offset, config);
+}
+
+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);
+ return 0;
+}
+
+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->gpio_info->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 platform_device *pdev)
+{
+ struct ljca_gpio_dev *ljca_gpio;
+ struct gpio_irq_chip *girq;
+ int ret;
+
+ ljca_gpio = devm_kzalloc(&pdev->dev, sizeof(*ljca_gpio), GFP_KERNEL);
+ if (!ljca_gpio)
+ return -ENOMEM;
+
+ ljca_gpio->gpio_info = dev_get_platdata(&pdev->dev);
+ ljca_gpio->connect_mode = devm_kcalloc(&pdev->dev, ljca_gpio->gpio_info->num,
+ sizeof(*ljca_gpio->connect_mode), GFP_KERNEL);
+ if (!ljca_gpio->connect_mode)
+ return -ENOMEM;
+
+ mutex_init(&ljca_gpio->irq_lock);
+ mutex_init(&ljca_gpio->trans_lock);
+ ljca_gpio->pdev = pdev;
+ ljca_gpio->gc.direction_input = ljca_gpio_direction_input;
+ ljca_gpio->gc.direction_output = ljca_gpio_direction_output;
+ ljca_gpio->gc.get = ljca_gpio_get_value;
+ ljca_gpio->gc.set = ljca_gpio_set_value;
+ ljca_gpio->gc.set_config = ljca_gpio_set_config;
+ ljca_gpio->gc.init_valid_mask = ljca_gpio_init_valid_mask;
+ ljca_gpio->gc.can_sleep = true;
+ ljca_gpio->gc.parent = &pdev->dev;
+
+ ljca_gpio->gc.base = -1;
+ ljca_gpio->gc.ngpio = ljca_gpio->gpio_info->num;
+ ljca_gpio->gc.label = ACPI_COMPANION(&pdev->dev) ?
+ acpi_dev_name(ACPI_COMPANION(&pdev->dev)) :
+ dev_name(&pdev->dev);
+ ljca_gpio->gc.owner = THIS_MODULE;
+
+ platform_set_drvdata(pdev, ljca_gpio);
+ ljca_register_event_cb(ljca_gpio->gpio_info->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_gpio->gpio_info->ljca);
+ mutex_destroy(&ljca_gpio->irq_lock);
+ mutex_destroy(&ljca_gpio->trans_lock);
+ }
+
+ return ret;
+}
+
+static int ljca_gpio_remove(struct platform_device *pdev)
+{
+ struct ljca_gpio_dev *ljca_gpio = platform_get_drvdata(pdev);
+
+ gpiochip_remove(&ljca_gpio->gc);
+ ljca_unregister_event_cb(ljca_gpio->gpio_info->ljca);
+ mutex_destroy(&ljca_gpio->irq_lock);
+ mutex_destroy(&ljca_gpio->trans_lock);
+ return 0;
+}
+
+#define LJCA_GPIO_DRV_NAME "ljca-gpio"
+static const struct platform_device_id ljca_gpio_id[] = {
+ { LJCA_GPIO_DRV_NAME, 0 },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, ljca_gpio_id);
+
+static struct platform_driver ljca_gpio_driver = {
+ .driver.name = LJCA_GPIO_DRV_NAME,
+ .probe = ljca_gpio_probe,
+ .remove = ljca_gpio_remove,
+};
+module_platform_driver(ljca_gpio_driver);
+
+MODULE_AUTHOR("Ye Xiang <[email protected]>");
+MODULE_AUTHOR("Wang Zhifeng <[email protected]>");
+MODULE_AUTHOR("Zhang Lixu <[email protected]>");
+MODULE_DESCRIPTION("Intel La Jolla Cove Adapter USB-GPIO driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(LJCA);
--
2.34.1
This patch 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 <[email protected]>
Reviewed-by: Sakari Ailus <[email protected]>
---
drivers/i2c/busses/Kconfig | 11 ++
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-ljca.c | 357 ++++++++++++++++++++++++++++++++++
3 files changed, 369 insertions(+)
create mode 100644 drivers/i2c/busses/i2c-ljca.c
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 25eb4e8fd22f..d523e9591425 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 MFD_LJCA
+ default MFD_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..3250297c15b8
--- /dev/null
+++ b/drivers/i2c/busses/i2c-ljca.c
@@ -0,0 +1,357 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Intel La Jolla Cove Adapter USB-I2C driver
+ *
+ * Copyright (c) 2023, Intel Corporation.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/dev_printk.h>
+#include <linux/i2c.h>
+#include <linux/mfd/ljca.h>
+#include <linux/module.h>
+#include <linux/platform_device.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_READ_XFER_TYPE,
+ LJCA_I2C_WRITE_XFER_TYPE,
+};
+
+/* I2C r/w Flags */
+#define LJCA_I2C_SLAVE_TRANSFER_WRITE (0)
+#define LJCA_I2C_SLAVE_TRANSFER_READ (1)
+
+/* 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 platform_device *pdev;
+ 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 == LJCA_I2C_READ_XFER_TYPE) ?
+ LJCA_I2C_SLAVE_TRANSFER_READ :
+ LJCA_I2C_SLAVE_TRANSFER_WRITE;
+}
+
+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->i2c_info->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->i2c_info->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->i2c_info->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->i2c_info->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->i2c_info->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 platform_device *pdev)
+{
+ struct ljca_i2c_dev *ljca_i2c;
+ int ret;
+
+ ljca_i2c = devm_kzalloc(&pdev->dev, sizeof(*ljca_i2c), GFP_KERNEL);
+ if (!ljca_i2c)
+ return -ENOMEM;
+
+ ljca_i2c->pdev = pdev;
+ ljca_i2c->i2c_info = dev_get_platdata(&pdev->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 = &pdev->dev;
+ device_set_node(&ljca_i2c->adap.dev, dev_fwnode(&pdev->dev));
+ i2c_set_adapdata(&ljca_i2c->adap, ljca_i2c);
+ snprintf(ljca_i2c->adap.name, sizeof(ljca_i2c->adap.name), "%s-%s-%d",
+ dev_name(&pdev->dev), dev_name(pdev->dev.parent),
+ ljca_i2c->i2c_info->id);
+
+ platform_set_drvdata(pdev, ljca_i2c);
+
+ ret = ljca_i2c_init(ljca_i2c, ljca_i2c->i2c_info->id);
+ if (ret) {
+ dev_err(&pdev->dev, "i2c init failed id:%d\n", ljca_i2c->i2c_info->id);
+ return -EIO;
+ }
+
+ ret = devm_i2c_add_adapter(&pdev->dev, &ljca_i2c->adap);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+#define LJCA_I2C_DRV_NAME "ljca-i2c"
+static const struct platform_device_id ljca_i2c_id[] = {
+ { LJCA_I2C_DRV_NAME, 0 },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, ljca_i2c_id);
+
+static struct platform_driver ljca_i2c_driver = {
+ .driver.name = LJCA_I2C_DRV_NAME,
+ .probe = ljca_i2c_probe,
+};
+
+module_platform_driver(ljca_i2c_driver);
+
+MODULE_AUTHOR("Ye Xiang <[email protected]>");
+MODULE_AUTHOR("Wang Zhifeng <[email protected]>");
+MODULE_AUTHOR("Zhang Lixu <[email protected]>");
+MODULE_DESCRIPTION("Intel La Jolla Cove Adapter USB-I2C driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(LJCA);
--
2.34.1
This patch 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 <[email protected]>
Reviewed-by: Sakari Ailus <[email protected]>
---
drivers/spi/Kconfig | 11 ++
drivers/spi/Makefile | 1 +
drivers/spi/spi-ljca.c | 291 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 303 insertions(+)
create mode 100644 drivers/spi/spi-ljca.c
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 47bbba04fe3a..024652e43e4c 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 MFD_LJCA
+ default MFD_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..74815f752867
--- /dev/null
+++ b/drivers/spi/spi-ljca.c
@@ -0,0 +1,291 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Intel La Jolla Cove Adapter USB-SPI driver
+ *
+ * Copyright (c) 2023, Intel Corporation.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/dev_printk.h>
+#include <linux/mfd/ljca.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/spi/spi.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)
+
+struct spi_init_packet {
+ u8 index;
+ u8 speed;
+ u8 mode;
+} __packed;
+
+#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_xfer_packet {
+ u8 indicator;
+ s8 len;
+ u8 data[];
+} __packed;
+
+struct ljca_spi_dev {
+ struct platform_device *pdev;
+ 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->spi_info->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->spi_info->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->spi_info->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->pdev->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->pdev->dev, "ljca spi transfer failed!\n");
+
+ return ret;
+}
+
+static int ljca_spi_probe(struct platform_device *pdev)
+{
+ struct spi_controller *controller;
+ struct ljca_spi_dev *ljca_spi;
+ int ret;
+
+ controller = devm_spi_alloc_master(&pdev->dev, sizeof(*ljca_spi));
+ if (!controller)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, controller);
+ ljca_spi = spi_controller_get_devdata(controller);
+
+ ljca_spi->spi_info = dev_get_platdata(&pdev->dev);
+ ljca_spi->controller = controller;
+ ljca_spi->pdev = pdev;
+ device_set_node(&ljca_spi->controller->dev, dev_fwnode(&pdev->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(&pdev->dev, "Failed to register controller\n");
+
+ return ret;
+}
+
+static int ljca_spi_dev_remove(struct platform_device *pdev)
+{
+ struct spi_controller *controller = platform_get_drvdata(pdev);
+ struct ljca_spi_dev *ljca_spi = spi_controller_get_devdata(controller);
+
+ spi_unregister_controller(controller);
+ ljca_spi_deinit(ljca_spi);
+ return 0;
+}
+
+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-spi"
+static const struct platform_device_id ljca_spi_id[] = {
+ { LJCA_SPI_DRV_NAME, 0 },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, ljca_spi_id);
+
+static struct platform_driver spi_ljca_driver = {
+ .driver = {
+ .name = LJCA_SPI_DRV_NAME,
+ .pm = &ljca_spi_pm,
+ },
+ .probe = ljca_spi_probe,
+ .remove = ljca_spi_dev_remove,
+};
+
+module_platform_driver(spi_ljca_driver);
+
+MODULE_AUTHOR("Ye Xiang <[email protected]>");
+MODULE_AUTHOR("Wang Zhifeng <[email protected]>");
+MODULE_AUTHOR("Zhang Lixu <[email protected]>");
+MODULE_DESCRIPTION("Intel La Jolla Cove Adapter USB-SPI driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(LJCA);
--
2.34.1
Add sysfs attributes Documentation entries for LJCA device
Signed-off-by: Ye Xiang <[email protected]>
Reviewed-by: Sakari Ailus <[email protected]>
---
.../ABI/testing/sysfs-bus-usb-devices-ljca | 22 +++++++++++++++++++
1 file changed, 22 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..e12fe78ae58b
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-usb-devices-ljca
@@ -0,0 +1,22 @@
+What: /sys/bus/usb/.../version
+Date: July 2023
+KernelVersion: 6.4
+Contact: Ye Xiang <[email protected]>
+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/.../cmd
+Date: July 2023
+KernelVersion: 6.4
+Contact: Ye Xiang <[email protected]>
+Description:
+ Commands supported by LJCA device.
+ When read, it will return valid commands.
+ When write with a command, it will execute the command.
+ Valid commands are [dfu, debug]
+ - Writing "dfu" to this file so as to put the device into
+ DFU mode so the firmware can be updated.
+ - Writing "debug" to this file to enable debug logging.
--
2.34.1
On Thu, Mar 09, 2023 at 03:10:56PM +0800, Ye Xiang wrote:
> This patch implements the USB part of Intel USB-I2C/GPIO/SPI adapter
> device named "La Jolla Cove Adapter" (LJCA).
Then why is this called "mfd" in the subject line?
> include/linux/mfd/ljca.h | 95 ++++
Why is this .h file in the mfd directory?
thanks,
greg k-h
On Thu, Mar 09, 2023 at 03:10:56PM +0800, Ye Xiang wrote:
> +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));
> +}
You have sysfs files, yet no Documentation/ABI/ entries? That's not
allowed, you know this :(
> +static ssize_t cmd_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);
> + struct ljca_stub *diag_stub = ljca_stub_find(ljca_dev, LJCA_DIAG_STUB);
> +
> + if (sysfs_streq(buf, "dfu"))
> + ljca_mng_set_dfu_mode(mng_stub);
> + else if (sysfs_streq(buf, "debug"))
> + ljca_diag_set_trace_level(diag_stub, 3);
Sorry, but no, you can't do this in a sysfs file.
> +
> + return count;
> +}
> +
> +static ssize_t cmd_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + return sysfs_emit(buf, "%s\n", "supported cmd: [dfu, debug]");
sysfs files do not show "help text".
thanks,
greg k-h
On Thu, Mar 9, 2023, at 08:10, Ye Xiang wrote:
> 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)
> }
> }
I'm a bit confused by this bit, does that mean this only works
if the device is integrated on the mainboard and the BIOS is
aware of it? This won't work if you plug it into a random
USB port, or have no ACPI firmware, right?
> Signed-off-by: Ye Xiang <[email protected]>
> Reviewed-by: Sakari Ailus <[email protected]>
> ---
> drivers/usb/misc/Kconfig | 13 +
> drivers/usb/misc/Makefile | 1 +
> drivers/usb/misc/ljca.c | 969 ++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/ljca.h | 95 ++++
Why is this in driver/usb/misc? It looks like a normal
mfd driver to me, and it evenhas the header in include/linux/mfd/
Arnd
Hi Greg,
Thanks for the review.
On Thu, Mar 09, 2023 at 08:49:33AM +0100, Greg Kroah-Hartman wrote:
> On Thu, Mar 09, 2023 at 03:10:56PM +0800, Ye Xiang wrote:
> > This patch implements the USB part of Intel USB-I2C/GPIO/SPI adapter
> > device named "La Jolla Cove Adapter" (LJCA).
>
> Then why is this called "mfd" in the subject line?
Sorry, it's a mistake. I forget to change mfd to usb in the commit message
because I just move the ljca.c from driver/mfd to drivers/usb/misc according
to previous review comments[1]. And I will address this on v5.
[1] https://www.spinics.net/lists/kernel/msg4708451.html
>
> > include/linux/mfd/ljca.h | 95 ++++
>
> Why is this .h file in the mfd directory?
It's a mistake as well. Will address it by moving include/linux/mfd/ljca.h
to include/linux/usb/ljca.h.
>
> thanks,
>
> greg k-h
--
Thanks
Ye Xiang
On Thu, Mar 09, 2023 at 05:10:48PM +0800, Ye, Xiang wrote:
> Hi Greg,
>
> Thanks for the review.
> On Thu, Mar 09, 2023 at 08:49:33AM +0100, Greg Kroah-Hartman wrote:
> > On Thu, Mar 09, 2023 at 03:10:56PM +0800, Ye Xiang wrote:
> > > This patch implements the USB part of Intel USB-I2C/GPIO/SPI adapter
> > > device named "La Jolla Cove Adapter" (LJCA).
> >
> > Then why is this called "mfd" in the subject line?
> Sorry, it's a mistake. I forget to change mfd to usb in the commit message
> because I just move the ljca.c from driver/mfd to drivers/usb/misc according
> to previous review comments[1]. And I will address this on v5.
>
> [1] https://www.spinics.net/lists/kernel/msg4708451.html
> >
> > > include/linux/mfd/ljca.h | 95 ++++
> >
> > Why is this .h file in the mfd directory?
> It's a mistake as well. Will address it by moving include/linux/mfd/ljca.h
> to include/linux/usb/ljca.h.
Why do you need a .h file at all for such a tiny .c file? If you don't
need it, don't have one please.
thanks,
greg k-h
On Thu, Mar 09, 2023 at 08:52:24AM +0100, Greg Kroah-Hartman wrote:
> On Thu, Mar 09, 2023 at 03:10:56PM +0800, Ye Xiang wrote:
> > +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));
> > +}
>
> You have sysfs files, yet no Documentation/ABI/ entries? That's not
> allowed, you know this :(
The Documentation/ABI/ entries is added for the sysfs on patch 5 of this series.
https://patchwork.kernel.org/project/linux-usb/patch/[email protected]/
>
> > +static ssize_t cmd_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);
> > + struct ljca_stub *diag_stub = ljca_stub_find(ljca_dev, LJCA_DIAG_STUB);
> > +
> > + if (sysfs_streq(buf, "dfu"))
> > + ljca_mng_set_dfu_mode(mng_stub);
> > + else if (sysfs_streq(buf, "debug"))
> > + ljca_diag_set_trace_level(diag_stub, 3);
>
> Sorry, but no, you can't do this in a sysfs file.
Do you mean that we can't use sysfs to send "debug" command to device?
Could you provide some detail or hints?
>
> > +
> > + return count;
> > +}
> > +
> > +static ssize_t cmd_show(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > + return sysfs_emit(buf, "%s\n", "supported cmd: [dfu, debug]");
>
> sysfs files do not show "help text".
Ok. I can remove this for this has been described in
Documentation/ABI/testing/sysfs-bus-usb-devices-ljca (added in patch 5
of this series).
>
> thanks,
>
> greg k-h
--
Thanks
Ye Xiang
On Thu, Mar 09, 2023 at 05:31:44PM +0800, Ye, Xiang wrote:
> On Thu, Mar 09, 2023 at 08:52:24AM +0100, Greg Kroah-Hartman wrote:
> > On Thu, Mar 09, 2023 at 03:10:56PM +0800, Ye Xiang wrote:
> > > +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));
> > > +}
> >
> > You have sysfs files, yet no Documentation/ABI/ entries? That's not
> > allowed, you know this :(
> The Documentation/ABI/ entries is added for the sysfs on patch 5 of this series.
> https://patchwork.kernel.org/project/linux-usb/patch/[email protected]/
Ah, missed that, sorry.
> >
> > > +static ssize_t cmd_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);
> > > + struct ljca_stub *diag_stub = ljca_stub_find(ljca_dev, LJCA_DIAG_STUB);
> > > +
> > > + if (sysfs_streq(buf, "dfu"))
> > > + ljca_mng_set_dfu_mode(mng_stub);
> > > + else if (sysfs_streq(buf, "debug"))
> > > + ljca_diag_set_trace_level(diag_stub, 3);
> >
> > Sorry, but no, you can't do this in a sysfs file.
> Do you mean that we can't use sysfs to send "debug" command to device?
That is correct, use the kernel-wide debugging facilities that we have
for this NEVER create your own custom interface just for one tiny
driver, that is not allowed.
> Could you provide some detail or hints?
dev_dbg().
thanks,
greg k-h
Hi Arnd,
Thanks for the review.
On Thu, Mar 09, 2023 at 08:56:05AM +0100, Arnd Bergmann wrote:
> On Thu, Mar 9, 2023, at 08:10, Ye Xiang wrote:
>
> > 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)
> > }
> > }
>
> I'm a bit confused by this bit, does that mean this only works
> if the device is integrated on the mainboard and the BIOS is
> aware of it? This won't work if you plug it into a random
> USB port, or have no ACPI firmware, right?
LJCA can work both when there is an ACPI firmware or not.
When there is a config like "The minimum code in ASL" in DSDT, LJCA device and
its sub-devices will bind to right ACPI devices. Our current use case needs the
the ACPI binding. Because we have a camera sensor depending on this LJCA device,
We use the ACPI binding to control the probe order, making sure the camera
sensor probe after LJCA and LJCA-I2C.
When there isn't a config in DSDT, the LJCA device also can works. But
it won't have an acpi binding.
So, If you plug into a random USB port or have no ACPI binding, It just
does not have an ACPI binding.
>
> > Signed-off-by: Ye Xiang <[email protected]>
> > Reviewed-by: Sakari Ailus <[email protected]>
> > ---
> > drivers/usb/misc/Kconfig | 13 +
> > drivers/usb/misc/Makefile | 1 +
> > drivers/usb/misc/ljca.c | 969 ++++++++++++++++++++++++++++++++++++++
> > include/linux/mfd/ljca.h | 95 ++++
>
> Why is this in driver/usb/misc? It looks like a normal
> mfd driver to me, and it evenhas the header in include/linux/mfd/
It's a mistake. I just moved it from driver/mfd and forget to move the header to right place.
It will be addressed on next version.
>
> Arnd
--
Thanks
Ye Xiang
On Thu, Mar 09, 2023 at 10:41:05AM +0100, Greg Kroah-Hartman wrote:
> On Thu, Mar 09, 2023 at 05:31:44PM +0800, Ye, Xiang wrote:
> > On Thu, Mar 09, 2023 at 08:52:24AM +0100, Greg Kroah-Hartman wrote:
> > > On Thu, Mar 09, 2023 at 03:10:56PM +0800, Ye Xiang wrote:
> > > > +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));
> > > > +}
> > >
> > > You have sysfs files, yet no Documentation/ABI/ entries? That's not
> > > allowed, you know this :(
> > The Documentation/ABI/ entries is added for the sysfs on patch 5 of this series.
> > https://patchwork.kernel.org/project/linux-usb/patch/[email protected]/
>
> Ah, missed that, sorry.
>
> > >
> > > > +static ssize_t cmd_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);
> > > > + struct ljca_stub *diag_stub = ljca_stub_find(ljca_dev, LJCA_DIAG_STUB);
> > > > +
> > > > + if (sysfs_streq(buf, "dfu"))
> > > > + ljca_mng_set_dfu_mode(mng_stub);
> > > > + else if (sysfs_streq(buf, "debug"))
> > > > + ljca_diag_set_trace_level(diag_stub, 3);
> > >
> > > Sorry, but no, you can't do this in a sysfs file.
> > Do you mean that we can't use sysfs to send "debug" command to device?
>
> That is correct, use the kernel-wide debugging facilities that we have
> for this NEVER create your own custom interface just for one tiny
> driver, that is not allowed.
>
> > Could you provide some detail or hints?
>
> dev_dbg().
I'm not sure this is the same thing, though, as it's not a drvier
to user debug message.
Ye, can you please explain better what this command does? You are
sending a LJCA_DIAG_SET_TRACE_LEVEL command to the device with a
parameter "3" which has a meaining only for you :)
What is ugly here is the parsing of "dfu" and "debug", I would
just make different boolean files, something like:
echo 1 > ljca_dfu
echo 0 > ljca_dfu
and
echo N > ljca_trace_level
with a proper documentation of the trace levels.
The show counterparast can provide a feedback on how the
interfaces are set rather than providing some help message, which
is not what sysfs files are meant for.
Would this work?
Andi
On Thu, Mar 09, 2023 at 10:26:27AM +0100, Greg Kroah-Hartman wrote:
> On Thu, Mar 09, 2023 at 05:10:48PM +0800, Ye, Xiang wrote:
> > Hi Greg,
> >
> > Thanks for the review.
> > On Thu, Mar 09, 2023 at 08:49:33AM +0100, Greg Kroah-Hartman wrote:
> > > On Thu, Mar 09, 2023 at 03:10:56PM +0800, Ye Xiang wrote:
> > > > This patch implements the USB part of Intel USB-I2C/GPIO/SPI adapter
> > > > device named "La Jolla Cove Adapter" (LJCA).
> > >
> > > Then why is this called "mfd" in the subject line?
> > Sorry, it's a mistake. I forget to change mfd to usb in the commit message
> > because I just move the ljca.c from driver/mfd to drivers/usb/misc according
> > to previous review comments[1]. And I will address this on v5.
> >
> > [1] https://www.spinics.net/lists/kernel/msg4708451.html
> > >
> > > > include/linux/mfd/ljca.h | 95 ++++
> > >
> > > Why is this .h file in the mfd directory?
> > It's a mistake as well. Will address it by moving include/linux/mfd/ljca.h
> > to include/linux/usb/ljca.h.
>
> Why do you need a .h file at all for such a tiny .c file? If you don't
> need it, don't have one please.
The ljca.h file is needed because there are three function driver in
this series (LJCA-SPI, LJCA-GPIO, and LJCA-I2C) which needs LJCA API
to do actual transfers. LJCA-USB can be considered as a transport driver,
and the other three function driver use LJCA API to transfer their own
packages.
--
Thanks
Ye Xiang
On Thu, Mar 09, 2023 at 08:56:05AM +0100, Arnd Bergmann wrote:
> On Thu, Mar 9, 2023, at 08:10, Ye Xiang wrote:
> > drivers/usb/misc/Kconfig | 13 +
> > drivers/usb/misc/Makefile | 1 +
> > drivers/usb/misc/ljca.c | 969 ++++++++++++++++++++++++++++++++++++++
> > include/linux/mfd/ljca.h | 95 ++++
> Why is this in driver/usb/misc? It looks like a normal
> mfd driver to me, and it evenhas the header in include/linux/mfd/
It was a MFD in the original version, Lee asked for it to be moved to
USB: https://lore.kernel.org/r/[email protected]
On Thu, Mar 9, 2023, at 12:03, Mark Brown wrote:
> On Thu, Mar 09, 2023 at 08:56:05AM +0100, Arnd Bergmann wrote:
>> On Thu, Mar 9, 2023, at 08:10, Ye Xiang wrote:
>
>> > drivers/usb/misc/Kconfig | 13 +
>> > drivers/usb/misc/Makefile | 1 +
>> > drivers/usb/misc/ljca.c | 969 ++++++++++++++++++++++++++++++++++++++
>> > include/linux/mfd/ljca.h | 95 ++++
>
>> Why is this in driver/usb/misc? It looks like a normal
>> mfd driver to me, and it evenhas the header in include/linux/mfd/
>
> It was a MFD in the original version, Lee asked for it to be moved to
> USB: https://lore.kernel.org/r/[email protected]
Ok, I see. That should probably be mentioned in the patch
description then. I'm still not sure I follow the reasoning
for the split between "usb functionality" and and mfd part:
This is just a usb driver as it is attached to a usb bus, and
drivers usually get put into a directory based on what they
provide, not how they are attached to a parent bus.
Arnd
On 09.03.23 08:10, Ye Xiang wrote:
> +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)
Why do you make ibuf_len a pointer?
> +{
> + 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) {
Memory leak in error case. You must free 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(dev->udev, usb_sndbulkpipe(dev->udev, 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;
If that triggers, you may have a pending URB.
You must kill it.
> + goto error_unlock;
> + }
> + }
> +
> + stub->ipacket.ibuf = NULL;
> + stub->ipacket.ibuf_len = NULL;
> + ret = 0;
> +error_unlock:
> + mutex_unlock(&dev->mutex);
> +error_put:
> + usb_autopm_put_interface(dev->intf);
> + return ret;
> +}
> +static int ljca_i2c_stub_init(struct ljca_dev *dev, struct ljca_i2c_descriptor *desc)
> +{
> + struct ljca_i2c_info *i2c_info;
> + struct ljca_stub *stub;
> + int ret;
> + int i;
> +
> + stub = ljca_stub_alloc(dev, LJCA_I2C_STUB, size_mul(desc->num, sizeof(*i2c_info)));
> + if (IS_ERR(stub))
> + return PTR_ERR(stub);
> +
> + i2c_info = ljca_priv(stub);
> +
> + for (i = 0; i < desc->num; i++) {
> + struct mfd_cell cell = {};
> +
> + i2c_info[i].ljca = &stub->ljca;
> + i2c_info[i].id = desc->info[i].id;
> + i2c_info[i].capacity = desc->info[i].capacity;
> + i2c_info[i].intr_pin = desc->info[i].intr_pin;
> +
> + cell.name = "ljca-i2c";
> + cell.platform_data = &i2c_info[i];
> + cell.pdata_size = sizeof(i2c_info[i]);
> +
> + if (i < ARRAY_SIZE(ljca_acpi_match_i2cs))
> + cell.acpi_match = &ljca_acpi_match_i2cs[i];
> +
> + ret = ljca_add_mfd_cell(dev, &cell);
> + if (ret)
> + return ret;
What happens to stub in the error case?
> + }
> +
> + return 0;
> +}
> +
> +static void ljca_disconnect(struct usb_interface *intf)
> +{
> + struct ljca_dev *dev = usb_get_intfdata(intf);
> +
> + ljca_stop(dev);
What prevents restarting the device here?
> + mfd_remove_devices(&intf->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);
So here you report errors, but at the same time you set "started"
even if errors occur.
Regards
Oliver
On 09.03.23 08:10, Ye Xiang wrote:
> +#define LJCA_GPIO_BUF_SIZE 60
> +struct ljca_gpio_dev {
> + struct platform_device *pdev;
> + 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);
> + 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];
And here we have a violation of DMA coherency rules.
Basically you cannot embed buffers into other data structures
if they can be subject to DMA.
> +static int ljca_gpio_remove(struct platform_device *pdev)
> +{
> + struct ljca_gpio_dev *ljca_gpio = platform_get_drvdata(pdev);
> +
> + gpiochip_remove(&ljca_gpio->gc);
> + ljca_unregister_event_cb(ljca_gpio->gpio_info->ljca);
> + mutex_destroy(&ljca_gpio->irq_lock);
> + mutex_destroy(&ljca_gpio->trans_lock);
At this time, what has made sure that no work is scheduled?
> + return 0;
> +}
Regards
Oliver
On Thu, Mar 09, 2023 at 02:40:10PM +0100, Oliver Neukum wrote:
> On 09.03.23 08:10, Ye Xiang wrote:
>
> > +#define LJCA_GPIO_BUF_SIZE 60
> > +struct ljca_gpio_dev {
> > + struct platform_device *pdev;
> > + 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);
> > + 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];
>
> And here we have a violation of DMA coherency rules.
> Basically you cannot embed buffers into other data structures
> if they can be subject to DMA.
Huh?!
The problem here is alignment. But other than that I can't see the issue with
embedding into structures the instances of which will be allocated on the heap.
--
With Best Regards,
Andy Shevchenko
On Thu, Mar 09, 2023 at 03:52:59PM +0200, Andy Shevchenko wrote:
> On Thu, Mar 09, 2023 at 02:40:10PM +0100, Oliver Neukum wrote:
> > On 09.03.23 08:10, Ye Xiang wrote:
> >
> > > +#define LJCA_GPIO_BUF_SIZE 60
> > > +struct ljca_gpio_dev {
> > > + struct platform_device *pdev;
> > > + 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);
> > > + 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];
> >
> > And here we have a violation of DMA coherency rules.
> > Basically you cannot embed buffers into other data structures
> > if they can be subject to DMA.
>
> Huh?!
>
> The problem here is alignment. But other than that I can't see the issue with
> embedding into structures the instances of which will be allocated on the heap.
As you said, the problem is alignment.
On Thu, Mar 9, 2023 at 2:53 PM Andy Shevchenko
<[email protected]> wrote:
> On Thu, Mar 09, 2023 at 02:40:10PM +0100, Oliver Neukum wrote:
> > On 09.03.23 08:10, Ye Xiang wrote:
> >
> > > +#define LJCA_GPIO_BUF_SIZE 60
> > > +struct ljca_gpio_dev {
> > > + struct platform_device *pdev;
> > > + 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);
> > > + 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];
> >
> > And here we have a violation of DMA coherency rules.
> > Basically you cannot embed buffers into other data structures
> > if they can be subject to DMA.
>
> Huh?!
>
> The problem here is alignment. But other than that I can't see the issue with
> embedding into structures the instances of which will be allocated on the heap.
Yups. And I think the solution looks something like this:
u8 obuf[LJCA_GPIO_BUF_SIZE] __aligned(8);
u8 ibuf[LJCA_GPIO_BUF_SIZE] __aligned(8);
__aligned(4) if it's 32bit DMA I guess? 8 always works that's
why we use it all over the IIO subsystem.
Yours,
Linus Walleij
On Thu, Mar 09, 2023 at 03:18:53PM +0100, Linus Walleij wrote:
> On Thu, Mar 9, 2023 at 2:53 PM Andy Shevchenko
> <[email protected]> wrote:
> > On Thu, Mar 09, 2023 at 02:40:10PM +0100, Oliver Neukum wrote:
> > > On 09.03.23 08:10, Ye Xiang wrote:
> > >
> > > > +#define LJCA_GPIO_BUF_SIZE 60
> > > > +struct ljca_gpio_dev {
> > > > + struct platform_device *pdev;
> > > > + 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);
> > > > + 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];
> > >
> > > And here we have a violation of DMA coherency rules.
> > > Basically you cannot embed buffers into other data structures
> > > if they can be subject to DMA.
> >
> > Huh?!
> >
> > The problem here is alignment. But other than that I can't see the issue with
> > embedding into structures the instances of which will be allocated on the heap.
>
> Yups. And I think the solution looks something like this:
>
> u8 obuf[LJCA_GPIO_BUF_SIZE] __aligned(8);
> u8 ibuf[LJCA_GPIO_BUF_SIZE] __aligned(8);
>
> __aligned(4) if it's 32bit DMA I guess? 8 always works that's
> why we use it all over the IIO subsystem.
To make it all simple, just make obuf and ibuf pointers to the data you
allocate with a call to kmalloc().
thanks,
greg k-h
On Thu, Mar 9, 2023, at 15:18, Linus Walleij wrote:
> On Thu, Mar 9, 2023 at 2:53 PM Andy Shevchenko
> <[email protected]> wrote:
>
> Yups. And I think the solution looks something like this:
>
> u8 obuf[LJCA_GPIO_BUF_SIZE] __aligned(8);
> u8 ibuf[LJCA_GPIO_BUF_SIZE] __aligned(8);
>
> __aligned(4) if it's 32bit DMA I guess? 8 always works that's
> why we use it all over the IIO subsystem.
I think it must be __aligned(ARCH_DMA_MINALIGN), to account
for architectures with non-coherent DMA.
Arnd
On Thu, Mar 09, 2023 at 11:06:25AM +0100, Andi Shyti wrote:
> On Thu, Mar 09, 2023 at 10:41:05AM +0100, Greg Kroah-Hartman wrote:
> > On Thu, Mar 09, 2023 at 05:31:44PM +0800, Ye, Xiang wrote:
> > > On Thu, Mar 09, 2023 at 08:52:24AM +0100, Greg Kroah-Hartman wrote:
> > > > On Thu, Mar 09, 2023 at 03:10:56PM +0800, Ye Xiang wrote:
> > > > > +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));
> > > > > +}
> > > >
> > > > You have sysfs files, yet no Documentation/ABI/ entries? That's not
> > > > allowed, you know this :(
> > > The Documentation/ABI/ entries is added for the sysfs on patch 5 of this series.
> > > https://patchwork.kernel.org/project/linux-usb/patch/[email protected]/
> >
> > Ah, missed that, sorry.
> >
> > > >
> > > > > +static ssize_t cmd_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);
> > > > > + struct ljca_stub *diag_stub = ljca_stub_find(ljca_dev, LJCA_DIAG_STUB);
> > > > > +
> > > > > + if (sysfs_streq(buf, "dfu"))
> > > > > + ljca_mng_set_dfu_mode(mng_stub);
> > > > > + else if (sysfs_streq(buf, "debug"))
> > > > > + ljca_diag_set_trace_level(diag_stub, 3);
> > > >
> > > > Sorry, but no, you can't do this in a sysfs file.
> > > Do you mean that we can't use sysfs to send "debug" command to device?
> >
> > That is correct, use the kernel-wide debugging facilities that we have
> > for this NEVER create your own custom interface just for one tiny
> > driver, that is not allowed.
> >
> > > Could you provide some detail or hints?
> >
> > dev_dbg().
But, this command is sent to SET LJCA Firmware logging level.
>
> I'm not sure this is the same thing, though, as it's not a drvier
> to user debug message.
>
> Ye, can you please explain better what this command does? You are
> sending a LJCA_DIAG_SET_TRACE_LEVEL command to the device with a
> parameter "3" which has a meaining only for you :)
Sure, the LJCA_DIAG_SET_TRACE_LEVEL command is used to set LJCA FW
logging level. 3 means debug level for FW. It is used for LJCA FW
debugging: when FW got some issue, we can send debug level to FW
to make FW print degging log for analysis.
>
> What is ugly here is the parsing of "dfu" and "debug", I would
> just make different boolean files, something like:
>
> echo 1 > ljca_dfu
> echo 0 > ljca_dfu
>
> and
>
> echo N > ljca_trace_level
>
> with a proper documentation of the trace levels.
>
> The show counterparast can provide a feedback on how the
> interfaces are set rather than providing some help message, which
> is not what sysfs files are meant for.
>
> Would this work?
Thanks. It should works, and I will try it.
--
Thanks
Ye Xiang
On Thu, Mar 09, 2023 at 11:45:51PM +0800, Ye, Xiang wrote:
> On Thu, Mar 09, 2023 at 11:06:25AM +0100, Andi Shyti wrote:
> > On Thu, Mar 09, 2023 at 10:41:05AM +0100, Greg Kroah-Hartman wrote:
> > > On Thu, Mar 09, 2023 at 05:31:44PM +0800, Ye, Xiang wrote:
> > > > On Thu, Mar 09, 2023 at 08:52:24AM +0100, Greg Kroah-Hartman wrote:
> > > > > On Thu, Mar 09, 2023 at 03:10:56PM +0800, Ye Xiang wrote:
> > > > > > +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));
> > > > > > +}
> > > > >
> > > > > You have sysfs files, yet no Documentation/ABI/ entries? That's not
> > > > > allowed, you know this :(
> > > > The Documentation/ABI/ entries is added for the sysfs on patch 5 of this series.
> > > > https://patchwork.kernel.org/project/linux-usb/patch/[email protected]/
> > >
> > > Ah, missed that, sorry.
> > >
> > > > >
> > > > > > +static ssize_t cmd_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);
> > > > > > + struct ljca_stub *diag_stub = ljca_stub_find(ljca_dev, LJCA_DIAG_STUB);
> > > > > > +
> > > > > > + if (sysfs_streq(buf, "dfu"))
> > > > > > + ljca_mng_set_dfu_mode(mng_stub);
> > > > > > + else if (sysfs_streq(buf, "debug"))
> > > > > > + ljca_diag_set_trace_level(diag_stub, 3);
> > > > >
> > > > > Sorry, but no, you can't do this in a sysfs file.
> > > > Do you mean that we can't use sysfs to send "debug" command to device?
> > >
> > > That is correct, use the kernel-wide debugging facilities that we have
> > > for this NEVER create your own custom interface just for one tiny
> > > driver, that is not allowed.
> > >
> > > > Could you provide some detail or hints?
> > >
> > > dev_dbg().
> But, this command is sent to SET LJCA Firmware logging level.
What command?
This isn't documented at all, sorry, I don't understand what you are
trying to do here.
> > I'm not sure this is the same thing, though, as it's not a drvier
> > to user debug message.
> >
> > Ye, can you please explain better what this command does? You are
> > sending a LJCA_DIAG_SET_TRACE_LEVEL command to the device with a
> > parameter "3" which has a meaining only for you :)
> Sure, the LJCA_DIAG_SET_TRACE_LEVEL command is used to set LJCA FW
> logging level. 3 means debug level for FW. It is used for LJCA FW
> debugging: when FW got some issue, we can send debug level to FW
> to make FW print degging log for analysis.
And where is that printed? In the kernel log? Somewhere else? What
does the firmware have to do with any of this?
thanks,
greg k-h
On 09.03.23 14:52, Andy Shevchenko wrote:
> On Thu, Mar 09, 2023 at 02:40:10PM +0100, Oliver Neukum wrote:
>> On 09.03.23 08:10, Ye Xiang wrote:
>>> + u8 obuf[LJCA_GPIO_BUF_SIZE];
>>> + u8 ibuf[LJCA_GPIO_BUF_SIZE];
>>
>> And here we have a violation of DMA coherency rules.
>> Basically you cannot embed buffers into other data structures
>> if they can be subject to DMA.
>
> Huh?!
>
> The problem here is alignment. But other than that I can't see the issue with
> embedding into structures the instances of which will be allocated on the heap.
Hi,
These constraints are documented in dma-api-howto, but perhaps this
is inconvenient to read through.
Let me explain for the input case. On certain CPUs DMA does not
update CPU caches. Hence when you want to read data generated by DMA
you must read from RAM. Hence you invalidate the cache line with dma_map_*
operations.
Those cache lines must stay invalidated. If you wish to guarantee that,
you cannot access a data structure that shares a cache line with a buffer,
until you are sure that DMA is finished.
On the affected architectures kmalloc() makes sure that no allocation straddles
cache lines.
Regards
Oliver
On 09.03.23 15:48, Arnd Bergmann wrote:
> On Thu, Mar 9, 2023, at 15:18, Linus Walleij wrote:
>> On Thu, Mar 9, 2023 at 2:53 PM Andy Shevchenko
>> <[email protected]> wrote:
>>
>> Yups. And I think the solution looks something like this:
>>
>> u8 obuf[LJCA_GPIO_BUF_SIZE] __aligned(8);
>> u8 ibuf[LJCA_GPIO_BUF_SIZE] __aligned(8);
>>
>> __aligned(4) if it's 32bit DMA I guess? 8 always works that's
>> why we use it all over the IIO subsystem.
>
> I think it must be __aligned(ARCH_DMA_MINALIGN), to account
> for architectures with non-coherent DMA.
In theory, yes, you can do that.
Is that a better solution than using what kmalloc() is
designed to provide. I don't think so.
Regards
Oliver
On Thu, Mar 09, 2023 at 04:58:45PM +0100, Greg Kroah-Hartman wrote:
> On Thu, Mar 09, 2023 at 11:45:51PM +0800, Ye, Xiang wrote:
> > On Thu, Mar 09, 2023 at 11:06:25AM +0100, Andi Shyti wrote:
> > > On Thu, Mar 09, 2023 at 10:41:05AM +0100, Greg Kroah-Hartman wrote:
> > > > On Thu, Mar 09, 2023 at 05:31:44PM +0800, Ye, Xiang wrote:
> > > > > On Thu, Mar 09, 2023 at 08:52:24AM +0100, Greg Kroah-Hartman wrote:
> > > > > > On Thu, Mar 09, 2023 at 03:10:56PM +0800, Ye Xiang wrote:
> > > > > > > +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));
> > > > > > > +}
> > > > > >
> > > > > > You have sysfs files, yet no Documentation/ABI/ entries? That's not
> > > > > > allowed, you know this :(
> > > > > The Documentation/ABI/ entries is added for the sysfs on patch 5 of this series.
> > > > > https://patchwork.kernel.org/project/linux-usb/patch/[email protected]/
> > > >
> > > > Ah, missed that, sorry.
> > > >
> > > > > >
> > > > > > > +static ssize_t cmd_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);
> > > > > > > + struct ljca_stub *diag_stub = ljca_stub_find(ljca_dev, LJCA_DIAG_STUB);
> > > > > > > +
> > > > > > > + if (sysfs_streq(buf, "dfu"))
> > > > > > > + ljca_mng_set_dfu_mode(mng_stub);
> > > > > > > + else if (sysfs_streq(buf, "debug"))
> > > > > > > + ljca_diag_set_trace_level(diag_stub, 3);
> > > > > >
> > > > > > Sorry, but no, you can't do this in a sysfs file.
> > > > > Do you mean that we can't use sysfs to send "debug" command to device?
> > > >
> > > > That is correct, use the kernel-wide debugging facilities that we have
> > > > for this NEVER create your own custom interface just for one tiny
> > > > driver, that is not allowed.
> > > >
> > > > > Could you provide some detail or hints?
> > > >
> > > > dev_dbg().
> > But, this command is sent to SET LJCA Firmware logging level.
>
> What command?
ljca_diag_set_trace_level send LJCA_DIAG_SET_TRACE_LEVEL command to LJCA
FW to set FW log level.
>
> This isn't documented at all, sorry, I don't understand what you are
> trying to do here.
Sorry for the doc missing. will try to add more details on comments and
Documentation/ABI/ entries.
>
> > > I'm not sure this is the same thing, though, as it's not a drvier
> > > to user debug message.
> > >
> > > Ye, can you please explain better what this command does? You are
> > > sending a LJCA_DIAG_SET_TRACE_LEVEL command to the device with a
> > > parameter "3" which has a meaining only for you :)
> > Sure, the LJCA_DIAG_SET_TRACE_LEVEL command is used to set LJCA FW
> > logging level. 3 means debug level for FW. It is used for LJCA FW
> > debugging: when FW got some issue, we can send debug level to FW
> > to make FW print degging log for analysis.
>
> And where is that printed? In the kernel log? Somewhere else? What
> does the firmware have to do with any of this?
The firmware log was printed through UART port of LJCA device to another
computer just for debugging purposes. After sending LJCA_DIAG_SET_TRACE_LEVEL
command to LJCA device, it will print logging according to the log level sent
before.
--
Thanks
Ye Xiang
Hi Oliver,
Thanks for your review.
On Thu, Mar 09, 2023 at 01:53:28PM +0100, Oliver Neukum wrote:
>
>
> On 09.03.23 08:10, Ye Xiang wrote:
>
> > +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)
>
> Why do you make ibuf_len a pointer?
Because ibuf_len is also used as output of this function here.
It stores the actual length of ibuf receive from LJCA device.
>
> > +{
> > + 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) {
>
> Memory leak in error case. You must free header.
Good catch. Thanks. Will address it.
>
> > + 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(dev->udev, usb_sndbulkpipe(dev->udev, 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;
>
> If that triggers, you may have a pending URB.
> You must kill it.
which URB? I guess what you mean is dev->in_urb?
But the in_urb should always be up to waiting for message from firmware,
even through this timeout happen.
>
> > + goto error_unlock;
> > + }
> > + }
> > +
> > + stub->ipacket.ibuf = NULL;
> > + stub->ipacket.ibuf_len = NULL;
> > + ret = 0;
> > +error_unlock:
> > + mutex_unlock(&dev->mutex);
> > +error_put:
> > + usb_autopm_put_interface(dev->intf);
> > + return ret;
> > +}
>
>
> > +static int ljca_i2c_stub_init(struct ljca_dev *dev, struct ljca_i2c_descriptor *desc)
> > +{
> > + struct ljca_i2c_info *i2c_info;
> > + struct ljca_stub *stub;
> > + int ret;
> > + int i;
> > +
> > + stub = ljca_stub_alloc(dev, LJCA_I2C_STUB, size_mul(desc->num, sizeof(*i2c_info)));
> > + if (IS_ERR(stub))
> > + return PTR_ERR(stub);
> > +
> > + i2c_info = ljca_priv(stub);
> > +
> > + for (i = 0; i < desc->num; i++) {
> > + struct mfd_cell cell = {};
> > +
> > + i2c_info[i].ljca = &stub->ljca;
> > + i2c_info[i].id = desc->info[i].id;
> > + i2c_info[i].capacity = desc->info[i].capacity;
> > + i2c_info[i].intr_pin = desc->info[i].intr_pin;
> > +
> > + cell.name = "ljca-i2c";
> > + cell.platform_data = &i2c_info[i];
> > + cell.pdata_size = sizeof(i2c_info[i]);
> > +
> > + if (i < ARRAY_SIZE(ljca_acpi_match_i2cs))
> > + cell.acpi_match = &ljca_acpi_match_i2cs[i];
> > +
> > + ret = ljca_add_mfd_cell(dev, &cell);
> > + if (ret)
> > + return ret;
>
> What happens to stub in the error case?
ljca_add_mfd_cell only failed when krealloc_array failing. When
ljca_add_mfd_cell fails, the related stub just be left alone here.
Maybe I should free the stub here when fails? what is your advice?
>
> > + }
> > +
> > + return 0;
> > +}
>
>
> > +
> > +static void ljca_disconnect(struct usb_interface *intf)
> > +{
> > + struct ljca_dev *dev = usb_get_intfdata(intf);
> > +
> > + ljca_stop(dev);
>
> What prevents restarting the device here?
Why need to restart the device here?
The ljca_disconnect function should be called when the LJCA device is
unplugged.
>
> > + mfd_remove_devices(&intf->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);
>
> So here you report errors, but at the same time you set "started"
> even if errors occur.
Thanks, Got it. Will address it on next version.
--
Thanks
Ye Xiang
On Thu, Mar 09, 2023 at 02:40:10PM +0100, Oliver Neukum wrote:
>
>
> On 09.03.23 08:10, Ye Xiang wrote:
>
> > +#define LJCA_GPIO_BUF_SIZE 60
> > +struct ljca_gpio_dev {
> > + struct platform_device *pdev;
> > + 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);
> > + 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];
>
> And here we have a violation of DMA coherency rules.
> Basically you cannot embed buffers into other data structures
> if they can be subject to DMA.
But obuf and ibuf does not used to do DMA transfer here.
It is actually copied from or to ljca buffer to do URB transfer.
Should it still need to follow the DMA coherency rules?
>
>
>
>
> > +static int ljca_gpio_remove(struct platform_device *pdev)
> > +{
> > + struct ljca_gpio_dev *ljca_gpio = platform_get_drvdata(pdev);
> > +
> > + gpiochip_remove(&ljca_gpio->gc);
> > + ljca_unregister_event_cb(ljca_gpio->gpio_info->ljca);
> > + mutex_destroy(&ljca_gpio->irq_lock);
> > + mutex_destroy(&ljca_gpio->trans_lock);
>
> At this time, what has made sure that no work is scheduled?
Can't make sure of that. Could Adding cancel_work_sync(&ljca_gpio->work) before
mutex_destroy can address it?
>
> > + return 0;
> > +}
>
>
--
Thanks
Ye Xiang
On Fri, Mar 10, 2023 at 01:01:11PM +0800, Ye, Xiang wrote:
> On Thu, Mar 09, 2023 at 02:40:10PM +0100, Oliver Neukum wrote:
> >
> >
> > On 09.03.23 08:10, Ye Xiang wrote:
> >
> > > +#define LJCA_GPIO_BUF_SIZE 60
> > > +struct ljca_gpio_dev {
> > > + struct platform_device *pdev;
> > > + 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);
> > > + 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];
> >
> > And here we have a violation of DMA coherency rules.
> > Basically you cannot embed buffers into other data structures
> > if they can be subject to DMA.
> But obuf and ibuf does not used to do DMA transfer here.
> It is actually copied from or to ljca buffer to do URB transfer.
urb transfers _ARE_ DMA transfers.
> Should it still need to follow the DMA coherency rules?
Yes, all buffers for USB urbs are required to follow those rules.
thanks,
greg k-h
On Fri, Mar 10, 2023 at 08:11:04AM +0100, Greg Kroah-Hartman wrote:
> On Fri, Mar 10, 2023 at 01:01:11PM +0800, Ye, Xiang wrote:
> > On Thu, Mar 09, 2023 at 02:40:10PM +0100, Oliver Neukum wrote:
> > >
> > >
> > > On 09.03.23 08:10, Ye Xiang wrote:
> > >
> > > > +#define LJCA_GPIO_BUF_SIZE 60
> > > > +struct ljca_gpio_dev {
> > > > + struct platform_device *pdev;
> > > > + 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);
> > > > + 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];
> > >
> > > And here we have a violation of DMA coherency rules.
> > > Basically you cannot embed buffers into other data structures
> > > if they can be subject to DMA.
> > But obuf and ibuf does not used to do DMA transfer here.
> > It is actually copied from or to ljca buffer to do URB transfer.
>
> urb transfers _ARE_ DMA transfers.
>
> > Should it still need to follow the DMA coherency rules?
>
> Yes, all buffers for USB urbs are required to follow those rules.
But these two buffers are not used to do USB urb transfer directly.
For the "u8 obuf[LJCA_GPIO_BUF_SIZE]", it will be copied to ljca buffer
("header->data" as below code [1] showed) first. Then the "header" is used
to do the actual urb transfer.
And the "header" is allocated by using kmalloc. It should has met the DMA
coherency rules.
[1] """
struct ljca_msg *header;
...
header = kmalloc(msg_len, GFP_KERNEL);
if (!header)
return -ENOMEM;
...
if (obuf)
memcpy(header->data, obuf, obuf_len);
"""
--
Thanks
Ye Xiang
On Fri, Mar 10, 2023 at 03:39:07PM +0800, Ye, Xiang wrote:
> On Fri, Mar 10, 2023 at 08:11:04AM +0100, Greg Kroah-Hartman wrote:
> > On Fri, Mar 10, 2023 at 01:01:11PM +0800, Ye, Xiang wrote:
> > > On Thu, Mar 09, 2023 at 02:40:10PM +0100, Oliver Neukum wrote:
> > > >
> > > >
> > > > On 09.03.23 08:10, Ye Xiang wrote:
> > > >
> > > > > +#define LJCA_GPIO_BUF_SIZE 60
> > > > > +struct ljca_gpio_dev {
> > > > > + struct platform_device *pdev;
> > > > > + 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);
> > > > > + 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];
> > > >
> > > > And here we have a violation of DMA coherency rules.
> > > > Basically you cannot embed buffers into other data structures
> > > > if they can be subject to DMA.
> > > But obuf and ibuf does not used to do DMA transfer here.
> > > It is actually copied from or to ljca buffer to do URB transfer.
> >
> > urb transfers _ARE_ DMA transfers.
> >
> > > Should it still need to follow the DMA coherency rules?
> >
> > Yes, all buffers for USB urbs are required to follow those rules.
> But these two buffers are not used to do USB urb transfer directly.
> For the "u8 obuf[LJCA_GPIO_BUF_SIZE]", it will be copied to ljca buffer
> ("header->data" as below code [1] showed) first. Then the "header" is used
> to do the actual urb transfer.
>
> And the "header" is allocated by using kmalloc. It should has met the DMA
> coherency rules.
>
> [1] """
> struct ljca_msg *header;
> ...
> header = kmalloc(msg_len, GFP_KERNEL);
> if (!header)
> return -ENOMEM;
Ok, that's good, but why have 2 buffers for this then?
thanks,
greg k-h
On Fri, Mar 10, 2023 at 08:53:10AM +0100, Greg Kroah-Hartman wrote:
> On Fri, Mar 10, 2023 at 03:39:07PM +0800, Ye, Xiang wrote:
> > On Fri, Mar 10, 2023 at 08:11:04AM +0100, Greg Kroah-Hartman wrote:
> > > On Fri, Mar 10, 2023 at 01:01:11PM +0800, Ye, Xiang wrote:
> > > > On Thu, Mar 09, 2023 at 02:40:10PM +0100, Oliver Neukum wrote:
> > > > >
> > > > >
> > > > > On 09.03.23 08:10, Ye Xiang wrote:
> > > > >
> > > > > > +#define LJCA_GPIO_BUF_SIZE 60
> > > > > > +struct ljca_gpio_dev {
> > > > > > + struct platform_device *pdev;
> > > > > > + 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);
> > > > > > + 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];
> > > > >
> > > > > And here we have a violation of DMA coherency rules.
> > > > > Basically you cannot embed buffers into other data structures
> > > > > if they can be subject to DMA.
> > > > But obuf and ibuf does not used to do DMA transfer here.
> > > > It is actually copied from or to ljca buffer to do URB transfer.
> > >
> > > urb transfers _ARE_ DMA transfers.
> > >
> > > > Should it still need to follow the DMA coherency rules?
> > >
> > > Yes, all buffers for USB urbs are required to follow those rules.
> > But these two buffers are not used to do USB urb transfer directly.
> > For the "u8 obuf[LJCA_GPIO_BUF_SIZE]", it will be copied to ljca buffer
> > ("header->data" as below code [1] showed) first. Then the "header" is used
> > to do the actual urb transfer.
> >
> > And the "header" is allocated by using kmalloc. It should has met the DMA
> > coherency rules.
> >
> > [1] """
> > struct ljca_msg *header;
> > ...
> > header = kmalloc(msg_len, GFP_KERNEL);
> > if (!header)
> > return -ENOMEM;
>
> Ok, that's good, but why have 2 buffers for this then?
"u8 obuf[LJCA_GPIO_BUF_SIZE]" is used as a out buffer of LJCA GPIO
package.
"u8 ibuf[LJCA_GPIO_BUF_SIZE]" is used as a in buffer of LJCA GPIO
package.
Both obuf and ibuf here are not used to do URB transfer directly.
This obuf and ibuf somtimes are using at the same time when calling
ljca_transfer. So we need these two buffers.
--
Thanks
Ye Xiang
On 09/03/2023 08:10, Ye Xiang wrote:
> This patch 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 <[email protected]>
> Reviewed-by: Sakari Ailus <[email protected]>
> Reviewed-by: Linus Walleij <[email protected]>
> ---
> drivers/gpio/Kconfig | 12 ++
> drivers/gpio/Makefile | 1 +
> drivers/gpio/gpio-ljca.c | 454 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 467 insertions(+)
> create mode 100644 drivers/gpio/gpio-ljca.c
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 13be729710f2..8be697f9f621 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1253,6 +1253,18 @@ config GPIO_KEMPLD
> This driver can also be built as a module. If so, the module will be
> called gpio-kempld.
>
> +config GPIO_LJCA
> + tristate "INTEL La Jolla Cove Adapter GPIO support"
> + depends on MFD_LJCA
> + select GPIOLIB_IRQCHIP
> + default MFD_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.
> +
> config GPIO_LP3943
> tristate "TI/National Semiconductor LP3943 GPIO expander"
> depends on MFD_LP3943
> 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..87863f0230f5
> --- /dev/null
> +++ b/drivers/gpio/gpio-ljca.c
> @@ -0,0 +1,454 @@
> +// 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/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/mfd/ljca.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/types.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 platform_device *pdev;
> + 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);
> + 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->gpio_info->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->gpio_info->ljca, LJCA_GPIO_READ, packet,
> + struct_size(packet, item, packet->num), ljca_gpio->ibuf, &ibuf_len);
> + if (ret)
> + goto out_unlock;
> +
> + if (!ibuf_len || ack_packet->num != packet->num) {
> + dev_err(&ljca_gpio->pdev->dev, "failed gpio_id:%u %u", gpio_id, ack_packet->num);
> + ret = -EIO;
> + }
> +
> +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->gpio_info->ljca, LJCA_GPIO_WRITE, packet,
> + struct_size(packet, item, packet->num), NULL, NULL);
> + mutex_unlock(&ljca_gpio->trans_lock);
Everywhere you have unusual coding pattern around return. There is
almost always line break before return. Adjust to the kernel style.
> + return ret;
> +}
(...)
> +
> +#define LJCA_GPIO_DRV_NAME "ljca-gpio"
> +static const struct platform_device_id ljca_gpio_id[] = {
> + { LJCA_GPIO_DRV_NAME, 0 },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(platform, ljca_gpio_id);
ljca_gpio_id is unused (except module autoloading). How do you bind your
devices?
> +
> +static struct platform_driver ljca_gpio_driver = {
> + .driver.name = LJCA_GPIO_DRV_NAME,
> + .probe = ljca_gpio_probe,
> + .remove = ljca_gpio_remove,
Best regards,
Krzysztof
Hi Krzysztof,
Thanks for your review.
On Sat, Mar 11, 2023 at 01:13:30PM +0100, Krzysztof Kozlowski wrote:
> On 09/03/2023 08:10, Ye Xiang wrote:
> > This patch 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 <[email protected]>
> > Reviewed-by: Sakari Ailus <[email protected]>
> > Reviewed-by: Linus Walleij <[email protected]>
> > ---
> > drivers/gpio/Kconfig | 12 ++
> > drivers/gpio/Makefile | 1 +
> > drivers/gpio/gpio-ljca.c | 454 +++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 467 insertions(+)
> > create mode 100644 drivers/gpio/gpio-ljca.c
> >
> > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> > index 13be729710f2..8be697f9f621 100644
> > --- a/drivers/gpio/Kconfig
> > +++ b/drivers/gpio/Kconfig
> > @@ -1253,6 +1253,18 @@ config GPIO_KEMPLD
> > This driver can also be built as a module. If so, the module will be
> > called gpio-kempld.
> >
> > +config GPIO_LJCA
> > + tristate "INTEL La Jolla Cove Adapter GPIO support"
> > + depends on MFD_LJCA
> > + select GPIOLIB_IRQCHIP
> > + default MFD_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.
> > +
> > config GPIO_LP3943
> > tristate "TI/National Semiconductor LP3943 GPIO expander"
> > depends on MFD_LP3943
> > 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..87863f0230f5
> > --- /dev/null
> > +++ b/drivers/gpio/gpio-ljca.c
> > @@ -0,0 +1,454 @@
> > +// 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/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/mfd/ljca.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/slab.h>
> > +#include <linux/types.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 platform_device *pdev;
> > + 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);
> > + 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->gpio_info->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->gpio_info->ljca, LJCA_GPIO_READ, packet,
> > + struct_size(packet, item, packet->num), ljca_gpio->ibuf, &ibuf_len);
> > + if (ret)
> > + goto out_unlock;
> > +
> > + if (!ibuf_len || ack_packet->num != packet->num) {
> > + dev_err(&ljca_gpio->pdev->dev, "failed gpio_id:%u %u", gpio_id, ack_packet->num);
> > + ret = -EIO;
> > + }
> > +
> > +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->gpio_info->ljca, LJCA_GPIO_WRITE, packet,
> > + struct_size(packet, item, packet->num), NULL, NULL);
> > + mutex_unlock(&ljca_gpio->trans_lock);
>
> Everywhere you have unusual coding pattern around return. There is
> almost always line break before return. Adjust to the kernel style.
Got it. Will add a blank line before return.
>
> > + return ret;
> > +}
>
> (...)
>
> > +
> > +#define LJCA_GPIO_DRV_NAME "ljca-gpio"
> > +static const struct platform_device_id ljca_gpio_id[] = {
> > + { LJCA_GPIO_DRV_NAME, 0 },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(platform, ljca_gpio_id);
>
> ljca_gpio_id is unused (except module autoloading). How do you bind your
> devices?
driver.name = LJCA_GPIO_DRV_NAME, this driver name is used to bind
with devices.
> > +
> > +static struct platform_driver ljca_gpio_driver = {
> > + .driver.name = LJCA_GPIO_DRV_NAME,
> > + .probe = ljca_gpio_probe,
> > + .remove = ljca_gpio_remove,
>
>
--
Thanks
Ye Xiang
On 10.03.23 05:14, Ye, Xiang wrote:
> Hi Oliver,
Hi,
sorry for the delayed answer.
> Thanks for your review.
> On Thu, Mar 09, 2023 at 01:53:28PM +0100, Oliver Neukum wrote:
>>
>>
>> On 09.03.23 08:10, Ye Xiang wrote:
>>
>>> +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)
>>
>> Why do you make ibuf_len a pointer?
> Because ibuf_len is also used as output of this function here.
> It stores the actual length of ibuf receive from LJCA device.
Yes, I understand that now, thank you for the explanation, yet
that is problematic, if we look at another issue. See further down:
>>> + ret = -ENODEV;
>>> + goto error_put;
>>> + }
>>> +
>>> + mutex_lock(&dev->mutex);
>>> + stub->cur_cmd = cmd;
>>> + stub->ipacket.ibuf = ibuf;
>>> + stub->ipacket.ibuf_len = ibuf_len;
Here you store the pointer into the stub. Hence we must make sure
that the location it points to stays valid.
>>> + stub->acked = false;
>>> + ret = usb_bulk_msg(dev->udev, usb_sndbulkpipe(dev->udev, 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;
The function will return an error in the timeout case.
This may be a trivial observation but it becomes important.
>> If that triggers, you may have a pending URB.
I misspoke. Pending IO would have been correct.
>> You must kill it.
> which URB? I guess what you mean is dev->in_urb?
> But the in_urb should always be up to waiting for message from firmware,
> even through this timeout happen.
Now let's look at ljca_mng_reset_handshake(). I am afraid I have to quote
its first part in full:
+static int ljca_mng_reset_handshake(struct ljca_stub *stub)
+{
+ struct ljca_mng_priv *priv;
+ __le32 reset_id;
+ __le32 reset_id_ret = 0;
+ unsigned int ilen = sizeof(__le32);
This is on the _stack_
Highly important !!!
+ int ret;
+
+ priv = ljca_priv(stub);
+ reset_id = cpu_to_le32(priv->reset_id++);
+ 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 we run into the timeout error case, ret will be -ETIMEDOUT.
+ if (ret)
+ return ret;
And thus here we return and free the stack _including_ "ilen", which we
still have a pointer to. That means if the operation concludes after
a timeout, we _will_ follow a rogue pointer.
A couple of functions have this race condition.
>> What happens to stub in the error case?
> ljca_add_mfd_cell only failed when krealloc_array failing. When
> ljca_add_mfd_cell fails, the related stub just be left alone here.
>
> Maybe I should free the stub here when fails? what is your advice?
Yes, that is the cleanest solution.
>>> + }
>>> +
>>> + return 0;
>>> +}
>>
>>
>>> +
>>> +static void ljca_disconnect(struct usb_interface *intf)
>>> +{
>>> + struct ljca_dev *dev = usb_get_intfdata(intf);
>>> +
>>> + ljca_stop(dev);
>>
>> What prevents restarting the device here?
Sorry, you are calling ljca_start() only in probe(9 and resume()
Your code is correct.
Regards
Oliver
Hi Oliver,
Thanks for the review.
On Mon, Mar 13, 2023 at 02:27:50PM +0100, Oliver Neukum wrote:
> On 10.03.23 05:14, Ye, Xiang wrote:
> > Hi Oliver,
>
> Hi,
>
> sorry for the delayed answer.
No problem.
>
> > Thanks for your review.
> > On Thu, Mar 09, 2023 at 01:53:28PM +0100, Oliver Neukum wrote:
> > >
> > >
> > > On 09.03.23 08:10, Ye Xiang wrote:
> > >
> > > > +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)
> > >
> > > Why do you make ibuf_len a pointer?
> > Because ibuf_len is also used as output of this function here.
> > It stores the actual length of ibuf receive from LJCA device.
>
> Yes, I understand that now, thank you for the explanation, yet
> that is problematic, if we look at another issue. See further down:
>
> > > > + ret = -ENODEV;
> > > > + goto error_put;
> > > > + }
> > > > +
> > > > + mutex_lock(&dev->mutex);
> > > > + stub->cur_cmd = cmd;
> > > > + stub->ipacket.ibuf = ibuf;
> > > > + stub->ipacket.ibuf_len = ibuf_len;
>
> Here you store the pointer into the stub. Hence we must make sure
> that the location it points to stays valid.
>
> Now let's look at ljca_mng_reset_handshake(). I am afraid I have to quote
> its first part in full:
>
> +static int ljca_mng_reset_handshake(struct ljca_stub *stub)
> +{
> + struct ljca_mng_priv *priv;
> + __le32 reset_id;
> + __le32 reset_id_ret = 0;
> + unsigned int ilen = sizeof(__le32);
>
> This is on the _stack_
> Highly important !!!
>
> + int ret;
> +
> + priv = ljca_priv(stub);
> + reset_id = cpu_to_le32(priv->reset_id++);
> + 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 we run into the timeout error case, ret will be -ETIMEDOUT.
>
> + if (ret)
> + return ret;
>
> And thus here we return and free the stack _including_ "ilen", which we
> still have a pointer to. That means if the operation concludes after
> a timeout, we _will_ follow a rogue pointer.
> A couple of functions have this race condition.
Got it. Will fix that on next version.
>
>
Thanks
Ye Xiang