Hi Hans

On 04/03/2021 13:37, Hans de Goede wrote:

Hi,

On 2/22/21 2:07 PM, Daniel Scally wrote:

quoted

v1 for this series was originally 14-18 of this series:
https://lore.kernel.org/linux-media/20201130133129.1024662-1-djrscally@gmail.com/T/#m91934e12e3d033da2e768e952ea3b4a125ee3e67 (local)

v2 was here:
https://lore.kernel.org/platform-driver-x86/20210118003428.568892-1-djrscally@gmail.com/ (local)

Series level changelog:

	- Dropped the patch moving acpi_lpss_dep() to utils since it's not used
	in acpi_dev_get_dependent_dev() anymore.
	- Replaced it with a patch extending acpi_walk_dep_device_list() to be
	able to apply a given callback against each device in acpi_dep_list
	- Dropped the patch creating acpi_i2c_dev_name() and simply open coded
	that functionality.

This has been tested on a number of devices, but currently **not** on a device
designed for ChromeOS, which we ideally need to do to ensure no regression
caused by replacing the tps68470 MFD driver. Sakari / Tomasz, is there any way
you could help with that? Unfortunately, I don't have a device to test it on
myself.

Original cover letter: 

At the moment in the kernel the ACPI _HID INT3472 is taken by the tps68470
MFD driver, but that driver can only handle some of the cases of that _HID
that we see. There are at least these three possibilities:

1. INT3472 devices that provide GPIOs through the usual framework and run
   power and clocks through an operation region; this is the situation that
   the current module handles and is seen on ChromeOS devices
2. INT3472 devices that provide GPIOs, plus clocks and regulators that are
   meant to be driven through the usual frameworks, usually seen on devices
   designed to run Windows
3. INT3472 devices that don't actually represent a physical tps68470, but
   are being used as a convenient way of grouping a bunch of system GPIO
   lines that are intended to enable power and clocks for sensors which
   are called out as dependent on them. Also seen on devices designed to
   run Windows.

This series introduces a new module which registers:

1. An i2c driver that determines which scenario (#1 or #2) applies to the
   machine and registers platform devices to be bound to GPIO, OpRegion,
   clock and regulator drivers as appropriate.
2. A platform driver that binds to the dummy INT3472 devices described in
   #3

The platform driver for the dummy device registers the GPIO lines that
enable the clocks and regulators to the sensors via those frameworks so
that sensor drivers can consume them in the usual fashion. The existing
GPIO and OpRegion tps68470 drivers will work with the i2c driver that's
registered. Clock and regulator drivers are available but have not so far been
tested, so aren't part of this series.

The existing mfd/tps68470.c driver being thus superseded, it is removed.

Thank you for this patch series. Since there have already been a whole
bunch of review-comments, I've not taken a detailed look at this yet.

No problem, I'm hoping to do a v3 over the weekend anyway.

I do wonder if you have thought about how this series should be merged?
This series is spread over quite a few subsytems and since there are
various interdependencies in the patches it is probably best if it gets
merged in its entirety through a single tree.

I guess that merging though either Rafael's (drivers/acpi) tree or
Lee's (drivers/mfd) tree makes the most sense.

As drivers/platform/x86 maintainer I'm happy with whatever solution
works for the other subsystem maintainers.

I also think it's a good idea to go through a single tree, and my plan
was to raise that probably after the next review round or so, but I
hadn't gotten as far as thinking about whos tree it should be or
anything yet. To be honest I'm not sure what factors dictate which
choice is best in that regard; handling complex git merges is a bit
outside my experience.

Regards,

Hans

quoted

Thanks
Dan

Daniel Scally (6):
  ACPI: scan: Extend acpi_walk_dep_device_list()
  ACPI: scan: Add function to fetch dependent of acpi device
  i2c: core: Add a format macro for I2C device names
  gpiolib: acpi: Export acpi_get_gpiod()
  platform/x86: Add intel_skl_int3472 driver
  mfd: tps68470: Remove tps68470 MFD driver

 MAINTAINERS                                   |   5 +
 drivers/acpi/ec.c                             |   2 +-
 drivers/acpi/pmic/Kconfig                     |   2 +-
 drivers/acpi/pmic/intel_pmic_chtdc_ti.c       |   2 +-
 drivers/acpi/scan.c                           |  92 ++-
 drivers/gpio/Kconfig                          |   2 +-
 drivers/gpio/gpiolib-acpi.c                   |  38 +-
 drivers/i2c/i2c-core-acpi.c                   |   2 +-
 drivers/i2c/i2c-core-base.c                   |   4 +-
 drivers/mfd/Kconfig                           |  18 -
 drivers/mfd/Makefile                          |   1 -
 drivers/mfd/tps68470.c                        |  97 ---
 drivers/platform/surface/surface3_power.c     |   2 +-
 drivers/platform/x86/Kconfig                  |   2 +
 drivers/platform/x86/Makefile                 |   1 +
 drivers/platform/x86/intel-int3472/Kconfig    |  31 +
 drivers/platform/x86/intel-int3472/Makefile   |   4 +
 .../intel-int3472/intel_skl_int3472_common.c  | 106 ++++
 .../intel-int3472/intel_skl_int3472_common.h  | 110 ++++
 .../intel_skl_int3472_discrete.c              | 592 ++++++++++++++++++
 .../intel_skl_int3472_tps68470.c              | 113 ++++
 include/acpi/acpi_bus.h                       |   8 +
 include/linux/acpi.h                          |   4 +-
 include/linux/gpio/consumer.h                 |   7 +
 include/linux/i2c.h                           |   3 +
 25 files changed, 1100 insertions(+), 148 deletions(-)
 delete mode 100644 drivers/mfd/tps68470.c
 create mode 100644 drivers/platform/x86/intel-int3472/Kconfig
 create mode 100644 drivers/platform/x86/intel-int3472/Makefile
 create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_common.c
 create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_common.h
 create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_discrete.c
 create mode 100644 drivers/platform/x86/intel-int3472/intel_skl_int3472_tps68470.c