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

[Maxime Chevallier]

Hi,

On 10/8/26 09:44, Nicolai Buchwitz wrote:
> Hi James
> 
> On 6.10.2026 12:36, 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. rmb() is used to ensure proper ordering on weakly ordered
>> architectures.
>>
>> Assisted-by: LLM
>> Signed-off-by: James Clark <jjc@xxxxxxxxxx>
>> ---
>> 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    584 ns
>>
>> On this board the 24 MHz arch timer counter advances in steps of 7
>> (~292 ns), so all intervals are multiples of that.
> 
>> [...]
> 
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
> 
>> [...]
> 
>> @@ -191,8 +192,14 @@ static void get_systime(void __iomem *ioaddr, u64 *systime)
>>      sec1 = readl_relaxed(ioaddr + PTP_STSR);
>>      do {
>>          sec0 = sec1;
>> +        ptp_read_system_prets(sts);
>> +        if (sts)
>> +            rmb();
>>          /* Get the TSSS value */
>>          ns = readl_relaxed(ioaddr + PTP_STNSR);
>> +        if (sts)
>> +            rmb();
>> +        ptp_read_system_postts(sts);
> 
> Additional thought while testing on my stm32mp2:
> 
> Would a plain readl() for PTP_STNSR be enough? readl() completes before
> a following delay loop, which is the counter read postts does. That would
> drop both barriers, like igb/ice/bnxt. Fine either way though, macb has
> the same rmb() pattern.
> 
> Maxime (Chevallier), what do you think?

I agree yes, I was also wondering about just going to non-relaxed accessor
and drop the barriers.

Maxime

> 
>> [...]
> 
> Thanks,
> Nicolai