Skip to contentSkip to conversation

ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

PATCH New Bartlomiej Zolnierkiewicz 5 participants 16 messages Oct 2, 2014 – Oct 9, 2014

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

Regulators for MMC2 (SD card) are PVDD_TFLASH_2V8 (LDO19) for vmmc and PVDD_APIO_MMCOFF_2V8 (LDO13) for vqmmc. Currently the device tree entry for MMC2 uses PVDD_PRE_1V8 (LDO10) for vmmc and vqmmc is not specified. Fix it.

Without this patch:
- "mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators" patch causes a SD card detection to fail
- "mmc: dw_mmc: Support voltage changes" patch causes a boot hang

This patch fixes both above problems.

---
 arch/arm/boot/dts/exynos5420-arndale-octa.dts |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/arm/boot/dts/exynos5420-arndale-octa.dts +2 -1
Index: b/arch/arm/boot/dts/exynos5420-arndale-octa.dts
===================================================================
@@ -74,7 +74,8 @@
 		samsung,dw-mshc-ddr-timing = <1 2>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
-		vmmc-supply = <&ldo10_reg>;
+		vmmc-supply = <&ldo19_reg>;
+		vqmmc-supply = <&ldo13_reg>;
 
 		slot at 0 {
 			reg = <0>;
#1 lore raw

PATCH 1/1 ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

Bartiomiej,

Quoted 26 lines · Bartlomiej Zolnierkiewicz

Regulators for MMC2 (SD card) are PVDD_TFLASH_2V8 (LDO19) for vmmc and PVDD_APIO_MMCOFF_2V8 (LDO13) for vqmmc. Currently the device tree entry for MMC2 uses PVDD_PRE_1V8 (LDO10) for vmmc and vqmmc is not specified. Fix it.

Without this patch:
- "mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators" patch causes a SD card detection to fail
- "mmc: dw_mmc: Support voltage changes" patch causes a boot hang

This patch fixes both above problems.

---
 arch/arm/boot/dts/exynos5420-arndale-octa.dts |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: b/arch/arm/boot/dts/exynos5420-arndale-octa.dts
===================================================================

arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 17:35:24.110600398 +0200 line 78

Quoted 6 lines
arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 17:35:24.110600398 +0200 renamed from arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 15:44:53.014826886 +0200 +2 -1
@@ -74,7 +74,8 @@
                samsung,dw-mshc-ddr-timing = <1 2>;
                pinctrl-names = "default";
                pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
-               vmmc-supply = <&ldo10_reg>;
+               vmmc-supply = <&ldo19_reg>;
+               vqmmc-supply = <&ldo13_reg>;

This looks right to me. ...but I notice that ldo13 and ldo19 are not "always-on" in the DTS. Are you sure card detect works for you if you eject your card and try to put it back in?

...eventually the "always-on" won't be needed, but for now I think it is...

-Doug

#2 lore raw

PATCH 1/1 ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

Quoted 41 lines · Doug Anderson

Bartiomiej,

Quoted 26 lines · Bartlomiej Zolnierkiewicz

Regulators for MMC2 (SD card) are PVDD_TFLASH_2V8 (LDO19) for vmmc and PVDD_APIO_MMCOFF_2V8 (LDO13) for vqmmc. Currently the device tree entry for MMC2 uses PVDD_PRE_1V8 (LDO10) for vmmc and vqmmc is not specified. Fix it.

Without this patch:
- "mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators" patch causes a SD card detection to fail
- "mmc: dw_mmc: Support voltage changes" patch causes a boot hang

This patch fixes both above problems.

---
 arch/arm/boot/dts/exynos5420-arndale-octa.dts |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: b/arch/arm/boot/dts/exynos5420-arndale-octa.dts
===================================================================

arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 17:35:24.110600398 +0200 line 78

Quoted 6 lines
arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 17:35:24.110600398 +0200 renamed from arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 15:44:53.014826886 +0200 +2 -1
@@ -74,7 +74,8 @@
                samsung,dw-mshc-ddr-timing = <1 2>;
                pinctrl-names = "default";
                pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
-               vmmc-supply = <&ldo10_reg>;
+               vmmc-supply = <&ldo19_reg>;
+               vqmmc-supply = <&ldo13_reg>;

This looks right to me. ...but I notice that ldo13 and ldo19 are not "always-on" in the DTS. Are you sure card detect works for you if you eject your card and try to put it back in?

Quoted 1 line

...eventually the "always-on" won't be needed, but for now I think it is...

Card detection works fine without "always-on".

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

#3 lore raw

PATCH 1/1 ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

Bartiomiej

Quoted 42 lines · Bartlomiej Zolnierkiewicz
Quoted 41 lines · Doug Anderson

Bartiomiej,

Quoted 26 lines · Bartlomiej Zolnierkiewicz

Regulators for MMC2 (SD card) are PVDD_TFLASH_2V8 (LDO19) for vmmc and PVDD_APIO_MMCOFF_2V8 (LDO13) for vqmmc. Currently the device tree entry for MMC2 uses PVDD_PRE_1V8 (LDO10) for vmmc and vqmmc is not specified. Fix it.

Without this patch:
- "mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators" patch causes a SD card detection to fail
- "mmc: dw_mmc: Support voltage changes" patch causes a boot hang

This patch fixes both above problems.

---
 arch/arm/boot/dts/exynos5420-arndale-octa.dts |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: b/arch/arm/boot/dts/exynos5420-arndale-octa.dts
===================================================================

arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 17:35:24.110600398 +0200 line 78

Quoted 6 lines
arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 17:35:24.110600398 +0200 renamed from arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 15:44:53.014826886 +0200 +2 -1
@@ -74,7 +74,8 @@
                samsung,dw-mshc-ddr-timing = <1 2>;
                pinctrl-names = "default";
                pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
-               vmmc-supply = <&ldo10_reg>;
+               vmmc-supply = <&ldo19_reg>;
+               vqmmc-supply = <&ldo13_reg>;

This looks right to me. ...but I notice that ldo13 and ldo19 are not "always-on" in the DTS. Are you sure card detect works for you if you eject your card and try to put it back in?

Quoted 3 lines
Quoted 1 line

...eventually the "always-on" won't be needed, but for now I think it is...

Card detection works fine without "always-on".

That's weird.

1. In the schematics I see XMMC2CDN has an external pullup to PVDD_TFLASH_2V8.

2. The internal pullup should (I think) be to VDDQ_MMC2 which is PVDD_APIO_MMCOFF_2V8.

3. In (51da224 mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators) we should be turning off both regulators in "MMC_POWER_OFF".

4. If I understand correctly MMC_POWER_OFF is called when the card is ejected, which means that both regulators should be off when the card is ejected.

5. I don't think card detect can work if neither regulator is powered.

One of the above points must be wrong. Any idea which one? Can you check to see if MMC_POWER_OFF is called for you when the card is ejected? Can you check to see if these regulators are off?

-Doug

#4 lore raw

PATCH 1/1 ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

Hi,

Quoted 65 lines · Doug Anderson

Bartiomiej

Quoted 42 lines · Bartlomiej Zolnierkiewicz
Quoted 41 lines · Doug Anderson

Bartiomiej,

Quoted 26 lines · Bartlomiej Zolnierkiewicz

Regulators for MMC2 (SD card) are PVDD_TFLASH_2V8 (LDO19) for vmmc and PVDD_APIO_MMCOFF_2V8 (LDO13) for vqmmc. Currently the device tree entry for MMC2 uses PVDD_PRE_1V8 (LDO10) for vmmc and vqmmc is not specified. Fix it.

Without this patch:
- "mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators" patch causes a SD card detection to fail
- "mmc: dw_mmc: Support voltage changes" patch causes a boot hang

This patch fixes both above problems.

---
 arch/arm/boot/dts/exynos5420-arndale-octa.dts |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: b/arch/arm/boot/dts/exynos5420-arndale-octa.dts
===================================================================

arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 17:35:24.110600398 +0200 line 78

Quoted 6 lines
arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 17:35:24.110600398 +0200 renamed from arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 15:44:53.014826886 +0200 +2 -1
@@ -74,7 +74,8 @@
                samsung,dw-mshc-ddr-timing = <1 2>;
                pinctrl-names = "default";
                pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
-               vmmc-supply = <&ldo10_reg>;
+               vmmc-supply = <&ldo19_reg>;
+               vqmmc-supply = <&ldo13_reg>;

This looks right to me. ...but I notice that ldo13 and ldo19 are not "always-on" in the DTS. Are you sure card detect works for you if you eject your card and try to put it back in?

Quoted 3 lines
Quoted 1 line

...eventually the "always-on" won't be needed, but for now I think it is...

Card detection works fine without "always-on".

That's weird.

1. In the schematics I see XMMC2CDN has an external pullup to PVDD_TFLASH_2V8.

2. The internal pullup should (I think) be to VDDQ_MMC2 which is PVDD_APIO_MMCOFF_2V8.

3. In (51da224 mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators) we should be turning off both regulators in "MMC_POWER_OFF".

4. If I understand correctly MMC_POWER_OFF is called when the card is ejected, which means that both regulators should be off when the card is ejected.

Quoted 6 lines

5. I don't think card detect can work if neither regulator is powered.

One of the above points must be wrong. Any idea which one? Can you check to see if MMC_POWER_OFF is called for you when the card is ejected? Can you check to see if these regulators are off?

MMC_POWER_OFF is called on card removal and both regulators get disabled (I have verified that they are really off with regulator_is_enabled() which returns 1 before and 0 after disabling regulator). It seems that 5. is wrong?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

#5 lore raw

PATCH 1/1 ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

Bartlomiej,

Quoted 72 lines · Bartlomiej Zolnierkiewicz

Hi,

Quoted 68 lines · Doug Anderson

Bartiomiej

Quoted 42 lines · Bartlomiej Zolnierkiewicz
Quoted 41 lines · Doug Anderson

Bartiomiej,

Quoted 26 lines · Bartlomiej Zolnierkiewicz

Regulators for MMC2 (SD card) are PVDD_TFLASH_2V8 (LDO19) for vmmc and PVDD_APIO_MMCOFF_2V8 (LDO13) for vqmmc. Currently the device tree entry for MMC2 uses PVDD_PRE_1V8 (LDO10) for vmmc and vqmmc is not specified. Fix it.

Without this patch:
- "mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators" patch causes a SD card detection to fail
- "mmc: dw_mmc: Support voltage changes" patch causes a boot hang

This patch fixes both above problems.

---
 arch/arm/boot/dts/exynos5420-arndale-octa.dts |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: b/arch/arm/boot/dts/exynos5420-arndale-octa.dts
===================================================================

arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 17:35:24.110600398 +0200 line 78

Quoted 6 lines
arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 17:35:24.110600398 +0200 renamed from arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 15:44:53.014826886 +0200 +2 -1
@@ -74,7 +74,8 @@
                samsung,dw-mshc-ddr-timing = <1 2>;
                pinctrl-names = "default";
                pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
-               vmmc-supply = <&ldo10_reg>;
+               vmmc-supply = <&ldo19_reg>;
+               vqmmc-supply = <&ldo13_reg>;

This looks right to me. ...but I notice that ldo13 and ldo19 are not "always-on" in the DTS. Are you sure card detect works for you if you eject your card and try to put it back in?

Quoted 3 lines
Quoted 1 line

...eventually the "always-on" won't be needed, but for now I think it is...

Card detection works fine without "always-on".

That's weird.

1. In the schematics I see XMMC2CDN has an external pullup to PVDD_TFLASH_2V8.

2. The internal pullup should (I think) be to VDDQ_MMC2 which is PVDD_APIO_MMCOFF_2V8.

3. In (51da224 mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators) we should be turning off both regulators in "MMC_POWER_OFF".

4. If I understand correctly MMC_POWER_OFF is called when the card is ejected, which means that both regulators should be off when the card is ejected.

5. I don't think card detect can work if neither regulator is powered.

Quoted 8 lines
Quoted 3 lines

One of the above points must be wrong. Any idea which one? Can you check to see if MMC_POWER_OFF is called for you when the card is ejected? Can you check to see if these regulators are off?

MMC_POWER_OFF is called on card removal and both regulators get disabled (I have verified that they are really off with regulator_is_enabled() which returns 1 before and 0 after disabling regulator). It seems that 5. is wrong?

This really doesn't make a lot of sense to me, so I'm still kinda confused. If you want to call it good then that's your (and Ulf's) decision, but it's the kind of thing that would keep me up at night. How can this pin be high if all the regulators pulling it up are off? Is there a current leak somewhere and that's why it's working?

