[U-Boot] [PATCH v3 2/2] amcore: add support for amcore board

[Wolfgang Denk]

Dear Angelo Dureghello,

please make sure to keep the mailing list on Cc: !

In message <20121105153011.GA8705 at angel3> you wrote:
> 
> > >  board/sysam/amcore/Makefile        |   43 ++++
> > >  board/sysam/amcore/amcore.c        |  168 ++++++++++++++
> > >  board/sysam/amcore/config.mk       |   23 ++
> > >  board/sysam/amcore/flash.c         |  444 ++++++++++++++++++++++++++++++++++++
> > >  board/sysam/amcore/u-boot.lds      |  101 ++++++++
> > >  boards.cfg                         |    1 +
> > >  include/configs/amcore.h           |  213 +++++++++++++++++
> > >  include/flash.h                    |    1 +
> > >  8 files changed, 994 insertions(+), 0 deletions(-)
> > 
> > Entry to MAINTAINERS missing.
> > 
> 
> MAINTAINERS entry has been created in patch 1/2.

This is wrong, then.  Adding the board maintainer obviously belongs
into the patch that adds the board support.


> > Did you bother to compile your code?  This is a function returning
> > "int", but I don't see any return statement.  I would expect to see
> > compiler warnings here?
> 
> Of course code compile and u-boot works perfect in my board.
> Fixed this and will check for all warnings anyway.

I don't understand what you are trying to tell me.  Do you mean your
compiler did not throw any warnings for this code?  Or did it, and you
ignored the warnings?

Note that the build must not produce _any_ compiler warnings.

> > > +/*
> > > + * For booting Linux, the board info and command line data
> > > + * have to be in the first 8 MB of memory, since this is
> > > + * the maximum mapped by the Linux kernel during initialization ??
> > > + */
> > 
> > Is this really the case?
> 
> I copied this config value and related comment from another board. 
> There are hundreds of boardsxxx.h saying the same.
> I didn't try to change this limit and assume the comment is true.

There are many boards where this may be correct.  But blindly copying
stuff around has never been a good idea.

> I will figure out if there is a different way
> without adding a new flash chip here. 

Just use the CFI flash driver?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
It's certainly  convenient  the  way  the  crime  (or  condition)  of
stupidity   carries   with   it  its  own  punishment,  automatically
admisistered without remorse, pity, or prejudice. :-)
         -- Tom Christiansen in <559seq$ag1$1 at csnews.cs.colorado.edu>