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