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@xxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxx>
>
> Thanks!
> Quentin
>
