lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  PHC 
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 30 Apr 2019 23:16:21 +0200
From:   Martin Blumenstingl <>
To:     Serge Semin <>
Cc:     Vladimir Oltean <>,
        Florian Fainelli <>,
        Andrew Lunn <>,
        Heiner Kallweit <>,
        "David S. Miller" <>,
        Serge Semin <>,
        netdev <>,
Subject: Re: [PATCH v2 2/2] net: phy: realtek: Change TX-delay setting for
 RGMII modes only

 Hello Serge,

On Mon, Apr 29, 2019 at 11:12 PM Serge Semin <> wrote:
> > > > Apparently the current config_init method doesn't support RXID setting.
> > > > The patch introduced current function code was submitted by
> > > > Martin Blumenstingl in 2016:
> > > >
> > > > and was reviewed by Florian. So we'd better ask him why it was ok to mark
> > > > the RGMII_ID as supported while only TX-delay could be set.
> > > > I also failed to find anything regarding programmatic rtl8211f delays setting
> > > > in the Internet. So at this point we can set TX-delay only for f-model of the PHY.
let me give you a bit of context on that patch:
most boards (SBCs and TV boxes) with an Amlogic SoC and a Gigabit
Ethernet PHY use a Realtek RTL8211F PHY. we were seeing high packet
loss when transmitting from the board to another device.
it took us very long to understand that a combination of different
hardware and driver pieces lead to this issue:
- in the MAC driver we enabled a 2ns TX delay by default, like Amlogic
does it in their vendor (BSP) kernel
- we used the upstream Realtek RTL8211F PHY driver which only enabled
the TX delay if requested (it never disabled the TX delay)
- hardware defaults or pin strapping of the Realtek RTL8211F PHY
enabled the TX delay in the PHY

This means that the TX delay was applied twice: once at the MAC and
once at the PHY.
That lead to high packet loss when transmitting data.
To solve that I wrote the patch you mentioned, which has since been
ported over to u-boot (for a non-Amlogic related board)

> > > > Anyway lets clarify the situation before to proceed further. You are suggesting
> > > > to return an error in case if either RGMII_ID or RGMII_RXID interface mode is
> > > > requested to be enabled for the PHY. It's fair seeing the driver can't fully
> > > > support either of them.
I don't have any datasheet for the Realtek RTL8211F PHY and I'm not in
the position to get one (company contracts seem to be required for
Linux is not my main job, I do driver development in my spare time.

there may or may not be a register or pin strapping to configure the RX delay.
due to this I decided to leave the RX delay behavior "not defined"
instead of rejecting RGMII_RXID and RGMII_ID.

> > > That is how I read Andrew's suggestion and it is reasonable. WRT to the
> > > original changes from Martin, he is probably the one you would want to
> > > add to this conversation in case there are any RX delay control knobs
> > > available, I certainly don't have the datasheet, and Martin's change
> > > looks and looked reasonable, seemingly independent of the direction of
> > > this very conversation we are having.
the changes in patch 1 are looking good to me (except that I would use
phy_modify_paged instead of open-coding it, functionally it's
identical with what you have already)

I'm not sure about patch 2:
personally I would wait for someone to come up with the requirement to
use RGMII_RXID with a RTL8211F PHY.
that person will then a board to test the changes and (hopefully) a
datasheet to explain the RX delay situation with that PHY.
that way we only change the RGMII_RXID behavior once (when someone
requests support for it) instead of twice (now with your change, later
on when someone needs RGMII_RXID support in the RTL8211F driver)

that said, the change in patch 2 itself looks fine on Amlogic boards
(because all upstream .dts let the MAC generate the TX delay). I
haven't runtime-tested your patch there yet.
but there seem to be other boards (than the Amlogic ones, the RTL8211F
PHY driver discussion in u-boot was not related to an Amlogic board)
out there with a RTL8211F PHY (these may or may not be supported in
mainline Linux or u-boot and may or may not use RGMII_RXID where you
are now changing the behavior). that's not a problem by itself, but
you should be aware of this.

> rtl8211(e|f) TX/RX delays can be configured either by external pins
> strapping or via software registers. This is one of the clue to provide
> a proper config_init method code. But not all rtl8211f phys provide
> that software register, and if they do it only concerns TX-delay (as we
> aware of). So we need to take this into account when creating the updated
> versions of these functions.
> (Martin, I also Cc'ed you in this discussion, so if you have anything to
> say in this matter, please don't hesitate to comment.)
Amlogic boards, such as the Hardkernel Odroid-C1 and Odroid-C2 as well
as the Khadas VIM2 use a "RTL8211F" RGMII PHY. I don't know whether
there are multiple versions of this PHY. all RTL8211F I have seen so
far did behave exactly the same.

I also don't know whether the RX delay is configurable (by pin
strapping or some register) on RTL8211F PHYs because I don't have
access to the datasheet.


Powered by blists - more mailing lists