RE: [PATCH 1/1] irqchip: gpcv2: make config option visible

From: Aisheng Dong
Date: Tue Jan 22 2019 - 07:27:20 EST


> From: Marc Zyngier [mailto:marc.zyngier@xxxxxxx]
> Sent: Tuesday, January 22, 2019 7:40 PM
> On Tue, 22 Jan 2019 11:04:48 +0000,
> Aisheng Dong <aisheng.dong@xxxxxxx> wrote:
> >
> >
> >
> > > From: Marc Zyngier [mailto:marc.zyngier@xxxxxxx]
> > > Sent: Friday, January 18, 2019 6:10 PM
> > [...]
> > > >>>
> > > >>> config IMX_GPCV2
> > > >>> - bool
> > > >>> + bool "i.MX GPCv2 IRQ chip"
> > > >>> + depends on ARCH_MXC || (COMPILE_TEST && OF)
> > > >>> select IRQ_DOMAIN
> > > >>> help
> > > >>> Enables the wakeup IRQs for IMX platforms with GPCv2 block
> > > >>>
> > > >>
> > > >> How does it help exactly? It is pretty difficult for a user to
> > > >> know exactly what they need. I'd rather see it selected by
> > > >> ARCH_MXC, which makes it
> > > >
> > > > ARM64 SoC maintainers suggest not add more driver specific options
> > > > except an Generic ARCH option.
> > > >
> > > > As GPCv2 is also used in MX8MQ. So we may select it in armv8 defconfig.
> > > > If this option is invisible, we can't select it.
> > >
> > > And conversely, users have no idea of what letter soup they have to
> > > select to make their HW work properly. Selecting the driver when the
> > > platform is supposed to be supported is the right way to solve this
> problem.
> > >
> >
> > I think the problem is that we have no platform specific CONFIGs for arm v8
> platforms.
> > We have only one CONFIG_ARCH_MXC for all MX8 SoCs, e.g. mx8qxp,
> mx8mq...
> > Only MX8MQ needs to use GPCv2. Selecting GPCv2 under ARCH_MXC means
> > users have no chance to disable it for mx8qxp which does not need it.
>
> And where is the problem to select this on platforms that do not strictly
> require it? Code bloat?

I think it's not a big problem.

>
> If you want fine grained selection for people dealing with a single SoC, make it
> depend on CONFIG_EXPERT. Don't force this on unsuspecting users who expect
> their HW to just work.
>

Seems not too necessary

> Something like:
>
> config IMX_GPCV2
> bool "i.MX GPCv2 IRQ chip" if EXPERT
> def_bool ARCH_MXC
> select IRQ_DOMAIN
> help
> Enables the wakeup IRQs for IMX platforms with GPCv2 block
>
> >
> > We probably could introduce SOC option under drivers/soc/ to do the
> > default configs selection. But we've already handled all other driver
> > selections in defconfig e.g. firmware, clk, pinctrl, power domain and etc.
> > Not sure whether GPCv2 should be an exception.
>
> I think something like the above should be the rule. Configuration feature
> creep is not helping anyone.

Got it, thanks for the suggestion.

Regards
Dong Aisheng

>
> M.
>
> --
> Jazz is not dead, it just smell funny.