Re: [PATCH v12 0/4] Add Intel LJCA device driver

Hans de Goede
2023-09-02T14:59:11Z

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

Hi,

On 9/1/23 07:36, Wentong Wu wrote:
> Add driver for Intel La Jolla Cove Adapter (LJCA) device. This is
> a USB-GPIO, USB-I2C and USB-SPI device. We add 4 drivers to support
> this device: a USB driver, a GPIO chip driver, a I2C controller
> driver and a SPI controller driver.

Thanks. I've been testing this on a Lenovo X1 ThinkPad Yoga gen 8
with an ov2740 sensor connected to the LJCA device.

I needed 2 small(ish) fixes to make everything work there.
I have attached the 2 fixes here.

With these 2 fixes this series is:

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

Regards,

Hans

> ---
> v12:
> - switch dev_err to dev_dbg for i2c-ljca driver
> - avoid err printing because of calling usb_kill_urb when
> attempts to resubmit the rx urb
>
> v11:
> - switch dev_err to dev_dbg for i2c-ljca driver
> - remove message length check because of defined quirk structure
> - remove I2C_FUNC_SMBUS_EMUL support
>
> v10:
> - remove ljca_i2c_format_slave_addr
> - remove memset before write write w_packet
> - make ljca_i2c_stop void and print err message in case failure
> - use dev_err_probe in ljca_i2c_probe function
>
> v9:
> - overhaul usb-ljca driver to make it more structured and easy understand
> - fix memory leak issue for usb-ljca driver
> - add spinlock to protect tx_buf and ex_buf
> - change exported APIs for usb-ljca driver
> - unify prefix for structures and functions for i2c-ljca driver
> - unify prefix for structures and functions for spi-ljca driver
> - unify prefix for structures and functions for gpio-ljca driver
> - update gpio-ljca, i2c-ljca and spi-ljca drivers according to usb-ljca's changes
>
> Wentong Wu (4):
> usb: Add support for Intel LJCA device
> i2c: Add support for Intel LJCA USB I2C driver
> spi: Add support for Intel LJCA USB SPI driver
> gpio: update Intel LJCA USB GPIO driver
>
> drivers/gpio/Kconfig | 4 +-
> drivers/gpio/gpio-ljca.c | 246 +++++++------
> drivers/i2c/busses/Kconfig | 11 +
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-ljca.c | 326 +++++++++++++++++
> drivers/spi/Kconfig | 11 +
> drivers/spi/Makefile | 1 +
> drivers/spi/spi-ljca.c | 297 +++++++++++++++
> drivers/usb/misc/Kconfig | 14 +
> drivers/usb/misc/Makefile | 1 +
> drivers/usb/misc/usb-ljca.c | 817 ++++++++++++++++++++++++++++++++++++++++++
> include/linux/usb/ljca.h | 113 ++++++
> 12 files changed, 1737 insertions(+), 105 deletions(-)
> create mode 100644 drivers/i2c/busses/i2c-ljca.c
> create mode 100644 drivers/spi/spi-ljca.c
> create mode 100644 drivers/usb/misc/usb-ljca.c
> create mode 100644 include/linux/usb/ljca.h
>

