| Message ID | 1693546577-17824-1-git-send-email-wentong.wu@intel.com |
|---|---|
| Headers | show |
| Series | Add Intel LJCA device driver | expand |
Hi Wentong, On Fri, Sep 01, 2023 at 01:36:16PM +0800, Wentong Wu wrote: > Implements the SPI function of Intel USB-I2C/GPIO/SPI adapter device > named "La Jolla Cove Adapter" (LJCA). It communicate with LJCA SPI > module with specific protocol through interfaces exported by LJCA USB > driver. > > Signed-off-by: Wentong Wu <wentong.wu@intel.com> > Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> Thanks, Andi
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 >
Hi Hans, Thanks > From: Hans de Goede <hdegoede@redhat.com> > > 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. Thanks, and I don’t have this laptop, could you please share your DSDT so that I can understand it more? And I will update this patch set after understand your DSDT and have the patches tested on my setup. > > 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> Thanks a lot BR, Wentong > > 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 > >
Hi Andi, Thanks for your review > From: Andi Shyti <andi.shyti@linux.intel.com> > > Hi Wentong, > > On Fri, Sep 01, 2023 at 01:36:16PM +0800, Wentong Wu wrote: > > Implements the SPI function of Intel USB-I2C/GPIO/SPI adapter device > > named "La Jolla Cove Adapter" (LJCA). It communicate with LJCA SPI > > module with specific protocol through interfaces exported by LJCA USB > > driver. > > > > Signed-off-by: Wentong Wu <wentong.wu@intel.com> > > Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> Thanks BR, Wentong > > Thanks, > Andi
Hi Andi, Thanks for your review > From: Andi Shyti <andi.shyti@linux.intel.com> > > Hi Wentong, > > On Fri, Sep 01, 2023 at 01:36:14PM +0800, Wentong Wu wrote: > > Implements the USB part of Intel USB-I2C/GPIO/SPI adapter device named > > "La Jolla Cove Adapter" (LJCA). > > > > The communication between the various LJCA module drivers and the > > hardware will be muxed/demuxed by this driver. Three modules ( I2C, > > GPIO, and SPI) are supported currently. > > > > Each sub-module of LJCA device is identified by type field within the > > LJCA message header. > > > > The sub-modules of LJCA can use ljca_transfer() to issue a transfer > > between host and hardware. And ljca_register_event_cb is exported to > > LJCA sub-module drivers for hardware event subscription. > > > > The minimum code in ASL that covers this board is Scope > > (\_SB.PCI0.DWC3.RHUB.HS01) > > { > > Device (GPIO) > > { > > Name (_ADR, Zero) > > Name (_STA, 0x0F) > > } > > > > Device (I2C) > > { > > Name (_ADR, One) > > Name (_STA, 0x0F) > > } > > > > Device (SPI) > > { > > Name (_ADR, 0x02) > > Name (_STA, 0x0F) > > } > > } > > > > Signed-off-by: Wentong Wu <wentong.wu@intel.com> > > Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > looks good: > > Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> Thanks BR, Wentong > > Andi
Hi, On 9/2/23 19:15, Wu, Wentong wrote: > Hi Hans, > > Thanks > >> From: Hans de Goede <hdegoede@redhat.com> >> >> 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. > > Thanks, and I don’t have this laptop, could you please share your DSDT > so that I can understand it more? I have send you an acpidump by private email. > And I will update this patch set after understand your DSDT and have the > patches tested on my setup. Ok. Note that the out of tree ivsc driver already does uid matching in the i2c-driver to fixup the wrong ACPI companion being assigned by the MFD driver, see: https://github.com/intel/ivsc-driver/blob/main/drivers/i2c/busses/i2c-ljca.c#L346 For upstream it seemed cleaner to me to directly pick the correct ACPI companion at aux-device instantiation, something which was not possible with the MFD approach but is possible with the auxilary bus approach. And the other change which I'm proposing has already been merged into the out of tree version of the LJCA i2c driver: https://github.com/intel/ivsc-driver/commit/70d95269169cf9e452580b0c15471829df4a6d59 Regards, Hans >> 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> > > Thanks a lot > > BR, > Wentong >> >> 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 >>>
Hi Hans, Thanks > From: Hans de Goede <hdegoede@redhat.com> > > Hi, > > On 9/2/23 19:15, Wu, Wentong wrote: > > Hi Hans, > > > > Thanks > > > >> From: Hans de Goede <hdegoede@redhat.com> > >> > >> 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. > > > > Thanks, and I don’t have this laptop, could you please share your DSDT > > so that I can understand it more? > > I have send you an acpidump by private email. > > > And I will update this patch set after understand your DSDT and have > > the patches tested on my setup. > > Ok. Note that the out of tree ivsc driver already does uid matching in the i2c- > driver to fixup the wrong ACPI companion being assigned by the MFD driver, see: > > https://github.com/intel/ivsc-driver/blob/main/drivers/i2c/busses/i2c- > ljca.c#L346 Thanks, there is a little bit change based on your patch, because since RPL the UID becomes "Name (_UID, "VIC0")" style. Could you please revisit below code? static int ljca_match_device_ids(struct acpi_device *adev, void *data) { struct ljca_match_ids_walk_data *wd = data; const char *uid = acpi_device_uid(adev); if (acpi_match_device_ids(adev, wd->ids)) return 0; if (!wd->uid) goto match; if (!uid) uid = "0"; else uid = memchr(uid, wd->uid[0], strlen(uid)); if (!uid || strcmp(uid, wd->uid)) return 0; match: wd->adev = adev; return 1; } Thanks Wentong > > For upstream it seemed cleaner to me to directly pick the correct ACPI Thanks > companion at aux-device instantiation, something which was not possible with > the MFD approach but is possible with the auxilary bus approach. > > And the other change which I'm proposing has already been merged into the out > of tree version of the LJCA i2c driver: > > https://github.com/intel/ivsc- > driver/commit/70d95269169cf9e452580b0c15471829df4a6d59 > > Regards, > > Hans > > > > > > > >> 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> > > > > Thanks a lot > > > > BR, > > Wentong > >> > >> 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 > >>>