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

[Herve Codina <herve.codina@bootlin.com>]

Hi,

+Cc Ricardo

On Fri, 29 Nov 2024 11:29:44 +0100
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Arnd,
> 
> On Fri, Nov 29, 2024 at 10:23 AM Arnd Bergmann <arnd@arndb.de> wrote:
> > On Fri, Nov 29, 2024, at 09:44, Geert Uytterhoeven wrote:  
> > > 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:
> > >> 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?  
> >

A patch has be done in that way by Ricardo Ribalda
  https://lore.kernel.org/all/20241129-lan966x-depend-v1-1-603fc4996c4f@chromium.org/

> > OF_OVERLAY is currently a user visible option, so I think it's
> > intended to be used with 'depends on'. The only other callers
> > of this interface are the kunit test modules that just leave
> > out the overlay code if that is disabled.  
> 
> Indeed, there are no real upstream users of OF_OVERLAY left.
> Until commit 1760eb547276299a ("drm: rcar-du: Drop leftovers
> dependencies from Kconfig"), the rcar-lvds driver selected OF_OVERLAY
> to be able to fix up old DTBs.
> 
> > If we decide to treat OF_OVERLAY as a library instead, it should
> > probably become a silent Kconfig option that gets selected by
> > all its users including the unit tests, and then we can remove
> > the #ifdef checks there.  
> 
> Yep.
> 
> > Since OF_OVERLAY pulls in OF_DYNAMIC, I would still prefer that
> > to be a user choice. Silently enabling OF_OVERLAY definitely has
> > a risk of introducing regressions since it changes some of the
> > interesting code paths in the core, in particular it enables
> > reference counting in of_node_get(), which many drivers get wrong.  
> 
> Distro kernels will have to enable this anyway, if they want to
> support LAN966x...
> 

Best regards,
Hervé