[PATCH] ARM: dts: sunxi: Explicitly enable pull-ups for MMC pins

[PATCH] ARM: dts: sunxi: Explicitly enable pull-ups for MMC pins

[Chen-Yu Tsai]

In the past, all the MMC pins had

	allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;

which was actually a no-op. We were relying on U-boot to set the bias
pull up for us. These properties were removed as part of the fix up to
actually support no bias on the pins. During the transition some boards
experienced regular MMC time-outs during normal operation, while others
completely failed to initialize the SD card.

Given that MMC starts in open-drain mode and the pull-ups are required,
it's best to enable it for all the pin settings.

Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
---
 arch/arm/boot/dts/sun4i-a10.dtsi     | 1 +
 arch/arm/boot/dts/sun5i.dtsi         | 1 +
 arch/arm/boot/dts/sun6i-a31.dtsi     | 4 ++++
 arch/arm/boot/dts/sun7i-a20.dtsi     | 2 ++
 arch/arm/boot/dts/sun8i-a23-a33.dtsi | 3 +++
 arch/arm/boot/dts/sun8i-a83t.dtsi    | 1 +
 arch/arm/boot/dts/sun8i-h3.dtsi      | 3 +++
 arch/arm/boot/dts/sun9i-a80.dtsi     | 3 +++
 8 files changed, 18 insertions(+)

diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index dae838e4dd9e..ba20b48c0702 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -1023,6 +1023,7 @@
 				       "PF3", "PF4", "PF5";
 				function = "mmc0";
 				drive-strength = <30>;
