| Message ID | 20180111083600.4121805-1-arnd@arndb.de |
|---|---|
| State | New |
| Headers | show |
| Series | gpio: winbond: fix ISA_BUS_API dependency | expand |
On Thu, Jan 11, 2018 at 9:35 AM, Arnd Bergmann <arnd@arndb.de> wrote: > The newly added GPIO driver for winbond chipsets causes a > circular dependency warning in Kconfig: > > drivers/gpio/Kconfig:13:error: recursive dependency detected! > drivers/gpio/Kconfig:13: symbol GPIOLIB is selected by STX104 > drivers/iio/adc/Kconfig:699: symbol STX104 depends on ISA_BUS_API > arch/Kconfig:830: symbol ISA_BUS_API is selected by GPIO_WINBOND > drivers/gpio/Kconfig:701: symbol GPIO_WINBOND depends on GPIOLIB > > The underlying problem is that ISA_BUS_API is not meant to be selected by > device drivers, instead it is provided by the architectures that support > ISA add-on card devices, or in case of x86 have this explicitly enabled. > > This particular driver appears to be different from the other ISA_BUS_API > based drivers, in that it is not normally an add-on card (ISA or PC104) > but instead is an LPC-attached component on the mainboard. We already > support other functionality provided by this chip, at least > drivers/watchdog/w83627hf_wdt.c and drivers/hwmon/w83627ehf.c, plus > there is a discovery function for this hardware in > drivers/parport/parport_pc.c. > > If we want to use this driver without having to enable CONFIG_EXPERT, > it might be better to not use the isa_bus_type for it, but rather > turn it into a platform_driver, acpi_driver or add an MFD for it that > is shared with the wdt and hwmon portions and does the probing. > > For now, this patch fixes the dependency by changing 'select' into > 'depends on'. > > Cc: William Breathitt Gray <vilhelm.gray@gmail.com> > Cc: Guenter Roeck <linux@roeck-us.net> > Cc: Maciej S. Szmigiero <mail@maciej.szmigiero.name> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Fixes: a0d65009411c ("gpio: winbond: Add driver") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> I've tentatively applied this patch so we don't break the builds. We had a bunch of discussion about this already ... I guess we need to go at it again, waiting for comments from Maciej and William. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11.01.2018 10:18, Linus Walleij wrote: > On Thu, Jan 11, 2018 at 9:35 AM, Arnd Bergmann <arnd@arndb.de> wrote: > >> The newly added GPIO driver for winbond chipsets causes a >> circular dependency warning in Kconfig: >> >> drivers/gpio/Kconfig:13:error: recursive dependency detected! >> drivers/gpio/Kconfig:13: symbol GPIOLIB is selected by STX104 >> drivers/iio/adc/Kconfig:699: symbol STX104 depends on ISA_BUS_API >> arch/Kconfig:830: symbol ISA_BUS_API is selected by GPIO_WINBOND >> drivers/gpio/Kconfig:701: symbol GPIO_WINBOND depends on GPIOLIB >> >> The underlying problem is that ISA_BUS_API is not meant to be selected by >> device drivers, instead it is provided by the architectures that support >> ISA add-on card devices, or in case of x86 have this explicitly enabled. >> >> This particular driver appears to be different from the other ISA_BUS_API >> based drivers, in that it is not normally an add-on card (ISA or PC104) >> but instead is an LPC-attached component on the mainboard. We already >> support other functionality provided by this chip, at least >> drivers/watchdog/w83627hf_wdt.c and drivers/hwmon/w83627ehf.c, plus >> there is a discovery function for this hardware in >> drivers/parport/parport_pc.c. >> >> If we want to use this driver without having to enable CONFIG_EXPERT, >> it might be better to not use the isa_bus_type for it, but rather >> turn it into a platform_driver, acpi_driver or add an MFD for it that >> is shared with the wdt and hwmon portions and does the probing. >> >> For now, this patch fixes the dependency by changing 'select' into >> 'depends on'. >> >> Cc: William Breathitt Gray <vilhelm.gray@gmail.com> >> Cc: Guenter Roeck <linux@roeck-us.net> >> Cc: Maciej S. Szmigiero <mail@maciej.szmigiero.name> >> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >> Fixes: a0d65009411c ("gpio: winbond: Add driver") >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > I've tentatively applied this patch so we don't break the builds. > > We had a bunch of discussion about this already ... I guess we > need to go at it again, waiting for comments from Maciej and > William. Hmm, you didn't finally pick William's ISA_BUS_API changes via your gpio tree? (sorry for not spotting this when this driver was merged) These changes make ISA_BUS_API selectable by drivers since it isn't really a bus support (which is enabled by ISA_BUS), but it is more like a library code. > Yours, > Linus Walleij > Best regards, Maciej Szmigiero -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 11, 2018 at 10:18:56AM +0100, Linus Walleij wrote: >On Thu, Jan 11, 2018 at 9:35 AM, Arnd Bergmann <arnd@arndb.de> wrote: > >> The newly added GPIO driver for winbond chipsets causes a >> circular dependency warning in Kconfig: >> >> drivers/gpio/Kconfig:13:error: recursive dependency detected! >> drivers/gpio/Kconfig:13: symbol GPIOLIB is selected by STX104 >> drivers/iio/adc/Kconfig:699: symbol STX104 depends on ISA_BUS_API >> arch/Kconfig:830: symbol ISA_BUS_API is selected by GPIO_WINBOND >> drivers/gpio/Kconfig:701: symbol GPIO_WINBOND depends on GPIOLIB >> >> The underlying problem is that ISA_BUS_API is not meant to be selected by >> device drivers, instead it is provided by the architectures that support >> ISA add-on card devices, or in case of x86 have this explicitly enabled. >> >> This particular driver appears to be different from the other ISA_BUS_API >> based drivers, in that it is not normally an add-on card (ISA or PC104) >> but instead is an LPC-attached component on the mainboard. We already >> support other functionality provided by this chip, at least >> drivers/watchdog/w83627hf_wdt.c and drivers/hwmon/w83627ehf.c, plus >> there is a discovery function for this hardware in >> drivers/parport/parport_pc.c. >> >> If we want to use this driver without having to enable CONFIG_EXPERT, >> it might be better to not use the isa_bus_type for it, but rather >> turn it into a platform_driver, acpi_driver or add an MFD for it that >> is shared with the wdt and hwmon portions and does the probing. >> >> For now, this patch fixes the dependency by changing 'select' into >> 'depends on'. >> >> Cc: William Breathitt Gray <vilhelm.gray@gmail.com> >> Cc: Guenter Roeck <linux@roeck-us.net> >> Cc: Maciej S. Szmigiero <mail@maciej.szmigiero.name> >> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >> Fixes: a0d65009411c ("gpio: winbond: Add driver") >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> > >I've tentatively applied this patch so we don't break the builds. > >We had a bunch of discussion about this already ... I guess we >need to go at it again, waiting for comments from Maciej and >William. > >Yours, >Linus Walleij Hi Linus, I don't believe this patch is necessary if you pull in the "Change ISA_BUS_API dependency to selection" patchset (https://patchwork.ozlabs.org/project/linux-gpio/list/?series=20657) as it has the proper fix for this recursive dependency. William Breathitt Gray -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 11, 2018 at 09:35:15AM +0100, Arnd Bergmann wrote: >This particular driver appears to be different from the other ISA_BUS_API >based drivers, in that it is not normally an add-on card (ISA or PC104) >but instead is an LPC-attached component on the mainboard. We already >support other functionality provided by this chip, at least >drivers/watchdog/w83627hf_wdt.c and drivers/hwmon/w83627ehf.c, plus >there is a discovery function for this hardware in >drivers/parport/parport_pc.c. > >If we want to use this driver without having to enable CONFIG_EXPERT, >it might be better to not use the isa_bus_type for it, but rather >turn it into a platform_driver, acpi_driver or add an MFD for it that >is shared with the wdt and hwmon portions and does the probing. I think there is some merit to reinvestigate an MFD driver at a later point; Super I/O chips by their design typically control various multiple devices, which I believe fits in well with the MFD paradigm. Although other existing Super I/O drivers in the kernel do not make use of MFD, this doesn't necessarily mean it is not apt -- it may be appropriate to refactor those drivers as well to take advantage of MFD. As far as this particular patch is concerned however, I'm going to suggest waiting for the ISA_BUS_API Kconfig fixes ((https://patchwork.ozlabs.org/project/linux-gpio/list/?series=20657) to be pulled-in; applying this patch now will just require a revert later once the patchset makes it into the tree. William Breathitt Gray -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" 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/gpio/Kconfig b/drivers/gpio/Kconfig index 440af077cc76..8dbb2280538d 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -700,7 +700,7 @@ config GPIO_TS5500 config GPIO_WINBOND tristate "Winbond Super I/O GPIO support" - select ISA_BUS_API + depends on ISA_BUS_API help This option enables support for GPIOs found on Winbond Super I/O chips.
The newly added GPIO driver for winbond chipsets causes a circular dependency warning in Kconfig: drivers/gpio/Kconfig:13:error: recursive dependency detected! drivers/gpio/Kconfig:13: symbol GPIOLIB is selected by STX104 drivers/iio/adc/Kconfig:699: symbol STX104 depends on ISA_BUS_API arch/Kconfig:830: symbol ISA_BUS_API is selected by GPIO_WINBOND drivers/gpio/Kconfig:701: symbol GPIO_WINBOND depends on GPIOLIB The underlying problem is that ISA_BUS_API is not meant to be selected by device drivers, instead it is provided by the architectures that support ISA add-on card devices, or in case of x86 have this explicitly enabled. This particular driver appears to be different from the other ISA_BUS_API based drivers, in that it is not normally an add-on card (ISA or PC104) but instead is an LPC-attached component on the mainboard. We already support other functionality provided by this chip, at least drivers/watchdog/w83627hf_wdt.c and drivers/hwmon/w83627ehf.c, plus there is a discovery function for this hardware in drivers/parport/parport_pc.c. If we want to use this driver without having to enable CONFIG_EXPERT, it might be better to not use the isa_bus_type for it, but rather turn it into a platform_driver, acpi_driver or add an MFD for it that is shared with the wdt and hwmon portions and does the probing. For now, this patch fixes the dependency by changing 'select' into 'depends on'. Cc: William Breathitt Gray <vilhelm.gray@gmail.com> Cc: Guenter Roeck <linux@roeck-us.net> Cc: Maciej S. Szmigiero <mail@maciej.szmigiero.name> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Fixes: a0d65009411c ("gpio: winbond: Add driver") Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/gpio/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)