Re: [PATCH v3 net-next 1/8] ptp: netc: use ioread64_lo_hi/iowrite64_lo_hi for 64-bit register access

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Data loss of external hardware timestamps due to discarding all but the last timestamp from the FIFO
--

commit 7676c74333a13c4f33beb4f741615e25a7faf299
Author: Wei Fang <wei.fang@nxp.com>

ptp: netc: use ioread64_lo_hi/iowrite64_lo_hi for 64-bit register access

This commit replaces open-coded 64-bit register read and write sequences
with ioread64_lo_hi() and iowrite64_lo_hi() helpers. It introduces new
macros to wrap these helpers to reduce boilerplate and enforce strict
L-before-H access ordering.

> diff --git a/drivers/ptp/ptp_netc.c b/drivers/ptp/ptp_netc.c
> index 19ca99d80e95b..376b3b0d1a38f 100644
> --- a/drivers/ptp/ptp_netc.c
> +++ b/drivers/ptp/ptp_netc.c

[ ... ]

> @@ -500,22 +473,19 @@ static void netc_timer_handle_etts_event(struct netc_timer *priv, int index,
>  					 bool update_event)
>  {
>  	struct ptp_clock_event event;
> -	u32 etts_l = 0, etts_h = 0;
> +	u64 etts = 0;
>  
> -	while (netc_timer_rd(priv, NETC_TMR_STAT) & TMR_STAT_ETS_VLD(index)) {
> -		etts_l = netc_timer_rd(priv, NETC_TMR_ETTS_L(index));
> -		etts_h = netc_timer_rd(priv, NETC_TMR_ETTS_H(index));
> -	}
> +	while (netc_timer_rd(priv, NETC_TMR_STAT) & TMR_STAT_ETS_VLD(index))
> +		etts = netc_timer_rd64(priv, NETC_TMR_ETTS_L(index));
>  
>  	/* Invalid time stamp */
> -	if (!etts_l && !etts_h)
> +	if (!etts)
>  		return;
>  
>  	if (update_event) {
>  		event.type = PTP_CLOCK_EXTTS;
>  		event.index = index;
> -		event.timestamp = (u64)etts_h << 32;
> -		event.timestamp |= etts_l;
> +		event.timestamp = etts;
>  		ptp_clock_event(priv->clock, &event);
>  	}
>  }

[Severity: High]
This is a pre-existing issue, but does this loop discard all but the last
timestamp from the hardware FIFO?

The while loop successfully drains multiple timestamps from the TMR_ETTS FIFO
if they accumulate before the interrupt is handled. However, because etts is
overwritten on each iteration and ptp_clock_event() is only called once
after the loop completes, it appears any prior external events in the batch
are silently lost.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831082335.1184605-1-wei.fang@oss.nxp.com?part=1