RE: [PATCH] ethernet/intel: fix PTP_1588_CLOCK dependencies

-----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...