Messages in this thread | | | Subject | Re: [PATCH] irqchip: create a Kconfig menu for irqchip drivers | From | Marc Zyngier <> | Date | Wed, 26 Jul 2017 11:37:36 +0100 |
| |
On 26/07/17 11:18, Masahiro Yamada wrote: > Hi Marc, > > > 2017-07-26 17:04 GMT+09:00 Marc Zyngier <marc.zyngier@arm.com>: >> On 26/07/17 05:03, Masahiro Yamada wrote: >>> Some irqchip drivers have a Kconfig prompt. When we run menuconfig >>> or friends, those drivers are directly listed in the "Device Drivers" >>> menu level. This does not look nice. Create a sub-system level menu. >>> >>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> >>> --- >>> >>> drivers/irqchip/Kconfig | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig >>> index f1fd5f44d1d4..7b66313a2952 100644 >>> --- a/drivers/irqchip/Kconfig >>> +++ b/drivers/irqchip/Kconfig >>> @@ -1,3 +1,5 @@ >>> +menu "IRQ chip support" >>> + >>> config IRQCHIP >>> def_bool y >>> depends on OF_IRQ >>> @@ -306,3 +308,5 @@ config QCOM_IRQ_COMBINER >>> help >>> Say yes here to add support for the IRQ combiner devices embedded >>> in Qualcomm Technologies chips. >>> + >>> +endmenu >>> >> >> I'm very reluctant to introduce this. IMHO, interrupt controllers are >> way too low level a thing to let them be selected by the user. They >> really should be selected by the platform that needs them > > This is true for the root irqchip. > Not necessarily true for child irqchips.
I dispute that argument. We've been able to make this work so far *without* exposing yet another menu maze to the user. What has changed?
> > >> Do you have any example in mind where having a user-selectable interrupt >> controller actually makes sense on its own? > > Yes. > > I see some user-selectable drivers in drivers/irqchip/Kconfig > and I'd like to add one more for my SoCs. > > > This patch: > https://github.com/uniphier/linux/commit/f39efdf0ce34f77ae9e324d9ec6c7f486f43a0ed > > This is really optional, so > I intentionally implemented it as a platform driver > instead of IRQCHIP_DECLARE().
I really cannot see how this could be optional. It means that you could end-up in a situation where the drivers for the devices being this irqchip could have been compiled in, but not their interrupt controller. How useful is that?
> Looks like irq-ts4800.c, irq-keystone.c are modules as well.
They are directly selected by their respective defconfig. On arm64, which is what I expect you driver targets, you should simply select it in your platform entry.
M. -- Jazz is not dead. It just smells funny...
| |