On 9/20/2023 11:07 AM, Alexander Lobakin wrote:
> DPLL code in ice unconditionally calls several PTP functions which are
> only built when CONFIG_PTP_1588_CLOCK is set. This throws a good bunch
> of link errors:
>
> ERROR: modpost: "ice_cgu_get_pin_name"
> [drivers/net/ethernet/intel/ice/ice.ko] undefined!
> ERROR: modpost: "ice_get_cgu_state"
> [drivers/net/ethernet/intel/ice/ice.ko] undefined!
> OR: modpost: "ice_is_cgu_present"
> [drivers/net/ethernet/intel/ice/ice.ko] undefined!
> ERROR: modpost: "ice_get_cgu_rclk_pin_info"
> [drivers/net/ethernet/intel/ice/ice.ko] undefined!
> ERROR: modpost: "ice_cgu_get_pin_type"
> [drivers/net/ethernet/intel/ice/ice.ko] undefined!
> ERROR: modpost: "ice_cgu_get_pin_freq_supp"
> [drivers/net/ethernet/intel/ice/ice.ko] undefined!
>
> ice_dpll_{,de}init() can be only called at runtime when the
> corresponding feature flags are set, which is not the case when PTP
> support is not compiled. However, the linker has no clue about this.
> Compile DPLL code only when CONFIG_PTP_1588_CLOCK is enabled and guard
> the mentioned init/deinit function calls, so that ice_dpll.o is only
> referred when it gets compiled>
> Note that ideally ice_is_feature_supported() needs to check for
> compile-time flags first to be able to handle this without any
> additional call guards, and we may want to do that in the future.
>
> Fixes: d7999f5ea64b ("ice: implement dpll interface to control cgu")
> Reported-by: kernel test robot <[email protected]>
> Closes:
> https://lore.kernel.org/oe-kbuild-all/[email protected]
> Signed-off-by: Alexander Lobakin <[email protected]>
> ---
> drivers/net/ethernet/intel/ice/Makefile | 5 ++---
> drivers/net/ethernet/intel/ice/ice_main.c | 8 +++++---
> 2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/Makefile
> b/drivers/net/ethernet/intel/ice/Makefile
> index 00806ddf5bf0..16f96d5210d8 100644
> --- a/drivers/net/ethernet/intel/ice/Makefile
> +++ b/drivers/net/ethernet/intel/ice/Makefile
> @@ -34,8 +34,7 @@ ice-y := ice_main.o \
> ice_lag.o \
> ice_ethtool.o \
> ice_repr.o \
> - ice_tc_lib.o \
> - ice_dpll.o
> + ice_tc_lib.o
> ice-$(CONFIG_PCI_IOV) += \
> ice_sriov.o \
> ice_virtchnl.o \
> @@ -44,7 +43,7 @@ ice-$(CONFIG_PCI_IOV) += \
> ice_vf_mbx.o \
> ice_vf_vsi_vlan_ops.o \
> ice_vf_lib.o
> -ice-$(CONFIG_PTP_1588_CLOCK) += ice_ptp.o ice_ptp_hw.o
> +ice-$(CONFIG_PTP_1588_CLOCK) += ice_dpll.o ice_ptp.o ice_ptp_hw.o
> ice-$(CONFIG_DCB) += ice_dcb.o ice_dcb_nl.o ice_dcb_lib.o
> ice-$(CONFIG_RFS_ACCEL) += ice_arfs.o
> ice-$(CONFIG_XDP_SOCKETS) += ice_xsk.o
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
> b/drivers/net/ethernet/intel/ice/ice_main.c
> index e22f41fea8db..9b48918dcdb7 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -4665,8 +4665,9 @@ static void ice_init_features(struct ice_pf *pf)
> if (ice_is_feature_supported(pf, ICE_F_GNSS))
> ice_gnss_init(pf);
>
> - if (ice_is_feature_supported(pf, ICE_F_CGU) ||
> - ice_is_feature_supported(pf, ICE_F_PHY_RCLK))
> + if (IS_ENABLED(CONFIG_PTP_1588_CLOCK) &&
> + (ice_is_feature_supported(pf, ICE_F_CGU) ||
> + ice_is_feature_supported(pf, ICE_F_PHY_RCLK)))
> ice_dpll_init(pf);
We can stub ice_dpll_init and dpll_deinit in the ice_dpll.h header so
that we don't have to check IS_ENABLED here.
I posted that fix at
https://lore.kernel.org/intel-wired-lan/[email protected]/
though I guess I sent it to intel-wired-lan first instead of sending
directly to netdev.
I would prefer the resolutions in my series, as I think they're
ultimately cleaner.
Of course: priority is getting this fixed so I won't begrudge pulling
this now.
Thanks,
Jake
>
> /* Note: Flow director init failure is non-fatal to load */
> @@ -4695,7 +4696,8 @@ static void ice_deinit_features(struct ice_pf *pf)
> ice_gnss_exit(pf);
> if (test_bit(ICE_FLAG_PTP_SUPPORTED, pf->flags))
> ice_ptp_release(pf);
> - if (test_bit(ICE_FLAG_DPLL, pf->flags))
> + if (IS_ENABLED(CONFIG_PTP_1588_CLOCK) &&
> + test_bit(ICE_FLAG_DPLL, pf->flags))
> ice_dpll_deinit(pf);
> }
>
_______________________________________________
Intel-wired-lan mailing list
[email protected]
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
