[U-Boot] [PATCH] sunxi: axp221: correct ALDO2 description for sun6i

[U-Boot] [PATCH] sunxi: axp221: correct ALDO2 description for sun6i

[Chen-Yu Tsai]

ALDO2 is used to power LPDDR2 SDRAM on both the reference design and the
Hummingbird A31, when this type of RAM is present.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---

Hi Hans,

It seems I was on something when I read the board schematics, as I misread
them. ALDO2 is only used for LPDDR2, which nobody seems to use, and we don't
support anyway.

VCC-PM-CPUS is powered from GPIO0-LDO, as is VCC-PB (which no one seems to
use either.)


Sorry for the mixup.

ChenYu

---
 drivers/power/Kconfig | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index ef0c093..df7a759 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -46,14 +46,13 @@ config AXP221_ALDO1_VOLT
 config AXP221_ALDO2_VOLT
 	int "axp221 aldo2 voltage"
 	depends on AXP221_POWER
-	default 1800 if MACH_SUN6I
+	default 0 if MACH_SUN6I
 	default 2500 if MACH_SUN8I
 	---help---
 	Set the voltage (mV) to program the axp221 aldo2 at, set to 0 to
-	disable aldo2. On sun6i (A31) boards this is typically connected to
-	VCC-PM, which powers the port M gpios, and should be set to 1.8V.
-	On sun8i (A23) this is typically connected to VDD-DLL and must be
-	set to 2.5V.
+	disable aldo2. On sun6i (A31) boards this is typically used for LPDDR2
+	SDRAM, and should be set to 1.8V if present. On sun8i (A23) this is
+	typically connected to VDD-DLL and must be set to 2.5V.
 
 config AXP221_ALDO3_VOLT
 	int "axp221 aldo3 voltage"
-- 
2.1.4

[U-Boot] [PATCH] sunxi: axp221: correct ALDO2 description for sun6i

[Ian Campbell]

On Sat, 2015-01-03 at 10:17 +0800, Chen-Yu Tsai wrote:
> ALDO2 is used to power LPDDR2 SDRAM on both the reference design and the
> Hummingbird A31, when this type of RAM is present.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

I think this one needs Hans' opinion more than mine, but looks OK to me
with one small comment:

> +	disable aldo2. On sun6i (A31) boards this is typically used for LPDDR2
> +	SDRAM, and should be set to 1.8V if present. On sun8i (A23) this is
> +	typically connected to VDD-DLL and must be set to 2.5V.

It took me a while to work out why "set to 1.8V if present" wasn't
reflected in the default (which you changed to zero in this patch), I
think there is an implied "which is almost never" on the end? But that
wouldn't be in keeping with the style of things. How about:

        [...] On sun6i (A31) boards this is typically unused and should
        be disabled, if it is used for LPDDR2 it should be set to 1.8V.
        [...]

?

>  
>  config AXP221_ALDO3_VOLT
>  	int "axp221 aldo3 voltage"

[U-Boot] [PATCH] sunxi: axp221: correct ALDO2 description for sun6i

[Chen-Yu Tsai]

On Sun, Jan 11, 2015 at 5:38 PM, Ian Campbell <ijc@hellion.org.uk> wrote:
> On Sat, 2015-01-03 at 10:17 +0800, Chen-Yu Tsai wrote:
>> ALDO2 is used to power LPDDR2 SDRAM on both the reference design and the
>> Hummingbird A31, when this type of RAM is present.
>>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>
> I think this one needs Hans' opinion more than mine, but looks OK to me
> with one small comment:
>
>> +     disable aldo2. On sun6i (A31) boards this is typically used for LPDDR2
>> +     SDRAM, and should be set to 1.8V if present. On sun8i (A23) this is
>> +     typically connected to VDD-DLL and must be set to 2.5V.
>
> It took me a while to work out why "set to 1.8V if present" wasn't
> reflected in the default (which you changed to zero in this patch), I
> think there is an implied "which is almost never" on the end? But that
> wouldn't be in keeping with the style of things. How about:
>
>         [...] On sun6i (A31) boards this is typically unused and should
>         be disabled, if it is used for LPDDR2 it should be set to 1.8V.
>         [...]
>
> ?

Yes that does seem less confusing. A full stop after "disabled" would be
even better.

>>
>>  config AXP221_ALDO3_VOLT
>>       int "axp221 aldo3 voltage"
>
>

[U-Boot] [PATCH] sunxi: axp221: correct ALDO2 description for sun6i

[Ian Campbell]

On Sun, 2015-01-11 at 17:41 +0800, Chen-Yu Tsai wrote:
> On Sun, Jan 11, 2015 at 5:38 PM, Ian Campbell <ijc@hellion.org.uk> wrote:
> > On Sat, 2015-01-03 at 10:17 +0800, Chen-Yu Tsai wrote:
> >> ALDO2 is used to power LPDDR2 SDRAM on both the reference design and the
> >> Hummingbird A31, when this type of RAM is present.
> >>
> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> >
> > I think this one needs Hans' opinion more than mine, but looks OK to me
> > with one small comment:
> >
> >> +     disable aldo2. On sun6i (A31) boards this is typically used for LPDDR2
> >> +     SDRAM, and should be set to 1.8V if present. On sun8i (A23) this is
> >> +     typically connected to VDD-DLL and must be set to 2.5V.
> >
> > It took me a while to work out why "set to 1.8V if present" wasn't
> > reflected in the default (which you changed to zero in this patch), I
> > think there is an implied "which is almost never" on the end? But that
> > wouldn't be in keeping with the style of things. How about:
> >
> >         [...] On sun6i (A31) boards this is typically unused and should
> >         be disabled, if it is used for LPDDR2 it should be set to 1.8V.
> >         [...]
> >
> > ?
> 
> Yes that does seem less confusing. A full stop after "disabled" would be
> even better.

I suppose either would work. I'll leave it to Hans to pick as he reviews
and/or commits it.

Ian.

[U-Boot] [PATCH] sunxi: axp221: correct ALDO2 description for sun6i

[Hans de Goede]

Hi,

On 11-01-15 10:41, Chen-Yu Tsai wrote:
> On Sun, Jan 11, 2015 at 5:38 PM, Ian Campbell <ijc@hellion.org.uk> wrote:
>> On Sat, 2015-01-03 at 10:17 +0800, Chen-Yu Tsai wrote:
>>> ALDO2 is used to power LPDDR2 SDRAM on both the reference design and the
>>> Hummingbird A31, when this type of RAM is present.
>>>
>>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>>
>> I think this one needs Hans' opinion more than mine, but looks OK to me
>> with one small comment:
>>
>>> +     disable aldo2. On sun6i (A31) boards this is typically used for LPDDR2
>>> +     SDRAM, and should be set to 1.8V if present. On sun8i (A23) this is
>>> +     typically connected to VDD-DLL and must be set to 2.5V.
>>
>> It took me a while to work out why "set to 1.8V if present" wasn't
>> reflected in the default (which you changed to zero in this patch), I
>> think there is an implied "which is almost never" on the end? But that
>> wouldn't be in keeping with the style of things. How about:
>>
>>          [...] On sun6i (A31) boards this is typically unused and should
>>          be disabled, if it is used for LPDDR2 it should be set to 1.8V.
>>          [...]
>>
>> ?
>
> Yes that does seem less confusing. A full stop after "disabled" would be
> even better.

I've committed this patch with the fix suggested by Ian, I've deviced to
keep the "," to make it clear that the if belongs to the "On sun6i".

I've queued this up for merging in u-boot-sunxi/next.

Regards,

Hans