| Message ID | 1420251444-19117-1-git-send-email-wens@csie.org |
|---|---|
| State | Accepted |
| Delegated to: | Hans de Goede |
| Headers | show |
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"
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" > >
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.
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
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"
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(-)