Le mercredi 24 août 2022 à 07:57 -0700, Doug Anderson a écrit : > On Tue, Aug 23, 2022 at 8:11 PM Jensen Huang > <jensenhuang@xxxxxxxxxxxxxxx> wrote: > > I realized that only some devices may be affected, so I considered > > modifying rk3399-nanopi4.dtsi only, > > but other boards without external pull-down should still need this > > patch. > > I guess the other alternative would be to change how the dt property > works. You could say: > > 1. If `enable-strobe-pulldown` is set then enable the strobe > pulldown. > > 2. If `enable-strobe-pulldown` is not set then don't touch the pin in > the kernel. > > 3. If someone later needs to explicitly disable the strobe pulldown > they could add a new property like `disable-strobe-pulldown`. > > > Obviously there are tradeoffs between that and what you've done and > I'm happy to let others make the call of which they'd prefer. > Christopher could you try "ROCK Pi 4" and "ROCK Pi 4+" with "enable-strobe-pulldown" instead of disabling HS400 as you did in July 2023? Could the behavior be reverted to let the vendor kernel default for the default case (ie enable pulldown)? I believe 99% of the boards are now broken with this new internal pulldown behavior (all these with internal pulldown). More on that later but to sum up, nobody complained because downstream kernels like Armbian all disabled HS400 for all boards, at least for rk3399. Do we really want to ask 99% of the board dts to add this "enable- strobe-pulldown" in their dts? Chris, was your custom board not working with the vender kernel default to enable unconditionaly? What was the rationale to choose the opposite default from the vendor kernel one? As told in the commit that introduced this new behavior the default for the vendor kernel was the opposite of what was introduced in the Linux kernel: " https://github.com/torvalds/linux/commit/8b5c2b45b8f0a11c9072da0f7baf9ee986d3151e commit 8b5c2b45b8f0a11c9072da0f7baf9ee986d3151e Author: Chris Ruehl <chris.ruehl@xxxxxxxxxxxx> Date: Sun Nov 29 13:44:14 2020 +0800 phy: rockchip: set pulldown for strobe line in dts This patch add support to set the internal pulldown via dt property and allow simplify the board design for the trace from emmc-phy to the eMMC chipset. Default to not set the pull-down. This patch was inspired from the 4.4 tree of the Rockchip SDK, where it is enabled unconditional. The patch had been tested with our rk3399 customized board. " For RK3588 I see this commit which makes me believe the internal pulldown case is the most common " commit 37f3d6108730713c411827ab4af764909f4dfc78 Author: Sam Edwards <cfsworks@xxxxxxxxx> Date: Tue Dec 5 12:29:00 2023 -0800 arm64: dts: rockchip: Fix eMMC Data Strobe PD on rk3588 JEDEC standard JESD84-B51 defines the eMMC Data Strobe line, which is currently used only in HS400 mode, as a device->host clock signal that "is used only in read operation. The Data Strobe is always High-Z (not driven by the device and pulled down by RDS) or Driven Low in write operation, except during CRC status response." RDS is a pull-down resistor specified in the 10K-100K ohm range. Thus per the standard, the Data Strobe is always pulled to ground (by the eMMC and/or RDS) during write operations. Evidently, the eMMC host controller in the RK3588 considers an active voltage on the eMMC-DS line during a write to be an error. The default (i.e. hardware reset, and Rockchip BSP) behavior for the RK3588 is to activate the eMMC-DS pin's builtin pull-down. As a result, many RK3588 board designers do not bother adding a dedicated RDS resistor, instead relying on the RK3588's internal bias. The current devicetree, however, disables this bias (`pcfg_pull_none`), breaking HS400-mode writes for boards without a dedicated RDS, but with an eMMC chip that chooses to High-Z (instead of drive-low) the eMMC-DS line. (The Turing RK1 is one such board.) Fix this by changing the bias in the (common) emmc_data_strobe case to reflect the expected hardware/BSP behavior. This is unlikely to cause regressions elsewhere: the pull-down is only relevant for High-Z eMMCs, and if this is redundant with a (dedicated) RDS resistor, the effective result is only a lower resistance to ground -- where the range of tolerance is quite high. If it does, it's better fixed in the specific devicetrees. " Lately two other upstream dts disabled HS400 due to this new behavior I believe: " https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/arch/arm64/boot/dts/rockchip?id=2bd1d2dd808c60532283e9cf05110bf1bf2f9079 author Christopher Obbard <chris.obbard@xxxxxxxxxxxxx> 2023- 07-05 15:42:55 +0100 committer Heiko Stuebner <heiko@xxxxxxxxx> 2023-07-10 15:43:24 +0200 commit 2bd1d2dd808c60532283e9cf05110bf1bf2f9079 (patch) tree 57cbf7eaa91deb68f143577d5d1dbc0d9141480e /arch/arm64/boot/dts/rockchip parent cee572756aa2cb46e959e9797ad4b730b78a050b (diff) download linux-2bd1d2dd808c60532283e9cf05110bf1bf2f9079.tar.gz arm64: dts: rockchip: Disable HS400 for eMMC on ROCK 4C+ https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/arch/arm64/boot/dts/rockchip?id=cee572756aa2cb46e959e9797ad4b730b78a050b author Christopher Obbard <chris.obbard@xxxxxxxxxxxxx> 2023- 07-05 15:42:54 +0100 committer Heiko Stuebner <heiko@xxxxxxxxx> 2023-07-10 15:43:24 +0200 commit cee572756aa2cb46e959e9797ad4b730b78a050b (patch) tree cf3ed8ff6230cbde04353503417c1a75ba15c249 /arch/arm64/boot/dts/rockchip parent 5ce6971e5279c569defc2f2ac800692049bbaa90 (diff) download linux-cee572756aa2cb46e959e9797ad4b730b78a050b.tar.gz arm64: dts: rockchip: Disable HS400 for eMMC on ROCK Pi 4 " All Armbian RK3399 boards, as far as I know, had to disable HS400, I also believe due to this commit. You can also search google for "running cqe recovery rk3399 armbian". This was never reported upstream though. But as HS400 is disabled everywhere nobody notice the regression nowadays. > > > BR, > > Jensen > > > > On Tue, Aug 23, 2022 at 10:13 PM Doug Anderson > > <dianders@xxxxxxxxxxxx> wrote: > > > > > > Hi, > > > > > > On Tue, Aug 23, 2022 at 4:53 AM Heiko Stübner <heiko@xxxxxxxxx> > > > wrote: > > > > > > > > Am Montag, 22. August 2022, 09:41:39 CEST schrieb Jensen Huang: > > > > > Internal pull-down for strobe line (GRF_EMMCPHY_CON2[9]) was > > > > > disabled > > > > > by commit 8b5c2b45b8f0, which causes I/O error in HS400 mode. > > > > > > > > > > Tested on NanoPC-T4. > > > > > > > > > > Fixes: 8b5c2b45b8f0 ("phy: rockchip: set pulldown for strobe > > > > > line in dts") > > > > > Signed-off-by: Jensen Huang <jensenhuang@xxxxxxxxxxxxxxx> > > > > > > > > ok, so this looks like it restores previous functionality. > > > > > > > > I'm just wondering as the "offending" patch is from 2020, why > > > > this > > > > only turns up now. Any ideas? > > > Probbaly because the introduction of PROBE_DEFERRED in regulator core before that (in 5.10.60) already broke at least my board HS400 due to double init. Thus why it took me so long to see this new pulldown behavior commit. I was testing every regulator core double init fixup patchset while not understanding why reverting the PROBE_DEFERRED commit on 5.10.60 worked but not on newer kernels (ie this new pulldown behavior was introduced in 5.11...). > > > Ah, I see. So before the offending patch we used to just leave > > > the > > > pull state at whatever the default was when the kernel was > > > booted. > > > After the offending patch we chose a default. > > > > > > On kevin I see an external pull down on this line. Enabling both > > > the > > > internal and external is probably not a huge deal, it'll just > > > affect > > > the strength of the pull. > > > > > > On bob I _think_ the external pull down is also stuffed. > > > > > > ...so I guess that would explain why it didn't cause a problem > > > for at > > > least those two boards? > > > > > > -Doug In my opinion it is about these board already being broken by the regulator core change, so nobody noticed the second regression. When the first regression was fixed, it was very hard to correlate the still broken behavior to the second regression. I confirm that on Helios64, setting "enable-strobe-pulldown" fixes the EMMC error I had when writing with HS400ES enabled: mmc1: running CQE recovery mmc1: cqhci: spurious TCN for tag 12 It also took me so long to report upstream as my board code (rk3399- kobol-helios64.dts) is not completely upstreamed yet so I use an Armbian patched kernel. Alban
