LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Serge Semin <firstname.lastname@example.org>
To: Martin Blumenstingl <email@example.com>
Cc: Vladimir Oltean <firstname.lastname@example.org>,
Florian Fainelli <email@example.com>,
Andrew Lunn <firstname.lastname@example.org>,
Heiner Kallweit <email@example.com>,
"David S. Miller" <firstname.lastname@example.org>,
Serge Semin <Sergey.Semin@t-platforms.ru>,
Subject: Re: [PATCH v2 2/2] net: phy: realtek: Change TX-delay setting for RGMII modes only
Date: Mon, 6 May 2019 17:39:07 +0300 [thread overview]
Message-ID: <20190506143906.o3tublcxr5ge46rg@mobilestation> (raw)
On Tue, Apr 30, 2019 at 11:16:21PM +0200, Martin Blumenstingl wrote:
> Hello Serge,
> On Mon, Apr 29, 2019 at 11:12 PM Serge Semin <email@example.com> 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:
> > > > > https://patchwork.kernel.org/patch/9447581/
> > > > > 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)
Yeah. This is a standard problem if you ever worked with a hardware just
designed, when you try to make MAC+PHY working together. If you experienced
packets loss and it's RGMII, then most likely the problem with delays.
> > > > > 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)
Nah, this isn't going to work since the config register is placed on an extension
page. So in order to reach the register first I needed to enable a standard page,
then select an extended page, then modify the register bits.
> 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.
Ok. Thanks for the comments. I am sure the RX-delay is configurable at list
via external RXD pin strapping at the chip powering up procedure. The only
problem with a way of software to change the setting.
I don't think there is going to be anyone revealing that realtek black boxed
registers layout anytime soon. So as I see it it's better to leave the
rtl8211f-part as is for now.
next prev parent reply other threads:[~2019-05-06 15:03 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-26 9:30 [PATCH] net: phy: realtek: Add rtl8211e rx/tx delays config Serge Semin
2019-04-26 13:28 ` Andrew Lunn
2019-04-26 19:19 ` Serge Semin
2019-04-26 20:05 ` Andrew Lunn
2019-04-26 20:28 ` Serge Semin
2019-04-26 17:17 ` Heiner Kallweit
2019-04-26 20:26 ` Serge Semin
2019-04-26 21:21 ` [PATCH v2 1/2] " Serge Semin
2019-04-26 21:40 ` Andrew Lunn
2019-04-26 23:45 ` Serge Semin
2019-04-27 3:11 ` Florian Fainelli
2019-04-27 7:44 ` Serge Semin
2019-04-27 15:21 ` Andrew Lunn
2019-04-28 19:19 ` Serge Semin
2019-04-27 19:20 ` Florian Fainelli
2019-05-08 1:29 ` [PATCH v3 0/2] net: phy: realtek: Fix RGMII TX/RX-delays initial config of rtl8211(e|f) Serge Semin
2019-05-08 1:29 ` [PATCH v3 1/2] net: phy: realtek: Add rtl8211e rx/tx delays config Serge Semin
2019-05-08 1:29 ` [PATCH v3 2/2] net: phy: realtek: Change TX-delay setting for RGMII modes only Serge Semin
2019-05-08 16:37 ` [PATCH v3 0/2] net: phy: realtek: Fix RGMII TX/RX-delays initial config of rtl8211(e|f) David Miller
2019-05-08 21:51 ` [PATCH v4 " Serge Semin
2019-05-08 21:51 ` [PATCH v4 1/2] net: phy: realtek: Add rtl8211e rx/tx delays config Serge Semin
2019-05-08 21:51 ` [PATCH v4 2/2] net: phy: realtek: Change TX-delay setting for RGMII modes only Serge Semin
2019-05-08 23:31 ` [PATCH v4 0/2] net: phy: realtek: Fix RGMII TX/RX-delays initial config of rtl8211(e|f) David Miller
2019-05-13 5:41 ` [PATCH v2 1/2] net: phy: realtek: Add rtl8211e rx/tx delays config Guenter Roeck
2019-05-13 10:37 ` Serge Semin
2019-04-26 21:21 ` [PATCH v2 2/2] net: phy: realtek: Change TX-delay setting for RGMII modes only Serge Semin
2019-04-26 21:46 ` Andrew Lunn
2019-04-26 23:35 ` Serge Semin
2019-04-29 17:37 ` Florian Fainelli
2019-04-29 18:29 ` Vladimir Oltean
2019-04-29 21:12 ` Serge Semin
2019-04-29 22:36 ` Vladimir Oltean
2019-04-30 12:54 ` Serge Semin
2019-04-30 20:44 ` Martin Blumenstingl
2019-05-08 0:48 ` Serge Semin
2019-04-30 21:16 ` Martin Blumenstingl
2019-05-01 23:03 ` Vladimir Oltean
2019-05-03 17:29 ` Martin Blumenstingl
2019-05-06 14:39 ` Serge Semin [this message]
2019-05-06 17:21 ` Martin Blumenstingl
2019-05-07 17:37 ` Heiner Kallweit
2019-05-07 20:09 ` Martin Blumenstingl
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 \
--subject='Re: [PATCH v2 2/2] net: phy: realtek: Change TX-delay setting for RGMII modes only' \
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).