* [PATCH] net: dsa: ocelot: add NET_VENDOR_MICROSEMI dependency
@ 2019-12-10 20:36 Arnd Bergmann
2019-12-10 21:37 ` Vladimir Oltean
0 siblings, 1 reply; 5+ messages in thread
From: Arnd Bergmann @ 2019-12-10 20:36 UTC (permalink / raw)
To: Vladimir Oltean, Claudiu Manoil
Cc: Arnd Bergmann, David S. Miller, Andrew Lunn, Vivien Didelot,
Florian Fainelli, netdev, linux-kernel
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(+)
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
--
2.20.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] net: dsa: ocelot: add NET_VENDOR_MICROSEMI dependency
2019-12-10 20:36 [PATCH] net: dsa: ocelot: add NET_VENDOR_MICROSEMI dependency Arnd Bergmann
@ 2019-12-10 21:37 ` Vladimir Oltean
2019-12-10 22:04 ` Arnd Bergmann
0 siblings, 1 reply; 5+ messages in thread
From: Vladimir Oltean @ 2019-12-10 21:37 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Vladimir Oltean, Claudiu Manoil, David S. Miller, Andrew Lunn,
Vivien Didelot, Florian Fainelli, netdev, lkml
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.
- Remove NET_VENDOR_MICROSEMI altogether. There is a single driver
under drivers/net/ethernet/mscc and it's already causing problems,
it's ridiculous.
- 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?
[0]: https://www.spinics.net/lists/netdev/msg614325.html
Regards,
-Vladimir
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] net: dsa: ocelot: add NET_VENDOR_MICROSEMI dependency
2019-12-10 21:37 ` Vladimir Oltean
@ 2019-12-10 22:04 ` Arnd Bergmann
2019-12-10 22:32 ` Vladimir Oltean
0 siblings, 1 reply; 5+ messages in thread
From: Arnd Bergmann @ 2019-12-10 22:04 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Vladimir Oltean, Claudiu Manoil, David S. Miller, Andrew Lunn,
Vivien Didelot, Florian Fainelli, netdev, lkml
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] net: dsa: ocelot: add NET_VENDOR_MICROSEMI dependency
2019-12-10 22:04 ` Arnd Bergmann
@ 2019-12-10 22:32 ` Vladimir Oltean
2019-12-11 7:57 ` Arnd Bergmann
0 siblings, 1 reply; 5+ messages in thread
From: Vladimir Oltean @ 2019-12-10 22:32 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Vladimir Oltean, Claudiu Manoil, David S. Miller, Andrew Lunn,
Vivien Didelot, Florian Fainelli, netdev, lkml
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:
> >
> > 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.
>
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.
> > - 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'.
>
That's strange too. MSCC_OCELOT_SWITCH just enables a common driver
core which is used by both NET_DSA_MSCC_FELIX and
MSCC_OCELOT_SWITCH_OCELOT (possibly by more in the future). I don't
see any reason why the common library (purely an implementation
detail) should even be user-visible, let alone why it should hide a
DSA driver.
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.
>
> Arnd
Thanks,
-Vladimir
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] net: dsa: ocelot: add NET_VENDOR_MICROSEMI dependency
2019-12-10 22:32 ` Vladimir Oltean
@ 2019-12-11 7:57 ` Arnd Bergmann
0 siblings, 0 replies; 5+ messages in thread
From: Arnd Bergmann @ 2019-12-11 7:57 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Vladimir Oltean, Claudiu Manoil, David S. Miller, Andrew Lunn,
Vivien Didelot, Florian Fainelli, netdev, lkml
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-12-11 7:58 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-10 20:36 [PATCH] net: dsa: ocelot: add NET_VENDOR_MICROSEMI dependency Arnd Bergmann
2019-12-10 21:37 ` Vladimir Oltean
2019-12-10 22:04 ` Arnd Bergmann
2019-12-10 22:32 ` Vladimir Oltean
2019-12-11 7:57 ` Arnd Bergmann
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.