| Message ID | 20200504165711.5621-1-clay@daemons.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] net: ethernet: ti: Remove TI_CPTS_MOD workaround | expand |
On 04/05/2020 19:57, Clay McClure wrote: > My recent commit b6d49cab44b5 ("net: Make PTP-specific drivers depend on PTP_1588_CLOCK") > exposes a missing dependency in defconfigs that select TI_CPTS without > selecting PTP_1588_CLOCK, leading to linker errors of the form: > > drivers/net/ethernet/ti/cpsw.o: in function `cpsw_ndo_stop': > cpsw.c:(.text+0x680): undefined reference to `cpts_unregister' > drivers/net/ethernet/ti/cpsw.o: in function `cpsw_remove': > cpsw.c:(.text+0x81c): undefined reference to `cpts_release' > drivers/net/ethernet/ti/cpsw.o: in function `cpsw_rx_handler': > cpsw.c:(.text+0x1324): undefined reference to `cpts_rx_timestamp' > drivers/net/ethernet/ti/cpsw.o: in function `cpsw_ndo_open': > cpsw.c:(.text+0x15ec): undefined reference to `cpts_register' > drivers/net/ethernet/ti/cpsw.o: in function `cpsw_probe': > cpsw.c:(.text+0x2468): undefined reference to `cpts_release' > > That's because TI_CPTS_MOD (which is the symbol gating the _compilation_ > of cpts.c) now depends on PTP_1588_CLOCK, and so is not enabled in these > configurations, but TI_CPTS (which is the symbol gating _calls_ to the > cpts functions) _is_ enabled. So we end up compiling calls to functions > that don't exist, resulting in the linker errors. > > The reason we have two symbols (TI_CPTS and TI_CPTS_MOD) for the same > driver is due to commit be9ca0d33c85 ("cpsw/netcp: work around reverse > cpts dependency"), which introduced TI_CPTS_MOD because (quoting the > commit message): > >> The dependency is reversed: cpsw and netcp call into cpts, >> but cpts depends on the other two in Kconfig. This can lead >> to cpts being a loadable module and its callers built-in: >> >> drivers/net/ethernet/ti/cpsw.o: In function `cpsw_remove': >> cpsw.c:(.text.cpsw_remove+0xd0): undefined reference to `cpts_release' >> drivers/net/ethernet/ti/cpsw.o: In function `cpsw_rx_handler': >> cpsw.c:(.text.cpsw_rx_handler+0x2dc): undefined reference to `cpts_rx_timestamp' >> drivers/net/ethernet/ti/cpsw.o: In function `cpsw_tx_handler': >> cpsw.c:(.text.cpsw_tx_handler+0x7c): undefined reference to `cpts_tx_timestamp' >> drivers/net/ethernet/ti/cpsw.o: In function `cpsw_ndo_stop': > > Both forms of linker error -- those caused by defconfigs that select > TI_CPTS without PTP_1588_CLOCK and those caused by configuring TI_CPSW > as a built-in and TI_CPTS as a module -- can be avoided by using the > IS_REACHABLE() macro to gate calls to cpts functions, and using the > TI_CPTS symbol to gate compilation of cpts.c. cpts.h already provides > the no-op stub implementations of the cpts functions required to make > this work, we just need to change the existing IS_ENABLED(TI_CPTS) > guards to IS_REACHABLE(TI_CPTS). > > With this change there is no longer any need for the TI_CPTS_MOD symbol, > so we can remove it. > > To preserve the existing behavior of defconfigs that select TI_CPTS, we > must also select PTP_1588_CLOCK so that the dependency is satisfied. > > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Grygorii Strashko <grygorii.strashko@ti.com> > Fixes: b6d49cab44b5 ("net: Make PTP-specific drivers depend on PTP_1588_CLOCK") It's better if you send v2 not as reply to v1. just to clarify. After these two patches - the PTP_1588_CLOCK can still be set to "M" - which will cause TI_CPTS to be "M", - but TI_CPSW will still be "Y". and all above will build and produce built-in CPSW without CPTS support and cpts.ko which is loadable, but not functional. Sorry, I'm a little bit lost regarding the target you'are trying to achieve. At least previously "imply PTP_1588_CLOCK" allowed to select properly PTP_1588_CLOCK without modifying every defconfig. > Reported-by: kbuild test robot <lkp@intel.com> > Signed-off-by: Clay McClure <clay@daemons.net> > --- > Changes in v2: > > - Don't regenerate the defconfigs, just add PTP_1588_CLOCK. > > arch/arm/configs/keystone_defconfig | 1 + > arch/arm/configs/omap2plus_defconfig | 1 + > drivers/net/ethernet/ti/Kconfig | 13 ++++--------- > drivers/net/ethernet/ti/Makefile | 2 +- > drivers/net/ethernet/ti/cpsw_ethtool.c | 2 +- > drivers/net/ethernet/ti/cpts.h | 3 +-- > drivers/net/ethernet/ti/netcp_ethss.c | 10 +++++----- > 7 files changed, 14 insertions(+), 18 deletions(-) > > diff --git a/arch/arm/configs/keystone_defconfig b/arch/arm/configs/keystone_defconfig > index 11e2211f9007..84a3b055f253 100644 > --- a/arch/arm/configs/keystone_defconfig > +++ b/arch/arm/configs/keystone_defconfig > @@ -147,6 +147,7 @@ CONFIG_I2C_DAVINCI=y > CONFIG_SPI=y > CONFIG_SPI_DAVINCI=y > CONFIG_SPI_SPIDEV=y > +CONFIG_PTP_1588_CLOCK=y > CONFIG_PINCTRL_SINGLE=y > CONFIG_GPIOLIB=y > CONFIG_GPIO_SYSFS=y > diff --git a/arch/arm/configs/omap2plus_defconfig b/arch/arm/configs/omap2plus_defconfig > index 3cc3ca5fa027..8b83d4a5d309 100644 > --- a/arch/arm/configs/omap2plus_defconfig > +++ b/arch/arm/configs/omap2plus_defconfig > @@ -274,6 +274,7 @@ CONFIG_SPI_TI_QSPI=m > CONFIG_HSI=m > CONFIG_OMAP_SSI=m > CONFIG_SSI_PROTOCOL=m > +CONFIG_PTP_1588_CLOCK=y > CONFIG_PINCTRL_SINGLE=y > CONFIG_DEBUG_GPIO=y > CONFIG_GPIO_SYSFS=y > diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig > index 8e348780efb6..f3f8bb724294 100644 > --- a/drivers/net/ethernet/ti/Kconfig > +++ b/drivers/net/ethernet/ti/Kconfig > @@ -77,23 +77,18 @@ config TI_CPSW_SWITCHDEV > will be called cpsw_new. > > config TI_CPTS > - bool "TI Common Platform Time Sync (CPTS) Support" > + tristate "TI Common Platform Time Sync (CPTS) Support" > depends on TI_CPSW || TI_KEYSTONE_NETCP || TI_CPSW_SWITCHDEV || COMPILE_TEST > depends on COMMON_CLK > - depends on POSIX_TIMERS > + depends on PTP_1588_CLOCK > + default y if TI_CPSW=y || TI_KEYSTONE_NETCP=y || TI_CPSW_SWITCHDEV=y even with above statement it's possible to force TI_CPTS="M" while CPSW/NETCP="Y" > + default m I could be mistaken by above 2 lines seems can be 'imply TI_CPTS' in TI_CPSW, TI_KEYSTONE_NETCP, TI_CPSW_SWITCHDEV > ---help--- > This driver supports the Common Platform Time Sync unit of > the CPSW Ethernet Switch and Keystone 2 1g/10g Switch Subsystem. > The unit can time stamp PTP UDP/IPv4 and Layer 2 packets, and the > driver offers a PTP Hardware Clock. > > -config TI_CPTS_MOD > - tristate > - depends on TI_CPTS > - depends on PTP_1588_CLOCK > - default y if TI_CPSW=y || TI_KEYSTONE_NETCP=y || TI_CPSW_SWITCHDEV=y > - default m and this prevented user from forcing TI_CPTS="M" while CPSW/NETCP="Y" > - > config TI_K3_AM65_CPSW_NUSS > tristate "TI K3 AM654x/J721E CPSW Ethernet driver" > depends on ARCH_K3 && OF && TI_K3_UDMA_GLUE_LAYER > diff --git a/drivers/net/ethernet/ti/Makefile b/drivers/net/ethernet/ti/Makefile > index 53792190e9c2..cb26a9d21869 100644 Below small diff should fix build fail: diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig index 8e348780efb6..eeaee47598aa 100644 --- a/drivers/net/ethernet/ti/Kconfig +++ b/drivers/net/ethernet/ti/Kconfig @@ -81,6 +81,7 @@ config TI_CPTS depends on TI_CPSW || TI_KEYSTONE_NETCP || TI_CPSW_SWITCHDEV || COMPILE_TEST depends on COMMON_CLK depends on POSIX_TIMERS + depends on PTP_1588_CLOCK ---help--- This driver supports the Common Platform Time Sync unit of the CPSW Ethernet Switch and Keystone 2 1g/10g Switch Subsystem. @@ -90,7 +91,6 @@ config TI_CPTS config TI_CPTS_MOD tristate depends on TI_CPTS - depends on PTP_1588_CLOCK default y if TI_CPSW=y || TI_KEYSTONE_NETCP=y || TI_CPSW_SWITCHDEV=y default m Then separate patch can be used to enable PTP_1588_CLOCK in defconfigs. My personal opinion - it might be better to revert TI CPTS part from b6d49cab44b5 ("net: Make PTP-specific drivers depend on PTP_1588_CLOCK") at all.
On Tue, May 05, 2020 at 10:41:26AM +0300, Grygorii Strashko wrote: > It's better if you send v2 not as reply to v1. Noted, thank you, and thank you for taking the time to review my patch. > just to clarify. After these two patches > - the PTP_1588_CLOCK can still be set to "M" > - which will cause TI_CPTS to be "M", > - but TI_CPSW will still be "Y". > > and all above will build and produce built-in CPSW without CPTS support > and cpts.ko which is loadable, but not functional. > > Sorry, I'm a little bit lost regarding the target you'are trying to achieve. > At least previously "imply PTP_1588_CLOCK" allowed to select properly PTP_1588_CLOCK > without modifying every defconfig. Well, I just wanted to squelch a WARN_ON(). As Arnd pointed out in [1], code that uses the stubbed cpts functions is supposed to gracefully handle receiving a null pointer. Splatting a warning is not graceful, and that's what I was trying to fix. But your question in [2] prompted me to consider whether it should be possible to build TI_CPTS without PTP_1588_CLOCK at all. I think the answer is no, so I tried to fix it, but you're right, it's still possible to end up with a nonfunctional module after my patch. If you prefer to revert, that's fine with me. Should I post a patch, or just ask David to revert?
On 06/05/2020 09:51, Clay McClure wrote: > On Tue, May 05, 2020 at 10:41:26AM +0300, Grygorii Strashko wrote: > >> It's better if you send v2 not as reply to v1. > > Noted, thank you, and thank you for taking the time to review my patch. > >> just to clarify. After these two patches >> - the PTP_1588_CLOCK can still be set to "M" >> - which will cause TI_CPTS to be "M", >> - but TI_CPSW will still be "Y". >> >> and all above will build and produce built-in CPSW without CPTS support >> and cpts.ko which is loadable, but not functional. >> >> Sorry, I'm a little bit lost regarding the target you'are trying to achieve. >> At least previously "imply PTP_1588_CLOCK" allowed to select properly PTP_1588_CLOCK >> without modifying every defconfig. > > Well, I just wanted to squelch a WARN_ON(). As Arnd pointed out in [1], > code that uses the stubbed cpts functions is supposed to gracefully > handle receiving a null pointer. Splatting a warning is not graceful, > and that's what I was trying to fix. > > But your question in [2] prompted me to consider whether it should be > possible to build TI_CPTS without PTP_1588_CLOCK at all. I think the > answer is no, so I tried to fix it, but you're right, it's still > possible to end up with a nonfunctional module after my patch. > > If you prefer to revert, that's fine with me. Should I post a patch, or > just ask David to revert? > Ok. After some thinking and hence you commit b6d49cab44b5 ("net: Make PTP-specific drivers depend on PTP_1588_CLOCK") seems was merged in net (not net-next) 1) for-net: defconfig changes can be separated to fix build fail, but add change for multi_v7_defconfig 2) for-net-next: rest of changes plus below diff on top of it diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig index f3f8bb724294..62f809b67469 100644 --- a/drivers/net/ethernet/ti/Kconfig +++ b/drivers/net/ethernet/ti/Kconfig @@ -49,6 +49,7 @@ config TI_CPSW_PHY_SEL config TI_CPSW tristate "TI CPSW Switch Support" depends on ARCH_DAVINCI || ARCH_OMAP2PLUS || COMPILE_TEST + depends on TI_CPTS || !TI_CPTS select TI_DAVINCI_MDIO select MFD_SYSCON select PAGE_POOL @@ -64,6 +65,7 @@ config TI_CPSW_SWITCHDEV tristate "TI CPSW Switch Support with switchdev" depends on ARCH_DAVINCI || ARCH_OMAP2PLUS || COMPILE_TEST depends on NET_SWITCHDEV + depends on TI_CPTS || !TI_CPTS select PAGE_POOL select TI_DAVINCI_MDIO select MFD_SYSCON @@ -78,11 +80,9 @@ config TI_CPSW_SWITCHDEV config TI_CPTS tristate "TI Common Platform Time Sync (CPTS) Support" - depends on TI_CPSW || TI_KEYSTONE_NETCP || TI_CPSW_SWITCHDEV || COMPILE_TEST + depends on ARCH_OMAP2PLUS || ARCH_KEYSTONE || COMPILE_TEST depends on COMMON_CLK depends on PTP_1588_CLOCK - default y if TI_CPSW=y || TI_KEYSTONE_NETCP=y || TI_CPSW_SWITCHDEV=y - default m ---help--- This driver supports the Common Platform Time Sync unit of the CPSW Ethernet Switch and Keystone 2 1g/10g Switch Subsystem. @@ -109,6 +109,7 @@ config TI_KEYSTONE_NETCP select TI_DAVINCI_MDIO depends on OF depends on KEYSTONE_NAVIGATOR_DMA && KEYSTONE_NAVIGATOR_QMSS + depends on TI_CPTS || !TI_CPTS ---help--- This driver supports TI's Keystone NETCP Core. It should properly resolve "M" vs "Y" dependencies between PTP_1588_CLOCK->TI_CPTS->TI_CPSW On thing, TI_CPTS can be selected without TI_CPSW, but it's probably ok.
On Wed, May 6, 2020 at 10:57 PM Grygorii Strashko <grygorii.strashko@ti.com> wrote: > On 06/05/2020 09:51, Clay McClure wrote: > > On Tue, May 05, 2020 at 10:41:26AM +0300, Grygorii Strashko wrote: > > > > Ok. After some thinking and hence you commit b6d49cab44b5 ("net: Make PTP-specific drivers depend on PTP_1588_CLOCK") > seems was merged in net (not net-next) > 1) for-net: defconfig changes can be separated to fix build fail, but add change for multi_v7_defconfig > 2) for-net-next: rest of changes plus below diff on top of it > > diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig > index f3f8bb724294..62f809b67469 100644 > --- a/drivers/net/ethernet/ti/Kconfig > +++ b/drivers/net/ethernet/ti/Kconfig > @@ -49,6 +49,7 @@ config TI_CPSW_PHY_SEL > config TI_CPSW > tristate "TI CPSW Switch Support" > depends on ARCH_DAVINCI || ARCH_OMAP2PLUS || COMPILE_TEST > + depends on TI_CPTS || !TI_CPTS > select TI_DAVINCI_MDIO > select MFD_SYSCON > select PAGE_POOL > @@ -64,6 +65,7 @@ config TI_CPSW_SWITCHDEV > tristate "TI CPSW Switch Support with switchdev" > depends on ARCH_DAVINCI || ARCH_OMAP2PLUS || COMPILE_TEST > depends on NET_SWITCHDEV > + depends on TI_CPTS || !TI_CPTS > select PAGE_POOL > select TI_DAVINCI_MDIO > select MFD_SYSCON > @@ -78,11 +80,9 @@ config TI_CPSW_SWITCHDEV > > config TI_CPTS > tristate "TI Common Platform Time Sync (CPTS) Support" > - depends on TI_CPSW || TI_KEYSTONE_NETCP || TI_CPSW_SWITCHDEV || COMPILE_TEST > + depends on ARCH_OMAP2PLUS || ARCH_KEYSTONE || COMPILE_TEST > depends on COMMON_CLK > depends on PTP_1588_CLOCK > - default y if TI_CPSW=y || TI_KEYSTONE_NETCP=y || TI_CPSW_SWITCHDEV=y > - default m > ---help--- > This driver supports the Common Platform Time Sync unit of > the CPSW Ethernet Switch and Keystone 2 1g/10g Switch Subsystem. > @@ -109,6 +109,7 @@ config TI_KEYSTONE_NETCP > select TI_DAVINCI_MDIO > depends on OF > depends on KEYSTONE_NAVIGATOR_DMA && KEYSTONE_NAVIGATOR_QMSS > + depends on TI_CPTS || !TI_CPTS > ---help--- > This driver supports TI's Keystone NETCP Core. > > It should properly resolve "M" vs "Y" dependencies between > PTP_1588_CLOCK->TI_CPTS->TI_CPSW > > On thing, TI_CPTS can be selected without TI_CPSW, but it's probably ok. I think that solution is reasonable. I had come up with a different for when I ran into the build failure, but I like yours better. Here is my patch, for reference: commit 94233d78655876f735189890eb40ef33c49e8523 (HEAD -> randconfig) Author: Arnd Bergmann <arnd@arndb.de> Date: Thu May 7 21:25:59 2020 +0200 cpsw: fix cpts link failure When CONFIG_PTP_1588_CLOCK is a loadable module, trying to build cpts support into the built-in cpsw driver results in a link error: arm-linux-gnueabi-ld: drivers/net/ethernet/ti/cpsw.o: in function `cpsw_probe': cpsw.c:(.text+0x918): undefined reference to `cpts_release' arm-linux-gnueabi-ld: drivers/net/ethernet/ti/cpsw.o: in function `cpsw_remove': cpsw.c:(.text+0x1048): undefined reference to `cpts_release' arm-linux-gnueabi-ld: drivers/net/ethernet/ti/cpsw.o: in function `cpsw_rx_handler': cpsw.c:(.text+0x12c0): undefined reference to `cpts_rx_timestamp' arm-linux-gnueabi-ld: drivers/net/ethernet/ti/cpsw.o: in function `cpsw_ndo_open': Add a dependency for CPTS that only allows it to be enabled when doing so does not cause link-time probles. Fixes: b6d49cab44b5 ("net: Make PTP-specific drivers depend on PTP_1588_CLOCK") Signed-off-by: Arnd Bergmann <arnd@arndb.de> diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig index 4ab35ce7b451..571caf4192c5 100644 --- a/drivers/net/ethernet/ti/Kconfig +++ b/drivers/net/ethernet/ti/Kconfig @@ -79,6 +79,9 @@ config TI_CPSW_SWITCHDEV config TI_CPTS bool "TI Common Platform Time Sync (CPTS) Support" depends on TI_CPSW || TI_KEYSTONE_NETCP || TI_CPSW_SWITCHDEV || COMPILE_TEST + depends on TI_CPSW=n || TI_CPSW=PTP_1588_CLOCK || PTP_1588_CLOCK=y + depends on TI_KEYSTONE_NETCP=n || TI_KEYSTONE_NETCP=PTP_1588_CLOCK || PTP_1588_CLOCK=y + depends on TI_CPSW_SWITCHDEV=n || TI_CPSW_SWITCHDEV=PTP_1588_CLOCK || PTP_1588_CLOCK=y depends on COMMON_CLK depends on POSIX_TIMERS ---help---
diff --git a/arch/arm/configs/keystone_defconfig b/arch/arm/configs/keystone_defconfig index 11e2211f9007..84a3b055f253 100644 --- a/arch/arm/configs/keystone_defconfig +++ b/arch/arm/configs/keystone_defconfig @@ -147,6 +147,7 @@ CONFIG_I2C_DAVINCI=y CONFIG_SPI=y CONFIG_SPI_DAVINCI=y CONFIG_SPI_SPIDEV=y +CONFIG_PTP_1588_CLOCK=y CONFIG_PINCTRL_SINGLE=y CONFIG_GPIOLIB=y CONFIG_GPIO_SYSFS=y diff --git a/arch/arm/configs/omap2plus_defconfig b/arch/arm/configs/omap2plus_defconfig index 3cc3ca5fa027..8b83d4a5d309 100644 --- a/arch/arm/configs/omap2plus_defconfig +++ b/arch/arm/configs/omap2plus_defconfig @@ -274,6 +274,7 @@ CONFIG_SPI_TI_QSPI=m CONFIG_HSI=m CONFIG_OMAP_SSI=m CONFIG_SSI_PROTOCOL=m +CONFIG_PTP_1588_CLOCK=y CONFIG_PINCTRL_SINGLE=y CONFIG_DEBUG_GPIO=y CONFIG_GPIO_SYSFS=y diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig index 8e348780efb6..f3f8bb724294 100644 --- a/drivers/net/ethernet/ti/Kconfig +++ b/drivers/net/ethernet/ti/Kconfig @@ -77,23 +77,18 @@ config TI_CPSW_SWITCHDEV will be called cpsw_new. config TI_CPTS - bool "TI Common Platform Time Sync (CPTS) Support" + tristate "TI Common Platform Time Sync (CPTS) Support" depends on TI_CPSW || TI_KEYSTONE_NETCP || TI_CPSW_SWITCHDEV || COMPILE_TEST depends on COMMON_CLK - depends on POSIX_TIMERS + depends on PTP_1588_CLOCK + default y if TI_CPSW=y || TI_KEYSTONE_NETCP=y || TI_CPSW_SWITCHDEV=y + default m ---help--- This driver supports the Common Platform Time Sync unit of the CPSW Ethernet Switch and Keystone 2 1g/10g Switch Subsystem. The unit can time stamp PTP UDP/IPv4 and Layer 2 packets, and the driver offers a PTP Hardware Clock. -config TI_CPTS_MOD - tristate - depends on TI_CPTS - depends on PTP_1588_CLOCK - default y if TI_CPSW=y || TI_KEYSTONE_NETCP=y || TI_CPSW_SWITCHDEV=y - default m - config TI_K3_AM65_CPSW_NUSS tristate "TI K3 AM654x/J721E CPSW Ethernet driver" depends on ARCH_K3 && OF && TI_K3_UDMA_GLUE_LAYER diff --git a/drivers/net/ethernet/ti/Makefile b/drivers/net/ethernet/ti/Makefile index 53792190e9c2..cb26a9d21869 100644 --- a/drivers/net/ethernet/ti/Makefile +++ b/drivers/net/ethernet/ti/Makefile @@ -13,7 +13,7 @@ obj-$(CONFIG_TI_DAVINCI_EMAC) += ti_davinci_emac.o ti_davinci_emac-y := davinci_emac.o davinci_cpdma.o obj-$(CONFIG_TI_DAVINCI_MDIO) += davinci_mdio.o obj-$(CONFIG_TI_CPSW_PHY_SEL) += cpsw-phy-sel.o -obj-$(CONFIG_TI_CPTS_MOD) += cpts.o +obj-$(CONFIG_TI_CPTS) += cpts.o obj-$(CONFIG_TI_CPSW) += ti_cpsw.o ti_cpsw-y := cpsw.o davinci_cpdma.o cpsw_ale.o cpsw_priv.o cpsw_sl.o cpsw_ethtool.o obj-$(CONFIG_TI_CPSW_SWITCHDEV) += ti_cpsw_new.o diff --git a/drivers/net/ethernet/ti/cpsw_ethtool.c b/drivers/net/ethernet/ti/cpsw_ethtool.c index fa54efe3be63..19a7370a4188 100644 --- a/drivers/net/ethernet/ti/cpsw_ethtool.c +++ b/drivers/net/ethernet/ti/cpsw_ethtool.c @@ -709,7 +709,7 @@ int cpsw_set_ringparam(struct net_device *ndev, return ret; } -#if IS_ENABLED(CONFIG_TI_CPTS) +#if IS_REACHABLE(CONFIG_TI_CPTS) int cpsw_get_ts_info(struct net_device *ndev, struct ethtool_ts_info *info) { struct cpsw_common *cpsw = ndev_to_cpsw(ndev); diff --git a/drivers/net/ethernet/ti/cpts.h b/drivers/net/ethernet/ti/cpts.h index bb997c11ee15..782e24c78e7a 100644 --- a/drivers/net/ethernet/ti/cpts.h +++ b/drivers/net/ethernet/ti/cpts.h @@ -8,7 +8,7 @@ #ifndef _TI_CPTS_H_ #define _TI_CPTS_H_ -#if IS_ENABLED(CONFIG_TI_CPTS) +#if IS_REACHABLE(CONFIG_TI_CPTS) #include <linux/clk.h> #include <linux/clkdev.h> @@ -171,5 +171,4 @@ static inline bool cpts_can_timestamp(struct cpts *cpts, struct sk_buff *skb) } #endif - #endif diff --git a/drivers/net/ethernet/ti/netcp_ethss.c b/drivers/net/ethernet/ti/netcp_ethss.c index fb36115e9c51..3de1d25128b7 100644 --- a/drivers/net/ethernet/ti/netcp_ethss.c +++ b/drivers/net/ethernet/ti/netcp_ethss.c @@ -181,7 +181,7 @@ #define HOST_TX_PRI_MAP_DEFAULT 0x00000000 -#if IS_ENABLED(CONFIG_TI_CPTS) +#if IS_REACHABLE(CONFIG_TI_CPTS) /* Px_TS_CTL register fields */ #define TS_RX_ANX_F_EN BIT(0) #define TS_RX_VLAN_LT1_EN BIT(1) @@ -2000,7 +2000,7 @@ static int keystone_set_link_ksettings(struct net_device *ndev, return phy_ethtool_ksettings_set(phy, cmd); } -#if IS_ENABLED(CONFIG_TI_CPTS) +#if IS_REACHABLE(CONFIG_TI_CPTS) static int keystone_get_ts_info(struct net_device *ndev, struct ethtool_ts_info *info) { @@ -2532,7 +2532,7 @@ static int gbe_del_vid(void *intf_priv, int vid) return 0; } -#if IS_ENABLED(CONFIG_TI_CPTS) +#if IS_REACHABLE(CONFIG_TI_CPTS) static void gbe_txtstamp(void *context, struct sk_buff *skb) { @@ -2977,7 +2977,7 @@ static int gbe_close(void *intf_priv, struct net_device *ndev) return 0; } -#if IS_ENABLED(CONFIG_TI_CPTS) +#if IS_REACHABLE(CONFIG_TI_CPTS) static void init_slave_ts_ctl(struct gbe_slave *slave) { slave->ts_ctl.uni = 1; @@ -3718,7 +3718,7 @@ static int gbe_probe(struct netcp_device *netcp_device, struct device *dev, gbe_dev->cpts = cpts_create(gbe_dev->dev, gbe_dev->cpts_reg, cpts_node); of_node_put(cpts_node); - if (IS_ENABLED(CONFIG_TI_CPTS) && IS_ERR(gbe_dev->cpts)) { + if (IS_REACHABLE(CONFIG_TI_CPTS) && IS_ERR(gbe_dev->cpts)) { ret = PTR_ERR(gbe_dev->cpts); goto free_sec_ports; }
My recent commit b6d49cab44b5 ("net: Make PTP-specific drivers depend on PTP_1588_CLOCK") exposes a missing dependency in defconfigs that select TI_CPTS without selecting PTP_1588_CLOCK, leading to linker errors of the form: drivers/net/ethernet/ti/cpsw.o: in function `cpsw_ndo_stop': cpsw.c:(.text+0x680): undefined reference to `cpts_unregister' drivers/net/ethernet/ti/cpsw.o: in function `cpsw_remove': cpsw.c:(.text+0x81c): undefined reference to `cpts_release' drivers/net/ethernet/ti/cpsw.o: in function `cpsw_rx_handler': cpsw.c:(.text+0x1324): undefined reference to `cpts_rx_timestamp' drivers/net/ethernet/ti/cpsw.o: in function `cpsw_ndo_open': cpsw.c:(.text+0x15ec): undefined reference to `cpts_register' drivers/net/ethernet/ti/cpsw.o: in function `cpsw_probe': cpsw.c:(.text+0x2468): undefined reference to `cpts_release' That's because TI_CPTS_MOD (which is the symbol gating the _compilation_ of cpts.c) now depends on PTP_1588_CLOCK, and so is not enabled in these configurations, but TI_CPTS (which is the symbol gating _calls_ to the cpts functions) _is_ enabled. So we end up compiling calls to functions that don't exist, resulting in the linker errors. The reason we have two symbols (TI_CPTS and TI_CPTS_MOD) for the same driver is due to commit be9ca0d33c85 ("cpsw/netcp: work around reverse cpts dependency"), which introduced TI_CPTS_MOD because (quoting the commit message): > The dependency is reversed: cpsw and netcp call into cpts, > but cpts depends on the other two in Kconfig. This can lead > to cpts being a loadable module and its callers built-in: > > drivers/net/ethernet/ti/cpsw.o: In function `cpsw_remove': > cpsw.c:(.text.cpsw_remove+0xd0): undefined reference to `cpts_release' > drivers/net/ethernet/ti/cpsw.o: In function `cpsw_rx_handler': > cpsw.c:(.text.cpsw_rx_handler+0x2dc): undefined reference to `cpts_rx_timestamp' > drivers/net/ethernet/ti/cpsw.o: In function `cpsw_tx_handler': > cpsw.c:(.text.cpsw_tx_handler+0x7c): undefined reference to `cpts_tx_timestamp' > drivers/net/ethernet/ti/cpsw.o: In function `cpsw_ndo_stop': Both forms of linker error -- those caused by defconfigs that select TI_CPTS without PTP_1588_CLOCK and those caused by configuring TI_CPSW as a built-in and TI_CPTS as a module -- can be avoided by using the IS_REACHABLE() macro to gate calls to cpts functions, and using the TI_CPTS symbol to gate compilation of cpts.c. cpts.h already provides the no-op stub implementations of the cpts functions required to make this work, we just need to change the existing IS_ENABLED(TI_CPTS) guards to IS_REACHABLE(TI_CPTS). With this change there is no longer any need for the TI_CPTS_MOD symbol, so we can remove it. To preserve the existing behavior of defconfigs that select TI_CPTS, we must also select PTP_1588_CLOCK so that the dependency is satisfied. Cc: Arnd Bergmann <arnd@arndb.de> Cc: Grygorii Strashko <grygorii.strashko@ti.com> Fixes: b6d49cab44b5 ("net: Make PTP-specific drivers depend on PTP_1588_CLOCK") Reported-by: kbuild test robot <lkp@intel.com> Signed-off-by: Clay McClure <clay@daemons.net> --- Changes in v2: - Don't regenerate the defconfigs, just add PTP_1588_CLOCK. arch/arm/configs/keystone_defconfig | 1 + arch/arm/configs/omap2plus_defconfig | 1 + drivers/net/ethernet/ti/Kconfig | 13 ++++--------- drivers/net/ethernet/ti/Makefile | 2 +- drivers/net/ethernet/ti/cpsw_ethtool.c | 2 +- drivers/net/ethernet/ti/cpts.h | 3 +-- drivers/net/ethernet/ti/netcp_ethss.c | 10 +++++----- 7 files changed, 14 insertions(+), 18 deletions(-)