From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Date: Tue, 13 Jan 2015 13:00:52 +0100 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: <54B508F4.1060101@redhat.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi, On 11-01-15 10:41, 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'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