Date: Wed, 20 Sep 2023 17:06:33 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: netdev@...r.kernel.org,
	Intel Wired LAN <intel-wired-lan@...ts.osuosl.org>
Cc: Anthony Nguyen <anthony.l.nguyen@...el.com>,
	Alexander Lobakin <aleksander.lobakin@...el.com>,
	David Miller <davem@...emloft.net>,
	Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>,
	Przemek Kitszel <przemyslaw.kitszel@...el.com>,
	Jacob Keller <jacob.e.keller@...el.com>,
	kernel test robot <lkp@...el.com>
Subject: [PATCH net-next] ice: fix linking when CONFIG_PTP_1588_CLOCK=n

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 <lkp@...el.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202309191214.TaYEct4H-lkp@intel.com
Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
---
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/20230920180745.1607563-1-aleksander.lobakin@intel.com/T/#mfeeb599a95334e78dba08330a6bd8edee7843fbb
[2]: https://lore.kernel.org/netdev/20230918212814.435688-1-anthony.l.nguyen@intel.com/

 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(-)

diff --git a/drivers/net/ethernet/intel/ice/Makefile b/drivers/net/ethernet/intel/ice/Makefile
index 00806ddf5bf0..0679907980f7 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_ptp.o ice_ptp_hw.o ice_dpll.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_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index 8f31ae449948..a1f1f037f327 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -477,9 +477,8 @@ ice_aq_get_netlist_node(struct ice_hw *hw, struct ice_aqc_get_link_topo *cmd,
  * netlist. When found ICE_SUCCESS is returned, ICE_ERR_DOES_NOT_EXIST
  * otherwise. If node_handle provided, it would be set to found node handle.
  */
-int
-ice_find_netlist_node(struct ice_hw *hw, u8 node_type_ctx, u8 node_part_number,
-		      u16 *node_handle)
+static int ice_find_netlist_node(struct ice_hw *hw, u8 node_type_ctx,
+				 u8 node_part_number, u16 *node_handle)
 {
 	struct ice_aqc_get_link_topo cmd;
 	u8 rec_node_part_number;
@@ -2764,6 +2763,67 @@ bool ice_is_pf_c827(struct ice_hw *hw)
 	return false;
 }
 
+/**
+ * ice_is_phy_rclk_in_netlist
+ * @hw: pointer to the hw struct
+ *
+ * Check if the PHY Recovered Clock device is present in the netlist
+ */
+bool ice_is_phy_rclk_in_netlist(struct ice_hw *hw)
+{
+	if (ice_find_netlist_node(hw, ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_CTRL,
+				  ICE_AQC_GET_LINK_TOPO_NODE_NR_C827, NULL) &&
+	    ice_find_netlist_node(hw, ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_CTRL,
+				  ICE_AQC_GET_LINK_TOPO_NODE_NR_E822_PHY, NULL))
+		return false;
+
+	return true;
+}
+
+/**
+ * ice_is_clock_mux_in_netlist
+ * @hw: pointer to the hw struct
+ *
+ * Check if the Clock Multiplexer device is present in the netlist
+ */
+bool ice_is_clock_mux_in_netlist(struct ice_hw *hw)
+{
+	if (ice_find_netlist_node(hw, ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_MUX,
+				  ICE_AQC_GET_LINK_TOPO_NODE_NR_GEN_CLK_MUX,
+				  NULL))
+		return false;
+
+	return true;
+}
+
+/**
+ * ice_is_cgu_in_netlist - check for CGU presence
+ * @hw: pointer to the hw struct
+ *
+ * Check if the Clock Generation Unit (CGU) device is present in the netlist.
+ * Save the CGU part number in the hw structure for later use.
+ * Return:
+ * * true - cgu is present
+ * * false - cgu is not present
+ */
+bool ice_is_cgu_in_netlist(struct ice_hw *hw)
+{
+	if (!ice_find_netlist_node(hw, ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_CTRL,
+				   ICE_AQC_GET_LINK_TOPO_NODE_NR_ZL30632_80032,
+				   NULL)) {
+		hw->cgu_part_number = ICE_AQC_GET_LINK_TOPO_NODE_NR_ZL30632_80032;
+		return true;
+	} else if (!ice_find_netlist_node(hw,
+					  ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_CTRL,
+					  ICE_AQC_GET_LINK_TOPO_NODE_NR_SI5383_5384,
+					  NULL)) {
+		hw->cgu_part_number = ICE_AQC_GET_LINK_TOPO_NODE_NR_SI5383_5384;
+		return true;
+	}
+
+	return false;
+}
+
 /**
  * ice_is_gps_in_netlist
  * @hw: pointer to the hw struct
diff --git a/drivers/net/ethernet/intel/ice/ice_common.h b/drivers/net/ethernet/intel/ice/ice_common.h
index 47a75651ca38..7a966a0c224f 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.h
+++ b/drivers/net/ethernet/intel/ice/ice_common.h
@@ -93,11 +93,11 @@ ice_aq_get_phy_caps(struct ice_port_info *pi, bool qual_mods, u8 report_mode,
 		    struct ice_aqc_get_phy_caps_data *caps,
 		    struct ice_sq_cd *cd);
 bool ice_is_pf_c827(struct ice_hw *hw);
+bool ice_is_phy_rclk_in_netlist(struct ice_hw *hw);
+bool ice_is_clock_mux_in_netlist(struct ice_hw *hw);
+bool ice_is_cgu_in_netlist(struct ice_hw *hw);
 bool ice_is_gps_in_netlist(struct ice_hw *hw);
 int
-ice_find_netlist_node(struct ice_hw *hw, u8 node_type_ctx, u8 node_part_number,
-		      u16 *node_handle);
-int
 ice_aq_get_netlist_node(struct ice_hw *hw, struct ice_aqc_get_link_topo *cmd,
 			u8 *node_part_number, u16 *node_handle);
 int
diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.h b/drivers/net/ethernet/intel/ice/ice_dpll.h
index 9c524c4bdfd7..2dfe764b81e1 100644
--- a/drivers/net/ethernet/intel/ice/ice_dpll.h
+++ b/drivers/net/ethernet/intel/ice/ice_dpll.h
@@ -97,8 +97,12 @@ struct ice_dplls {
 	s32 output_phase_adj_max;
 };
 
+#if IS_ENABLED(CONFIG_PTP_1588_CLOCK)
 void ice_dpll_init(struct ice_pf *pf);
-
 void ice_dpll_deinit(struct ice_pf *pf);
+#else
+static inline void ice_dpll_init(struct ice_pf *pf) { }
+static inline void ice_dpll_deinit(struct ice_pf *pf) { }
+#endif
 
 #endif
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 1549890a3cbf..3f0f780110de 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -3989,14 +3989,14 @@ void ice_init_feature_support(struct ice_pf *pf)
 	case ICE_DEV_ID_E810_XXV_QSFP:
 	case ICE_DEV_ID_E810_XXV_SFP:
 		ice_set_feature_support(pf, ICE_F_DSCP);
-		if (ice_is_phy_rclk_present(&pf->hw))
+		if (ice_is_phy_rclk_in_netlist(&pf->hw))
 			ice_set_feature_support(pf, ICE_F_PHY_RCLK);
 		/* If we don't own the timer - don't enable other caps */
 		if (!ice_pf_src_tmr_owned(pf))
 			break;
-		if (ice_is_cgu_present(&pf->hw))
+		if (ice_is_cgu_in_netlist(&pf->hw))
 			ice_set_feature_support(pf, ICE_F_CGU);
-		if (ice_is_clock_mux_present_e810t(&pf->hw))
+		if (ice_is_clock_mux_in_netlist(&pf->hw))
 			ice_set_feature_support(pf, ICE_F_SMA_CTRL);
 		if (ice_gnss_is_gps_present(&pf->hw))
 			ice_set_feature_support(pf, ICE_F_GNSS);
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
index f839f186797d..de16cf14c4b2 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
@@ -3556,45 +3556,6 @@ int ice_clear_phy_tstamp(struct ice_hw *hw, u8 block, u8 idx)
 	}
 }
 
