mbox series

[v19,0/4] Add Intel LJCA device driver

Message ID 1694890416-14409-1-git-send-email-wentong.wu@intel.com (mailing list archive)
Headers show
Series Add Intel LJCA device driver | expand

Message

Wu, Wentong Sept. 16, 2023, 6:53 p.m. UTC
Add driver for Intel La Jolla Cove Adapter (LJCA) device. This
IO-expander adds additional functions to the host system such
as GPIO, I2C and SPI with USB host interface. We add 4 drivers
to support this device: a USB driver, a GPIO chip driver, a I2C
controller driver and a SPI controller driver.

---
v19:
 - add v17's change which v18 doesn't apply

v18:
 - rebase patch set on top of Linus' master branch (57d88e8a5974644039fbc47806bac7bb12025636)

v17:
 - change valid_pins type to __le32 and access valid_pins with get_unaligned_le32
 - remove COMPILE_TEST for USB_LJCA Kconfig

v16:
 - drop all void * and use real types in the exported apis and internal ljca_send()
 - remove #ifdef in usb-ljca.c file
 - add documentation in ljca.h for the public structures
 - add error message in ljca_handle_cmd_ack() if error happens and remove blank line
 - use the functionality in cleanup.h for spinlock to make function much simpler
 - change the type of ex_buf in struct ljca_adapter to u8 *

v15:
 - enhance disconnect() of usb-ljca driver
 - change memchr to strchr in ljca_match_device_ids() of usb-ljca driver

v14:
 - fix build error: implicit declaration of function 'acpi_dev_clear_dependencies'

v13:
 - make ljca-usb more robust with the help of Hans de Goede
 - call acpi_dev_clear_dependencies() to mark _DEP ACPI dependencies on the I2C controller as satisfied, and patch is from Hans de Goede

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 | 342 +++++++++++++++++
 drivers/spi/Kconfig           |  11 +
 drivers/spi/Makefile          |   1 +
 drivers/spi/spi-ljca.c        | 297 +++++++++++++++
 drivers/usb/misc/Kconfig      |  13 +
 drivers/usb/misc/Makefile     |   1 +
 drivers/usb/misc/usb-ljca.c   | 837 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/ljca.h      | 145 ++++++++
 12 files changed, 1804 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

Comments

Marc Zyngier Sept. 17, 2023, 10:37 a.m. UTC | #1
On 2023-09-16 19:53, Wentong Wu wrote:
> Add driver for Intel La Jolla Cove Adapter (LJCA) device. This
> IO-expander adds additional functions to the host system such
> as GPIO, I2C and SPI with USB host interface. We add 4 drivers
> to support this device: a USB driver, a GPIO chip driver, a I2C
> controller driver and a SPI controller driver.

