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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox