On Fri, Nov 29, 2024, at 09:10, Herve Codina wrote:
> Hi Michal,
> On Thu, 28 Nov 2024 20:42:53 +0100
> Michal Kubecek <mkubecek@xxxxxxx> 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.
>
> I don't have a strong opinion on this 'select' vs 'depends on' for those
> symbols.
>
> I used select because the dependency is not obvious for a user that just
> want the driver for the LAN966x PCI device.
The general rules for picking one over the other are:
- use 'select' for symbols that can not be enabled manually
- prefer 'depends on' for user-visible options
- do whatever other driver do for the same symbol, to avoid
dependency loops.
Arnd
