diff mbox

[Resend,9/9] I2C/ACPI: Add CONFIG_I2C_ACPI config

Message ID 1398147855-9868-10-git-send-email-tianyu.lan@intel.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

lan,Tianyu April 22, 2014, 6:24 a.m. UTC
This patch is to add CONFIG_I2C_ACPI. Current there is a race between
removing I2C ACPI operation region and ACPI AML code accessing.
So make i2c core built-in if CONFIG_I2C_ACPI is set.

Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
 drivers/i2c/Kconfig  | 17 ++++++++++++++++-
 drivers/i2c/Makefile |  2 +-
 include/linux/i2c.h  |  2 +-
 3 files changed, 18 insertions(+), 3 deletions(-)

Comments

Mika Westerberg April 22, 2014, 11:45 a.m. UTC | #1
On Tue, Apr 22, 2014 at 02:24:15PM +0800, Lan Tianyu wrote:
> This patch is to add CONFIG_I2C_ACPI. Current there is a race between
> removing I2C ACPI operation region and ACPI AML code accessing.
> So make i2c core built-in if CONFIG_I2C_ACPI is set.
> 
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> ---
>  drivers/i2c/Kconfig  | 17 ++++++++++++++++-
>  drivers/i2c/Makefile |  2 +-
>  include/linux/i2c.h  |  2 +-
>  3 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
> index 7b7ea32..c670d49 100644
> --- a/drivers/i2c/Kconfig
> +++ b/drivers/i2c/Kconfig
> @@ -2,7 +2,9 @@
>  # I2C subsystem configuration
>  #
>  
> -menuconfig I2C
> +menu "I2C support"
> +
> +config I2C
>  	tristate "I2C support"
>  	select RT_MUTEXES
>  	---help---
> @@ -21,6 +23,17 @@ menuconfig I2C
>  	  This I2C support can also be built as a module.  If so, the module
>  	  will be called i2c-core.
>  
> +config I2C_ACPI
> +	bool "I2C ACPI support"
> +	select I2C
> +	depends on ACPI
> +	default y
> +	help
> +	  Say Y here if you want to enable I2C ACPI function. ACPI table
> +	  provides I2C slave devices' information to enumerate these devices.
> +	  This option also allows ACPI AML code to access I2C slave devices
> +	  via I2C ACPI operation region to fulfill ACPI method.
> +

I'm wondering, can we provide some sort of wrapper function from ACPI core
that is guaranteed to be built in to the kernel image and use it instead of
adding new Kconfig options?
--
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
lan,Tianyu April 23, 2014, 5:39 a.m. UTC | #2
On 2014?04?22? 19:45, Mika Westerberg wrote:
> On Tue, Apr 22, 2014 at 02:24:15PM +0800, Lan Tianyu wrote:
>> This patch is to add CONFIG_I2C_ACPI. Current there is a race between
>> removing I2C ACPI operation region and ACPI AML code accessing.
>> So make i2c core built-in if CONFIG_I2C_ACPI is set.
>>
>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
>> ---
>>  drivers/i2c/Kconfig  | 17 ++++++++++++++++-
>>  drivers/i2c/Makefile |  2 +-
>>  include/linux/i2c.h  |  2 +-
>>  3 files changed, 18 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
>> index 7b7ea32..c670d49 100644
>> --- a/drivers/i2c/Kconfig
>> +++ b/drivers/i2c/Kconfig
>> @@ -2,7 +2,9 @@
>>  # I2C subsystem configuration
>>  #
>>  
>> -menuconfig I2C
>> +menu "I2C support"
>> +
>> +config I2C
>>  	tristate "I2C support"
>>  	select RT_MUTEXES
>>  	---help---
>> @@ -21,6 +23,17 @@ menuconfig I2C
>>  	  This I2C support can also be built as a module.  If so, the module
>>  	  will be called i2c-core.
>>  
>> +config I2C_ACPI
>> +	bool "I2C ACPI support"
>> +	select I2C
>> +	depends on ACPI
>> +	default y
>> +	help
>> +	  Say Y here if you want to enable I2C ACPI function. ACPI table
>> +	  provides I2C slave devices' information to enumerate these devices.
>> +	  This option also allows ACPI AML code to access I2C slave devices
>> +	  via I2C ACPI operation region to fulfill ACPI method.
>> +
> 
> I'm wondering, can we provide some sort of wrapper function from ACPI core
> that is guaranteed to be built in to the kernel image and use it instead of
> adding new Kconfig options?
> 
Cc: LV

