From: Arnd Bergmann <arnd@arndb.de>
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':
For the other Intel network drivers, there is no link error when the
drivers are built-in and PTP is a loadable module, because
linux/ptp_clock_kernel.h contains an IS_REACHABLE() check, but this
just changes the compile-time failure to a runtime failure, which is
arguably worse.
Change all the Intel drivers to use the 'depends on PTP_1588_CLOCK ||
!PTP_1588_CLOCK' trick to prevent the broken configuration, as we
already do for several other drivers. To avoid circular dependencies,
this also requires changing the IGB driver back to using the normal
'depends on I2C' instead of 'select I2C'.
Fixes: 06c16d89d2cb ("ice: register 1588 PTP clock device object for E810 devices")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/ethernet/intel/Kconfig | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
drivers/i2c/Kconfig:8: symbol I2C is selected by IGB
drivers/net/ethernet/intel/Kconfig:87: symbol IGB depends on PTP_1588_CLOCK
drivers/ptp/Kconfig:8: symbol PTP_1588_CLOCK is implied by MLX4_EN
drivers/net/ethernet/mellanox/mlx4/Kconfig:6: symbol MLX4_EN depends on NET_VENDOR_MELLANOX
drivers/net/ethernet/mellanox/Kconfig:6: symbol NET_VENDOR_MELLANOX depends on I2C
For a resolution refer to Documentation/kbuild/kconfig-language.rst
subsection "Kconfig recursive dependency limitations"
vim +8 drivers/i2c/Kconfig
da3c6647ee0871 Lan Tianyu 2014-05-20 7
da3c6647ee0871 Lan Tianyu 2014-05-20 @8 config I2C
^1da177e4c3f41 Linus Torvalds 2005-04-16 9 tristate "I2C support"
194684e596af4b Mika Kuoppala 2009-12-06 10 select RT_MUTEXES
4d5538f5882a6b Benjamin Tissoires 2016-10-13 11 select IRQ_DOMAIN
a7f7f6248d9740 Masahiro Yamada 2020-06-14 12 help
622e040d577dc8 Michael Witten 2011-07-08 13 I2C (pronounce: I-squared-C) is a slow serial bus protocol used in
^1da177e4c3f41 Linus Torvalds 2005-04-16 14 many micro controller applications and developed by Philips. SMBus,
^1da177e4c3f41 Linus Torvalds 2005-04-16 15 or System Management Bus is a subset of the I2C protocol. More
^1da177e4c3f41 Linus Torvalds 2005-04-16 16 information is contained in the directory <file:Documentation/i2c/>,
^1da177e4c3f41 Linus Torvalds 2005-04-16 17 especially in the file called "summary" there.
^1da177e4c3f41 Linus Torvalds 2005-04-16 18
^1da177e4c3f41 Linus Torvalds 2005-04-16 19 Both I2C and SMBus are supported here. You will need this for
^1da177e4c3f41 Linus Torvalds 2005-04-16 20 hardware sensors support, and also for Video For Linux support.
^1da177e4c3f41 Linus Torvalds 2005-04-16 21
^1da177e4c3f41 Linus Torvalds 2005-04-16 22 If you want I2C support, you should say Y here and also to the
^1da177e4c3f41 Linus Torvalds 2005-04-16 23 specific driver for your bus adapter(s) below.
^1da177e4c3f41 Linus Torvalds 2005-04-16 24
^1da177e4c3f41 Linus Torvalds 2005-04-16 25 This I2C support can also be built as a module. If so, the module
^1da177e4c3f41 Linus Torvalds 2005-04-16 26 will be called i2c-core.
^1da177e4c3f41 Linus Torvalds 2005-04-16 27
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
drivers/i2c/Kconfig:8: symbol I2C is selected by IGB
drivers/net/ethernet/intel/Kconfig:87: symbol IGB depends on PTP_1588_CLOCK
drivers/ptp/Kconfig:8: symbol PTP_1588_CLOCK is implied by MLX4_EN
drivers/net/ethernet/mellanox/mlx4/Kconfig:6: symbol MLX4_EN depends on NET_VENDOR_MELLANOX
drivers/net/ethernet/mellanox/Kconfig:6: symbol NET_VENDOR_MELLANOX depends on I2C
For a resolution refer to Documentation/kbuild/kconfig-language.rst
subsection "Kconfig recursive dependency limitations"
Sorry about this, the patch I was testing with has this additional hunk
@@ -88,7 +88,7 @@ config IGB tristate "Intel(R) 82575/82576 PCI-Express Gigabit Ethernet support" depends on PCI depends on PTP_1588_CLOCK || !PTP_1588_CLOCK- select I2C+ depends on I2C select I2C_ALGOBIT help This driver supports Intel(R) 82575/82576 gigabit ethernet family of
that I even describe in the changelog but forgot to include in the patch I sent.
Arnd
drivers/i2c/Kconfig:8: symbol I2C is selected by IGB
drivers/net/ethernet/intel/Kconfig:87: symbol IGB depends on PTP_1588_CLOCK
drivers/ptp/Kconfig:8: symbol PTP_1588_CLOCK is implied by MLX4_EN
drivers/net/ethernet/mellanox/mlx4/Kconfig:6: symbol MLX4_EN depends on NET_VENDOR_MELLANOX
drivers/net/ethernet/mellanox/Kconfig:6: symbol NET_VENDOR_MELLANOX depends on I2C
For a resolution refer to Documentation/kbuild/kconfig-language.rst
subsection "Kconfig recursive dependency limitations"
vim +8 drivers/i2c/Kconfig
da3c6647ee0871 Lan Tianyu 2014-05-20 7
da3c6647ee0871 Lan Tianyu 2014-05-20 @8 config I2C
^1da177e4c3f41 Linus Torvalds 2005-04-16 9 tristate "I2C support"
194684e596af4b Mika Kuoppala 2009-12-06 10 select RT_MUTEXES
4d5538f5882a6b Benjamin Tissoires 2016-10-13 11 select IRQ_DOMAIN
a7f7f6248d9740 Masahiro Yamada 2020-06-14 12 help
622e040d577dc8 Michael Witten 2011-07-08 13 I2C (pronounce: I-squared-C) is a slow serial bus protocol used in
^1da177e4c3f41 Linus Torvalds 2005-04-16 14 many micro controller applications and developed by Philips. SMBus,
^1da177e4c3f41 Linus Torvalds 2005-04-16 15 or System Management Bus is a subset of the I2C protocol. More
^1da177e4c3f41 Linus Torvalds 2005-04-16 16 information is contained in the directory <file:Documentation/i2c/>,
^1da177e4c3f41 Linus Torvalds 2005-04-16 17 especially in the file called "summary" there.
^1da177e4c3f41 Linus Torvalds 2005-04-16 18
^1da177e4c3f41 Linus Torvalds 2005-04-16 19 Both I2C and SMBus are supported here. You will need this for
^1da177e4c3f41 Linus Torvalds 2005-04-16 20 hardware sensors support, and also for Video For Linux support.
^1da177e4c3f41 Linus Torvalds 2005-04-16 21
^1da177e4c3f41 Linus Torvalds 2005-04-16 22 If you want I2C support, you should say Y here and also to the
^1da177e4c3f41 Linus Torvalds 2005-04-16 23 specific driver for your bus adapter(s) below.
^1da177e4c3f41 Linus Torvalds 2005-04-16 24
^1da177e4c3f41 Linus Torvalds 2005-04-16 25 This I2C support can also be built as a module. If so, the module
^1da177e4c3f41 Linus Torvalds 2005-04-16 26 will be called i2c-core.
^1da177e4c3f41 Linus Torvalds 2005-04-16 27
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
From: "Keller, Jacob E" <jacob.e.keller@intel.com> Date: 2021-07-26 21:21:17
-----Original Message-----
From: Arnd Bergmann <arnd@kernel.org>
Sent: Monday, July 26, 2021 1:45 AM
To: Brandeburg, Jesse <redacted>; Nguyen, Anthony L
[off-list ref]; David S. Miller [off-list ref]; Jakub
Kicinski [off-list ref]; Keller, Jacob E [off-list ref]
Cc: Arnd Bergmann <arnd@arndb.de>; Kurt Kanzenbach <kurt@linutronix.de>;
Saleem, Shiraz [off-list ref]; Ertman, David M
[off-list ref]; intel-wired-lan@lists.osuosl.org;
netdev@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: [PATCH] ethernet/intel: fix PTP_1588_CLOCK dependencies
From: Arnd Bergmann <arnd@arndb.de>
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':
For the other Intel network drivers, there is no link error when the
drivers are built-in and PTP is a loadable module, because
linux/ptp_clock_kernel.h contains an IS_REACHABLE() check, but this
just changes the compile-time failure to a runtime failure, which is
arguably worse.
Change all the Intel drivers to use the 'depends on PTP_1588_CLOCK ||
!PTP_1588_CLOCK' trick to prevent the broken configuration, as we
already do for several other drivers. To avoid circular dependencies,
this also requires changing the IGB driver back to using the normal
'depends on I2C' instead of 'select I2C'.
Fixes: 06c16d89d2cb ("ice: register 1588 PTP clock device object for E810 devices")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Thanks for fixing this!
It feels like Kconfig should have a simpler way to write this, and/or we should update the doc, since I would expect this to be a common issue with optional dependencies
Obviously "depends" handles this right but it forces a dependency in all cases, instead of being optional. select is used to ensure that some bit is turned on if you turn on that item, and imply is supposed to be that but optional...
Commit message said you changed IGB to use depends I2C, but the content doesn't...
quoted hunk
select I2C_ALGOBIT
help
@@ -159,9 +159,9 @@ config IXGB config IXGBE tristate "Intel(R) 10GbE PCI Express adapters support" depends on PCI+ depends on PTP_1588_CLOCK || !PTP_1588_CLOCK select MDIO select PHYLIB- imply PTP_1588_CLOCK help This driver supports Intel(R) 10GbE PCI Express family of adapters. For more information on how to identify your adapter, go
@@ -239,7 +239,7 @@ config IXGBEVF_IPSEC config I40E tristate "Intel(R) Ethernet Controller XL710 Family support"- imply PTP_1588_CLOCK+ depends on PTP_1588_CLOCK || !PTP_1588_CLOCK depends on PCI select AUXILIARY_BUS help
@@ -295,11 +295,11 @@ config ICE tristate "Intel(R) Ethernet Connection E800 Series Support" default n depends on PCI_MSI+ depends on PTP_1588_CLOCK || !PTP_1588_CLOCK select AUXILIARY_BUS select DIMLIB select NET_DEVLINK select PLDMFW- imply PTP_1588_CLOCK help This driver supports Intel(R) Ethernet Connection E800 Series of devices. For more information on how to identify your adapter, go
@@ -317,7 +317,7 @@ config FM10K tristate "Intel(R) FM10000 Ethernet Switch Host Interface Support" default n depends on PCI_MSI- imply PTP_1588_CLOCK+ depends on PTP_1588_CLOCK || !PTP_1588_CLOCK help This driver supports Intel(R) FM10000 Ethernet Switch Host Interface. For more information on how to identify your adapter,--
drivers/i2c/Kconfig:8: symbol I2C is selected by IGB
drivers/net/ethernet/intel/Kconfig:87: symbol IGB depends on PTP_1588_CLOCK
drivers/ptp/Kconfig:8: symbol PTP_1588_CLOCK is implied by MLX4_EN
drivers/net/ethernet/mellanox/mlx4/Kconfig:6: symbol MLX4_EN depends on NET_VENDOR_MELLANOX
drivers/net/ethernet/mellanox/Kconfig:6: symbol NET_VENDOR_MELLANOX depends on I2C
For a resolution refer to Documentation/kbuild/kconfig-language.rst
subsection "Kconfig recursive dependency limitations"
Sorry about this, the patch I was testing with has this additional hunk
@@ -88,7 +88,7 @@ config IGB tristate "Intel(R) 82575/82576 PCI-Express Gigabit Ethernet support" depends on PCI depends on PTP_1588_CLOCK || !PTP_1588_CLOCK- select I2C+ depends on I2C select I2C_ALGOBIT help This driver supports Intel(R) 82575/82576 gigabit ethernet family of
that I even describe in the changelog but forgot to include in the patch I sent.
Arnd
With this hunk applied, everything looks good to me. Thanks for the fix!
It would be nice if this sort of dependency had a keyword or some other
slightly more intuitive way of handling it.
We could make run-time IS_REACHABLE checks so that the functions which
call into enable PTP support were disabled at run time in that case, I
suppose as an alternative fix to this....
Acked-by: Jacob Keller <jacob.e.keller@intel.com>
-----Original Message-----
From: Intel-wired-lan <redacted> On Behalf Of
Arnd Bergmann
Sent: Monday, July 26, 2021 2:15 PM
To: Brandeburg, Jesse <redacted>; Nguyen, Anthony L
[off-list ref]; David S. Miller [off-list ref];
Jakub Kicinski [off-list ref]; Keller, Jacob E
[off-list ref]
Cc: Arnd Bergmann <arnd@arndb.de>; netdev@vger.kernel.org; Kurt
Kanzenbach [off-list ref]; linux-kernel@vger.kernel.org; intel-
wired-lan@lists.osuosl.org; Saleem, Shiraz [off-list ref]
Subject: [Intel-wired-lan] [PATCH] ethernet/intel: fix PTP_1588_CLOCK
dependencies
From: Arnd Bergmann <arnd@arndb.de>
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':
For the other Intel network drivers, there is no link error when the drivers
are built-in and PTP is a loadable module, because linux/ptp_clock_kernel.h
contains an IS_REACHABLE() check, but this just changes the compile-time
failure to a runtime failure, which is arguably worse.
Change all the Intel drivers to use the 'depends on PTP_1588_CLOCK ||
!PTP_1588_CLOCK' trick to prevent the broken configuration, as we already
do for several other drivers. To avoid circular dependencies, this also requires
changing the IGB driver back to using the normal 'depends on I2C' instead of
'select I2C'.
Fixes: 06c16d89d2cb ("ice: register 1588 PTP clock device object for E810
devices")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/ethernet/intel/Kconfig | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
Tested-by: Gurucharan G <redacted> (A Contingent Worker at Intel)
On Mon, Aug 2, 2021 at 3:10 PM G, GurucharanX [off-list ref] wrote:
quoted
From: Arnd Bergmann <arnd@arndb.de>
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:
...
Tested-by: Gurucharan G <redacted> (A Contingent Worker at Intel)