Re: [PATCH] phy: aquantia: Configure SERDES mode by default

From: Vladimir Oltean <olteanv@gmail.com>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Sean Anderson <sean.anderson@seco.com>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Russell King <linux@armlinux.org.uk>,
	netdev@vger.kernel.org, Eric Dumazet <edumazet@google.com>,
	Tim Harvey <tharvey@gateworks.com>,
	"David S . Miller" <davem@davemloft.net>,
	linux-kernel@vger.kernel.org, Paolo Abeni <pabeni@redhat.com>,
	Jakub Kicinski <kuba@kernel.org>
Subject: Re: [PATCH] phy: aquantia: Configure SERDES mode by default
Date: Fri, 18 Nov 2022 19:16:43 +0200	[thread overview]
Message-ID: <20221118171643.vu6uxbnmog4sna65@skbuf> (raw)
In-Reply-To: <Y3bLlUk1wxzAqKmj@lunn.ch>

On Fri, Nov 18, 2022 at 01:02:29AM +0100, Andrew Lunn wrote:
> > Well, part of my goal in sending out this patch is to get some feedback
> > on the right thing to do here. As I see it, there are three ways of
> > configuring this phy:
> > 
> > - Always rate adapt to whatever the initial phy interface mode is
> > - Switch phy interfaces depending on the link speed
> > - Do whatever the firmware sets up
> 
> My understanding of the aQuantia firmware is that it is split into two
> parts. The first is the actual firmware that runs on the PHY. The
> second is provisioning, which seems to be a bunch of instructions to
> put value X in register Y. It seems like aQuantia, now Marvell, give
> different provisioning to different customers.
> 
> What this means is, you cannot really trust any register contains what
> you want, that your devices does the same as somebody elses' device in
> its reset state.
> 
> So i would say, "Do whatever the firmware sets up" is the worst
> choice. Assume nothing, set every register which is important to the
> correct value.

If "do whatever the firmware sets up" is the worst choice, it means you
think it's worse than "doing whatever the firmware sets up, except a few
fixups here and there which worked on my board". Whereas I think _that's_
actually even worse.

What might be an even bigger offence than giving different provisioning
to different customers is giving different documentation to different
customers. In the Aquantia Register Specification for Gen4 PHYs given
to NXP, the SerDes mode field in register 1E.31C cannot even _take_ the
value of 6. They're all documented only from 0 to 5. I only learned that
6 (XFI/2) was a thing from the discussion between Sean and Tim.