RE: [Intel-wired-lan] [PATCH net v1] idpf: read lower clock bits inside the time sandwich

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@xxxxxxxxxx> On Behalf
> Of Mina Almasry
> Sent: Thursday, December 11, 2025 11:19 AM
> To: netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Cc: Mina Almasry <almasrymina@xxxxxxxxxx>; Nguyen, Anthony L
> <anthony.l.nguyen@xxxxxxxxx>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@xxxxxxxxx>; Andrew Lunn <andrew+netdev@xxxxxxx>;
> David S. Miller <davem@xxxxxxxxxxxxx>; Eric Dumazet
> <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo Abeni
> <pabeni@xxxxxxxxxx>; Richard Cochran <richardcochran@xxxxxxxxx>;
> Rizzo, Luigi <lrizzo@xxxxxxxxxx>; namangulati@xxxxxxxxxx;
> willemb@xxxxxxxxxx; intel-wired-lan@xxxxxxxxxxxxxxxx; Olech, Milena
> <milena.olech@xxxxxxxxx>; Keller, Jacob E <jacob.e.keller@xxxxxxxxx>;
> Shachar Raindel <shacharr@xxxxxxxxxx>
> 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: lrizzo@xxxxxxxxxx
> Cc: namangulati@xxxxxxxxxx
> Cc: willemb@xxxxxxxxxx
> Cc: intel-wired-lan@xxxxxxxxxxxxxxxx
> Cc: milena.olech@xxxxxxxxx
> Cc: jacob.e.keller@xxxxxxxxx
> 
> Fixes: 5cb8805d2366 ("idpf: negotiate PTP capabilities and get PTP
> clock")
> Suggested-by: Shachar Raindel <shacharr@xxxxxxxxxx>
> Signed-off-by: Mina Almasry <almasrymina@xxxxxxxxxx>
> ---
>  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?


> 
>  	/* Read the system timestamp post PHC read */
>  	ptp_read_system_postts(sts);
> 
> -	lo = readl(ptp->dev_clk_regs.dev_clk_ns_l);
>  	hi = readl(ptp->dev_clk_regs.dev_clk_ns_h);
> 
>  	spin_unlock(&ptp->read_dev_clk_lock);
> 
> base-commit: 885bebac9909994050bbbeed0829c727e42bd1b7
> --
> 2.52.0.223.gf5cc29aaa4-goog