All of lore.kernel.org
 help / color / mirror / Atom feed
From: Angelo Dureghello <sysamfw@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v3 2/2] amcore: add support for amcore board
Date: Mon, 5 Nov 2012 21:38:50 +0100	[thread overview]
Message-ID: <20121105203849.GA9026@angel3> (raw)
In-Reply-To: <20121105184856.ECF432005A9@gemini.denx.de>

On Mon, Nov 05, 2012 at 07:48:56PM +0100, Wolfgang Denk wrote:
> Dear Angelo Dureghello,
> 
> please make sure to keep the mailing list on Cc: !
> 

ok.

> In message <20121105153011.GA8705@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.
> 
> 

ok, will send it together with the board related part of the patch (2/2).

> > > 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.
> 

Code compile, but i lost some warning on the way in the build process. 
I will check that there are not warning of any kind.

> > > > +/*
> > > > + * 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.
> 

Ok, i will go deeper here and will remove the comment if the case.

> > I will figure out if there is a different way
> > without adding a new flash chip here. 
> 
> Just use the CFI flash driver?
> 

This first version of the board has a data byte swap hw issue.
The 16bit data bus that connect the flash to the cpu is swapped.
Linux has an option fo "forgive" this kind of issues:

Flash cmd/query data swapping (LITTLE_ENDIAN_BYTE) 

So if there u-boot has a similar "byte-swap" sw option i can
probably use std CFI driver, otherwise let me know if you can
accept this specific 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@csnews.cs.colorado.edu>

  reply	other threads:[~2012-11-05 20:38 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-04 20:00 [U-Boot] [PATCH v3 2/2] amcore: add support for amcore board angelo
2012-11-04 22:02 ` Wolfgang Denk
     [not found]   ` <20121105153011.GA8705@angel3>
2012-11-05 18:48     ` Wolfgang Denk
2012-11-05 20:38       ` Angelo Dureghello [this message]
2012-11-05 21:10   ` Angelo Dureghello
2012-11-06  7:24     ` Wolfgang Denk

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=20121105203849.GA9026@angel3 \
    --to=sysamfw@gmail.com \
    --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.