On 9/21/23 02:06, Jacob Keller wrote:
The recent support for DPLL introduced by commit 8a3a565ff210 ("ice: add admin commands to access cgu configuration") and commit d7999f5ea64b ("ice: implement dpll interface to control cgu") broke linking the ice driver if CONFIG_PTP_1588_CLOCK=n:ld: vmlinux.o: in function `ice_init_feature_support': (.text+0x8702b8): undefined reference to `ice_is_phy_rclk_present' ld: (.text+0x8702cd): undefined reference to `ice_is_cgu_present' ld: (.text+0x8702d9): undefined reference to `ice_is_clock_mux_present_e810t' ld: vmlinux.o: in function `ice_dpll_init_info_direct_pins': ice_dpll.c:(.text+0x894167): undefined reference to `ice_cgu_get_pin_freq_supp' ld: ice_dpll.c:(.text+0x894197): undefined reference to `ice_cgu_get_pin_name' ld: ice_dpll.c:(.text+0x8941a8): undefined reference to `ice_cgu_get_pin_type' ld: vmlinux.o: in function `ice_dpll_update_state': ice_dpll.c:(.text+0x894494): undefined reference to `ice_get_cgu_state' ld: vmlinux.o: in function `ice_dpll_init': (.text+0x8953d5): undefined reference to `ice_get_cgu_rclk_pin_info' The first commit broke things by calling functions in ice_init_feature_support that are compiled as part of ice_ptp_hw.o, including: * ice_is_phy_rclk_present * ice_is_clock_mux_present_e810t * ice_is_cgU_present The second commit continued the break by calling several CGU functions defined in ice_ptp_hw.c in the DPLL code. Because the ice_dpll.c file is compiled unconditionally, it will not link when CONFIG_PTP_1588_CLOCK=n. It might be possible to break this dependency and expose those functions without CONFIG_PTP_1588_CLOCK, but that is not clear to me. For the DPLL case, simply compile ice_dpll.o only when we have CONFIG_PTP_1588_CLOCK. Add stub no-op implementation of ice_dpll_init() and ice_dpll_uninit() when CONFIG_PTP_1588_CLOCK=n into ice_dpll.h The other functions are part of checking the netlist to see if hardware features are enabled. These checks don't really belong in ice_ptp_hw.c, and make more sense as part of the ice_common.c file. We already have ice_is_gps_in_netlist() in ice_common.c which is doing a similar check. Move the functions into ice_common.c and rename them to have the similar postfix of "in_netlist()" to be more expressive of what they are actually checking. This also makes the ice_find_netlist_node only called from within ice_common.c, so its safe to mark it static and stop declaring it in the ice_common.h header as well. Fixes: 8a3a565ff210 ("ice: add admin commands to access cgu configuration") 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: Jacob Keller <[email protected]> --- This is an alternative approach to fixing the same compilation errors of the ice driver compared to [1]. It does not include the fix for idpf which I have no issue with. I prefer this over the stubs for the functions in ice_ptp_hw.h, and I had proposed these a while ago as part of [2], but the DPLL code merged first and had apparently duplicated some of the work. [1]: https://lore.kernel.org/netdev/[email protected]/T/#mfeeb599a95334e78dba08330a6bd8edee7843fbb [2]: https://lore.kernel.org/netdev/[email protected]/ drivers/net/ethernet/intel/ice/Makefile | 5 +- drivers/net/ethernet/intel/ice/ice_common.c | 66 ++++++++++++++++++++- drivers/net/ethernet/intel/ice/ice_common.h | 6 +- drivers/net/ethernet/intel/ice/ice_dpll.h | 6 +- drivers/net/ethernet/intel/ice/ice_lib.c | 6 +- drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 66 --------------------- drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 3 - 7 files changed, 76 insertions(+), 82 deletions(-)
Reviewed-by: Przemek Kitszel <[email protected]> _______________________________________________ Intel-wired-lan mailing list [email protected] https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
