On Thu, Feb 14, 2019 at 12:49:36PM +0200, Peter Ujfalusi wrote: > Hi Niklas, > > On 13/02/2019 19.40, Niklas Cassel wrote: > > On Wed, Feb 13, 2019 at 02:40:18PM +0100, Marc Gonzalez wrote: > >> On 13/02/2019 14:29, Andrew Lunn wrote: > >> > >>>> So we have these modes: > >>>> > >>>> PHY_INTERFACE_MODE_RGMII: TX and RX delays disabled > >>>> PHY_INTERFACE_MODE_RGMII_ID: TX and RX delays enabled > >>>> PHY_INTERFACE_MODE_RGMII_RXID: RX delay enabled, TX delay disabled > >>>> PHY_INTERFACE_MODE_RGMII_TXID: TX delay enabled, RX delay disabled > >>>> > >>>> What I don't like with this patch, is that if we specify phy-mode > >>>> PHY_INTERFACE_MODE_RGMII_TXID, this patch will enable TX delay, > >>>> but RX delay will not be explicitly set. > >>> > >>> That is not the behaviour we want. It is best to assume the device is > >>> in a random state, and correctly enable/disable all delays as > >>> requested. Only leave the hardware alone if PHY_INTERFACE_MODE_NA is > >>> used. > >> > >> That's what my patch did: > >> https://www.spinics.net/lists/netdev/msg445053.html > >> > >> But see Florian's remarks: > >> https://www.spinics.net/lists/netdev/msg445133.html > > > > Hello Marc, > > > > I saw that comment from Florian. However that was way back in 2017. > > Maybe the phy-modes were not as well defined back then? > > > > Andrew recently suggested to fix the driver so that it conforms with the > > phy-modes, and fix any SoC that specified an incorrect phy-mode in DT > > and thus relied upon the broken behavior of the PHY driver: > > https://www.spinics.net/lists/netdev/msg445133.html > > > > > > So, I've rebased your old patch, see attachment. > > I suggest that Peter test it on am335x-evm. > > with the patch + s/rgmii-txid/rgmii-id in the am335x-evmsk.dts ethernet > is working. > I don't have am335x-evm to test, but it has the same PHY as evmsk. > Florian's concern was that this PHY driver looked at "phy-mode" from the perspective of the MAC rather than the PHY. However, if s/rgmii-txid/rgmii-id is the correct fix for am335x-evm, then this means that this PHY driver was just broken. If the driver had misinterpreted the perspective, then the correct fix for am335x-evm would have been s/rgmii-txid/rgmii-rxid. So considering that this driver seems to be really broken (rather then just inverted perspective), perhaps we can merge the patch I attached in my previous email after all? (Together with a s/rgmii-txid/rgmii-id in the am335x-evmsk.dts.) Kind regards, Niklas > > am335x-evm appears to rely on the current broken behavior of the PHY > > driver, so we will probably need to fix the am335x-evm according to this: > > https://www.spinics.net/lists/netdev/msg445117.html > > and merge that as well. > > > > > > Andrew, Florian, do you both agree? > > > > > > Kind regards, > > Niklas > > > > - Péter > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
