On Sun, May 17, 2020 at 10:51:50PM +0200, Andrew Lunn wrote:
> > > +static int marvell_vct5_amplitude_distance(struct phy_device *phydev,
> > > + int meters)
> > > +{
> > > + int mV_pair0, mV_pair1, mV_pair2, mV_pair3;
> > > + int distance;
> > > + u16 reg;
> > > + int err;
> > > +
> > > + distance = meters * 1000 / 805;
> >
> > With this integer based meters representation, it seems to me that we
> > are artificially reducing the resolution of the distance sampling.
> > For a 100 meter cable, the Marvell implementation looks to support 124
> > sample points. This could result in incorrect data reporting as two
> > adjacent meter numbers would resolve to the same disatance value
> > entered into the register. (eg - 2 meters = 2 distance 3 meters = 2
> > distance)
> >
> > Is there a better way of doing this which would allow for userspace to
> > use the full resolution of the hardware?
>
> Hi Chris
>
> I don't see a simple solution to this.
>
> PHYs/vendors seem to disagree about the ratio. Atheros use
> 824. Marvell use 805. I've no idea what Broadcom, aQuantia uses. We
> would need to limit the choice of step to multiples of whatever the
> vendor picks as its ratio. If the user picks a step of 2m, the driver
> needs to return an error and say sorry, please try 2.488 meter steps
> for Marvell, 2.427 meter steps on Atheros, and who knows what for
> Broadcom. And when the user requests data just for 1-25 meters, the
> driver needs to say sorry, try again with 0.824-24.62, or maybe
> 0.805-24.955. That is not a nice user experience.
How about this?
- user would use meters as unit on ethtool command line but non-integer
values like "2.4" would be allowed
- request message would use e.g. cm (for consistency with existing cable
test results)
- driver would round requested values to closest supported (you actually
already round them)
- optionally, actual values used could be returned in request reply or
start notification
Michal
