On Fri, Nov 29, 2024, at 09:10, Herve Codina wrote:

Hi Michal,
On Thu, 28 Nov 2024 20:42:53 +0100
Michal Kubecek [off-list ref] wrote:

quoted

quoted

--- 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