diff mbox series

ethernet/intel: fix PTP_1588_CLOCK dependencies

Message ID 20210726084540.3282344-1-arnd@kernel.org (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series ethernet/intel: fix PTP_1588_CLOCK dependencies | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Guessed tree name to be net-next
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit fail Errors and warnings before: 0 this patch: 5
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 55 lines checked
netdev/build_allmodconfig_warn fail Errors and warnings before: 0 this patch: 5
netdev/header_inline success Link

Commit Message

Arnd Bergmann July 26, 2021, 8:45 a.m. UTC 
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(-)

Comments

kernel test robot July 26, 2021, 10:27 a.m. UTC | #1 
Hi Arnd,

I love your patch! Yet something to improve:

[auto build test ERROR on tnguy-next-queue/dev-queue]
[also build test ERROR on v5.14-rc3 next-20210723]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Arnd-Bergmann/ethernet-intel-fix-PTP_1588_CLOCK-dependencies/20210726-164755
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git dev-queue
config: um-x86_64_defconfig (attached as .config)
compiler: gcc-10 (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/400b6b5bda753bdd933ea7f6b55be00cc4a692ed
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Arnd-Bergmann/ethernet-intel-fix-PTP_1588_CLOCK-dependencies/20210726-164755
        git checkout 400b6b5bda753bdd933ea7f6b55be00cc4a692ed
        # save the attached .config to linux build tree
        make W=1 ARCH=um SUBARCH=x86_64

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/i2c/Kconfig:8:error: recursive dependency detected!
   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
Arnd Bergmann July 26, 2021, 12:27 p.m. UTC | #2 
On Mon, Jul 26, 2021 at 12:29 PM kernel test robot <lkp@intel.com> wrote:

> >> drivers/i2c/Kconfig:8:error: recursive dependency detected!
>    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
kernel test robot July 26, 2021, 1:44 p.m. UTC | #3 
Hi Arnd,

I love your patch! Yet something to improve:

[auto build test ERROR on tnguy-next-queue/dev-queue]
[also build test ERROR on v5.14-rc3 next-20210723]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Arnd-Bergmann/ethernet-intel-fix-PTP_1588_CLOCK-dependencies/20210726-164755
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git dev-queue
config: mips-randconfig-r002-20210726 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project c63dbd850182797bc4b76124d08e1c320ab2365d)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install mips cross compiling tool for clang build
        # apt-get install binutils-mips-linux-gnu
        # https://github.com/0day-ci/linux/commit/400b6b5bda753bdd933ea7f6b55be00cc4a692ed
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Arnd-Bergmann/ethernet-intel-fix-PTP_1588_CLOCK-dependencies/20210726-164755
        git checkout 400b6b5bda753bdd933ea7f6b55be00cc4a692ed
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=mips 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/i2c/Kconfig:8:error: recursive dependency detected!
   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
Jacob Keller July 26, 2021, 9:21 p.m. UTC | #4 
> -----Original Message-----
> From: Arnd Bergmann <arnd@kernel.org>
> Sent: Monday, July 26, 2021 1:45 AM
> To: Brandeburg, Jesse <jesse.brandeburg@intel.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; David S. Miller <davem@davemloft.net>; Jakub
> Kicinski <kuba@kernel.org>; Keller, Jacob E <jacob.e.keller@intel.com>
> Cc: Arnd Bergmann <arnd@arndb.de>; Kurt Kanzenbach <kurt@linutronix.de>;
> Saleem, Shiraz <shiraz.saleem@intel.com>; Ertman, David M
> <david.m.ertman@intel.com>; 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...


> ---
>  drivers/net/ethernet/intel/Kconfig | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/Kconfig
> b/drivers/net/ethernet/intel/Kconfig
> index 2aa84bd97287..ab75cde0c4ec 100644
> --- a/drivers/net/ethernet/intel/Kconfig
> +++ b/drivers/net/ethernet/intel/Kconfig
> @@ -58,8 +58,8 @@ config E1000
>  config E1000E
>  	tristate "Intel(R) PRO/1000 PCI-Express Gigabit Ethernet support"
>  	depends on PCI && (!SPARC32 || BROKEN)
> +	depends on PTP_1588_CLOCK || !PTP_1588_CLOCK
>  	select CRC32
> -	imply PTP_1588_CLOCK
>  	help
>  	  This driver supports the PCI-Express Intel(R) PRO/1000 gigabit
>  	  ethernet family of adapters. For PCI or PCI-X e1000 adapters,
> @@ -87,7 +87,7 @@ config E1000E_HWTS
>  config IGB
>  	tristate "Intel(R) 82575/82576 PCI-Express Gigabit Ethernet support"
>  	depends on PCI
> -	imply PTP_1588_CLOCK
> +	depends on PTP_1588_CLOCK || !PTP_1588_CLOCK
>  	select I2C


Commit message said you changed IGB to use depends I2C, but the content doesn't...

>  	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,
> --
> 2.29.2
Jacob Keller July 27, 2021, 7:25 a.m. UTC | #5 
On 7/26/2021 5:27 AM, Arnd Bergmann wrote:
> On Mon, Jul 26, 2021 at 12:29 PM kernel test robot <lkp@intel.com> wrote:
> 
>>>> drivers/i2c/Kconfig:8:error: recursive dependency detected!
>>    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>
G, GurucharanX Aug. 2, 2021, 1:07 p.m. UTC | #6 
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Arnd Bergmann
> Sent: Monday, July 26, 2021 2:15 PM
> To: Brandeburg, Jesse <jesse.brandeburg@intel.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; David S. Miller <davem@davemloft.net>;
> Jakub Kicinski <kuba@kernel.org>; Keller, Jacob E
> <jacob.e.keller@intel.com>
> Cc: Arnd Bergmann <arnd@arndb.de>; netdev@vger.kernel.org; Kurt
> Kanzenbach <kurt@linutronix.de>; linux-kernel@vger.kernel.org; intel-
> wired-lan@lists.osuosl.org; Saleem, Shiraz <shiraz.saleem@intel.com>
> 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 <Gurucharanx.g@intel.com> (A Contingent Worker at Intel)
Arnd Bergmann Aug. 2, 2021, 2:37 p.m. UTC | #7 
On Mon, Aug 2, 2021 at 3:10 PM G, GurucharanX <gurucharanx.g@intel.com> wrote:
> >
> > 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 <Gurucharanx.g@intel.com> (A Contingent Worker at Intel)

Sorry for the delay. I had remembered that there was a previous discussion
about that option but couldn't find the thread at first.

I now found
https://lore.kernel.org/netdev/CAK8P3a3=eOxE-K25754+fB_-i_0BZzf9a9RfPTX3ppSwu9WZXw@mail.gmail.com/

and will add Richard to Cc for my new version as well, just in case he
has objections
to this version and wants to fix it differently.

       Arnd
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/Kconfig b/drivers/net/ethernet/intel/Kconfig
index 2aa84bd97287..ab75cde0c4ec 100644
--- a/drivers/net/ethernet/intel/Kconfig
+++ b/drivers/net/ethernet/intel/Kconfig
@@ -58,8 +58,8 @@  config E1000
 config E1000E
 	tristate "Intel(R) PRO/1000 PCI-Express Gigabit Ethernet support"
 	depends on PCI && (!SPARC32 || BROKEN)
+	depends on PTP_1588_CLOCK || !PTP_1588_CLOCK
 	select CRC32
-	imply PTP_1588_CLOCK
 	help
 	  This driver supports the PCI-Express Intel(R) PRO/1000 gigabit
 	  ethernet family of adapters. For PCI or PCI-X e1000 adapters,
@@ -87,7 +87,7 @@  config E1000E_HWTS
 config IGB
 	tristate "Intel(R) 82575/82576 PCI-Express Gigabit Ethernet support"
 	depends on PCI
-	imply PTP_1588_CLOCK
+	depends on PTP_1588_CLOCK || !PTP_1588_CLOCK
 	select I2C
 	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,