From: Geert Uytterhoeven <email@example.com>
To: Andrew Lunn <firstname.lastname@example.org>
Cc: Oleksij Rempel <email@example.com>,
Philippe Schenker <firstname.lastname@example.org>,
Kazuya Mizuguchi <email@example.com>,
Grygorii Strashko <firstname.lastname@example.org>
Subject: Re: [PATCH net-next v3] net: phy: micrel: add phy-mode support for the KSZ9031 PHY
Date: Fri, 29 May 2020 12:17:36 +0200 [thread overview]
Message-ID: <CAMuHMdUXC8O8fqfDkbV+LzoPH5Jke0rZroGQiUihW31-yhGckg@mail.gmail.com> (raw)
On Thu, May 28, 2020 at 6:08 PM Andrew Lunn <email@example.com> wrote:
> On Thu, May 28, 2020 at 03:10:06PM +0200, Geert Uytterhoeven wrote:
> > On Wed, May 27, 2020 at 10:52 PM Andrew Lunn <firstname.lastname@example.org> 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..."
> > message.
> > 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.
> In general, i suggest that the PHY implements the delay, not the MAC.
> Most PHYs support it, where as most MACs don't. It keeps maintenance
> and understanding easier, if everything is the same. But there are
> cases where the PHY does not have the needed support, and the MAC does
> the delays.
I can confirm disabling the MAC delay ("phy-mode = "rgmii""), and adding
a PHY delay ("txc-skew-ps = <1500>") also fixes the slowness on
Salvator-X with R-Car M3-W ES1.0.
However, I would like to be a cit cautious here: on Ebisu with R-Car E3,
the hardware engineers advised to add "max-speed = <100>", as EtherAVB
on R-Car E3 does not support the MAC delay, and the KSZ9031 does not
allow sufficient delay, leading to unreliable communication.
Nevertheless, I never had problems without that limitation, and 1 Gbps
still seems to work after removing it, with and without "txc-skew-ps =
> > 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.
> Do you actually need MAC internal delays?
Given the Ebisu issue, I think we do.
Note that the EtherAVB MAC TX delay, when enabled, is 2.0 ns, and
KSZ9031 supports 0..1860 ps, with 900 ps being the centerpoint, so AFAIU
that is -900..960 ps, i.e. much less than 2.0 ns.
> > To fix the issue, I came up with the following problem statement and
> > plan:
> > A. Old behavior:
> > 1. ravb acts upon "rgmii-*id" (on SoCs that support it),
> > 2. ksz9031 ignored "rgmii-*id", using hardware defaults for skew
> > values.
> So two bugs which cancelled each other out :-)
> > 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:
> I don't know if it is possible, but i would prefer that ravb does
> nothing and the PHY does the delay. The question is, can you get to
> this state without more things breaking?
While that seems to work for me, the delay would be a bit too small to work
reliably, according to the hardware engineers.
Hence my proposal for C now, to fix the regressions, and D later.
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- email@example.com
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
next prev parent reply other threads:[~2020-05-29 10:17 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
2020-05-28 16:08 ` Andrew Lunn
2020-05-29 4:59 ` Oleksij Rempel
2020-05-29 10:17 ` Geert Uytterhoeven [this message]
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
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 \
* 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.