From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Schocher Date: Fri, 24 Aug 2012 12:17:58 +0200 Subject: [U-Boot] [PATCH 3/6] SPL: Port SPL framework to powerpc In-Reply-To: <50373883.5090601@denx.de> References: <1345709565-28862-1-git-send-email-sr@denx.de> <1345709565-28862-4-git-send-email-sr@denx.de> <503663EE.9000902@ti.com> <50367371.7080206@denx.de> <50368523.4080107@ti.com> <50373883.5090601@denx.de> Message-ID: <503754D6.1050405@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hello Stefan On 24.08.2012 10:17, Stefan Roese wrote: > Hi Tom, > > On 08/23/2012 09:31 PM, Tom Rini wrote: >>>>> @@ -89,7 +106,11 @@ void spl_parse_image_header(const struct image_header *header) >>>>> spl_image.size = __be32_to_cpu(header->ih_size) + header_size; >>>>> spl_image.entry_point = __be32_to_cpu(header->ih_load); >>>>> /* Load including the header */ >>>>> +#ifdef CONFIG_ARM >>>>> spl_image.load_addr = spl_image.entry_point - header_size; >>>>> +#else >>>>> + spl_image.load_addr = __be32_to_cpu(header->ih_load); >>>>> +#endif >>>> >>>> This isn't an ARM-ism but is instead because spl_nor.c isn't offsetting >>>> where the header is like mmc/nand/ymodem do, yes? Would it be possible >>>> to make spl_nor.c behave like the others? One of the reasons I ask is >>>> I'm looking at a NOR chip on my desk... >>> >>> I was wondering about this line as well. Please explain: Why can't ARM >>> just use header->ih_load as load_addr? >> >> Off the top of my head, I believe what goes on is that we read things >> into SDRAM such that the header is taken into account and we don't need >> to relocate the payload (U-Boot or Linux). > > Hmmm. So for example, when ih_load is set to 0x100000, then you load the > image to (0x100000 - 0x40) = 0xfffc0? Is this correct? > > This can't work for powerpc. As here for Linux both load-address and > entry-point are set to 0. So when loading the image (e.g. from NOR flash) > can't copy the image header in front of the image. > > Another thing I'm wondering about: Why is only ih_load from the mkimage > header used and not ih_ep (entry-point)? > > I suggest that we switch to copying the "real" image (payload) to the load > address, skipping the header. Then "ih_load" and "ih_ep" can be used > without modification. Yep, this seems a good idea to me. > BTW: There also seems to be a bug in some of the SPL loaders: > > For example in drivers/mtd/nand/nand_spl_load.c: > > ... > if (header->ih_os == IH_OS_LINUX) { > /* happy - was a linux */ > nand_spl_load_image(CONFIG_SYS_NAND_SPL_KERNEL_OFFS, > spl_image.size, (void *)spl_image.load_addr); > > The problem here is that the last 64 bytes of the image are not > copied to SDRAM. Since the header is copied which is not included > in the spl_image.size variable. With my idea of only copying > the payload (skipping the header) this would be: > > nand_spl_load_image(CONFIG_SYS_NAND_SPL_KERNEL_OFFS + > sizeof(struct image_header), > spl_image.size, (void *)spl_image.load_addr); > > What do you think? Should we switch to this way of loading images? > Seems more logical to me. And we don't run into problems where the > load address is 0. Yes, that should be the way to go ... @Simon: Do you see here some reason for not switching to copy the real payload? bye, Heiko -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany