Re: [PATCH] irqchip: LAN966X_OIC should depend on SOC_LAN966 || MFD_LAN966X_PCI

[Herve Codina]

On Thu, 25 Jul 2024 09:47:46 +0200
Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:

> Hi Hervé,
> 
> On Thu, Jul 25, 2024 at 9:27 AM Herve Codina <herve.codina@xxxxxxxxxxx> wrote:
> > On Tue, 23 Jul 2024 09:17:53 +0200
> > Geert Uytterhoeven <geert+renesas@xxxxxxxxx> wrote:  
> > > The Microchip LAN966x outband interrupt controller is only present on
> > > Microchip LAN966x SoCs.  However, when used as a PCI endpoint, all
> > > peripherals of the LAN966x SoC can be accessed by the PCI host.  Hence
> > > add dependencies on SOC_LAN966 and MFD_LAN966X_PCI, to prevent asking
> > > the user about this driver when configuring a kernel without Microchip
> > > LAN966x SoC and PCIe support.  
> >
> > I would expect a make olddefconfig silently disable LAN966X_OIC.
> > This is not the case ?  
> 
> I guess it does.  However, I never use that, as it would cause me
> to miss new symbols that I do want to enable for my target platforms.
> 
> Quoting Linus Torvals[1]:
> 
>     I use "make oldconfig" for every single machine I boot, because
> why wouldn't I?
>     Isn't that what everybody does?
> 
> "make oldconfig" is what I have been using for ages, too...
> 
> > > Fixes: 3e3a7b35332924c8 ("irqchip: Add support for LAN966x OIC")
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> > > ---
> > > The patch defining MFD_LAN966X_PCI has not been accepted yet.
> > > Hence my initial thought was to add a dependency on PCI instead, but
> > > that wouldn't make much sense, as the OIC driver cannot be used without
> > > the MFD driver anyway.  Alternatively, the MFD_LAN966X_PCI dependency
> > > could be dropped for now, requiring a follow-up patch later.
> > >
> > > "[PATCH v2 18/19] mfd: Add support for LAN966x PCI device"
> > > https://lore.kernel.org/all/20240527161450.326615-19-herve.codina@xxxxxxxxxxx/
> > > ---
> > >  drivers/irqchip/Kconfig | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> > > index bac1f0cb26e67a2b..b8d5ca3183824c93 100644
> > > --- a/drivers/irqchip/Kconfig
> > > +++ b/drivers/irqchip/Kconfig
> > > @@ -171,6 +171,7 @@ config IXP4XX_IRQ
> > >
> > >  config LAN966X_OIC
> > >       tristate "Microchip LAN966x OIC Support"
> > > +     depends on SOC_LAN966 || MFD_LAN966X_PCI || COMPILE_TEST
> > >       select GENERIC_IRQ_CHIP
> > >       select IRQ_DOMAIN
> > >       help  
> >
> > SOC_LAN966 is used only for the SOC mode of the LAN966x.
> > In that case, the LAN966x OIC driver is not used. Indeed, this
> > driver is used only in LAN966x PCI endpoint mode.  
> 
> IC. From the description in the bindings (and the commit message of
> 3e3a7b35332924c8), I was under the impression it is used for both:
> 
>   The Microchip LAN966x outband interrupt controller (OIC) maps the internal
>   interrupt sources of the LAN966x device to an external interrupt.
>   When the LAN966x device is used as a PCI device, the external interrupt is
>   routed to the PCI interrupt.
> 
> Perhaps it should be reworded, e.g.:
> 
>     The Microchip LAN966x outband interrupt controller (OIC) maps the internal
>     interrupt sources of the LAN966x device to a PCI interrupt when the LAN966x
>     device is used as a PCI device.
> 
> Or perhaps I misunderstood completely, and the internal interrupt
> sources can be mapped to a different external interrupt for other use
> cases, too?

You understood correctly.
And I agree, the description could be reworded.

> 
> > depends on MFD_LAN966X_PCI is indeed correct but, as you mentioned
> > it, patch defining MFD_LAN966X_PCI has not been accepter yet and
> > MFD_LAN966X_PCI is probably going to be renamed (the driver is
> > going to move from drivers/mfd to drivers/misc).  
> 
> IC. So a dependency on PCI is currently the best option?

Probably yes and this dependencies will have to be updated to MFD_LAN966X_PCI
or equivalent as soon as MFD_LAN966X_PCI or equivalent is available.

Best regards,
Hervé