Messages in this thread | | | Subject | Re: [PATCH net-next v2 4/4] net: dp83869: Add RGMII internal delay configuration | From | Dan Murphy <> | Date | Tue, 26 May 2020 12:48:11 -0500 |
| |
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
| |