mbox series

[v8,0/6] Add Intel LJCA device driver

Message ID 20230511175844.185070-1-xiang.ye@intel.com
Headers show
Series Add Intel LJCA device driver | expand

Message

Ye Xiang May 11, 2023, 5:58 p.m. UTC
Add driver for Intel La Jolla Cove Adapter (LJCA) device.
This is a USB-GPIO, USB-I2C and USB-SPI device. We add 4
drivers to support this device: a USB driver, a GPIO chip
driver, a I2C controller driver and a SPI controller driver.

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

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

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

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

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

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

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

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

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

Comments

Mark Brown May 12, 2023, 4:18 a.m. UTC | #1
On Fri, May 12, 2023 at 01:58:44AM +0800, Ye Xiang wrote:

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

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

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

These should be namespaced, especially since they look likely to collide
with other things.  Otherwise this looks fine.
Greg Kroah-Hartman May 13, 2023, 8:50 a.m. UTC | #2
On Fri, May 12, 2023 at 01:58:38AM +0800, Ye Xiang wrote:
> Add driver for Intel La Jolla Cove Adapter (LJCA) device.
> This is a USB-GPIO, USB-I2C and USB-SPI device. We add 4
> drivers to support this device: a USB driver, a GPIO chip
> driver, a I2C controller driver and a SPI controller driver.

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

thanks,

greg k-h
Wu, Wentong June 20, 2023, 6:47 a.m. UTC | #3
Hi Brown,

Thanks for your review

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

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

BR,
Wentong

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

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

struct ljca_spi_xfer_packet {
	u8 indicator;
	s8 len;
	u8 data[];
} __packed;
Wu, Wentong June 20, 2023, 8:10 a.m. UTC | #4
Hi Greg, 

Thanks for your review.

BTW, somehow I will take care this patch set.

Thanks
Wentong

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

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

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

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

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