Skip to content

Improve synchronization accuracy between the BCM54210PE PHC and system clock - #7673

Open
jclark wants to merge 4 commits into
raspberrypi:rpi-6.18.yfrom
jclark:mdio-frame-timestamps-review
Open

jclark wants to merge 4 commits into
raspberrypi:rpi-6.18.yfrom
jclark:mdio-frame-timestamps-review

Conversation

@jclark

@jclark jclark commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

On a Raspberry Pi CM5, using PTP_SYS_OFFSET_EXTENDED to synchronize the system clock from the BCM54210PE PHC results in the system clock being about 32 µs ahead. On a CM4, this is reduced to about 7 µs. (The difference is because the MDIO clock frequency is higher on the CM4: 10 MHz as opposed to 2.08 MHz on the CM5.) PTP_SYS_OFFSET_EXTENDED uses gettimex64 to bracket each PHC read with system clock reads. With the BCM54210PE PHY, these brackets include multiple MDIO transfers and are very wide: about 218 µs on the CM5 and 43 µs on the CM4. This is using rpi-6.18.y at 6.18.54. See below for measurement process.

This PR introduces a new MDIO operation that allows PHY drivers to ask MDIO bus drivers to compute a bracket for the completion of an MDIO write transfer. It then makes bcm-phy-ptp use this operation, and implements it for both the macb and UniMAC drivers.

With these patches, the system clock synchronization error is reduced from about 32 µs to below 0.1 µs on the CM5, and from about 7 µs to below 1 µs on the CM4. PTP_SYS_OFFSET_EXTENDED brackets are reduced from 218 µs to about 2.2 µs on the CM5 and from 43 µs to 1.1 µs on the CM4.

This PR has four commits:

  1. Introduce the new MDIO operation
  2. Use it in the BCM54210PE PHY driver
  3. Implement it for macb
  4. Implement it for UniMAC

Some additional work should be done as follow-ups:

a. UniMAC should get the clock frequency from DT, but I don't know how to do this properly for the BCM2711.
b. macb should probably use a higher MDIO clock frequency: the CM4 runs at 10 MHz, but the CM5 runs at about 2.08 MHz.
c. The BCM54210PE PHY driver does not consistently check errors from MDIO operations.
d. The upstream macb MDIO polling interval is 1 µs, versus 100 µs in the Raspberry Pi tree, making PHC reads much slower. But the downstream change (commit 707efde) was done to reduce CPU wakeups, so it shouldn't simply be reverted.
e. The BCM54210PE PHY code is assuming that the time latches immediately on the completion of the MDIO write, but in practice there will be a small delay. If this is in the spec sheet somewhere or can be measured somehow, we could compensate for it.

I have done the patch in a way that I hope will be acceptable to upstream. Note that SPI introduced a similar facility to allow a device driver to obtain timestamp bounds from the bus controller in commit 79591b7 ("spi: Add a PTP system timestamp to the transfer structure"). I plan to submit to upstream, but I would like to get some feedback from Raspberry Pi maintainers on points (a) and (e) first. Upstream might object to an MDIO operation with only a single PHY user, but I surveyed other PHY drivers and identified at least two for which this operation could be implemented: micrel (specifically LAN8814) and dp83640, although they would need updating to use gettimex64 first.

Measurement process

On the CM5, it is possible to measure the inaccuracy by taking advantage of the recently introduced feature that allows selection of the PHC used for packet timestamping. /dev/ptp0 is the BCM54210PE PHC; /dev/ptp1 is the macb MAC PHC. PTP_SYS_OFFSET_EXTENDED brackets with macb are about 1 µs after upstream commit 9ca4ba2 ("net: macb: fix ordering around PTP timestamp read"). So we can use the following measurement approach: synchronize /dev/ptp1 to an external PTP grandmaster; synchronize /dev/ptp0 to a PPS from a GPS; synchronize the system clock from /dev/ptp1 using phc2sys or chrony; then measure the offset between the system clock and the result of PTP_SYS_OFFSET_EXTENDED.

