From: Niklas Cassel <niklas.cassel@linaro.org>
To: Peter Ujfalusi <peter.ujfalusi@ti.com>
Cc: Marc Gonzalez <marc.w.gonzalez@free.fr>,
Andrew Lunn <andrew@lunn.ch>,
Florian Fainelli <f.fainelli@gmail.com>,
Vinod Koul <vkoul@kernel.org>,
David S Miller <davem@davemloft.net>,
linux-arm-msm@vger.kernel.org,
Bjorn Andersson <bjorn.andersson@linaro.org>,
netdev@vger.kernel.org, "Nori, Sekhar" <nsekhar@ti.com>
Subject: Re: [PATCH] net: phy: at803x: disable delay only for RGMII mode
Date: Thu, 14 Feb 2019 13:39:22 +0100 [thread overview]
Message-ID: <20190214123922.GA28897@centauri.ideon.se> (raw)
In-Reply-To: <3356ff05-8d08-591e-03bf-9d846f79097b@ti.com>
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
next prev parent reply other threads:[~2019-02-14 12:39 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-12 14:19 [PATCH] net: phy: at803x: disable delay only for RGMII mode Vinod Koul
2019-02-13 7:02 ` Peter Ujfalusi
2019-02-15 10:16 ` Vinod Koul
2019-02-13 13:12 ` Niklas Cassel
2019-02-13 13:29 ` Andrew Lunn
2019-02-13 13:40 ` Marc Gonzalez
2019-02-13 17:40 ` Niklas Cassel
2019-02-13 17:59 ` Florian Fainelli
2019-02-13 20:07 ` Niklas Cassel
2019-02-13 21:38 ` Florian Fainelli
2019-02-14 10:49 ` Peter Ujfalusi
2019-02-14 12:39 ` Niklas Cassel [this message]
2019-02-14 13:22 ` Peter Ujfalusi
2019-02-14 15:06 ` Niklas Cassel
2019-02-15 0:14 ` Florian Fainelli
2019-02-14 16:38 ` David Miller
2019-02-14 16:46 ` Marc Gonzalez
2019-02-14 17:33 ` David Miller
2019-02-15 9:58 ` Vinod Koul
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190214123922.GA28897@centauri.ideon.se \
--to=niklas.cassel@linaro.org \
--cc=andrew@lunn.ch \
--cc=bjorn.andersson@linaro.org \
--cc=davem@davemloft.net \
--cc=f.fainelli@gmail.com \
--cc=linux-arm-msm@vger.kernel.org \
--cc=marc.w.gonzalez@free.fr \
--cc=netdev@vger.kernel.org \
--cc=nsekhar@ti.com \
--cc=peter.ujfalusi@ti.com \
--cc=vkoul@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).