Messages in this thread | | | Date | Tue, 25 Jul 2023 23:59:33 +0300 | From | Vladimir Oltean <> | Subject | Re: [PATCH] [v2] dpaa: avoid linking objects into multiple modules |
| |
Hi Arnd,
On Tue, Jul 25, 2023 at 10:39:40PM +0200, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > Each object file contains information about which module it gets linked > into, so linking the same file into multiple modules now causes a warning: > > scripts/Makefile.build:254: drivers/net/ethernet/freescale/dpaa2/Makefile: dpaa2-mac.o is added to multiple modules: fsl-dpaa2-eth fsl-dpaa2-switch > scripts/Makefile.build:254: drivers/net/ethernet/freescale/dpaa2/Makefile: dpmac.o is added to multiple modules: fsl-dpaa2-eth fsl-dpaa2-switch > > Chang the way that dpaa2 is built by moving the two common files into a > separate module with exported symbols instead. > > To avoid a link failure when the switch driver is built-in, but the dpio driver > is a loadable module, add the same dependency in there that exists for > the ethernet driver.
Unrelated change should be separate.
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > v2: add missing module description
There was a previous attempt at this before, by Alexander Lobakin: https://patchwork.kernel.org/project/netdevbpf/patch/20221119225650.1044591-16-alobakin@pm.me/
but it seems he wasn't persistent enough with it.
Something which I liked better in his proposal was the naming of the config option (FSL_DPAA2_MAC) and of the new module (fsl-dpaa2-mac).
Something which I like better in your proposal is the straightforward EXPORT_SYMBOL_GPL rather than EXPORT_SYMBOL_NS_GPL() whose purpose I did not understand.
A mix of the 2 would be great.
> --- > drivers/net/ethernet/freescale/Makefile | 4 +--- > drivers/net/ethernet/freescale/dpaa2/Kconfig | 1 + > drivers/net/ethernet/freescale/dpaa2/Makefile | 9 +++++---- > drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c | 12 ++++++++++++ > 4 files changed, 19 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/ethernet/freescale/Makefile b/drivers/net/ethernet/freescale/Makefile > index de7b318422330..c63e0c090f8f7 100644 > --- a/drivers/net/ethernet/freescale/Makefile > +++ b/drivers/net/ethernet/freescale/Makefile > @@ -22,6 +22,4 @@ ucc_geth_driver-objs := ucc_geth.o ucc_geth_ethtool.o > obj-$(CONFIG_FSL_FMAN) += fman/ > obj-$(CONFIG_FSL_DPAA_ETH) += dpaa/ > > -obj-$(CONFIG_FSL_DPAA2_ETH) += dpaa2/ > - > -obj-y += enetc/ > +obj-y += enetc/ dpaa2/ > diff --git a/drivers/net/ethernet/freescale/dpaa2/Kconfig b/drivers/net/ethernet/freescale/dpaa2/Kconfig > index d029b69c3f183..4e26b5a4bc5c4 100644 > --- a/drivers/net/ethernet/freescale/dpaa2/Kconfig > +++ b/drivers/net/ethernet/freescale/dpaa2/Kconfig > @@ -32,6 +32,7 @@ config FSL_DPAA2_PTP_CLOCK > > config FSL_DPAA2_SWITCH > tristate "Freescale DPAA2 Ethernet Switch" > + depends on FSL_MC_BUS && FSL_MC_DPIO > depends on BRIDGE || BRIDGE=n > depends on NET_SWITCHDEV > help > diff --git a/drivers/net/ethernet/freescale/dpaa2/Makefile b/drivers/net/ethernet/freescale/dpaa2/Makefile > index 1b05ba8d1cbff..c042d2c27926c 100644 > --- a/drivers/net/ethernet/freescale/dpaa2/Makefile > +++ b/drivers/net/ethernet/freescale/dpaa2/Makefile > @@ -3,15 +3,16 @@ > # Makefile for the Freescale DPAA2 Ethernet controller > # > > -obj-$(CONFIG_FSL_DPAA2_ETH) += fsl-dpaa2-eth.o > +obj-$(CONFIG_FSL_DPAA2_ETH) += fsl-dpaa2-eth.o fsl-dpaa2-common.o > obj-$(CONFIG_FSL_DPAA2_PTP_CLOCK) += fsl-dpaa2-ptp.o > -obj-$(CONFIG_FSL_DPAA2_SWITCH) += fsl-dpaa2-switch.o > +obj-$(CONFIG_FSL_DPAA2_SWITCH) += fsl-dpaa2-switch.o fsl-dpaa2-common.o
I am not completely sure how this works. Can fsl-dpaa2-common appear both in obj-y (added by $(CONFIG_FSL_DPAA2_ETH)) and in obj-m (added by $(CONFIG_FSL_DPAA2_SWITCH))? Olek proposed the introduction of a new, hidden and selectable tristate FSL_DPAA2_MAC. At least I understand how that works.
> > -fsl-dpaa2-eth-objs := dpaa2-eth.o dpaa2-ethtool.o dpni.o dpaa2-mac.o dpmac.o dpaa2-eth-devlink.o dpaa2-xsk.o > +fsl-dpaa2-eth-objs := dpaa2-eth.o dpaa2-ethtool.o dpni.o dpaa2-eth-devlink.o dpaa2-xsk.o > fsl-dpaa2-eth-${CONFIG_FSL_DPAA2_ETH_DCB} += dpaa2-eth-dcb.o > fsl-dpaa2-eth-${CONFIG_DEBUG_FS} += dpaa2-eth-debugfs.o > fsl-dpaa2-ptp-objs := dpaa2-ptp.o dprtc.o > -fsl-dpaa2-switch-objs := dpaa2-switch.o dpaa2-switch-ethtool.o dpsw.o dpaa2-switch-flower.o dpaa2-mac.o dpmac.o > +fsl-dpaa2-switch-objs := dpaa2-switch.o dpaa2-switch-ethtool.o dpsw.o dpaa2-switch-flower.o > +fsl-dpaa2-common-objs += dpaa2-mac.o dpmac.o > > # Needed by the tracing framework > CFLAGS_dpaa2-eth.o := -I$(src) > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c > index a69bb22c37eab..c51dbd84d199e 100644 > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c > @@ -348,6 +348,7 @@ void dpaa2_mac_start(struct dpaa2_mac *mac) > > phylink_start(mac->phylink); > } > +EXPORT_SYMBOL_GPL(dpaa2_mac_start); > > void dpaa2_mac_stop(struct dpaa2_mac *mac) > { > @@ -358,6 +359,7 @@ void dpaa2_mac_stop(struct dpaa2_mac *mac) > if (mac->serdes_phy) > phy_power_off(mac->serdes_phy); > } > +EXPORT_SYMBOL_GPL(dpaa2_mac_stop); > > int dpaa2_mac_connect(struct dpaa2_mac *mac) > { > @@ -450,6 +452,7 @@ int dpaa2_mac_connect(struct dpaa2_mac *mac) > > return err; > } > +EXPORT_SYMBOL_GPL(dpaa2_mac_connect); > > void dpaa2_mac_disconnect(struct dpaa2_mac *mac) > { > @@ -462,6 +465,7 @@ void dpaa2_mac_disconnect(struct dpaa2_mac *mac) > of_phy_put(mac->serdes_phy); > mac->serdes_phy = NULL; > } > +EXPORT_SYMBOL_GPL(dpaa2_mac_disconnect); > > int dpaa2_mac_open(struct dpaa2_mac *mac) > { > @@ -510,6 +514,7 @@ int dpaa2_mac_open(struct dpaa2_mac *mac) > dpmac_close(mac->mc_io, 0, dpmac_dev->mc_handle); > return err; > } > +EXPORT_SYMBOL_GPL(dpaa2_mac_open); > > void dpaa2_mac_close(struct dpaa2_mac *mac) > { > @@ -519,6 +524,7 @@ void dpaa2_mac_close(struct dpaa2_mac *mac) > if (mac->fw_node) > fwnode_handle_put(mac->fw_node); > } > +EXPORT_SYMBOL_GPL(dpaa2_mac_close); > > static char dpaa2_mac_ethtool_stats[][ETH_GSTRING_LEN] = { > [DPMAC_CNT_ING_ALL_FRAME] = "[mac] rx all frames", > @@ -557,6 +563,7 @@ int dpaa2_mac_get_sset_count(void) > { > return DPAA2_MAC_NUM_STATS; > } > +EXPORT_SYMBOL_GPL(dpaa2_mac_get_sset_count); > > void dpaa2_mac_get_strings(u8 *data) > { > @@ -568,6 +575,7 @@ void dpaa2_mac_get_strings(u8 *data) > p += ETH_GSTRING_LEN; > } > } > +EXPORT_SYMBOL_GPL(dpaa2_mac_get_strings); > > void dpaa2_mac_get_ethtool_stats(struct dpaa2_mac *mac, u64 *data) > { > @@ -587,3 +595,7 @@ void dpaa2_mac_get_ethtool_stats(struct dpaa2_mac *mac, u64 *data) > *(data + i) = value; > } > } > +EXPORT_SYMBOL_GPL(dpaa2_mac_get_ethtool_stats); > + > +MODULE_LICENSE("GPL"); > +MODULE_DESCRIPTION("DPAA2 Ethernet core library");
s/core/MAC/ everywhere
> -- > 2.39.2 >
Also, the commit message prefix is wrong. "dpaa:" gets used for "drivers/net/ethernet/freescale/dpaa/". DPAA(1) and DPAA2 have as much in common as King Charles II and Charles III.
pw-bot: cr
| |