mbox series

[RFC,v2,0/5] dwmac-meson8b: picosecond precision RX delay support

Message ID 20201115185210.573739-1-martin.blumenstingl@googlemail.com
Headers show
Series dwmac-meson8b: picosecond precision RX delay support | expand

Message

Martin Blumenstingl Nov. 15, 2020, 6:52 p.m. UTC
Hello,

with the help of Jianxin Pan (many thanks!) the meaning of the "new"
PRG_ETH1[19:16] register bits on Amlogic Meson G12A, G12B and SM1 SoCs
are finally known. These SoCs allow fine-tuning the RGMII RX delay in
200ps steps (contrary to what I have thought in the past [0] these are
not some "calibration" values).

The vendor u-boot has code to automatically detect the best RX/TX delay
settings. For now we keep it simple and add a device-tree property with
200ps precision to select the "right" RX delay for each board.

While here, deprecate the "amlogic,rx-delay-ns" property as it's not
used on any upstream .dts (yet). The driver is backwards compatible.

I have tested this on an X96 Air 4GB board (not upstream yet). Testing
with iperf3 gives 938 Mbits/sec in both directions (RX and TX). The
following network settings were used in the .dts (2ns TX delay
generated by the PHY, 800ps RX delay generated by the MAC as the PHY
only supports 0ns or 2ns RX delays):
        &ext_mdio {
                external_phy: ethernet-phy@0 {
                        /* Realtek RTL8211F (0x001cc916) */
                        reg = <0>;
                        eee-broken-1000t;

                        reset-assert-us = <10000>;
                        reset-deassert-us = <30000>;
                        reset-gpios = <&gpio GPIOZ_15 (GPIO_ACTIVE_LOW |
                                                GPIO_OPEN_DRAIN)>;

                        interrupt-parent = <&gpio_intc>;
                        /* MAC_INTR on GPIOZ_14 */
                        interrupts = <26 IRQ_TYPE_LEVEL_LOW>;
                };
        };

        &ethmac {
                status = "okay";

                pinctrl-0 = <&eth_pins>, <&eth_rgmii_pins>;
                pinctrl-names = "default";

                phy-mode = "rgmii-txid";
                phy-handle = <&external_phy>;

                amlogic,rgmii-rx-delay-ps = <800>;
        };

To use the same settings from vendor u-boot (which in my case has broken
Ethernet) the following commands can be used:
  mw.l 0xff634540 0x1621
  mw.l 0xff634544 0x30000
  phyreg w 0x0 0x1040
  phyreg w 0x1f 0xd08
  phyreg w 0x11 0x9
  phyreg w 0x15 0x11
  phyreg w 0x1f 0x0
  phyreg w 0x0 0x9200

Also I have tested this on a X96 Max board without any .dts changes
to confirm that other boards with the same IP block still work fine
with these changes.


Changes since v1 at [1]:
- updated patch 1 by making it more clear when the RX delay is applied.
  Thanks to Andrew for the suggestion!
- added a fix to enabling the timing-adjustment clock only when really
  needed. Found by Andrew - thanks!
- added testing not about X96 Max
- v1 did not go to the netdev mailing list, v2 fixes this


[0] https://lore.kernel.org/netdev/CAFBinCATt4Hi9rigj52nMf3oygyFbnopZcsakGL=KyWnsjY3JA@mail.gmail.com/
[1] https://patchwork.kernel.org/project/linux-amlogic/list/?series=384279

Martin Blumenstingl (5):
  dt-bindings: net: dwmac-meson: use picoseconds for the RGMII RX delay
  net: stmmac: dwmac-meson8b: fix enabling the timing-adjustment clock
  net: stmmac: dwmac-meson8b: use picoseconds for the RGMII RX delay
  net: stmmac: dwmac-meson8b: move RGMII delays into a separate function
  net: stmmac: dwmac-meson8b: add support for the RGMII RX delay on G12A

 .../bindings/net/amlogic,meson-dwmac.yaml     | 61 +++++++++++-
 .../ethernet/stmicro/stmmac/dwmac-meson8b.c   | 92 +++++++++++++++----
 2 files changed, 128 insertions(+), 25 deletions(-)

Comments

Andrew Lunn Nov. 17, 2020, 2:05 a.m. UTC | #1
On Sun, Nov 15, 2020 at 07:52:09PM +0100, Martin Blumenstingl wrote:
> Newer SoCs starting with the Amlogic Meson G12A have more a precise
> RGMII RX delay configuration register. This means more complexity in the
> code. Extract the existing RGMII delay configuration code into a
> separate function to make it easier to read/understand even when adding
> more logic in the future.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
Florian Fainelli Nov. 17, 2020, 6:36 p.m. UTC | #2
On 11/15/20 10:52 AM, Martin Blumenstingl wrote:
> Amlogic Meson G12A, G12B and SM1 SoCs have a more advanced RGMII RX
> delay register which allows picoseconds precision. Parse the new
> "amlogic,rgmii-rx-delay-ps" property or fall back to the old
> "amlogic,rx-delay-ns".
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

Maybe also issue a warning when the 'amlogic,rx-delay-ns' property is
found in addition to the 'amlogic,rgmii-rx-delay-ps'? Up to you how to
manage existing DTBs being deployed.
Florian Fainelli Nov. 17, 2020, 6:36 p.m. UTC | #3
On 11/15/20 10:52 AM, Martin Blumenstingl wrote:
> Newer SoCs starting with the Amlogic Meson G12A have more a precise

> RGMII RX delay configuration register. This means more complexity in the

> code. Extract the existing RGMII delay configuration code into a

> separate function to make it easier to read/understand even when adding

> more logic in the future.

> 

> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>


Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

-- 
Florian
Florian Fainelli Nov. 17, 2020, 6:37 p.m. UTC | #4
On 11/15/20 10:52 AM, Martin Blumenstingl wrote:
> Amlogic Meson G12A (and newer: G12B, SM1) SoCs have a more advanced RX

> delay logic. Instead of fine-tuning the delay in the nanoseconds range

> it now allows tuning in 200 picosecond steps. This support comes with

> new bits in the PRG_ETH1[19:16] register.

> 

> Add support for validating the RGMII RX delay as well as configuring the

> register accordingly on these platforms.

> 

> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>


Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

-- 
Florian
Martin Blumenstingl Nov. 17, 2020, 10:50 p.m. UTC | #5
Hi Florian,

On Tue, Nov 17, 2020 at 7:36 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>

> On 11/15/20 10:52 AM, Martin Blumenstingl wrote:

> > Amlogic Meson G12A, G12B and SM1 SoCs have a more advanced RGMII RX

> > delay register which allows picoseconds precision. Parse the new

> > "amlogic,rgmii-rx-delay-ps" property or fall back to the old

> > "amlogic,rx-delay-ns".

> >

> > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

>

> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

first of all: thanks for reviewing this (and the rest of the series)!

> Maybe also issue a warning when the 'amlogic,rx-delay-ns' property is

> found in addition to the 'amlogic,rgmii-rx-delay-ps'? Up to you how to

> manage existing DTBs being deployed.

none of the upstream DTBs uses amlogic,rx-delay-ns - and I am also not
aware of anything being in use "downstream".
I will add a sentence to the commit description when I re-send this
without RFC, something along those lines: "No upstream DTB uses the
old amlogic,rx-delay-ns (yet). Only include minimalistic logic to fall
back to the old property, without any special validation (for example:
old and new property are given at the same time)"

What do you think?


Best regards,
Martin