[U-Boot] [PATCHv1] ARM: Add Altera SOCFPGA Cyclone5

[Pavel Machek]

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