lkml.org 
[lkml]   [2022]   [Nov]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 14/18] dsa: ocelot: fix mixed module-builtin object
    On Sat, Nov 19, 2022 at 11:09:28PM +0000, Alexander Lobakin wrote:
    > With CONFIG_NET_DSA_MSCC_FELIX=m and CONFIG_NET_DSA_MSCC_SEVILLE=y
    > (or vice versa), felix.o are linked to a module and also to vmlinux
    > even though the expected CFLAGS are different between builtins and
    > modules.
    > This is the same situation as fixed by
    > commit 637a642f5ca5 ("zstd: Fixing mixed module-builtin objects").
    > There's also no need to duplicate relatively big piece of object
    > code into two modules.
    >
    > Introduce the new module, mscc_core, to provide the common functions
    > to both mscc_felix and mscc_seville.
    >
    > Fixes: d60bc62de4ae ("net: dsa: seville: build as separate module")
    > Suggested-by: Masahiro Yamada <masahiroy@kernel.org>
    > Signed-off-by: Alexander Lobakin <alobakin@pm.me>
    > ---

    I don't disagree with the patch, but I dislike the name chosen.
    How about NET_DSA_OCELOT_LIB and mscc_ocelot_dsa_lib.o? The "core" of
    the hardware support is arguably mscc_ocelot_switch_lib.o, I don't think
    it would be good to use that word here, since the code you're moving is
    no more than a thin glue layer with some DSA specific code.

    Adding Colin for a second opinion on the naming. I'm sure things could
    have been done better in the first place, just not sure how.

    Also, could you please make the commit prefix "net: dsa: ocelot" when
    you resend this and the other networking patches to the "net" tree?

    > drivers/net/dsa/ocelot/Kconfig | 18 ++++++++++--------
    > drivers/net/dsa/ocelot/Makefile | 12 +++++-------
    > drivers/net/dsa/ocelot/felix.c | 6 ++++++
    > drivers/net/dsa/ocelot/felix_vsc9959.c | 2 ++
    > drivers/net/dsa/ocelot/seville_vsc9953.c | 2 ++
    > 5 files changed, 25 insertions(+), 15 deletions(-)
    >
    > diff --git a/drivers/net/dsa/ocelot/Kconfig b/drivers/net/dsa/ocelot/Kconfig
    > index 08db9cf76818..59845274e374 100644
    > --- a/drivers/net/dsa/ocelot/Kconfig
    > +++ b/drivers/net/dsa/ocelot/Kconfig
    > @@ -1,4 +1,12 @@
    > # SPDX-License-Identifier: GPL-2.0-only
    > +
    > +config NET_DSA_MSCC_CORE
    > + tristate
    > + select MSCC_OCELOT_SWITCH_LIB
    > + select NET_DSA_TAG_OCELOT_8021Q
    > + select NET_DSA_TAG_OCELOT
    > + select PCS_LYNX

    Please keep PCS_LYNX selected by MSCC_FELIX and MSCC_SEVILLE respectively.

    > +
    > config NET_DSA_MSCC_FELIX
    > tristate "Ocelot / Felix Ethernet switch support"
    > depends on NET_DSA && PCI
    > @@ -7,11 +15,8 @@ config NET_DSA_MSCC_FELIX
    > depends on HAS_IOMEM
    > depends on PTP_1588_CLOCK_OPTIONAL
    > depends on NET_SCH_TAPRIO || NET_SCH_TAPRIO=n
    > - select MSCC_OCELOT_SWITCH_LIB
    > - select NET_DSA_TAG_OCELOT_8021Q
    > - select NET_DSA_TAG_OCELOT
    > + select NET_DSA_MSCC_CORE
    > select FSL_ENETC_MDIO
    > - select PCS_LYNX
    > help
    > This driver supports the VSC9959 (Felix) switch, which is embedded as
    > a PCIe function of the NXP LS1028A ENETC RCiEP.
    > @@ -22,11 +27,8 @@ config NET_DSA_MSCC_SEVILLE
    > depends on NET_VENDOR_MICROSEMI
    > depends on HAS_IOMEM
    > depends on PTP_1588_CLOCK_OPTIONAL
    > + select NET_DSA_MSCC_CORE
    > select MDIO_MSCC_MIIM
    > - select MSCC_OCELOT_SWITCH_LIB
    > - select NET_DSA_TAG_OCELOT_8021Q
    > - select NET_DSA_TAG_OCELOT
    > - select PCS_LYNX
    > help
    > This driver supports the VSC9953 (Seville) switch, which is embedded
    > as a platform device on the NXP T1040 SoC.
    > diff --git a/drivers/net/dsa/ocelot/Makefile b/drivers/net/dsa/ocelot/Makefile
    > index f6dd131e7491..f8c74b59b996 100644
    > --- a/drivers/net/dsa/ocelot/Makefile
    > +++ b/drivers/net/dsa/ocelot/Makefile
    > @@ -1,11 +1,9 @@
    > # SPDX-License-Identifier: GPL-2.0-only
    > +
    > +obj-$(CONFIG_NET_DSA_MSCC_CORE) += mscc_core.o
    > obj-$(CONFIG_NET_DSA_MSCC_FELIX) += mscc_felix.o
    > obj-$(CONFIG_NET_DSA_MSCC_SEVILLE) += mscc_seville.o
    >
    > -mscc_felix-objs := \
    > - felix.o \
    > - felix_vsc9959.o
    > -
    > -mscc_seville-objs := \
    > - felix.o \
    > - seville_vsc9953.o
    > +mscc_core-objs := felix.o
    > +mscc_felix-objs := felix_vsc9959.o
    > +mscc_seville-objs := seville_vsc9953.o
    > diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
    > index dd3a18cc89dd..f9d0a24ebc3a 100644
    > --- a/drivers/net/dsa/ocelot/felix.c
    > +++ b/drivers/net/dsa/ocelot/felix.c
    > @@ -2112,6 +2112,7 @@ const struct dsa_switch_ops felix_switch_ops = {
    > .port_set_host_flood = felix_port_set_host_flood,
    > .port_change_master = felix_port_change_master,
    > };
    > +EXPORT_SYMBOL_NS_GPL(felix_switch_ops, NET_DSA_MSCC_CORE);

    What do we gain practically with the symbol namespacing?

    >
    > struct net_device *felix_port_to_netdev(struct ocelot *ocelot, int port)
    > {
    > @@ -2123,6 +2124,7 @@ struct net_device *felix_port_to_netdev(struct ocelot *ocelot, int port)
    >
    > return dsa_to_port(ds, port)->slave;
    > }
    > +EXPORT_SYMBOL_NS_GPL(felix_port_to_netdev, NET_DSA_MSCC_CORE);
    >
    > int felix_netdev_to_port(struct net_device *dev)
    > {
    > @@ -2134,3 +2136,7 @@ int felix_netdev_to_port(struct net_device *dev)
    >
    > return dp->index;
    > }
    > +EXPORT_SYMBOL_NS_GPL(felix_netdev_to_port, NET_DSA_MSCC_CORE);
    > +
    > +MODULE_DESCRIPTION("MSCC Switch driver core");

    Ocelot switch DSA library

    > +MODULE_LICENSE("GPL");
    > diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
    > index 26a35ae322d1..52c8bff79fa3 100644
    > --- a/drivers/net/dsa/ocelot/felix_vsc9959.c
    > +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
    > @@ -2736,5 +2736,7 @@ static struct pci_driver felix_vsc9959_pci_driver = {
    > };
    > module_pci_driver(felix_vsc9959_pci_driver);
    >
    > +MODULE_IMPORT_NS(NET_DSA_MSCC_CORE);
    > +
    > MODULE_DESCRIPTION("Felix Switch driver");
    > MODULE_LICENSE("GPL v2");
    > diff --git a/drivers/net/dsa/ocelot/seville_vsc9953.c b/drivers/net/dsa/ocelot/seville_vsc9953.c
    > index 7af33b2c685d..e9c63c642489 100644
    > --- a/drivers/net/dsa/ocelot/seville_vsc9953.c
    > +++ b/drivers/net/dsa/ocelot/seville_vsc9953.c
    > @@ -1115,5 +1115,7 @@ static struct platform_driver seville_vsc9953_driver = {
    > };
    > module_platform_driver(seville_vsc9953_driver);
    >
    > +MODULE_IMPORT_NS(NET_DSA_MSCC_CORE);
    > +
    > MODULE_DESCRIPTION("Seville Switch driver");
    > MODULE_LICENSE("GPL v2");
    > --
    > 2.38.1
    >
    >

    \
     
     \ /
      Last update: 2022-11-21 18:55    [W:2.340 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site