Subject: Re: [RFC] option to use proper skew timings for Micrel KSZ9021

From: Andrew Lunn <[email-masked]>
Date: Fri, 11 Nov 2022 20:35:32 +0100

On Fri, Nov 11, 2022 at 07:03:08PM +0000, Ian Abbott wrote:
> On 11/11/2022 17:53, Andrew Lunn wrote:
> > > > And since you are adding more foot guns, please validate the values in
> > > > DT as strictly as possible, without breaking the existing binding.
> > > 
> > > Yes, some min/max clamping of skew values would be good.  The code for
> > > KSZ9131 does that already.
> > 
> > I would want much more strict checking than that. The old and the new
> > values probably don't intersect. So if you see an old value while
> > micrel,skew-equals-real-picoseconds is in force, fail the probe with
> > -EINVAL. It looks like the old binding silently preforms rounding to
> > the nearest delay. So you probably should not do the opposite, error
> > out for a new value when micrel,skew-equals-real-picoseconds is not in
> > force. But you can add range checks. A negative value is clearly wrong
> > for the old values and should be -EINVAL. You just need to watch out
> > for that the current code reads the values as u32, not s32, so you
> > won't actually see a negative value.
> 
> I'm not sure how to tell old values and new values apart (except for
> negative new values).  A divisibility test won't work for values that are
> divisible by 600 (lcm(120, 200)).

I might have this wrong, but i think they are:

 Old	New
===== ======
0	-840
200	-720
400	-600
600	-480
800	-360
1000	-240
1200	-120
1400	0
1600	120
1800	240
2000	360
2200	480
2400	600
2600	720
2800	840
3000	960

The only overlap is 0 and 600. You can just special case those two
values.

	Andrew