From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Date: Fri, 19 Dec 2014 17:51:48 +0100 Subject: [U-Boot] [PATCH 13/14] sun8i: Add dram initialization support In-Reply-To: <1418930270.26985.110.camel@hellion.org.uk> References: <1418761900-14035-1-git-send-email-hdegoede@redhat.com> <1418761900-14035-13-git-send-email-hdegoede@redhat.com> <1418930270.26985.110.camel@hellion.org.uk> Message-ID: <549457A4.50001@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 18-12-14 20:17, Ian Campbell wrote: > On Tue, 2014-12-16 at 21:31 +0100, Hans de Goede wrote: >> Based on the register / dram_para headers from the Allwinner u-boot / linux >> sources + the init sequences from boot0. >> >> Signed-off-by: Hans de Goede >> +/* >> + * Note this code uses a lot of magic hex values, that is because this code >> + * simply replays the init sequence as done by the Allwinner boot0 code, so >> + * we do not know what these values mean. There are no symbolic constants for >> + * these magic values, since we do not know how to name them and making up >> + * names for them is not useful. > > On that basis I've only really given this a quick glance. I've no > problem with it. > > Couple of queries about the defconfig changes: > >> diff --git a/configs/Ippo_q8h_v5_defconfig b/configs/Ippo_q8h_v5_defconfig >> index 50c2f93..37aa46d 100644 >> --- a/configs/Ippo_q8h_v5_defconfig >> +++ b/configs/Ippo_q8h_v5_defconfig >> @@ -1,8 +1,15 @@ >> +CONFIG_SPL=y >> CONFIG_SYS_EXTRA_OPTIONS="CONS_INDEX=5" >> -CONFIG_ARM=y >> -CONFIG_ARCH_SUNXI=y >> -CONFIG_MACH_SUN8I=y >> -CONFIG_TARGET_IPPO_Q8H_V5=y > > Not replaced with a S: variant? I know you want CONFIG_TARGET to go, but > I don't think that was part of what you intended in this patch. Right, I'll add that back. > >> -CONFIG_DEFAULT_DEVICE_TREE="sun8i-a23-ippo-q8h-v5.dtb" >> +CONFIG_FDTFILE="sun8i-a23-ippo-q8h-v5.dtb" > > The switch from CONFIG_DEFAULT_DEVICE_TREE to CONFIG_FDTFILE conversion > seems a little out of place too. That was deliberate, the CONFIG_DEFAULT_DEVICE_TREE is the wrong CONFIG define to use to set the dtb for the kernel. I'll split the Ippo_q8h_v5_defconfig changes out into a separate patch and mention this in the commit message. Regards, Hans