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

From: Marek Behún <[email-masked]>
Date: Wed, 8 Dec 2021 16:28:52 +0100

On Wed, 8 Dec 2021 12:29:13 +0000
Holger Brunck <[email-masked]> wrote:

> Hi Marek,
> 
> > > The mv88e6352, mv88e6240 and mv88e6176  have a serdes interface. This
> > > patch allows to configure the output swing to a desired value in the
> > > devicetree node of the port. As the chips only supports eight
> > > dedicated values we return EINVAL if the value in the DTS does not match.
> > >
> > > CC: Andrew Lunn <[email-masked]>
> > > CC: Jakub Kicinski <[email-masked]>
> > > CC: Marek Behún <[email-masked]>
> > > Signed-off-by: Holger Brunck <[email-masked]>  
> > 
> > Holger, Andrew,
> > 
> > there is another issue with this, which I only realized yesterday. What if the
> > different amplitude needs to be set only for certain SerDes modes?
> > 
> > I am bringing this up because I discovered that on Turris Mox we need to
> > increase SerDes output amplitude when A3720 SOC is connected directly to
> > 88E6141 switch, but only for 2500base-x mode. For 1000base-x, the default
> > amplitude is okay. (Also when the SOC is connected to 88E6190, the amplitude
> > does not need to be changed at all.)
> >   
> 
> on my board I have a fixed link connected with SGMII and there is no dedicated
> value given in the manual.
> 
> > I plan to solve this in the comphy driver, not in device-tree.
> > 
> > But if the solution is to be done in DTS, shouldn't there be a possibility to define
> > the amplitude for a specific serdes mode only?
> > 
> > For example
> >   serdes-2500base-x-tx-amplitude-millivolt
> > or
> >   serdes-tx-amplitude-millivolt-2500base-x
> > or
> >   serdes-tx-amplitude-millivolt,2500base-x
> > ?
> > 
> > What do you think?
> >   
> 
> in the data sheet for the MV6352 I am using there are no dedicated values stated for
> different modes at all, they can be chosen arbitrary. So in my case I think it makes
> sense to keep it as it is for now. Other driver may have other needs and may enhance
> this later on.

Hi Holger,

but the mv88e6xxx driver also drives switches that allow changing
serdes modes. There does not need be dedicated TX amplitude register for
each serdes mode, the point is that we may want to declare different
amplitudes for different modes.

So the question is: if we go with your binding proposal for the whole
mv88e6xxx driver, and in the future someone will want to declare
different amplitudes for different modes on another model, would he
need to deprecate your binding or would it be easy to extend?

Marek