Re: [PATCH net-next 2/3] dt-bindings: net: micrel: Configure latency values and timestamping check for LAN8814 phy

[Allan W. Nielsen]

Hi Richard,

On Fri, Mar 11, 2022 at 07:08:42AM -0800, Richard Cochran wrote:
> On Fri, Mar 11, 2022 at 03:28:14PM +0100, Horatiu Vultur wrote:
> 
> > What about adding only some sane values in the driver like here [1].
> > And the allow the user to use linuxptp to fine tune all this.
> 
> I mean, that is the point.  Users will surely have to tune it
> themselves, second guessing the driver in any case.  So having hard
> coded constants in the driver is useless.
> 
> Probably even the tuned values will differ by link speed, so having
> the per-link speed constants in the driver doesn't help either.
> 
> (And yes, linuxptp should offer configuration variables per link
> speed, monitor actual link speed, and switch automatically.  So far no
> one is demanding that loudly)

I did skim through the articles, and as you hinted he does find small
latency differences across different packets. (but as I understood, very
few PHYs was tested).

Also, I know that we (Vitesse -> Microsemi -> Microchip) have been
offering ways to calibrate the individual PHYs in other PTP-SW products.
So, this makes good sense.

With this in mind, I do agree with you that it does not make much sense
to compensate they few cm of PCB tracks without also calibrating for
differences from packet to packet.

But this is not really an argument for not having _default_ values
hard-coded in the driver (or DT, but lets forget about DT for now).
Looking at the default compensation numbers provided in the driver, they
are a lot larger than what we expect to find in calibration.

As pointed out by other, most users will not care about the small error
introduced by the few cm PCB track. My claim is that if we provide
default hard-coded delay values in the driver, most users will not care
about the few ns noise that each packet differs. And those who do care,
have all the hooks and handle to calibrate further by using PTP4L.

If we do not offer default delays directly in the driver, everybody will
have to calibrate all boards just to have decent results, we will not
have a good way to provide default delay numbers, and this will be
different from what is done in other drivers.

I do understand that you have a concern that these numbers may change in
future updates. But this has not been a problem in other drivers doing
the same. But if this is still a concern, we can add a comment to say
that these numbers must be treated as UAPI, and chancing them, may
cause regressions on calibrated PHYs.

Long story short, I can see any real down-sides of adding these delay
numbers, and I see plenty in not doing so.

/Allan