RE: [PATCH iwl-next] ice: describe generic DPLL pins from firmware pin classification

[Kubalewski, Arkadiusz]

>From: Petr Oros <poros@xxxxxxxxxx>
>Sent: Thursday, September 17, 2026 9:04 PM
>
>When the number of CGU pins reported by firmware does not match the
>static pin table for the device, the driver falls back to the generic
>pins added by commit 6e58c33106220c ("ice: fix crash on probe for DPLL
>enabled E810 LOM"), named "0" to "15". The same labels are used for
>inputs and outputs, so a PIN_ID_GET lookup by board label fails with
>"multiple matches" for any index present on both sides and userspace
>such as synce4l cannot address the pin at all. The pin type is guessed
>from the current frequency and no supported frequency list is exposed.
>
>Firmware already classifies every input pin. Get CGU Input Config
>(0x0C63) returns a type field marking GPS, PHY recovered clock and
>external inputs, and a capability field with the supported 1PPS and
>10 MHz frequencies. The driver discards both, ice_dpll_pin_state_update()
>passes NULL for them.
>
>Use them in the generic path. Inputs become GNSS-<n>, RCLK-<n>, EXT-<n>,
>or IN-<n> when firmware reports no kind, with the matching dpll pin type
>and frequency list. An external input flagged as accepting any frequency
>advertises the standard 1PPS and 10 MHz pair. Outputs become OUT-<n>.
>Labels are unique across both directions and the 16 pin limit, which
>existed only because of the static label table, is gone. Labels that
>happened to be unique before, indexes above the smaller of the two pin
>counts, change as well.
>
>On an E810-C timing board firmware reports the C827 recovered clock
>inputs as PHY, the SMA inputs as external with 1PPS and 10 MHz, and the
>GNSS input as GPS with 1PPS, matching the static table for that board.
>A vendor board with 8 inputs and 15 outputs, which takes the generic
>path today, gets addressable and correctly typed pins instead of
>ambiguous numbers. The DPLL registration on such boards is fixed by
>"ice: DPLL init fixes for E810 timing boards" on iwl-net, this patch
>is independent of it.
>
>Signed-off-by: Petr Oros <poros@xxxxxxxxxx>


Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@xxxxxxxxx>

>---
> drivers/net/ethernet/intel/ice/ice_dpll.c   | 72 +++++++++++++++++----
> drivers/net/ethernet/intel/ice/ice_dpll.h   |  2 +
> drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 31 +++++++++
> drivers/net/ethernet/intel/ice/ice_ptp_hw.h |  2 +
> 4 files changed, 95 insertions(+), 12 deletions(-)
>
>diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c
>b/drivers/net/ethernet/intel/ice/ice_dpll.c
>index 81bb32d2b23012..6e3e8fa94a3823 100644
>--- a/drivers/net/ethernet/intel/ice/ice_dpll.c
>+++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
>@@ -4284,10 +4284,60 @@ static void ice_dpll_phase_range_set(struct
>dpll_pin_phase_adjust_range *range,
> 	range->max = phase_adj;
> }
>
>+/**
>+ * ice_dpll_init_info_pin_generic_input - describe a generic input pin
>+ * @pf: board private structure
>+ * @pin: pin to describe
>+ *
>+ * Derive the pin type, board label and supported frequencies from the
>pin
>+ * classification reported by firmware.
>+ *
>+ * Return:
>+ * * 0 - success
>+ * * negative - AQ failure
>+ */
>+static int
>+ice_dpll_init_info_pin_generic_input(struct ice_pf *pf, struct
>ice_dpll_pin *pin)
>+{
>+	u8 type, caps, num;
>+	const char *kind;
>+	int ret;
>+
>+	ret = ice_aq_get_input_pin_cfg(&pf->hw, pin->idx, NULL, &type,
>&caps,
>+				       NULL, NULL, NULL);
>+	if (ret)
>+		return ret;
>+
>+	if (type & ICE_AQC_GET_CGU_IN_CFG_TYPE_GPS) {
>+		pin->prop.type = DPLL_PIN_TYPE_GNSS;
>+		kind = "GNSS";
>+	} else if (type & ICE_AQC_GET_CGU_IN_CFG_TYPE_PHY) {
>+		pin->prop.type = DPLL_PIN_TYPE_MUX;
>+		kind = "RCLK";
>+	} else if (type & ICE_AQC_GET_CGU_IN_CFG_TYPE_EXTERNAL) {
>+		pin->prop.type = DPLL_PIN_TYPE_EXT;
>+		kind = "EXT";
>+		if (caps & ICE_AQC_GET_CGU_IN_CFG_FLG1_ANYFREQ)
>+			caps |= ICE_AQC_GET_CGU_IN_CFG_FLG1_1PPS_SUPP |
>+				ICE_AQC_GET_CGU_IN_CFG_FLG1_10MHZ_SUPP;
>+	} else {
>+		pin->prop.type = pin->freq == ICE_DPLL_PIN_GEN_RCLK_FREQ ?
>+				 DPLL_PIN_TYPE_MUX : DPLL_PIN_TYPE_EXT;
>+		kind = "IN";
>+	}
>+	snprintf(pin->label, sizeof(pin->label), "%s-%u", kind, pin->idx);
>+	pin->prop.board_label = pin->label;
>+	pin->prop.freq_supported =
>+		ice_cgu_get_pin_freq_supp_by_caps(caps, &num);
>+	pin->prop.freq_supported_num = num;
>+
>+	return 0;
>+}
>+
> /**
>  * ice_dpll_init_info_pins_generic - initializes generic pins info
>  * @pf: board private structure
>- * @input: if input pins initialized
>+ * @input: if we are initializing input pins
>  *
>  * Init information for generic pins, cache them in PF's pins structures.
>  *
>@@ -4298,13 +4348,10 @@ static void ice_dpll_phase_range_set(struct
>dpll_pin_phase_adjust_range *range,
> static int ice_dpll_init_info_pins_generic(struct ice_pf *pf, bool input)
> {
> 	struct ice_dpll *de = &pf->dplls.eec, *dp = &pf->dplls.pps;
>-	static const char labels[][sizeof("99")] = {
>-		"0", "1", "2", "3", "4", "5", "6", "7", "8",
>-		"9", "10", "11", "12", "13", "14", "15" };
> 	u32 cap = DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE;
> 	enum ice_dpll_pin_type pin_type;
>-	int i, pin_num, ret = -EINVAL;
> 	struct ice_dpll_pin *pins;
>+	int i, pin_num, ret = 0;
> 	u32 phase_adj_max;
>
> 	if (input) {
>@@ -4319,12 +4366,9 @@ static int ice_dpll_init_info_pins_generic(struct
>ice_pf *pf, bool input)
> 		phase_adj_max = pf->dplls.output_phase_adj_max;
> 		pin_type = ICE_DPLL_PIN_TYPE_OUTPUT;
> 	}
>-	if (pin_num > ARRAY_SIZE(labels))
>-		return ret;
>
> 	for (i = 0; i < pin_num; i++) {
> 		pins[i].idx = i;
>-		pins[i].prop.board_label = labels[i];
> 		ice_dpll_phase_range_set(&pins[i].prop.phase_range,
> 					 phase_adj_max);
> 		pins[i].prop.capabilities = cap;
>@@ -4332,12 +4376,16 @@ static int ice_dpll_init_info_pins_generic(struct
>ice_pf *pf, bool input)
> 		ret = ice_dpll_pin_state_update(pf, &pins[i], pin_type, NULL);
> 		if (ret)
> 			break;
>-		if (input && pins[i].freq == ICE_DPLL_PIN_GEN_RCLK_FREQ)
>-			pins[i].prop.type = DPLL_PIN_TYPE_MUX;
>-		else
>+		if (!input) {
>+			snprintf(pins[i].label, sizeof(pins[i].label),
>+				 "OUT-%u", pins[i].idx);
>+			pins[i].prop.board_label = pins[i].label;
> 			pins[i].prop.type = DPLL_PIN_TYPE_EXT;
>-		if (!input)
> 			continue;
>+		}
>+		ret = ice_dpll_init_info_pin_generic_input(pf, &pins[i]);
>+		if (ret)
>+			break;
> 		ret = ice_aq_get_cgu_ref_prio(&pf->hw, de->dpll_idx, i,
> 					      &de->input_prio[i]);
> 		if (ret)
>diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.h
>b/drivers/net/ethernet/intel/ice/ice_dpll.h
>index 103ba3e490682c..2360d920eb9ed8 100644
>--- a/drivers/net/ethernet/intel/ice/ice_dpll.h
>+++ b/drivers/net/ethernet/intel/ice/ice_dpll.h
>@@ -6,6 +6,7 @@
>
> #include "ice.h"
>
>+#define ICE_DPLL_PIN_LABEL_LEN		16
> #define ICE_DPLL_RCLK_NUM_MAX	4
> #define ICE_DPLL_TXCLK_NUM_MAX	2
> #define E825_EXT_EREF_PIN_IDX	0
>@@ -83,6 +84,7 @@ struct ice_dpll_pin {
> 	bool active;
> 	bool hidden;
> 	enum ice_e825c_ref_clk tx_ref_src;
>+	char label[ICE_DPLL_PIN_LABEL_LEN];
> };
>
> /** ice_dpll - store info required for DPLL control
>diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
>b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
>index 3a41c711e751b5..2ee6a720bd8929 100644
>--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
>+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
>@@ -6059,6 +6059,37 @@ ice_cgu_get_pin_freq_supp(struct ice_hw *hw, u8
>pin, bool input, u8 *num)
> 	return t[pin].freq_supp;
> }
>
>+/**
>+ * ice_cgu_get_pin_freq_supp_by_caps - get supported frequencies from pin
>caps
>+ * @caps: input pin capability flags from Get CGU Input Config
>+ * @num: number of returned frequencies
>+ *
>+ * Return: array of supported frequencies, NULL if no fixed frequency is
>+ * advertised.
>+ */
>+struct dpll_pin_frequency *
>+ice_cgu_get_pin_freq_supp_by_caps(u8 caps, u8 *num)
>+{
>+	bool pps = caps & ICE_AQC_GET_CGU_IN_CFG_FLG1_1PPS_SUPP;
>+	bool mhz = caps & ICE_AQC_GET_CGU_IN_CFG_FLG1_10MHZ_SUPP;
>+
>+	if (pps && mhz) {
>+		*num = ARRAY_SIZE(ice_cgu_pin_freq_common);
>+		return ice_cgu_pin_freq_common;
>+	}
>+	if (pps) {
>+		*num = ARRAY_SIZE(ice_cgu_pin_freq_1_hz);
>+		return ice_cgu_pin_freq_1_hz;
>+	}
>+	if (mhz) {
>+		*num = ARRAY_SIZE(ice_cgu_pin_freq_10_mhz);
>+		return ice_cgu_pin_freq_10_mhz;
>+	}
>+	*num = 0;
>+
>+	return NULL;
>+}
>+
> /**
>  * ice_cgu_get_pin_name - get pin's name
>  * @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 16b1988e993d2f..c59d9488b52220 100644
>--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
>+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
>@@ -367,6 +367,8 @@ int ice_cgu_get_num_pins(struct ice_hw *hw, bool
>input);
> 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);
>+struct dpll_pin_frequency *
>+ice_cgu_get_pin_freq_supp_by_caps(u8 caps, u8 *num);
> const char *ice_cgu_get_pin_name(struct ice_hw *hw, u8 pin, bool input);
> int ice_get_cgu_state(struct ice_hw *hw, u8 dpll_idx,
> 		      enum dpll_lock_status last_dpll_state, u8 *pin,
>--
>2.55.0