RE: [PATCH iwl-net v7 2/3] ice: fix missing dpll notifications for SW pins

[Kubalewski, Arkadiusz]

>From: Petr Oros <poros@xxxxxxxxxx>
>Sent: Friday, April 17, 2026 4:59 PM
>
>The SMA/U.FL pin redesign (commit 2dd5d03c77e2 ("ice: redesign dpll
>sma/u.fl pins control")) introduced software-controlled pins that wrap
>backing CGU input/output pins, but never updated the notification and
>data paths to propagate pin events to these SW wrappers.
>
>The periodic work sends dpll_pin_change_ntf() only for direct CGU input
>pins.  SW pins that wrap these inputs never receive change or phase
>offset notifications, so userspace consumers such as synce4l monitoring
>SMA pins via dpll netlink never learn about state transitions or phase
>offset updates.  Similarly, ice_dpll_phase_offset_get() reads the SW
>pin's own phase_offset field which is never updated; the PPS monitor
>writes to the backing CGU input's field instead.
>
>Fix by introducing ice_dpll_pin_ntf(), a wrapper around
>dpll_pin_change_ntf() that also notifies any registered SMA/U.FL pin
>whose backing CGU input matches.  Replace all direct
>dpll_pin_change_ntf() calls in the periodic notification paths with
>this wrapper.  Fix ice_dpll_phase_offset_get() to return the backing
>CGU input's phase_offset for input-direction SW pins.
>
>Fixes: 2dd5d03c77e2 ("ice: redesign dpll sma/u.fl pins control")

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

>Signed-off-by: Petr Oros <poros@xxxxxxxxxx>
>---
> drivers/net/ethernet/intel/ice/ice_dpll.c | 47 +++++++++++++++++------
> 1 file changed, 36 insertions(+), 11 deletions(-)
>
>diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c
>b/drivers/net/ethernet/intel/ice/ice_dpll.c
>index 3a90a2940fdc6e..11b942b83500fb 100644
>--- a/drivers/net/ethernet/intel/ice/ice_dpll.c
>+++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
>@@ -1963,7 +1963,10 @@ ice_dpll_phase_offset_get(const struct dpll_pin
>*pin, void *pin_priv,
> 				       d->active_input == p->input->pin))
> 		*phase_offset = d->phase_offset *
>ICE_DPLL_PHASE_OFFSET_FACTOR;
> 	else if (d->phase_offset_monitor_period)
>-		*phase_offset = p->phase_offset *
>ICE_DPLL_PHASE_OFFSET_FACTOR;
>+		*phase_offset = (p->input &&
>+				 p->direction == DPLL_PIN_DIRECTION_INPUT ?
>+				 p->input->phase_offset :
>+				 p->phase_offset) * ICE_DPLL_PHASE_OFFSET_FACTOR;
> 	else
> 		*phase_offset = 0;
> 	mutex_unlock(&pf->dplls.lock);
>@@ -2659,6 +2662,27 @@ static u64 ice_generate_clock_id(struct ice_pf *pf)
> 	return pci_get_dsn(pf->pdev);
> }
>
>+/**
>+ * ice_dpll_pin_ntf - notify pin change including any SW pin wrappers
>+ * @dplls: pointer to dplls struct
>+ * @pin: the dpll_pin that changed
>+ *
>+ * Send a change notification for @pin and for any registered SMA/U.FL
>pin
>+ * whose backing CGU input matches @pin.
>+ */
>+static void ice_dpll_pin_ntf(struct ice_dplls *dplls, struct dpll_pin
>*pin)
>+{
>+	dpll_pin_change_ntf(pin);
>+	for (int i = 0; i < ICE_DPLL_PIN_SW_NUM; i++) {
>+		if (dplls->sma[i].pin && dplls->sma[i].input &&
>+		    dplls->sma[i].input->pin == pin)
>+			dpll_pin_change_ntf(dplls->sma[i].pin);
>+		if (dplls->ufl[i].pin && dplls->ufl[i].input &&
>+		    dplls->ufl[i].input->pin == pin)
>+			dpll_pin_change_ntf(dplls->ufl[i].pin);
>+	}
>+}
>+
> /**
>  * ice_dpll_notify_changes - notify dpll subsystem about changes
>  * @d: pointer do dpll
>@@ -2667,6 +2691,7 @@ static u64 ice_generate_clock_id(struct ice_pf *pf)
>  */
> static void ice_dpll_notify_changes(struct ice_dpll *d)
> {
>+	struct ice_dplls *dplls = &d->pf->dplls;
> 	bool pin_notified = false;
>
> 	if (d->prev_dpll_state != d->dpll_state) {
>@@ -2675,17 +2700,17 @@ static void ice_dpll_notify_changes(struct
>ice_dpll *d)
> 	}
> 	if (d->prev_input != d->active_input) {
> 		if (d->prev_input)
>-			dpll_pin_change_ntf(d->prev_input);
>+			ice_dpll_pin_ntf(dplls, d->prev_input);
> 		d->prev_input = d->active_input;
> 		if (d->active_input) {
>-			dpll_pin_change_ntf(d->active_input);
>+			ice_dpll_pin_ntf(dplls, d->active_input);
> 			pin_notified = true;
> 		}
> 	}
> 	if (d->prev_phase_offset != d->phase_offset) {
> 		d->prev_phase_offset = d->phase_offset;
> 		if (!pin_notified && d->active_input)
>-			dpll_pin_change_ntf(d->active_input);
>+			ice_dpll_pin_ntf(dplls, d->active_input);
> 	}
> }
>
>@@ -2714,6 +2739,7 @@ static bool ice_dpll_is_pps_phase_monitor(struct
>ice_pf *pf)
>
> /**
>  * ice_dpll_pins_notify_mask - notify dpll subsystem about bulk pin
>changes
>+ * @dplls: pointer to dplls struct
>  * @pins: array of ice_dpll_pin pointers registered within dpll subsystem
>  * @pin_num: number of pins
>  * @phase_offset_ntf_mask: bitmask of pin indexes to notify
>@@ -2723,15 +2749,14 @@ static bool ice_dpll_is_pps_phase_monitor(struct
>ice_pf *pf)
>  *
>  * Context: Must be called while pf->dplls.lock is released.
>  */
>-static void ice_dpll_pins_notify_mask(struct ice_dpll_pin *pins,
>+static void ice_dpll_pins_notify_mask(struct ice_dplls *dplls,
>+				      struct ice_dpll_pin *pins,
> 				      u8 pin_num,
> 				      u32 phase_offset_ntf_mask)
> {
>-	int i = 0;
>-
>-	for (i = 0; i < pin_num; i++)
>-		if (phase_offset_ntf_mask & (1 << i))
>-			dpll_pin_change_ntf(pins[i].pin);
>+	for (int i = 0; i < pin_num; i++)
>+		if (phase_offset_ntf_mask & BIT(i))
>+			ice_dpll_pin_ntf(dplls, pins[i].pin);
> }
>
> /**
>@@ -2907,7 +2932,7 @@ static void ice_dpll_periodic_work(struct
>kthread_work *work)
> 	ice_dpll_notify_changes(de);
> 	ice_dpll_notify_changes(dp);
> 	if (phase_offset_ntf)
>-		ice_dpll_pins_notify_mask(d->inputs, d->num_inputs,
>+		ice_dpll_pins_notify_mask(d, d->inputs, d->num_inputs,
> 					  phase_offset_ntf);
>
> resched:
>--
>2.52.0