| Message ID | 1407740455-21269-1-git-send-email-tianyu.lan@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, Aug 11, 2014 at 03:00:55PM +0800, Lan Tianyu wrote: > Commit da3c6647(I2C/ACPI: Clean up I2C ACPI code and Add CONFIG_I2C_ACPI > config) adds a new kernel config I2C_ACPI and make I2C core built in > when the config is selected. This is wrong because distributions > etc generally compile I2C as a module and the commit broken that. > This patch is to make I2C core able to be a module when I2C_ACPI is > selected. Original issue the commit da3c6647 tried to avoid will > be fixed in ACPICA and it's rarely triggered during unloading module. > > Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> I wonder if we can do depends on I2C=y here? If I understand it right, then we only build the ACPI_I2C if I2C is compiled into the kernel. That way the problem da3c6647 tried to solve doens't re-appear. We can later on relax this once ACPICA has been fixed. Thoughts? -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 12, 2014 at 12:53:21PM +0300, Mika Westerberg wrote: > On Mon, Aug 11, 2014 at 03:00:55PM +0800, Lan Tianyu wrote: > > Commit da3c6647(I2C/ACPI: Clean up I2C ACPI code and Add CONFIG_I2C_ACPI > > config) adds a new kernel config I2C_ACPI and make I2C core built in > > when the config is selected. This is wrong because distributions > > etc generally compile I2C as a module and the commit broken that. > > This patch is to make I2C core able to be a module when I2C_ACPI is > > selected. Original issue the commit da3c6647 tried to avoid will > > be fixed in ACPICA and it's rarely triggered during unloading module. > > > > Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> > > I wonder if we can do > > depends on I2C=y > > here? If I understand it right, then we only build the ACPI_I2C if I2C > is compiled into the kernel. That way the problem da3c6647 tried to > solve doens't re-appear. > > We can later on relax this once ACPICA has been fixed. Thoughts? I had the same idea yet my travel to Chicago interrupted thinking about it further. Once I get rid of my jetlag, I'll have a closer look. Unless you already came up with the perfect solution until then, of course ;)
On 2014年08月13日 10:03, Wolfram Sang wrote: > On Tue, Aug 12, 2014 at 12:53:21PM +0300, Mika Westerberg wrote: >> On Mon, Aug 11, 2014 at 03:00:55PM +0800, Lan Tianyu wrote: >>> Commit da3c6647(I2C/ACPI: Clean up I2C ACPI code and Add CONFIG_I2C_ACPI >>> config) adds a new kernel config I2C_ACPI and make I2C core built in >>> when the config is selected. This is wrong because distributions >>> etc generally compile I2C as a module and the commit broken that. >>> This patch is to make I2C core able to be a module when I2C_ACPI is >>> selected. Original issue the commit da3c6647 tried to avoid will >>> be fixed in ACPICA and it's rarely triggered during unloading module. >>> >>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> >> >> I wonder if we can do >> >> depends on I2C=y >> >> here? If I understand it right, then we only build the ACPI_I2C if I2C >> is compiled into the kernel. That way the problem da3c6647 tried to >> solve doens't re-appear. >> >> We can later on relax this once ACPICA has been fixed. Thoughts? > > I had the same idea yet my travel to Chicago interrupted thinking about > it further. Once I get rid of my jetlag, I'll have a closer look. Unless > you already came up with the perfect solution until then, of course ;) > Hi Mika & Wolfram: I have one concern about "depends on I2C=y". If distribution config file selects I2C core as a module, the original code can enumerate I2C slave devices from ACPI table. But now I2C_ACPI depends on I2C core built in, the I2C module can't enumerate devices from ACPI table. This maybe a regression for distribution?
On Wed, Aug 13, 2014 at 10:50:32AM +0800, Lan Tianyu wrote: > On 2014年08月13日 10:03, Wolfram Sang wrote: > > On Tue, Aug 12, 2014 at 12:53:21PM +0300, Mika Westerberg wrote: > >> On Mon, Aug 11, 2014 at 03:00:55PM +0800, Lan Tianyu wrote: > >>> Commit da3c6647(I2C/ACPI: Clean up I2C ACPI code and Add CONFIG_I2C_ACPI > >>> config) adds a new kernel config I2C_ACPI and make I2C core built in > >>> when the config is selected. This is wrong because distributions > >>> etc generally compile I2C as a module and the commit broken that. > >>> This patch is to make I2C core able to be a module when I2C_ACPI is > >>> selected. Original issue the commit da3c6647 tried to avoid will > >>> be fixed in ACPICA and it's rarely triggered during unloading module. > >>> > >>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> > >> > >> I wonder if we can do > >> > >> depends on I2C=y > >> > >> here? If I understand it right, then we only build the ACPI_I2C if I2C > >> is compiled into the kernel. That way the problem da3c6647 tried to > >> solve doens't re-appear. > >> > >> We can later on relax this once ACPICA has been fixed. Thoughts? > > > > I had the same idea yet my travel to Chicago interrupted thinking about > > it further. Once I get rid of my jetlag, I'll have a closer look. Unless > > you already came up with the perfect solution until then, of course ;) > > > > Hi Mika & Wolfram: > I have one concern about "depends on I2C=y". If distribution config > file selects I2C core as a module, the original code can enumerate I2C > slave devices from ACPI table. But now I2C_ACPI depends on I2C core > built in, the I2C module can't enumerate devices from ACPI table. This > maybe a regression for distribution? True, but only the I2C OpRegion parts needs to have I2C=y. Does it make sense to name ACPI_I2C to ACPI_I2C_OPREGION (or something like that) and only enable it when I2C=y? Then we would have ACPI I2C enumeration still in place. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2014年08月13日 15:07, Mika Westerberg wrote: > On Wed, Aug 13, 2014 at 10:50:32AM +0800, Lan Tianyu wrote: >> On 2014年08月13日 10:03, Wolfram Sang wrote: >>> On Tue, Aug 12, 2014 at 12:53:21PM +0300, Mika Westerberg wrote: >>>> On Mon, Aug 11, 2014 at 03:00:55PM +0800, Lan Tianyu wrote: >>>>> Commit da3c6647(I2C/ACPI: Clean up I2C ACPI code and Add CONFIG_I2C_ACPI >>>>> config) adds a new kernel config I2C_ACPI and make I2C core built in >>>>> when the config is selected. This is wrong because distributions >>>>> etc generally compile I2C as a module and the commit broken that. >>>>> This patch is to make I2C core able to be a module when I2C_ACPI is >>>>> selected. Original issue the commit da3c6647 tried to avoid will >>>>> be fixed in ACPICA and it's rarely triggered during unloading module. >>>>> >>>>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> >>>> >>>> I wonder if we can do >>>> >>>> depends on I2C=y >>>> >>>> here? If I understand it right, then we only build the ACPI_I2C if I2C >>>> is compiled into the kernel. That way the problem da3c6647 tried to >>>> solve doens't re-appear. >>>> >>>> We can later on relax this once ACPICA has been fixed. Thoughts? >>> >>> I had the same idea yet my travel to Chicago interrupted thinking about >>> it further. Once I get rid of my jetlag, I'll have a closer look. Unless >>> you already came up with the perfect solution until then, of course ;) >>> >> >> Hi Mika & Wolfram: >> I have one concern about "depends on I2C=y". If distribution config >> file selects I2C core as a module, the original code can enumerate I2C >> slave devices from ACPI table. But now I2C_ACPI depends on I2C core >> built in, the I2C module can't enumerate devices from ACPI table. This >> maybe a regression for distribution? > > True, but only the I2C OpRegion parts needs to have I2C=y. Does it make > sense to name ACPI_I2C to ACPI_I2C_OPREGION (or something like that) > and only enable it when I2C=y? Then we would have ACPI I2C enumeration > still in place. > Yes, this makes sense to me. I will rewrite the patch.
> > True, but only the I2C OpRegion parts needs to have I2C=y. Does it make > > sense to name ACPI_I2C to ACPI_I2C_OPREGION (or something like that) > > and only enable it when I2C=y? Then we would have ACPI I2C enumeration > > still in place. > > > > Yes, this makes sense to me. I will rewrite the patch. Ping. I'd like to send the fix to Linus during the merge-window. Thanks, Wolfram
diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig index 3e3b680..436ba27 100644 --- a/drivers/i2c/Kconfig +++ b/drivers/i2c/Kconfig @@ -2,9 +2,7 @@ # I2C subsystem configuration # -menu "I2C support" - -config I2C +menuconfig I2C tristate "I2C support" select RT_MUTEXES ---help--- @@ -25,8 +23,7 @@ config I2C config I2C_ACPI bool "I2C ACPI support" - select I2C - depends on ACPI + depends on I2C && ACPI default y help Say Y here if you want to enable ACPI I2C support. This includes support @@ -139,4 +136,3 @@ config I2C_DEBUG_BUS endif # I2C -endmenu
Commit da3c6647(I2C/ACPI: Clean up I2C ACPI code and Add CONFIG_I2C_ACPI config) adds a new kernel config I2C_ACPI and make I2C core built in when the config is selected. This is wrong because distributions etc generally compile I2C as a module and the commit broken that. This patch is to make I2C core able to be a module when I2C_ACPI is selected. Original issue the commit da3c6647 tried to avoid will be fixed in ACPICA and it's rarely triggered during unloading module. Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> --- drivers/i2c/Kconfig | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)