Re: [PATCH net-next v2] net: stmmac: ptp: switch to gettimex64() interface

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

On 08/10/2026 11:38, James Clark wrote:
> The stmmac PTP support currently implements the gettime64 callback to
> retrieve the hardware clock time. Update the implementation to provide
> the gettimex64 callback instead, adding support for the
> PTP_SYS_OFFSET_EXTENDED ioctl.
> 
> The system clock readings are taken around the read of the nanoseconds
> register in get_systime(), so get_systime() gains a ptp_system_timestamp
> argument.
> 
> Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
> Assisted-by: LLM
> Signed-off-by: James Clark <jjc@jclark.com>
> ---
> Changes in v2:
> - Read PTP_STNSR with readl() and drop the two rmb() calls
>    (Nicolai Buchwitz, Maxime Chevallier)
> - Fix the kernel-doc of stmmac_get_timex() and an over-long line
>    in stmmac_adjust_time() (netdev CI)
> - Link to v1: https://lore.kernel.org/netdev/20261006103610.98277-1-jjc@jclark.com/
> 
> Tested on a Radxa ZERO 3E (RK3566, DWMAC 4/5) running net-next.
> Width of the interval between the two system clock readings
> bracketing each PHC read (2000 calls of 25 samples each):
> 
>                                           min      median
> Before patch (PTP_SYS_OFFSET):           875 ns   1167 ns
> After patch (PTP_SYS_OFFSET_EXTENDED):   291 ns    583 ns
> 
> On this board the 24 MHz arch timer counter advances in steps of 7
> (~292 ns), so all intervals are multiples of that.
> 
>   drivers/net/ethernet/stmicro/stmmac/hwif.h    |  5 ++++-
>   .../ethernet/stmicro/stmmac/stmmac_hwtstamp.c |  7 +++++--
>   .../net/ethernet/stmicro/stmmac/stmmac_ptp.c  | 21 ++++++++++---------
>   .../stmicro/stmmac/stmmac_selftests.c         |  4 ++--
>   .../net/ethernet/stmicro/stmmac/stmmac_tc.c   |  3 ++-
>   5 files changed, 24 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> index a8a5c8fdd5e..6efbfe50102 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> @@ -493,6 +493,8 @@ struct stmmac_ops {
>   #define stmmac_fpe_map_preemption_class(__priv, __args...) \
>   	stmmac_do_callback(__priv, mac, fpe_map_preemption_class, __args)
>   
> +struct ptp_system_timestamp;
> +
>   /* PTP and HW Timer helpers */
>   struct stmmac_hwtimestamp {
>   	void (*config_hw_tstamping) (void __iomem *ioaddr, u32 data);
> @@ -502,7 +504,8 @@ struct stmmac_hwtimestamp {
>   	int (*config_addend) (void __iomem *ioaddr, u32 addend);
>   	int (*adjust_systime) (void __iomem *ioaddr, u32 sec, u32 nsec,
>   			       int add_sub, int gmac4);
> -	void (*get_systime) (void __iomem *ioaddr, u64 *systime);
> +	void (*get_systime)(void __iomem *ioaddr, u64 *systime,
> +			    struct ptp_system_timestamp *sts);
>   	void (*get_ptptime)(void __iomem *ioaddr, u64 *ptp_time);
>   	void (*timestamp_interrupt)(struct stmmac_priv *priv);
>   	void (*hwtstamp_correct_latency)(struct stmmac_priv *priv);
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
> index b9a985fa772..870c740b4a5 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
> @@ -183,7 +183,8 @@ static int adjust_systime(void __iomem *ioaddr, u32 sec, u32 nsec,
>   					 10, 100000);
>   }
>   
> -static void get_systime(void __iomem *ioaddr, u64 *systime)
> +static void get_systime(void __iomem *ioaddr, u64 *systime,
> +			struct ptp_system_timestamp *sts)
>   {
>   	u64 ns, sec0, sec1;
>   
> @@ -191,8 +192,10 @@ static void get_systime(void __iomem *ioaddr, u64 *systime)
>   	sec1 = readl_relaxed(ioaddr + PTP_STSR);
>   	do {
>   		sec0 = sec1;
> +		ptp_read_system_prets(sts);
>   		/* Get the TSSS value */
> -		ns = readl_relaxed(ioaddr + PTP_STNSR);
> +		ns = readl(ioaddr + PTP_STNSR);
> +		ptp_read_system_postts(sts);
>   		/* Get the TSS value */
>   		sec1 = readl_relaxed(ioaddr + PTP_STSR);
>   	} while (sec0 != sec1);
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> index 3bfcc9760dc..e54a3c88782 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> @@ -89,7 +89,8 @@ static int stmmac_adjust_time(struct ptp_clock_info *ptp, s64 delta)
>   		u64 cycle_time;
>   
>   		mutex_lock(&priv->est_lock);
> -		priv->ptp_clock_ops.gettime64(&priv->ptp_clock_ops, &current_time);
> +		priv->ptp_clock_ops.gettimex64(&priv->ptp_clock_ops,
> +					       &current_time, NULL);
>   		current_time_ns = timespec64_to_ktime(current_time);
>   		time.tv_nsec = priv->est->btr_reserve[0];
>   		time.tv_sec = priv->est->btr_reserve[1];
> @@ -114,15 +115,15 @@ static int stmmac_adjust_time(struct ptp_clock_info *ptp, s64 delta)
>   }
>   
>   /**
> - * stmmac_get_time
> - *
> + * stmmac_get_timex - read the PTP clock, with system timestamps if requested
>    * @ptp: pointer to ptp_clock_info structure
>    * @ts: pointer to hold time/result
> + * @sts: pointer to hold system timestamps around the read, or NULL
>    *
> - * Description: this function will read the current time from the
> - * hardware clock and store it in @ts.
> + * Return: 0
>    */
> -static int stmmac_get_time(struct ptp_clock_info *ptp, struct timespec64 *ts)
> +static int stmmac_get_timex(struct ptp_clock_info *ptp, struct timespec64 *ts,
> +			    struct ptp_system_timestamp *sts)
>   {
>   	struct stmmac_priv *priv =
>   	    container_of(ptp, struct stmmac_priv, ptp_clock_ops);
> @@ -130,7 +131,7 @@ static int stmmac_get_time(struct ptp_clock_info *ptp, struct timespec64 *ts)
>   	u64 ns = 0;
>   
>   	read_lock_irqsave(&priv->ptp_lock, flags);
> -	stmmac_get_systime(priv, priv->ptpaddr, &ns);
> +	stmmac_get_systime(priv, priv->ptpaddr, &ns, sts);
>   	read_unlock_irqrestore(&priv->ptp_lock, flags);
>   
>   	*ts = ns_to_timespec64(ns);
> @@ -200,7 +201,7 @@ static int stmmac_enable(struct ptp_clock_info *ptp,
>   
>   		target_ns = cfg->start.tv_nsec + ((u64)cfg->start.tv_sec * NSEC_PER_SEC);
>   
> -		stmmac_get_systime(priv, priv->ptpaddr, &ns);
> +		stmmac_get_systime(priv, priv->ptpaddr, &ns, NULL);
>   		if (ns > TIME64_MAX - PTP_SAFE_TIME_OFFSET_NS)
>   			return -EINVAL;
>   
> @@ -304,7 +305,7 @@ const struct ptp_clock_info stmmac_ptp_clock_ops = {
>   	.pps = 0,
>   	.adjfine = stmmac_adjust_freq,
>   	.adjtime = stmmac_adjust_time,
> -	.gettime64 = stmmac_get_time,
> +	.gettimex64 = stmmac_get_timex,
>   	.settime64 = stmmac_set_time,
>   	.enable = stmmac_enable,
>   };
> @@ -321,7 +322,7 @@ const struct ptp_clock_info dwmac1000_ptp_clock_ops = {
>   	.pps = 0,
>   	.adjfine = stmmac_adjust_freq,
>   	.adjtime = stmmac_adjust_time,
> -	.gettime64 = stmmac_get_time,
> +	.gettimex64 = stmmac_get_timex,
>   	.settime64 = stmmac_set_time,
>   	.enable = dwmac1000_ptp_enable,
>   };
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
> index 6097f312fce..2ae4f0e760c 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
> @@ -1778,7 +1778,7 @@ static int stmmac_test_tbs(struct stmmac_priv *priv)
>   		return ret;
>   
>   	read_lock_irqsave(&priv->ptp_lock, flags);
> -	stmmac_get_systime(priv, priv->ptpaddr, &curr_time);
> +	stmmac_get_systime(priv, priv->ptpaddr, &curr_time, NULL);
>   	read_unlock_irqrestore(&priv->ptp_lock, flags);
>   
>   	if (!curr_time) {
> @@ -1800,7 +1800,7 @@ static int stmmac_test_tbs(struct stmmac_priv *priv)
>   
>   	/* Check if expected time has elapsed */
>   	read_lock_irqsave(&priv->ptp_lock, flags);
> -	stmmac_get_systime(priv, priv->ptpaddr, &curr_time);
> +	stmmac_get_systime(priv, priv->ptpaddr, &curr_time, NULL);
>   	read_unlock_irqrestore(&priv->ptp_lock, flags);
>   
>   	if ((curr_time - start_time) < STMMAC_TBS_LT_OFFSET)
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> index 42a00446e9b..57ebeb2c19c 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> @@ -1070,7 +1070,8 @@ static int tc_taprio_configure(struct stmmac_priv *priv,
>   
>   	mutex_lock(&priv->est_lock);
>   	/* Adjust for real system time */
> -	priv->ptp_clock_ops.gettime64(&priv->ptp_clock_ops, &current_time);
> +	priv->ptp_clock_ops.gettimex64(&priv->ptp_clock_ops, &current_time,
> +				       NULL);
>   	current_time_ns = timespec64_to_ktime(current_time);
>   	time = stmmac_calc_tas_basetime(qopt->base_time, current_time_ns,
>   					qopt->cycle_time);

Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>