Hi!
> >> I think once you get the answers you should be able to re-post the
> >> series cleanly and depend on my v5 (or v6) branch. Thanks!
> >
> > Ok, figuring the .lds might be tricky. But... that's mostly additional
> > (safety) feature. Would it be possible to merge it without that and
> > add it later?
> >
> > Port to v5 of the patch should not be a problem.
> >
> > For the record and review, current version of patch is here.
>
> A few small comments. No need to re-post intermediate patches, just
> address in the next full go-round. Thanks!
They were mostly for synchronization with Dinh. Sorry for the noise.
> > +void spl_board_init(void)
> [snip]
> > + puts("SPL Boot\n");
>
> Sure you want to keep that line? We already have a general "U-Boot SPL
> ..." line being printed.
Killed.
> > + * miscellaneous platform dependent initialisations
> > + */
> > +int misc_init_r(void)
> > +{
> > + /* Set to "n" for not verifying the uImage */
> > + setenv("verify", "n");
> > + return 0;
> > +}
>
> Not good to enforce policy like this. Just don't set it in the config
> file by default and let users do as they need.
Hmm. I moved it to CONFIG_EXTRA_ENV_SETTINGS as done by for eample
u8500_href.h. Is that ok?
> [snip]
> > diff --git a/common/spl/spl.c b/common/spl/spl.c
> > index eaea1c8..5adbf0e 100644
> > --- a/common/spl/spl.c
> > +++ b/common/spl/spl.c
> > @@ -78,6 +78,7 @@ void spl_parse_image_header(const struct image_header
> > *header)
> > u32 header_size = sizeof(struct image_header);
> >
> > if (__be32_to_cpu(header->ih_magic) == IH_MAGIC) {
> > + /* Valid image. Extract information out of 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 */
>
> Just an extra comment, drop please. Or split out if you feel it's
> really helpful. No strong opinion here other than not in the same patch
> as the rest.
Ok, I'll drop it, and the omap cleanup also. Not worth the merge
effort.
spl_ram_load_image... will I need to create some kind of #ifdef? Or
would #ifdef BOOT_DEVICE_RAM do the trick?
> [snip]
> > diff --git a/include/common.h b/include/common.h
> > index 55025c0..469a3af 100644
> > --- a/include/common.h
> > +++ b/include/common.h
> > @@ -704,7 +704,9 @@ void external_interrupt (struct pt_regs *);
> > void irq_install_handler(int, interrupt_handler_t *, void *);
> > void irq_free_handler (int);
> > void reset_timer (void);
> > -ulong get_timer (ulong base);
> > +ulong get_timer(ulong base);
> > +void reset_timer_count(void);
> > +ulong get_timer_count(ulong base);
> > void enable_interrupts (void);
> > int disable_interrupts (void);
>
> Make the new declarations match the old, spacing wise please.
Ok, but checkpatch will complain. ... .... hmm. Why do I have feeling
that they are not needed at all? Removed, and things still work. Good.
Rest was fixed in obvious way. Thanks for comments!
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