+				bias-pull-up;
 			};
 
 			mmc0_cd_pin_reference_design: mmc0_cd_pin@0 {
diff --git a/arch/arm/boot/dts/sun5i.dtsi b/arch/arm/boot/dts/sun5i.dtsi
index 7ab6b336533e..54170147040f 100644
--- a/arch/arm/boot/dts/sun5i.dtsi
+++ b/arch/arm/boot/dts/sun5i.dtsi
@@ -582,6 +582,7 @@
 				       "PF4", "PF5";
 				function = "mmc0";
 				drive-strength = <30>;
+				bias-pull-up;
 			};
 
 			mmc2_pins_a: mmc2@0 {
diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index 7ea1116c7c88..20a0331ddfb5 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -547,6 +547,7 @@
 						 "PF3", "PF4", "PF5";
 				function = "mmc0";
 				drive-strength = <30>;
+				bias-pull-up;
 			};
 
 			mmc1_pins_a: mmc1@0 {
@@ -554,6 +555,7 @@
 						 "PG4", "PG5";
 				function = "mmc1";
 				drive-strength = <30>;
+				bias-pull-up;
 			};
 
 			mmc2_pins_a: mmc2@0 {
@@ -571,6 +573,7 @@
 						 "PC24";
 				function = "mmc2";
 				drive-strength = <30>;
+				bias-pull-up;
 			};
 
 			mmc3_8bit_emmc_pins: mmc3@1 {
@@ -580,6 +583,7 @@
 						 "PC24";
 				function = "mmc3";
 				drive-strength = <40>;
+				bias-pull-up;
 			};
 
 			uart0_pins_a: uart0@0 {
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 34d613b0dd73..a1ee4197129a 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -1179,6 +1179,7 @@
 				       "PF3", "PF4", "PF5";
 				function = "mmc0";
 				drive-strength = <30>;
+				bias-pull-up;
 			};
 
 			mmc0_cd_pin_reference_design: mmc0_cd_pin@0 {
@@ -1200,6 +1201,7 @@
 				       "PI7", "PI8", "PI9";
 				function = "mmc3";
 				drive-strength = <30>;
+				bias-pull-up;
 			};
 
 			ps20_pins_a: ps20@0 {
diff --git a/arch/arm/boot/dts/sun8i-a23-a33.dtsi b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
index ecb49a5a7615..bc3e936edfcf 100644
--- a/arch/arm/boot/dts/sun8i-a23-a33.dtsi
+++ b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
@@ -293,6 +293,7 @@
 				       "PF3", "PF4", "PF5";
 				function = "mmc0";
 				drive-strength = <30>;
+				bias-pull-up;
 			};
 
 			mmc1_pins_a: mmc1@0 {
@@ -300,6 +301,7 @@
 				       "PG3", "PG4", "PG5";
 				function = "mmc1";
 				drive-strength = <30>;
+				bias-pull-up;
 			};
 
 			mmc2_8bit_pins: mmc2_8bit {
@@ -309,6 +311,7 @@
 				       "PC15", "PC16";
 				function = "mmc2";
 				drive-strength = <30>;
+				bias-pull-up;
 			};
 
 			pwm0_pins: pwm0 {
diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 656cdb5f7a88..79eaa7139f43 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -171,6 +171,7 @@
 				       "PF3", "PF4", "PF5";
 				function = "mmc0";
 				drive-strength = <30>;
+				bias-pull-up;
 			};
 
 			uart0_pins_a: uart0@0 {
diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 416b825ddb9f..fca66bf2dec5 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -348,6 +348,7 @@
 				       "PF4", "PF5";
 				function = "mmc0";
 				drive-strength = <30>;
+				bias-pull-up;
 			};
 
 			mmc0_cd_pin: mmc0_cd_pin@0 {
@@ -361,6 +362,7 @@
 				       "PG4", "PG5";
 				function = "mmc1";
 				drive-strength = <30>;
+				bias-pull-up;
 			};
 
 			mmc2_8bit_pins: mmc2_8bit {
@@ -370,6 +372,7 @@
 				       "PC15", "PC16";
 				function = "mmc2";
 				drive-strength = <30>;
+				bias-pull-up;
 			};
 
 			spi0_pins: spi0 {
diff --git a/arch/arm/boot/dts/sun9i-a80.dtsi b/arch/arm/boot/dts/sun9i-a80.dtsi
index b97db1df0803..7231d2c90dde 100644
--- a/arch/arm/boot/dts/sun9i-a80.dtsi
+++ b/arch/arm/boot/dts/sun9i-a80.dtsi
@@ -696,6 +696,7 @@
 				       "PF4", "PF5";
 				function = "mmc0";
 				drive-strength = <30>;
+				bias-pull-up;
 			};
 
 			mmc1_pins: mmc1 {
@@ -703,6 +704,7 @@
 						 "PG4", "PG5";
 				function = "mmc1";
 				drive-strength = <30>;
+				bias-pull-up;
 			};
 
 			mmc2_8bit_pins: mmc2_8bit {
@@ -712,6 +714,7 @@
 				       "PC16";
 				function = "mmc2";
 				drive-strength = <30>;
+				bias-pull-up;
 			};
 
 			uart0_pins_a: uart0@0 {
-- 
2.10.2

Re: [PATCH] ARM: dts: sunxi: Explicitly enable pull-ups for MMC pins

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 810 bytes --]

On Thu, Nov 17, 2016 at 05:34:38PM +0800, Chen-Yu Tsai wrote:
> In the past, all the MMC pins had
> 
> 	allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
> 
> which was actually a no-op. We were relying on U-boot to set the bias
> pull up for us. These properties were removed as part of the fix up to
> actually support no bias on the pins. During the transition some boards
> experienced regular MMC time-outs during normal operation, while others
> completely failed to initialize the SD card.
> 
> Given that MMC starts in open-drain mode and the pull-ups are required,
> it's best to enable it for all the pin settings.
> 
> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>

Applied, thanks.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH] ARM: dts: sunxi: Explicitly enable pull-ups for MMC pins

[klaus.goger]

On 2016-11-17 10:34, Chen-Yu Tsai wrote:
> Given that MMC starts in open-drain mode and the pull-ups are required,
> it's best to enable it for all the pin settings.

