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

[Holger Brunck <holger.brunck@hitachienergy.com>]

> >
> > > > 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 <andrew@lunn.ch>
> > > > CC: Jakub Kicinski <kuba@kernel.org>
> > > > CC: Marek Behún <kabel@kernel.org>
> > > > Signed-off-by: Holger Brunck <holger.brunck@hitachienergy.com>
> > >
> > > 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?
> 

ok I see. So if I follow your proposal in my case it would be something like:
serdes-sgmii-tx-amplitude-millivolt to start with ? 

I can do that. Andrew what do you think?

Best regards
Holger