On 12/11/25 23:06, Jacob Keller wrote:
On 12/11/2025 2:37 AM, Loktionov, Aleksandr wrote:
-----Original Message-----
From: Intel-wired-lan <[email protected]> On Behalf
Of Mina Almasry
Sent: Thursday, December 11, 2025 11:19 AM
To: [email protected]; [email protected]
Cc: Mina Almasry <[email protected]>; Nguyen, Anthony L
<[email protected]>; Kitszel, Przemyslaw
<[email protected]>; Andrew Lunn <[email protected]>;
David S. Miller <[email protected]>; Eric Dumazet
<[email protected]>; Jakub Kicinski <[email protected]>; Paolo Abeni
<[email protected]>; Richard Cochran <[email protected]>;
Rizzo, Luigi <[email protected]>; [email protected];
[email protected]; [email protected]; Olech, Milena
<[email protected]>; Keller, Jacob E <[email protected]>;
Shachar Raindel <[email protected]>
Subject: [Intel-wired-lan] [PATCH net v1] idpf: read lower clock bits
inside the time sandwich
PCIe reads need to be done inside the time sandwich because PCIe
writes may get buffered in the PCIe fabric and posted to the device
after the _postts completes. Doing the PCIe read inside the time
sandwich guarantees that the write gets flushed before the _postts
timestamp is taken.
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Fixes: 5cb8805d2366 ("idpf: negotiate PTP capabilities and get PTP
clock")
Suggested-by: Shachar Raindel <[email protected]>
Signed-off-by: Mina Almasry <[email protected]>
---
drivers/net/ethernet/intel/idpf/idpf_ptp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.c
b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
index 3e1052d070cf..0a8b50350b86 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_ptp.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
@@ -108,11 +108,11 @@ static u64
idpf_ptp_read_src_clk_reg_direct(struct idpf_adapter *adapter,
ptp_read_system_prets(sts);
idpf_ptp_enable_shtime(adapter);
+ lo = readl(ptp->dev_clk_regs.dev_clk_ns_l);
The high 32 bits (hi) are still read outside the time sandwich (after
ptp_read_system_postts()),
which defeats the stated purpose of ensuring PCIe write flush before timestamp
capture.
/* I think he "time sandwich" is defined by the region between
ptp_read_system_prets(sts) and ptp_read_system_postts(sts) */ Isn't it?
Any read will cause writes to flush, so we don't need to move both
registers.
The point here is that we write to the shadow register to snapshot time,
and it won't guarantee to be flushed to the device until a read. By
moving a single read in side the time sandwhich, we ensure that its
actually complete before the time snapshot is taken. We don't need to
wait for both registers because of the snapshot behavior.
very nice explanation Jake, thank you
I don't know if it should be considered "basic common knowledge", or
warrants an entry in commit message/code comment
For sure we don't want anyone not knowing that to touch the code, so
barrier to entry is also a good thing ;)
I think the patch is fine-as-is.
given the scope of the function, agree
Reviewed-by: Jacob Keller <[email protected]>
Thanks,
Jake
