On Thu, Jan 04, 2018 at 08:00:53AM +0100, Heiner Kallweit wrote: > Parameter mask of phy_modify() holds the bits to be cleared. > In the mentioned commit parameter mask seems to be inverted in > few cases, what IMO is wrong (see example).
I'd be grateful if you could list those that you think are wrong please. > Maybe I miss something, could you please check? It's entirely possible that some are wrong - the patch started out as having the mask argument inverted, but during its evolution, that was corrected, and I thought all places had been updated - maybe they were initially wrong. I did go through the patch several times before sending it to try to ensure that it was correct, but must have overlooked some, because the one you quote is one I definitely looked at several times. It's highly likely that if I have another look through the patch, I still won't spot those that you've found. > And somehow related: > When adding such helpers, wouldn't it make sense to add > helpers for setting / clearing bits too? Something like: > phy_set_bits(phydev, reg, val) -> phy_modify(phydev, reg, 0, val) Maybe, but lets try and solve the problems with the existing patch first. Thanks for reporting this, and sorry for the hassle. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up
