Netdev Archive on lore.kernel.org
 help / color / Atom feed
From: Arnd Bergmann <arnd@kernel.org>
To: Andrew Lunn <andrew@lunn.ch>
Cc: "Jernej Škrabec" <jernej.skrabec@gmail.com>,
	"Ard Biesheuvel" <ardb@kernel.org>,
	"Daniel Thompson" <daniel.thompson@linaro.org>,
	"Sumit Garg" <sumit.garg@linaro.org>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"Masami Hiramatsu" <mhiramat@kernel.org>,
	"Steve McIntyre" <steve@einval.com>,
	"open list:BPF JIT for MIPS (32-BIT AND 64-BIT)"
	<netdev@vger.kernel.org>, "Willy Liu" <willy.liu@realtek.com>,
	"David S. Miller" <davem@davemloft.net>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Sasha Levin" <sashal@kernel.org>,
	"Florian Fainelli" <f.fainelli@gmail.com>,
	"Heiner Kallweit" <hkallweit1@gmail.com>,
	"Masahisa Kojima" <masahisa.kojima@linaro.org>,
	"Ilias Apalodimas" <ilias.apalodimas@linaro.org>
Subject: Re: Re: realtek PHY commit bbc4d71d63549 causes regression
Date: Fri, 13 Nov 2020 22:26:53 +0100
Message-ID: <CAK8P3a3ABKRYg_wyjz_zUPd+gE1=f3PsVs5Ac-y1jpa0+Kt1fA@mail.gmail.com> (raw)
In-Reply-To: <20201113165625.GN1456319@lunn.ch>

On Fri, Nov 13, 2020 at 5:57 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > > Hi Arnd
> > >
> > > This PHY driver bug hiding DT bug is always hard to handle. We have
> > > been though it once before with the Atheros PHY. All the buggy DT
> > > files were fixed in about one cycle.
> >
> > Do you have a link to the problem for the Atheros PHY?
>
> commit cd28d1d6e52e740130745429b3ff0af7cbba7b2c
> Author: Vinod Koul <vkoul@kernel.org>
> Date:   Mon Jan 21 14:43:17 2019 +0530
>
>     net: phy: at803x: Disable phy delay for RGMII mode
>
>     For RGMII mode, phy delay should be disabled. Add this case along
>     with disable delay routines.
>
>     Signed-off-by: Vinod Koul <vkoul@kernel.org>
>     Signed-off-by: David S. Miller <davem@davemloft.net>
>
> and
>
> commit 6d4cd041f0af5b4c8fc742b4a68eac22e420e28c
> Author: Vinod Koul <vkoul@kernel.org>
> Date:   Thu Feb 21 15:53:15 2019 +0530
>
>     net: phy: at803x: disable delay only for RGMII mode
>
>     Per "Documentation/devicetree/bindings/net/ethernet.txt" RGMII mode
>     should not have delay in PHY whereas RGMII_ID and RGMII_RXID/RGMII_TXID
>     can have delay in PHY.
>
>     So disable the delay only for RGMII mode and enable for other modes.
>     Also treat the default case as disabled delays.
>
>     Fixes: cd28d1d6e52e: ("net: phy: at803x: Disable phy delay for RGMII mode")
>
> Looking at the git history, it seems like it also took two attempts to
> get it working correctly, but the time between the two patches was
> much shorted for the atheros PHY.

ok, thanks.

> > I agree this makes the problem harder, but I have still hope that
> > we can come up with a code solution that can deal with this
> > one board that needs to have the correct settings applied as well
> > as the others on which we have traditionally ignored them.
> >
> > As I understand it so far, the reason this board needs a different
> > setting is that the strapping pins are wired incorrectly, while all
> > other boards set them right and work correctly by default. I would
> > much prefer a way to identify this behavior in dts and have the phy
> > driver just warn when it finds a mismatch between the internal
> > delay setting in DT and the strapping pins but keep using the
> > setting from the strapping pins when there is a conflict.
>
> So what you are suggesting is that the pine board, and any other board
> which comes along in the future using this PHY which really wants
> RGMII, needs a boolean DT property:
>
> "realtek,IRealyDoWantRGMII_IAmNotBroken"
>
> in the PHY node?
>
> And if it is missing, we ignore when the MAC asks for RGMII and
> actually do RGMII_ID?

