On Tue, 2020-04-28 at 17:47 +0200, Andrew Lunn wrote: > On Tue, Apr 28, 2020 at 05:28:30PM +0200, Geert Uytterhoeven wrote: > > This triggers on Renesas Salvator-X(S): > > > > Micrel KSZ9031 Gigabit PHY e6800000.ethernet-ffffffff:00: > > *-skew-ps values should be used only with phy-mode = "rgmii" > > > > which uses: > > > > phy-mode = "rgmii-txid"; > > > > and: > > > > rxc-skew-ps = <1500>; > > > > If I understand Documentation/devicetree/bindings/net/ethernet- > > controller.yaml > > correctly: > > Hi Geert > > Checking for skews which might contradict the PHY-mode is new. I think > this is the first PHY driver to do it. So i'm not too surprised it has > triggered a warning, or there is contradictory documentation. > > Your use cases is reasonable. Have the normal transmit delay, and a > bit shorted receive delay. So we should allow it. It just makes the > validation code more complex :-( > > Andrew Hello Geert and Andrew I reviewed Oleksij's patch that introduced this warning. I just want to explain our thinking why this is a good thing, but yes maybe we change that warning a little bit until it lands in mainline. The KSZ9031 driver didn't support for proper phy-modes until now as it don't have dedicated registers to control tx and rx delays. With Oleksij's patch this delay is now done accordingly in skew registers as best as possible. If you now also set the rxc-skew-ps registers those values you previously set with rgmii-txid or rxid get overwritten. We chose the warning to occur on phy-modes 'rgmii-id', 'rgmii-rxid' and 'rgmii-txid' as on those, with the 'rxc-skew-ps' value present, overwriting skew values could occur and you end up with values you do not wanted. We thought, that most of the boards have just 'rgmii' set in phy-mode with specific skew-values present. @Geert if you actually want the PHY to apply RXC and TXC delays just insert 'rgmii-id' in your DT and remove those *-skew-ps values. If you need custom timing due to PCB routing it was thought out to use the phy- mode 'rgmii' and do the whole required timing with the *-skew-ps values. @Andrew This warning might be not the best solution but we should definitely warn that values might get overwritten from what was intended with e.g. 'rgmii-txid'. The out-of-reset behaviour of the PHY actually is 'rgmii-txid' so we may also throw now warning if this mode is set. What is your oppinion? Regards, Philippe
