[PATCH v3] ACPI: Force I2C to be selected as a built-in module

[PATCH v3] ACPI: Force I2C to be selected as a built-in module

[Sinan Kaya]

From: Auger Eric <eric.auger@redhat.com>

If I2C is built as a module, ACPI_I2C_OPREGION cannot be set
and any ACPI opregion calls targeting I2C fail with no opregion found.

Commit da3c6647ee08 ("I2C/ACPI: Clean up I2C ACPI code and Add
CONFIG_I2C_ACPI config") says following:

"Current there is a race between removing I2C ACPI operation region
and ACPI AML code accessing."

This patch forces core I2C support to be compiled as a built-in
module if ACPI is selected as code is not ready for dynamic module
removal.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 drivers/acpi/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 4650539..5b48098 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -9,6 +9,8 @@ menuconfig ACPI
 	depends on IA64 || X86 || ARM64
 	depends on PCI
 	select PNP
+	# force building I2C in on ACPI systems, for opregion availability
+	imply I2C
 	default y if (IA64 || X86)
 	help
 	  Advanced Configuration and Power Interface (ACPI) support for 
-- 
1.9.1

[PATCH v3] ACPI: Force I2C to be selected as a built-in module

[Sinan Kaya]

From: Auger Eric <eric.auger@redhat.com>

If I2C is built as a module, ACPI_I2C_OPREGION cannot be set
and any ACPI opregion calls targeting I2C fail with no opregion found.

Commit da3c6647ee08 ("I2C/ACPI: Clean up I2C ACPI code and Add
CONFIG_I2C_ACPI config") says following:

"Current there is a race between removing I2C ACPI operation region
and ACPI AML code accessing."

This patch forces core I2C support to be compiled as a built-in
module if ACPI is selected as code is not ready for dynamic module
removal.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 drivers/acpi/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 4650539..5b48098 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -9,6 +9,8 @@ menuconfig ACPI
 	depends on IA64 || X86 || ARM64
 	depends on PCI
 	select PNP
+	# force building I2C in on ACPI systems, for opregion availability
+	imply I2C
 	default y if (IA64 || X86)
 	help
 	  Advanced Configuration and Power Interface (ACPI) support for 
-- 
1.9.1

Re: [PATCH v3] ACPI: Force I2C to be selected as a built-in module

[Rafael J. Wysocki]

On Thu, Jan 25, 2018 at 4:43 PM, Sinan Kaya <okaya@codeaurora.org> wrote:
> From: Auger Eric <eric.auger@redhat.com>
>
> If I2C is built as a module, ACPI_I2C_OPREGION cannot be set
> and any ACPI opregion calls targeting I2C fail with no opregion found.
>
> Commit da3c6647ee08 ("I2C/ACPI: Clean up I2C ACPI code and Add
> CONFIG_I2C_ACPI config") says following:
>
> "Current there is a race between removing I2C ACPI operation region
> and ACPI AML code accessing."
>
> This patch forces core I2C support to be compiled as a built-in
> module if ACPI is selected as code is not ready for dynamic module
> removal.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
>  drivers/acpi/Kconfig | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 4650539..5b48098 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -9,6 +9,8 @@ menuconfig ACPI
>         depends on IA64 || X86 || ARM64
>         depends on PCI
>         select PNP
> +       # force building I2C in on ACPI systems, for opregion availability
> +       imply I2C
>         default y if (IA64 || X86)
>         help
>           Advanced Configuration and Power Interface (ACPI) support for
> --

I'm not sure how much this helps.

I2C opregions will only work if the requisite I2C controller driver is
present anyway and this change doesn't guarantee that AFAICS.

OTOH, there are systems using ACPI without I2C opregions, so are we
really better off by forcing everybody using ACPI to also build I2C?

Thanks,
Rafael

Re: [PATCH v3] ACPI: Force I2C to be selected as a built-in module

[Sinan Kaya]

On 1/25/2018 11:25 AM, Rafael J. Wysocki wrote:
> On Thu, Jan 25, 2018 at 4:43 PM, Sinan Kaya <okaya@codeaurora.org> wrote:
>> From: Auger Eric <eric.auger@redhat.com>
>>
>> If I2C is built as a module, ACPI_I2C_OPREGION cannot be set
>> and any ACPI opregion calls targeting I2C fail with no opregion found.
>>
>> Commit da3c6647ee08 ("I2C/ACPI: Clean up I2C ACPI code and Add
>> CONFIG_I2C_ACPI config") says following:
>>
>> "Current there is a race between removing I2C ACPI operation region
>> and ACPI AML code accessing."
>>
>> This patch forces core I2C support to be compiled as a built-in
>> module if ACPI is selected as code is not ready for dynamic module
>> removal.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
>> ---
>>  drivers/acpi/Kconfig | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>> index 4650539..5b48098 100644
>> --- a/drivers/acpi/Kconfig
>> +++ b/drivers/acpi/Kconfig
>> @@ -9,6 +9,8 @@ menuconfig ACPI
>>         depends on IA64 || X86 || ARM64
>>         depends on PCI
>>         select PNP
>> +       # force building I2C in on ACPI systems, for opregion availability
>> +       imply I2C
>>         default y if (IA64 || X86)
>>         help
>>           Advanced Configuration and Power Interface (ACPI) support for
>> --
> 
> I'm not sure how much this helps.
> 
> I2C opregions will only work if the requisite I2C controller driver is
> present anyway and this change doesn't guarantee that AFAIC> 
> OTOH, there are systems using ACPI without I2C opregions, so are we
> really better off by forcing everybody using ACPI to also build I2C?

I was trying to find a good balance by choosing imply instead of select.

If an OS chooses to include an I2C driver as a module (most distros do),
core-i2c functionality becomes a built-in module with ACPI.

If an OS doesn't choose any I2C drivers either built-in/module, then I2C core
functionality wouldn't be selected; right?

> 
> Thanks,
> Rafael
> 


-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH v3] ACPI: Force I2C to be selected as a built-in module

[Rafael J. Wysocki]

On Thu, Jan 25, 2018 at 5:32 PM, Sinan Kaya <okaya@codeaurora.org> wrote:
> On 1/25/2018 11:25 AM, Rafael J. Wysocki wrote:
>> On Thu, Jan 25, 2018 at 4:43 PM, Sinan Kaya <okaya@codeaurora.org> wrote:
>>> From: Auger Eric <eric.auger@redhat.com>
>>>
>>> If I2C is built as a module, ACPI_I2C_OPREGION cannot be set
>>> and any ACPI opregion calls targeting I2C fail with no opregion found.
>>>
>>> Commit da3c6647ee08 ("I2C/ACPI: Clean up I2C ACPI code and Add
>>> CONFIG_I2C_ACPI config") says following:
>>>
>>> "Current there is a race between removing I2C ACPI operation region
>>> and ACPI AML code accessing."
>>>
>>> This patch forces core I2C support to be compiled as a built-in
>>> module if ACPI is selected as code is not ready for dynamic module
>>> removal.
>>>
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
>>> ---
>>>  drivers/acpi/Kconfig | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>>> index 4650539..5b48098 100644
>>> --- a/drivers/acpi/Kconfig
>>> +++ b/drivers/acpi/Kconfig
>>> @@ -9,6 +9,8 @@ menuconfig ACPI
>>>         depends on IA64 || X86 || ARM64
>>>         depends on PCI
>>>         select PNP
>>> +       # force building I2C in on ACPI systems, for opregion availability
>>> +       imply I2C
>>>         default y if (IA64 || X86)
>>>         help
>>>           Advanced Configuration and Power Interface (ACPI) support for
>>> --
>>
>> I'm not sure how much this helps.
>>
>> I2C opregions will only work if the requisite I2C controller driver is
>> present anyway and this change doesn't guarantee that AFAIC>
>> OTOH, there are systems using ACPI without I2C opregions, so are we
>> really better off by forcing everybody using ACPI to also build I2C?
>
> I was trying to find a good balance by choosing imply instead of select.
>
> If an OS chooses to include an I2C driver as a module (most distros do),
> core-i2c functionality becomes a built-in module with ACPI.

So what would be wrong with allowing ACPI_I2C_OPREGION to be set if
I2C is a module?

Re: [PATCH v3] ACPI: Force I2C to be selected as a built-in module

[Sinan Kaya]

On 1/25/2018 11:47 AM, Rafael J. Wysocki wrote:
> On Thu, Jan 25, 2018 at 5:32 PM, Sinan Kaya <okaya@codeaurora.org> wrote:
>> On 1/25/2018 11:25 AM, Rafael J. Wysocki wrote:
>>> On Thu, Jan 25, 2018 at 4:43 PM, Sinan Kaya <okaya@codeaurora.org> wrote:
>>>> From: Auger Eric <eric.auger@redhat.com>
>>>>
>>>> If I2C is built as a module, ACPI_I2C_OPREGION cannot be set
>>>> and any ACPI opregion calls targeting I2C fail with no opregion found.
>>>>
>>>> Commit da3c6647ee08 ("I2C/ACPI: Clean up I2C ACPI code and Add
>>>> CONFIG_I2C_ACPI config") says following:
>>>>
>>>> "Current there is a race between removing I2C ACPI operation region
>>>> and ACPI AML code accessing."
>>>>
>>>> This patch forces core I2C support to be compiled as a built-in
>>>> module if ACPI is selected as code is not ready for dynamic module
>>>> removal.
>>>>
>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
>>>> ---
>>>>  drivers/acpi/Kconfig | 2 ++
>>>>  1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>>>> index 4650539..5b48098 100644
>>>> --- a/drivers/acpi/Kconfig
>>>> +++ b/drivers/acpi/Kconfig
>>>> @@ -9,6 +9,8 @@ menuconfig ACPI
>>>>         depends on IA64 || X86 || ARM64
>>>>         depends on PCI
>>>>         select PNP
>>>> +       # force building I2C in on ACPI systems, for opregion availability
>>>> +       imply I2C
>>>>         default y if (IA64 || X86)
>>>>         help
>>>>           Advanced Configuration and Power Interface (ACPI) support for
>>>> --
>>>
>>> I'm not sure how much this helps.
>>>
>>> I2C opregions will only work if the requisite I2C controller driver is
>>> present anyway and this change doesn't guarantee that AFAIC>
>>> OTOH, there are systems using ACPI without I2C opregions, so are we
>>> really better off by forcing everybody using ACPI to also build I2C?
>>
>> I was trying to find a good balance by choosing imply instead of select.
>>
>> If an OS chooses to include an I2C driver as a module (most distros do),
>> core-i2c functionality becomes a built-in module with ACPI.
> 
> So what would be wrong with allowing ACPI_I2C_OPREGION to be set if
> I2C is a module?

That was the original patch. Folks are saying that it breaks some systems.

Conversation is here:

https://patchwork.kernel.org/patch/10142425/


> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH v3] ACPI: Force I2C to be selected as a built-in module

[Rafael J. Wysocki]

On Thu, Jan 25, 2018 at 5:53 PM, Sinan Kaya <okaya@codeaurora.org> wrote:
> On 1/25/2018 11:47 AM, Rafael J. Wysocki wrote:
>> On Thu, Jan 25, 2018 at 5:32 PM, Sinan Kaya <okaya@codeaurora.org> wrote:
>>> On 1/25/2018 11:25 AM, Rafael J. Wysocki wrote:
>>>> On Thu, Jan 25, 2018 at 4:43 PM, Sinan Kaya <okaya@codeaurora.org> wrote:
>>>>> From: Auger Eric <eric.auger@redhat.com>
>>>>>
>>>>> If I2C is built as a module, ACPI_I2C_OPREGION cannot be set
>>>>> and any ACPI opregion calls targeting I2C fail with no opregion found.
>>>>>
>>>>> Commit da3c6647ee08 ("I2C/ACPI: Clean up I2C ACPI code and Add
>>>>> CONFIG_I2C_ACPI config") says following:
>>>>>
>>>>> "Current there is a race between removing I2C ACPI operation region
>>>>> and ACPI AML code accessing."
>>>>>
>>>>> This patch forces core I2C support to be compiled as a built-in
>>>>> module if ACPI is selected as code is not ready for dynamic module
>>>>> removal.
>>>>>
>>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
>>>>> ---
>>>>>  drivers/acpi/Kconfig | 2 ++
>>>>>  1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>>>>> index 4650539..5b48098 100644
>>>>> --- a/drivers/acpi/Kconfig
>>>>> +++ b/drivers/acpi/Kconfig
>>>>> @@ -9,6 +9,8 @@ menuconfig ACPI
>>>>>         depends on IA64 || X86 || ARM64
>>>>>         depends on PCI
>>>>>         select PNP
>>>>> +       # force building I2C in on ACPI systems, for opregion availability
>>>>> +       imply I2C
>>>>>         default y if (IA64 || X86)
>>>>>         help
>>>>>           Advanced Configuration and Power Interface (ACPI) support for
>>>>> --
>>>>
>>>> I'm not sure how much this helps.
>>>>
>>>> I2C opregions will only work if the requisite I2C controller driver is
>>>> present anyway and this change doesn't guarantee that AFAIC>
>>>> OTOH, there are systems using ACPI without I2C opregions, so are we
>>>> really better off by forcing everybody using ACPI to also build I2C?
>>>
>>> I was trying to find a good balance by choosing imply instead of select.
>>>
>>> If an OS chooses to include an I2C driver as a module (most distros do),
>>> core-i2c functionality becomes a built-in module with ACPI.
>>
>> So what would be wrong with allowing ACPI_I2C_OPREGION to be set if
>> I2C is a module?
>
> That was the original patch. Folks are saying that it breaks some systems.
>
> Conversation is here:
>
> https://patchwork.kernel.org/patch/10142425/

Couldn't that be addressed by preventing the I2C module from being
unloaded in the relevant cases?

Re: [PATCH v3] ACPI: Force I2C to be selected as a built-in module

[Sinan Kaya]

On 1/25/2018 11:57 AM, Rafael J. Wysocki wrote:
>> That was the original patch. Folks are saying that it breaks some systems.
>>
>> Conversation is here:
>>
>> https://patchwork.kernel.org/patch/10142425/
> Couldn't that be addressed by preventing the I2C module from being
> unloaded in the relevant cases?
> 

Maybe, I'll let Andy And Hans speak for themselves. It is not my code.

I listed the spec behavior and expectation from the OS in the link above.
Apparently, Linux isn't playing nice.

Hans is also raising a concern about driver load order that is not being
handled well between the I2C driver, its sub modules like PMIC and the ACPI support.

"Lets pretend that all DSTDs are perfect and that some device described in ACPI
has a _PS0 method which uses an opregion to turn on some regulator powering
the device through i2c, but only if the _REG method for that opregion has
been called. So now lets say that the driver for this device loads and
tries to bind before the i2c-module has loaded. Before the driver's probe
method gets called the driver-core will call _PS0 to power-up the device,
which is a nop (*). Then the drivers probe function tries to talk to the
device, but fails as the device is not powered, so it returns with -ENODEV."

I'd think that driver would return -EPROBE_DEFER in order to wait until its
dependencies come in place. Maybe, driver was not designed for this.

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH v3] ACPI: Force I2C to be selected as a built-in module

[Hans de Goede]

Hi,

On 25-01-18 18:07, Sinan Kaya wrote:
> On 1/25/2018 11:57 AM, Rafael J. Wysocki wrote:
>>> That was the original patch. Folks are saying that it breaks some systems.
>>>
>>> Conversation is here:
>>>
>>> https://patchwork.kernel.org/patch/10142425/
>> Couldn't that be addressed by preventing the I2C module from being
>> unloaded in the relevant cases?
>>
> 
> Maybe, I'll let Andy And Hans speak for themselves. It is not my code.
> 
> I listed the spec behavior and expectation from the OS in the link above.
> Apparently, Linux isn't playing nice.
> 
> Hans is also raising a concern about driver load order that is not being
> handled well between the I2C driver, its sub modules like PMIC and the ACPI support.
> 
> "Lets pretend that all DSTDs are perfect and that some device described in ACPI
> has a _PS0 method which uses an opregion to turn on some regulator powering
> the device through i2c, but only if the _REG method for that opregion has
> been called. So now lets say that the driver for this device loads and
> tries to bind before the i2c-module has loaded. Before the driver's probe
> method gets called the driver-core will call _PS0 to power-up the device,
> which is a nop (*). Then the drivers probe function tries to talk to the
> device, but fails as the device is not powered, so it returns with -ENODEV."
> 
> I'd think that driver would return -EPROBE_DEFER in order to wait until its
> dependencies come in place. Maybe, driver was not designed for this.

The driver is not aware of any power-management done by ACPI, so the driver
cannot return -EPROBE_DEFER.

Regards,

Hans

Re: [PATCH v3] ACPI: Force I2C to be selected as a built-in module

[Randy Dunlap]

On 01/25/2018 08:25 AM, Rafael J. Wysocki wrote:
> On Thu, Jan 25, 2018 at 4:43 PM, Sinan Kaya <okaya@codeaurora.org> wrote:
>> From: Auger Eric <eric.auger@redhat.com>
>>
>> If I2C is built as a module, ACPI_I2C_OPREGION cannot be set
>> and any ACPI opregion calls targeting I2C fail with no opregion found.
>>
>> Commit da3c6647ee08 ("I2C/ACPI: Clean up I2C ACPI code and Add
>> CONFIG_I2C_ACPI config") says following:
>>
>> "Current there is a race between removing I2C ACPI operation region
>> and ACPI AML code accessing."
>>
>> This patch forces core I2C support to be compiled as a built-in
>> module if ACPI is selected as code is not ready for dynamic module
>> removal.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
>> ---
>>  drivers/acpi/Kconfig | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>> index 4650539..5b48098 100644
>> --- a/drivers/acpi/Kconfig
>> +++ b/drivers/acpi/Kconfig
>> @@ -9,6 +9,8 @@ menuconfig ACPI
>>         depends on IA64 || X86 || ARM64
>>         depends on PCI
>>         select PNP
>> +       # force building I2C in on ACPI systems, for opregion availability
>> +       imply I2C
>>         default y if (IA64 || X86)
>>         help
>>           Advanced Configuration and Power Interface (ACPI) support for
>> --
> 
> I'm not sure how much this helps.
> 
> I2C opregions will only work if the requisite I2C controller driver is
> present anyway and this change doesn't guarantee that AFAICS.
> 
> OTOH, there are systems using ACPI without I2C opregions, so are we
> really better off by forcing everybody using ACPI to also build I2C?

Definitely not.

-- 
~Randy

Re: [PATCH v3] ACPI: Force I2C to be selected as a built-in module

[Sinan Kaya]

Hi Rafael,

On 1/25/2018 12:36 PM, Randy Dunlap wrote:
> On 01/25/2018 08:25 AM, Rafael J. Wysocki wrote:
>> On Thu, Jan 25, 2018 at 4:43 PM, Sinan Kaya <okaya@codeaurora.org> wrote:
>>> From: Auger Eric <eric.auger@redhat.com>
>>>
>>> If I2C is built as a module, ACPI_I2C_OPREGION cannot be set
>>> and any ACPI opregion calls targeting I2C fail with no opregion found.
>>>
>>> Commit da3c6647ee08 ("I2C/ACPI: Clean up I2C ACPI code and Add
>>> CONFIG_I2C_ACPI config") says following:
>>>
>>> "Current there is a race between removing I2C ACPI operation region
>>> and ACPI AML code accessing."
>>>
>>> This patch forces core I2C support to be compiled as a built-in
>>> module if ACPI is selected as code is not ready for dynamic module
>>> removal.
>>>
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
>>> ---
>>>  drivers/acpi/Kconfig | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>>> index 4650539..5b48098 100644
>>> --- a/drivers/acpi/Kconfig
>>> +++ b/drivers/acpi/Kconfig
>>> @@ -9,6 +9,8 @@ menuconfig ACPI
>>>         depends on IA64 || X86 || ARM64
>>>         depends on PCI
>>>         select PNP
>>> +       # force building I2C in on ACPI systems, for opregion availability
>>> +       imply I2C
>>>         default y if (IA64 || X86)
>>>         help
>>>           Advanced Configuration and Power Interface (ACPI) support for
>>> --
>>
>> I'm not sure how much this helps.
>>
>> I2C opregions will only work if the requisite I2C controller driver is
>> present anyway and this change doesn't guarantee that AFAICS.
>>
>> OTOH, there are systems using ACPI without I2C opregions, so are we
>> really better off by forcing everybody using ACPI to also build I2C?
> 
> Definitely not.
> 

Where do we stand on this? Do you have a better suggestion?


Sinan

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH v3] ACPI: Force I2C to be selected as a built-in module

[Rafael J. Wysocki]

On Tuesday, February 6, 2018 3:25:58 PM CET Sinan Kaya wrote:
> Hi Rafael,
> 
> On 1/25/2018 12:36 PM, Randy Dunlap wrote:
> > On 01/25/2018 08:25 AM, Rafael J. Wysocki wrote:
> >> On Thu, Jan 25, 2018 at 4:43 PM, Sinan Kaya <okaya@codeaurora.org> wrote:
> >>> From: Auger Eric <eric.auger@redhat.com>
> >>>
> >>> If I2C is built as a module, ACPI_I2C_OPREGION cannot be set
> >>> and any ACPI opregion calls targeting I2C fail with no opregion found.
> >>>
> >>> Commit da3c6647ee08 ("I2C/ACPI: Clean up I2C ACPI code and Add
> >>> CONFIG_I2C_ACPI config") says following:
> >>>
> >>> "Current there is a race between removing I2C ACPI operation region
> >>> and ACPI AML code accessing."
> >>>
> >>> This patch forces core I2C support to be compiled as a built-in
> >>> module if ACPI is selected as code is not ready for dynamic module
> >>> removal.
> >>>
> >>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> >>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> >>> ---
> >>>  drivers/acpi/Kconfig | 2 ++
> >>>  1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> >>> index 4650539..5b48098 100644
> >>> --- a/drivers/acpi/Kconfig
> >>> +++ b/drivers/acpi/Kconfig
> >>> @@ -9,6 +9,8 @@ menuconfig ACPI
> >>>         depends on IA64 || X86 || ARM64
> >>>         depends on PCI
> >>>         select PNP
> >>> +       # force building I2C in on ACPI systems, for opregion availability
> >>> +       imply I2C
> >>>         default y if (IA64 || X86)
> >>>         help
> >>>           Advanced Configuration and Power Interface (ACPI) support for
> >>> --
> >>
> >> I'm not sure how much this helps.
> >>
> >> I2C opregions will only work if the requisite I2C controller driver is
> >> present anyway and this change doesn't guarantee that AFAICS.
> >>
> >> OTOH, there are systems using ACPI without I2C opregions, so are we
> >> really better off by forcing everybody using ACPI to also build I2C?
> > 
> > Definitely not.
> > 
> 
> Where do we stand on this? Do you have a better suggestion?

Not at the moment, but I also don't think that this approach is valid.

Thanks,
Rafael