Messages in this thread | | | From | Michael Walle <> | Subject | [RFC PATCH v2 0/2] AT8031 PHY timestamping support | Date | Fri, 28 Feb 2020 19:02:24 +0100 |
| |
I've put some additional time into figuring out how the PHY works since my last RFC. My previous comments on the interrupt handling was caused by my broken setup.
This patchset is the current state of my work for adding PHY timestamping support. I just wanted to post this to the mailinglist before I never do it. Maybe its a starting point for other people. That being said, I wouldn't mind comments ;) Also I like to share my findings about the PHY. The PHY has three major caveats which IMHO makes it really hard to work properly.
(1) The PHY doesn't support atomic reading of the (timestamp, messageType, sequenceId) tuple. The workaround is to read the timestamp again and check if it has changed. Actually, you'd have to read the complete tuple again. (2) The PHY generates an interrupt on every PTP packet not only on event messages. Thus the interrupt handler may read the capture registers and then determine that nothing has changed. This also means we have to remember the last read timestamp. I make the assumption that the timestamp is unique. (3) The biggest drawback is that the PHY only provide one set of RX and TX capture registers. It is possible that the timestamp will change when another PTP event message is received while we are still reading the timestamp of the previously received packet.
Example A The driver basically works when there is low PTP traffic. Eg. the following works pretty good.
ptp4l -m -2 -i eth0 -f ptp.cfg -s
ptp.cfg: [global] tx_timestamp_timeout 100
Example B But if you're using a P2P clock with peer delay requests this whole thing falls apart because of caveat (3). You'll often see messages like received SYNC without timestamp or received PDELAY_RESP without timestamp in linuxptp. Sometimes it working for some time and then it starts to loosing packets. I suspect this depends on how the PDELAY messages are interleaved with the SYNC message. If there is not enough time to until the next event message is received either of these two messages won't have a timestamp.
ptp4l -m -f gPTP.cfg -i eth0 -s
gPTP.cfg is the one from stock linuxptp with tx_timestamp_timeout set to 100.
The PHY also supports appending the timestamp to the actual ethernet frame, but this seems to only work when the PHY is connected via RGMII. I've never get it to work with a SGMII connection.
The first patch might actually be useful outside of this series. See also https://lore.kernel.org/netdev/bd47f8e1ebc04fa98856ed8d89b91419@walle.cc/
Changes since RFC v1: net: phy: let the driver register its own IRQ handler - fixed mistake for calling phy_request_interrupt(). Thanks Heiner. - removed phy_drv_interrupt(). just calling phy_mac_interrupt()
net: phy: at803x: add PTP support for AR8031 - moved rereading the timestamp out of at8031_read_ts() to at8031_get_rxts()/at8031_get_txts(). - call phy_mac_interrupt() instead of phy_drv_interrupt()
Please note that Heiner suggested that I should use handle_interrupt() instead of registering my own handler. I've still included my old patch here because the discussion is still ongoing and the proposed fix is only working for this use case. See also https://lore.kernel.org/netdev/2e4371354d84231abf3a63deae1a0d04@walle.cc/
-michael
Michael Walle (2): net: phy: let the driver register its own IRQ handler net: phy: at803x: add PTP support for AR8031
drivers/net/phy/Kconfig | 17 + drivers/net/phy/at803x.c | 855 ++++++++++++++++++++++++++++++++++- drivers/net/phy/phy_device.c | 6 +- include/linux/phy.h | 1 + 4 files changed, 852 insertions(+), 27 deletions(-)
-- 2.20.1
| |