Re: [U-Boot] [PATCH 3/6] SPL: Port SPL framework to powerpc
On 08/24/2012 09:15 PM, Tom Rini wrote: I still would like to move to my suggestion to not copy the header and use the mkimage header values ih_load and ih_ep directly. Right now I don't see any showstopper for doing it this way. I'll send a patch to change this shortly (if everything works out). Hmmm. As it seems some SPL loading drivers (block like mmc, streaming like ymodem) are not that easily converted to skipping the header. So I'm not so sure if we should go this way after all... Maybe I'm missing something, but maybe we just need to mimic the behavior full U-Boot does and if we haven't been loaded where we need to execute, shift bits around? Of course its possible. The main problem is speed here. With the problematic drivers like mmc, you would need to copy the image a 2nd time, once from MMC to a temp. SDRAM location, and then to its final (load_addr) destination. With the current approach, the image is only copied once from MMC to SDRAM. The 2nd copy shouldn't take that long though. Its SDRAM to SDRAM after all. I really do like Daniel's approach, with the decompression support: http://dev.phrozen.org/gitweb/?p=uboot-upstream.git;a=commitdiff;h=39165fa145b2d959f1eaa6faa3ab3053823bb985 We should try to merge this into the current SPL framework. I'll try to look into this next week. Thanks, Stefan ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] SPL: Port SPL framework to powerpc
Hi Tom,
On 08/23/2012 11:39 PM, Tom Rini wrote:
On 08/23/2012 01:12 AM, Stefan Roese wrote:
This patch enables the SPL framework to be used on powerpc platforms
and not only ARM.
[snip]
+#ifdef CONFIG_ARM
gd = gdata;
+#endif
So, here's what I don't understand. On ARM, in general, we can't rely
on the global data pointer register (r8) to be set to a useful value, so
we do the above to ensure it points to something useful. Are you always
able to rely on r2 it looks like pointing to something useful? Or do
you take care of this much earlier on in powerpc? Thanks!
You are correct, I missed something here. r2 was still configured to the
value written to it from the real U-Boot (pointing to internal SRAM).
I can't use the code in preloader_console_init() though to setup the gd
pointer. As I need to write some values in gd *before* calling
preloader_console_init() (mainly clocks for serial driver). And since
this gd stuff is quite platform specific, we should probably move this
into an platform/arch spl file instead. As you also mentioned in another
reply to create an arch/${ARCH}/lib/spl.c file.
What do you think? Can you move this gd init stuff into such a common
ARM spl file in the next patchset version?
Thanks,
Stefan
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] SPL: Port SPL framework to powerpc
Hi Tom,
On 08/23/2012 11:52 PM, Tom Rini wrote:
On 08/23/2012 01:12 AM, Stefan Roese wrote:
This patch enables the SPL framework to be used on powerpc platforms
and not only ARM.
[snip]
+#ifdef CONFIG_PPC
+static void __noreturn jump_to_image_linux(void *arg)
+{
+debug(Entering kernel arg pointer: 0x%p\n, arg);
+typedef void (*image_entry_arg_t)(void *, ulong r4, ulong r5, ulong r6,
+ ulong r7, ulong r8, ulong r9)
+__attribute__ ((noreturn));
+image_entry_arg_t image_entry =
+(image_entry_arg_t)spl_image.entry_point;
+
+image_entry(arg, 0, 0, EPAPR_MAGIC, CONFIG_SYS_BOOTMAPSZ, 0, 0);
+}
+#endif /* CONFIG_PPC */
+#endif /* CONFIG_SPL_OS_BOOT */
This, along with board_init_f make me wonder if we shouldn't have an
arch/${ARCH}/lib/spl.c that contains them and make them non-static.
Perhaps sharing with the non-SPL code portion as well?
Yes. Makes definitely sense. Please see my comments in the other mail.
Thanks,
Stefan
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] SPL: Port SPL framework to powerpc
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 0x10, then you load the
image to (0x10 - 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.
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.
Thanks,
Stefan
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] SPL: Port SPL framework to powerpc
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 0x10, then you load the
image to (0x10 - 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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] SPL: Port SPL framework to powerpc
Hi Heiko,
On 08/24/2012 12:17 PM, Heiko Schocher wrote:
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.
Okay. I just noticed that it's not a bug. spl_image.size is set to
header-ih_size + header_size. So 64 is added and the complete payload
is copied.
I still would like to move to my suggestion to not copy the header and
use the mkimage header values ih_load and ih_ep directly. Right now I
don't see any showstopper for doing it this way. I'll send a patch to
change this shortly (if everything works out).
Stay tuned...
Thanks,
Stefan
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: off...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] SPL: Port SPL framework to powerpc
On 08/24/2012 12:56 PM, Stefan Roese wrote: I still would like to move to my suggestion to not copy the header and use the mkimage header values ih_load and ih_ep directly. Right now I don't see any showstopper for doing it this way. I'll send a patch to change this shortly (if everything works out). Hmmm. As it seems some SPL loading drivers (block like mmc, streaming like ymodem) are not that easily converted to skipping the header. So I'm not so sure if we should go this way after all... Comments? Thanks, Stefan ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] SPL: Port SPL framework to powerpc
Hi Stefan,
2012/8/24 Stefan Roese s...@denx.de:
On 08/24/2012 12:56 PM, Stefan Roese wrote:
I still would like to move to my suggestion to not copy the header and
use the mkimage header values ih_load and ih_ep directly. Right now I
don't see any showstopper for doing it this way. I'll send a patch to
change this shortly (if everything works out).
Hmmm. As it seems some SPL loading drivers (block like mmc, streaming
like ymodem) are not that easily converted to skipping the header. So
I'm not so sure if we should go this way after all...
Comments?
I did similar work for my upcoming Lantiq MIPS SoC port. In my
approach I also support compressed u-boot
images as payload (LZO, LZMA). The u-boot image is merged with SPL
image without any padding or
fixed flash offsets to achieve a maximum reduction of flash footprint.
I ended up with doing something like this:
static int spl_parse_image(const image_header_t *hdr, struct spl_image *spl)
{
u32 magic;
magic = image_get_magic(hdr);
if (magic != IH_MAGIC)
return -1;
spl-data_addr += image_get_header_size();
spl-entry_addr = image_get_load(hdr);
spl-size = image_get_data_size(hdr);
spl-comp = image_get_comp(hdr);
if (spl-comp == IH_COMP_NONE)
spl-load_addr = spl-entry_addr;
else
spl-load_addr = CONFIG_LOADADDR;
return 0;
}
spl-data_addr points to the image header of the payload and is
initialized by the caller
dependent on the used load mechanism. If the payload is uncompressed
it can be directly copied to
its final RAM location. A compressed payload needs an intermediate
copy step if it is stored in SPI or NAND flash.
I chose CONFIG_LOADADDR. After this the decompression function
extracts the image to its final RAM location.
I think we should keep an architecture-neutral SPL image context
inside the generic SPL framework.
The content of this context should be filled by architecture/SoC/board
specific code.
--
Best regards,
Daniel
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] SPL: Port SPL framework to powerpc
Hi Daniel,
On 08/24/2012 01:49 PM, Daniel Schwierzeck wrote:
I still would like to move to my suggestion to not copy the header and
use the mkimage header values ih_load and ih_ep directly. Right now I
don't see any showstopper for doing it this way. I'll send a patch to
change this shortly (if everything works out).
Hmmm. As it seems some SPL loading drivers (block like mmc, streaming
like ymodem) are not that easily converted to skipping the header. So
I'm not so sure if we should go this way after all...
Comments?
I did similar work for my upcoming Lantiq MIPS SoC port. In my
approach I also support compressed u-boot
images as payload (LZO, LZMA).
Yes, I thought about adding this as well. Nice.
The u-boot image is merged with SPL
image without any padding or
fixed flash offsets to achieve a maximum reduction of flash footprint.
Interesting. I'm still padding to the fixed offset. Let me look into
squeezing those two images together as well...
I ended up with doing something like this:
static int spl_parse_image(const image_header_t *hdr, struct spl_image *spl)
{
u32 magic;
magic = image_get_magic(hdr);
if (magic != IH_MAGIC)
return -1;
spl-data_addr += image_get_header_size();
spl-entry_addr = image_get_load(hdr);
spl-size = image_get_data_size(hdr);
spl-comp = image_get_comp(hdr);
if (spl-comp == IH_COMP_NONE)
spl-load_addr = spl-entry_addr;
else
spl-load_addr = CONFIG_LOADADDR;
return 0;
}
So you introduced a new struct spl_image and did not use struct
spl_image_info from the SPL framework? Why is that? We should
definitely consolidate to one structure here.
spl-data_addr points to the image header of the payload and is
initialized by the caller
dependent on the used load mechanism.
Good idea. Looks like your spl_parse_image might be a potential
replacement for spl_parse_image_header() from this common SPL framework.
If the payload is uncompressed
it can be directly copied to
its final RAM location. A compressed payload needs an intermediate
copy step if it is stored in SPI or NAND flash.
I chose CONFIG_LOADADDR. After this the decompression function
extracts the image to its final RAM location.
Sounds good. Do you have some patches that you could send for review?
Did you try to keep as much platform/arch independent as possible?
I think we should keep an architecture-neutral SPL image context
inside the generic SPL framework.
The content of this context should be filled by architecture/SoC/board
specific code.
Ack. It would be great to review your code here. Please keep us informed.
Thanks,
Stefan
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] SPL: Port SPL framework to powerpc
Hi Stefan,
2012/8/24 Stefan Roese s...@denx.de:
Hi Daniel,
On 08/24/2012 01:49 PM, Daniel Schwierzeck wrote:
I still would like to move to my suggestion to not copy the header and
use the mkimage header values ih_load and ih_ep directly. Right now I
don't see any showstopper for doing it this way. I'll send a patch to
change this shortly (if everything works out).
Hmmm. As it seems some SPL loading drivers (block like mmc, streaming
like ymodem) are not that easily converted to skipping the header. So
I'm not so sure if we should go this way after all...
Comments?
I did similar work for my upcoming Lantiq MIPS SoC port. In my
approach I also support compressed u-boot
images as payload (LZO, LZMA).
Yes, I thought about adding this as well. Nice.
The u-boot image is merged with SPL
image without any padding or
fixed flash offsets to achieve a maximum reduction of flash footprint.
Interesting. I'm still padding to the fixed offset. Let me look into
squeezing those two images together as well...
I ended up with doing something like this:
static int spl_parse_image(const image_header_t *hdr, struct spl_image *spl)
{
u32 magic;
magic = image_get_magic(hdr);
if (magic != IH_MAGIC)
return -1;
spl-data_addr += image_get_header_size();
spl-entry_addr = image_get_load(hdr);
spl-size = image_get_data_size(hdr);
spl-comp = image_get_comp(hdr);
if (spl-comp == IH_COMP_NONE)
spl-load_addr = spl-entry_addr;
else
spl-load_addr = CONFIG_LOADADDR;
return 0;
}
So you introduced a new struct spl_image and did not use struct
spl_image_info from the SPL framework? Why is that? We should
definitely consolidate to one structure here.
I implemented this after the initial SPL infrastructure got merged
into mainline one year ago.
At this time a generic SPL framework did not exist ;)
spl-data_addr points to the image header of the payload and is
initialized by the caller
dependent on the used load mechanism.
Good idea. Looks like your spl_parse_image might be a potential
replacement for spl_parse_image_header() from this common SPL framework.
If the payload is uncompressed
it can be directly copied to
its final RAM location. A compressed payload needs an intermediate
copy step if it is stored in SPI or NAND flash.
I chose CONFIG_LOADADDR. After this the decompression function
extracts the image to its final RAM location.
Sounds good. Do you have some patches that you could send for review?
Did you try to keep as much platform/arch independent as possible?
Not yet. I had not time yet to adopt my work to the new common SPL framework.
My current SPL solution is queued here:
http://dev.phrozen.org/gitweb/?p=uboot-upstream.git;a=commitdiff;h=39165fa145b2d959f1eaa6faa3ab3053823bb985
I think we should keep an architecture-neutral SPL image context
inside the generic SPL framework.
The content of this context should be filled by architecture/SoC/board
specific code.
Ack. It would be great to review your code here. Please keep us informed.
Actually this was my response to your original question.
Quote:
I still would like to move to my suggestion to not copy the header and
use the mkimage header values ih_load and ih_ep directly. Right now I
don't see any showstopper for doing it this way. I'll send a patch to
change this shortly (if everything works out).
So I suggest to not remove the 'struct spl_image_info'.
--
Best regards,
Daniel
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] SPL: Port SPL framework to powerpc
On 08/24/2012 12:01 AM, Stefan Roese wrote:
Hi Tom,
On 08/23/2012 11:39 PM, Tom Rini wrote:
On 08/23/2012 01:12 AM, Stefan Roese wrote:
This patch enables the SPL framework to be used on powerpc platforms
and not only ARM.
[snip]
+#ifdef CONFIG_ARM
gd = gdata;
+#endif
So, here's what I don't understand. On ARM, in general, we can't rely
on the global data pointer register (r8) to be set to a useful value, so
we do the above to ensure it points to something useful. Are you always
able to rely on r2 it looks like pointing to something useful? Or do
you take care of this much earlier on in powerpc? Thanks!
You are correct, I missed something here. r2 was still configured to the
value written to it from the real U-Boot (pointing to internal SRAM).
I can't use the code in preloader_console_init() though to setup the gd
pointer. As I need to write some values in gd *before* calling
preloader_console_init() (mainly clocks for serial driver). And since
this gd stuff is quite platform specific, we should probably move this
into an platform/arch spl file instead. As you also mentioned in another
reply to create an arch/${ARCH}/lib/spl.c file.
What do you think? Can you move this gd init stuff into such a common
ARM spl file in the next patchset version?
Yes. We should probably say it's the job of board_init_f. I know full
U-Boot does it at the start of board_init_r but I think for SPL it makes
sense to document that board_init_f does any early init it needs to,
clears BSS, sets up gd and calls board_init_r.
--
Tom
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] SPL: Port SPL framework to powerpc
Hi Daniel, On 08/24/2012 04:11 PM, Stefan Roese wrote: The u-boot image is merged with SPL image without any padding or fixed flash offsets to achieve a maximum reduction of flash footprint. Interesting. I'm still padding to the fixed offset. Let me look into squeezing those two images together as well... Looks good. One question though: How do you make sure, that your SPL image length is 4-byte aligned? Thanks, Stefan ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] SPL: Port SPL framework to powerpc
On 08/24/2012 05:55 PM, Tom Rini wrote:
So, here's what I don't understand. On ARM, in general, we can't rely
on the global data pointer register (r8) to be set to a useful value, so
we do the above to ensure it points to something useful. Are you always
able to rely on r2 it looks like pointing to something useful? Or do
you take care of this much earlier on in powerpc? Thanks!
You are correct, I missed something here. r2 was still configured to the
value written to it from the real U-Boot (pointing to internal SRAM).
I can't use the code in preloader_console_init() though to setup the gd
pointer. As I need to write some values in gd *before* calling
preloader_console_init() (mainly clocks for serial driver). And since
this gd stuff is quite platform specific, we should probably move this
into an platform/arch spl file instead. As you also mentioned in another
reply to create an arch/${ARCH}/lib/spl.c file.
What do you think? Can you move this gd init stuff into such a common
ARM spl file in the next patchset version?
Yes. We should probably say it's the job of board_init_f. I know full
U-Boot does it at the start of board_init_r but I think for SPL it makes
sense to document that board_init_f does any early init it needs to,
clears BSS, sets up gd and calls board_init_r.
Full ack. In my next patchset version, I already extracted this gd setup
into board_init_f().
Thanks,
Stefan
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] SPL: Port SPL framework to powerpc
On 08/24/2012 09:07 AM, Stefan Roese wrote:
On 08/24/2012 05:55 PM, Tom Rini wrote:
So, here's what I don't understand. On ARM, in general, we can't rely
on the global data pointer register (r8) to be set to a useful value, so
we do the above to ensure it points to something useful. Are you always
able to rely on r2 it looks like pointing to something useful? Or do
you take care of this much earlier on in powerpc? Thanks!
You are correct, I missed something here. r2 was still configured to the
value written to it from the real U-Boot (pointing to internal SRAM).
I can't use the code in preloader_console_init() though to setup the gd
pointer. As I need to write some values in gd *before* calling
preloader_console_init() (mainly clocks for serial driver). And since
this gd stuff is quite platform specific, we should probably move this
into an platform/arch spl file instead. As you also mentioned in another
reply to create an arch/${ARCH}/lib/spl.c file.
What do you think? Can you move this gd init stuff into such a common
ARM spl file in the next patchset version?
Yes. We should probably say it's the job of board_init_f. I know full
U-Boot does it at the start of board_init_r but I think for SPL it makes
sense to document that board_init_f does any early init it needs to,
clears BSS, sets up gd and calls board_init_r.
Full ack. In my next patchset version, I already extracted this gd setup
into board_init_f().
OK good, I just booted and reset my arm9 platform with this
proof-of-concepted over. I should post v4 today.
--
Tom
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] SPL: Port SPL framework to powerpc
Hi Stefan, 2012/8/24 Stefan Roese stefan.ro...@gmail.com: Hi Daniel, On 08/24/2012 04:11 PM, Stefan Roese wrote: The u-boot image is merged with SPL image without any padding or fixed flash offsets to achieve a maximum reduction of flash footprint. Interesting. I'm still padding to the fixed offset. Let me look into squeezing those two images together as well... Looks good. One question though: How do you make sure, that your SPL image length is 4-byte aligned? I have 4-byte alignments for each section and for each address symbol in my linker script. Actually all MIPS linker scripts do this. -- Best regards, Daniel ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] SPL: Port SPL framework to powerpc
On 08/24/2012 06:19 PM, Tom Rini wrote: What do you think? Can you move this gd init stuff into such a common ARM spl file in the next patchset version? Yes. We should probably say it's the job of board_init_f. I know full U-Boot does it at the start of board_init_r but I think for SPL it makes sense to document that board_init_f does any early init it needs to, clears BSS, sets up gd and calls board_init_r. Full ack. In my next patchset version, I already extracted this gd setup into board_init_f(). OK good, I just booted and reset my arm9 platform with this proof-of-concepted over. I should post v4 today. Great. I'll rebase my patches on top of that next week then. Thanks, Stefan ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] SPL: Port SPL framework to powerpc
On 08/24/2012 06:42 PM, Daniel Schwierzeck wrote: The u-boot image is merged with SPL image without any padding or fixed flash offsets to achieve a maximum reduction of flash footprint. Interesting. I'm still padding to the fixed offset. Let me look into squeezing those two images together as well... Looks good. One question though: How do you make sure, that your SPL image length is 4-byte aligned? I have 4-byte alignments for each section and for each address symbol in my linker script. Actually all MIPS linker scripts do this. Yes. I have those section alignments in the linker script as well. The problem is the end of the last (physical end in flash) section. If I have here a string with length 7 for example, the SPL binary has a non-4-byte aligned length. Any ideas on this? Thanks, Stefan ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] SPL: Port SPL framework to powerpc
On 08/24/2012 04:13 AM, Stefan Roese wrote: On 08/24/2012 12:56 PM, Stefan Roese wrote: I still would like to move to my suggestion to not copy the header and use the mkimage header values ih_load and ih_ep directly. Right now I don't see any showstopper for doing it this way. I'll send a patch to change this shortly (if everything works out). Hmmm. As it seems some SPL loading drivers (block like mmc, streaming like ymodem) are not that easily converted to skipping the header. So I'm not so sure if we should go this way after all... Maybe I'm missing something, but maybe we just need to mimic the behavior full U-Boot does and if we haven't been loaded where we need to execute, shift bits around? -- Tom ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] SPL: Port SPL framework to powerpc
2012/8/24 Stefan Roese stefan.ro...@gmail.com: On 08/24/2012 06:42 PM, Daniel Schwierzeck wrote: The u-boot image is merged with SPL image without any padding or fixed flash offsets to achieve a maximum reduction of flash footprint. Interesting. I'm still padding to the fixed offset. Let me look into squeezing those two images together as well... Looks good. One question though: How do you make sure, that your SPL image length is 4-byte aligned? I have 4-byte alignments for each section and for each address symbol in my linker script. Actually all MIPS linker scripts do this. Yes. I have those section alignments in the linker script as well. The problem is the end of the last (physical end in flash) section. If I have here a string with length 7 for example, the SPL binary has a non-4-byte aligned length. Any ideas on this? MIPS linker scripts have a symbol in front of the BSS section that is named uboot_end_data. I think most ARM linker scripts have it too but it is named __image_copy_end. This symbol is 4-byte-aligned. If I want to know the length of SPL image i can simply do ulong len = (ulong) __image_copy_end - CONFIG_SPL_TEXT_BASE and len is 4-byte-aligned -- Best regards, Daniel ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] SPL: Port SPL framework to powerpc
On 08/23/2012 01:12 AM, Stefan Roese wrote: This patch enables the SPL framework to be used on powerpc platforms and not only ARM. [snip] +#ifdef CONFIG_ARM /* Define global data structure pointer to it*/ gd_t gdata __attribute__ ((section(.data))); +#endif So you handle cleaning up the BSS differently, interesting. I'm going to see if that would work for ARM too.. [snip] @@ -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... -- Tom ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] SPL: Port SPL framework to powerpc
On 08/23/2012 07:10 PM, Tom Rini wrote: +#ifdef CONFIG_ARM /* Define global data structure pointer to it*/ gd_t gdata __attribute__ ((section(.data))); +#endif So you handle cleaning up the BSS differently, interesting. I'm going to see if that would work for ARM too.. Yes. Might be that I missed something though. I'll re-check tomorrow. [snip] @@ -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? Thanks, Stefan ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] SPL: Port SPL framework to powerpc
On 08/23/2012 11:16 AM, Stefan Roese wrote: On 08/23/2012 07:10 PM, Tom Rini wrote: +#ifdef CONFIG_ARM /* Define global data structure pointer to it*/ gd_t gdata __attribute__ ((section(.data))); +#endif So you handle cleaning up the BSS differently, interesting. I'm going to see if that would work for ARM too.. Yes. Might be that I missed something though. I'll re-check tomorrow. [snip] @@ -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). -- Tom ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] SPL: Port SPL framework to powerpc
On 08/23/2012 01:12 AM, Stefan Roese wrote: This patch enables the SPL framework to be used on powerpc platforms and not only ARM. [snip] +#ifdef CONFIG_ARM gd = gdata; +#endif So, here's what I don't understand. On ARM, in general, we can't rely on the global data pointer register (r8) to be set to a useful value, so we do the above to ensure it points to something useful. Are you always able to rely on r2 it looks like pointing to something useful? Or do you take care of this much earlier on in powerpc? Thanks! -- Tom ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] SPL: Port SPL framework to powerpc
On 08/23/2012 01:12 AM, Stefan Roese wrote:
This patch enables the SPL framework to be used on powerpc platforms
and not only ARM.
[snip]
+#ifdef CONFIG_PPC
+static void __noreturn jump_to_image_linux(void *arg)
+{
+ debug(Entering kernel arg pointer: 0x%p\n, arg);
+ typedef void (*image_entry_arg_t)(void *, ulong r4, ulong r5, ulong r6,
+ ulong r7, ulong r8, ulong r9)
+ __attribute__ ((noreturn));
+ image_entry_arg_t image_entry =
+ (image_entry_arg_t)spl_image.entry_point;
+
+ image_entry(arg, 0, 0, EPAPR_MAGIC, CONFIG_SYS_BOOTMAPSZ, 0, 0);
+}
+#endif /* CONFIG_PPC */
+#endif /* CONFIG_SPL_OS_BOOT */
This, along with board_init_f make me wonder if we shouldn't have an
arch/${ARCH}/lib/spl.c that contains them and make them non-static.
Perhaps sharing with the non-SPL code portion as well?
--
Tom
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
