Re: [PATCH ethtool 2/2] Ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE and PHY downshift

["Allan W. Nielsen" <allan.nielsen@microsemi.com>]

On 04/11/16 12:59, Andrew Lunn wrote:
> > +++ b/ethtool.8.in
> > @@ -340,6 +340,14 @@ ethtool \- query or control network driver and hardware settings
> >  .B2 tx-lpi on off
> >  .BN tx-timer
> >  .BN advertise
> > +.HP
> > +.B ethtool \-\-set\-phy\-tunable
> > +.I devname
> > +.B3 downshift on off N
> > +.HP
> 
> I don't think there is any other option which is on|off|N. The general
> pattern would be
> 
> --set-phy-tunable downshift on|off [count N]
> 
> With count being optional.
> 
> This also allows avoiding the ambiguity of what
> 
> --set-phy-tunable downshift 0
> 
> means. To me, that means downshift after 0 retries. But it actually
> seems to mean never downshift. You can then error out when somebody
> does
> 
> --set-phy-tunable downshift on count 0
> or
> --set-phy-tunable downshift off count 42
Okay, I do not have any strong feelings about the syntax here.

We could not find any other signature which had the same need, so we have been
looking for things that was "close" to...

What is being proposed is similar to "-s [ mdix auto|on|off ]".

What you request is similar to "-e|--eeprom-dump [ raw on|off ] [ offset N ]".

So, unless there are other comments, then we will change this in the next
version.

> Please also add a few sentences about what downshift is, and what N
> means. Is it tries, or retries?
Sure.

We will wait a few days to see what other comments we get.

Thanks for reviewing.

/Allan