Re: [PATCH net-next v3 3/4] net: wangxun: Implement do_aux_work of ptp_clock_info

[Vadim Fedorenko <vadim.fedorenko@linux.dev>]

On 10/01/2025 03:17, Jiawen Wu wrote:
> Implement watchdog task to detect SYSTIME overflow and error cases of
> Rx/Tx timestamp.
> 
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> ---
>   drivers/net/ethernet/wangxun/libwx/wx_ptp.c   | 113 ++++++++++++++++++
>   drivers/net/ethernet/wangxun/libwx/wx_type.h  |   2 +
>   drivers/net/ethernet/wangxun/ngbe/ngbe_mdio.c |   1 +
>   .../net/ethernet/wangxun/txgbe/txgbe_phy.c    |   1 +
>   4 files changed, 117 insertions(+)
> 
> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_ptp.c b/drivers/net/ethernet/wangxun/libwx/wx_ptp.c
> index d97be12ef37c..e7dc9196ba54 100644
> --- a/drivers/net/ethernet/wangxun/libwx/wx_ptp.c
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_ptp.c
> @@ -251,6 +251,116 @@ static void wx_ptp_tx_hwtstamp_work(struct work_struct *work)
>   	}
>   }
>   
> +/**
> + * wx_ptp_overflow_check - watchdog task to detect SYSTIME overflow
> + * @wx: pointer to wx struct
> + *
> + * this watchdog task periodically reads the timecounter
> + * in order to prevent missing when the system time registers wrap
> + * around. This needs to be run approximately twice a minute for the fastest
> + * overflowing hardware. We run it for all hardware since it shouldn't have a
> + * large impact.
> + */
> +static void wx_ptp_overflow_check(struct wx *wx)
> +{
> +	bool timeout = time_is_before_jiffies(wx->last_overflow_check +
> +					      WX_OVERFLOW_PERIOD);
> +	unsigned long flags;
> +
> +	if (timeout) {
> +		/* Update the timecounter */
> +		spin_lock_irqsave(&wx->tmreg_lock, flags);
> +		timecounter_read(&wx->hw_tc);
> +		spin_unlock_irqrestore(&wx->tmreg_lock, flags);
> +
> +		wx->last_overflow_check = jiffies;
> +	}
> +}
> +
> +/**
> + * wx_ptp_rx_hang - detect error case when Rx timestamp registers latched
> + * @wx: pointer to wx struct
> + *
> + * this watchdog task is scheduled to detect error case where hardware has
> + * dropped an Rx packet that was timestamped when the ring is full. The
> + * particular error is rare but leaves the device in a state unable to
> + * timestamp any future packets.
> + */
> +static void wx_ptp_rx_hang(struct wx *wx)
> +{
> +	struct wx_ring *rx_ring;
> +	unsigned long rx_event;
> +	u32 tsyncrxctl;
> +	int n;
> +
> +	tsyncrxctl = rd32(wx, WX_PSR_1588_CTL);
> +
> +	/* if we don't have a valid timestamp in the registers, just update the
> +	 * timeout counter and exit
> +	 */
> +	if (!(tsyncrxctl & WX_PSR_1588_CTL_VALID)) {
> +		wx->last_rx_ptp_check = jiffies;
> +		return;
> +	}
> +
> +	/* determine the most recent watchdog or rx_timestamp event */
> +	rx_event = wx->last_rx_ptp_check;
> +	for (n = 0; n < wx->num_rx_queues; n++) {
> +		rx_ring = wx->rx_ring[n];
> +		if (time_after(rx_ring->last_rx_timestamp, rx_event))
> +			rx_event = rx_ring->last_rx_timestamp;
> +	}
> +
> +	/* only need to read the high RXSTMP register to clear the lock */
> +	if (time_is_before_jiffies(rx_event + 5 * HZ)) {
> +		rd32(wx, WX_PSR_1588_STMPH);
> +		wx->last_rx_ptp_check = jiffies;
> +
> +		wx->rx_hwtstamp_cleared++;
> +		dev_warn(&wx->pdev->dev, "clearing RX Timestamp hang");
> +	}
> +}
> +
> +/**
> + * wx_ptp_tx_hang - detect error case where Tx timestamp never finishes
> + * @wx: private network wx structure
> + */
> +static void wx_ptp_tx_hang(struct wx *wx)
> +{
> +	bool timeout = time_is_before_jiffies(wx->ptp_tx_start +
> +					      WX_PTP_TX_TIMEOUT);
> +
> +	if (!wx->ptp_tx_skb)
> +		return;
> +
> +	if (!test_bit(WX_STATE_PTP_TX_IN_PROGRESS, wx->state))
> +		return;
> +
> +	/* If we haven't received a timestamp within the timeout, it is
> +	 * reasonable to assume that it will never occur, so we can unlock the
> +	 * timestamp bit when this occurs.
> +	 */
> +	if (timeout) {
> +		cancel_work_sync(&wx->ptp_tx_work);
> +		wx_ptp_clear_tx_timestamp(wx);
> +		wx->tx_hwtstamp_timeouts++;
> +		dev_warn(&wx->pdev->dev, "clearing Tx timestamp hang\n");
> +	}
> +}
> +
> +static long wx_ptp_do_aux_work(struct ptp_clock_info *ptp)
> +{
> +	struct wx *wx = container_of(ptp, struct wx, ptp_caps);
> +
> +	wx_ptp_overflow_check(wx);
> +	if (unlikely(test_bit(WX_FLAG_RX_HWTSTAMP_IN_REGISTER,
> +			      wx->flags)))
> +		wx_ptp_rx_hang(wx);
> +	wx_ptp_tx_hang(wx);
> +
> +	return HZ;
> +}
> +
>   /**
>    * wx_ptp_create_clock
>    * @wx: the private board structure
> @@ -283,6 +393,7 @@ static long wx_ptp_create_clock(struct wx *wx)
>   	wx->ptp_caps.adjtime = wx_ptp_adjtime;
>   	wx->ptp_caps.gettimex64 = wx_ptp_gettimex64;
>   	wx->ptp_caps.settime64 = wx_ptp_settime64;
> +	wx->ptp_caps.do_aux_work = wx_ptp_do_aux_work;
>   	if (wx->mac.type == wx_mac_em)
>   		wx->ptp_caps.max_adj = 500000000;
>   	else
> @@ -568,6 +679,8 @@ void wx_ptp_reset(struct wx *wx)
>   	timecounter_init(&wx->hw_tc, &wx->hw_cc,
>   			 ktime_to_ns(ktime_get_real()));
>   	spin_unlock_irqrestore(&wx->tmreg_lock, flags);
> +
> +	wx->last_overflow_check = jiffies;
>   }
>   EXPORT_SYMBOL(wx_ptp_reset);

I think ptp_schedule_worker(wx->ptp_clock, HZ); is missing in either
wx_ptp_create_clock() or wx_ptp_start(). Otherwise there is nothing to
kick in the first iteration of overflow check.

> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_type.h b/drivers/net/ethernet/wangxun/libwx/wx_type.h
> index e70b397d1104..867d92547b61 100644
> --- a/drivers/net/ethernet/wangxun/libwx/wx_type.h
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_type.h
> @@ -1174,6 +1174,8 @@ struct wx {
>   	u32 tx_hwtstamp_timeouts;
>   	u32 tx_hwtstamp_skipped;
>   	u32 rx_hwtstamp_cleared;
> +	unsigned long last_overflow_check;
> +	unsigned long last_rx_ptp_check;
>   	unsigned long ptp_tx_start;
>   	spinlock_t tmreg_lock; /* spinlock for ptp */
>   	struct cyclecounter hw_cc;
> diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_mdio.c b/drivers/net/ethernet/wangxun/ngbe/ngbe_mdio.c
> index c7944e62838a..ea1d7e9a91f3 100644
> --- a/drivers/net/ethernet/wangxun/ngbe/ngbe_mdio.c
> +++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_mdio.c
> @@ -111,6 +111,7 @@ static void ngbe_mac_link_up(struct phylink_config *config,
>   	wr32(wx, WX_MAC_WDG_TIMEOUT, reg);
>   
>   	wx->speed = speed;
> +	wx->last_rx_ptp_check = jiffies;
>   	if (test_bit(WX_STATE_PTP_RUNNING, wx->state))
>   		wx_ptp_reset_cyclecounter(wx);
>   }
> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
> index 60e5f3288ad8..7e17d727c2ba 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
> @@ -222,6 +222,7 @@ static void txgbe_mac_link_up(struct phylink_config *config,
>   	wr32(wx, WX_MAC_WDG_TIMEOUT, wdg);
>   
>   	wx->speed = speed;
> +	wx->last_rx_ptp_check = jiffies;
>   	if (test_bit(WX_STATE_PTP_RUNNING, wx->state))
>   		wx_ptp_reset_cyclecounter(wx);
>   }