How this is supposed to work (as I understand it):

1. When no card is inserted then this pin is supposed to be pulled up to VDDQ_MMC2. That could be either an internal or an external pullup. It should be pulled up to VDDQ_MMC2 (as opposed to any other voltage) since the exynos manual documents that this pin lives in the VDDQ_MMC2 io domain. Note that it could be pulled up externally to a different supply than the one going to VDDQ_MMC2, but for correctness it should be the same voltage.

2. When a card is inserted, the pin will be grounded (AKA this is an open drain pin).

With your patch, can you probe the pin and see if card detect is high when all the regulators are off? Any idea how it gets high? If you turn off the internal pullup is it still high?

-Doug

#6 lore raw

PATCH 1/1 ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

Hi,

Quoted 101 lines · Doug Anderson

Bartlomiej,

Quoted 72 lines · Bartlomiej Zolnierkiewicz

Hi,

Quoted 68 lines · Doug Anderson

Bartiomiej

Quoted 42 lines · Bartlomiej Zolnierkiewicz
Quoted 41 lines · Doug Anderson

Bartiomiej,

Quoted 26 lines · Bartlomiej Zolnierkiewicz

Regulators for MMC2 (SD card) are PVDD_TFLASH_2V8 (LDO19) for vmmc and PVDD_APIO_MMCOFF_2V8 (LDO13) for vqmmc. Currently the device tree entry for MMC2 uses PVDD_PRE_1V8 (LDO10) for vmmc and vqmmc is not specified. Fix it.

