[PATCH] ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

[PATCH] ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

[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.

Suggested-by: Doug Anderson <dianders@google.com>
Cc: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 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
===================================================================
--- a/arch/arm/boot/dts/exynos5420-arndale-octa.dts	2014-10-02 15:44:53.014826886 +0200
+++ b/arch/arm/boot/dts/exynos5420-arndale-octa.dts	2014-10-02 17:35:24.110600398 +0200
@@ -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>;

[PATCH] ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

[Doug Anderson]

Bartiomiej,

On Thu, Oct 2, 2014 at 9:10 AM, Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:
> 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.
>
> Suggested-by: Doug Anderson <dianders@google.com>
> Cc: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  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
> ===================================================================
> --- a/arch/arm/boot/dts/exynos5420-arndale-octa.dts     2014-10-02 15:44:53.014826886 +0200
> +++ b/arch/arm/boot/dts/exynos5420-arndale-octa.dts     2014-10-02 17:35:24.110600398 +0200
> @@ -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

[PATCH] ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

[Bartlomiej Zolnierkiewicz]

On Thursday, October 02, 2014 09:19:08 AM Doug Anderson wrote:
> Bartiomiej,
> 
> On Thu, Oct 2, 2014 at 9:10 AM, Bartlomiej Zolnierkiewicz
> <b.zolnierkie@samsung.com> wrote:
> > 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.
> >
> > Suggested-by: Doug Anderson <dianders@google.com>
> > Cc: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
> > Cc: Ulf Hansson <ulf.hansson@linaro.org>
> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> >  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
> > ===================================================================
> > --- a/arch/arm/boot/dts/exynos5420-arndale-octa.dts     2014-10-02 15:44:53.014826886 +0200
> > +++ b/arch/arm/boot/dts/exynos5420-arndale-octa.dts     2014-10-02 17:35:24.110600398 +0200
> > @@ -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...

Card detection works fine without "always-on".

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

[PATCH] ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

[Doug Anderson]

Bartiomiej

On Thu, Oct 2, 2014 at 9:39 AM, Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:
> On Thursday, October 02, 2014 09:19:08 AM Doug Anderson wrote:
>> Bartiomiej,
>>
>> On Thu, Oct 2, 2014 at 9:10 AM, Bartlomiej Zolnierkiewicz
>> <b.zolnierkie@samsung.com> wrote:
>> > 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.
>> >
>> > Suggested-by: Doug Anderson <dianders@google.com>
>> > Cc: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
>> > Cc: Ulf Hansson <ulf.hansson@linaro.org>
>> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
>> > Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>> > ---
>> >  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
>> > ===================================================================
>> > --- a/arch/arm/boot/dts/exynos5420-arndale-octa.dts     2014-10-02 15:44:53.014826886 +0200
>> > +++ b/arch/arm/boot/dts/exynos5420-arndale-octa.dts     2014-10-02 17:35:24.110600398 +0200
>> > @@ -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...
>
> 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

[PATCH] ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

[Bartlomiej Zolnierkiewicz]

Hi,

On Thursday, October 02, 2014 09:45:41 AM Doug Anderson wrote:
> Bartiomiej
> 
> On Thu, Oct 2, 2014 at 9:39 AM, Bartlomiej Zolnierkiewicz
> <b.zolnierkie@samsung.com> wrote:
> > On Thursday, October 02, 2014 09:19:08 AM Doug Anderson wrote:
> >> Bartiomiej,
> >>
> >> On Thu, Oct 2, 2014 at 9:10 AM, Bartlomiej Zolnierkiewicz
> >> <b.zolnierkie@samsung.com> wrote:
> >> > 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.
> >> >
> >> > Suggested-by: Doug Anderson <dianders@google.com>
> >> > Cc: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
> >> > Cc: Ulf Hansson <ulf.hansson@linaro.org>
> >> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> >> > Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> >> > ---
> >> >  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
> >> > ===================================================================
> >> > --- a/arch/arm/boot/dts/exynos5420-arndale-octa.dts     2014-10-02 15:44:53.014826886 +0200
> >> > +++ b/arch/arm/boot/dts/exynos5420-arndale-octa.dts     2014-10-02 17:35:24.110600398 +0200
> >> > @@ -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...
> >
> > 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?

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

[PATCH] ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

[Ulf Hansson]

On 2 October 2014 18:10, Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:
> 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.
>
> Suggested-by: Doug Anderson <dianders@google.com>
> Cc: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>

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

Kind regards
Uffe


> ---
>  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
> ===================================================================
> --- a/arch/arm/boot/dts/exynos5420-arndale-octa.dts     2014-10-02 15:44:53.014826886 +0200
> +++ b/arch/arm/boot/dts/exynos5420-arndale-octa.dts     2014-10-02 17:35:24.110600398 +0200
> @@ -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>;
>

[PATCH] ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

[Doug Anderson]

Bartlomiej,

On Thu, Oct 2, 2014 at 10:24 AM, Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:
>
> Hi,
>
> On Thursday, October 02, 2014 09:45:41 AM Doug Anderson wrote:
>> Bartiomiej
>>
>> On Thu, Oct 2, 2014 at 9:39 AM, Bartlomiej Zolnierkiewicz
>> <b.zolnierkie@samsung.com> wrote:
>> > On Thursday, October 02, 2014 09:19:08 AM Doug Anderson wrote:
>> >> Bartiomiej,
>> >>
>> >> On Thu, Oct 2, 2014 at 9:10 AM, Bartlomiej Zolnierkiewicz
>> >> <b.zolnierkie@samsung.com> wrote:
>> >> > 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.
>> >> >
>> >> > Suggested-by: Doug Anderson <dianders@google.com>
>> >> > Cc: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
>> >> > Cc: Ulf Hansson <ulf.hansson@linaro.org>
>> >> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
>> >> > Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>> >> > ---
>> >> >  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
>> >> > ===================================================================
>> >> > --- a/arch/arm/boot/dts/exynos5420-arndale-octa.dts     2014-10-02 15:44:53.014826886 +0200
>> >> > +++ b/arch/arm/boot/dts/exynos5420-arndale-octa.dts     2014-10-02 17:35:24.110600398 +0200
>> >> > @@ -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...
>> >
>> > 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?
>
> 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

[PATCH] ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

[Jaehoon Chung]

Hi,

On 10/07/2014 01:51 AM, Doug Anderson wrote:
> Bartlomiej,
> 
> On Thu, Oct 2, 2014 at 10:24 AM, Bartlomiej Zolnierkiewicz
> <b.zolnierkie@samsung.com> wrote:
>>
>> Hi,
>>
>> On Thursday, October 02, 2014 09:45:41 AM Doug Anderson wrote:
>>> Bartiomiej
>>>
>>> On Thu, Oct 2, 2014 at 9:39 AM, Bartlomiej Zolnierkiewicz
>>> <b.zolnierkie@samsung.com> wrote:
>>>> On Thursday, October 02, 2014 09:19:08 AM Doug Anderson wrote:
>>>>> Bartiomiej,
>>>>>
>>>>> On Thu, Oct 2, 2014 at 9:10 AM, Bartlomiej Zolnierkiewicz
>>>>> <b.zolnierkie@samsung.com> wrote:
>>>>>> 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.
>>>>>>
>>>>>> Suggested-by: Doug Anderson <dianders@google.com>
>>>>>> Cc: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
>>>>>> Cc: Ulf Hansson <ulf.hansson@linaro.org>
>>>>>> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
>>>>>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>>>>>> ---
>>>>>>  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
>>>>>> ===================================================================
>>>>>> --- a/arch/arm/boot/dts/exynos5420-arndale-octa.dts     2014-10-02 15:44:53.014826886 +0200
>>>>>> +++ b/arch/arm/boot/dts/exynos5420-arndale-octa.dts     2014-10-02 17:35:24.110600398 +0200
>>>>>> @@ -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...
>>>>
>>>> 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?
>>
>> 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?

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

> 
> 
> -Doug
> 

[PATCH] ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

[Ulf Hansson]

On 8 October 2014 02:19, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Hi,
>
> On 10/07/2014 01:51 AM, Doug Anderson wrote:
>> Bartlomiej,
>>
>> On Thu, Oct 2, 2014 at 10:24 AM, Bartlomiej Zolnierkiewicz
>> <b.zolnierkie@samsung.com> wrote:
>>>
>>> Hi,
>>>
>>> On Thursday, October 02, 2014 09:45:41 AM Doug Anderson wrote:
>>>> Bartiomiej
>>>>
>>>> On Thu, Oct 2, 2014 at 9:39 AM, Bartlomiej Zolnierkiewicz
>>>> <b.zolnierkie@samsung.com> wrote:
>>>>> On Thursday, October 02, 2014 09:19:08 AM Doug Anderson wrote:
>>>>>> Bartiomiej,
>>>>>>
>>>>>> On Thu, Oct 2, 2014 at 9:10 AM, Bartlomiej Zolnierkiewicz
>>>>>> <b.zolnierkie@samsung.com> wrote:
>>>>>>> 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.
>>>>>>>
>>>>>>> Suggested-by: Doug Anderson <dianders@google.com>
>>>>>>> Cc: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
>>>>>>> Cc: Ulf Hansson <ulf.hansson@linaro.org>
>>>>>>> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
>>>>>>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>>>>>>> ---
>>>>>>>  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
>>>>>>> ===================================================================
>>>>>>> --- a/arch/arm/boot/dts/exynos5420-arndale-octa.dts     2014-10-02 15:44:53.014826886 +0200
>>>>>>> +++ b/arch/arm/boot/dts/exynos5420-arndale-octa.dts     2014-10-02 17:35:24.110600398 +0200
>>>>>>> @@ -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...
>>>>>
>>>>> 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?
>>>
>>> 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?
>
> 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

[PATCH] ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

[Arnd Bergmann]

On Thursday 02 October 2014 20:16:44 Ulf Hansson wrote:
> On 2 October 2014 18:10, Bartlomiej Zolnierkiewicz
> <b.zolnierkie@samsung.com> wrote:
> > 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.
> >
> > Suggested-by: Doug Anderson <dianders@google.com>
> > Cc: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
> > Cc: Ulf Hansson <ulf.hansson@linaro.org>
> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> 
> 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, 

Acked-by: Arnd Bergmann <arnd@arndb.de>

[PATCH] ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

[Ulf Hansson]

On 8 October 2014 14:07, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 02 October 2014 20:16:44 Ulf Hansson wrote:
>> On 2 October 2014 18:10, Bartlomiej Zolnierkiewicz
>> <b.zolnierkie@samsung.com> wrote:
>> > 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.
>> >
>> > Suggested-by: Doug Anderson <dianders@google.com>
>> > Cc: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
>> > Cc: Ulf Hansson <ulf.hansson@linaro.org>
>> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
>> > Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>>
>> 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,
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>

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

[PATCH] ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

[Arnd Bergmann]

On Wednesday 08 October 2014 14:50:46 Ulf Hansson wrote:
> On 8 October 2014 14:07, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thursday 02 October 2014 20:16:44 Ulf Hansson wrote:
> >> On 2 October 2014 18:10, Bartlomiej Zolnierkiewicz
> >> <b.zolnierkie@samsung.com> wrote:
> >> > 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.
> >> >
> >> > Suggested-by: Doug Anderson <dianders@google.com>
> >> > Cc: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
> >> > Cc: Ulf Hansson <ulf.hansson@linaro.org>
> >> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> >> > Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> >>
> >> 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,
> >
> > Acked-by: Arnd Bergmann <arnd@arndb.de>
> 
> 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

[PATCH] ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

[Doug Anderson]

Hi,

On Wed, Oct 8, 2014 at 8:20 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> 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

[PATCH] ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

[Arnd Bergmann]

On Wednesday 08 October 2014 08:29:16 Doug Anderson wrote:
> 
> On Wed, Oct 8, 2014 at 8:20 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > 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

[PATCH] ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

[Ulf Hansson]

On 8 October 2014 20:28, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 08 October 2014 08:29:16 Doug Anderson wrote:
>>
>> On Wed, Oct 8, 2014 at 8:20 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > 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

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

[PATCH] ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board

[Arnd Bergmann]

On Thursday 09 October 2014 10:46:59 Ulf Hansson wrote:
> On 8 October 2014 20:28, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Wednesday 08 October 2014 08:29:16 Doug Anderson wrote:
> >>
> >> On Wed, Oct 8, 2014 at 8:20 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> > 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
> 
> 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