Received-SPF: pass (google.com: domain of linux-kernel+bounces-215430-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date: Fri, 14 Jun 2024 18:34:19 +0000
Message-Id: <D1ZYFRA8S0KC.3GG78DB9BA5AG@folker-schwesinger.de>
Subject: Re: [PATCH 1/3] phy: rockchip: emmc: Enable pulldown for strobe
 line
Cc: "Vinod Koul" <vkoul@kernel.org>, "Kishon Vijay Abraham I"
 <kishon@kernel.org>, "Chris Ruehl" <chris.ruehl@gtsys.com.hk>, "Rob
 Herring" <robh@kernel.org>, "Krzysztof Kozlowski"
 <krzysztof.kozlowski+dt@linaro.org>, "Conor Dooley" <conor+dt@kernel.org>,
 "Christopher Obbard" <chris.obbard@collabora.com>, "Doug Anderson"
 <dianders@chromium.org>, "Brian Norris" <briannorris@chromium.org>, "Jensen
 Huang" <jensenhuang@friendlyarm.com>, <linux-phy@lists.infradead.org>,
 <linux-arm-kernel@lists.infradead.org>,
 <linux-rockchip@lists.infradead.org>, <linux-kernel@vger.kernel.org>,
 <devicetree@vger.kernel.org>
From: "Folker Schwesinger" <dev@folker-schwesinger.de>
To: "Alban Browaeys" <alban.browaeys@gmail.com>,
 =?utf-8?q?Heiko_St=C3=BCbner?= <heiko@sntech.de>, "Conor Dooley"
 <conor@kernel.org>
References: <20240326-rk-default-enable-strobe-pulldown-v1-0-f410c71605c0@folker-schwesinger.de> <313d5a24b6cffa1a9160e624bb6855aa7f66589e.camel@gmail.com> <20240411-mushily-pucker-732583c1d340@spud> <2192003.Icojqenx9y@diego> <2427291970ac0962bf56b2455e5cb26e49d42c51.camel@gmail.com>
In-Reply-To: <2427291970ac0962bf56b2455e5cb26e49d42c51.camel@gmail.com>

Hi Alban,

thanks for aggregating all the background information about the issue.

On Tue Jun 11, 2024 at 9:38 PM CEST, Alban Browaeys wrote:
>
> Could you test the enable-strobe property on all the dts that  disabled
> hs400es due to this new default (I lack the hardware to test the
> patches).
> At least arch/arm64/boot/dts/rockchip/rk3399-khadas-edge.dtsi
> arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts
> arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
> and ping the board tester for other boards to test if they require
> "rockchip,enable-strobe-pulldown" for EMMC HS400 write support (read is
> fine even with the new default).
>

I tested some of the boards that include rk3399-rock-pi-4.dtsi (see
below).

> On Tue, 2024-02-27 at 10:11 +0000, Folker Schwesinger wrote:
> > with the following applied, the EMMC related errors are gone. dmesg
> > only
> > shows "Purging ... bytes" during my tests:
> >=20
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
> > b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
> > index f2279aa6ca9e..ae0fb87e1a8b 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
> > @@ -647,8 +647,10 @@ &saradc {
> >  &sdhci {
> >         max-frequency =3D <150000000>;
> >         bus-width =3D <8>;
> > -       mmc-hs200-1_8v;
> > +       mmc-hs400-1_8v;
> > +       mmc-hs400-enhanced-strobe;
> >         non-removable;
> > +       rockchip,enable-strobe-pulldown;
> >         status =3D "okay";
> >  };
> >=20
> > For testing I ran dd three times in a row:
> >=20
> > dd if=3D/dev/zero of=3D./zero.bin bs=3D1M count=3D5000
> >=20
> > I tested this on both a Rock 4SE board and a Rock Pi 4B+ board with
> > the
> > same results.
>
> Folker, are you confident "Rock 4SE board and Rock Pi 4B+" were fixed
> with above patch?
> Ie the "rockchip,enable-strobe-pulldown;" should be under an
> "rockchip,rk3399-emmc-phy" compaible node, that is &emmc_phy in
> arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi, not sdhci.
>

The above diff was just a quick shot at testing the
"rockchip,enable-strobe-pulldown" property when I first learned about
it. I later realized that the property belongs under the &emmc_phy node
as you suggested. That's what I did in the other patchset I sent a bit
later, which was accepted and applied:

https://lists.infradead.org/pipermail/linux-rockchip/2024-March/045723.html
f720dd9b8b6d8b2160beda789429d5489ce8a099
c1b1f340dd7db11f273e426e110697551c9f501f

So yes, the Rock 4SE, Rock Pi 4B and Rock Pi 4B+ boards all were fixed
with the patch. I regularly have the Rock 4SE and Rock 4B running from
EMMC, always with the patch applied since I sent it.