amd-xgbe: fix PTP addend overflow causing frozen clock

[PATCH] amd-xgbe: fix PTP addend overflow causing frozen clock

Posted by Gregory Fuchedgi via B4 Relay 1 month, 2 weeks ago

From: Gregory Fuchedgi <gfuchedgi@gmail.com>

XGBE_PTP_ACT_CLK_FREQ and XGBE_V2_PTP_ACT_CLK_FREQ were 10x too
large (500MHz/1GHz instead of 50MHz/100MHz), causing the computed
addend to overflow the 32-bit tstamp_addend. In the general case
this would result in the clock advancing at the wrong rate. For v2
(PCI), ptpclk_rate is hardcoded to 125MHz, so the addend formula
(ACT_CLK_FREQ << 32) / ptpclk_rate yields exactly 8 * 2^32, and
when stored to the 32-bit tstamp_addend the value is zero. With
addend = 0 the hardware accumulator never overflows and the PTP
clock is fully stopped. For v1 (platform), ptpclk_rate is read from
ACPI/DT so the exact overflow behavior depends on the
firmware-reported frequency.

Define the constants as NSEC_PER_SEC / SSINC so the relationship is
explicit and cannot drift out of sync.

Fixes: fbd47be098b5 ("amd-xgbe: add hardware PTP timestamping support")
Tested-by: Gregory Fuchedgi <gfuchedgi@gmail.com>
Signed-off-by: Gregory Fuchedgi <gfuchedgi@gmail.com>
---
Tested by running ptp4l and verifying successful clock synchronization on v2
(PCI) hardware.
---
 drivers/net/ethernet/amd/xgbe/xgbe.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe.h b/drivers/net/ethernet/amd/xgbe/xgbe.h
index 60b7e53206d1..3d3b09010d48 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe.h
+++ b/drivers/net/ethernet/amd/xgbe/xgbe.h
@@ -135,11 +135,11 @@
  */
 #define XGBE_TSTAMP_SSINC	20
 #define XGBE_TSTAMP_SNSINC	0
-#define XGBE_PTP_ACT_CLK_FREQ	500000000
+#define XGBE_PTP_ACT_CLK_FREQ	(NSEC_PER_SEC / XGBE_TSTAMP_SSINC)
 
 #define XGBE_V2_TSTAMP_SSINC	0xA
 #define XGBE_V2_TSTAMP_SNSINC	0
-#define XGBE_V2_PTP_ACT_CLK_FREQ	1000000000
+#define XGBE_V2_PTP_ACT_CLK_FREQ	(NSEC_PER_SEC / XGBE_V2_TSTAMP_SSINC)
 
 /* Define maximum supported values */
 #define XGBE_MAX_PPS_OUT	4

---
base-commit: 0c7a5ba011d336df4fcd1f667fcc16ea5549be12
change-id: 20260428-fix-xgbe-ptp-addend-ccb1df627622

Best regards,
-- 
Gregory Fuchedgi <gfuchedgi@gmail.com>

Re: [PATCH] amd-xgbe: fix PTP addend overflow causing frozen clock

Posted by Simon Horman 1 month, 2 weeks ago

On Wed, Apr 29, 2026 at 02:54:14PM -0700, Gregory Fuchedgi via B4 Relay wrote:
> From: Gregory Fuchedgi <gfuchedgi@gmail.com>
> 
> XGBE_PTP_ACT_CLK_FREQ and XGBE_V2_PTP_ACT_CLK_FREQ were 10x too
> large (500MHz/1GHz instead of 50MHz/100MHz), causing the computed
> addend to overflow the 32-bit tstamp_addend. In the general case
> this would result in the clock advancing at the wrong rate. For v2
> (PCI), ptpclk_rate is hardcoded to 125MHz, so the addend formula
> (ACT_CLK_FREQ << 32) / ptpclk_rate yields exactly 8 * 2^32, and
> when stored to the 32-bit tstamp_addend the value is zero. With
> addend = 0 the hardware accumulator never overflows and the PTP
> clock is fully stopped. For v1 (platform), ptpclk_rate is read from
> ACPI/DT so the exact overflow behavior depends on the
> firmware-reported frequency.
> 
> Define the constants as NSEC_PER_SEC / SSINC so the relationship is
> explicit and cannot drift out of sync.
> 
> Fixes: fbd47be098b5 ("amd-xgbe: add hardware PTP timestamping support")
> Tested-by: Gregory Fuchedgi <gfuchedgi@gmail.com>
> Signed-off-by: Gregory Fuchedgi <gfuchedgi@gmail.com>

Reviewed-by: Simon Horman <horms@kernel.org>

There is an AI generated review of this patch available on sashiko.dev.
While I do believe the issues flagged there warrant investigation
as possible follow-up, I do not think they should delay progress
of this patch.

Re: [PATCH] amd-xgbe: fix PTP addend overflow causing frozen clock

Posted by Gregory Fuchedgi 1 month, 2 weeks ago

Thanks for the review, Simon.

I don't know what frequency gets returned for v1 in practice. Maybe we could
increase v1 SSINC if the ptpclk_rate can be low (<=50MHz), but I don't have
v1 hardware to experiment with.

On Fri, May 1, 2026 at 8:32 AM Simon Horman <horms@kernel.org> wrote:
>
> On Wed, Apr 29, 2026 at 02:54:14PM -0700, Gregory Fuchedgi via B4 Relay wrote:
> > From: Gregory Fuchedgi <gfuchedgi@gmail.com>
> >
> > XGBE_PTP_ACT_CLK_FREQ and XGBE_V2_PTP_ACT_CLK_FREQ were 10x too
> > large (500MHz/1GHz instead of 50MHz/100MHz), causing the computed
> > addend to overflow the 32-bit tstamp_addend. In the general case
> > this would result in the clock advancing at the wrong rate. For v2
> > (PCI), ptpclk_rate is hardcoded to 125MHz, so the addend formula
> > (ACT_CLK_FREQ << 32) / ptpclk_rate yields exactly 8 * 2^32, and
> > when stored to the 32-bit tstamp_addend the value is zero. With
> > addend = 0 the hardware accumulator never overflows and the PTP
> > clock is fully stopped. For v1 (platform), ptpclk_rate is read from
> > ACPI/DT so the exact overflow behavior depends on the
> > firmware-reported frequency.
> >
> > Define the constants as NSEC_PER_SEC / SSINC so the relationship is
> > explicit and cannot drift out of sync.
> >
> > Fixes: fbd47be098b5 ("amd-xgbe: add hardware PTP timestamping support")
> > Tested-by: Gregory Fuchedgi <gfuchedgi@gmail.com>
> > Signed-off-by: Gregory Fuchedgi <gfuchedgi@gmail.com>
>
> Reviewed-by: Simon Horman <horms@kernel.org>
>
> There is an AI generated review of this patch available on sashiko.dev.
> While I do believe the issues flagged there warrant investigation
> as possible follow-up, I do not think they should delay progress
> of this patch.