Re: [PATCH 6.6 126/244] arm64: dts: rockchip: Fix eMMC Data Strobe PD on rk3588

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/11/23 11:20, Greg Kroah-Hartman wrote:
6.6-stable review patch.  If anyone has any objections, please let me know.

Hi Greg,

This is my first stable review and I don't know the policy on what we do with "won't hurt, might help, not strictly needed" cases (which I believe this one is). I'll instead list the reasons for/against it (to give some background) and will let you/others make the call.

Reasons FOR including this patch in 6.6-stable:
- It is the correct (i.e. standards-compliant) thing to do.
- Because of that, I'd be very surprised if it caused a regression.
- It would be helpful to people who are backporting support for the
  affected board(s) onto 6.6 while they wait for 6.7. (I am one.)

Reasons AGAINST including this patch in 6.6-stable:
- The bug it fixes is a solid, reliable crash on boot, which happens
  virtually 100% of the time on affected boards. If it affected any of
  the boards supported by 6.6, we'd probably have heard of it by now.
- 6.6 isn't LTS, so it isn't likely to be targeted with backported
  support for affected board(s) from 6.7 once 6.7 releases. That is,
  my last point in favor only applies in the short term.

Ultimately, either including it or not will have my full support.

Hope this helps!

Happy Monday,
Sam


------------------

From: Sam Edwards <cfsworks@xxxxxxxxx>

[ Upstream commit 37f3d6108730713c411827ab4af764909f4dfc78 ]

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.

Fixes: d85f8a5c798d5 ("arm64: dts: rockchip: Add rk3588 pinctrl data")
Signed-off-by: Sam Edwards <CFSworks@xxxxxxxxx>
Link: https://lore.kernel.org/r/20231205202900.4617-2-CFSworks@xxxxxxxxx
Signed-off-by: Heiko Stuebner <heiko@xxxxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---
  arch/arm64/boot/dts/rockchip/rk3588s-pinctrl.dtsi | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-pinctrl.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s-pinctrl.dtsi
index 48181671eacb0..0933652bafc30 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-pinctrl.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-pinctrl.dtsi
@@ -369,7 +369,7 @@
  		emmc_data_strobe: emmc-data-strobe {
  			rockchip,pins =
  				/* emmc_data_strobe */
-				<2 RK_PA2 1 &pcfg_pull_none>;
+				<2 RK_PA2 1 &pcfg_pull_down>;
  		};
  	};




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux