Re: [PATCH v3 05/10] platform/x86: i2c-multi-instantiate: Move it to drivers/acpi folder

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 1/19/22 18:44, Rafael J. Wysocki wrote:
> On Wed, Jan 19, 2022 at 6:33 PM Lucas tanure
> <tanureal@opensource.cirrus.com> wrote:
>>
>> On 1/19/22 16:53, Rafael J. Wysocki wrote:
>>> On Tue, Jan 18, 2022 at 3:53 PM Stefan Binding
>>> <sbinding@opensource.cirrus.com> wrote:
>>>>
>>>> From: Lucas Tanure <tanureal@opensource.cirrus.com>
>>>>
>>>> Moving I2C multi instantiate driver to drivers/acpi folder for
>>>> upcoming conversion into a generic bus multi instantiate
>>>> driver for SPI and I2C
>>>>
>>>> Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com>
>>>> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
>>>
>>> Why are you moving it away from platform/x86?
>>>
>>> Adding SPI to the mix doesn't seem to be a sufficient reason.
>>>
>>> If this were going to be needed on non-x86, that would be a good
>>> reason for moving it, but is that actually the case?  If so, why isn't
>>> that mentioned in the changelog above?
>>>
>>
>> It was a request made by Andy Shevchenko:
>> https://lkml.org/lkml/2021/12/3/347
> 
> But he hasn't given any reasons why that'd be better.
> 
>> There is no plan to use our CS35L41 HDA with non-x86 platforms and we
>> can't comment about i2c-multi-instantiate use.
>> For us it can stay in x86 folder until an actual request.
> 
> I'd prefer that if Hans agrees.

Ack, keeping this in drivers/platform/x86 is fine with me.

I'll try to make some time to review this new version next week.

Looking at the subjects of the patches I see that this now
refactors the SPI code to re-use the existing SPI ACPI resource
parsing there, thank you for doing that!

Regards,

Hans





> 
>>>> ---
>>>>   MAINTAINERS                                           |  2 +-
>>>>   drivers/acpi/Kconfig                                  | 11 +++++++++++
>>>>   drivers/acpi/Makefile                                 |  1 +
>>>>   .../{platform/x86 => acpi}/i2c-multi-instantiate.c    |  0
>>>>   drivers/acpi/scan.c                                   |  2 +-
>>>>   drivers/platform/x86/Kconfig                          | 11 -----------
>>>>   drivers/platform/x86/Makefile                         |  1 -
>>>>   7 files changed, 14 insertions(+), 14 deletions(-)
>>>>   rename drivers/{platform/x86 => acpi}/i2c-multi-instantiate.c (100%)
>>>>
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index 4e828542b089..546f9e149d28 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -392,7 +392,7 @@ ACPI I2C MULTI INSTANTIATE DRIVER
>>>>   M:     Hans de Goede <hdegoede@redhat.com>
>>>>   L:     platform-driver-x86@vger.kernel.org
>>>>   S:     Maintained
>>>> -F:     drivers/platform/x86/i2c-multi-instantiate.c
>>>> +F:     drivers/acpi/i2c-multi-instantiate.c
>>>>
>>>>   ACPI PCC(Platform Communication Channel) MAILBOX DRIVER
>>>>   M:     Sudeep Holla <sudeep.holla@arm.com>
>>>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>>>> index ba45541b1f1f..2fd78366af6f 100644
>>>> --- a/drivers/acpi/Kconfig
>>>> +++ b/drivers/acpi/Kconfig
>>>> @@ -295,6 +295,17 @@ config ACPI_PROCESSOR
>>>>            To compile this driver as a module, choose M here:
>>>>            the module will be called processor.
>>>>
>>>> +config ACPI_I2C_MULTI_INST
>>>> +       tristate "I2C multi instantiate pseudo device driver"
>>>> +       depends on I2C
>>>> +       help
>>>> +         Some ACPI-based systems list multiple i2c-devices in a single ACPI
>>>> +         firmware-node. This driver will instantiate separate i2c-clients
>>>> +         for each device in the firmware-node.
>>>> +
>>>> +         To compile this driver as a module, choose M here: the module
>>>> +         will be called i2c-multi-instantiate.
>>>> +
>>>>   config ACPI_IPMI
>>>>          tristate "IPMI"
>>>>          depends on IPMI_HANDLER
>>>> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
>>>> index bb757148e7ba..d4db7fb0baf0 100644
>>>> --- a/drivers/acpi/Makefile
>>>> +++ b/drivers/acpi/Makefile
>>>> @@ -104,6 +104,7 @@ obj-$(CONFIG_ACPI_SPCR_TABLE)       += spcr.o
>>>>   obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
>>>>   obj-$(CONFIG_ACPI_PPTT)        += pptt.o
>>>>   obj-$(CONFIG_ACPI_PFRUT)       += pfr_update.o pfr_telemetry.o
>>>> +obj-$(CONFIG_ACPI_I2C_MULTI_INST)      += i2c-multi-instantiate.o
>>>>
>>>>   # processor has its own "processor." module_param namespace
>>>>   processor-y                    := processor_driver.o
>>>> diff --git a/drivers/platform/x86/i2c-multi-instantiate.c b/drivers/acpi/i2c-multi-instantiate.c
>>>> similarity index 100%
>>>> rename from drivers/platform/x86/i2c-multi-instantiate.c
>>>> rename to drivers/acpi/i2c-multi-instantiate.c
>>>> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
>>>> index 1331756d4cfc..3e85a02f6ba2 100644
>>>> --- a/drivers/acpi/scan.c
>>>> +++ b/drivers/acpi/scan.c
>>>> @@ -1738,7 +1738,7 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device)
>>>>           * must be instantiated for each, each with its own i2c_device_id.
>>>>           * Normally we only instantiate an i2c-client for the first resource,
>>>>           * using the ACPI HID as id. These special cases are handled by the
>>>> -        * drivers/platform/x86/i2c-multi-instantiate.c driver, which knows
>>>> +        * drivers/acpi/i2c-multi-instantiate.c driver, which knows
>>>>           * which i2c_device_id to use for each resource.
>>>>           */
>>>>                  {"BSG1160", },
>>>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>>>> index 24deeeb29af2..37c1c150508d 100644
>>>> --- a/drivers/platform/x86/Kconfig
>>>> +++ b/drivers/platform/x86/Kconfig
>>>> @@ -990,17 +990,6 @@ config TOPSTAR_LAPTOP
>>>>
>>>>            If you have a Topstar laptop, say Y or M here.
>>>>
>>>> -config I2C_MULTI_INSTANTIATE
>>>> -       tristate "I2C multi instantiate pseudo device driver"
>>>> -       depends on I2C && ACPI
>>>> -       help
>>>> -         Some ACPI-based systems list multiple i2c-devices in a single ACPI
>>>> -         firmware-node. This driver will instantiate separate i2c-clients
>>>> -         for each device in the firmware-node.
>>>> -
>>>> -         To compile this driver as a module, choose M here: the module
>>>> -         will be called i2c-multi-instantiate.
>>>> -
>>>>   config MLX_PLATFORM
>>>>          tristate "Mellanox Technologies platform support"
>>>>          depends on I2C && REGMAP
>>>> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
>>>> index c12a9b044fd8..6c7870190564 100644
>>>> --- a/drivers/platform/x86/Makefile
>>>> +++ b/drivers/platform/x86/Makefile
>>>> @@ -110,7 +110,6 @@ obj-$(CONFIG_TOPSTAR_LAPTOP)        += topstar-laptop.o
>>>>
>>>>   # Platform drivers
>>>>   obj-$(CONFIG_FW_ATTR_CLASS)            += firmware_attributes_class.o
>>>> -obj-$(CONFIG_I2C_MULTI_INSTANTIATE)    += i2c-multi-instantiate.o
>>>>   obj-$(CONFIG_MLX_PLATFORM)             += mlx-platform.o
>>>>   obj-$(CONFIG_TOUCHSCREEN_DMI)          += touchscreen_dmi.o
>>>>   obj-$(CONFIG_WIRELESS_HOTKEY)          += wireless-hotkey.o
>>>> --
>>>> 2.25.1
>>>>
>>
>