Introduce a TX clock DPLL instance and pins for E825-based devices.
The TXC domain exposes two reference pins:
  - EXT_EREF0 (board external electrical reference)
  - SYNCE (port-derived reference, described via fwnode)

A new pin type (ICE_DPLL_PIN_TYPE_TXCLK) and pin ops are added. The
SYNCE pin is registered when its fwnode becomes available, while
EXT_EREF0 is registered directly. The notifier worker is updated to
register/unregister TXC pins and to ignore notifications that are a
side effect of internal registration by checking the src_id.

A new per-pin attribute tracks the TX reference source
(enum ice_e825c_ref_clk). The TXC DPLL device is created and torn
down alongside existing PPS/EEC devices on E825, with specific init
and deinit flows for the TXC pins.

Current TXC pin state getters return DISCONNECTED as a placeholder;
hardware-backed state reporting will be implemented in a follow-up.

Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_dpll.c   | 267 ++++++++++++++++++--
 drivers/net/ethernet/intel/ice/ice_dpll.h   |   6 +
 drivers/net/ethernet/intel/ice/ice_ptp_hw.h |   7 +
 3 files changed, 260 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c
index 62f75701d652..a1258f2e03a9 100644
--- a/drivers/net/ethernet/intel/ice/ice_dpll.c
+++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
@@ -19,6 +19,11 @@
 #define ICE_DPLL_SW_PIN_INPUT_BASE_QSFP		6
 #define ICE_DPLL_SW_PIN_OUTPUT_BASE		0
 
+#define E825_EXT_EREF_PIN_IDX			0
+#define E825_EXT_SYNCE_PIN_IDX			1
+#define E825_RCLK_PARENT_0_PIN_IDX		0
+#define E825_RCLK_PARENT_1_PIN_IDX		1
+
 #define ICE_DPLL_PIN_SW_INPUT_ABS(in_idx) \
 	(ICE_DPLL_SW_PIN_INPUT_BASE_SFP + (in_idx))
 
@@ -57,6 +62,7 @@
  * @ICE_DPLL_PIN_TYPE_OUTPUT: output pin
  * @ICE_DPLL_PIN_TYPE_RCLK_INPUT: recovery clock input pin
  * @ICE_DPLL_PIN_TYPE_SOFTWARE: software controlled SMA/U.FL pins
