[PATCH iwl-net v5] ice: fix missing dpll notifications for SW pins

[PATCH iwl-net v5] ice: fix missing dpll notifications for SW pins

[Petr Oros]

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.

On top of that, when SMA or U.FL pin state changes via PCA9575 GPIO
write, the paired pin's state also changes because they share physical
signal paths, but no notification is sent for the peer pin.

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.  Add
ice_dpll_sw_pin_notify_peer() to send a notification for the paired
SW pin after PCA9575 writes in ice_dpll_sma_direction_set() and
ice_dpll_ufl_pin_state_set().

Fixes: 2dd5d03c77e2 ("ice: redesign dpll sma/u.fl pins control")
Signed-off-by: Petr Oros <poros@redhat.com>
---
v5:
 - add ice_dpll_sw_pin_notify_peer() for SMA/U.FL peer notification
   when PCA9575 routing changes affect the paired pin (reported by
   Intel test: SMA state change did not log U.FL status change in
   subscribe monitor)
v4: https://lore.kernel.org/all/20260319205256.998876-1-poros@redhat.com/
v3: https://lore.kernel.org/all/20260220140700.2910174-1-poros@redhat.com/
v2: https://lore.kernel.org/all/20260219131500.2271897-1-poros@redhat.com/
v1: https://lore.kernel.org/all/20260218211414.1411163-1-poros@redhat.com/
---
 drivers/net/ethernet/intel/ice/ice_dpll.c | 74 +++++++++++++++++++----
 1 file changed, 63 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c
index 3f8cd5b8298b57..d817f17dcf1951 100644
--- a/drivers/net/ethernet/intel/ice/ice_dpll.c
+++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
@@ -1154,6 +1154,30 @@ ice_dpll_input_state_get(const struct dpll_pin *pin, void *pin_priv,
 				      extack, ICE_DPLL_PIN_TYPE_INPUT);
 }
 
