lkml.org 
[lkml]   [2020]   [May]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH net-next v2 4/4] net: dp83869: Add RGMII internal delay configuration
From
Date
Andrew

On 5/23/20 5:07 PM, Andrew Lunn wrote:
>>> Any why is your PHY special, in that is does care about out of range
>>> delays, when others using new the new core helper don't?
>> We are not rounding to nearest here.  Basically the helper works to find the
>> best match
>>
>> If the delay passed in is less than or equal to the smallest delay then
>> return the smallest delay index
>>
>> If the delay passed in is greater then the largest delay then return the max
>> delay index
> + /* Find an approximate index by looking up the table */
> + if (delay > delay_values[i - 1] &&
> + delay < delay_values[i]) {
> + if (delay - delay_values[i - 1] < delay_values[i] - delay)
> + return i - 1;
> + else
> + return i;
>
> This appears to round to the nearest value when it is not an exact
> match.
>
> The documentation is a hint to the DT developer what value to put in
> DT. By saying it rounders, the developer does not need to go digging
> through the source code to find an exact value, otherwise -EINVAL will
> be returned. They can just use the value the HW engineer suggested,
> and the PHY will pick whatever is nearest.
>
>> Not sure what you mean about this PHY being special.  This helper is
>> not PHY specific.
> As you said, if out of range, the helper returns the top/bottom
> value. Your PHY is special, the top/bottom value is not good enough,
> you throw an error.
>
> The point of helpers is to give uniform behaviour. We have one line
> helpers, simply because they give uniform behaviour, rather than have
> each driver do it subtlety different. But it also means drivers should
> try to not add additional constraints over what the helper already
> has, unless it is actually required by the hardware.
>
>> After I think about this more I am thinking a helper may be over kill here
>> and the delay to setting should be done within the PHY driver itself
> The helper is useful, it will result in uniform handling of rounding
> between DT values and what the PHY can actually do. But please also
> move your range check and error message inside the helper.


I re-worked v3 to be a bit more of a helper and incorporated Florian's
and you comments

Dan


> Andrew

\
 
 \ /
  Last update: 2020-05-26 19:49    [W:0.040 / U:0.052 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site