-/**
- * ice_is_phy_rclk_present - check recovered clk presence
- * @hw: pointer to the hw struct
- *
- * Check if the PHY Recovered Clock device is present in the netlist
- * Return:
- * * true - device found in netlist
- * * false - device not found
- */
-bool ice_is_phy_rclk_present(struct ice_hw *hw)
-{
-	if (ice_find_netlist_node(hw, ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_CTRL,
-				  ICE_AQC_GET_LINK_TOPO_NODE_NR_C827, NULL) &&
-	    ice_find_netlist_node(hw, ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_CTRL,
-				  ICE_AQC_GET_LINK_TOPO_NODE_NR_E822_PHY, NULL))
-		return false;
-
-	return true;
-}
-
-/**
- * ice_is_clock_mux_present_e810t
- * @hw: pointer to the hw struct
- *
- * Check if the Clock Multiplexer device is present in the netlist
- * Return:
- * * true - device found in netlist
- * * false - device not found
- */
-bool ice_is_clock_mux_present_e810t(struct ice_hw *hw)
-{
-	if (ice_find_netlist_node(hw, ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_MUX,
-				  ICE_AQC_GET_LINK_TOPO_NODE_NR_GEN_CLK_MUX,
-				  NULL))
-		return false;
-
-	return true;
-}
-
 /**
  * ice_get_pf_c827_idx - find and return the C827 index for the current pf
  * @hw: pointer to the hw struct
@@ -3708,33 +3669,6 @@ int ice_get_phy_tx_tstamp_ready(struct ice_hw *hw, u8 block, u64 *tstamp_ready)
 	}
 }
 
-/**
- * ice_is_cgu_present - check for CGU presence
- * @hw: pointer to the hw struct
- *
- * Check if the Clock Generation Unit (CGU) device is present in the netlist
- * Return:
- * * true - cgu is present
- * * false - cgu is not present
- */
-bool ice_is_cgu_present(struct ice_hw *hw)
-{
-	if (!ice_find_netlist_node(hw, ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_CTRL,
-				   ICE_AQC_GET_LINK_TOPO_NODE_NR_ZL30632_80032,
-				   NULL)) {
-		hw->cgu_part_number = ICE_AQC_GET_LINK_TOPO_NODE_NR_ZL30632_80032;
-		return true;
-	} else if (!ice_find_netlist_node(hw,
-					  ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_CTRL,
-					  ICE_AQC_GET_LINK_TOPO_NODE_NR_SI5383_5384,
-					  NULL)) {
-		hw->cgu_part_number = ICE_AQC_GET_LINK_TOPO_NODE_NR_SI5383_5384;
-		return true;
-	}
-
-	return false;
-}
-
 /**
  * ice_cgu_get_pin_desc_e823 - get pin description array
  * @hw: pointer to the hw struct
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
index 6f277e7b06b9..18a993134826 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
@@ -271,10 +271,7 @@ int ice_read_sma_ctrl_e810t(struct ice_hw *hw, u8 *data);
 int ice_write_sma_ctrl_e810t(struct ice_hw *hw, u8 data);
 int ice_read_pca9575_reg_e810t(struct ice_hw *hw, u8 offset, u8 *data);
 bool ice_is_pca9575_present(struct ice_hw *hw);
-bool ice_is_phy_rclk_present(struct ice_hw *hw);
-bool ice_is_clock_mux_present_e810t(struct ice_hw *hw);
 int ice_get_pf_c827_idx(struct ice_hw *hw, u8 *idx);
-bool ice_is_cgu_present(struct ice_hw *hw);
 enum dpll_pin_type ice_cgu_get_pin_type(struct ice_hw *hw, u8 pin, bool input);
 struct dpll_pin_frequency *
 ice_cgu_get_pin_freq_supp(struct ice_hw *hw, u8 pin, bool input, u8 *num);
-- 
2.41.0.1.g9857a21e0017.dirty

Powered by blists - more mailing lists