Message-ID: <20250416151949.6bddde35@kmaincent-XPS-13-7390>
Date: Wed, 16 Apr 2025 15:19:49 +0200
From: Kory Maincent <kory.maincent@...tlin.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>,
 Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski
 <kuba@...nel.org>, Marcin Wojtas <marcin.s.wojtas@...il.com>,
 netdev@...r.kernel.org, Paolo Abeni <pabeni@...hat.com>, Richard Cochran
 <richardcochran@...il.com>, Vladimir Oltean <olteanv@...il.com>
Subject: Re: [PATCH RFC net-next 0/5] Marvell PTP support

On Wed, 16 Apr 2025 11:51:27 +0100
"Russell King (Oracle)" <linux@...linux.org.uk> wrote:

> On Fri, Apr 11, 2025 at 10:26:15PM +0100, Russell King (Oracle) wrote:
> > Hi,
> > 
> > This series is a work in progress, and represents the current state of
> > things, superseding Kory's patches which were based in a very old
> > version of my patches - and my patches were subsequently refactored
> > and further developed about five years ago. Due to them breaking
> > mvpp2 if merged, there was no point in posting them until such time
> > that the underlying issues with PTP were resolved - and they now have
> > been.
> > 
> > Marvell re-uses their PTP IP in several of their products - PHYs,
> > switches and even some ethernet MACs contain the same IP. It really
> > doesn't make sense to duplicate the code in each of these use cases.
> > 
> > Therefore, this series introduces a Marvell PTP core that can be
> > re-used - a TAI module, which handles the global parts of the PTP
> > core, and the TS module, which handles the per-port timestamping.
> > 
> > I will note at this point that although the Armada 388 TRM states that
> > NETA contains the same IP, attempts to access the registers returns
> > zero, and it is not known if that is due to the board missing something
> > or whether it isn't actually implemented. I do have some early work
> > re-using this, but when I discovered that the TAI registers read as
> > zero and wouldn't accept writes, I haven't progressed that.
> > 
> > Today, I have converted the mv88e6xxx DSA code to use the Marvell TAI
> > module from patch 1, and for the sake of getting the code out there,
> > I have included the "hacky" patches in this series - with the issues
> > with DSA VLANs that I reported this evening and subsequently
> > investigated, I've not had any spare time to properly prepare that
> > part of this series. (Being usurped from phylink by stmmac - for which
> > I have a big stack of patches that I can't get out because of being
> > usurped, and then again by Marvell PTP, and then again by DSA VLAN
> > stuff... yea, I'm feeling like I have zero time to do anything right
> > now.) The mv88e6xxx DSA code still needs to be converted to use the
> > Marvell TS part of patch 1, but I won't be able to test that after
> > Sunday, and I'm certainly not working on this over this weekend.
> > 
> > Anyway, this is what it is - and this is likely the state of it for
> > a while yet, because I won't be able to sensibly access the hardware
> > for testing for an undefined period of time.
> > 
> > The PHY parts seem to work, although not 100% reliably, with the
> > occasional overrun, particularly on the receive side. I'm not sure
> > whether this is down to a hardware bug or not, or MDIO driver bug,
> > because we certainly aren't missing timestamping a SKB. This has been
> > tested at L2 and L4.
> > 
> > I'm not sure which packets we should be timestamping (remembering
> > that this is global config across all ports.)
> > https://chronos.uk/wordpress/wp-content/uploads/TechnicalBrief-IEEE1588v2PTP.pdf
> > suggests Sync, Delay_req and Delay_resp need to be timestamped,
> > possibly PDelay_req and PDelay_resp as well, but I haven't seen
> > those produced by PTPDv2 nor ptp4l.
> > 
> > There's probably other stuff I should mention, but as I've been at
> > this into the evening for almost every day this week, I'm mentally
> > exhausted.
> > 
> > Sorry also if this isn't coherent.  
> 
> I've just updated this series for the supported pins flags that was
> merged into net-next last night.
> 
> Kory, if you have any changes you want me to review before sending
> out the updates, please send soon. Thanks.

Maybe using kdoc in marvell_ts.c. You can copy paste the ones I wrote on the v2
of your driver I posted.

Also don't forget to run checkpatch, patchwork reported errors on your series.
It was not an issue as it was a RFC but it will if you remove it.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Powered by blists - more mailing lists