LV tried to fix the issue via wrapper solution in the ACPI code before.
https://lkml.org/lkml/2013/7/23/87

He has a plan to resolve the issue in ACPICA later.

Other choice is to increase the i2c-core module count to prevent it
being unloaded when i2c operation region handler is installed. Remove
the code When LV finish his job.
Lv Zheng April 23, 2014, 6:47 a.m. UTC | #3
SGksIFRpYW55dQ0KDQo+IEZyb206IExhbiwgVGlhbnl1DQo+IFNlbnQ6IFdlZG5lc2RheSwgQXBy
aWwgMjMsIDIwMTQgMTo0MCBQTQ0KPiANCj4gT24gMjAxNOW5tDA05pyIMjLml6UgMTk6NDUsIE1p
a2EgV2VzdGVyYmVyZyB3cm90ZToNCj4gPiBPbiBUdWUsIEFwciAyMiwgMjAxNCBhdCAwMjoyNDox
NVBNICswODAwLCBMYW4gVGlhbnl1IHdyb3RlOg0KPiA+PiBUaGlzIHBhdGNoIGlzIHRvIGFkZCBD
T05GSUdfSTJDX0FDUEkuIEN1cnJlbnQgdGhlcmUgaXMgYSByYWNlIGJldHdlZW4NCj4gPj4gcmVt
b3ZpbmcgSTJDIEFDUEkgb3BlcmF0aW9uIHJlZ2lvbiBhbmQgQUNQSSBBTUwgY29kZSBhY2Nlc3Np
bmcuDQo+ID4+IFNvIG1ha2UgaTJjIGNvcmUgYnVpbHQtaW4gaWYgQ09ORklHX0kyQ19BQ1BJIGlz
IHNldC4NCj4gPj4NCj4gPj4gU2lnbmVkLW9mZi1ieTogTGFuIFRpYW55dSA8dGlhbnl1LmxhbkBp
bnRlbC5jb20+DQo+ID4+IC0tLQ0KPiA+PiAgZHJpdmVycy9pMmMvS2NvbmZpZyAgfCAxNyArKysr
KysrKysrKysrKysrLQ0KPiA+PiAgZHJpdmVycy9pMmMvTWFrZWZpbGUgfCAgMiArLQ0KPiA+PiAg
aW5jbHVkZS9saW51eC9pMmMuaCAgfCAgMiArLQ0KPiA+PiAgMyBmaWxlcyBjaGFuZ2VkLCAxOCBp
bnNlcnRpb25zKCspLCAzIGRlbGV0aW9ucygtKQ0KPiA+Pg0KPiA+PiBkaWZmIC0tZ2l0IGEvZHJp
dmVycy9pMmMvS2NvbmZpZyBiL2RyaXZlcnMvaTJjL0tjb25maWcNCj4gPj4gaW5kZXggN2I3ZWEz
Mi4uYzY3MGQ0OSAxMDA2NDQNCj4gPj4gLS0tIGEvZHJpdmVycy9pMmMvS2NvbmZpZw0KPiA+PiAr
KysgYi9kcml2ZXJzL2kyYy9LY29uZmlnDQo+ID4+IEBAIC0yLDcgKzIsOSBAQA0KPiA+PiAgIyBJ
MkMgc3Vic3lzdGVtIGNvbmZpZ3VyYXRpb24NCj4gPj4gICMNCj4gPj4NCj4gPj4gLW1lbnVjb25m
aWcgSTJDDQo+ID4+ICttZW51ICJJMkMgc3VwcG9ydCINCj4gPj4gKw0KPiA+PiArY29uZmlnIEky
Qw0KPiA+PiAgCXRyaXN0YXRlICJJMkMgc3VwcG9ydCINCj4gPj4gIAlzZWxlY3QgUlRfTVVURVhF
Uw0KPiA+PiAgCS0tLWhlbHAtLS0NCj4gPj4gQEAgLTIxLDYgKzIzLDE3IEBAIG1lbnVjb25maWcg
STJDDQo+ID4+ICAJICBUaGlzIEkyQyBzdXBwb3J0IGNhbiBhbHNvIGJlIGJ1aWx0IGFzIGEgbW9k
dWxlLiAgSWYgc28sIHRoZSBtb2R1bGUNCj4gPj4gIAkgIHdpbGwgYmUgY2FsbGVkIGkyYy1jb3Jl
Lg0KPiA+Pg0KPiA+PiArY29uZmlnIEkyQ19BQ1BJDQo+ID4+ICsJYm9vbCAiSTJDIEFDUEkgc3Vw
cG9ydCINCj4gPj4gKwlzZWxlY3QgSTJDDQo+ID4+ICsJZGVwZW5kcyBvbiBBQ1BJDQo+ID4+ICsJ
ZGVmYXVsdCB5DQo+ID4+ICsJaGVscA0KPiA+PiArCSAgU2F5IFkgaGVyZSBpZiB5b3Ugd2FudCB0
byBlbmFibGUgSTJDIEFDUEkgZnVuY3Rpb24uIEFDUEkgdGFibGUNCj4gPj4gKwkgIHByb3ZpZGVz
IEkyQyBzbGF2ZSBkZXZpY2VzJyBpbmZvcm1hdGlvbiB0byBlbnVtZXJhdGUgdGhlc2UgZGV2aWNl
cy4NCj4gPj4gKwkgIFRoaXMgb3B0aW9uIGFsc28gYWxsb3dzIEFDUEkgQU1MIGNvZGUgdG8gYWNj
ZXNzIEkyQyBzbGF2ZSBkZXZpY2VzDQo+ID4+ICsJICB2aWEgSTJDIEFDUEkgb3BlcmF0aW9uIHJl
Z2lvbiB0byBmdWxmaWxsIEFDUEkgbWV0aG9kLg0KPiA+PiArDQo+ID4NCj4gPiBJJ20gd29uZGVy
aW5nLCBjYW4gd2UgcHJvdmlkZSBzb21lIHNvcnQgb2Ygd3JhcHBlciBmdW5jdGlvbiBmcm9tIEFD
UEkgY29yZQ0KPiA+IHRoYXQgaXMgZ3VhcmFudGVlZCB0byBiZSBidWlsdCBpbiB0byB0aGUga2Vy
bmVsIGltYWdlIGFuZCB1c2UgaXQgaW5zdGVhZCBvZg0KPiA+IGFkZGluZyBuZXcgS2NvbmZpZyBv
cHRpb25zPw0KPiA+DQo+IENjOiBMVg0KPiANCj4gTFYgdHJpZWQgdG8gZml4IHRoZSBpc3N1ZSB2
aWEgd3JhcHBlciBzb2x1dGlvbiBpbiB0aGUgQUNQSSBjb2RlIGJlZm9yZS4NCj4gaHR0cHM6Ly9s
a21sLm9yZy9sa21sLzIwMTMvNy8yMy84Nw0KPiANCj4gSGUgaGFzIGEgcGxhbiB0byByZXNvbHZl
IHRoZSBpc3N1ZSBpbiBBQ1BJQ0EgbGF0ZXIuDQo+IA0KPiBPdGhlciBjaG9pY2UgaXMgdG8gaW5j
cmVhc2UgdGhlIGkyYy1jb3JlIG1vZHVsZSBjb3VudCB0byBwcmV2ZW50IGl0DQo+IGJlaW5nIHVu
bG9hZGVkIHdoZW4gaTJjIG9wZXJhdGlvbiByZWdpb24gaGFuZGxlciBpcyBpbnN0YWxsZWQuIFJl
bW92ZQ0KPiB0aGUgY29kZSBXaGVuIExWIGZpbmlzaCBoaXMgam9iLg0KDQpZb3UgbWF5IHNlZSBp
dCBpbXBsZW1lbnRlZCBpbiBBQ1BJQ0EgYWZ0ZXIgc2V2ZXJhbCByZWxlYXNlLg0KSWYgeW91IG5l
ZWQgYSBmaXggZm9yIG5vdywgeW91IGNhbiB1c2UgdGhlIHBhdGNoIHBvaW50ZWQgdG8gYnkgdGhl
IGxpbmsgeW91J3ZlIHByb3ZpZGVkLA0KT3IgeW91IGNvdWxkIGZpbmQgYW4gdXBkYXRlZCBvbmUg
aGVyZToNCiBhY3BpLWlwbWkxMy5wYXRjaCBhcmNoaXZlZCBpbiAoaHR0cHM6Ly9idWd6aWxsYS5r
ZXJuZWwub3JnL2F0dGFjaG1lbnQuY2dpP2lkPTExMjYxMSkNCg0KSSB0aGluayB0aGUgc29sdXRp
b24geW91J3ZlIHByb3ZpZGVkIGluIHRoaXMgcGF0Y2ggaXMgYWxzbyByZWFzb25hYmxlIGZvciBu
b3cuDQpJUE1JIGFsc28gdXNlcyBhIHNpbWlsYXIgc29sdXRpb24gdG8gc29sdmUgdGhpcyBpc3N1
ZS4NClBsZWFzZSByZWZlciB0byB0aGUgQ09ORklHX0FDUElfSVBNSS4NCg0KVGhlIHN0b3J5IGNh
biBiZSBmb3VuZCBhdDoNCmh0dHA6Ly93d3cuc3Bpbmljcy5uZXQvbGlzdHMvbGludXgtYWNwaS9t
c2c0OTA0NC5odG1sDQpBbmQgdGhlIHNpbWlsYXIgc29sdXRpb24gY2FuIGJlIGZvdW5kIGF0Og0K
aHR0cDovL3d3dy5zcGluaWNzLm5ldC9saXN0cy9saW51eC1hY3BpL21zZzQ5MTg0Lmh0bWwNCg0K
VGhhbmtzIGFuZCBiZXN0IHJlZ2FyZHMNCi1Mdg0KDQo+IA0KPiAtLQ0KPiBCZXN0IHJlZ2FyZHMN
Cj4gVGlhbnl1IExhbg0K
--
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
Mika Westerberg April 23, 2014, 7:40 a.m. UTC | #4
On Wed, Apr 23, 2014 at 06:47:05AM +0000, Zheng, Lv wrote:
> Hi, Tianyu
> 
> > From: Lan, Tianyu
> > Sent: Wednesday, April 23, 2014 1:40 PM
> > 
> > On 2014?04?22? 19:45, Mika Westerberg wrote:
> > > On Tue, Apr 22, 2014 at 02:24:15PM +0800, Lan Tianyu wrote:
> > >> This patch is to add CONFIG_I2C_ACPI. Current there is a race between
> > >> removing I2C ACPI operation region and ACPI AML code accessing.
> > >> So make i2c core built-in if CONFIG_I2C_ACPI is set.
> > >>
> > >> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> > >> ---
> > >>  drivers/i2c/Kconfig  | 17 ++++++++++++++++-
> > >>  drivers/i2c/Makefile |  2 +-
> > >>  include/linux/i2c.h  |  2 +-
> > >>  3 files changed, 18 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
> > >> index 7b7ea32..c670d49 100644
> > >> --- a/drivers/i2c/Kconfig
> > >> +++ b/drivers/i2c/Kconfig
> > >> @@ -2,7 +2,9 @@
> > >>  # I2C subsystem configuration
> > >>  #
> > >>
> > >> -menuconfig I2C
> > >> +menu "I2C support"
> > >> +
> > >> +config I2C
> > >>  	tristate "I2C support"
> > >>  	select RT_MUTEXES
> > >>  	---help---
> > >> @@ -21,6 +23,17 @@ menuconfig I2C
> > >>  	  This I2C support can also be built as a module.  If so, the module
> > >>  	  will be called i2c-core.
> > >>
> > >> +config I2C_ACPI
> > >> +	bool "I2C ACPI support"
> > >> +	select I2C
> > >> +	depends on ACPI
> > >> +	default y
> > >> +	help
> > >> +	  Say Y here if you want to enable I2C ACPI function. ACPI table
> > >> +	  provides I2C slave devices' information to enumerate these devices.
> > >> +	  This option also allows ACPI AML code to access I2C slave devices
> > >> +	  via I2C ACPI operation region to fulfill ACPI method.
> > >> +
> > >
> > > I'm wondering, can we provide some sort of wrapper function from ACPI core
> > > that is guaranteed to be built in to the kernel image and use it instead of
> > > adding new Kconfig options?
> > >
> > Cc: LV
> > 
> > LV tried to fix the issue via wrapper solution in the ACPI code before.
> > https://lkml.org/lkml/2013/7/23/87
> > 
> > He has a plan to resolve the issue in ACPICA later.
> > 
> > Other choice is to increase the i2c-core module count to prevent it
> > being unloaded when i2c operation region handler is installed. Remove
> > the code When LV finish his job.
> 
> You may see it implemented in ACPICA after several release.
> If you need a fix for now, you can use the patch pointed to by the link you've provided,
> Or you could find an updated one here:
>  acpi-ipmi13.patch archived in (https://bugzilla.kernel.org/attachment.cgi?id=112611)
> 
> I think the solution you've provided in this patch is also reasonable for now.
> IPMI also uses a similar solution to solve this issue.
> Please refer to the CONFIG_ACPI_IPMI.
> 
> The story can be found at:
> http://www.spinics.net/lists/linux-acpi/msg49044.html
> And the similar solution can be found at:
> http://www.spinics.net/lists/linux-acpi/msg49184.html

Thanks for the pointers.

Given that the IPMI problem was solved like this I guess I2C operation
regions can follow the same pattern if there is no better solution
available.

Out of curiousity: how did you plan to fix this in ACPICA?
--
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
diff mbox

Patch

diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
index 7b7ea32..c670d49 100644
--- a/drivers/i2c/Kconfig
+++ b/drivers/i2c/Kconfig
@@ -2,7 +2,9 @@ 
 # I2C subsystem configuration
 #
 
-menuconfig I2C
+menu "I2C support"
+
+config I2C
 	tristate "I2C support"
 	select RT_MUTEXES
 	---help---
@@ -21,6 +23,17 @@  menuconfig I2C
 	  This I2C support can also be built as a module.  If so, the module
 	  will be called i2c-core.
 
+config I2C_ACPI
+	bool "I2C ACPI support"
+	select I2C
+	depends on ACPI
+	default y
+	help
+	  Say Y here if you want to enable I2C ACPI function. ACPI table
+	  provides I2C slave devices' information to enumerate these devices.
+	  This option also allows ACPI AML code to access I2C slave devices
+	  via I2C ACPI operation region to fulfill ACPI method.
+
 if I2C
 
 config I2C_BOARDINFO
@@ -124,3 +137,5 @@  config I2C_DEBUG_BUS
 	  on.
 
 endif # I2C
+
+endmenu
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
index 80db307..37464ee 100644
--- a/drivers/i2c/Makefile
+++ b/drivers/i2c/Makefile
@@ -3,7 +3,7 @@ 
 #
 
 i2ccore-y := i2c-core.o
-i2ccore-$(CONFIG_ACPI)		+= i2c-acpi.o
+i2ccore-$(CONFIG_I2C_ACPI)		+= i2c-acpi.o
 
 obj-$(CONFIG_I2C_BOARDINFO)	+= i2c-boardinfo.o
 obj-$(CONFIG_I2C)		+= i2ccore.o
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index fc1ef42..d0ece9f 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -583,7 +583,7 @@  static inline struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node
 }
 #endif /* CONFIG_OF */
 
-#ifdef CONFIG_ACPI
+#ifdef CONFIG_I2C_ACPI
 int acpi_i2c_install_space_handler(struct i2c_adapter *adapter);
 void acpi_i2c_remove_space_handler(struct i2c_adapter *adapter);
 void acpi_i2c_register_devices(struct i2c_adapter *adap);