Without this patch:
- "mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators" patch causes a SD card detection to fail
- "mmc: dw_mmc: Support voltage changes" patch causes a boot hang

This patch fixes both above problems.

---
 arch/arm/boot/dts/exynos5420-arndale-octa.dts |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: b/arch/arm/boot/dts/exynos5420-arndale-octa.dts
===================================================================

arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 17:35:24.110600398 +0200 line 78

Quoted 6 lines
arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 17:35:24.110600398 +0200 renamed from arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 15:44:53.014826886 +0200 +2 -1
@@ -74,7 +74,8 @@
                samsung,dw-mshc-ddr-timing = <1 2>;
                pinctrl-names = "default";
                pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
-               vmmc-supply = <&ldo10_reg>;
+               vmmc-supply = <&ldo19_reg>;
+               vqmmc-supply = <&ldo13_reg>;

This looks right to me. ...but I notice that ldo13 and ldo19 are not "always-on" in the DTS. Are you sure card detect works for you if you eject your card and try to put it back in?

Quoted 3 lines
Quoted 1 line

...eventually the "always-on" won't be needed, but for now I think it is...

Card detection works fine without "always-on".

That's weird.

1. In the schematics I see XMMC2CDN has an external pullup to PVDD_TFLASH_2V8.