[-- Attachment #2: 0001-usb-ljca-Fix-picking-the-ACPI-companion-for-the-I2C-.patch --]
[-- Type: text/x-patch, Size: 3285 bytes --]

From c43f73014c652a6616b360edc2be582098960382 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sat, 2 Sep 2023 12:08:16 +0200
Subject: [PATCH 1/4] usb: ljca: Fix picking the ACPI companion for the I2C
controllers

The ljca IO-expander has 2 I2C controllers, which both have
the same API HID, e.g. "INTC1097" on a Lenovo X1 ThinkPad Yoga gen 8.

Taking the first ACPI companion which matches the HID results in both
I2C controllers getting assigned the same ACPI companion.

Add an ACPI UID check to assign the right companion to both
I2C controllers. This fixes no i2c-client getting instantiated for
the ov2740 sensor on a Lenovo X1 ThinkPad Yoga gen 8.

Note some DSDTs have only 1 ACPI companion for the 2 I2C controllers
and these don't set a UID at all. On these models only the first I2C
controller is used. So if a HID match has no UID use "0" for the HID.
assigning the ACPI companion to the first I2C controller.
An example device with this setup is the Dell Latitude 9420.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/usb/misc/usb-ljca.c | 30 ++++++++++++++++++++++--------
1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/misc/usb-ljca.c b/drivers/usb/misc/usb-ljca.c
index 4b0a77725bdd..82b8e1ea6bbb 100644
--- a/drivers/usb/misc/usb-ljca.c
+++ b/drivers/usb/misc/usb-ljca.c
@@ -349,6 +349,7 @@ EXPORT_SYMBOL_NS_GPL(ljca_unregister_event_cb, LJCA);
#ifdef CONFIG_ACPI
struct ljca_match_ids_walk_data {
const struct acpi_device_id *ids;
+ const char *uid;
struct acpi_device *adev;
};

@@ -379,23 +380,34 @@ static const struct acpi_device_id ljca_spi_hids[] = {
static int ljca_match_device_ids(struct acpi_device *adev, void *data)
{
struct ljca_match_ids_walk_data *wd = data;
+ const char *uid = acpi_device_uid(adev);

- if (!acpi_match_device_ids(adev, wd->ids)) {
- wd->adev = adev;
- return 1;
- }
+ if (acpi_match_device_ids(adev, wd->ids))
+ return 0;

- return 0;
+ if (!wd->uid)
+ goto match;
+
+ if (!uid)
+ uid = "0";
+
+ if (strcmp(uid, wd->uid))
+ return 0;
+
+match:
+ wd->adev = adev;
+ return 1;
}

/* bind auxiliary device to acpi device */
static void ljca_auxdev_acpi_bind(struct ljca_adapter *adap,
struct auxiliary_device *auxdev,
- u64 adr)
+ u64 adr, u8 id)
{
struct ljca_match_ids_walk_data wd = { 0 };
struct acpi_device *parent, *adev;
struct device *dev = adap->dev;
+ char uid[4];

parent = ACPI_COMPANION(dev);
if (!parent)
@@ -421,6 +433,8 @@ static void ljca_auxdev_acpi_bind(struct ljca_adapter *adap,
break;
case LJCA_I2C1_ACPI_ADR:
case LJCA_I2C2_ACPI_ADR:
+ snprintf(uid, sizeof(uid), "%d", id);
+ wd.uid = uid;
wd.ids = ljca_i2c_hids;
break;
case LJCA_SPI1_ACPI_ADR:
@@ -449,7 +463,7 @@ static void ljca_auxdev_acpi_bind(struct ljca_adapter *adap,
#else
static void ljca_auxdev_acpi_bind(struct ljca_adapter *adap,
struct auxiliary_device *auxdev,
- u64 adr)
+ u64 adr, u8 id)
{
}
#endif
@@ -489,7 +503,7 @@ static int ljca_new_client_device(struct ljca_adapter *adap, u8 type, u8 id,
if (ret)
goto err_free;

- ljca_auxdev_acpi_bind(adap, auxdev, adr);
+ ljca_auxdev_acpi_bind(adap, auxdev, adr, id);

ret = auxiliary_device_add(auxdev);
if (ret)
--
2.41.0

[-- Attachment #3: 0002-i2c-ljca-Call-acpi_dev_clear_dependencies.patch --]
[-- Type: text/x-patch, Size: 1304 bytes --]

From dfa3bb3d5f403b36b779509b9dcfbf1114eebee2 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Fri, 1 Sep 2023 19:47:48 +0200
Subject: [PATCH 2/4] i2c: ljca: Call acpi_dev_clear_dependencies()

Call acpi_dev_clear_dependencies() to mark _DEP ACPI dependencies on
the I2C controller as satisfied so that acpi_dev_ready_for_enumeration()
for the I2C device nodes in APCI will return true once the I2C controller
is registered.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/i2c/busses/i2c-ljca.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-ljca.c b/drivers/i2c/busses/i2c-ljca.c
index 357234894dea..3aff4de52920 100644
--- a/drivers/i2c/busses/i2c-ljca.c
+++ b/drivers/i2c/busses/i2c-ljca.c
@@ -303,7 +303,14 @@ static int ljca_i2c_probe(struct auxiliary_device *auxdev,
"i2c init failed id: %d\n",
ljca_i2c->i2c_info->id);

- return devm_i2c_add_adapter(&auxdev->dev, &ljca_i2c->adap);
+ ret = devm_i2c_add_adapter(&auxdev->dev, &ljca_i2c->adap);
+ if (ret)
+ return ret;
+
+ if (has_acpi_companion(&ljca_i2c->adap.dev))
+ acpi_dev_clear_dependencies(ACPI_COMPANION(&ljca_i2c->adap.dev));
+
+ return 0;
}

static const struct auxiliary_device_id ljca_i2c_id_table[] = {
--
2.41.0