It's even more complicated than that with MMC. It starts in open-drain 
mode for
CMD during initialization but changes to push-pull afterwards. The card 
has
internal pull-ups to prevent bus floating during setup and will disable 
them
after switching to 4bit mode (or 8bit for eMMC when available).
But even after switching to push-pull drivers there are states the bus 
would
float and pull ups have to ensure a high level on the bus.

See JESD84-A441 chapter 7.15 ff as reference.

   The difference between the P-bit and Z-bit is that a P-bit is actively 
driven
   to HIGH by the card respectively host output driver, while Z-bit is 
driven to
  (respectively kept) HIGH by the pull-up resistors Rcmd respectively 
Rdat.


Enabling the pull ups on the CPU would be the right choice considering 
that
most boards will not have external pull-ups. Even if they would have 
one,
adding the internal in parallel would work in almost all cases and the
increase in power consumption would be negligible.

Cheers,
Klaus

Re: [PATCH] ARM: dts: sunxi: Explicitly enable pull-ups for MMC pins

[Linus Walleij]

[Notice this reply has little to do with the patch in question: I think
it should be applied. I just want to involve some MMC/SD people here]

On Thu, Nov 17, 2016 at 9:57 PM,  <klaus.goger-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org> wrote:
> On 2016-11-17 10:34, Chen-Yu Tsai wrote:
>>
>> Given that MMC starts in open-drain mode and the pull-ups are required,
>> it's best to enable it for all the pin settings.
>
> It's even more complicated than that with MMC. It starts in open-drain mode
> for
> CMD during initialization but changes to push-pull afterwards. The card has
> internal pull-ups to prevent bus floating during setup and will disable them
> after switching to 4bit mode (or 8bit for eMMC when available).
> But even after switching to push-pull drivers there are states the bus would
> float and pull ups have to ensure a high level on the bus.
>
> See JESD84-A441 chapter 7.15 ff as reference.
>
>   The difference between the P-bit and Z-bit is that a P-bit is actively
> driven
>   to HIGH by the card respectively host output driver, while Z-bit is driven
> to
>  (respectively kept) HIGH by the pull-up resistors Rcmd respectively Rdat.
>
> Enabling the pull ups on the CPU would be the right choice considering that
> most boards will not have external pull-ups. Even if they would have one,
> adding the internal in parallel would work in almost all cases and the
> increase in power consumption would be negligible.

I guess you are referring to software-controlled pull up on the pad ring of
the SoC. Nominally that should be handled by pin control like in this
patch.

It is not clear from context to me which of the lines need to be handled
like this? Just the data lines? DAT0 would be the critical line to pull up
in that case, since the others will not be used until bus switch anyways
right?

But what about CMD?

I assume CLK should always be push-pull?

Also: if the pins have an explicit Open Drain setting, should that
be used? I guess so?

Overall I think this construction is pretty common.

We essentially have two classes of connections:

- Those connecting the eMMC or even MMC/SD card directly to
  the SoC. No pull-ups on the board. Here it makes sense to have:

  1. A pin control default state with open drain and pull-ups
     enabled for those lines

  and it then needs

  2. A second "4/8bit mode" that will switch these
     pins to push-pull mode and turn off the pull-ups.

  If the OD+pull-up state is the default we should probably
  standardize a default name for the state when we kick in 4/8bit
  mode from the IOS operation.

- Those connection the MMC/SD on an external slot through a
  levelshifter/EMI filter. In that case I guess it is pretty much
  dependent on the levelshifter or EMI thing how the lines out
  of the SoC should be configured, and usually it is static so
  you do not need to worry about it after boot configuration
  of pins. (Mostly no bias, push-pull I think.)

I highly suspect a whole bunch of SoC drivers are not getting
this business right today. Not even in out-of-tree vendor kernels.

Yours,
Linus Walleij