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

[Geert Uytterhoeven]

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?

> 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?

[1] https://lore.kernel.org/all/CAHk-=wiqETvfxW_mG6++9uX4tY5gYbqqXsMURDw1nQy0q0qohw@xxxxxxxxxxxxxx/

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds