Re: [PATCH net-next v2] net: phy: broadcom: 1588 support on bcm54210pe

From: Lasse Johnsen <lasse@timebeat.app>
To: Jonathan Lemon <jonathan.lemon@gmail.com>
Cc: Richard Cochran <richardcochran@gmail.com>,
	netdev@vger.kernel.org,
	Gordon Hollingworth <gordon@raspberrypi.com>,
	Ahmad Byagowi <clk@fb.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Russell King <linux@armlinux.org.uk>,
	bcm-kernel-feedback-list@broadcom.com,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>
Subject: Re: [PATCH net-next v2] net: phy: broadcom: 1588 support on bcm54210pe
Date: Fri, 22 Apr 2022 19:11:12 +0100	[thread overview]
Message-ID: <567C8D9F-BF2B-4DE6-8991-DB86A845C49C@timebeat.app> (raw)
In-Reply-To: <20220422152209.cwofghzr2wyxopek@bsd-mbp.local>

Hi Jonathan,

I suspect you make the conflation I also made when I started working on this PHY driver. Broadcom has a number of different, nearly identical chips. The BCM54210, the BCM54210E, the BCM54210PE, the BCM54210S and the BCM54210SE.

It’s hard to imagine, but only the BCM54210PE is a first generation PHY and the BCM54210 (and others) are second generation. I have to be mighty careful not to breach my NDA, but I can furnish you with these quotes directly from the Broadcom engineers I worked with during the development:

24 March:

"The BCM54210PE is the first-gen 40-nm GPHY, but the BCM54210 is the second-gen 40-nm GPHY.”

"The 1588 Inband function only applied to BCM54210 or later PHYs. It doesn't be supported in the BCM54210PE”

So, I quite agree with you that in-band would be preferable (subject to the issue with hawking the reserved field used in 1588-2019 I described in my note to Richard), but I am convinced that it is not supported in the BCM54210PE. Indeed if you are looking at a document describing features based on the RDB register access method it is not supported by the BCM54210PE.

I would like nothing better than to be wrong, but you will need to provide me with something substantial to investigate further. (Offline is NDA requires it - happy to discuss any time).

In any event, I’m sure the time is not wasted and will be relevant when the Raspberry PI CM5,6&7 is launched… :-)

Thank you for your note and all the best,

Lasse

> On 22 Apr 2022, at 16:22, Jonathan Lemon <jonathan.lemon@gmail.com> wrote:
> 
> On Fri, Apr 22, 2022 at 04:08:18PM +0100, Lasse Johnsen wrote:
>>> On 21 Apr 2022, at 15:48, Richard Cochran <richardcochran@gmail.com> wrote:
>>> Moreover: Does this device provide in-band Rx time stamps?  If so, why
>>> not use them?
>> 
>> This is the first generation PHY and it does not do in-band RX. I asked BCM and studied the documentation. I’m sure I’m allowed to say, that the second generation 40nm BCM PHY (which - "I am not making this up" is available in 3 versions: BCM54210, BCM54210S and BCM54210SE - not “PE”) - supports in-band rx timestamps. However, as a matter of curiosity, BCM utilise the field in the header now used for minor versioning in 1588-2019, so in due course using this silicon feature will be a significant challenge.
> 
> Actually, it does support in-band RX timestamps.  Doing this would be
> cleaner, and you'd only need to capture TX timestamps.
> -- 
> Jonathan