net: dsa: ocelot: add NET_VENDOR_MICROSEMI dependency

Message ID 20191210203710.2987983-1-arnd@arndb.de
State Accepted
Commit 95bed1a9fb2b7ee13b58d0d29710282a62082a89
Headers show
Series
  • net: dsa: ocelot: add NET_VENDOR_MICROSEMI dependency
Related show

Commit Message

Arnd Bergmann Dec. 10, 2019, 8:36 p.m.
Selecting MSCC_OCELOT_SWITCH is not possible when NET_VENDOR_MICROSEMI
is disabled:

WARNING: unmet direct dependencies detected for MSCC_OCELOT_SWITCH
  Depends on [n]: NETDEVICES [=y] && ETHERNET [=n] && NET_VENDOR_MICROSEMI [=n] && NET_SWITCHDEV [=y] && HAS_IOMEM [=y]
  Selected by [m]:
  - NET_DSA_MSCC_FELIX [=m] && NETDEVICES [=y] && HAVE_NET_DSA [=y] && NET_DSA [=y] && PCI [=y]

Add a Kconfig dependency on NET_VENDOR_MICROSEMI, which also implies
CONFIG_NETDEVICES.

Fixes: 56051948773e ("net: dsa: ocelot: add driver for Felix switch family")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 drivers/net/dsa/ocelot/Kconfig | 1 +
 1 file changed, 1 insertion(+)

-- 
2.20.0

Comments

Arnd Bergmann Dec. 10, 2019, 10:04 p.m. | #1
On Tue, Dec 10, 2019 at 10:37 PM Vladimir Oltean <olteanv@gmail.com> wrote:
>

> Hi Arnd,

>

> On Tue, 10 Dec 2019 at 22:37, Arnd Bergmann <arnd@arndb.de> wrote:

> >

> > Selecting MSCC_OCELOT_SWITCH is not possible when NET_VENDOR_MICROSEMI

> > is disabled:

> >

> > WARNING: unmet direct dependencies detected for MSCC_OCELOT_SWITCH

> >   Depends on [n]: NETDEVICES [=y] && ETHERNET [=n] && NET_VENDOR_MICROSEMI [=n] && NET_SWITCHDEV [=y] && HAS_IOMEM [=y]

> >   Selected by [m]:

> >   - NET_DSA_MSCC_FELIX [=m] && NETDEVICES [=y] && HAVE_NET_DSA [=y] && NET_DSA [=y] && PCI [=y]

> >

> > Add a Kconfig dependency on NET_VENDOR_MICROSEMI, which also implies

> > CONFIG_NETDEVICES.

> >

> > Fixes: 56051948773e ("net: dsa: ocelot: add driver for Felix switch family")

> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> > ---

>

> This has been submitted before, here [0].

>

> It isn't wrong, but in principle I agree with David that it is strange

> to put a "depends" relationship between a driver in drivers/net/dsa

> and the Kconfig vendor umbrella from drivers/net/ethernet/mscc ("why

> would the user care/need to enable NET_VENDOR_MICROSEMI to see the DSA

> driver" is a valid point to me). This is mainly because I don't

> understand the point of CONFIG_NET_VENDOR_* options, they're a bit

> tribalistic to my ears.

>

> Nonetheless, alternatives may be:

> - Move MSCC_OCELOT_SWITCH core option outside of the

> NET_VENDOR_MICROSEMI umbrella, and make it invisible to menuconfig,

> just selectable from the 2 driver instances (MSCC_OCELOT_SWITCH_OCELOT

> and NET_DSA_MSCC_FELIX). MSCC_OCELOT_SWITCH has no reason to be

> selectable by the user anyway.


You still need 'depends on NETDEVICES' in that case, otherwise this sounds
like a good option.

> - Remove NET_VENDOR_MICROSEMI altogether. There is a single driver

> under drivers/net/ethernet/mscc and it's already causing problems,

> it's ridiculous.


It's only there for consistency with the other directories under
drivers/net/ethernet/.

> - Leave it as it is. I genuinely ask: if the build system tells you

> that the build dependencies are not met, does it matter if it compiles

> or not?


We try very hard to allow all randconfig builds to complete without
any output from the build process when building with 'make -s'.
Random warnings like this just clutter up the output, even if it's
harmless there is a risk of missing something important.

Yet another option is
- Change NET_DSA_MSCC_FELIX to use 'depends on
  MSCC_OCELOT_SWITCH' instead of 'select NET_DSA_MSCC_FELIX'.


     Arnd
Arnd Bergmann Dec. 11, 2019, 7:57 a.m. | #2
On Tue, Dec 10, 2019 at 11:33 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> On Wed, 11 Dec 2019 at 00:04, Arnd Bergmann <arnd@arndb.de> wrote:

> > On Tue, Dec 10, 2019 at 10:37 PM Vladimir Oltean <olteanv@gmail.com> wrote:

> > > Nonetheless, alternatives may be:

> > > - Move MSCC_OCELOT_SWITCH core option outside of the

> > > NET_VENDOR_MICROSEMI umbrella, and make it invisible to menuconfig,

> > > just selectable from the 2 driver instances (MSCC_OCELOT_SWITCH_OCELOT

> > > and NET_DSA_MSCC_FELIX). MSCC_OCELOT_SWITCH has no reason to be

> > > selectable by the user anyway.

> >

> > You still need 'depends on NETDEVICES' in that case, otherwise this sounds

> > like a good option.

> >

>

> I don't completely understand this. Looks like NETDEVICES is another

> one of those options that don't enable any code. I would have expected

> that NET_SWITCHDEV depended on it already? But anyway, it's still a

> small compromise and not a problem.


My mistake: When I had tried it, I did run into this problem, but it was
CONFIG_ETHERNET, not CONFIG_NETDEVICES.

NET_SWITCHDEV depends only on INET, NET_DSA also depends on
NETDEVICES, but neither of them depends on ETHERNET, which
only controls drivers under drivers/net/ethernet, but not support for
ethernet itself.
>

> So, if you agree, I can take care of this tomorrow by reworking the

> Kconfig in the 1st proposed way. I hope you don't mind that I'm

> volunteering to do it, but the change will require a bit of explaining

> which is non-trivial, and I don't expect that you really want to know

> these details, just that it compiles with no issue from all angles.


Sounds good, thanks!

If you like, I can give your patch a spin on my randconfig build
system before you submit it for inclusion, in case there is another
problem we both missed.

        Arnd

Patch

diff --git a/drivers/net/dsa/ocelot/Kconfig b/drivers/net/dsa/ocelot/Kconfig
index 0031ca814346..6f9804093150 100644
--- a/drivers/net/dsa/ocelot/Kconfig
+++ b/drivers/net/dsa/ocelot/Kconfig
@@ -2,6 +2,7 @@ 
 config NET_DSA_MSCC_FELIX
 	tristate "Ocelot / Felix Ethernet switch support"
 	depends on NET_DSA && PCI
+	depends on NET_VENDOR_MICROSEMI
 	select MSCC_OCELOT_SWITCH
 	select NET_DSA_TAG_OCELOT
 	help