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
