| Message ID | 20200510095019.20981-8-Sergey.Semin@baikalelectronics.ru (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | i2c: designeware: Add Baikal-T1 System I2C support | expand |
On 5/10/20 12:50 PM, Serge Semin wrote: > Currently Intel Baytrail I2C semaphore is a feature of the DW APB I2C > platform driver. It's a bit confusing to see it's config in the menu at > some separated place with no reference to the platform code. Lets move the > config definition under the if-I2C_DESIGNWARE_PLATFORM clause. By doing so > the config menu will display the feature right below the DW I2C platform > driver item and will indent it to the right so signifying its belonging. > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de> > Cc: Paul Burton <paulburton@kernel.org> > Cc: Ralf Baechle <ralf@linux-mips.org> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Mika Westerberg <mika.westerberg@linux.intel.com> > Cc: Wolfram Sang <wsa@the-dreams.de> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Frank Rowand <frowand.list@gmail.com> > Cc: linux-mips@vger.kernel.org > Cc: devicetree@vger.kernel.org > --- > drivers/i2c/busses/Kconfig | 30 +++++++++++++++++------------- > 1 file changed, 17 insertions(+), 13 deletions(-) > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > index 368aa64e9266..ed6927c4c540 100644 > --- a/drivers/i2c/busses/Kconfig > +++ b/drivers/i2c/busses/Kconfig > @@ -530,8 +530,8 @@ config I2C_DESIGNWARE_CORE > > config I2C_DESIGNWARE_PLATFORM > tristate "Synopsys DesignWare Platform" > - select I2C_DESIGNWARE_CORE > depends on (ACPI && COMMON_CLK) || !ACPI > + select I2C_DESIGNWARE_CORE > help > If you say yes to this option, support will be included for the > Synopsys DesignWare I2C adapter. > @@ -539,6 +539,22 @@ config I2C_DESIGNWARE_PLATFORM > This driver can also be built as a module. If so, the module > will be called i2c-designware-platform. > > +if I2C_DESIGNWARE_PLATFORM > + > +config I2C_DESIGNWARE_BAYTRAIL > + bool "Intel Baytrail I2C semaphore support" > + depends on ACPI > + depends on (I2C_DESIGNWARE_PLATFORM=m && IOSF_MBI) || \ > + (I2C_DESIGNWARE_PLATFORM=y && IOSF_MBI=y) > + help > + This driver enables managed host access to the PMIC I2C bus on select > + Intel BayTrail platforms using the X-Powers AXP288 PMIC. It allows > + the host to request uninterrupted access to the PMIC's I2C bus from > + the platform firmware controlling it. You should say Y if running on > + a BayTrail system using the AXP288. > + > +endif # I2C_DESIGNWARE_PLATFORM > + Is the added "if I2C_DESIGNWARE_PLATFORM" needed here? Should the "depends on" be enough? Jarkko
On Wed, May 20, 2020 at 03:16:14PM +0300, Jarkko Nikula wrote: > On 5/10/20 12:50 PM, Serge Semin wrote: > > Currently Intel Baytrail I2C semaphore is a feature of the DW APB I2C > > platform driver. It's a bit confusing to see it's config in the menu at > > some separated place with no reference to the platform code. Lets move the > > config definition under the if-I2C_DESIGNWARE_PLATFORM clause. By doing so > > the config menu will display the feature right below the DW I2C platform > > driver item and will indent it to the right so signifying its belonging. > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru> > > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de> > > Cc: Paul Burton <paulburton@kernel.org> > > Cc: Ralf Baechle <ralf@linux-mips.org> > > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Cc: Mika Westerberg <mika.westerberg@linux.intel.com> > > Cc: Wolfram Sang <wsa@the-dreams.de> > > Cc: Rob Herring <robh+dt@kernel.org> > > Cc: Frank Rowand <frowand.list@gmail.com> > > Cc: linux-mips@vger.kernel.org > > Cc: devicetree@vger.kernel.org > > --- > > drivers/i2c/busses/Kconfig | 30 +++++++++++++++++------------- > > 1 file changed, 17 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > > index 368aa64e9266..ed6927c4c540 100644 > > --- a/drivers/i2c/busses/Kconfig > > +++ b/drivers/i2c/busses/Kconfig > > @@ -530,8 +530,8 @@ config I2C_DESIGNWARE_CORE > > config I2C_DESIGNWARE_PLATFORM > > tristate "Synopsys DesignWare Platform" > > - select I2C_DESIGNWARE_CORE > > depends on (ACPI && COMMON_CLK) || !ACPI > > + select I2C_DESIGNWARE_CORE > > help > > If you say yes to this option, support will be included for the > > Synopsys DesignWare I2C adapter. > > @@ -539,6 +539,22 @@ config I2C_DESIGNWARE_PLATFORM > > This driver can also be built as a module. If so, the module > > will be called i2c-designware-platform. > > +if I2C_DESIGNWARE_PLATFORM > > + > > +config I2C_DESIGNWARE_BAYTRAIL > > + bool "Intel Baytrail I2C semaphore support" > > + depends on ACPI > > + depends on (I2C_DESIGNWARE_PLATFORM=m && IOSF_MBI) || \ > > + (I2C_DESIGNWARE_PLATFORM=y && IOSF_MBI=y) > > + help > > + This driver enables managed host access to the PMIC I2C bus on select > > + Intel BayTrail platforms using the X-Powers AXP288 PMIC. It allows > > + the host to request uninterrupted access to the PMIC's I2C bus from > > + the platform firmware controlling it. You should say Y if running on > > + a BayTrail system using the AXP288. > > + > > +endif # I2C_DESIGNWARE_PLATFORM > > + > > Is the added "if I2C_DESIGNWARE_PLATFORM" needed here? Should the "depends > on" be enough? The idea was to add if-endif clause here for features possibly added sometime in future. But using normal "depends on I2C_DESIGNWARE_PLATFORM" shall make the config depicted as an indented sub-config as well. Would you like me to remove the if-clause and use the depends on operator instead? -Sergey > > Jarkko
On 5/21/20 5:22 AM, Serge Semin wrote: > On Wed, May 20, 2020 at 03:16:14PM +0300, Jarkko Nikula wrote: >> On 5/10/20 12:50 PM, Serge Semin wrote: >>> Currently Intel Baytrail I2C semaphore is a feature of the DW APB I2C >>> platform driver. It's a bit confusing to see it's config in the menu at >>> some separated place with no reference to the platform code. Lets move the >>> config definition under the if-I2C_DESIGNWARE_PLATFORM clause. By doing so >>> the config menu will display the feature right below the DW I2C platform >>> driver item and will indent it to the right so signifying its belonging. >>> >>> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> >>> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru> >>> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de> >>> Cc: Paul Burton <paulburton@kernel.org> >>> Cc: Ralf Baechle <ralf@linux-mips.org> >>> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >>> Cc: Mika Westerberg <mika.westerberg@linux.intel.com> >>> Cc: Wolfram Sang <wsa@the-dreams.de> >>> Cc: Rob Herring <robh+dt@kernel.org> >>> Cc: Frank Rowand <frowand.list@gmail.com> >>> Cc: linux-mips@vger.kernel.org >>> Cc: devicetree@vger.kernel.org >>> --- >>> drivers/i2c/busses/Kconfig | 30 +++++++++++++++++------------- >>> 1 file changed, 17 insertions(+), 13 deletions(-) >>> >>> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig >>> index 368aa64e9266..ed6927c4c540 100644 >>> --- a/drivers/i2c/busses/Kconfig >>> +++ b/drivers/i2c/busses/Kconfig >>> @@ -530,8 +530,8 @@ config I2C_DESIGNWARE_CORE >>> config I2C_DESIGNWARE_PLATFORM >>> tristate "Synopsys DesignWare Platform" >>> - select I2C_DESIGNWARE_CORE >>> depends on (ACPI && COMMON_CLK) || !ACPI >>> + select I2C_DESIGNWARE_CORE >>> help >>> If you say yes to this option, support will be included for the >>> Synopsys DesignWare I2C adapter. >>> @@ -539,6 +539,22 @@ config I2C_DESIGNWARE_PLATFORM >>> This driver can also be built as a module. If so, the module >>> will be called i2c-designware-platform. >>> +if I2C_DESIGNWARE_PLATFORM >>> + >>> +config I2C_DESIGNWARE_BAYTRAIL >>> + bool "Intel Baytrail I2C semaphore support" >>> + depends on ACPI >>> + depends on (I2C_DESIGNWARE_PLATFORM=m && IOSF_MBI) || \ >>> + (I2C_DESIGNWARE_PLATFORM=y && IOSF_MBI=y) >>> + help >>> + This driver enables managed host access to the PMIC I2C bus on select >>> + Intel BayTrail platforms using the X-Powers AXP288 PMIC. It allows >>> + the host to request uninterrupted access to the PMIC's I2C bus from >>> + the platform firmware controlling it. You should say Y if running on >>> + a BayTrail system using the AXP288. >>> + >>> +endif # I2C_DESIGNWARE_PLATFORM >>> + >> >> Is the added "if I2C_DESIGNWARE_PLATFORM" needed here? Should the "depends >> on" be enough? > > The idea was to add if-endif clause here for features possibly added sometime > in future. But using normal "depends on I2C_DESIGNWARE_PLATFORM" shall make > the config depicted as an indented sub-config as well. Would you like me to > remove the if-clause and use the depends on operator instead? > Yes, please remove it from this patch. Keeps this patch simpler and if some future feature needs it then that patch(set) is the right place to add it. Jarkko
On Mon, May 25, 2020 at 04:01:26PM +0300, Jarkko Nikula wrote: > On 5/21/20 5:22 AM, Serge Semin wrote: > > On Wed, May 20, 2020 at 03:16:14PM +0300, Jarkko Nikula wrote: > > > On 5/10/20 12:50 PM, Serge Semin wrote: > > > > Currently Intel Baytrail I2C semaphore is a feature of the DW APB I2C > > > > platform driver. It's a bit confusing to see it's config in the menu at > > > > some separated place with no reference to the platform code. Lets move the > > > > config definition under the if-I2C_DESIGNWARE_PLATFORM clause. By doing so > > > > the config menu will display the feature right below the DW I2C platform > > > > driver item and will indent it to the right so signifying its belonging. > > > > > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > > > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru> > > > > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de> > > > > Cc: Paul Burton <paulburton@kernel.org> > > > > Cc: Ralf Baechle <ralf@linux-mips.org> > > > > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > > Cc: Mika Westerberg <mika.westerberg@linux.intel.com> > > > > Cc: Wolfram Sang <wsa@the-dreams.de> > > > > Cc: Rob Herring <robh+dt@kernel.org> > > > > Cc: Frank Rowand <frowand.list@gmail.com> > > > > Cc: linux-mips@vger.kernel.org > > > > Cc: devicetree@vger.kernel.org > > > > --- > > > > drivers/i2c/busses/Kconfig | 30 +++++++++++++++++------------- > > > > 1 file changed, 17 insertions(+), 13 deletions(-) > > > > > > > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > > > > index 368aa64e9266..ed6927c4c540 100644 > > > > --- a/drivers/i2c/busses/Kconfig > > > > +++ b/drivers/i2c/busses/Kconfig > > > > @@ -530,8 +530,8 @@ config I2C_DESIGNWARE_CORE > > > > config I2C_DESIGNWARE_PLATFORM > > > > tristate "Synopsys DesignWare Platform" > > > > - select I2C_DESIGNWARE_CORE > > > > depends on (ACPI && COMMON_CLK) || !ACPI > > > > + select I2C_DESIGNWARE_CORE > > > > help > > > > If you say yes to this option, support will be included for the > > > > Synopsys DesignWare I2C adapter. > > > > @@ -539,6 +539,22 @@ config I2C_DESIGNWARE_PLATFORM > > > > This driver can also be built as a module. If so, the module > > > > will be called i2c-designware-platform. > > > > +if I2C_DESIGNWARE_PLATFORM > > > > + > > > > +config I2C_DESIGNWARE_BAYTRAIL > > > > + bool "Intel Baytrail I2C semaphore support" > > > > + depends on ACPI > > > > + depends on (I2C_DESIGNWARE_PLATFORM=m && IOSF_MBI) || \ > > > > + (I2C_DESIGNWARE_PLATFORM=y && IOSF_MBI=y) > > > > + help > > > > + This driver enables managed host access to the PMIC I2C bus on select > > > > + Intel BayTrail platforms using the X-Powers AXP288 PMIC. It allows > > > > + the host to request uninterrupted access to the PMIC's I2C bus from > > > > + the platform firmware controlling it. You should say Y if running on > > > > + a BayTrail system using the AXP288. > > > > + > > > > +endif # I2C_DESIGNWARE_PLATFORM > > > > + > > > > > > Is the added "if I2C_DESIGNWARE_PLATFORM" needed here? Should the "depends > > > on" be enough? > > > > The idea was to add if-endif clause here for features possibly added sometime > > in future. But using normal "depends on I2C_DESIGNWARE_PLATFORM" shall make > > the config depicted as an indented sub-config as well. Would you like me to > > remove the if-clause and use the depends on operator instead? > > > Yes, please remove it from this patch. Keeps this patch simpler and if some > future feature needs it then that patch(set) is the right place to add it. Agreed. I'll do this in v3. -Sergey > > Jarkko
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 368aa64e9266..ed6927c4c540 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -530,8 +530,8 @@ config I2C_DESIGNWARE_CORE config I2C_DESIGNWARE_PLATFORM tristate "Synopsys DesignWare Platform" - select I2C_DESIGNWARE_CORE depends on (ACPI && COMMON_CLK) || !ACPI + select I2C_DESIGNWARE_CORE help If you say yes to this option, support will be included for the Synopsys DesignWare I2C adapter. @@ -539,6 +539,22 @@ config I2C_DESIGNWARE_PLATFORM This driver can also be built as a module. If so, the module will be called i2c-designware-platform. +if I2C_DESIGNWARE_PLATFORM + +config I2C_DESIGNWARE_BAYTRAIL + bool "Intel Baytrail I2C semaphore support" + depends on ACPI + depends on (I2C_DESIGNWARE_PLATFORM=m && IOSF_MBI) || \ + (I2C_DESIGNWARE_PLATFORM=y && IOSF_MBI=y) + help + This driver enables managed host access to the PMIC I2C bus on select + Intel BayTrail platforms using the X-Powers AXP288 PMIC. It allows + the host to request uninterrupted access to the PMIC's I2C bus from + the platform firmware controlling it. You should say Y if running on + a BayTrail system using the AXP288. + +endif # I2C_DESIGNWARE_PLATFORM + config I2C_DESIGNWARE_SLAVE bool "Synopsys DesignWare Slave" depends on I2C_DESIGNWARE_CORE @@ -561,18 +577,6 @@ config I2C_DESIGNWARE_PCI This driver can also be built as a module. If so, the module will be called i2c-designware-pci. -config I2C_DESIGNWARE_BAYTRAIL - bool "Intel Baytrail I2C semaphore support" - depends on ACPI - depends on (I2C_DESIGNWARE_PLATFORM=m && IOSF_MBI) || \ - (I2C_DESIGNWARE_PLATFORM=y && IOSF_MBI=y) - help - This driver enables managed host access to the PMIC I2C bus on select - Intel BayTrail platforms using the X-Powers AXP288 PMIC. It allows - the host to request uninterrupted access to the PMIC's I2C bus from - the platform firmware controlling it. You should say Y if running on - a BayTrail system using the AXP288. - config I2C_DIGICOLOR tristate "Conexant Digicolor I2C driver" depends on ARCH_DIGICOLOR || COMPILE_TEST
Currently Intel Baytrail I2C semaphore is a feature of the DW APB I2C platform driver. It's a bit confusing to see it's config in the menu at some separated place with no reference to the platform code. Lets move the config definition under the if-I2C_DESIGNWARE_PLATFORM clause. By doing so the config menu will display the feature right below the DW I2C platform driver item and will indent it to the right so signifying its belonging. Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de> Cc: Paul Burton <paulburton@kernel.org> Cc: Ralf Baechle <ralf@linux-mips.org> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Cc: Mika Westerberg <mika.westerberg@linux.intel.com> Cc: Wolfram Sang <wsa@the-dreams.de> Cc: Rob Herring <robh+dt@kernel.org> Cc: Frank Rowand <frowand.list@gmail.com> Cc: linux-mips@vger.kernel.org Cc: devicetree@vger.kernel.org --- drivers/i2c/busses/Kconfig | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-)