* [PATCH 0/3] Meson8b/8m2: Ethernet RGMII TX delay fixes @ 2019-12-25 0:56 Martin Blumenstingl 2019-12-25 0:56 ` [PATCH 1/3] net: stmmac: dwmac-meson8b: Fix the RGMII TX delay on Meson8b/8m2 SoCs Martin Blumenstingl ` (3 more replies) 0 siblings, 4 replies; 11+ messages in thread From: Martin Blumenstingl @ 2019-12-25 0:56 UTC (permalink / raw) To: linux-amlogic, netdev, davem, khilman Cc: linus.luessing, balbes-150, linux-kernel, Martin Blumenstingl, linux-arm-kernel, ingrassia, jbrunet The Ethernet TX performance has been historically bad on Meson8b and Meson8m2 SoCs because high packet loss was seen. Today I (presumably) found out why this is: the input clock (which feeds the RGMII TX clock) has to be at least 4 times 125MHz. With the fixed "divide by 2" in the clock tree this means that m250_div needs to be at least 2. Now the PRG_ETH0 register in Linux matches what u-boot and the vendor 3.10 kernel use. iperf3 output on my Odroid-C1 (where this series has been tested): # iperf3 -c 192.168.1.100 Connecting to host 192.168.1.100, port 5201 [ 5] local 192.168.1.163 port 42636 connected to 192.168.1.100 port 5201 [ ID] Interval Transfer Bitrate Retr Cwnd [ 5] 0.00-1.00 sec 105 MBytes 878 Mbits/sec 0 609 KBytes [ 5] 1.00-2.00 sec 106 MBytes 885 Mbits/sec 0 683 KBytes [ 5] 2.00-3.09 sec 73.7 MBytes 570 Mbits/sec 0 683 KBytes [ 5] 3.09-4.00 sec 81.9 MBytes 754 Mbits/sec 0 795 KBytes [ 5] 4.00-5.00 sec 104 MBytes 869 Mbits/sec 0 877 KBytes [ 5] 5.00-6.00 sec 105 MBytes 878 Mbits/sec 0 877 KBytes [ 5] 6.00-7.00 sec 68.0 MBytes 571 Mbits/sec 0 877 KBytes [ 5] 7.00-8.00 sec 80.7 MBytes 676 Mbits/sec 0 877 KBytes [ 5] 8.00-9.01 sec 102 MBytes 853 Mbits/sec 0 877 KBytes [ 5] 9.01-10.00 sec 101 MBytes 859 Mbits/sec 0 877 KBytes - - - - - - - - - - - - - - - - - - - - - - - - - [ ID] Interval Transfer Bitrate Retr [ 5] 0.00-10.00 sec 927 MBytes 778 Mbits/sec 0 sender [ 5] 0.00-10.01 sec 927 MBytes 777 Mbits/sec receiver @David: please only apply patch #1 from this series. I included the .dts changes so others can test them together with the driver update (as the .dts has to be updated to fully fix the TX packet loss - with the old TX delay in the .dts there is still packet loss, even with a fixed driver). I will ask Kevin to apply patches #2 and #3 through his linux-amlogic tree - or resend them. When applying the .dts patches without the fix in the driver then I get 100% packet loss on my Odroid-C1. So unfortunately there's a hard dependency between patch 1 and 2/3. Martin Blumenstingl (3): net: stmmac: dwmac-meson8b: Fix the RGMII TX delay on Meson8b/8m2 SoCs ARM: dts: meson8b: odroidc1: use the same RGMII TX delay as u-boot ARM: dts: meson8m2: mxiii-plus: use the same RGMII TX delay as u-boot arch/arm/boot/dts/meson8b-odroidc1.dts | 2 +- arch/arm/boot/dts/meson8m2-mxiii-plus.dts | 2 +- .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 14 +++++++++++--- 3 files changed, 13 insertions(+), 5 deletions(-) -- 2.24.1 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] net: stmmac: dwmac-meson8b: Fix the RGMII TX delay on Meson8b/8m2 SoCs 2019-12-25 0:56 [PATCH 0/3] Meson8b/8m2: Ethernet RGMII TX delay fixes Martin Blumenstingl @ 2019-12-25 0:56 ` Martin Blumenstingl 2019-12-25 15:08 ` Andrew Lunn 2019-12-25 0:56 ` [PATCH 2/3] ARM: dts: meson8b: odroidc1: use the same RGMII TX delay as u-boot Martin Blumenstingl ` (2 subsequent siblings) 3 siblings, 1 reply; 11+ messages in thread From: Martin Blumenstingl @ 2019-12-25 0:56 UTC (permalink / raw) To: linux-amlogic, netdev, davem, khilman Cc: linus.luessing, balbes-150, linux-kernel, Martin Blumenstingl, linux-arm-kernel, ingrassia, jbrunet GXBB and newer SoCs use the fixed FCLK_DIV2 (1GHz) clock as input for the m250_sel clock. Meson8b and Meson8m2 use MPLL2 instead, whose rate can be adjusted at runtime. So far we have been running MPLL2 with ~250MHz (and the internal m250_div with value 1), which worked enough that we could transfer data with an TX delay of 4ns. Unfortunately there is high packet loss with an RGMII PHY when transferring data (receiving data works fine though). Odroid-C1's u-boot is running with a TX delay of only 2ns as well as the internal m250_div set to 2 - no lost (TX) packets can be observed with that setting in u-boot. Manual testing has shown that the TX packet loss goes away when using the following settings in Linux: - MPLL2 clock set to ~500MHz - m250_div set to 2 - TX delay set to 2ns Update the m250_div divider settings to only accept dividers greater or equal 2. This will allow the Meson8 and Meson8m2 .dts to be updated to use a TX delay of 2ns (instead of 4ns) to fix the TX packet loss. iperf3 results before the change: [ ID] Interval Transfer Bitrate Retr [ 5] 0.00-10.00 sec 182 MBytes 153 Mbits/sec 514 sender [ 5] 0.00-10.00 sec 182 MBytes 152 Mbits/sec receiver iperf3 results after the change (including an updated TX delay of 2ns): [ ID] Interval Transfer Bitrate Retr Cwnd [ 5] 0.00-10.00 sec 927 MBytes 778 Mbits/sec 0 sender [ 5] 0.00-10.01 sec 927 MBytes 777 Mbits/sec receiver Fixes: 4f6a71b84e1afd ("net: stmmac: dwmac-meson8b: fix internal RGMII clock configuration") Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> --- .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c index bd6c01004913..0e2fa14f1423 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c @@ -112,6 +112,14 @@ static int meson8b_init_rgmii_tx_clk(struct meson8b_dwmac *dwmac) struct device *dev = dwmac->dev; const char *parent_name, *mux_parent_names[MUX_CLK_NUM_PARENTS]; struct meson8b_dwmac_clk_configs *clk_configs; + static const struct clk_div_table div_table[] = { + { .div = 2, .val = 2, }, + { .div = 3, .val = 3, }, + { .div = 4, .val = 4, }, + { .div = 5, .val = 5, }, + { .div = 6, .val = 6, }, + { .div = 7, .val = 7, }, + }; clk_configs = devm_kzalloc(dev, sizeof(*clk_configs), GFP_KERNEL); if (!clk_configs) @@ -146,9 +154,9 @@ static int meson8b_init_rgmii_tx_clk(struct meson8b_dwmac *dwmac) clk_configs->m250_div.reg = dwmac->regs + PRG_ETH0; clk_configs->m250_div.shift = PRG_ETH0_CLK_M250_DIV_SHIFT; clk_configs->m250_div.width = PRG_ETH0_CLK_M250_DIV_WIDTH; - clk_configs->m250_div.flags = CLK_DIVIDER_ONE_BASED | - CLK_DIVIDER_ALLOW_ZERO | - CLK_DIVIDER_ROUND_CLOSEST; + clk_configs->m250_div.table = div_table; + clk_configs->m250_div.flags = CLK_DIVIDER_ALLOW_ZERO | + CLK_DIVIDER_ROUND_CLOSEST; clk = meson8b_dwmac_register_clk(dwmac, "m250_div", &parent_name, 1, &clk_divider_ops, &clk_configs->m250_div.hw); -- 2.24.1 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] net: stmmac: dwmac-meson8b: Fix the RGMII TX delay on Meson8b/8m2 SoCs 2019-12-25 0:56 ` [PATCH 1/3] net: stmmac: dwmac-meson8b: Fix the RGMII TX delay on Meson8b/8m2 SoCs Martin Blumenstingl @ 2019-12-25 15:08 ` Andrew Lunn 2019-12-25 15:33 ` Martin Blumenstingl 0 siblings, 1 reply; 11+ messages in thread From: Andrew Lunn @ 2019-12-25 15:08 UTC (permalink / raw) To: Martin Blumenstingl Cc: linus.luessing, balbes-150, khilman, linux-kernel, ingrassia, netdev, linux-amlogic, davem, linux-arm-kernel, jbrunet On Wed, Dec 25, 2019 at 01:56:53AM +0100, Martin Blumenstingl wrote: > GXBB and newer SoCs use the fixed FCLK_DIV2 (1GHz) clock as input for > the m250_sel clock. Meson8b and Meson8m2 use MPLL2 instead, whose rate > can be adjusted at runtime. > > So far we have been running MPLL2 with ~250MHz (and the internal > m250_div with value 1), which worked enough that we could transfer data > with an TX delay of 4ns. Unfortunately there is high packet loss with > an RGMII PHY when transferring data (receiving data works fine though). > Odroid-C1's u-boot is running with a TX delay of only 2ns as well as > the internal m250_div set to 2 - no lost (TX) packets can be observed > with that setting in u-boot. > > Manual testing has shown that the TX packet loss goes away when using > the following settings in Linux: > - MPLL2 clock set to ~500MHz > - m250_div set to 2 > - TX delay set to 2ns Hi Martin The delay will depend on the PHY, the value of phy-mode, and the PCB layout. https://ethernetfmc.com/rgmii-interface-timing-considerations/ RGMII requires a delay of 2ns between the data and the clock signal. There are at least three ways this can happen. 1) The MAC adds the delay 2) The PCB adds the delay by making the clock line longer than the data line. 3) The PHY adds the delay. In linux you configure this using the phy-mode in DT. # RX and TX delays are added by the MAC when required - rgmii # RGMII with internal RX and TX delays provided by the PHY, # the MAC should not add the RX or TX delays in this case - rgmii-id # RGMII with internal RX delay provided by the PHY, the MAC # should not add an RX delay in this case - rgmii-rxid # RGMII with internal TX delay provided by the PHY, the MAC # should not add an TX delay in this case - rgmii-txid So ideally, you want the MAC to add no delay at all, and then use the correct phy-mode so the PHY adds the correct delay. This gives you the most flexibility in terms of PHY and PCB design. This does however require that the PHY implements the delay, which not all do. Looking at patches 2 and 3, the phy-mode is set to rgmii. What you might actually need to do is set this to rgmii-txid, or maybe rgmii-id, once you have the MAC not inserting any delay. With MAC/PHY issues, it is a good idea to Cc: the PHY maintainers. Andrew _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] net: stmmac: dwmac-meson8b: Fix the RGMII TX delay on Meson8b/8m2 SoCs 2019-12-25 15:08 ` Andrew Lunn @ 2019-12-25 15:33 ` Martin Blumenstingl 2019-12-26 10:50 ` Andrew Lunn 0 siblings, 1 reply; 11+ messages in thread From: Martin Blumenstingl @ 2019-12-25 15:33 UTC (permalink / raw) To: Andrew Lunn Cc: linus.luessing, balbes-150, khilman, linux-kernel, ingrassia, netdev, linux-amlogic, davem, linux-arm-kernel, jbrunet Hi Andrew, thank you as always for taking a close look at my patches :-) On Wed, Dec 25, 2019 at 4:08 PM Andrew Lunn <andrew@lunn.ch> wrote: > > On Wed, Dec 25, 2019 at 01:56:53AM +0100, Martin Blumenstingl wrote: > > GXBB and newer SoCs use the fixed FCLK_DIV2 (1GHz) clock as input for > > the m250_sel clock. Meson8b and Meson8m2 use MPLL2 instead, whose rate > > can be adjusted at runtime. > > > > So far we have been running MPLL2 with ~250MHz (and the internal > > m250_div with value 1), which worked enough that we could transfer data > > with an TX delay of 4ns. Unfortunately there is high packet loss with > > an RGMII PHY when transferring data (receiving data works fine though). > > Odroid-C1's u-boot is running with a TX delay of only 2ns as well as > > the internal m250_div set to 2 - no lost (TX) packets can be observed > > with that setting in u-boot. > > > > Manual testing has shown that the TX packet loss goes away when using > > the following settings in Linux: > > - MPLL2 clock set to ~500MHz > > - m250_div set to 2 > > - TX delay set to 2ns > > Hi Martin > > The delay will depend on the PHY, the value of phy-mode, and the PCB > layout. > > https://ethernetfmc.com/rgmii-interface-timing-considerations/ > > RGMII requires a delay of 2ns between the data and the clock > signal. There are at least three ways this can happen. > > 1) The MAC adds the delay > > 2) The PCB adds the delay by making the clock line longer than the > data line. > > 3) The PHY adds the delay. > > In linux you configure this using the phy-mode in DT. > > # RX and TX delays are added by the MAC when required > - rgmii > > # RGMII with internal RX and TX delays provided by the PHY, > # the MAC should not add the RX or TX delays in this case > - rgmii-id > > # RGMII with internal RX delay provided by the PHY, the MAC > # should not add an RX delay in this case > - rgmii-rxid > > # RGMII with internal TX delay provided by the PHY, the MAC > # should not add an TX delay in this case > - rgmii-txid > > So ideally, you want the MAC to add no delay at all, and then use the > correct phy-mode so the PHY adds the correct delay. This gives you the > most flexibility in terms of PHY and PCB design. This does however > require that the PHY implements the delay, which not all do. these boards (with RGMII PHY) that I am aware of are using an RTL8211F PHY which implements a 2ns PHY TX delay however, the 3.10 vendor kernel also supports Micrel RGMII (and RMII) PHYs where I don't know if they implement a (configurable) TX delay. > Looking at patches 2 and 3, the phy-mode is set to rgmii. What you > might actually need to do is set this to rgmii-txid, or maybe > rgmii-id, once you have the MAC not inserting any delay. please let us split this discussion: 1) I believe that this patch is still correct and required whenever the MAC *has to* generate the TX delay (one use-case could be the Micrel PHYs I mentioned above) 2) the correct phy-mode and where the TX delay is being generated. I have tried "rgmii-txid" on my own Odroid-C1 and it's working fine there. however, it's the only board with RGMII PHY that I have from this generation of SoCs (and other testers are typically rare for this platform, because it's an older SoC). so my idea was to use the same settings as the 3.10 vendor kernel because these seem to be the "known working" ones. what do you think about 2)? my main concern is that this *could* break Ethernet on other people's boards. on the other hand I have no idea how likely that actually is. Martin _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] net: stmmac: dwmac-meson8b: Fix the RGMII TX delay on Meson8b/8m2 SoCs 2019-12-25 15:33 ` Martin Blumenstingl @ 2019-12-26 10:50 ` Andrew Lunn 2019-12-26 11:39 ` Martin Blumenstingl 0 siblings, 1 reply; 11+ messages in thread From: Andrew Lunn @ 2019-12-26 10:50 UTC (permalink / raw) To: Martin Blumenstingl Cc: Florian Fainelli, linus.luessing, balbes-150, khilman, linux-kernel, ingrassia, netdev, linux-amlogic, davem, linux-arm-kernel, jbrunet > > # RX and TX delays are added by the MAC when required > > - rgmii > > > > # RGMII with internal RX and TX delays provided by the PHY, > > # the MAC should not add the RX or TX delays in this case > > - rgmii-id > > > > # RGMII with internal RX delay provided by the PHY, the MAC > > # should not add an RX delay in this case > > - rgmii-rxid > > > > # RGMII with internal TX delay provided by the PHY, the MAC > > # should not add an TX delay in this case > > - rgmii-txid > > > > So ideally, you want the MAC to add no delay at all, and then use the > > correct phy-mode so the PHY adds the correct delay. This gives you the > > most flexibility in terms of PHY and PCB design. This does however > > require that the PHY implements the delay, which not all do. > these boards (with RGMII PHY) that I am aware of are using an RTL8211F > PHY which implements a 2ns PHY TX delay We need to be careful here... Earlier this year we got into a mess with a PHY driver wrongly implemented these delays. DT contained 'rgmii', but the PHY driver actually implemented rgmii-id'. Boards worked, because they actually needed rgmii-id. But then came along a board which really did need rgmii. We took the decision, maybe the wrong decision, to fix the PHY driver, and fixup DT files as we found boards which had the incorrect setting. We broke a lot of boards for a while and caused lots of people pain. You might have something which works, but i want to be sure it is actually correct, not two bugs cancelling each other out. You say the RTL8211F PHY implements a 2ns PHY TX delay. So in DT, do you have the phy-mode of 'rgmii-txid'? That would be the correct setting to say that the PHY provides only the TX delay. > however, the 3.10 vendor kernel also supports Micrel RGMII (and RMII) > PHYs where I don't know if they implement a (configurable) TX delay. > > > Looking at patches 2 and 3, the phy-mode is set to rgmii. What you > > might actually need to do is set this to rgmii-txid, or maybe > > rgmii-id, once you have the MAC not inserting any delay. > please let us split this discussion: > 1) I believe that this patch is still correct and required whenever > the MAC *has to* generate the TX delay (one use-case could be the > Micrel PHYs I mentioned above) I think this patch splits into two parts. One is getting a 25MHz clock. That part i can agree with straight away. The second part is setting a 2ns TX delay. This we need to be careful of. What is the MAC actually doing after this patch? What is the configured RX delay? Does the driver explicitly configure the RX delay? To what? If you look at the definitions above, if the phy-mode is rgmii, the MAC is responsible for both RX and TX delay. > 2) the correct phy-mode and where the TX delay is being generated. I > have tried "rgmii-txid" on my own Odroid-C1 and it's working fine > there. however, it's the only board with RGMII PHY that I have from > this generation of SoCs (and other testers are typically rare for this > platform, because it's an older SoC). so my idea was to use the same > settings as the 3.10 vendor kernel because these seem to be the "known > working" ones. Vendor kernels have the alternative of 'vendor crap' for a good reason. Just because it works does not mean it is correct. > what do you think about 2)? my main concern is that this *could* break > Ethernet on other people's boards. > on the other hand I have no idea how likely that actually is. From what i understand, Ethernet is already broken? Or is it broken on just some boards? Looking at the function rtl8211f_config_init(), that PHY driver can only control TX delays. RX delays are controlled by a strapping pin. The Micrel PHY driver can also control its clock skew, but it does it in an odd way, not via the phy-mode, but via additional properties. See the binding document. What we normally say is make the MAC add no delays, and pass the correct configuration to the PHY so it adds the delay. But due to the strapping pin on the rtl8211f, we are in a bit of a grey area. I would suggest the MAC adds no delay, phy-mode is set to rmgii-id, the PHY driver adds TX delay in software, we assume the strapping pin is set to add RX delay, and we add a big fat comment in the DT. For the Micrel PHY, we do the same, plus add the vendor properties to configure the clock skew. But as i said, we are in a bit of a grey area. We can consider other options, but everything needs to be self consistent, between what the MAC is doing, what the PHY is doing, and what phy-mode is set to in DT. Andrew _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] net: stmmac: dwmac-meson8b: Fix the RGMII TX delay on Meson8b/8m2 SoCs 2019-12-26 10:50 ` Andrew Lunn @ 2019-12-26 11:39 ` Martin Blumenstingl 2019-12-26 12:01 ` Andrew Lunn 0 siblings, 1 reply; 11+ messages in thread From: Martin Blumenstingl @ 2019-12-26 11:39 UTC (permalink / raw) To: Andrew Lunn Cc: Florian Fainelli, linus.luessing, balbes-150, khilman, linux-kernel, ingrassia, netdev, linux-amlogic, davem, linux-arm-kernel, jbrunet Hi Andrew, On Thu, Dec 26, 2019 at 11:50 AM Andrew Lunn <andrew@lunn.ch> wrote: > > > > # RX and TX delays are added by the MAC when required > > > - rgmii > > > > > > # RGMII with internal RX and TX delays provided by the PHY, > > > # the MAC should not add the RX or TX delays in this case > > > - rgmii-id > > > > > > # RGMII with internal RX delay provided by the PHY, the MAC > > > # should not add an RX delay in this case > > > - rgmii-rxid > > > > > > # RGMII with internal TX delay provided by the PHY, the MAC > > > # should not add an TX delay in this case > > > - rgmii-txid > > > > > > So ideally, you want the MAC to add no delay at all, and then use the > > > correct phy-mode so the PHY adds the correct delay. This gives you the > > > most flexibility in terms of PHY and PCB design. This does however > > > require that the PHY implements the delay, which not all do. > > these boards (with RGMII PHY) that I am aware of are using an RTL8211F > > PHY which implements a 2ns PHY TX delay > > We need to be careful here... > > Earlier this year we got into a mess with a PHY driver wrongly > implemented these delays. DT contained 'rgmii', but the PHY driver > actually implemented rgmii-id'. Boards worked, because they actually > needed rgmii-id. But then came along a board which really did need > rgmii. We took the decision, maybe the wrong decision, to fix the PHY > driver, and fixup DT files as we found boards which had the incorrect > setting. We broke a lot of boards for a while and caused lots of > people pain. > > You might have something which works, but i want to be sure it is > actually correct, not two bugs cancelling each other out. (wow, that sounds painful) > You say the RTL8211F PHY implements a 2ns PHY TX delay. So in DT, do > you have the phy-mode of 'rgmii-txid'? That would be the correct > setting to say that the PHY provides only the TX delay. yes, in my experiment (for which I did not send patches to the list yet because we're discussing that part) I set phy-mode = "rgmii-txid"; this also makes the dwmac-meson8b driver ignore any TX delay on the MAC side > > however, the 3.10 vendor kernel also supports Micrel RGMII (and RMII) > > PHYs where I don't know if they implement a (configurable) TX delay. > > > > > Looking at patches 2 and 3, the phy-mode is set to rgmii. What you > > > might actually need to do is set this to rgmii-txid, or maybe > > > rgmii-id, once you have the MAC not inserting any delay. > > please let us split this discussion: > > 1) I believe that this patch is still correct and required whenever > > the MAC *has to* generate the TX delay (one use-case could be the > > Micrel PHYs I mentioned above) > > I think this patch splits into two parts. One is getting a 25MHz > clock. That part i can agree with straight away. The second part is > setting a 2ns TX delay. This we need to be careful of. What is the MAC > actually doing after this patch? What is the configured RX delay? Does > the driver explicitly configure the RX delay? To what? good to see that we agree on the clock part! the MAC is not capable of generating an RX delay (at least as far as I know). to me this means that we are using the default on the PHY side (I *assume* - but I have no proof - that this means the RX delay is enabled, just like TX delay is enabled after a full chip reset) > > 2) the correct phy-mode and where the TX delay is being generated. I > > have tried "rgmii-txid" on my own Odroid-C1 and it's working fine > > there. however, it's the only board with RGMII PHY that I have from > > this generation of SoCs (and other testers are typically rare for this > > platform, because it's an older SoC). so my idea was to use the same > > settings as the 3.10 vendor kernel because these seem to be the "known > > working" ones. > > Vendor kernels have the alternative of 'vendor crap' for a good > reason. Just because it works does not mean it is correct. yes, there's no general rule about the quality of vendor code in my case I found Ethernet TX to be stable and close to Gbit/s speeds on the vendor kernel while mainline was dropping packets and speeds were worse that still doesn't mean the vendor code is good, but from a user perspective it's better than what we have in mainline > > what do you think about 2)? my main concern is that this *could* break > > Ethernet on other people's boards. > > on the other hand I have no idea how likely that actually is. > > From what i understand, Ethernet is already broken? Or is it broken on > just some boards? it's mostly "broken" (high TX packet loss, slow TX speeds) for the two supported boards with an RGMII PHY (meson8b-odroidc1.dts and meson8m2-mxiii-plus.dts) examples on the many ways it was broken will follow - feel free to skip this part before this patch we had: input clock at 250MHz |- m250_sel (inheriting the rate of the input clock because it's a mux) |- m250_div set to 1 |- fixed_div_by_2 (outputting 125MHz for the RGMII TX clock) together with a configured (but suspicious) TX delay of 4ns on the MAC side in the board .dts Transmitting ("sending") data via Ethernet has heavy packet loss and far from Gbit/s speeds (setting the TX delay on the MAC in this case to 2ns broke Ethernet completely, even DHCP was failing) after this patch we have: input clock at 500MHz (double as before) |- m250_sel (inheriting the rate of the input clock because it's a mux) |- m250_div set to 2 |- fixed_div_by_2 (still outputting 125MHz for the RGMII TX clock) with the old TX delay of 4ns on the MAC side there is still packet loss updating the TX delay on the MAC side to 2ns (which is what the vendor driver does) fixes the packet loss and transmit speeds > The Micrel PHY driver can also control its clock skew, but it does it > in an odd way, not via the phy-mode, but via additional > properties. See the binding document. I see, thank you for the hint > What we normally say is make the MAC add no delays, and pass the > correct configuration to the PHY so it adds the delay. But due to the > strapping pin on the rtl8211f, we are in a bit of a grey area. I would > suggest the MAC adds no delay, phy-mode is set to rmgii-id, the PHY > driver adds TX delay in software, we assume the strapping pin is set > to add RX delay, and we add a big fat comment in the DT. > > For the Micrel PHY, we do the same, plus add the vendor properties to > configure the clock skew. > > But as i said, we are in a bit of a grey area. We can consider other > options, but everything needs to be self consistent, between what the > MAC is doing, what the PHY is doing, and what phy-mode is set to in > DT. do you think it's worth the effort to get clarification from Realtek on the RX delay behavior (and whether there's a register to control it)? (when I previously asked them about interrupt support they answered all my questions so we were able to confirm that it's implemented properly upstream) before this email I would have asked Realtek about the RX delay and sent a patch updating rtl8211f_config_init (the PHY_INTERFACE_MODE_RGMII_RXID and PHY_INTERFACE_MODE_RGMII_ID cases). you mentioned that there was breakage earlier this year, so I'm not sure anymore (that leaves me thinking: asking them is still useful to get out of this grey area) Martin _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] net: stmmac: dwmac-meson8b: Fix the RGMII TX delay on Meson8b/8m2 SoCs 2019-12-26 11:39 ` Martin Blumenstingl @ 2019-12-26 12:01 ` Andrew Lunn 2019-12-26 18:17 ` Martin Blumenstingl 0 siblings, 1 reply; 11+ messages in thread From: Andrew Lunn @ 2019-12-26 12:01 UTC (permalink / raw) To: Martin Blumenstingl Cc: Florian Fainelli, linus.luessing, balbes-150, khilman, linux-kernel, ingrassia, netdev, linux-amlogic, davem, linux-arm-kernel, jbrunet > the MAC is not capable of generating an RX delay (at least as far as I know). So that immediately means rgmii is invalid as a phy-mode, since the documentation implies the MAC needs to add RX delay. > it's mostly "broken" (high TX packet loss, slow TX speeds) for the two > supported boards with an RGMII PHY (meson8b-odroidc1.dts and > meson8m2-mxiii-plus.dts) > examples on the many ways it was broken will follow - feel free to > skip this part That is actually good. If it never worked, we don't need to worry about breaking it! We can spend our time getting this correct, and not have to worry about backwards compatibility, etc. > > What we normally say is make the MAC add no delays, and pass the > > correct configuration to the PHY so it adds the delay. But due to the > > strapping pin on the rtl8211f, we are in a bit of a grey area. I would > > suggest the MAC adds no delay, phy-mode is set to rmgii-id, the PHY > > driver adds TX delay in software, we assume the strapping pin is set > > to add RX delay, and we add a big fat comment in the DT. > > > > For the Micrel PHY, we do the same, plus add the vendor properties to > > configure the clock skew. > > > > But as i said, we are in a bit of a grey area. We can consider other > > options, but everything needs to be self consistent, between what the > > MAC is doing, what the PHY is doing, and what phy-mode is set to in > > DT. > do you think it's worth the effort to get clarification from Realtek > on the RX delay behavior (and whether there's a register to control > it)? You can ask. There are also datasheet here: http://files.pine64.org/doc/datasheet/rock64/RTL8211F-CG-Realtek.pdf https://datasheet.lcsc.com/szlcsc/1909021205_Realtek-Semicon-RTL8211F-CG_C187932.pdf It looks like both RX and TX delay can be controlled via strapping. But the register for controlling the TX delay is not documented. > you mentioned that there was breakage earlier this year, so I'm not sure anymore > (that leaves me thinking: asking them is still useful to get out of > this grey area) It was an Atheros PHY with breakage. If we can fully control the RX and TX delays, that would be great. It would also be useful if there was a way to determine how the PHY has been strapped. If we cannot fully control the delays but we can find out what delays it is using, we can check the requested configuration against the strapped configuration, and warn if they are different. Andrew _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] net: stmmac: dwmac-meson8b: Fix the RGMII TX delay on Meson8b/8m2 SoCs 2019-12-26 12:01 ` Andrew Lunn @ 2019-12-26 18:17 ` Martin Blumenstingl 0 siblings, 0 replies; 11+ messages in thread From: Martin Blumenstingl @ 2019-12-26 18:17 UTC (permalink / raw) To: Andrew Lunn Cc: Florian Fainelli, linus.luessing, balbes-150, khilman, linux-kernel, ingrassia, netdev, linux-amlogic, davem, linux-arm-kernel, jbrunet Hi Andrew, On Thu, Dec 26, 2019 at 1:01 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > the MAC is not capable of generating an RX delay (at least as far as I know). > > So that immediately means rgmii is invalid as a phy-mode, since the > documentation implies the MAC needs to add RX delay. things turned out even more confusing thanks to your persistence (keep reading, it will get better though) :-) I have tested the following on my Odroid-C1 which has an RTL8211F PHY. With patch #1 from this series I knew that the following was working: - phy-mode = "rgmii" and 2ns TX delay on the MAC (RX delay is seemingly not configured anywhere) - phy-mode = "rgmii-txid" (again, the RX delay is seemingly not configured anywhere) with the patch to change the RX delay on the RTL8211F PHY I decided to try out phy-mode = "rgmii-id": this broke Ethernet. then I looked at the MAC registers and spotted that bits 13 (adj_enable) and 14 (adj_setup) are set (first time I'm noticing this). unsetting them makes phy-mode = "rgmii-id" work! I also confirmed the opposite case: unsetting bit 13 and 14 breaks Ethernet with phy-mode = "rgmii-txid". so it seems that there *is* a way to configure the RX delay on Meson8b and Meson8m2 SoCs (at least). I will spin up a RfC patch to discuss this with the Amlogic team and because I don't know what these bits do exactly > > it's mostly "broken" (high TX packet loss, slow TX speeds) for the two > > supported boards with an RGMII PHY (meson8b-odroidc1.dts and > > meson8m2-mxiii-plus.dts) > > examples on the many ways it was broken will follow - feel free to > > skip this part > > That is actually good. If it never worked, we don't need to worry > about breaking it! We can spend our time getting this correct, and not > have to worry about backwards compatibility, etc. ACK > > > What we normally say is make the MAC add no delays, and pass the > > > correct configuration to the PHY so it adds the delay. But due to the > > > strapping pin on the rtl8211f, we are in a bit of a grey area. I would > > > suggest the MAC adds no delay, phy-mode is set to rmgii-id, the PHY > > > driver adds TX delay in software, we assume the strapping pin is set > > > to add RX delay, and we add a big fat comment in the DT. > > > > > > For the Micrel PHY, we do the same, plus add the vendor properties to > > > configure the clock skew. > > > > > > But as i said, we are in a bit of a grey area. We can consider other > > > options, but everything needs to be self consistent, between what the > > > MAC is doing, what the PHY is doing, and what phy-mode is set to in > > > DT. > > > do you think it's worth the effort to get clarification from Realtek > > on the RX delay behavior (and whether there's a register to control > > it)? > > You can ask. There are also datasheet here: > > http://files.pine64.org/doc/datasheet/rock64/RTL8211F-CG-Realtek.pdf > https://datasheet.lcsc.com/szlcsc/1909021205_Realtek-Semicon-RTL8211F-CG_C187932.pdf > > It looks like both RX and TX delay can be controlled via > strapping. But the register for controlling the TX delay is not > documented. I checked the mails I got from Realtek I while ago and they even included the RX delay bits! I even sent a patch two years ago but I must have dropped it at some point (maybe I assumed that it wasn't relevant anymore - I don't remember): [0] > > you mentioned that there was breakage earlier this year, so I'm not sure anymore > > (that leaves me thinking: asking them is still useful to get out of > > this grey area) > > It was an Atheros PHY with breakage. > > If we can fully control the RX and TX delays, that would be great. It > would also be useful if there was a way to determine how the PHY has > been strapped. If we cannot fully control the delays but we can find > out what delays it is using, we can check the requested configuration > against the strapped configuration, and warn if they are different. I am currently testing whether the pin strapping configuration can be read back by the RX and TX delay registers my Odroid-C1 has both strapped to GND which means off but my Khadas VIM2 has TX delay strapped to ETH_VDDIO which means on (RX delay is still strapped to GND) once I am done testing I will send patches for the RTL8211F PHY driver Martin [0] https://patchwork.ozlabs.org/patch/843946/ _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] ARM: dts: meson8b: odroidc1: use the same RGMII TX delay as u-boot 2019-12-25 0:56 [PATCH 0/3] Meson8b/8m2: Ethernet RGMII TX delay fixes Martin Blumenstingl 2019-12-25 0:56 ` [PATCH 1/3] net: stmmac: dwmac-meson8b: Fix the RGMII TX delay on Meson8b/8m2 SoCs Martin Blumenstingl @ 2019-12-25 0:56 ` Martin Blumenstingl 2019-12-25 0:56 ` [PATCH 3/3] ARM: dts: meson8m2: mxiii-plus: " Martin Blumenstingl 2019-12-28 0:33 ` [PATCH 0/3] Meson8b/8m2: Ethernet RGMII TX delay fixes David Miller 3 siblings, 0 replies; 11+ messages in thread From: Martin Blumenstingl @ 2019-12-25 0:56 UTC (permalink / raw) To: linux-amlogic, netdev, davem, khilman Cc: linus.luessing, balbes-150, linux-kernel, Martin Blumenstingl, linux-arm-kernel, ingrassia, jbrunet Due to a bug in the MPLL2 clock setup (which is used as input for the RGMII TX clock) a TX delay of 2ns did not work previously. With a TX delay of 4ns Ethernet worked enough to get an IP via DHCP but there was still high packet loss when transmitting data. Update the TX delay to 2ns - which is the same value that u-boot and the vendor kernel use - to fix the packet loss when transmitting data. Fixes: 9c15795a4f96cb ("ARM: dts: meson8b-odroidc1: ethernet support") Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> --- arch/arm/boot/dts/meson8b-odroidc1.dts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/meson8b-odroidc1.dts b/arch/arm/boot/dts/meson8b-odroidc1.dts index a2a47804fc4a..e2ba2d66d8d9 100644 --- a/arch/arm/boot/dts/meson8b-odroidc1.dts +++ b/arch/arm/boot/dts/meson8b-odroidc1.dts @@ -204,7 +204,7 @@ ðmac { phy-mode = "rgmii"; phy-handle = <ð_phy>; - amlogic,tx-delay-ns = <4>; + amlogic,tx-delay-ns = <2>; nvmem-cells = <ðernet_mac_address>; nvmem-cell-names = "mac-address"; -- 2.24.1 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] ARM: dts: meson8m2: mxiii-plus: use the same RGMII TX delay as u-boot 2019-12-25 0:56 [PATCH 0/3] Meson8b/8m2: Ethernet RGMII TX delay fixes Martin Blumenstingl 2019-12-25 0:56 ` [PATCH 1/3] net: stmmac: dwmac-meson8b: Fix the RGMII TX delay on Meson8b/8m2 SoCs Martin Blumenstingl 2019-12-25 0:56 ` [PATCH 2/3] ARM: dts: meson8b: odroidc1: use the same RGMII TX delay as u-boot Martin Blumenstingl @ 2019-12-25 0:56 ` Martin Blumenstingl 2019-12-28 0:33 ` [PATCH 0/3] Meson8b/8m2: Ethernet RGMII TX delay fixes David Miller 3 siblings, 0 replies; 11+ messages in thread From: Martin Blumenstingl @ 2019-12-25 0:56 UTC (permalink / raw) To: linux-amlogic, netdev, davem, khilman Cc: linus.luessing, balbes-150, linux-kernel, Martin Blumenstingl, linux-arm-kernel, ingrassia, jbrunet Due to a bug in the MPLL2 clock setup (which is used as input for the RGMII TX clock) a TX delay of 2ns did not work previously. With a TX delay of 4ns Ethernet worked enough to get an IP via DHCP but there was still high packet loss when transmitting data. Update the TX delay to 2ns - which is the same value that u-boot and the vendor kernel use - to fix the packet loss when transmitting data. Fixes: 35ee52bea66c74 ("ARM: dts: meson8m2: add support for the Tronsmart MXIII Plus") Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> --- arch/arm/boot/dts/meson8m2-mxiii-plus.dts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/meson8m2-mxiii-plus.dts b/arch/arm/boot/dts/meson8m2-mxiii-plus.dts index d54477b1001c..fd94b5cbd845 100644 --- a/arch/arm/boot/dts/meson8m2-mxiii-plus.dts +++ b/arch/arm/boot/dts/meson8m2-mxiii-plus.dts @@ -71,7 +71,7 @@ ðmac { phy-handle = <ð_phy0>; phy-mode = "rgmii"; - amlogic,tx-delay-ns = <4>; + amlogic,tx-delay-ns = <2>; mdio { compatible = "snps,dwmac-mdio"; -- 2.24.1 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] Meson8b/8m2: Ethernet RGMII TX delay fixes 2019-12-25 0:56 [PATCH 0/3] Meson8b/8m2: Ethernet RGMII TX delay fixes Martin Blumenstingl ` (2 preceding siblings ...) 2019-12-25 0:56 ` [PATCH 3/3] ARM: dts: meson8m2: mxiii-plus: " Martin Blumenstingl @ 2019-12-28 0:33 ` David Miller 3 siblings, 0 replies; 11+ messages in thread From: David Miller @ 2019-12-28 0:33 UTC (permalink / raw) To: martin.blumenstingl Cc: linus.luessing, balbes-150, khilman, linux-kernel, ingrassia, netdev, linux-amlogic, linux-arm-kernel, jbrunet From: Martin Blumenstingl <martin.blumenstingl@googlemail.com> Date: Wed, 25 Dec 2019 01:56:52 +0100 > The Ethernet TX performance has been historically bad on Meson8b and > Meson8m2 SoCs because high packet loss was seen. Today I (presumably) > found out why this is: the input clock (which feeds the RGMII TX clock) > has to be at least 4 times 125MHz. With the fixed "divide by 2" in the > clock tree this means that m250_div needs to be at least 2. ... It looks there needs to be more discussion on this series, please respin once the discussions are resolved. Thank you. _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-12-28 0:34 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-25 0:56 [PATCH 0/3] Meson8b/8m2: Ethernet RGMII TX delay fixes Martin Blumenstingl 2019-12-25 0:56 ` [PATCH 1/3] net: stmmac: dwmac-meson8b: Fix the RGMII TX delay on Meson8b/8m2 SoCs Martin Blumenstingl 2019-12-25 15:08 ` Andrew Lunn 2019-12-25 15:33 ` Martin Blumenstingl 2019-12-26 10:50 ` Andrew Lunn 2019-12-26 11:39 ` Martin Blumenstingl 2019-12-26 12:01 ` Andrew Lunn 2019-12-26 18:17 ` Martin Blumenstingl 2019-12-25 0:56 ` [PATCH 2/3] ARM: dts: meson8b: odroidc1: use the same RGMII TX delay as u-boot Martin Blumenstingl 2019-12-25 0:56 ` [PATCH 3/3] ARM: dts: meson8m2: mxiii-plus: " Martin Blumenstingl 2019-12-28 0:33 ` [PATCH 0/3] Meson8b/8m2: Ethernet RGMII TX delay fixes David Miller
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).