Re: [U-Boot] [PATCH v3 2/2] amcore: add support for amcore board
Dear Angelo Dureghello,
please make sure to keep the mailing list on Cc: !
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.
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
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@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
participants (2)
-
Angelo Dureghello -
Wolfgang Denk