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

Hi All,

Here is v9 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.

The i2c-core and i2c-i801 dependencies have both been merged into 6.13-rc1
so I believe that this series is ready to be merged now .

Patch 2/4 does still touch the i2c-i801 code removing the quirk code which
is moved to the pdx86 dell-smo8800 code. I think it would be best if Ilpo
prepares an immutable branch with this series to be merged into both
pdx86/for-next and the i2c-subsystem. Andi can we get your Ack for merging
the i2c-i801 changes through the pdx86 tree ?

Moving the i2c_client instantiation 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.

Changes in v9:
- Rebase on top of v6.13-rc1
- Drop already merged i2c-core and i2c-i801 dependencies

Changes in v8:
- Address some minor review remarks from Andy

Changes in v7:
- Rebase on v6.11-rc1

Changes in v6:
- Use i2c_new_scanned_device() instead of re-inventing it

Changes in v5:
- Make match_acpi_device_ids() and match_acpi_device_ids[] __init[const]
- Add "Depends on I2C" to Kconfig (to fix kernel-test-robot reported issues)
- Add "this may be dangerous warning" to MODULE_PARM_DESC(probe_i2c_addr)

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 (4):
  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                | 124 ---------
 drivers/platform/x86/dell/Kconfig            |   1 +
 drivers/platform/x86/dell/Makefile           |   1 +
 drivers/platform/x86/dell/dell-lis3lv02d.c   | 252 +++++++++++++++++++
 drivers/platform/x86/dell/dell-smo8800-ids.h |  27 ++
 drivers/platform/x86/dell/dell-smo8800.c     |  16 +-
 6 files changed, 282 insertions(+), 139 deletions(-)
 create mode 100644 drivers/platform/x86/dell/dell-lis3lv02d.c
 create mode 100644 drivers/platform/x86/dell/dell-smo8800-ids.h

All patches except the last one are reviewed and should be ready, that
it fine. The last patch is basically no-go, as it was discussed many
times, it should not be accepted at all. As Hans said that would not
respond to my emails, I'm also not going to discuss more with him.

On Monday 09 December 2024 19:35:53 Hans de Goede wrote:
> Hi All,
> 
> Here is v9 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.
> 
> The i2c-core and i2c-i801 dependencies have both been merged into 6.13-rc1
> so I believe that this series is ready to be merged now .
> 
> Patch 2/4 does still touch the i2c-i801 code removing the quirk code which
> is moved to the pdx86 dell-smo8800 code. I think it would be best if Ilpo
> prepares an immutable branch with this series to be merged into both
> pdx86/for-next and the i2c-subsystem. Andi can we get your Ack for merging
> the i2c-i801 changes through the pdx86 tree ?
> 
> Moving the i2c_client instantiation 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.
> 
> Changes in v9:
> - Rebase on top of v6.13-rc1
> - Drop already merged i2c-core and i2c-i801 dependencies
> 
> Changes in v8:
> - Address some minor review remarks from Andy
> 
> Changes in v7:
> - Rebase on v6.11-rc1
> 
> Changes in v6:
> - Use i2c_new_scanned_device() instead of re-inventing it
> 
> Changes in v5:
> - Make match_acpi_device_ids() and match_acpi_device_ids[] __init[const]
> - Add "Depends on I2C" to Kconfig (to fix kernel-test-robot reported issues)
> - Add "this may be dangerous warning" to MODULE_PARM_DESC(probe_i2c_addr)
> 
> 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 (4):
>   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                | 124 ---------
>  drivers/platform/x86/dell/Kconfig            |   1 +
>  drivers/platform/x86/dell/Makefile           |   1 +
>  drivers/platform/x86/dell/dell-lis3lv02d.c   | 252 +++++++++++++++++++
>  drivers/platform/x86/dell/dell-smo8800-ids.h |  27 ++
>  drivers/platform/x86/dell/dell-smo8800.c     |  16 +-
>  6 files changed, 282 insertions(+), 139 deletions(-)
>  create mode 100644 drivers/platform/x86/dell/dell-lis3lv02d.c
>  create mode 100644 drivers/platform/x86/dell/dell-smo8800-ids.h
> 
> -- 
> 2.47.1
>

On Mon, 9 Dec 2024, Hans de Goede wrote:

> Hi All,
> 
> Here is v9 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.
> 
> The i2c-core and i2c-i801 dependencies have both been merged into 6.13-rc1
> so I believe that this series is ready to be merged now .
> 
> Patch 2/4 does still touch the i2c-i801 code removing the quirk code which
> is moved to the pdx86 dell-smo8800 code. I think it would be best if Ilpo
> prepares an immutable branch with this series to be merged into both
> pdx86/for-next and the i2c-subsystem. Andi can we get your Ack for merging
> the i2c-i801 changes through the pdx86 tree ?
> 
> Moving the i2c_client instantiation 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.
> 
> Changes in v9:
> - Rebase on top of v6.13-rc1
> - Drop already merged i2c-core and i2c-i801 dependencies
> 
> Changes in v8:
> - Address some minor review remarks from Andy
> 
> Changes in v7:
> - Rebase on v6.11-rc1
> 
> Changes in v6:
> - Use i2c_new_scanned_device() instead of re-inventing it
> 
> Changes in v5:
> - Make match_acpi_device_ids() and match_acpi_device_ids[] __init[const]
> - Add "Depends on I2C" to Kconfig (to fix kernel-test-robot reported issues)
> - Add "this may be dangerous warning" to MODULE_PARM_DESC(probe_i2c_addr)
> 
> 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 (4):
>   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                | 124 ---------
>  drivers/platform/x86/dell/Kconfig            |   1 +
>  drivers/platform/x86/dell/Makefile           |   1 +
>  drivers/platform/x86/dell/dell-lis3lv02d.c   | 252 +++++++++++++++++++
>  drivers/platform/x86/dell/dell-smo8800-ids.h |  27 ++
>  drivers/platform/x86/dell/dell-smo8800.c     |  16 +-
>  6 files changed, 282 insertions(+), 139 deletions(-)
>  create mode 100644 drivers/platform/x86/dell/dell-lis3lv02d.c
>  create mode 100644 drivers/platform/x86/dell/dell-smo8800-ids.h

Patches 1-3 applied into review-ilpo-next branch.