Can I be a pain and ask you to limit the rate at which you
repost this series? You send it every other day, hence
actively discouraging people from reviewing it (why would
they, there's another version coming).

Once a week is a perfectly good rate, and would probably
lead to better results.

Alternatively, if you decide that you really want to keep
sending it that often, please drop me from the Cc list.

Thanks,

         M.
Greg KH Sept. 17, 2023, 10:42 a.m. UTC | #2
On Sun, Sep 17, 2023 at 02:53:32AM +0800, Wentong Wu wrote:
> Add driver for Intel La Jolla Cove Adapter (LJCA) device. This
> IO-expander adds additional functions to the host system such
> as GPIO, I2C and SPI with USB host interface. We add 4 drivers
> to support this device: a USB driver, a GPIO chip driver, a I2C
> controller driver and a SPI controller driver.
> 
> ---
> v19:
>  - add v17's change which v18 doesn't apply

I don't understand this changelog line at all, what do you mean?

> v18:
>  - rebase patch set on top of Linus' master branch (57d88e8a5974644039fbc47806bac7bb12025636)

Why?  What required this?

As Marc says, you are sending this way too often.  I'm going to move
this to the bottom of my pile and get to it in a week or so as with the
constant resends, there are way more changes that were sent before yours
that need to be reviewed.

thanks,

greg k-h
Hans de Goede Sept. 17, 2023, 11:26 a.m. UTC | #3
Hi Marc, Greg,

On 9/17/23 12:42, Greg KH wrote:
> On Sun, Sep 17, 2023 at 02:53:32AM +0800, Wentong Wu wrote:
>> Add driver for Intel La Jolla Cove Adapter (LJCA) device. This
>> IO-expander adds additional functions to the host system such
>> as GPIO, I2C and SPI with USB host interface. We add 4 drivers
>> to support this device: a USB driver, a GPIO chip driver, a I2C
>> controller driver and a SPI controller driver.
>>
>> ---
>> v19:
>>  - add v17's change which v18 doesn't apply
> 
> I don't understand this changelog line at all, what do you mean?
> 
>> v18:
>>  - rebase patch set on top of Linus' master branch (57d88e8a5974644039fbc47806bac7bb12025636)
> 
> Why?  What required this?

I noticed that this did not apply cleanly (the SPI patch did not apply
cleanly), so I asked for the next version to be based on 6.6-rc1 or
Linus' master branch.

Note I did not ask for a new version to be send right away, but
I'm afraid there has been a bit of miscommunication and instead
of rebasing the next version based on further review Wentong has
send out a new rebased version immediately, sorry about that.

Regards,

Hans
Wu, Wentong Sept. 18, 2023, 3:08 a.m. UTC | #4
Hi Greg and Marc,

It's my bad. I'm so sorry for this, and apologize for it here.

And I thought you have lots of patches to review, so I just try to address all
the issues currently I know before you start to review, so that my patch
doesn't take over your bandwidth much.

And now I understand your pain here, I will follow your advise going forward. Thanks

Also thank @Hans de Goede for the help. Thanks

BR,
Wentong

> From: Greg KH
> 
> On Sun, Sep 17, 2023 at 02:53:32AM +0800, Wentong Wu wrote:
> > Add driver for Intel La Jolla Cove Adapter (LJCA) device. This
> > IO-expander adds additional functions to the host system such as GPIO,
> > I2C and SPI with USB host interface. We add 4 drivers to support this
> > device: a USB driver, a GPIO chip driver, a I2C controller driver and
> > a SPI controller driver.
> >
> > ---
> > v19:
> >  - add v17's change which v18 doesn't apply
> 
> I don't understand this changelog line at all, what do you mean?

In v18, I forgot to include v17's change, so resend with v19 immediately, sorry
for this again.

> 
> > v18:
> >  - rebase patch set on top of Linus' master branch
> > (57d88e8a5974644039fbc47806bac7bb12025636)
> 
> Why?  What required this?

This is just a rebase for you and other viewers review and apply the patch easily.
Sorry again.

BR,
Wentong
> 
> As Marc says, you are sending this way too often.  I'm going to move this to the
> bottom of my pile and get to it in a week or so as with the constant resends,
> there are way more changes that were sent before yours that need to be
> reviewed.
> 
> thanks,
> 
> greg k-h
Oliver Neukum Sept. 28, 2023, 10:18 a.m. UTC | #5
On 17.09.23 13:26, Hans de Goede wrote:
  
> Note I did not ask for a new version to be send right away, but
> I'm afraid there has been a bit of miscommunication and instead
> of rebasing the next version based on further review Wentong has
> send out a new rebased version immediately, sorry about that.

Hi,

what to do now? It's been ten days.
I am sure this driver has been very thoroughly reviewed by now.
We are dragging this out. Do we want the developer to do another release
or do we ask Greg to take it as is?
This is becoming almost comical, but that is not what we want driver
submission to be.

As far as I am concerned on the USB side everything is fine now.
Hans? Greg?

	Regards
		Oliver
Hans de Goede Sept. 28, 2023, 12:20 p.m. UTC | #6
Hi,

On 9/28/23 12:18, Oliver Neukum wrote:
> On 17.09.23 13:26, Hans de Goede wrote:
>  
>> Note I did not ask for a new version to be send right away, but
>> I'm afraid there has been a bit of miscommunication and instead
>> of rebasing the next version based on further review Wentong has
>> send out a new rebased version immediately, sorry about that.
> 
> Hi,
> 
> what to do now? It's been ten days.
> I am sure this driver has been very thoroughly reviewed by now.
> We are dragging this out. Do we want the developer to do another release
> or do we ask Greg to take it as is?
> This is becoming almost comical, but that is not what we want driver
> submission to be.
> 
> As far as I am concerned on the USB side everything is fine now.
> Hans? Greg?

Note I have been mostly involved in testing these patches I have
*not* thoroughly reviewed them. I have taken a quick(ish) look
which did not find anything obviously wrong.

I agree that at least patch 1/4 is ready for merging. I'm
not sure if Greg should pick-up the entire series or if
the rest should be merged through there relevant subsystems
to also give the relevant subsys maintainer tree.

For the series:

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

Regards,

Hans
Greg KH Sept. 28, 2023, 12:24 p.m. UTC | #7
On Thu, Sep 28, 2023 at 02:20:04PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 9/28/23 12:18, Oliver Neukum wrote:
> > On 17.09.23 13:26, Hans de Goede wrote:
> >  
> >> Note I did not ask for a new version to be send right away, but
> >> I'm afraid there has been a bit of miscommunication and instead
> >> of rebasing the next version based on further review Wentong has
> >> send out a new rebased version immediately, sorry about that.
> > 
> > Hi,
> > 
> > what to do now? It's been ten days.
> > I am sure this driver has been very thoroughly reviewed by now.
> > We are dragging this out. Do we want the developer to do another release
> > or do we ask Greg to take it as is?
> > This is becoming almost comical, but that is not what we want driver
> > submission to be.
> > 
> > As far as I am concerned on the USB side everything is fine now.
> > Hans? Greg?
> 
> Note I have been mostly involved in testing these patches I have
> *not* thoroughly reviewed them. I have taken a quick(ish) look
> which did not find anything obviously wrong.
> 
> I agree that at least patch 1/4 is ready for merging. I'm
> not sure if Greg should pick-up the entire series or if
> the rest should be merged through there relevant subsystems
> to also give the relevant subsys maintainer tree.
> 
> For the series:
> 
> Acked-by: Hans de Goede <hdegoede@redhat.com>
> Tested-by: Hans de Goede <hdegoede@redhat.com>

Ok, I'll take a look at these again next week when I return home and
catch up on my pending patch review queue.

thanks for the review!

greg k-h
Andi Shyti Sept. 28, 2023, 12:28 p.m. UTC | #8
Hi,

On Thu, Sep 28, 2023 at 12:18:50PM +0200, Oliver Neukum wrote:
> On 17.09.23 13:26, Hans de Goede wrote:
> > Note I did not ask for a new version to be send right away, but
> > I'm afraid there has been a bit of miscommunication and instead
> > of rebasing the next version based on further review Wentong has
> > send out a new rebased version immediately, sorry about that.
> 
> Hi,
> 
> what to do now? It's been ten days.
> I am sure this driver has been very thoroughly reviewed by now.
> We are dragging this out. Do we want the developer to do another release
> or do we ask Greg to take it as is?
> This is becoming almost comical, but that is not what we want driver
> submission to be.
> 
> As far as I am concerned on the USB side everything is fine now.
> Hans? Greg?

i2c is also good to go and the rest looks good, as well. I have
some concerns on patch 4 that looks like a mixture of many random
things.

Andi
Mark Brown Sept. 28, 2023, 12:43 p.m. UTC | #9
On Thu, Sep 28, 2023 at 02:20:04PM +0200, Hans de Goede wrote:

> I agree that at least patch 1/4 is ready for merging. I'm
> not sure if Greg should pick-up the entire series or if
> the rest should be merged through there relevant subsystems
> to also give the relevant subsys maintainer tree.

I stopped paying attention when it was obvious that the USB stuff was
struggling badly and getting lots of resends, I've not checked the SPI 
stuff in a while.
Bartosz Golaszewski Sept. 28, 2023, 1:56 p.m. UTC | #10
On Thu, Sep 28, 2023 at 2:29 PM Andi Shyti <andi.shyti@linux.intel.com> wrote:
>
> Hi,
>
> On Thu, Sep 28, 2023 at 12:18:50PM +0200, Oliver Neukum wrote:
> > On 17.09.23 13:26, Hans de Goede wrote:
> > > Note I did not ask for a new version to be send right away, but
> > > I'm afraid there has been a bit of miscommunication and instead
> > > of rebasing the next version based on further review Wentong has
> > > send out a new rebased version immediately, sorry about that.
> >
> > Hi,
> >
> > what to do now? It's been ten days.
> > I am sure this driver has been very thoroughly reviewed by now.
> > We are dragging this out. Do we want the developer to do another release
> > or do we ask Greg to take it as is?
> > This is becoming almost comical, but that is not what we want driver
> > submission to be.
> >
> > As far as I am concerned on the USB side everything is fine now.
> > Hans? Greg?
>
> i2c is also good to go and the rest looks good, as well. I have
> some concerns on patch 4 that looks like a mixture of many random
> things.
>
> Andi

It's got a lot of coding style fixes ninja-packed in there that are
not mentioned by the commit message. But as it's been reviewed by
Linus, acked by Andy (and myself) and tested by Hans, I'm ready to let
it slide if that saves me from seeing ten additional versions of this
series in my inbox.

Bart