[PATCH] riscv: dts: starfive: Update ethernet phy0 delay parameter values for Star64

E Shattow posted 1 patch 1 month ago
arch/riscv/boot/dts/starfive/jh7110-pine64-star64.dts | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
[PATCH] riscv: dts: starfive: Update ethernet phy0 delay parameter values for Star64
Posted by E Shattow 1 month ago
Improve function of Star64 bottom network port phy0 with updated delay values.
Initial upstream patches supporting Star64 use the same vendor board support
package parameters known to result in an unreliable bottom network port.

Success acquiring DHCP lease and no dropped packets to ping LAN address:
rx  900: tx 1500 1650 1800 1950
rx  750: tx      1650 1800 1950
rx  600: tx           1800 1950
rx 1050: tx      1650 1800 1950
rx 1200: tx 1500 1650 1800 1950
rx 1350: tx 1500 1650 1800 1950
rx 1500: tx 1500 1650 1800 1950
rx 1650: tx 1500 1650 1800 1950
rx 1800: tx 1500 1650 1800 1950
rx 1900: tx                1950
rx 1950: tx                1950

Failure acquiring DHCP lease or many dropped packets:
rx  450: tx                1500      1800 1950
rx  600: tx      1200 1350      1650
rx  750: tx           1350 1500
rx  900: tx      1200 1350
rx 1050: tx 1050 1200 1350 1500
rx 1200: tx           1350
rx 1350: tx           1350
rx 1500: tx      1200 1350
rx 1650: tx 1050 1200 1350
rx 1800: tx 1050 1200 1350
rx 1900: tx                1500 1650 1800
rx 1950: tx      1200 1350

Non-functional:
rx    0: tx 0  150  300  450  600  750  900 1050 1200 1350 1500 1650 1800 1950
rx  150: tx 0  150  300  450  600  750  900 1050 1200 1350 1500 1650 1800 1950
rx  300: tx 0  150  300  450  600  750  900 1050 1200 1350 1500 1650 1800 1950
rx  450: tx 0  150  300  450  600  750  900 1050 1200 1350      1650
rx  600: tx 0  150  300  450  600  750  900 1050
rx  750: tx 0  150  300  450  600  750  900 1050 1200
rx  900: tx 0  150  300  450  600  750  900 1050
rx 1050: tx 0  150  300  450  600  750  900
rx 1200: tx 0  150  300  450  600  750  900 1050 1200
rx 1350: tx 0  150  300  450  600  750  900 1050 1200
rx 1500: tx 0  150  300  450  600  750  900 1050
rx 1650: tx 0  150  300  450  600  750  900
rx 1800: tx 0  150  300  450  600  750  900
rx 1900: tx 0  150  300  450  600  750  900 1050 1200 1350
rx 1950: tx 0  150  300  450  600  750  900 1050

Selecting the median of all working rx delay values 1500 combined with tx delay
values 1500, 1650, 1800, and 1950 only the tx delay value of 1950 (default) is
reliable as tested in both Linux 6.11.2 and U-Boot v2024.10

Signed-off-by: E Shattow <e@freeshell.de>
---
 arch/riscv/boot/dts/starfive/jh7110-pine64-star64.dts | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/riscv/boot/dts/starfive/jh7110-pine64-star64.dts b/arch/riscv/boot/dts/starfive/jh7110-pine64-star64.dts