Something of that sort. I also see a similar patch in KSZ9031
now, see 7dd8f0ba88fc ("arm: dts: imx6qdl-udoo: fix rgmii phy-mode
for ksz9031 phy")

As this exact mismatch between rgmii and rgmii-id mode is apparently
a more widespread problem, the best workaround I can think of
is that we redefine the phy-mode="rgmii" property to actually mean
"use rgmii mode and let the phy driver decide the delay configuration",
with a new string to mean specifically rgmii mode with no delay.

That way, the existing behavior of all phy drivers would become
conformant, and we could change the one board that needs to
disable the internal delay to use the new property.

If we have three drivers that confused the meaning of
phy-mode="rgmii", there may still be others.

> We might also need to talk to the FreeBSD folks.
>
> https://reviews.freebsd.org/D13591
>
> Do we need to ask them to be bug compatible to Linux? Are the same DT
> file being used?

We are moving towards sharing the dts files better, yes.
In a lot of cases this does mean bug compatibility, the same
way that the DT support on Linux still has some quirks that
go back to bugs in MacOS or AIX on PowerPC.

> That still leaves ACPI systems. Do we want to stuff this DT property
> into an ACPI table? That seems to go against what ACPI people say
> saying, ACPI is not DT with an extra wrapper around it.

As far as I know, the only affected ACPI system has the same
issue and it should be handled the same way, also not requiring
a firmware update.

      Arnd

  reply index

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-17 14:20 Ard Biesheuvel
2020-10-17 14:44 ` Andrew Lunn
2020-10-17 14:46   ` Ard Biesheuvel
2020-10-17 15:11     ` Andrew Lunn
2020-10-17 15:18       ` Ard Biesheuvel
2020-10-17 16:14         ` Ilias Apalodimas
2020-10-17 16:17           ` Ard Biesheuvel
2020-10-17 16:21             ` Ilias Apalodimas
2020-10-17 18:04             ` Andrew Lunn
2020-10-17 18:11               ` Ard Biesheuvel
2020-10-17 18:17                 ` Ard Biesheuvel
2020-10-17 18:27                 ` Andrew Lunn
2020-10-17 18:55                   ` Ard Biesheuvel
2020-10-17 19:49                     ` Andrew Lunn
2020-10-17 22:19                       ` Ard Biesheuvel
2020-10-17 23:02                         ` Andrew Lunn
2020-10-18 10:35                           ` Ard Biesheuvel
2020-10-18 10:56                             ` Ard Biesheuvel
2020-10-18 15:45                               ` Andrew Lunn
2020-10-25 14:16                                 ` Ard Biesheuvel
2020-10-25 14:28                                   ` Andrew Lunn
2020-10-25 14:34                                     ` Ard Biesheuvel
2020-10-25 14:42                                       ` Andrew Lunn
2020-10-29 14:21                                         ` Ilias Apalodimas
2020-10-29 14:39                                           ` Andrew Lunn
2020-10-29 14:42                                             ` Ard Biesheuvel
2020-10-29 14:50                                               ` Andrew Lunn
2020-10-29 14:46                                             ` Ilias Apalodimas
2020-11-05 17:31                                               ` Jernej Škrabec
2020-11-13 13:50                                                 ` Arnd Bergmann
2020-11-13 14:44                                                   ` Andrew Lunn
2020-11-13 15:33                                                     ` Arnd Bergmann
2020-11-13 16:56                                                       ` Andrew Lunn
2020-11-13 21:26                                                         ` Arnd Bergmann [this message]
2020-11-13 22:43                                                           ` Andrew Lunn
2020-11-13 22:49                                                             ` Ard Biesheuvel
2020-11-14  0:40                                                               ` Andrew Lunn
2020-11-14 10:09                                                                 ` Ard Biesheuvel
2020-10-18 15:38                             ` Andrew Lunn
2020-10-18 14:20                         ` Masami Hiramatsu
2020-10-17 18:01           ` Andrew Lunn

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='CAK8P3a3ABKRYg_wyjz_zUPd+gE1=f3PsVs5Ac-y1jpa0+Kt1fA@mail.gmail.com' \
    --to=arnd@kernel.org \
    --cc=alex.bennee@linaro.org \
    --cc=andrew@lunn.ch \
    --cc=ardb@kernel.org \
    --cc=daniel.thompson@linaro.org \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=hkallweit1@gmail.com \
    --cc=ilias.apalodimas@linaro.org \
    --cc=jernej.skrabec@gmail.com \
    --cc=kuba@kernel.org \
    --cc=masahisa.kojima@linaro.org \
    --cc=mhiramat@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=sashal@kernel.org \
    --cc=steve@einval.com \
    --cc=sumit.garg@linaro.org \
    --cc=willy.liu@realtek.com \
    /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

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org
	public-inbox-index netdev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git