2. The internal pullup should (I think) be to VDDQ_MMC2 which is PVDD_APIO_MMCOFF_2V8.

3. In (51da224 mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators) we should be turning off both regulators in "MMC_POWER_OFF".

4. If I understand correctly MMC_POWER_OFF is called when the card is ejected, which means that both regulators should be off when the card is ejected.

5. I don't think card detect can work if neither regulator is powered.

Quoted 8 lines
Quoted 3 lines

One of the above points must be wrong. Any idea which one? Can you check to see if MMC_POWER_OFF is called for you when the card is ejected? Can you check to see if these regulators are off?

MMC_POWER_OFF is called on card removal and both regulators get disabled (I have verified that they are really off with regulator_is_enabled() which returns 1 before and 0 after disabling regulator). It seems that 5. is wrong?

This really doesn't make a lot of sense to me, so I'm still kinda confused. If you want to call it good then that's your (and Ulf's) decision, but it's the kind of thing that would keep me up at night. How can this pin be high if all the regulators pulling it up are off? Is there a current leak somewhere and that's why it's working?

How this is supposed to work (as I understand it):

1. When no card is inserted then this pin is supposed to be pulled up to VDDQ_MMC2. That could be either an internal or an external pullup. It should be pulled up to VDDQ_MMC2 (as opposed to any other voltage) since the exynos manual documents that this pin lives in the VDDQ_MMC2 io domain. Note that it could be pulled up externally to a different supply than the one going to VDDQ_MMC2, but for correctness it should be the same voltage.

Quoted 7 lines

2. When a card is inserted, the pin will be grounded (AKA this is an open drain pin).

With your patch, can you probe the pin and see if card detect is high when all the regulators are off? Any idea how it gets high? If you turn off the internal pullup is it still high?

I remembered that I and Doug were discussed for this problem with exynos5420-peach board(?), right? Is arndale-octa board the same circuit with peach?
If it's same, I think Doug's comment is right.
But if card-detect pis is used with other power, we don't need to consider the VDDQ_MMC2 power domain. It needs more information and checks its schematic.

