linux-kernel - Re: [PATCH] arm64: dts: rockchip: increase gmac rx

lists.openwall.net		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 linux-hardening linux-cve-announce 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]

Message-ID: <2578458.4XsnlVU6TS@diego>
Date: Mon, 02 Dec 2024 10:56:55 +0100
From: Heiko Stübner <heiko@...ech.de>
To: Jakob Unterwurzacher <jakobunt@...il.com>,
 Rob Herring <robh+dt@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>, Sasha Levin <sashal@...nel.org>,
 Iskander Amara <iskander.amara@...obroma-systems.com>,
 Jakob Unterwurzacher <jakob.unterwurzacher@...rry.de>,
 Vahe Grigoryan <vahe.grigoryan@...obroma-systems.com>,
 Quentin Schulz <quentin.schulz@...rry.de>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject:
 Re: [PATCH] arm64: dts: rockchip: increase gmac rx_delay to 0x11 on
 rk3399-puma

Am Montag, 2. Dezember 2024, 10:52:06 CET schrieb Quentin Schulz:
> Hi Jakob,
> 
> On 12/2/24 10:04 AM, Jakob Unterwurzacher wrote:
> > During mass manufacturing, we noticed the mmc_rx_crc_error counter,
> > as reported by "ethtool -S eth0 | grep mmc_rx_crc_error" to increase
> > above zero during nuttcp speedtests.
> > 
> > Cycling through the rx_delay range on two boards shows that is a large
> > "good" region from 0x11 to 0x35 (see below for details).
> > 
> 
> Is this missing a "there" after that? "that there is a large good region"?
> 
> > This commit increases rx_delay to 0x11, which is the smallest
> > possible change that fixes the issue we are seeing on the KSZ9031 PHY.
> > This also matches what most other rk3399 boards do.
> > 
> > Tests for Puma PCBA S/N TT0069903:
> > 
> > 	rx_delay mmc_rx_crc_error
> > 	-------- ----------------
> > 	0x09 (dhcp broken)
> > 	0x10 897
> > 	0x11 0
> > 	0x20 0
> > 	0x30 0
> > 	0x35 0
> > 	0x3a 745
> > 	0x3b 11375
> > 	0x3c 36680
> > 	0x40 (dhcp broken)
> > 	0x7f (dhcp broken)
> > 
> > Tests for Puma PCBA S/N TT0157733:
> > 
> > 	rx_delay mmc_rx_crc_error
> > 	-------- ----------------
> > 	0x10 59
> > 	0x11 0
> > 	0x35 0
> > 
> > Signed-off-by: Jakob Unterwurzacher <jakob.unterwurzacher@...rry.de>
> 
> This would be a candidate for backporting I believe.
> 
> Therefore, a

also please include a

Fixes: 2c66fc34e945 ("arm64: dts: rockchip: add RK3399-Q7 (Puma) SoM")

> Cc: <stable@...r.kernel.org>
> 
> here would have been nice (in the commit log), c.f. 
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#select-the-recipients-for-your-patch
> 
> > ---
> >   arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
> > index 9efcdce0f593..13d0c511046b 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
> > @@ -181,7 +181,7 @@ &gmac {
> >   	snps,reset-active-low;
> >   	snps,reset-delays-us = <0 10000 50000>;
> >   	tx_delay = <0x10>;
> > -	rx_delay = <0x10>;
> > +	rx_delay = <0x11>;
> 
> While at it, we could reorder this alphabetically and move rx_delay 

I would disagree. This is a "fix", so should ideally only do the minimal
changes to make life of the stable people easier.

Doing this one-line change is way easier to understand than stuff also
moving around.

Heiko

> between pinctrl-0 and snps,reset-gpio? c.f. 
> https://www.kernel.org/doc/html/latest/devicetree/bindings/dts-coding-style.html#order-of-properties-in-device-node 
> rx_delay and tx_delay seem to be vendor-specific but without the vendor 
> prefix, but so is snps,reset-gpio so that should be fine to reorder this 
> way.
> 
> Considering we have an option for KSZ9031 on RK3588 Jaguar and RK3588 
> Tiger and the "same" MAC IP is used and that we use the same TXD and RXD 
> delay than on RK3399 Puma right now, I guess we would want to check 
> those don't need a change as well? (funnily enough, all RK3588-based 
> boards in 6.12 actually have 0x00 for rx_delay and 0x43/0x44 for 
> tx_delay, except ours which are at 0x10). Not a blocker for this patch 
> though, so:
> 
> Reviewed-by: Quentin Schulz <quentin.schulz@...rry.de>
> 
> Thanks!
> Quentin
>