From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Date: Sun, 11 Jan 2015 09:51:25 +0000 Subject: [U-Boot] [PATCH] sunxi: axp221: correct ALDO2 description for sun6i In-Reply-To: References: <1420251444-19117-1-git-send-email-wens@csie.org> <1420969094.11796.123.camel@hellion.org.uk> Message-ID: <1420969885.11796.131.camel@hellion.org.uk> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Sun, 2015-01-11 at 17:41 +0800, Chen-Yu Tsai wrote: > On Sun, Jan 11, 2015 at 5:38 PM, Ian Campbell 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 > > > > 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.