diff mbox

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

Message ID 1398695268-28645-10-git-send-email-tianyu.lan@intel.com (mailing list archive)
State Changes Requested, archived
Headers show

Commit Message

lan,Tianyu April 28, 2014, 2:27 p.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 29, 2014, 8:16 a.m. UTC | #1
On Mon, Apr 28, 2014 at 10:27:48PM +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 would prefer something like:

Say Y here if you want to enable ACPI I2C support. This includes support
for automatic enumeration of I2C slave devices and support for ACPI I2C
Operation Regions. Operation Regions allow firmware (BIOS) code to
access I2C slave devices, such as smart batteries through an I2C host
controller driver.

But it is really up to you so,

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
--
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
Wolfram Sang May 17, 2014, 5:48 p.m. UTC | #2
On Tue, Apr 29, 2014 at 11:16:09AM +0300, Mika Westerberg wrote:
> On Mon, Apr 28, 2014 at 10:27:48PM +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 would prefer something like:
> 
> Say Y here if you want to enable ACPI I2C support. This includes support
> for automatic enumeration of I2C slave devices and support for ACPI I2C
> Operation Regions. Operation Regions allow firmware (BIOS) code to
> access I2C slave devices, such as smart batteries through an I2C host
> controller driver.
> 
> But it is really up to you so,
> 
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

How does this fit into the context of
55e71edb81b2b45273e7b284cce13ff24bde846f ("i2c: move ACPI helpers into
the core")?
Mika Westerberg May 19, 2014, 8:49 a.m. UTC | #3
On Sat, May 17, 2014 at 07:48:34PM +0200, Wolfram Sang wrote:
> On Tue, Apr 29, 2014 at 11:16:09AM +0300, Mika Westerberg wrote:
> > On Mon, Apr 28, 2014 at 10:27:48PM +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 would prefer something like:
> > 
> > Say Y here if you want to enable ACPI I2C support. This includes support
> > for automatic enumeration of I2C slave devices and support for ACPI I2C
> > Operation Regions. Operation Regions allow firmware (BIOS) code to
> > access I2C slave devices, such as smart batteries through an I2C host
> > controller driver.
> > 
> > But it is really up to you so,
> > 
> > Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> How does this fit into the context of
> 55e71edb81b2b45273e7b284cce13ff24bde846f ("i2c: move ACPI helpers into
> the core")?

With that commit we moved ACPI code to live inside I2C module (given
that it was compiled as a module). However, you still can remove that
module from userspace.

With this patch we make sure that the I2C core can't be removed if you
have ACPI enabled. This prevents the potential race.
--
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 May 19, 2014, 9:44 a.m. UTC | #4
On 05/19/2014 04:49 PM, Mika Westerberg wrote:
> On Sat, May 17, 2014 at 07:48:34PM +0200, Wolfram Sang wrote:
>> On Tue, Apr 29, 2014 at 11:16:09AM +0300, Mika Westerberg wrote:
>>> On Mon, Apr 28, 2014 at 10:27:48PM +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 would prefer something like:
>>>
>>> Say Y here if you want to enable ACPI I2C support. This includes support
>>> for automatic enumeration of I2C slave devices and support for ACPI I2C
>>> Operation Regions. Operation Regions allow firmware (BIOS) code to
>>> access I2C slave devices, such as smart batteries through an I2C host
>>> controller driver.
>>>
>>> But it is really up to you so,
>>>
>>> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>>
>> How does this fit into the context of
>> 55e71edb81b2b45273e7b284cce13ff24bde846f ("i2c: move ACPI helpers into
>> the core")?
>
> With that commit we moved ACPI code to live inside I2C module (given
> that it was compiled as a module). However, you still can remove that
> module from userspace.
>
> With this patch we make sure that the I2C core can't be removed if you
> have ACPI enabled. This prevents the potential race.
>

Yes, these are the two patches' purposes. Thanks Mika's explanation. If you 
like, I can merge them into one patch.

--
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
Rafael J. Wysocki May 19, 2014, 8:23 p.m. UTC | #5
On Monday, May 19, 2014 05:44:47 PM Lan Tianyu wrote:
> On 05/19/2014 04:49 PM, Mika Westerberg wrote:
> > On Sat, May 17, 2014 at 07:48:34PM +0200, Wolfram Sang wrote:
> >> On Tue, Apr 29, 2014 at 11:16:09AM +0300, Mika Westerberg wrote:
> >>> On Mon, Apr 28, 2014 at 10:27:48PM +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 would prefer something like:
> >>>
> >>> Say Y here if you want to enable ACPI I2C support. This includes support
> >>> for automatic enumeration of I2C slave devices and support for ACPI I2C
> >>> Operation Regions. Operation Regions allow firmware (BIOS) code to
> >>> access I2C slave devices, such as smart batteries through an I2C host
> >>> controller driver.
> >>>
> >>> But it is really up to you so,
> >>>
> >>> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> >>
> >> How does this fit into the context of
> >> 55e71edb81b2b45273e7b284cce13ff24bde846f ("i2c: move ACPI helpers into
> >> the core")?
> >
> > With that commit we moved ACPI code to live inside I2C module (given
> > that it was compiled as a module). However, you still can remove that
> > module from userspace.
> >
> > With this patch we make sure that the I2C core can't be removed if you
> > have ACPI enabled. This prevents the potential race.
> >
> 
> Yes, these are the two patches' purposes. Thanks Mika's explanation. If you 
> like, I can merge them into one patch.

I think that would be easier to follow.

Thanks!
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 4b6fcdb..8c2d9b5 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);