Re: [PATCH v9 1/6] misc: Add support for LAN966x PCI device

Geert Uytterhoeven
2024-11-29T08:44:26Z

On Fri, Nov 29, 2024 at 9:25 AM Arnd Bergmann <arnd@arndb.de> wrote:
> On Fri, Nov 29, 2024, at 09:10, Herve Codina wrote:
> > On Thu, 28 Nov 2024 20:42:53 +0100
> > Michal Kubecek <mkubecek@suse.cz> wrote:
> >> > --- a/drivers/misc/Kconfig
> >> > +++ b/drivers/misc/Kconfig
> >> > @@ -610,6 +610,30 @@ config MARVELL_CN10K_DPI
> >> > To compile this driver as a module, choose M here: the module
> >> > will be called mrvl_cn10k_dpi.
> >> >
> >> > +config MCHP_LAN966X_PCI
> >> > + tristate "Microchip LAN966x PCIe Support"
> >> > + depends on PCI
> >> > + select OF
> >> > + select OF_OVERLAY
> >>
> >> Are these "select" statements what we want? When configuring current
> >> mainline snapshot, I accidentally enabled this driver and ended up
> >> flooded with an enormous amount of new config options, most of which
> >> didn't make much sense on x86_64. It took quite long to investigate why.
> >>
> >> Couldn't we rather use
> >>
> >> depends on PCI && OF && OF_OVERLAY
> >>
> >> like other drivers?
>
> Agreed.
>
> I would write in two lines as
>
> depends on PCI
> depends on OF_OVERLAY
>
> since OF_OVERLAY already depends on OF, that can be left out.
> The effect is the same as your variant though.

What about

depends on OF
select OF_OVERLAY

as "OF" is a clear bus dependency, due to the driver providing an OF
child bus (cfr. I2C or SPI bus controller drivers depending on I2C or
SPI), and OF_OVERLAY is an optional software mechanism?

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