From: Pavel Machek <pavel@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCHv1] ARM: Add Altera SOCFPGA Cyclone5
Date: Thu, 30 Aug 2012 19:18:42 +0200 [thread overview]
Message-ID: <20120830171842.GD5980@elf.ucw.cz> (raw)
In-Reply-To: <503EAD32.90806@ti.com>
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
next prev parent reply other threads:[~2012-08-30 17:18 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-16 22:45 [U-Boot] [PATCHv1] ARM: Add Altera SOCFPGA Cyclone5 dinguyen at altera.com
2012-08-22 22:10 ` Pavel Machek
2012-08-22 22:26 ` Marek Vasut
2012-08-23 1:13 ` Tom Rini
2012-08-23 10:43 ` Pavel Machek
2012-08-23 11:18 ` Stefan Roese
2012-08-29 12:30 ` Pavel Machek
2012-08-29 12:32 ` Stefan Roese
2012-08-24 19:18 ` Tom Rini
2012-08-25 9:03 ` Pavel Machek
2012-08-25 10:42 ` Pavel Machek
2012-08-25 11:56 ` Pavel Machek
2012-08-27 15:43 ` Tom Rini
2012-08-29 12:07 ` Pavel Machek
2012-08-25 12:05 ` Pavel Machek
2012-08-27 15:57 ` Tom Rini
2012-08-29 12:27 ` Pavel Machek
2012-08-29 13:41 ` Pavel Machek
2012-08-29 18:26 ` Tom Rini
2012-08-29 23:21 ` Pavel Machek
2012-08-30 0:00 ` Tom Rini
2012-08-30 17:18 ` Pavel Machek [this message]
2012-08-30 17:34 ` Tom Rini
2012-08-30 17:46 ` Tom Rini
2012-08-30 18:05 ` Pavel Machek
2012-08-30 18:30 ` Tom Rini
2012-08-30 20:42 ` Pavel Machek
2012-08-30 22:25 ` Tom Rini
2012-08-31 21:26 ` Dinh Nguyen
2012-08-31 21:37 ` Tom Rini
2012-08-31 7:02 ` Stefan Roese
2012-08-29 23:34 ` Pavel Machek
2012-08-29 23:56 ` Marek Vasut
2012-08-30 16:59 ` Pavel Machek
2012-08-30 18:02 ` Marek Vasut
2012-08-30 18:12 ` Pavel Machek
2012-08-30 18:16 ` Marek Vasut
2012-09-03 2:59 ` Chin Liang See
2012-08-30 0:03 ` Tom Rini
2012-08-30 17:20 ` Pavel Machek
2012-08-23 10:56 ` Pavel Machek
2012-08-23 10:50 ` Pavel Machek
2012-08-24 22:59 ` Tom Rini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120830171842.GD5980@elf.ucw.cz \
--to=pavel@denx.de \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.