Re: [v3 2/2] dsa: mv88e6xxx: make serdes SGMII/Fiber output amplitude configurable

From: "Marek Behún" <kabel@kernel.org>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Vladimir Oltean <olteanv@gmail.com>,
	Holger Brunck <holger.brunck@hitachienergy.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Jakub Kicinski <kuba@kernel.org>
Subject: Re: [v3 2/2] dsa: mv88e6xxx: make serdes SGMII/Fiber output amplitude configurable
Date: Wed, 8 Dec 2021 19:15:54 +0100	[thread overview]
Message-ID: <20211208191554.3ac7fd0b@thinkpad> (raw)
In-Reply-To: <YbDxpJH3GgPDge+O@lunn.ch>

On Wed, 8 Dec 2021 18:55:48 +0100
Andrew Lunn <andrew@lunn.ch> wrote:

> On Wed, Dec 08, 2021 at 06:36:26PM +0100, Marek Behún wrote:
> > On Wed, 8 Dec 2021 19:19:09 +0200
> > Vladimir Oltean <olteanv@gmail.com> wrote:
> >   
> > > On Wed, Dec 08, 2021 at 06:00:57PM +0100, Marek Behún wrote:  
> > > > > Also, maybe drop the "serdes-" prefix? The property will sit under a
> > > > > SERDES lane node, so it would be a bit redundant?    
> > > > 
> > > > Hmm. Holger's proposal adds the property into the port node, not SerDes
> > > > lane node. mv88e6xxx does not define bindings for SerDes lane nodes
> > > > (yet).    
> > > 
> > > We need to be careful about that. You're saying that there chances of
> > > there being a separate SERDES driver for mv88e6xxx in the future?  
> > 
> > I don't think so. Although Russell is working on rewriting the SerDes
> > code to new Phylink API, the SerDes code will always be a part of
> > mv88e6xxx driver, I think.  
> 
> In theory, the 6352 family uses standard c22 layout for its SERDES. It
> might be possible to use generic code for that. But given the
> architecture, i expect such a change would have the mv88e6xxx
> instantiate such generic code, not use an external device.
> 
> For the 6390 family the SERDES and the MAC are pretty intertwined, and
> it is not a 1:1 mapping. It might be possible to make use of shared
> code, but i've much doubt it will be a separate device.
> 
> I would put the properties in the port nodes, next to phy-mode,
> phy-handle, etc.
> 
> Where it might get interesting is the 10G modes, where there are 4
> lanes. Is it possible to configure the voltage for each lane? Or is it
> one setting for all 4 lanes? I've not looked at the data sheet, so i
> cannot answer this.
> y
>     Andrew

The FS for PHY and Serdes for 6390X does not document TX amplitude
registers. Release notes document some additional registers, or mention
how to change frequency, but do not document the registers explicitly.

So we don't know currently how to change TX amplitude on those
switches. But I guess I could find out the same way I found out about
88E6393X frequency change from undocumented register. Or if some vendor
needs it, they can ask Marvell which registers they should use to
change TX amplitude.

I personally don't have any device with these switches though.

Marek