+ * @ICE_DPLL_PIN_TYPE_TXCLK: transmit clock reference input pin
  */
 enum ice_dpll_pin_type {
 	ICE_DPLL_PIN_INVALID,
@@ -64,6 +70,7 @@ enum ice_dpll_pin_type {
 	ICE_DPLL_PIN_TYPE_OUTPUT,
 	ICE_DPLL_PIN_TYPE_RCLK_INPUT,
 	ICE_DPLL_PIN_TYPE_SOFTWARE,
+	ICE_DPLL_PIN_TYPE_TXCLK,
 };
 
 static const char * const pin_type_name[] = {
@@ -71,10 +78,13 @@ static const char * const pin_type_name[] = {
 	[ICE_DPLL_PIN_TYPE_OUTPUT] = "output",
 	[ICE_DPLL_PIN_TYPE_RCLK_INPUT] = "rclk-input",
 	[ICE_DPLL_PIN_TYPE_SOFTWARE] = "software",
+	[ICE_DPLL_PIN_TYPE_TXCLK] = "txclk-input",
 };
 
 static const char * const ice_dpll_sw_pin_sma[] = { "SMA1", "SMA2" };
 static const char * const ice_dpll_sw_pin_ufl[] = { "U.FL1", "U.FL2" };
+static const char * const ice_dpll_ext_eref_pin = "EXT_EREF0";
+static const char * const ice_dpll_fwnode_ext_synce = "clk_ref_synce";
 
 static const struct dpll_pin_frequency ice_esync_range[] = {
 	DPLL_PIN_FREQUENCY_RANGE(0, DPLL_PIN_FREQUENCY_1_HZ),
@@ -2517,12 +2527,75 @@ ice_dpll_rclk_state_on_pin_get(const struct dpll_pin *pin, void *pin_priv,
 	return ret;
 }
 
+/**
+ * ice_dpll_txclk_state_on_dpll_set - set a state on TX clk pin
+ * @pin: pointer to a pin
+ * @pin_priv: private data pointer passed on pin registration
+ * @dpll: registered dpll pointer
+ * @dpll_priv: private data pointer passed on dpll registration
+ * @state: state to be set on pin
+ * @extack: error reporting
+ *
+ * Dpll subsystem callback, set a state of a Tx reference clock pin
+ *
+ * Return:
+ * * negative - failure
+ */
+static int
+ice_dpll_txclk_state_on_dpll_set(const struct dpll_pin *pin, void *pin_priv,
+				 const struct dpll_device *dpll,
+				 void *dpll_priv, enum dpll_pin_state state,
+				 struct netlink_ext_ack *extack)
+{
+	/*
+	 * TODO: set HW accordingly to selected TX reference clock.
+	 * To be added in the follow up patches.
+	 */
+	return -EOPNOTSUPP;
+}
+
+/**
+ * ice_dpll_txclk_state_on_dpll_get - get a state of Tx clk reference pin
+ * @pin: pointer to a pin
+ * @pin_priv: private data pointer passed on pin registration
+ * @dpll: registered dpll pointer
+ * @dpll_priv: private data pointer passed on dpll registration
+ * @state: on success holds pin state on parent pin
+ * @extack: error reporting
+ *
+ * dpll subsystem callback, get a state of a TX clock reference pin.
+ *
+ * Return:
+ * * 0 - success
+ */
+static int
+ice_dpll_txclk_state_on_dpll_get(const struct dpll_pin *pin, void *pin_priv,
+				 const struct dpll_device *dpll,
+				 void *dpll_priv,
+				 enum dpll_pin_state *state,
+				 struct netlink_ext_ack *extack)
+{
+	/*
+	 * TODO: query HW status to determine if the TX reference is selected.
+	 * To be added in the follow up patches.
+	 */
+	*state = DPLL_PIN_STATE_DISCONNECTED;
+
+	return 0;
+}
+
 static const struct dpll_pin_ops ice_dpll_rclk_ops = {
 	.state_on_pin_set = ice_dpll_rclk_state_on_pin_set,
 	.state_on_pin_get = ice_dpll_rclk_state_on_pin_get,
 	.direction_get = ice_dpll_input_direction,
 };
 
+static const struct dpll_pin_ops ice_dpll_txclk_ops = {
+	.state_on_dpll_set = ice_dpll_txclk_state_on_dpll_set,
+	.state_on_dpll_get = ice_dpll_txclk_state_on_dpll_get,
+	.direction_get = ice_dpll_input_direction,
+};
+
 static const struct dpll_pin_ops ice_dpll_pin_sma_ops = {
 	.state_on_dpll_set = ice_dpll_sma_pin_state_set,
 	.state_on_dpll_get = ice_dpll_sw_pin_state_get,
@@ -3199,19 +3272,40 @@ static bool ice_dpll_is_fwnode_pin(struct ice_dpll_pin *pin)
 	return !IS_ERR_OR_NULL(pin->fwnode);
 }
 
+static bool ice_dpll_fwnode_eq(const struct fwnode_handle *a,
+			       const struct fwnode_handle *b)
+{
+	return a && b && a == b;
+}
+
 static void ice_dpll_pin_notify_work(struct work_struct *work)
 {
 	struct ice_dpll_pin_work *w = container_of(work,
 						   struct ice_dpll_pin_work,
 						   work);
 	struct ice_dpll_pin *pin, *parent = w->pin;
+	bool is_tx_synce_parent = false;
 	struct ice_pf *pf = parent->pf;
+	bool is_rclk_parent = false;
 	int ret;
 
 	wait_for_completion(&pf->dplls.dpll_init);
 	if (!test_bit(ICE_FLAG_DPLL, pf->flags))
 		goto out; /* DPLL initialization failed */
 
+	/* Decide which parent we are handling, defensively checking FWNs */
+	is_rclk_parent =
+		ice_dpll_fwnode_eq(parent->fwnode,
+				   pf->dplls.inputs[E825_RCLK_PARENT_0_PIN_IDX].fwnode) ||
+		ice_dpll_fwnode_eq(parent->fwnode,
+				   pf->dplls.inputs[E825_RCLK_PARENT_1_PIN_IDX].fwnode);
+
+	is_tx_synce_parent =
+		ice_dpll_fwnode_eq(parent->fwnode,
+				   pf->dplls.txclks[E825_EXT_SYNCE_PIN_IDX].fwnode);
+	if (!is_rclk_parent && !is_tx_synce_parent)
+		goto out;
+
 	switch (w->action) {
 	case DPLL_PIN_CREATED:
 		if (!IS_ERR_OR_NULL(parent->pin)) {
@@ -3228,16 +3322,28 @@ static void ice_dpll_pin_notify_work(struct work_struct *work)
 			goto out;
 		}
 
-		/* Register rclk pin */
-		pin = &pf->dplls.rclk;
-		ret = dpll_pin_on_pin_register(parent->pin, pin->pin,
-					       &ice_dpll_rclk_ops, pin);
-		if (ret) {
-			dev_err(ice_pf_to_dev(pf),
-				"Failed to register pin: %pe\n", ERR_PTR(ret));
-			dpll_pin_put(parent->pin, &parent->tracker);
-			parent->pin = NULL;
-			goto out;
+		if (is_rclk_parent) {
+			/* Register rclk pin via on-pin relationship */
+			pin = &pf->dplls.rclk;
+			ret = dpll_pin_on_pin_register(parent->pin, pin->pin,
+						       &ice_dpll_rclk_ops, pin);
+			if (ret) {
+				dev_err(ice_pf_to_dev(pf),
+					"RCLK pin register failed: %pe\n",
+					ERR_PTR(ret));
+				goto drop_parent_ref;
+			}
+		} else if (is_tx_synce_parent) {
+			/* Register TX-CLK SYNCE pin directly to TXC DPLL */
+			pin = &pf->dplls.txclks[E825_EXT_SYNCE_PIN_IDX];
+			ret = dpll_pin_register(pf->dplls.txc.dpll, pin->pin,
+						&ice_dpll_txclk_ops, pin);
+			if (ret) {
+				dev_err(ice_pf_to_dev(pf),
+					"TX SYNCE pin register failed: %pe\n",
+					ERR_PTR(ret));
+				goto drop_parent_ref;
+			}
 		}
 		break;
 	case DPLL_PIN_DELETED:
@@ -3246,11 +3352,18 @@ static void ice_dpll_pin_notify_work(struct work_struct *work)
 			goto out;
 		}
 
-		/* Unregister rclk pin */
-		pin = &pf->dplls.rclk;
-		dpll_pin_on_pin_unregister(parent->pin, pin->pin,
-					   &ice_dpll_rclk_ops, pin);
-
+		if (is_rclk_parent) {
+			/* Unregister rclk pin */
+			pin = &pf->dplls.rclk;
+			dpll_pin_on_pin_unregister(parent->pin, pin->pin,
+						   &ice_dpll_rclk_ops, pin);
+		} else if (is_tx_synce_parent) {
+			/* Unregister TX-CLK SYNCE pin from TXC DPLL */
+			pin = &pf->dplls.txclks[E825_EXT_SYNCE_PIN_IDX];
+			dpll_pin_unregister(pf->dplls.txc.dpll, pin->pin,
+					    &ice_dpll_txclk_ops, pin);
+		}
+drop_parent_ref:
 		/* Drop fwnode pin reference */
 		dpll_pin_put(parent->pin, &parent->tracker);
 		parent->pin = NULL;
@@ -3276,6 +3389,12 @@ static int ice_dpll_pin_notify(struct notifier_block *nb, unsigned long action,
 	if (pin->fwnode != info->fwnode)
 		return NOTIFY_DONE; /* Not this pin */
 
+	/* Ignore notification which are the outcome of internal pin
+	 * registration/unregistration calls - synce pin case.
+	 */
+	if (info->src_id == pin->pf->dplls.clock_id)
+		return NOTIFY_DONE;
+
 	work = kzalloc_obj(*work);
 	if (!work)
 		return NOTIFY_DONE;
@@ -3401,6 +3520,19 @@ ice_dpll_deinit_fwnode_pins(struct ice_pf *pf, struct ice_dpll_pin *pins,
 	destroy_workqueue(pf->dplls.wq);
 }
 
+static int ice_dpll_deinit_txclk_pins(struct ice_pf *pf)
+{
+	struct ice_dpll_pin *synce_pin = &pf->dplls.txclks[E825_EXT_SYNCE_PIN_IDX];
+	struct ice_dpll *dt = &pf->dplls.txc;
+
+	ice_dpll_unregister_pins(dt->dpll, pf->dplls.txclks,
+				 &ice_dpll_txclk_ops,
+				 ARRAY_SIZE(pf->dplls.txclks));
+	ice_dpll_release_pins(&pf->dplls.txclks[E825_EXT_EREF_PIN_IDX], 1);
+	ice_dpll_deinit_fwnode_pin(synce_pin);
+	return 0;
+}
+
 /**
  * ice_dpll_deinit_pins - deinitialize direct pins
  * @pf: board private structure
@@ -3420,8 +3552,10 @@ static void ice_dpll_deinit_pins(struct ice_pf *pf, bool cgu)
 	struct ice_dpll *dp = &d->pps;
 
 	ice_dpll_deinit_rclk_pin(pf);
-	if (pf->hw.mac_type == ICE_MAC_GENERIC_3K_E825)
+	if (pf->hw.mac_type == ICE_MAC_GENERIC_3K_E825) {
+		ice_dpll_deinit_txclk_pins(pf);
 		ice_dpll_deinit_fwnode_pins(pf, pf->dplls.inputs, 0);
+	}
 	if (cgu) {
 		ice_dpll_unregister_pins(dp->dpll, inputs, &ice_dpll_input_ops,
 					 num_inputs);
@@ -3552,6 +3686,39 @@ ice_dpll_init_fwnode_pins(struct ice_pf *pf, struct ice_dpll_pin *pins,
 	return ret;
 }
 
+static int ice_dpll_init_txclk_pins(struct ice_pf *pf, int start_idx)
+{
+	struct ice_dpll_pin *ref_pin = pf->dplls.txclks;
+	struct ice_dpll *txc = &pf->dplls.txc;
+	int ret = 0;
+
+	/* configure EXT_EREF0 pin */
+	ret = ice_dpll_get_pins(pf, ref_pin, start_idx, 1, pf->dplls.clock_id);
+	if (ret)
+		return ret;
+	ret = dpll_pin_register(txc->dpll, ref_pin->pin, &ice_dpll_txclk_ops,
+				ref_pin);
+	if (ret)
+		return ret;
+
+	/* configure EXT_SYNCE pin which is based on fwnode pin */
+	ref_pin++;
+	ret = ice_dpll_init_fwnode_pin(ref_pin, ice_dpll_fwnode_ext_synce);
+	if (ret)
+		return ret;
+
+	ret = dpll_pin_register(txc->dpll, ref_pin->pin, &ice_dpll_txclk_ops,
+				ref_pin);
+	if (ret)
+		return ret;
+
+	if (IS_ERR_OR_NULL(ref_pin->pin))
+		dev_dbg(ice_pf_to_dev(pf),
+			"Tx-clk mux pin not registered yet\n");
+
+	return 0;
+}
+
 /**
  * ice_dpll_init_pins_e825 - init pins and register pins with a dplls
  * @pf: board private structure
@@ -3574,6 +3741,15 @@ static int ice_dpll_init_pins_e825(struct ice_pf *pf)
 
 	ret = ice_dpll_init_rclk_pin(pf, DPLL_PIN_IDX_UNSPEC,
 				     &ice_dpll_rclk_ops);
+
+	if (ret)
+		goto unregister_pins;
+
+	ret = ice_dpll_init_txclk_pins(pf, 0);
+	if (ret)
+		ice_dpll_deinit_rclk_pin(pf);
+
+unregister_pins:
 	if (ret) {
 		/* Inform DPLL notifier works that DPLL init was finished
 		 * unsuccessfully (ICE_DPLL_FLAG not set).
@@ -3692,7 +3868,7 @@ static int ice_dpll_init_pins(struct ice_pf *pf, bool cgu)
 static void
 ice_dpll_deinit_dpll(struct ice_pf *pf, struct ice_dpll *d, bool cgu)
 {
-	if (cgu)
+	if (cgu || pf->hw.mac_type == ICE_MAC_GENERIC_3K_E825)
 		dpll_device_unregister(d->dpll, d->ops, d);
 	dpll_device_put(d->dpll, &d->tracker);
 }
@@ -3727,12 +3903,13 @@ ice_dpll_init_dpll(struct ice_pf *pf, struct ice_dpll *d, bool cgu,
 		return ret;
 	}
 	d->pf = pf;
-	if (cgu) {
+	if (cgu || pf->hw.mac_type == ICE_MAC_GENERIC_3K_E825) {
 		const struct dpll_device_ops *ops = &ice_dpll_ops;
 
 		if (type == DPLL_TYPE_PPS && ice_dpll_is_pps_phase_monitor(pf))
 			ops =  &ice_dpll_pom_ops;
-		ice_dpll_update_state(pf, d, true);
+		if (cgu)
+			ice_dpll_update_state(pf, d, true);
 		ret = dpll_device_register(d->dpll, type, ops, d);
 		if (ret) {
 			dpll_device_put(d->dpll, &d->tracker);
@@ -4081,6 +4258,36 @@ static int ice_dpll_init_info_sw_pins(struct ice_pf *pf)
 	return 0;
 }
 
+/**
+ * ice_dpll_init_info_txclk_pins_e825c - initializes tx-clk pins information
+ * @pf: board private structure
+ *
+ * Init information for tx-clks pin, cache them in pf->dplls.txclks
+ *
+ * Return:
+ * * 0 - success
+ */
+static int ice_dpll_init_info_txclk_pins_e825c(struct ice_pf *pf)
+{
+	struct ice_dpll_pin *tx_pin;
+
+	for (int i = 0; i < ICE_DPLL_TXCLK_NUM_MAX; i++) {
+		tx_pin = &pf->dplls.txclks[i];
+		tx_pin->prop.type = DPLL_PIN_TYPE_EXT;
+		tx_pin->prop.capabilities |=
+				 DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE;
+		tx_pin->pf = pf;
+		if (i == E825_EXT_EREF_PIN_IDX) {
+			tx_pin->prop.board_label = ice_dpll_ext_eref_pin;
+			tx_pin->tx_ref_src = ICE_REF_CLK_EREF0;
+		} else if (i == E825_EXT_SYNCE_PIN_IDX) {
+			tx_pin->tx_ref_src = ICE_REF_CLK_SYNCE;
+		}
+	}
+
+	return 0;
+}
+
 /**
  * ice_dpll_init_pins_info - init pins info wrapper
  * @pf: board private structure
@@ -4106,6 +4313,9 @@ ice_dpll_init_pins_info(struct ice_pf *pf, enum ice_dpll_pin_type pin_type)
 			return ice_dpll_init_info_rclk_pin(pf);
 	case ICE_DPLL_PIN_TYPE_SOFTWARE:
 		return ice_dpll_init_info_sw_pins(pf);
+
+	case ICE_DPLL_PIN_TYPE_TXCLK:
+		return ice_dpll_init_info_txclk_pins_e825c(pf);
 	default:
 		return -EINVAL;
 	}
@@ -4139,11 +4349,15 @@ static void ice_dpll_deinit_info(struct ice_pf *pf)
 static int ice_dpll_init_info_e825c(struct ice_pf *pf)
 {
 	struct ice_dplls *d = &pf->dplls;
+	struct ice_dpll *dt = &d->txc;
 	int ret = 0;
 	int i;
 
 	d->clock_id = ice_generate_clock_id(pf);
 	d->num_inputs = ICE_SYNCE_CLK_NUM;
+	dt->dpll_state = DPLL_LOCK_STATUS_LOCKED;
+	dt->mode = DPLL_MODE_MANUAL;
+	dt->dpll_idx = pf->ptp.port.port_num;
 
 	d->inputs = kzalloc_objs(*d->inputs, d->num_inputs);
 	if (!d->inputs)
@@ -4160,6 +4374,11 @@ static int ice_dpll_init_info_e825c(struct ice_pf *pf)
 	ret = ice_dpll_init_pins_info(pf, ICE_DPLL_PIN_TYPE_RCLK_INPUT);
 	if (ret)
 		goto deinit_info;
+
+	ret = ice_dpll_init_pins_info(pf, ICE_DPLL_PIN_TYPE_TXCLK);
+	if (ret)
+		goto deinit_info;
+
 	dev_dbg(ice_pf_to_dev(pf),
 		"%s - success, inputs: %u, outputs: %u, rclk-parents: %u\n",
 		 __func__, d->num_inputs, d->num_outputs, d->rclk.num_parents);
@@ -4292,6 +4511,9 @@ void ice_dpll_deinit(struct ice_pf *pf)
 		ice_dpll_deinit_dpll(pf, &pf->dplls.pps, cgu);
 	if (!IS_ERR_OR_NULL(pf->dplls.eec.dpll))
 		ice_dpll_deinit_dpll(pf, &pf->dplls.eec, cgu);
+	if (!IS_ERR_OR_NULL(pf->dplls.txc.dpll))
+		ice_dpll_deinit_dpll(pf, &pf->dplls.txc, false);
+
 	ice_dpll_deinit_info(pf);
 	mutex_destroy(&pf->dplls.lock);
 }
@@ -4317,14 +4539,19 @@ static void ice_dpll_init_e825(struct ice_pf *pf)
 	err = ice_dpll_init_info_e825c(pf);
 	if (err)
 		goto err_exit;
-	err = ice_dpll_init_pins_e825(pf);
+	err = ice_dpll_init_dpll(pf, &pf->dplls.txc, false, DPLL_TYPE_TXC);
 	if (err)
 		goto deinit_info;
+	err = ice_dpll_init_pins_e825(pf);
+	if (err)
+		goto deinit_txclk;
 	set_bit(ICE_FLAG_DPLL, pf->flags);
 	complete_all(&d->dpll_init);
 
 	return;
 
+deinit_txclk:
+	ice_dpll_deinit_dpll(pf, &pf->dplls.txc, false);
 deinit_info:
 	ice_dpll_deinit_info(pf);
 err_exit:
diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.h b/drivers/net/ethernet/intel/ice/ice_dpll.h
index ae42cdea0ee1..23f9d4da73c5 100644
--- a/drivers/net/ethernet/intel/ice/ice_dpll.h
+++ b/drivers/net/ethernet/intel/ice/ice_dpll.h
@@ -7,6 +7,7 @@
 #include "ice.h"
 
 #define ICE_DPLL_RCLK_NUM_MAX	4
+#define ICE_DPLL_TXCLK_NUM_MAX	2
 
 /**
  * enum ice_dpll_pin_sw - enumerate ice software pin indices:
@@ -63,6 +64,7 @@ struct ice_dpll_pin {
 	u8 ref_sync;
 	bool active;
 	bool hidden;
+	enum ice_e825c_ref_clk tx_ref_src;
 };
 
 /** ice_dpll - store info required for DPLL control
@@ -111,9 +113,11 @@ struct ice_dpll {
  * @lock: locks access to configuration of a dpll
  * @eec: pointer to EEC dpll dev
  * @pps: pointer to PPS dpll dev
+ * @txc: pointer to TXC dpll dev
  * @inputs: input pins pointer
  * @outputs: output pins pointer
  * @rclk: recovered pins pointer
+ * @txclks: TX clock reference pins pointer
  * @num_inputs: number of input pins available on dpll
  * @num_outputs: number of output pins available on dpll
  * @cgu_state_acq_err_num: number of errors returned during periodic work
@@ -131,11 +135,13 @@ struct ice_dplls {
 	struct completion dpll_init;
 	struct ice_dpll eec;
 	struct ice_dpll pps;
+	struct ice_dpll txc;
 	struct ice_dpll_pin *inputs;
 	struct ice_dpll_pin *outputs;
 	struct ice_dpll_pin sma[ICE_DPLL_PIN_SW_NUM];
 	struct ice_dpll_pin ufl[ICE_DPLL_PIN_SW_NUM];
 	struct ice_dpll_pin rclk;
+	struct ice_dpll_pin txclks[ICE_DPLL_TXCLK_NUM_MAX];
 	u8 num_inputs;
 	u8 num_outputs;
 	u8 sma_data;
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
index 9bfd3e79c580..cbc9693179a1 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
@@ -265,6 +265,13 @@ struct ice_cgu_pin_desc {
 	struct dpll_pin_frequency *freq_supp;
 };
 
+enum ice_e825c_ref_clk {
+	ICE_REF_CLK_ENET,
+	ICE_REF_CLK_SYNCE,
+	ICE_REF_CLK_EREF0,
+	ICE_REF_CLK_MAX,
+};
+
 #define E810C_QSFP_C827_0_HANDLE 2
 #define E810C_QSFP_C827_1_HANDLE 3
 
-- 
2.39.3

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
ice: add TX clock (TXC) DPLL interface for E825 devices

This commit introduces a TX clock DPLL instance and pins for E825-based
devices. The TXC domain exposes two reference pins: EXT_EREF0 (board
external electrical reference) and SYNCE (port-derived reference,
described via fwnode). A new pin type and pin ops are added to support
TX clock references.

> diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c
> index 62f75701d652..a1258f2e03a9 100644
> --- a/drivers/net/ethernet/intel/ice/ice_dpll.c
> +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c

[ ... ]

> @@ -3552,6 +3686,39 @@ ice_dpll_init_fwnode_pins(struct ice_pf *pf, struct ice_dpll_pin *pins,
>  	return ret;
>  }
>
> +static int ice_dpll_init_txclk_pins(struct ice_pf *pf, int start_idx)
> +{
> +	struct ice_dpll_pin *ref_pin = pf->dplls.txclks;
> +	struct ice_dpll *txc = &pf->dplls.txc;
> +	int ret = 0;
> +
> +	/* configure EXT_EREF0 pin */
> +	ret = ice_dpll_get_pins(pf, ref_pin, start_idx, 1, pf->dplls.clock_id);
> +	if (ret)
> +		return ret;
> +	ret = dpll_pin_register(txc->dpll, ref_pin->pin, &ice_dpll_txclk_ops,
> +				ref_pin);
> +	if (ret)
> +		return ret;
> +
> +	/* configure EXT_SYNCE pin which is based on fwnode pin */
> +	ref_pin++;
> +	ret = ice_dpll_init_fwnode_pin(ref_pin, ice_dpll_fwnode_ext_synce);
> +	if (ret)
> +		return ret;
> +
> +	ret = dpll_pin_register(txc->dpll, ref_pin->pin, &ice_dpll_txclk_ops,
> +				ref_pin);
> +	if (ret)
> +		return ret;

Does ice_dpll_init_txclk_pins leak the EXT_EREF0 pin on error?

If dpll_pin_register for the EXT_SYNCE pin fails, the function returns
the error without cleaning up the EXT_EREF0 pin that was already
successfully registered. Looking at the call chain:

ice_dpll_init_pins_e825() calls:
    ice_dpll_init_txclk_pins()
        ice_dpll_get_pins() -> acquires pin reference for EXT_EREF0
        dpll_pin_register() -> registers EXT_EREF0 with TXC DPLL
        ice_dpll_init_fwnode_pin() -> may fail here
        dpll_pin_register() -> or may fail here for EXT_SYNCE

If either of the last two steps fail, the function returns the error
directly without calling dpll_pin_unregister or ice_dpll_release_pins
for the EXT_EREF0 pin.

The caller ice_dpll_init_pins_e825() also doesn't compensate - on
txclk init failure it only calls ice_dpll_deinit_rclk_pin() and
ice_dpll_deinit_fwnode_pins(), neither of which clean up txclk
resources.

Compare with ice_dpll_init_fwnode_pins() and ice_dpll_get_pins() which
both have proper error-path rollback using goto labels.

> +
> +	if (IS_ERR_OR_NULL(ref_pin->pin))
> +		dev_dbg(ice_pf_to_dev(pf),
> +			"Tx-clk mux pin not registered yet\n");

Can this cause a NULL pointer dereference?

The IS_ERR_OR_NULL check appears after calling dpll_pin_register with
ref_pin->pin. Looking at the code flow:

ice_dpll_init_fwnode_pin() returns 0 (success) even when
fwnode_dpll_pin_find() returns NULL and sets ref_pin->pin = NULL. This
is the expected case - the commit message states "The SYNCE pin is
registered when its fwnode becomes available" and the code prints "DPLL
pin for %pfwp not registered yet".

However, the code unconditionally calls:

    dpll_pin_register(txc->dpll, ref_pin->pin, ...)

where ref_pin->pin is NULL. Looking at dpll_pin_register in
dpll_core.c:

    int dpll_pin_register(struct dpll_device *dpll, struct dpll_pin *pin, ...)
    {
        ...
        ret = dpll_pin_fwnode_register(dpll, pin, &ice_dpll_txclk_ops, pin_priv);
        ...
    }

which then calls code that dereferences pin->fwnode without a NULL
check, causing a kernel oops.

The IS_ERR_OR_NULL guard at line 3716 was clearly intended to protect
the dpll_pin_register call but was placed after it instead of before it.

> +
> +	return 0;
> +}

[ ... ]

> -----Original Message-----
> From: Simon Horman <horms@kernel.org>
> Sent: Thursday, March 19, 2026 6:38 PM
> To: Nitka, Grzegorz <grzegorz.nitka@intel.com>
> Cc: Simon Horman <horms@kernel.org>; netdev@vger.kernel.org;
> andrew+netdev@lunn.ch; pabeni@redhat.com;
> Prathosh.Satish@microchip.com; jiri@resnulli.us; kuba@kernel.org;
> Loktionov, Aleksandr <aleksandr.loktionov@intel.com>;
> richardcochran@gmail.com; vadim.fedorenko@linux.dev; Nguyen, Anthony
> L <anthony.l.nguyen@intel.com>; Vecera, Ivan <ivecera@redhat.com>;
> donald.hunter@gmail.com; intel-wired-lan@lists.osuosl.org;
> davem@davemloft.net; Kubalewski, Arkadiusz
> <arkadiusz.kubalewski@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Oros, Petr <poros@redhat.com>; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH net-next 5/8] ice: add TX clock (TXC) DPLL interface for
> E825 devices
> 
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> 
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-
> local.html
> ---
> ice: add TX clock (TXC) DPLL interface for E825 devices
> 
> This commit introduces a TX clock DPLL instance and pins for E825-based
> devices. The TXC domain exposes two reference pins: EXT_EREF0 (board
> external electrical reference) and SYNCE (port-derived reference,
> described via fwnode). A new pin type and pin ops are added to support
> TX clock references.
> 
> > diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c
> b/drivers/net/ethernet/intel/ice/ice_dpll.c
> > index 62f75701d652..a1258f2e03a9 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_dpll.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
> 
> [ ... ]
> 
> > @@ -3552,6 +3686,39 @@ ice_dpll_init_fwnode_pins(struct ice_pf *pf,
> struct ice_dpll_pin *pins,
> >  	return ret;
> >  }
> >
> > +static int ice_dpll_init_txclk_pins(struct ice_pf *pf, int start_idx)
> > +{
> > +	struct ice_dpll_pin *ref_pin = pf->dplls.txclks;
> > +	struct ice_dpll *txc = &pf->dplls.txc;
> > +	int ret = 0;
> > +
> > +	/* configure EXT_EREF0 pin */
> > +	ret = ice_dpll_get_pins(pf, ref_pin, start_idx, 1, pf->dplls.clock_id);
> > +	if (ret)
> > +		return ret;
> > +	ret = dpll_pin_register(txc->dpll, ref_pin->pin, &ice_dpll_txclk_ops,
> > +				ref_pin);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* configure EXT_SYNCE pin which is based on fwnode pin */
> > +	ref_pin++;
> > +	ret = ice_dpll_init_fwnode_pin(ref_pin,
> ice_dpll_fwnode_ext_synce);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = dpll_pin_register(txc->dpll, ref_pin->pin, &ice_dpll_txclk_ops,
> > +				ref_pin);
> > +	if (ret)
> > +		return ret;
> 
> Does ice_dpll_init_txclk_pins leak the EXT_EREF0 pin on error?
> 
> If dpll_pin_register for the EXT_SYNCE pin fails, the function returns
> the error without cleaning up the EXT_EREF0 pin that was already
> successfully registered. Looking at the call chain:
> 
> ice_dpll_init_pins_e825() calls:
>     ice_dpll_init_txclk_pins()
>         ice_dpll_get_pins() -> acquires pin reference for EXT_EREF0
>         dpll_pin_register() -> registers EXT_EREF0 with TXC DPLL
>         ice_dpll_init_fwnode_pin() -> may fail here
>         dpll_pin_register() -> or may fail here for EXT_SYNCE
> 
> If either of the last two steps fail, the function returns the error
> directly without calling dpll_pin_unregister or ice_dpll_release_pins
> for the EXT_EREF0 pin.
> 
> The caller ice_dpll_init_pins_e825() also doesn't compensate - on
> txclk init failure it only calls ice_dpll_deinit_rclk_pin() and
> ice_dpll_deinit_fwnode_pins(), neither of which clean up txclk
> resources.
> 
> Compare with ice_dpll_init_fwnode_pins() and ice_dpll_get_pins() which
> both have proper error-path rollback using goto labels.
> 
> > +
> > +	if (IS_ERR_OR_NULL(ref_pin->pin))
> > +		dev_dbg(ice_pf_to_dev(pf),
> > +			"Tx-clk mux pin not registered yet\n");
> 
> Can this cause a NULL pointer dereference?
> 
> The IS_ERR_OR_NULL check appears after calling dpll_pin_register with
> ref_pin->pin. Looking at the code flow:
> 
> ice_dpll_init_fwnode_pin() returns 0 (success) even when
> fwnode_dpll_pin_find() returns NULL and sets ref_pin->pin = NULL. This
> is the expected case - the commit message states "The SYNCE pin is
> registered when its fwnode becomes available" and the code prints "DPLL
> pin for %pfwp not registered yet".
> 
> However, the code unconditionally calls:
> 
>     dpll_pin_register(txc->dpll, ref_pin->pin, ...)
> 
> where ref_pin->pin is NULL. Looking at dpll_pin_register in
> dpll_core.c:
> 
>     int dpll_pin_register(struct dpll_device *dpll, struct dpll_pin *pin, ...)
>     {
>         ...
>         ret = dpll_pin_fwnode_register(dpll, pin, &ice_dpll_txclk_ops, pin_priv);
>         ...
>     }
> 
> which then calls code that dereferences pin->fwnode without a NULL
> check, causing a kernel oops.
> 
> The IS_ERR_OR_NULL guard at line 3716 was clearly intended to protect
> the dpll_pin_register call but was placed after it instead of before it.
> 
> > +
> > +	return 0;
> > +}
> 

AI review comment is valid. To be fixed in the next iteration.
Thanks

Grzegorz
> [ ... ]