From: Vladimir Oltean <olteanv@gmail.com> Date: 2019-12-12 17:12:01
From: Vladimir Oltean <vladimir.oltean@nxp.com>
NET_DSA_MSCC_FELIX and MSCC_OCELOT_SWITCH_OCELOT are 2 different drivers
that use the same core operations, compiled under MSCC_OCELOT_SWITCH.
The DSA driver depends on HAVE_NET_DSA, and the switchdev driver depends
on NET_VENDOR_MICROSEMI. This dependency is purely cosmetic (to hide
options per driver class, or per networking vendor) from menuconfig
choices.
However, there is an issue with the fact that the common core depends on
NET_VENDOR_MICROSEMI, as can be seen below, when building the DSA
driver:
WARNING: unmet direct dependencies detected for MSCC_OCELOT_SWITCH
Depends on [n]: NETDEVICES [=y] && ETHERNET [=y] &&
NET_VENDOR_MICROSEMI [=n] && NET_SWITCHDEV [=y] && HAS_IOMEM [=y]
Selected by [y]:
NET_DSA_MSCC_FELIX [=y] && NETDEVICES [=y] && HAVE_NET_DSA [=y]
&& NET_DSA [=y] && PCI [=y]
We don't want to make NET_DSA_MSCC_FELIX hidden under
NET_VENDOR_MICROSEMI, since it is physically located under
drivers/net/dsa and already findable in the configurator through DSA.
So we move the common Ocelot core outside the NET_VENDOR_MICROSEMI
selector, and we make the switchdev and DSA drivers select it by
default. In that process, we hide it from menuconfig, since the user
shouldn't need to know anything about it, and we change it from tristate
to bool, since it doesn't make a lot of sense to have it as yet another
loadable kernel module.
With this, the DSA driver also needs to explicitly depend on ETHERNET,
to avoid an unmet dependency situation caused by selecting
MSCC_OCELOT_SWITCH when ETHERNET is disabled.
A few more dependencies were shuffled around. HAS_IOMEM is now "depends"
to avoid a circular dependency:
symbol HAS_IOMEM is selected by MSCC_OCELOT_SWITCH
symbol MSCC_OCELOT_SWITCH depends on NETDEVICES
symbol NETDEVICES is selected by SCSI_CXGB3_ISCSI
symbol SCSI_CXGB3_ISCSI depends on SCSI_LOWLEVEL
symbol SCSI_LOWLEVEL depends on SCSI
symbol SCSI is selected by ATA
symbol ATA depends on HAS_IOMEM
Fixes: 56051948773e ("net: dsa: ocelot: add driver for Felix switch family")
Reported-by: Arnd Bergmann <arnd@arndb.de>
Reported-by: Mao Wenan <redacted>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/dsa/ocelot/Kconfig | 2 +-
drivers/net/ethernet/mscc/Kconfig | 27 ++++++++++++++++-----------
2 files changed, 17 insertions(+), 12 deletions(-)
From: Jakub Kicinski <hidden> Date: 2019-12-14 00:10:34
On Thu, 12 Dec 2019 19:11:25 +0200, Vladimir Oltean wrote:
From: Vladimir Oltean <vladimir.oltean@nxp.com>
NET_DSA_MSCC_FELIX and MSCC_OCELOT_SWITCH_OCELOT are 2 different drivers
that use the same core operations, compiled under MSCC_OCELOT_SWITCH.
The DSA driver depends on HAVE_NET_DSA, and the switchdev driver depends
on NET_VENDOR_MICROSEMI. This dependency is purely cosmetic (to hide
options per driver class, or per networking vendor) from menuconfig
choices.
However, there is an issue with the fact that the common core depends on
NET_VENDOR_MICROSEMI, as can be seen below, when building the DSA
driver:
WARNING: unmet direct dependencies detected for MSCC_OCELOT_SWITCH
Depends on [n]: NETDEVICES [=y] && ETHERNET [=y] &&
NET_VENDOR_MICROSEMI [=n] && NET_SWITCHDEV [=y] && HAS_IOMEM [=y]
Selected by [y]:
NET_DSA_MSCC_FELIX [=y] && NETDEVICES [=y] && HAVE_NET_DSA [=y]
&& NET_DSA [=y] && PCI [=y]
We don't want to make NET_DSA_MSCC_FELIX hidden under
NET_VENDOR_MICROSEMI, since it is physically located under
drivers/net/dsa and already findable in the configurator through DSA.
So we move the common Ocelot core outside the NET_VENDOR_MICROSEMI
selector, and we make the switchdev and DSA drivers select it by
default. In that process, we hide it from menuconfig, since the user
shouldn't need to know anything about it, and we change it from tristate
to bool, since it doesn't make a lot of sense to have it as yet another
loadable kernel module.
Mmm. Is that really the only choice? Wouldn't it be better to do
something like:
Presumably if user wants the end driver to be loadable module the
library should default to a module?
quoted hunk
With this, the DSA driver also needs to explicitly depend on ETHERNET,
to avoid an unmet dependency situation caused by selecting
MSCC_OCELOT_SWITCH when ETHERNET is disabled.
A few more dependencies were shuffled around. HAS_IOMEM is now "depends"
to avoid a circular dependency:
symbol HAS_IOMEM is selected by MSCC_OCELOT_SWITCH
symbol MSCC_OCELOT_SWITCH depends on NETDEVICES
symbol NETDEVICES is selected by SCSI_CXGB3_ISCSI
symbol SCSI_CXGB3_ISCSI depends on SCSI_LOWLEVEL
symbol SCSI_LOWLEVEL depends on SCSI
symbol SCSI is selected by ATA
symbol ATA depends on HAS_IOMEM
Fixes: 56051948773e ("net: dsa: ocelot: add driver for Felix switch family")
Reported-by: Arnd Bergmann <arnd@arndb.de>
Reported-by: Mao Wenan <redacted>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/dsa/ocelot/Kconfig | 2 +-
drivers/net/ethernet/mscc/Kconfig | 27 ++++++++++++++++-----------
2 files changed, 17 insertions(+), 12 deletions(-)
The move of select PHYLIB seems unnecessary, is there a reason?
Otherwise since this is net material perhaps better to refrain from it.
help
- This driver supports the Ocelot network switch device.
+ This is the core library for the Vitesse / Microsemi / Microchip
+ Ocelot network switch family. It offers a set of DSA-compatible
+ switchdev operations for managing switches of this class, like:
+ - VSC7514
+ - VSC9959
+if NET_VENDOR_MICROSEMI
config MSCC_OCELOT_SWITCH_OCELOT
- tristate "Ocelot switch driver on Ocelot"
- depends on MSCC_OCELOT_SWITCH
- depends on GENERIC_PHY
- depends on OF_NET
+ tristate "Ocelot switch driver for local management CPUs"
+ select MSCC_OCELOT_SWITCH
+ select GENERIC_PHY
+ select OF_NET
help
- This driver supports the Ocelot network switch device as present on
- the Ocelot SoCs.
+ This supports the network switch present on the Ocelot SoCs
+ (VSC7514). The driver is intended for use on the local MIPS
+ management CPU. Frame transfer is through polled I/O or DMA.
endif # NET_VENDOR_MICROSEMI
+ tristate "Ocelot switch driver for local management CPUs"
+ select MSCC_OCELOT_SWITCH
endif # NET_VENDOR_MICROSEMI
The "if NET_VENDOR_MICROSEMI" must come directly after the
config NET_VENDOR_MICROSEMI, otherwise the indentation
in "make menuconfig" is wrong. So please move MSCC_OCELOT_SWITCH
after the "endif".
Arnd
On Thu, Dec 12, 2019 at 6:11 PM Vladimir Oltean [off-list ref] wrote:
From: Vladimir Oltean <vladimir.oltean@nxp.com>
NET_DSA_MSCC_FELIX and MSCC_OCELOT_SWITCH_OCELOT are 2 different drivers
that use the same core operations, compiled under MSCC_OCELOT_SWITCH.
Fixes: 56051948773e ("net: dsa: ocelot: add driver for Felix switch family")
Reported-by: Arnd Bergmann <arnd@arndb.de>
Reported-by: Mao Wenan <redacted>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
I did some more build testing and ran into another issue now that
MSCC_OCELOT_SWITCH_OCELOT can be built without
CONFIG_SWITCHDEV:
WARNING: unmet direct dependencies detected for MSCC_OCELOT_SWITCH
Depends on [n]: NETDEVICES [=y] && ETHERNET [=y] && NET_SWITCHDEV
[=n] && HAS_IOMEM [=y]
Selected by [y]:
- MSCC_OCELOT_SWITCH_OCELOT [=y] && NETDEVICES [=y] && ETHERNET [=y]
&& NET_VENDOR_MICROSEMI [=y]
drivers/net/ethernet/mscc/ocelot_board.c: In function 'ocelot_xtr_irq_handler':
drivers/net/ethernet/mscc/ocelot_board.c:176:7: error: 'struct
sk_buff' has no member named 'offload_fwd_mark'
skb->offload_fwd_mark = 1;
^~
Adding another "depends on NET_SWITCHDEV" fixed it for me.
Arnd
On Sat, Dec 14, 2019 at 4:16 PM Arnd Bergmann [off-list ref] wrote:
On Thu, Dec 12, 2019 at 6:11 PM Vladimir Oltean [off-list ref] wrote:
quoted
From: Vladimir Oltean <vladimir.oltean@nxp.com>
NET_DSA_MSCC_FELIX and MSCC_OCELOT_SWITCH_OCELOT are 2 different drivers
that use the same core operations, compiled under MSCC_OCELOT_SWITCH.
quoted
Fixes: 56051948773e ("net: dsa: ocelot: add driver for Felix switch family")
Reported-by: Arnd Bergmann <arnd@arndb.de>
Reported-by: Mao Wenan <redacted>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
I did some more build testing and ran into another issue now that
MSCC_OCELOT_SWITCH_OCELOT can be built without
CONFIG_SWITCHDEV:
From: Vladimir Oltean <olteanv@gmail.com> Date: 2019-12-15 21:56:38
On Sat, 14 Dec 2019 at 22:49, Arnd Bergmann [off-list ref] wrote:
quoted hunk
On Sat, Dec 14, 2019 at 4:16 PM Arnd Bergmann [off-list ref] wrote:
quoted
On Thu, Dec 12, 2019 at 6:11 PM Vladimir Oltean [off-list ref] wrote:
quoted
From: Vladimir Oltean <vladimir.oltean@nxp.com>
NET_DSA_MSCC_FELIX and MSCC_OCELOT_SWITCH_OCELOT are 2 different drivers
that use the same core operations, compiled under MSCC_OCELOT_SWITCH.
quoted
Fixes: 56051948773e ("net: dsa: ocelot: add driver for Felix switch family")
Reported-by: Arnd Bergmann <arnd@arndb.de>
Reported-by: Mao Wenan <redacted>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
I did some more build testing and ran into another issue now that
MSCC_OCELOT_SWITCH_OCELOT can be built without
CONFIG_SWITCHDEV: