Hi, +Cc Ricardo On Fri, 29 Nov 2024 11:29:44 +0100 Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: > Hi Arnd, > > On Fri, Nov 29, 2024 at 10:23 AM Arnd Bergmann <arnd@xxxxxxxx> wrote: > > On Fri, Nov 29, 2024, at 09:44, Geert Uytterhoeven wrote: > > > On Fri, Nov 29, 2024 at 9:25 AM Arnd Bergmann <arnd@xxxxxxxx> 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@xxxxxxxxxxxx/ > > 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é