From mboxrd@z Thu Jan 1 00:00:00 1970 From: Niklas Cassel Subject: Re: [PATCH] net: phy: at803x: disable delay only for RGMII mode Date: Thu, 14 Feb 2019 13:39:22 +0100 Message-ID: <20190214123922.GA28897@centauri.ideon.se> References: <20190212141922.12849-1-vkoul@kernel.org> <20190213131206.GA460@centauri.lan> <20190213132900.GA24589@lunn.ch> <1ab5edac-a36c-9dc5-52e5-dbd3b70e7728@free.fr> <20190213174034.GA6954@centauri.lan> <3356ff05-8d08-591e-03bf-9d846f79097b@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Return-path: Content-Disposition: inline In-Reply-To: <3356ff05-8d08-591e-03bf-9d846f79097b@ti.com> Sender: netdev-owner@vger.kernel.org To: Peter Ujfalusi Cc: Marc Gonzalez , Andrew Lunn , Florian Fainelli , Vinod Koul , David S Miller , linux-arm-msm@vger.kernel.org, Bjorn Andersson , netdev@vger.kernel.org, "Nori, Sekhar" List-Id: linux-arm-msm@vger.kernel.org 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