Commit e5f31552 authored by Arnd Bergmann's avatar Arnd Bergmann Committed by Jakub Kicinski
Browse files

ethernet: fix PTP_1588_CLOCK dependencies

The 'imply' keyword does not do what most people think it does, it only
politely asks Kconfig to turn on another symbol, but does not prevent
it from being disabled manually or built as a loadable module when the
user is built-in. In the ICE driver, the latter now causes a link failure:

aarch64-linux-ld: drivers/net/ethernet/intel/ice/ice_main.o: in function `ice_eth_ioctl':
ice_main.c:(.text+0x13b0): undefined reference to `ice_ptp_get_ts_config'
ice_main.c:(.text+0x13b0): relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol `ice_ptp_get_ts_config'
aarch64-linux-ld: ice_main.c:(.text+0x13bc): undefined reference to `ice_ptp_set_ts_config'
ice_main.c:(.text+0x13bc): relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol `ice_ptp_set_ts_config'
aarch64-linux-ld: drivers/net/ethernet/intel/ice/ice_main.o: in function `ice_prepare_for_reset':
ice_main.c:(.text+0x31fc): undefined reference to `ice_ptp_release'
ice_main.c:(.text+0x31fc): relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol `ice_ptp_release'
aarch64-linux-ld: drivers/net/ethernet/intel/ice/ice_main.o: in function `ice_rebuild':

This is a recurring problem in many drivers, and we have discussed
it several times befores, without reaching a consensus. I'm providing
a link to the previous email thread for reference, which discusses
some related problems.

To solve the dependency issue better than the 'imply' keyword, introduce a
separate Kconfig symbol "CONFIG_PTP_1588_CLOCK_OPTIONAL" that any driver
can depend on if it is able to use PTP support when available, but works
fine without it. Whenever CONFIG_PTP_1588_CLOCK=m, those drivers are
then prevented from being built-in, the same way as with a 'depends on
PTP_1588_CLOCK || !PTP_1588_CLOCK' dependency that does the same trick,
but that can be rather confusing when you first see it.

Since this should cover the dependencies correctly, the IS_REACHABLE()
hack in the header is no longer needed now, and can be turned back
into a normal IS_ENABLED() check. Any driver that gets the dependency
wrong will now cause a link time failure rather than being unable to use
PTP support when that is in a loadable module.

However, the two recently added ptp_get_vclocks_index() and
ptp_convert_timestamp() interfaces are only called from builtin code with
ethtool and socket timestamps, so keep the current behavior by stubbing
those out completely when PTP is in a loadable module. This should be
addressed properly in a follow-up.

As Richard suggested, we may want to actually turn PTP support into a
'bool' option later on, preventing it from being a loadable module
altogether, which would be one way to solve the problem with the ethtool
interface.

Fixes: 06c16d89 ("ice: register 1588 PTP clock device object for E810 devices")
Link: https://lore.kernel.org/netdev/20210804121318.337276-1-arnd@kernel.org/
Link: https://lore.kernel.org/netdev/CAK8P3a06enZOf=XyZ+zcAwBczv41UuCTz+=0FMf2gBz1_cOnZQ@mail.gmail.com/
Link: https://lore.kernel.org/netdev/CAK8P3a3=eOxE-K25754+fB_-i_0BZzf9a9RfPTX3ppSwu9WZXw@mail.gmail.com/
Link: https://lore.kernel.org/netdev/20210726084540.3282344-1-arnd@kernel.org/


Acked-by: default avatarShannon Nelson <snelson@pensando.io>
Acked-by: default avatarJacob Keller <jacob.e.keller@intel.com>
Acked-by: default avatarRichard Cochran <richardcochran@gmail.com>
Reviewed-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
Link: https://lore.kernel.org/r/20210812183509.1362782-1-arnd@kernel.org


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent b697d9d3
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -2,6 +2,7 @@
config NET_DSA_MV88E6XXX
	tristate "Marvell 88E6xxx Ethernet switch fabric support"
	depends on NET_DSA
	depends on PTP_1588_CLOCK_OPTIONAL
	select IRQ_DOMAIN
	select NET_DSA_TAG_EDSA
	select NET_DSA_TAG_DSA
+2 −0
Original line number Diff line number Diff line
@@ -5,6 +5,7 @@ config NET_DSA_MSCC_FELIX
	depends on NET_VENDOR_MICROSEMI
	depends on NET_VENDOR_FREESCALE
	depends on HAS_IOMEM
	depends on PTP_1588_CLOCK_OPTIONAL
	select MSCC_OCELOT_SWITCH_LIB
	select NET_DSA_TAG_OCELOT_8021Q
	select NET_DSA_TAG_OCELOT
@@ -19,6 +20,7 @@ config NET_DSA_MSCC_SEVILLE
	depends on NET_DSA
	depends on NET_VENDOR_MICROSEMI
	depends on HAS_IOMEM
	depends on PTP_1588_CLOCK_OPTIONAL
	select MSCC_OCELOT_SWITCH_LIB
	select NET_DSA_TAG_OCELOT_8021Q
	select NET_DSA_TAG_OCELOT
+1 −0
Original line number Diff line number Diff line
@@ -2,6 +2,7 @@
config NET_DSA_SJA1105
tristate "NXP SJA1105 Ethernet switch family support"
	depends on NET_DSA && SPI
	depends on PTP_1588_CLOCK_OPTIONAL
	select NET_DSA_TAG_SJA1105
	select PCS_XPCS
	select PACKING
+1 −1
Original line number Diff line number Diff line
@@ -170,11 +170,11 @@ config AMD_XGBE
	tristate "AMD 10GbE Ethernet driver"
	depends on ((OF_NET && OF_ADDRESS) || ACPI || PCI) && HAS_IOMEM
	depends on X86 || ARM64 || COMPILE_TEST
	depends on PTP_1588_CLOCK_OPTIONAL
	select BITREVERSE
	select CRC32
	select PHYLIB
	select AMD_XGBE_HAVE_ECC if X86
	imply PTP_1588_CLOCK
	help
	  This driver supports the AMD 10GbE Ethernet device found on an
	  AMD SoC.
+3 −3
Original line number Diff line number Diff line
@@ -122,8 +122,8 @@ config SB1250_MAC
config TIGON3
	tristate "Broadcom Tigon3 support"
	depends on PCI
	depends on PTP_1588_CLOCK_OPTIONAL
	select PHYLIB
	imply PTP_1588_CLOCK
	help
	  This driver supports Broadcom Tigon3 based gigabit Ethernet cards.

@@ -140,7 +140,7 @@ config TIGON3_HWMON
config BNX2X
	tristate "Broadcom NetXtremeII 10Gb support"
	depends on PCI
	imply PTP_1588_CLOCK
	depends on PTP_1588_CLOCK_OPTIONAL
	select FW_LOADER
	select ZLIB_INFLATE
	select LIBCRC32C
@@ -206,7 +206,7 @@ config SYSTEMPORT
config BNXT
	tristate "Broadcom NetXtreme-C/E support"
	depends on PCI
	imply PTP_1588_CLOCK
	depends on PTP_1588_CLOCK_OPTIONAL
	select FW_LOADER
	select LIBCRC32C
	select NET_DEVLINK
Loading