index b720cdd15ed6..8e39fdc73ecb 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-pine64-star64.dts
+++ b/arch/riscv/boot/dts/starfive/jh7110-pine64-star64.dts
@@ -44,8 +44,7 @@ &pcie1 {
 };
 
 &phy0 {
-	rx-internal-delay-ps = <1900>;
-	tx-internal-delay-ps = <1500>;
+	rx-internal-delay-ps = <1500>;
 	motorcomm,rx-clk-drv-microamp = <2910>;
 	motorcomm,rx-data-drv-microamp = <2910>;
 	motorcomm,tx-clk-adj-enabled;
-- 
2.45.2
Re: [PATCH] riscv: dts: starfive: Update ethernet phy0 delay parameter values for Star64
Posted by Conor Dooley 1 month ago
From: Conor Dooley <conor.dooley@microchip.com>

On Mon, 21 Oct 2024 23:09:51 -0700, E Shattow wrote:
> Improve function of Star64 bottom network port phy0 with updated delay values.
> Initial upstream patches supporting Star64 use the same vendor board support
> package parameters known to result in an unreliable bottom network port.
> 
> Success acquiring DHCP lease and no dropped packets to ping LAN address:
> rx  900: tx 1500 1650 1800 1950
> rx  750: tx      1650 1800 1950
> rx  600: tx           1800 1950
> rx 1050: tx      1650 1800 1950
> rx 1200: tx 1500 1650 1800 1950
> rx 1350: tx 1500 1650 1800 1950
> rx 1500: tx 1500 1650 1800 1950
> rx 1650: tx 1500 1650 1800 1950
> rx 1800: tx 1500 1650 1800 1950
> rx 1900: tx                1950
> rx 1950: tx                1950
> 
> [...]

Applied to riscv-soc-fixes, thanks!

[1/1] riscv: dts: starfive: Update ethernet phy0 delay parameter values for Star64
      https://git.kernel.org/conor/c/825bb69228c8

Thanks,
Conor.
Re: [PATCH] riscv: dts: starfive: Update ethernet phy0 delay parameter values for Star64
Posted by Emil Renner Berthing 1 month ago
E Shattow wrote:
> Improve function of Star64 bottom network port phy0 with updated delay values.
> Initial upstream patches supporting Star64 use the same vendor board support
> package parameters known to result in an unreliable bottom network port.
>
> Success acquiring DHCP lease and no dropped packets to ping LAN address:
> rx  900: tx 1500 1650 1800 1950
> rx  750: tx      1650 1800 1950
> rx  600: tx           1800 1950
> rx 1050: tx      1650 1800 1950
> rx 1200: tx 1500 1650 1800 1950
> rx 1350: tx 1500 1650 1800 1950
> rx 1500: tx 1500 1650 1800 1950
> rx 1650: tx 1500 1650 1800 1950
> rx 1800: tx 1500 1650 1800 1950
> rx 1900: tx                1950
> rx 1950: tx                1950
>
> Failure acquiring DHCP lease or many dropped packets:
> rx  450: tx                1500      1800 1950
> rx  600: tx      1200 1350      1650
> rx  750: tx           1350 1500
> rx  900: tx      1200 1350
> rx 1050: tx 1050 1200 1350 1500
> rx 1200: tx           1350
> rx 1350: tx           1350
> rx 1500: tx      1200 1350
> rx 1650: tx 1050 1200 1350
> rx 1800: tx 1050 1200 1350
> rx 1900: tx                1500 1650 1800
> rx 1950: tx      1200 1350
>
> Non-functional:
> rx    0: tx 0  150  300  450  600  750  900 1050 1200 1350 1500 1650 1800 1950
> rx  150: tx 0  150  300  450  600  750  900 1050 1200 1350 1500 1650 1800 1950
> rx  300: tx 0  150  300  450  600  750  900 1050 1200 1350 1500 1650 1800 1950
> rx  450: tx 0  150  300  450  600  750  900 1050 1200 1350      1650
> rx  600: tx 0  150  300  450  600  750  900 1050
> rx  750: tx 0  150  300  450  600  750  900 1050 1200
> rx  900: tx 0  150  300  450  600  750  900 1050
> rx 1050: tx 0  150  300  450  600  750  900
> rx 1200: tx 0  150  300  450  600  750  900 1050 1200
> rx 1350: tx 0  150  300  450  600  750  900 1050 1200
> rx 1500: tx 0  150  300  450  600  750  900 1050
> rx 1650: tx 0  150  300  450  600  750  900
> rx 1800: tx 0  150  300  450  600  750  900
> rx 1900: tx 0  150  300  450  600  750  900 1050 1200 1350
> rx 1950: tx 0  150  300  450  600  750  900 1050
>
> Selecting the median of all working rx delay values 1500 combined with tx delay
> values 1500, 1650, 1800, and 1950 only the tx delay value of 1950 (default) is
> reliable as tested in both Linux 6.11.2 and U-Boot v2024.10
>
> Signed-off-by: E Shattow <e@freeshell.de>

Thanks for working this out.

Acked-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
Re: [PATCH] riscv: dts: starfive: Update ethernet phy0 delay parameter values for Star64
Posted by Conor Dooley 1 month ago
On Mon, Oct 21, 2024 at 11:09:51PM -0700, E Shattow wrote:
> Improve function of Star64 bottom network port phy0 with updated delay values.
> Initial upstream patches supporting Star64 use the same vendor board support
> package parameters known to result in an unreliable bottom network port.

Should I add:
Fixes: 2606bf583b962 ("riscv: dts: starfive: add Star64 board devicetree")
CC: stable@vger.kernel.org
?

"unreliable" sounds to me like something that is worthy of going to
fixes/stable
Re: [PATCH] riscv: dts: starfive: Update ethernet phy0 delay parameter values for Star64
Posted by E Shattow 1 month ago
On 10/22/24 09:41, Conor Dooley wrote:
> On Mon, Oct 21, 2024 at 11:09:51PM -0700, E Shattow wrote:
>> Improve function of Star64 bottom network port phy0 with updated delay values.
>> Initial upstream patches supporting Star64 use the same vendor board support
>> package parameters known to result in an unreliable bottom network port.
> Should I add:
> Fixes: 2606bf583b962 ("riscv: dts: starfive: add Star64 board devicetree")
> CC: stable@vger.kernel.org
> ?
>
> "unreliable" sounds to me like something that is worthy of going to
> fixes/stable

Applying as a fix to stable sounds reasonable, thanks. The bottom 
network port has always been known by Star64 users in reviews and 
discussions to be affected by dropped packets and low network 
throughput. If we want to prove correctness does this require expertise 
and use of an oscilloscope to characterize the signal timing? Though I 
am not sure I got it right, it's not worse than previously was on any of 
these Star64 boards in the wild and probably is better for at least some 
(if not all).

Notable aside is to mention the re-worked motorcomm driver of 
more-recent Linux kernel releases (when compared to the vendor board 
support package) dropped the Fast Ethernet configuration parameters on 
the reasoning that Fast Ethernet (as compared to Gigabit Ethernet) is 
relatively slow enough of a signal that a default delay parameter is 
good enough for all use cases. The non-default Fast Ethernet delay 
parameter values missing from the upstream effort are not possible to 
implement or test for in my effort here, but are no worse or better for 
having this patch applied.

-E