Re: [PATCH net-next v2 2/2] net: phy: Add Marvell PHY PTP support

[Russell King (Oracle)]

On Fri, Apr 11, 2025 at 10:01:27AM +0200, Kory Maincent wrote:
> On Thu, 10 Apr 2025 20:40:12 +0100
> "Russell King (Oracle)" <linux@xxxxxxxxxxxxxxx> wrote:
> 
> > On Thu, Apr 10, 2025 at 07:16:47PM +0100, Russell King (Oracle) wrote:
> > > On Thu, Apr 10, 2025 at 06:02:05PM +0200, Kory Maincent wrote:  
> > > > On Thu, 10 Apr 2025 16:41:06 +0100
> > > > "Russell King (Oracle)" <linux@xxxxxxxxxxxxxxx> wrote:
> 
> > > > It seems you are still using your Marvell PHY drivers without my change.
> > > > PTP L2 was broken on your first patch and I fixed it.
> > > > I have the same result without the -2 which mean ptp4l uses UDP IPV4.  
> > > 
> > > I'm not sure what you're referring to.  
> > 
> > Okay, turns out to be nothing to do with any fixes in my code or not
> > (even though I still don't know what the claimed brokenness you
> > refer to actually was.)
> 
> If I remember well you need the PTP global config 1 register set to 3 to have
> the L2 PTP working.

The PTP global config 1 register determines which message IDs get
timestamped, both for incoming and outgoing messages.

Setting it to 0x3 means that only Sync and Delay_Req messages only
get stamped, irrespective of whether userspace wants to stamp
messages in the transmit path with other message IDs.

With it set to ~0 as I have it, this means all PTP messages are
candidates for being stamped.

As this is a global register, which can be shared between ports (not
for 1510, but may be for other PHYs or DSA), and we have no idea which
messages will need to be stamped, setting it to ~0 is sensible, and
it's also what would be expected of the PTP layers, because we report
back to userspace HWTSTAMP_FILTER_SOME which means "return value: time
stamp all packets requested plus some others" (from the documentation).

I'm currently using 0x0203 (sync, delay_req, delay_resp) and it's
working mostly fine, although I do from time to time see rx overruns
and sometimes tx timestamps missed. I'm trying to fix that before I
post updated patches.

It should be fine with other values too, and should have no effect
whether L2 and L4 are used. DSA sets this to 0x0f, which uses the
same hardware, and I assume is well tested:

        /* MV88E6XXX_PTP_MSG_TYPE is a mask of PTP message types to
         * timestamp. This affects all ports that have timestamping enabled,
         * but the timestamp config is per-port; thus we configure all events
         * here and only support the HWTSTAMP_FILTER_*_EVENT filter types.
         */
        err = mv88e6xxx_ptp_write(chip, MV88E6XXX_PTP_MSGTYPE,
                                  MV88E6XXX_PTP_MSGTYPE_ALL_EVENT);

#define MV88E6XXX_PTP_MSGTYPE_ALL_EVENT         0x000f

with peer delay response messages directed to the second arrival
timestamp registers:

        /* Use ARRIVAL1 for peer delay response messages. */
        err = mv88e6xxx_ptp_write(chip, MV88E6XXX_PTP_TS_ARRIVAL_PTR,
                                  MV88E6XXX_PTP_MSGTYPE_PDLAY_RES);

#define MV88E6XXX_PTP_MSGTYPE_PDLAY_RES         0x0008

Please re-test with other values and report how it doesn't work.

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