lkml.org 
[lkml]   [2020]   [May]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH net-next v3] net: phy: micrel: add phy-mode support for the KSZ9031 PHY
Hi Andrew,

On Wed, May 27, 2020 at 10:52 PM Andrew Lunn <andrew@lunn.ch> 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.

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.

Indeed...

> 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
plan:

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
values.

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
backwards-compatibility,
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>;

to:

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

or:

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
affected.
- 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?
Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

\
 
 \ /
  Last update: 2020-05-28 15:11    [W:0.081 / U:1.996 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site