All of
 help / color / mirror / Atom feed
From: Geert Uytterhoeven <>
To: Andrew Lunn <>
Cc: Oleksij Rempel <>,
	Philippe Schenker <>,
	"" <>,
	"" <>,
	"" <>,
	"" <>,
	"" <>,
	"" <>,
	"" <>,
	"" <>,
	Kazuya Mizuguchi <>,
	Grygorii Strashko <>
Subject: Re: [PATCH net-next v3] net: phy: micrel: add phy-mode support for the KSZ9031 PHY
Date: Thu, 28 May 2020 15:10:06 +0200	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

Hi Andrew,

On Wed, May 27, 2020 at 10:52 PM Andrew Lunn <> wrote:
> > You may wonder what's the difference between 3 and 4? It's not just the
> > PHY driver that looks at phy-mode!
> > drivers/net/ethernet/renesas/ravb_main.c:ravb_set_delay_mode() also
> > does, and configures an additional TX clock delay of 1.8 ns if TXID is
> > enabled.
> That sounds like a MAC bug. Either the MAC insert the delay, or the
> PHY does. If the MAC decides it is going to insert the delay, it
> should be masking what it passes to phylib so that the PHY does not
> add a second delay.

And so I gave this a try, and modified the ravb driver to pass "rgmii"
to the PHY if it has inserted a delay.
That fixes the speed issue on R-Car M3-W!
And gets rid of the "*-skew-ps values should be used only with..."

I also tried if I can get rid of "rxc-skew-ps = <1500>". After dropping
the property, DHCP failed.  Compensating by changing the PHY mode in DT
from "rgmii-txid" to "rgmii-id" makes it work again.

However, given Philippe's comment that the rgmi-*id apply to the PHY
only, I think we need new DT properties for enabling MAC internal delays.

> This whole area of RGMII delays has a number of historical bugs, which
> often counter act each other. So you fix one, and it break somewhere
> else.


> In this case, not allowing skews for plain RGMII is probably being too
> strict. We probably should relax that constrain in this case, for this
> PHY driver.

That description is not quite correct: the driver expects skews for
plain RGMII only. For RGMII-*ID, it prints a warning, but still applies
the supplied skew values.

To fix the issue, I came up with the following problem statement and

A. Old behavior:

  1. ravb acts upon "rgmii-*id" (on SoCs that support it[1]),
  2. ksz9031 ignored "rgmii-*id", using hardware defaults for skew

B. New behavior (broken):

  1. ravb acts upon "rgmii-*id",
  2. ksz9031 acts upon "rgmii-*id".

C. Quick fix for v5.8 (workaround, backwards-compatible with old DTB):

  1. ravb acts upon "rgmii-*id", but passes "rgmii" to phy,
  2. ksz9031 acts upon "rgmi", using new "rgmii" skew values.

D. Long-term fix:
  1. Check if new boolean "renesas,[rt]x-delay"[2] values are
      specified in DTB.
       No: ravb acts upon "rgmii-*id", but passes "rgmii" to phy, for
       Yes: ravb enables TX clock delay of 2.0 ns and/or RX clock delay
            of 1.8 ns, based on "renesas,[rt]x-delay" values, and passes
            the unmodified interface type to phy,
  2. ksz9031 acts upon "rgmii*",
  3. Salvator-X(S) DTS makes things explicit by changing it from

        phy-mode = "rgmii-txid";
        rxc-skew-ps = <1500>;


        phy-mode = "rgmii";
        renesas,rx-delay = <false>;
        renesas,tx-delay = <true>;
        rxc-skew-ps = <1500>;


        phy-mode = "rgmii";
        renesas,rx-delay = <true>;
        renesas,tx-delay = <true>;

[2] Should we use numerical "renesas,[rt]x-delay-ps" instead?
     The only supported values are 0 and 2000 (TX) or 1800 (RX).

The ULCB boards are very similar to Salvator-X(S), so I guess they
behave the same, and are thus affected.

Unfortunately there are other boards that use R-Car Gen3 EtherAVB:
  - The Silicon Linux sub board for CAT874 (CAT875) connects EtherAVB to
    an RTL8211E PHY.  As it uses the "rgmii" mode, it should not be
  - The HiHope RZ/G2M sub board connects EtherAVB to an RTL8211E PHY.
    It uses the "rgmii-txid" mode, so it will be affacted by modifying
    the ravb driver.
  - Eagle and V3MSK connect EtherAVB to a KSZ9031, and use the "rgmii-id"
    mode with rxc-skew-ps = <1500>, so they are affected.
  - Ebisu and Draak connect EtherAVB to a KSZ9031, and use the "rgmii"
    mode with rxc-skew-ps = <1500>.  However, they're limited to 100
    Mbps, as EtherAVB on R-Car E3 and D3 do not support TX clock
    internal delay mode[1], and the delays provided by KSZ9031 clock pad
    skew were deemed insufficient.

Obviously, the affected boards will need testing (I only have
Salvator-X(S) and Ebisu).

Does the above make sense?



Geert Uytterhoeven -- There's lots of Linux beyond ia32 --

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

  reply	other threads:[~2020-05-28 13:10 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-22  7:21 [PATCH net-next v3] net: phy: micrel: add phy-mode support for the KSZ9031 PHY Oleksij Rempel
2020-04-22  8:48 ` Philippe Schenker
2020-04-22 13:39 ` Andrew Lunn
2020-04-23  2:39 ` David Miller
2020-04-28 15:28 ` Geert Uytterhoeven
2020-04-28 15:47   ` Andrew Lunn
2020-04-28 16:16     ` Philippe Schenker
2020-04-29  8:45       ` Geert Uytterhoeven
2020-04-29  9:26         ` Oleksij Rempel
2020-05-27 19:11           ` Geert Uytterhoeven
2020-05-27 20:52             ` Andrew Lunn
2020-05-28 13:10               ` Geert Uytterhoeven [this message]
2020-05-28 16:08                 ` Andrew Lunn
2020-05-29  4:59                   ` Oleksij Rempel
2020-05-29 10:17                   ` Geert Uytterhoeven
2020-05-28  8:20             ` Philippe Schenker
2020-05-28 12:51               ` Geert Uytterhoeven
2020-05-28 23:31                 ` Florian Fainelli
2020-04-29 10:02         ` Philippe Schenker
2020-05-05 18:26 ` Grygorii Strashko
2020-05-06  4:51   ` Oleksij Rempel
2020-07-10 22:36 ` Alexandre Belloni
2020-07-10 22:54   ` Andrew Lunn
2020-07-10 23:25     ` Alexandre Belloni
  -- strict thread matches above, loose matches on Subject: below --
2020-04-07  9:36 Oleksij Rempel
2020-04-07 10:57 ` Philippe Schenker
2020-04-07 11:02   ` Marc Kleine-Budde
2020-04-07 12:34     ` Philippe Schenker
2020-04-07 11:05   ` Russell King - ARM Linux admin
2020-04-07 20:13 ` David Miller

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='' \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \

* 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.