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