Message ID | 1398147855-9868-10-git-send-email-tianyu.lan@intel.com (mailing list archive) |
---|---|
State | Superseded, archived |
Headers | show |
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
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.
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
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 --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);
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(-)