Received-SPF: pass (google.com: domain of linux-kernel+bounces-64116-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Tue, 13 Feb 2024 19:46:45 +0100
From: Andrew Lunn <andrew@lunn.ch>
To: Christian Marangi <ansuelsmth@gmail.com>
Cc: Heiner Kallweit <hkallweit1@gmail.com>,
	Russell King <linux@armlinux.org.uk>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Robert Marko <robimarko@gmail.com>,
	"Russell King (Oracle)" <rmk+kernel@armlinux.org.uk>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [net-next RFC PATCH 0/2] net: phy: aquantia: fix system
 interface provision
Message-ID: <ee00b11a-4679-4ba4-be42-10f15d5e9f65@lunn.ch>
References: <20240213182415.17223-1-ansuelsmth@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240213182415.17223-1-ansuelsmth@gmail.com>

On Tue, Feb 13, 2024 at 07:24:10PM +0100, Christian Marangi wrote:
> Posting this as RFC as I think this require some discussion on the topic.
> 
> There is currently a problem. OEM multiple time provision Aquantia FW
> with random and wrong data that may apply for one board but doesn't for
> another. And at the same time OEM use the same broken FW for multiple
> board and apply fixup at runtime.
> 
> This is the common case for AQR112 where downstream (uboot, OEM sdk,
> openwrt to have the port correctly working) hack patch are used to fixup
> broken system interface provision from the FW.
> 
> The downstream patch do one simple thing, they setup the SERDES startup
> rate (that the FW may wrongly not init) and overwrite the
> global system config for each rate to default values for the rwquested PHY
> interface.
> 
> Now setting the SERDES startup value is SAFE, and this can be implemented
> right away.
> 
> Overwriting the SERDES modes for each rate tho might pose some question
> on how this is correct or wrong.
> 
> Reality is that probably every user an Aquantia PHY in one way or another
> makes use of the SDK and have this patch in use making any kind of
> provision on the FW ignored, (since the default values are always applied
> at runtime) making the introduction of this change safe and restoring
> correct functionality of AQR112 in the case of a broken FW loaded.

This is part of the discussion i had with Aquantia about
provisioning. Basically, you cannot trust any register to contain a
known value, e.g the value the data sheet indicates the reset value
should be, or that the 802.3 standard says it should be.

So in effect, the driver needs to write every single register it
depends on.

> This might be the safest change but again would not give us 100% idea that
> the thing provision by the FW are correct.

I would say, we have to assume provision is 100% wrong. Write every
single register with the needed value.

Is the provisioning information available? Can it be read from the
flash? Can it be dumped from firmware we have on disk? Dumping it for
a number of devices could give a list of register values which are
highly suspect, ones that OEMs typically mess with. We could start by
always setting those registers.

       Andrew