Best Regards,
Jaehoon Chung

Quoted 4 lines

-Doug

#7 lore raw

PATCH 1/1 ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

Quoted 112 lines · Jaehoon Chung

Hi,

Quoted 101 lines · Doug Anderson

Bartlomiej,

Quoted 72 lines · Bartlomiej Zolnierkiewicz

Hi,

Quoted 68 lines · Doug Anderson

Bartiomiej

Quoted 42 lines · Bartlomiej Zolnierkiewicz
Quoted 41 lines · Doug Anderson

Bartiomiej,

Quoted 26 lines · Bartlomiej Zolnierkiewicz

Regulators for MMC2 (SD card) are PVDD_TFLASH_2V8 (LDO19) for vmmc and PVDD_APIO_MMCOFF_2V8 (LDO13) for vqmmc. Currently the device tree entry for MMC2 uses PVDD_PRE_1V8 (LDO10) for vmmc and vqmmc is not specified. Fix it.

Without this patch:
- "mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators" patch causes a SD card detection to fail
- "mmc: dw_mmc: Support voltage changes" patch causes a boot hang

This patch fixes both above problems.

---
 arch/arm/boot/dts/exynos5420-arndale-octa.dts |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: b/arch/arm/boot/dts/exynos5420-arndale-octa.dts
===================================================================

arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 17:35:24.110600398 +0200 line 78

Quoted 6 lines
arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 17:35:24.110600398 +0200 renamed from arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 15:44:53.014826886 +0200 +2 -1
@@ -74,7 +74,8 @@
                samsung,dw-mshc-ddr-timing = <1 2>;
                pinctrl-names = "default";
                pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
-               vmmc-supply = <&ldo10_reg>;
+               vmmc-supply = <&ldo19_reg>;
+               vqmmc-supply = <&ldo13_reg>;

This looks right to me. ...but I notice that ldo13 and ldo19 are not "always-on" in the DTS. Are you sure card detect works for you if you eject your card and try to put it back in?

Quoted 3 lines
Quoted 1 line

...eventually the "always-on" won't be needed, but for now I think it is...

Card detection works fine without "always-on".

That's weird.

1. In the schematics I see XMMC2CDN has an external pullup to PVDD_TFLASH_2V8.

2. The internal pullup should (I think) be to VDDQ_MMC2 which is PVDD_APIO_MMCOFF_2V8.

3. In (51da224 mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators) we should be turning off both regulators in "MMC_POWER_OFF".

4. If I understand correctly MMC_POWER_OFF is called when the card is ejected, which means that both regulators should be off when the card is ejected.

5. I don't think card detect can work if neither regulator is powered.

Quoted 8 lines
Quoted 3 lines

One of the above points must be wrong. Any idea which one? Can you check to see if MMC_POWER_OFF is called for you when the card is ejected? Can you check to see if these regulators are off?

MMC_POWER_OFF is called on card removal and both regulators get disabled (I have verified that they are really off with regulator_is_enabled() which returns 1 before and 0 after disabling regulator). It seems that 5. is wrong?

This really doesn't make a lot of sense to me, so I'm still kinda confused. If you want to call it good then that's your (and Ulf's) decision, but it's the kind of thing that would keep me up at night. How can this pin be high if all the regulators pulling it up are off? Is there a current leak somewhere and that's why it's working?

How this is supposed to work (as I understand it):

1. When no card is inserted then this pin is supposed to be pulled up to VDDQ_MMC2. That could be either an internal or an external pullup. It should be pulled up to VDDQ_MMC2 (as opposed to any other voltage) since the exynos manual documents that this pin lives in the VDDQ_MMC2 io domain. Note that it could be pulled up externally to a different supply than the one going to VDDQ_MMC2, but for correctness it should be the same voltage.

Quoted 7 lines

2. When a card is inserted, the pin will be grounded (AKA this is an open drain pin).

With your patch, can you probe the pin and see if card detect is high when all the regulators are off? Any idea how it gets high? If you turn off the internal pullup is it still high?

Quoted 5 lines

I remembered that I and Doug were discussed for this problem with exynos5420-peach board(?), right? Is arndale-octa board the same circuit with peach?
If it's same, I think Doug's comment is right.
But if card-detect pis is used with other power, we don't need to consider the VDDQ_MMC2 power domain. It needs more information and checks its schematic.

