Re: [PATCH net-next 01/10] ice: dpll: notify TSPLL lock status from the periodic work only

[Petr Oros <poros@redhat.com>]

On 11/6/26 1:09 PM, Ivan Vecera wrote:
> ice_dpll_tspll_state_on_dpll_set() notifies the TSPLL device from a
> callback of the TIME_REF pin. This pin belongs to an external DPLL and the
> DPLL core is going to run its callbacks under a lock of the pin's owner,
> which does not cover the TSPLL device.
> 
> Only update dpll_state in the callback, so a device get reports the new
> state right away, and leave prev_dpll_state to
> ice_dpll_periodic_work_e825(), which then sends the notification. It now
> arrives up to 500 ms later and after the reply to the request.
> 
> Refresh dpll_state on every run of the work, as the state written by the
> callback can be transient and the lock status can return to
> prev_dpll_state before the next run. No notification is sent in this case.
> 
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
>   drivers/net/ethernet/intel/ice/ice_dpll.c | 14 +++++---------
>   1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c
> index ade1fb3dd06743..bdbcf7af296eb4 100644
> --- a/drivers/net/ethernet/intel/ice/ice_dpll.c
> +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
> @@ -2914,7 +2914,6 @@ ice_dpll_tspll_state_on_dpll_set(const struct dpll_pin *pin, void *pin_priv,
>   	struct ice_dpll_pin *p = pin_priv;
>   	struct ice_pf *pf = p->pf;
>   	enum ice_clk_src clk_src;
> -	bool changed = false;
>   	struct ice_dpll *tp;
>   	int ret;
>   
> @@ -2947,15 +2946,12 @@ ice_dpll_tspll_state_on_dpll_set(const struct dpll_pin *pin, void *pin_priv,
>   		WRITE_ONCE(pf->ptp.tspll_locked,
>   			   new_state == DPLL_LOCK_STATUS_LOCKED);
>   
> -		if (tp->prev_dpll_state != new_state) {
> -			tp->dpll_state = new_state;
> -			tp->prev_dpll_state = new_state;
> -			changed = true;
> -		}
> +		/* ice_dpll_periodic_work_e825() compares the state with
> +		 * prev_dpll_state and sends the notification.
> +		 */
> +		tp->dpll_state = new_state;
>   	}
>   	mutex_unlock(&pf->dplls.lock);
> -	if (changed)
> -		__dpll_device_change_ntf(tp->dpll);
>   	/* TSPLL reconfiguration may complete before lock is reacquired.
>   	 * PTP periodic monitoring tracks and restores the final lock state.
>   	 */
> @@ -3372,8 +3368,8 @@ static void ice_dpll_periodic_work_e825(struct ice_pf *pf)
>   	mutex_lock(&pf->dplls.lock);
>   	new_state = ice_dpll_tspll_lock_status_get(pf, true);
>   
> +	tp->dpll_state = new_state;
>   	if (tp->prev_dpll_state != new_state) {
> -		tp->dpll_state = new_state;
>   		tp->prev_dpll_state = new_state;
>   		changed = true;
>   	}

Reviewed-by: Petr Oros <poros@redhat.com>