+/**
+ * ice_dpll_sw_pin_notify_peer - notify the paired SW pin after a state change
+ * @d: pointer to dplls struct
+ * @changed: the SW pin that was explicitly changed (already notified by dpll core)
+ *
+ * SMA and U.FL pins share physical signal paths in pairs (SMA1/U.FL1 and
+ * SMA2/U.FL2).  When one pin's routing changes via the PCA9575 GPIO
+ * expander, the paired pin's state may also change.  Send a change
+ * notification for the peer pin so userspace consumers monitoring the
+ * peer via dpll netlink learn about the update.
+ *
+ * Context: Can be called under pf->dplls.lock, dpll_pin_change_ntf() is safe.
+ */
+static void ice_dpll_sw_pin_notify_peer(struct ice_dplls *d,
+					struct ice_dpll_pin *changed)
+{
+	struct ice_dpll_pin *peer;
+
+	peer = (changed >= d->sma && changed < d->sma + ICE_DPLL_PIN_SW_NUM) ?
+		&d->ufl[changed->idx] : &d->sma[changed->idx];
+	if (peer->pin)
+		dpll_pin_change_ntf(peer->pin);
+}
+
 /**
  * ice_dpll_sma_direction_set - set direction of SMA pin
  * @p: pointer to a pin
@@ -1233,6 +1257,8 @@ static int ice_dpll_sma_direction_set(struct ice_dpll_pin *p,
 			ret = ice_dpll_pin_state_update(p->pf, target,
 							type, extack);
 	}
+	if (!ret)
+		ice_dpll_sw_pin_notify_peer(d, p);
 
 	return ret;
 }
@@ -1334,6 +1360,7 @@ ice_dpll_ufl_pin_state_set(const struct dpll_pin *pin, void *pin_priv,
 					extack);
 	if (ret)
 		goto unlock;
+	ice_dpll_sw_pin_notify_peer(&pf->dplls, p);
 
 	if (enable)
 		ret = ice_dpll_pin_enable(hw, target, d->dpll_idx, type, extack);
@@ -1963,7 +1990,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);
@@ -2657,6 +2687,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
@@ -2665,6 +2716,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) {
@@ -2673,17 +2725,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);
 	}
 }
 
@@ -2712,6 +2764,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
@@ -2721,15 +2774,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);
 }
 
 /**
@@ -2905,7 +2957,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

Re: [PATCH iwl-net v5] ice: fix missing dpll notifications for SW pins

[Michal Schmidt]

On 4/9/26 12:25, Petr Oros wrote:
> ---
>   drivers/net/ethernet/intel/ice/ice_dpll.c | 74 +++++++++++++++++++----
>   1 file changed, 63 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c
> index 3f8cd5b8298b57..d817f17dcf1951 100644
> --- a/drivers/net/ethernet/intel/ice/ice_dpll.c
> +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
> @@ -1154,6 +1154,30 @@ ice_dpll_input_state_get(const struct dpll_pin *pin, void *pin_priv,
>   				      extack, ICE_DPLL_PIN_TYPE_INPUT);
>   }
>   
> +/**
> + * ice_dpll_sw_pin_notify_peer - notify the paired SW pin after a state change
> + * @d: pointer to dplls struct
> + * @changed: the SW pin that was explicitly changed (already notified by dpll core)
> + *
> + * SMA and U.FL pins share physical signal paths in pairs (SMA1/U.FL1 and
> + * SMA2/U.FL2).  When one pin's routing changes via the PCA9575 GPIO
> + * expander, the paired pin's state may also change.  Send a change
> + * notification for the peer pin so userspace consumers monitoring the
> + * peer via dpll netlink learn about the update.
> + *
> + * Context: Can be called under pf->dplls.lock, dpll_pin_change_ntf() is safe.
> + */
> +static void ice_dpll_sw_pin_notify_peer(struct ice_dplls *d,
> +					struct ice_dpll_pin *changed)
> +{
> +	struct ice_dpll_pin *peer;
> +
> +	peer = (changed >= d->sma && changed < d->sma + ICE_DPLL_PIN_SW_NUM) ?
> +		&d->ufl[changed->idx] : &d->sma[changed->idx];
> +	if (peer->pin)
> +		dpll_pin_change_ntf(peer->pin);
> +}
> +
>   /**
>    * ice_dpll_sma_direction_set - set direction of SMA pin
>    * @p: pointer to a pin
> @@ -1233,6 +1257,8 @@ static int ice_dpll_sma_direction_set(struct ice_dpll_pin *p,
>   			ret = ice_dpll_pin_state_update(p->pf, target,
>   							type, extack);
>   	}
> +	if (!ret)
> +		ice_dpll_sw_pin_notify_peer(d, p);
>   
>   	return ret;
>   }

ice_dpll_sma_direction_set() runs to process a DPLL_CMD_PIN_SET command 
from userspace. It runs with dpll_lock held - taken in dpll_pin_pre_doit().
ice_dpll_sw_pin_notify_peer() -> dpll_pin_change_ntf() will take 
dpll_lock again and deadlock.

Michal

Re: [Intel-wired-lan] [PATCH iwl-net v5] ice: fix missing dpll notifications for SW pins

[Jacob Keller]

On 4/14/2026 12:16 PM, Michal Schmidt wrote:
> On 4/9/26 12:25, Petr Oros wrote:
>> ---
>>   drivers/net/ethernet/intel/ice/ice_dpll.c | 74 +++++++++++++++++++----
>>   1 file changed, 63 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/
>> ethernet/intel/ice/ice_dpll.c
>> index 3f8cd5b8298b57..d817f17dcf1951 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_dpll.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
>> @@ -1154,6 +1154,30 @@ ice_dpll_input_state_get(const struct dpll_pin
>> *pin, void *pin_priv,
>>                         extack, ICE_DPLL_PIN_TYPE_INPUT);
>>   }
>>   +/**
>> + * ice_dpll_sw_pin_notify_peer - notify the paired SW pin after a
>> state change
>> + * @d: pointer to dplls struct
>> + * @changed: the SW pin that was explicitly changed (already notified
>> by dpll core)
>> + *
>> + * SMA and U.FL pins share physical signal paths in pairs (SMA1/U.FL1
>> and
>> + * SMA2/U.FL2).  When one pin's routing changes via the PCA9575 GPIO
>> + * expander, the paired pin's state may also change.  Send a change
>> + * notification for the peer pin so userspace consumers monitoring the
>> + * peer via dpll netlink learn about the update.
>> + *
>> + * Context: Can be called under pf->dplls.lock, dpll_pin_change_ntf()
>> is safe.
>> + */
>> +static void ice_dpll_sw_pin_notify_peer(struct ice_dplls *d,
>> +                    struct ice_dpll_pin *changed)
>> +{
>> +    struct ice_dpll_pin *peer;
>> +
>> +    peer = (changed >= d->sma && changed < d->sma +
>> ICE_DPLL_PIN_SW_NUM) ?
>> +        &d->ufl[changed->idx] : &d->sma[changed->idx];
>> +    if (peer->pin)
>> +        dpll_pin_change_ntf(peer->pin);
>> +}
>> +
>>   /**
>>    * ice_dpll_sma_direction_set - set direction of SMA pin
>>    * @p: pointer to a pin
>> @@ -1233,6 +1257,8 @@ static int ice_dpll_sma_direction_set(struct
>> ice_dpll_pin *p,
>>               ret = ice_dpll_pin_state_update(p->pf, target,
>>                               type, extack);
>>       }
>> +    if (!ret)
>> +        ice_dpll_sw_pin_notify_peer(d, p);
>>         return ret;
>>   }
> 
> ice_dpll_sma_direction_set() runs to process a DPLL_CMD_PIN_SET command
> from userspace. It runs with dpll_lock held - taken in dpll_pin_pre_doit().
> ice_dpll_sw_pin_notify_peer() -> dpll_pin_change_ntf() will take
> dpll_lock again and deadlock.
> 

Yep. I think you could use __dpll_pin_change_ntf() which is the version
that assumes the lock is held.. but that function is not exported
outside of drivers/dpll.

Either way, this needs to be fixed somehow before I can apply it.

Thanks,
Jake

> Michal
> 

Re: [Intel-wired-lan] [PATCH iwl-net v5] ice: fix missing dpll notifications for SW pins

[Ivan Vecera]

On 4/14/26 10:46 PM, Jacob Keller wrote:
> On 4/14/2026 12:16 PM, Michal Schmidt wrote:
>> On 4/9/26 12:25, Petr Oros wrote:
>>> ---
>>>    drivers/net/ethernet/intel/ice/ice_dpll.c | 74 +++++++++++++++++++----
>>>    1 file changed, 63 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/
>>> ethernet/intel/ice/ice_dpll.c
>>> index 3f8cd5b8298b57..d817f17dcf1951 100644
>>> --- a/drivers/net/ethernet/intel/ice/ice_dpll.c
>>> +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
>>> @@ -1154,6 +1154,30 @@ ice_dpll_input_state_get(const struct dpll_pin
>>> *pin, void *pin_priv,
>>>                          extack, ICE_DPLL_PIN_TYPE_INPUT);
>>>    }
>>>    +/**
>>> + * ice_dpll_sw_pin_notify_peer - notify the paired SW pin after a
>>> state change
>>> + * @d: pointer to dplls struct
>>> + * @changed: the SW pin that was explicitly changed (already notified
>>> by dpll core)
>>> + *
>>> + * SMA and U.FL pins share physical signal paths in pairs (SMA1/U.FL1
>>> and
>>> + * SMA2/U.FL2).  When one pin's routing changes via the PCA9575 GPIO
>>> + * expander, the paired pin's state may also change.  Send a change
>>> + * notification for the peer pin so userspace consumers monitoring the
>>> + * peer via dpll netlink learn about the update.
>>> + *
>>> + * Context: Can be called under pf->dplls.lock, dpll_pin_change_ntf()
>>> is safe.
>>> + */
>>> +static void ice_dpll_sw_pin_notify_peer(struct ice_dplls *d,
>>> +                    struct ice_dpll_pin *changed)
>>> +{
>>> +    struct ice_dpll_pin *peer;
>>> +
>>> +    peer = (changed >= d->sma && changed < d->sma +
>>> ICE_DPLL_PIN_SW_NUM) ?
>>> +        &d->ufl[changed->idx] : &d->sma[changed->idx];
>>> +    if (peer->pin)
>>> +        dpll_pin_change_ntf(peer->pin);
>>> +}
>>> +
>>>    /**
>>>     * ice_dpll_sma_direction_set - set direction of SMA pin
>>>     * @p: pointer to a pin
>>> @@ -1233,6 +1257,8 @@ static int ice_dpll_sma_direction_set(struct
>>> ice_dpll_pin *p,
>>>                ret = ice_dpll_pin_state_update(p->pf, target,
>>>                                type, extack);
>>>        }
>>> +    if (!ret)
>>> +        ice_dpll_sw_pin_notify_peer(d, p);
>>>          return ret;
>>>    }
>>
>> ice_dpll_sma_direction_set() runs to process a DPLL_CMD_PIN_SET command
>> from userspace. It runs with dpll_lock held - taken in dpll_pin_pre_doit().
>> ice_dpll_sw_pin_notify_peer() -> dpll_pin_change_ntf() will take
>> dpll_lock again and deadlock.
>>
> 
> Yep. I think you could use __dpll_pin_change_ntf() which is the version
> that assumes the lock is held.. but that function is not exported
> outside of drivers/dpll.
> 
> Either way, this needs to be fixed somehow before I can apply it.
> 
> Thanks,
> Jake

I'm solving the similar situation where some setting on some output pin
can change also sibling pin.

E.g. changing frequency on OUTxP also changes frequency on OUTxN in
certain situations (depending on signal format of the output)...

In such cases would be useful to inform about such change on sibling
pin.

Thanks,
Ivan