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

["Arnd Bergmann" <arnd@arndb.de>]

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?

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.

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.

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.

      Arnd