Message ID | 20191212171125.9933-1-olteanv@gmail.com |
---|---|
State | Changes Requested |
Delegated to: | David Miller |
Headers | show |
Series | net: mscc: ocelot: hide MSCC_OCELOT_SWITCH and move outside NET_VENDOR_MICROSEMI | expand |
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: diff --git a/drivers/net/ethernet/mscc/Kconfig b/drivers/net/ethernet/mscc/Kconfig index 13fa11c30f68..991db8b94a9c 100644 --- a/drivers/net/ethernet/mscc/Kconfig +++ b/drivers/net/ethernet/mscc/Kconfig @@ -10,7 +10,8 @@ config NET_VENDOR_MICROSEMI the questions about Microsemi devices. config MSCC_OCELOT_SWITCH - bool + tristate + default (MSCC_OCELOT_SWITCH_OCELOT || NET_DSA_MSCC_FELIX) depends on NET_SWITCHDEV depends on HAS_IOMEM select REGMAP_MMIO Presumably if user wants the end driver to be loadable module the library should default to a 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 <maowenan@huawei.com> > 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(-) > > diff --git a/drivers/net/dsa/ocelot/Kconfig b/drivers/net/dsa/ocelot/Kconfig > index 0031ca814346..c41d50ca98b7 100644 > --- a/drivers/net/dsa/ocelot/Kconfig > +++ b/drivers/net/dsa/ocelot/Kconfig > @@ -1,7 +1,7 @@ > # SPDX-License-Identifier: GPL-2.0-only > config NET_DSA_MSCC_FELIX > tristate "Ocelot / Felix Ethernet switch support" > - depends on NET_DSA && PCI > + depends on NET_DSA && PCI && ETHERNET > select MSCC_OCELOT_SWITCH > select NET_DSA_TAG_OCELOT > help > diff --git a/drivers/net/ethernet/mscc/Kconfig b/drivers/net/ethernet/mscc/Kconfig > index bcec0587cf61..13fa11c30f68 100644 > --- a/drivers/net/ethernet/mscc/Kconfig > +++ b/drivers/net/ethernet/mscc/Kconfig > @@ -9,24 +9,29 @@ config NET_VENDOR_MICROSEMI > kernel: saying N will just cause the configurator to skip all > the questions about Microsemi devices. > > -if NET_VENDOR_MICROSEMI > - > config MSCC_OCELOT_SWITCH > - tristate "Ocelot switch driver" > + bool > depends on NET_SWITCHDEV > depends on HAS_IOMEM > - select PHYLIB > select REGMAP_MMIO > + select PHYLIB 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
On Sat, Dec 14, 2019 at 1:11 AM Jakub Kicinski <jakub.kicinski@netronome.com> wrote: > On Thu, 12 Dec 2019 19:11:25 +0200, Vladimir Oltean wrote: > > From: Vladimir Oltean <vladimir.oltean@nxp.com> > Mmm. Is that really the only choice? Wouldn't it be better to do > something like: > > diff --git a/drivers/net/ethernet/mscc/Kconfig b/drivers/net/ethernet/mscc/Kconfig > index 13fa11c30f68..991db8b94a9c 100644 > --- a/drivers/net/ethernet/mscc/Kconfig > +++ b/drivers/net/ethernet/mscc/Kconfig > @@ -10,7 +10,8 @@ config NET_VENDOR_MICROSEMI > the questions about Microsemi devices. > > config MSCC_OCELOT_SWITCH > - bool > + tristate > + default (MSCC_OCELOT_SWITCH_OCELOT || NET_DSA_MSCC_FELIX) > depends on NET_SWITCHDEV > depends on HAS_IOMEM > select REGMAP_MMIO > > Presumably if user wants the end driver to be loadable module the > library should default to a module? Agreed, should be tristate. The 'default' isn't necessary here, the 'select' does it all the same and there only needs to be one of the two. > > diff --git a/drivers/net/ethernet/mscc/Kconfig b/drivers/net/ethernet/mscc/Kconfig > > index bcec0587cf61..13fa11c30f68 100644 > > --- a/drivers/net/ethernet/mscc/Kconfig > > +++ b/drivers/net/ethernet/mscc/Kconfig > > @@ -9,24 +9,29 @@ config NET_VENDOR_MICROSEMI > > kernel: saying N will just cause the configurator to skip all > > the questions about Microsemi devices. > > > > -if NET_VENDOR_MICROSEMI > > - > > config MSCC_OCELOT_SWITCH > > - tristate "Ocelot switch driver" > > + bool ... > > > > +if NET_VENDOR_MICROSEMI > > > > config MSCC_OCELOT_SWITCH_OCELOT ... > > + 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 <olteanv@gmail.com> 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 <maowenan@huawei.com> > 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 <arnd@arndb.de> wrote: > > On Thu, Dec 12, 2019 at 6:11 PM Vladimir Oltean <olteanv@gmail.com> 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 <maowenan@huawei.com> > > 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: And another one when CONFIG_NET_VENDOR_MICROSEMI is disabled: ERROR: "ocelot_fdb_dump" [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! ERROR: "ocelot_regfields_init" [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! ERROR: "ocelot_regmap_init" [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! ERROR: "ocelot_init" [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! ERROR: "ocelot_fdb_del" [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! ERROR: "__ocelot_write_ix" [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! ERROR: "ocelot_bridge_stp_state_set" [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! ERROR: "ocelot_port_vlan_filtering" [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! ERROR: "ocelot_get_ethtool_stats" [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! ERROR: "ocelot_port_enable" [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! ERROR: "ocelot_vlan_del" [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! ERROR: "ocelot_deinit" [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! ERROR: "ocelot_init_port" [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! ERROR: "ocelot_fdb_add" [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! This fixes it: diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile index f8f38dcb5f8a..83351228734a 100644 --- a/drivers/net/ethernet/Makefile +++ b/drivers/net/ethernet/Makefile @@ -55,7 +55,7 @@ obj-$(CONFIG_NET_VENDOR_MEDIATEK) += mediatek/ obj-$(CONFIG_NET_VENDOR_MELLANOX) += mellanox/ obj-$(CONFIG_NET_VENDOR_MICREL) += micrel/ obj-$(CONFIG_NET_VENDOR_MICROCHIP) += microchip/ -obj-$(CONFIG_NET_VENDOR_MICROSEMI) += mscc/ +obj-y += mscc/ obj-$(CONFIG_NET_VENDOR_MOXART) += moxa/ obj-$(CONFIG_NET_VENDOR_MYRI) += myricom/ obj-$(CONFIG_FEALNX) += fealnx.o Arnd
On Sat, 14 Dec 2019 at 22:49, Arnd Bergmann <arnd@arndb.de> wrote: > > On Sat, Dec 14, 2019 at 4:16 PM Arnd Bergmann <arnd@arndb.de> wrote: > > > > On Thu, Dec 12, 2019 at 6:11 PM Vladimir Oltean <olteanv@gmail.com> 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 <maowenan@huawei.com> > > > 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: > > And another one when CONFIG_NET_VENDOR_MICROSEMI is disabled: > > ERROR: "ocelot_fdb_dump" [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! > ERROR: "ocelot_regfields_init" [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! > ERROR: "ocelot_regmap_init" [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! > ERROR: "ocelot_init" [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! > ERROR: "ocelot_fdb_del" [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! > ERROR: "__ocelot_write_ix" [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! > ERROR: "ocelot_bridge_stp_state_set" > [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! > ERROR: "ocelot_port_vlan_filtering" > [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! > ERROR: "ocelot_get_ethtool_stats" > [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! > ERROR: "ocelot_port_enable" [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! > ERROR: "ocelot_vlan_del" [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! > ERROR: "ocelot_deinit" [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! > ERROR: "ocelot_init_port" [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! > ERROR: "ocelot_fdb_add" [drivers/net/dsa/ocelot/mscc_felix.ko] undefined! > > This fixes it: > > diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile > index f8f38dcb5f8a..83351228734a 100644 > --- a/drivers/net/ethernet/Makefile > +++ b/drivers/net/ethernet/Makefile > @@ -55,7 +55,7 @@ obj-$(CONFIG_NET_VENDOR_MEDIATEK) += mediatek/ > obj-$(CONFIG_NET_VENDOR_MELLANOX) += mellanox/ > obj-$(CONFIG_NET_VENDOR_MICREL) += micrel/ > obj-$(CONFIG_NET_VENDOR_MICROCHIP) += microchip/ > -obj-$(CONFIG_NET_VENDOR_MICROSEMI) += mscc/ > +obj-y += mscc/ Thanks Arnd. This is getting out of hand. I'll just opt for the simple solution and make it depend on NET_VENDOR_MICROSEMI. > obj-$(CONFIG_NET_VENDOR_MOXART) += moxa/ > obj-$(CONFIG_NET_VENDOR_MYRI) += myricom/ > obj-$(CONFIG_FEALNX) += fealnx.o > > Arnd
diff --git a/drivers/net/dsa/ocelot/Kconfig b/drivers/net/dsa/ocelot/Kconfig index 0031ca814346..c41d50ca98b7 100644 --- a/drivers/net/dsa/ocelot/Kconfig +++ b/drivers/net/dsa/ocelot/Kconfig @@ -1,7 +1,7 @@ # SPDX-License-Identifier: GPL-2.0-only config NET_DSA_MSCC_FELIX tristate "Ocelot / Felix Ethernet switch support" - depends on NET_DSA && PCI + depends on NET_DSA && PCI && ETHERNET select MSCC_OCELOT_SWITCH select NET_DSA_TAG_OCELOT help diff --git a/drivers/net/ethernet/mscc/Kconfig b/drivers/net/ethernet/mscc/Kconfig index bcec0587cf61..13fa11c30f68 100644 --- a/drivers/net/ethernet/mscc/Kconfig +++ b/drivers/net/ethernet/mscc/Kconfig @@ -9,24 +9,29 @@ config NET_VENDOR_MICROSEMI kernel: saying N will just cause the configurator to skip all the questions about Microsemi devices. -if NET_VENDOR_MICROSEMI - config MSCC_OCELOT_SWITCH - tristate "Ocelot switch driver" + bool depends on NET_SWITCHDEV depends on HAS_IOMEM - select PHYLIB select REGMAP_MMIO + select PHYLIB 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