lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 26 May 2020 12:48:11 -0500 From: Dan Murphy <dmurphy@...com> To: Andrew Lunn <andrew@...n.ch> CC: Florian Fainelli <f.fainelli@...il.com>, <hkallweit1@...il.com>, <davem@...emloft.net>, <robh@...nel.org>, <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org> Subject: Re: [PATCH net-next v2 4/4] net: dp83869: Add RGMII internal delay configuration 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
Powered by blists - more mailing lists