irqchip: LAN966X_OIC should depend on SOC_LAN966 || MFD_LAN966X_PCI

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

Posted by Geert Uytterhoeven 2 months, 3 weeks ago

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.

Fixes: 3e3a7b35332924c8 ("irqchip: Add support for LAN966x OIC")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
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@bootlin.com/
---
 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
-- 
2.34.1

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

Posted by Herve Codina 2 months, 3 weeks ago

Hi Geert,

On Tue, 23 Jul 2024 09:17:53 +0200
Geert Uytterhoeven <geert+renesas@glider.be> 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 ?

> 
> Fixes: 3e3a7b35332924c8 ("irqchip: Add support for LAN966x OIC")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> 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@bootlin.com/
> ---
>  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.

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).


Best regards,
Hervé

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

Posted by Geert Uytterhoeven 2 months, 3 weeks ago

Hi Hervé,

On Thu, Jul 25, 2024 at 9:27 AM Herve Codina <herve.codina@bootlin.com> wrote:
> On Tue, 23 Jul 2024 09:17:53 +0200
> Geert Uytterhoeven <geert+renesas@glider.be> 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@glider.be>
> > ---
> > 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@bootlin.com/
> > ---
> >  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@mail.gmail.com/

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

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

Posted by Herve Codina 2 months, 3 weeks ago

On Thu, 25 Jul 2024 09:47:46 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Hervé,
> 
> On Thu, Jul 25, 2024 at 9:27 AM Herve Codina <herve.codina@bootlin.com> wrote:
> > On Tue, 23 Jul 2024 09:17:53 +0200
> > Geert Uytterhoeven <geert+renesas@glider.be> 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@glider.be>
> > > ---
> > > 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@bootlin.com/
> > > ---
> > >  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é