Re: [PATCH v3 net-next 06/15] ptp: netc: add periodic pulse output support

[Vladimir Oltean]

On Tue, Aug 12, 2025 at 05:46:25PM +0800, Wei Fang wrote:
> @@ -210,77 +343,178 @@ static void netc_timer_set_pps_alarm(struct netc_timer *priv, int channel,
>  static int netc_timer_enable_pps(struct netc_timer *priv,
>  				 struct ptp_clock_request *rq, int on)
>  {
> -	u32 fiper, fiper_ctrl;
> +	struct device *dev = &priv->pdev->dev;
>  	unsigned long flags;
> +	struct netc_pp *pp;
> +	int err = 0;
>  
>  	spin_lock_irqsave(&priv->lock, flags);
>  
> -	fiper_ctrl = netc_timer_rd(priv, NETC_TMR_FIPER_CTRL);
> -
>  	if (on) {
		...
>  	} else {
> -		if (!priv->pps_enabled)
> +		/* pps_channel is invalid if PPS is not enabled, so no
> +		 * processing is needed.
> +		 */
> +		if (priv->pps_channel >= NETC_TMR_FIPER_NUM)
>  			goto unlock_spinlock;
>  
> -		fiper = NETC_TMR_DEFAULT_FIPER;
> -		priv->tmr_emask &= ~(TMR_TEVNET_PPEN(0) |
> -				     TMR_TEVENT_ALMEN(0));
> -		fiper_ctrl |= FIPER_CTRL_DIS(0);
> -		priv->pps_enabled = false;
> -		netc_timer_alarm_write(priv, NETC_TMR_DEFAULT_ALARM, 0);
> +		netc_timer_disable_periodic_pulse(priv, priv->pps_channel);
> +		priv->fs_alarm_bitmap &= ~BIT(pp->alarm_id);

You dereference "pp"->alarm_id before assigning "pp" one line below.

> +		pp = &priv->pp[priv->pps_channel];
> +		memset(pp, 0, sizeof(*pp));
> +		priv->pps_channel = NETC_TMR_INVALID_CHANNEL;
>  	}
>  
> -	netc_timer_wr(priv, NETC_TMR_TEMASK, priv->tmr_emask);
> -	netc_timer_wr(priv, NETC_TMR_FIPER(0), fiper);
> -	netc_timer_wr(priv, NETC_TMR_FIPER_CTRL, fiper_ctrl);
> +unlock_spinlock:
> +	spin_unlock_irqrestore(&priv->lock, flags);
> +
> +	return err;
> +}