An alternative way to check, which does not require a PTP grandmaster, is to connect the same PPS signal to both the GPIO PPS pin (header pin 12) and to SYNC_OUT. Then have an NTP server discipline the system clock using the GPS PPS signal, and as before measure the offset between the system clock and the result of PTP_SYS_OFFSET_EXTENDED. But kernel PPS has a significant bias (of the order of 10 µs). However, I have developed a tool called ppsbias to measure this, which works by polling GPIO memory. If you configure the NTP server with the offset from ppsbias, then you can get an estimate that is accurate to within about 1 µs. The results from ppsbias are consistent with results from using PTP with MAC PHC. On a CM4, this is the only method available, since /dev/ptp1 is not available.

@jclark

jclark commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

@nbuchwitz Could you possibly have a look? @lasselj You might be interested in this.

@nbuchwitz

Copy link
Copy Markdown
Contributor

I will have a look once I'm back from LPC in Prague. Ideally this goes via upstream, but let me check the patches and then make a plan how it would be upstreamed

@pelwell

pelwell commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

I'd like to publicly thank Nicolai for his large and continuing investment of time into supporting the Raspberry Pi platform, and to make it clear to everyone that they should not expect his attention and support - he doesn't get paid by us. We are very grateful for his many contributions.

jclark added 4 commits October 6, 2026 16:24
Add an optional write_sts bus operation returning system timestamp
bounds for completion of an MDIO write. This allows PHY drivers to
obtain tighter bounds when implementing gettimex64. SPI introduced a
similar facility to allow a device driver to obtain timestamp bounds
from the bus controller in commit 79591b7 ("spi: Add a PTP system
timestamp to the transfer structure").

A PHY driver using this operation is responsible for fallback if the bus
driver does not implement it.

Signed-off-by: James Clark <jjc@jclark.com>
Use __phy_write_sts() to obtain tighter system timestamp bounds in
gettimex64 by timestamping completion of the write that triggers
capture of the PHC time.

Signed-off-by: James Clark <jjc@jclark.com>
Implement write_sts to provide system timestamp bounds for completion
of an MDIO write. This enables tighter system timestamp bounds in PHY
implementations of gettimex64.

Take system timestamps around the command register write, then add
bounds on the transfer time calculated from the peripheral clock rate
and configured MDC divider.

Signed-off-by: James Clark <jjc@jclark.com>
Implement write_sts to provide system timestamp bounds for completion
of an MDIO write. This enables tighter system timestamp bounds in PHY
implementations of gettimex64.

Take system timestamps around the command start, then add bounds on
the transfer time calculated from the reference clock rate and
configured MDC divider.

Use a 200 MHz reference rate for BCM2711 GENET, whose clock is not
described in DT.

Signed-off-by: James Clark <jjc@jclark.com>
@jclark
jclark force-pushed the mdio-frame-timestamps-review branch from a75dcbe to d882728 Compare October 6, 2026 09:28
@jclark

jclark commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

@pelwell I force-pushed to drop the Assisted-by: LLM trailers. These were the right format for upstream, but the CI's checkpatch does not recognize them. The code is otherwise unchanged.

I echo your thanks to Nicolai. He was very helpful last month with a couple of Raspberry Pi PHC related patches that I submitted upstream last month.

@pelwell

pelwell commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

I'm not bothered about checkpatch's objection to the Assisted-by tags (which are only recognised in newer kernels branches) - we're quite relaxed about the commit message content - but accepting a patch downstream that adds a new facility that isn't specific to Raspberry Pi devices is something we are reluctant to do because of the ongoing support burden. I've kicked off the autobuilds so people can confirm the utility of your work, and we can even iterate the patches here a few times, but we'd much prefer this work to be accepted upstream first - at which point we will take a backport.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants