[v4,0/6] i2c-i801 / dell-lis3lv02d: Move instantiation of lis3lv02d i2c_client from i2c-i801 to dell-lis3lv02d

Message

Hans de Goede June 24, 2024, 11:15 a.m. UTC

Hi All,

Here is v4 of my patch series to move the manual instantation of lis3lv02d
i2c_client-s for SMO88xx ACPI device from the generic i2c-i801.c code to
a SMO88xx specific dell-lis3lv02d driver.

Moving the i2c_client instantiation there has the following advantages:

1. This moves the SMO88xx ACPI device quirk handling away from the generic
i2c-i801 module which is loaded on all Intel x86 machines to a module
which will only be loaded when there is an ACPI SMO88xx device.

2. This removes the duplication of the SMO88xx ACPI Hardware ID (HID) table
between the i2c-i801 and dell-smo8800 drivers.

3. This allows extending the quirk handling by adding new code and related
module parameters to the dell-lis3lv02d driver, without needing to modify
the i2c-i801 code.

This series also extends the i2c_client instantiation with support for
probing for the i2c-address of the lis3lv02d chip on devices which
are not yet listed in the DMI table with i2c-addresses for known models.
This probing is only done when requested through a module parameter.

The probing support adds quite a bit of code which shows why it is good
to move the handling out of the generic i2c-i801 code.

Changes in v4:
- Move the i2c_client instantiation to a new dell-lis3lv02d driver instead
  of adding it to the dell-smo8800 driver
- Address a couple of other minor review comments

Changes in v3:
- Use an i2c bus notifier so that the i2c_client will still be instantiated if
  the i801 i2c_adapter shows up later or is re-probed (removed + added again).
  This addresses the main concern / review-comments made during review of v2.
- Add 2 prep patches to the i2c-core / the i2c-i801 driver to allow bus-notifier
  use / to avoid the need to duplicate the PCI-ids of IDF i2c-i801 adapters.
- Switch to standard dmi_system_id matching to check both sys-vendor +
  product-name DMI fields
- Drop the patch to alternatively use the st_accel IIO driver instead of
  drivers/misc/lis3lv02d/lis3lv02d.c

Changes in v2:
- Drop "[PATCH 1/6] platform/x86: dell-smo8800: Only load on Dell laptops"
- Use a pci_device_id table to check for IDF (non main) i2c-i801 SMBusses
- Add a comment documenting the IDF PCI device ids
- Keep using drivers/misc/lis3lv02d/lis3lv02d.c by default
- Rename the module-parameter to use_iio_driver which can be set to
  use the IIO st_accel driver instead
- Add a new patch adding the accelerometer address for the 2 models
  I have tested this on to dell_lis3lv02d_devices[]

Since this touches files under both drivers/i2c and drivers/platform/x86
some subsystem coordination is necessary. I think it would be best to just
merge the entire series through the i2c subsystem since this touches some
core i2c files. As pdx86 subsys co-maintainer I'm fine with doing so.

Regards,

Hans


Hans de Goede (6):
  i2c: core: Setup i2c_adapter runtime-pm before calling device_add()
  i2c: i801: Use a different adapter-name for IDF adapters
  platform/x86: dell-smo8800: Move SMO88xx acpi_device_ids to
    dell-smo8800-ids.h
  platform/x86: dell-smo8800: Move instantiation of lis3lv02d i2c_client
    from i2c-i801 to dell-lis3lv02d
  platform/x86: dell-smo8800: Add a couple more models to
    lis3lv02d_devices[]
  platform/x86: dell-smo8800: Add support for probing for the
    accelerometer i2c address

 drivers/i2c/busses/i2c-i801.c                | 133 +-------
 drivers/i2c/i2c-core-base.c                  |  18 +-
 drivers/platform/x86/dell/Makefile           |   1 +
 drivers/platform/x86/dell/dell-lis3lv02d.c   | 331 +++++++++++++++++++
 drivers/platform/x86/dell/dell-smo8800-ids.h |  26 ++
 drivers/platform/x86/dell/dell-smo8800.c     |  16 +-
 6 files changed, 379 insertions(+), 146 deletions(-)
 create mode 100644 drivers/platform/x86/dell/dell-lis3lv02d.c
 create mode 100644 drivers/platform/x86/dell/dell-smo8800-ids.h

Comments

Pali Rohár June 24, 2024, 6:28 p.m. UTC | #1

On Monday 24 June 2024 13:15:12 Hans de Goede wrote:
> Hans de Goede (6):
>   i2c: core: Setup i2c_adapter runtime-pm before calling device_add()
>   i2c: i801: Use a different adapter-name for IDF adapters
>   platform/x86: dell-smo8800: Move SMO88xx acpi_device_ids to
>     dell-smo8800-ids.h
>   platform/x86: dell-smo8800: Move instantiation of lis3lv02d i2c_client
>     from i2c-i801 to dell-lis3lv02d
>   platform/x86: dell-smo8800: Add a couple more models to
>     lis3lv02d_devices[]
>   platform/x86: dell-smo8800: Add support for probing for the
>     accelerometer i2c address

Patches 1-5 looks good. There are just a few minor things, but you can add
Reviewed-by: Pali Rohár <[e-mail]>

For patch 6 as I mentioned previously I'm strictly against this change
until somebody goes and politely ask Dell about the current situation of
the discovering of accelerometer's i2c address. And if there is no other
option than start discussion if Dell can include this information into
DMI / ACPI / WMI or other part of firmware data which they can send from
BIOS/UEFI to operating system.

>  drivers/i2c/busses/i2c-i801.c                | 133 +-------
>  drivers/i2c/i2c-core-base.c                  |  18 +-
>  drivers/platform/x86/dell/Makefile           |   1 +
>  drivers/platform/x86/dell/dell-lis3lv02d.c   | 331 +++++++++++++++++++
>  drivers/platform/x86/dell/dell-smo8800-ids.h |  26 ++
>  drivers/platform/x86/dell/dell-smo8800.c     |  16 +-
>  6 files changed, 379 insertions(+), 146 deletions(-)
>  create mode 100644 drivers/platform/x86/dell/dell-lis3lv02d.c
>  create mode 100644 drivers/platform/x86/dell/dell-smo8800-ids.h
> 
> -- 
> 2.45.1
>