Received-SPF: pass (google.com: domain of linux-kernel+bounces-69476-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <65cfef13.7b0a0220.822e5.f9f1@mx.google.com>
Date: Sat, 17 Feb 2024 00:26:14 +0100
From: Christian Marangi <ansuelsmth@gmail.com>
To: Andrew Lunn <andrew@lunn.ch>
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
References: <20240213182415.17223-1-ansuelsmth@gmail.com>
 <ee00b11a-4679-4ba4-be42-10f15d5e9f65@lunn.ch>
 <65cbbacb.050a0220.b81d1.eb5b@mx.google.com>
 <a71e473e-bb92-4769-9f52-53de63f6a4ae@lunn.ch>
 <65cbd90d.050a0220.7cc10.ef02@mx.google.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <65cbd90d.050a0220.7cc10.ef02@mx.google.com>

On Tue, Feb 13, 2024 at 10:03:05PM +0100, Christian Marangi wrote:
> On Tue, Feb 13, 2024 at 09:58:59PM +0100, Andrew Lunn wrote:
> > > > So in effect, the driver needs to write every single register it
> > > > depends on.
> > > >
> > > 
> > > Well if that's the case then this RFC patch is a must. With a
> > > misconfigured System Interface configuration, the PHY can't comunicate
> > > with the MAC.
> > > 
> > > > > 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.
> > > >
> > > 
> > > We know where they are stored in the FW but it's not documented how the
> > > provision values are stored in the FW. (the format, how they are
> > > organized...) I can waste some time trying to reverse it and produce a
> > > tool to parse them if needed.
> > 
> > It might be worth it. How complex could it be? The obvious format is a
> > C45 mmd.reg pair and a value.
> >
> 
> Working on it. I already confirmed the FW have actually a provision part
> and is not empty.
> 
> The format looks to be u16 reg 16 value but I need to understand it
> better as not everything about provision is in mmd 1e so there must be
> some magic values to signal where the section has to be appled.
> 
> > > Would love also some comments by Russell about this, there was a patch
> > > adding support for WoL where another user was messing with these regs
> > > and he was with the idea of being careful with overwriting the provision
> > > values.
> > 
> > I expect the SERDES eye configuration is in there somewhere, and we
> > should not touch that. That was one of the arguments Aquantia made at
> > the time, that needs to be stored somewhere, and is board specific.
> > 
> > But knowing what standard 802.3 registers are commonly changed would
> > be useful, and could help track down silly problems like the
> > transmitter being disabled by default by provisioning.
> >
> 
> Yes having a tool to parse them would probably be useful and eventually
> even apply fixup in the firmware loading (if we really want)
>

As promised, I reversed the format and created a script. It's still WIP
in the sense that I have to still to find a better way to show the
values. Here the script [1].

Feel free to suggest improvements to it. Various discovery were done
while reversing this, especially the thing with the BUG.

[1] https://github.com/Ansuel/aqr_prov_table_parser

-- 
	Ansuel