May I suggest we go ahead and apply this patch to fix the problems for Arndale Octa!? I am soon about to send the PR for 3.18.

We should then follow up on this discussion and sort out why/how it actually works.

So, I need an ack from some of the ARM SOC maintainers. I have looped in Arnd Bergmann, if Kukjin is busy.

Kind regards
Uffe

#8 lore raw

PATCH 1/1 ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

In reply to Bartlomiej Zolnierkiewicz
Quoted 10 lines · Bartlomiej Zolnierkiewicz

Regulators for MMC2 (SD card) are PVDD_TFLASH_2V8 (LDO19) for vmmc and PVDD_APIO_MMCOFF_2V8 (LDO13) for vqmmc. Currently the device tree entry for MMC2 uses PVDD_PRE_1V8 (LDO10) for vmmc and vqmmc is not specified. Fix it.

Without this patch:
- "mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators" patch causes a SD card detection to fail
- "mmc: dw_mmc: Support voltage changes" patch causes a boot hang

Quoted 7 lines

This patch fixes both above problems.

Unless it causes a bunch non-trivial conflicts, I suggest we take this through my mmc tree to not break bisectability.

Kind regards
Uffe

Quoted 19 lines
---
 arch/arm/boot/dts/exynos5420-arndale-octa.dts |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: b/arch/arm/boot/dts/exynos5420-arndale-octa.dts
===================================================================

arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 17:35:24.110600398 +0200 renamed from arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 15:44:53.014826886 +0200 +2 -1
@@ -74,7 +74,8 @@
                samsung,dw-mshc-ddr-timing = <1 2>;
                pinctrl-names = "default";
                pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
