diff mbox series

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

Message ID 20220118145251.1548-6-sbinding@opensource.cirrus.com (mailing list archive)
State Superseded, archived
Headers show
Series Support Spi in i2c-multi-instantiate driver | expand

Commit Message

Stefan Binding Jan. 18, 2022, 2:52 p.m. UTC
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>
---
 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%)

Comments

Rafael J. Wysocki Jan. 19, 2022, 4:53 p.m. UTC | #1
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?

> ---
>  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
>
Lucas Tanure Jan. 19, 2022, 5:33 p.m. UTC | #2
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

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.

Thanks
Lucas Tanure


>> ---
>>   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
>>
Rafael J. Wysocki Jan. 19, 2022, 5:44 p.m. UTC | #3
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.

> >> ---
> >>   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
> >>
>
Hans de Goede Jan. 19, 2022, 5:48 p.m. UTC | #4
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
>>>>
>>
>
Andy Shevchenko Jan. 21, 2022, 8:11 p.m. UTC | #5
On Fri, Jan 21, 2022 at 9:53 PM Rafael J. Wysocki <rafael@kernel.org> 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:

...

> > > 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.

My thoughts were that these are related to ACPI handling the serial
buses in one place. However, counter arguments might be that the cases
of the resources like this are found only on x86 hardware (while ACPI
should be agnostic to that) and that the i2c and spi already do ACPI
stuff on their own. That said, there are pros and cons and I'm fine
with either choice at the end of the day.
diff mbox series

Patch

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