Re: [PATCH RFC net-next 3/5] net: phy: add Marvell PHY PTP support

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Mon, Apr 14, 2025 at 04:43:14PM +0200, Kory Maincent wrote:
> On Fri, 11 Apr 2025 22:26:42 +0100
> Russell King <rmk+kernel@armlinux.org.uk> wrote:
> 
> > Add PTP basic support for Marvell 88E151x single port PHYs.  These
> > PHYs support timestamping the egress and ingress of packets, but does
> > not support any packet modification, nor do we support any filtering
> > beyond selecting packets that the hardware recognises as PTP/802.1AS.
> > 
> > The PHYs support hardware pins for providing an external clock for the
> > TAI counter, and a separate pin that can be used for event capture or
> > generation of a trigger (either a pulse or periodic). Only event
> > capture is supported.
> > 
> > We currently use a delayed work to poll for the timestamps which is
> > far from ideal, but we also provide a function that can be called from
> > an interrupt handler - which would be good to tie into the main Marvell
> > PHY driver.
> > 
> > The driver takes inspiration from the Marvell 88E6xxx DSA and DP83640
> > drivers. The hardware is very similar to the implementation found in
> > the 88E6xxx DSA driver, but the access methods are very different,
> > although it may be possible to create a library that both can use
> > along with accessor functions.
> 
> It seems a lot less stable than the first version of the driver.
> 
> Lots of overrun:
> ptp4l[949.894]: port 1 (eth0): assuming the grand master role
> [  954.899275] Marvell 88E1510 ff0d0000.ethernet-ffffffff:01: tx timestamp overrun (stat=0x5 seq=0)
> [  954.908670] Marvell 88E1510 ff0d0000.ethernet-ffffffff:01: rx timestamp overrun (q=1 stat=0x5 seq=0)

I've explained why this happens - it will occur if timestamping has
been enabled but there's been no packets to be stamped for a while
but there _have_ been packets on the network that the hardware
decided to stamp. There is no way around this because the driver isn't
told when to shutdown timestamping.

> PTP L2 in master mode is not working at all maybe because there is no
> configuration of PTP global config 1 register.

No, sorry, wrong. There is, and you haven't read what I've written
previously if you're still thinking this.

Look in "ptp: marvell: add core support for Marvell PTP v2.1".

Look at marvell_tai_global_config(). (I've explained why it's there.)

Look at:

+       /* MsdIDTSEn - Enable timestamping on all PTP MessageIDs */
+       err = tai->ops->ptp_global_write(tai->dev, PTPG_CONFIG_1,
+                                        MV_PTP_MSD_ID_TS_EN);

> Faced also a case where it has really high offsets and I need to reboot the
> board to see precise synchronization again:
> ptp4l[4649.618]: port 1: LISTENING to UNCALIBRATED on RS_SLAVE
> ptp4l[4652.519]: master offset 7650600217 s0 freq +32767998 path delay -33923681843
> ptp4l[4653.487]: master offset 7617827584 s1 freq      +6 path delay -33923681843
> ptp4l[4654.454]: master offset     -27201 s2 freq  -27195 path delay -33923681843
> ptp4l[4654.454]: port 1: UNCALIBRATED to SLAVE on MASTER_CLOCK_SELECTED
> ptp4l[4655.422]: master offset 33926879673 s2 freq +32767999 path delay -67850561538
> ptp4l[4656.389]: master offset 33649307442 s2 freq +32767999 path delay -67605734496
> ptp4l[4657.356]: master offset 33454635535 s2 freq +32767999 path delay -67443834753

I don't see that, I get way smaller offsets with this when using the
ZII rev B platform as the master.

> The PTP set as master mode seems really buggy.

That's something I didn't test, but as the hardware is symmetrical (there
aren't separate settings for PTP MSGIDs that get stamped on the transmit
and receive paths) I'm not sure what's going on. If one enables
timestamping for MSG ID 0 (Sync) then the hardware will stamp packets
received and transmitted that have a PTP MSG ID 0.

In any case, I can't do any testing now, sorry.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!