-               vmmc-supply = <&ldo10_reg>;
+               vmmc-supply = <&ldo19_reg>;
+               vqmmc-supply = <&ldo13_reg>;
 
                slot at 0 {
                        reg = <0>;
#9 lore raw

PATCH 1/1 ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

Quoted 14 lines · On Thursday 02 October 2014 20:16:44 Ulf Hansson wrote:
Quoted 12 lines · Bartlomiej Zolnierkiewicz

Regulators for MMC2 (SD card) are PVDD_TFLASH_2V8 (LDO19) for vmmc and PVDD_APIO_MMCOFF_2V8 (LDO13) for vqmmc. Currently the device tree entry for MMC2 uses PVDD_PRE_1V8 (LDO10) for vmmc and vqmmc is not specified. Fix it.

Without this patch:
- "mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators" patch causes a SD card detection to fail
- "mmc: dw_mmc: Support voltage changes" patch causes a boot hang

This patch fixes both above problems.

Quoted 8 lines
Quoted 5 lines

Unless it causes a bunch non-trivial conflicts, I suggest we take this through my mmc tree to not break bisectability.

Have you tried if it conflicts with arm-soc? If not,

#10 lore raw

PATCH 1/1 ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

Quoted 21 lines · Arnd Bergmann
Quoted 20 lines · On Thursday 02 October 2014 20:16:44 Ulf Hansson wrote:
Quoted 17 lines · Bartlomiej Zolnierkiewicz

Regulators for MMC2 (SD card) are PVDD_TFLASH_2V8 (LDO19) for vmmc and PVDD_APIO_MMCOFF_2V8 (LDO13) for vqmmc. Currently the device tree entry for MMC2 uses PVDD_PRE_1V8 (LDO10) for vmmc and vqmmc is not specified. Fix it.

Without this patch:
- "mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators" patch causes a SD card detection to fail
- "mmc: dw_mmc: Support voltage changes" patch causes a boot hang

This patch fixes both above problems.

Quoted 6 lines
Quoted 2 lines

Unless it causes a bunch non-trivial conflicts, I suggest we take this through my mmc tree to not break bisectability.

Have you tried if it conflicts with arm-soc? If not,

In conflicts, but it's trivial to fix it.

Since it actually fixes a regression which is inserted from my mmc tree, I thought it makes sense to take it through here. Anyway, the decision is yours.

Br
Uffe

#11 lore raw

PATCH 1/1 ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

Quoted 27 lines · On Wednesday 08 October 2014 14:50:46 Ulf Hansson wrote:
Quoted 26 lines · Arnd Bergmann
Quoted 14 lines · On Thursday 02 October 2014 20:16:44 Ulf Hansson wrote:
Quoted 12 lines · Bartlomiej Zolnierkiewicz

Regulators for MMC2 (SD card) are PVDD_TFLASH_2V8 (LDO19) for vmmc and PVDD_APIO_MMCOFF_2V8 (LDO13) for vqmmc. Currently the device tree entry for MMC2 uses PVDD_PRE_1V8 (LDO10) for vmmc and vqmmc is not specified. Fix it.

Without this patch:
- "mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators" patch causes a SD card detection to fail
- "mmc: dw_mmc: Support voltage changes" patch causes a boot hang

This patch fixes both above problems.

Quoted 8 lines
Quoted 5 lines

Unless it causes a bunch non-trivial conflicts, I suggest we take this through my mmc tree to not break bisectability.

Have you tried if it conflicts with arm-soc? If not,

Quoted 8 lines
Quoted 1 line

In conflicts, but it's trivial to fix it.

Since it actually fixes a regression which is inserted from my mmc tree, I thought it makes sense to take it through here. Anyway, the decision is yours.

Why does it cause a regression though? Does this mean you are breaking any boot with an old DT file and a new kernel? That would be very bad, the driver is supposed to keep working with an existing dtb.

Arnd

#12 lore raw

PATCH 1/1 ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

Hi,

Quoted 3 lines · Arnd Bergmann

Why does it cause a regression though? Does this mean you are breaking any boot with an old DT file and a new kernel? That would be very bad, the driver is supposed to keep working with an existing dtb.

The old dts file specified completely the wrong regulator. It happened that the old code didn't really care, but when the code was fixed to care then things broke. If we need to keep old dts files working then the most sensible place to do it would be in a "fixup old broken device trees" stage somewhere in kernel boot.

-Doug

#13 lore raw

PATCH 1/1 ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

Quoted 11 lines · On Wednesday 08 October 2014 08:29:16 Doug Anderson wrote:
Quoted 3 lines · Arnd Bergmann

Why does it cause a regression though? Does this mean you are breaking any boot with an old DT file and a new kernel? That would be very bad, the driver is supposed to keep working with an existing dtb.

The old dts file specified completely the wrong regulator. It happened that the old code didn't really care, but when the code was fixed to care then things broke. If we need to keep old dts files working then the most sensible place to do it would be in a "fixup old broken device trees" stage somewhere in kernel boot.

Ok, got it. I guess it's better to apply this one first then.

Arnd

#14 lore raw

PATCH 1/1 ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

Quoted 13 lines · Arnd Bergmann
Quoted 11 lines · On Wednesday 08 October 2014 08:29:16 Doug Anderson wrote:
Quoted 3 lines · Arnd Bergmann

Why does it cause a regression though? Does this mean you are breaking any boot with an old DT file and a new kernel? That would be very bad, the driver is supposed to keep working with an existing dtb.

The old dts file specified completely the wrong regulator. It happened that the old code didn't really care, but when the code was fixed to care then things broke. If we need to keep old dts files working then the most sensible place to do it would be in a "fixup old broken device trees" stage somewhere in kernel boot.

Quoted 3 lines

Ok, got it. I guess it's better to apply this one first then.

Arnd

Thanks for your comments Arnd! I have applied this once to my mmc tree now. Please tell me soon if you want to carry this through arm-soc instead, thus I can drop it.

Kind regards
Uffe

#15 lore raw

PATCH 1/1 ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

Quoted 14 lines · On Thursday 09 October 2014 10:46:59 Ulf Hansson wrote:
Quoted 13 lines · Arnd Bergmann
Quoted 11 lines · On Wednesday 08 October 2014 08:29:16 Doug Anderson wrote:
Quoted 3 lines · Arnd Bergmann

Why does it cause a regression though? Does this mean you are breaking any boot with an old DT file and a new kernel? That would be very bad, the driver is supposed to keep working with an existing dtb.

The old dts file specified completely the wrong regulator. It happened that the old code didn't really care, but when the code was fixed to care then things broke. If we need to keep old dts files working then the most sensible place to do it would be in a "fixup old broken device trees" stage somewhere in kernel boot.

Quoted 7 lines
Quoted 3 lines

Ok, got it. I guess it's better to apply this one first then.

Arnd

Thanks for your comments Arnd! I have applied this once to my mmc tree now. Please tell me soon if you want to carry this through arm-soc instead, thus I can drop it.

No, please take it yourself, Linus has already taken the arm-soc pull requests.

Arnd