[U-Boot-Users] [GIT PULL] [ARM] Please pull from git://denx.de/git/u-boot-arm.git
Wolfgang
Please pull from git://denx.de/git/u-boot-arm.git
It (actually) contains the following changes from Ulf - next time the commit messages will match the CHANGELOG entries......
Add MACH_TYPE records for several AT91 boards Merge to two at45.c files into a common file and split to at45.c and spi.c Fix spelling error in DM9161 PHY Support Initialize at91rm9200 board (and set LED) Add PIO control for at91rm9200dk LEDs and Mux. Change dataflash partition boundaries to be compatible with Linux 2.6
Signed-off-by: Peter Pearse peter.pearse@arm.com Signed-off-by: Ulf Samuelsson ulf@atmel.com
Unfortunately I shall not have time to request any more pulls before 17th August 2007.
Regards
Peter
---
The following changes since commit 4ef35e53c693556c54b0c22d6f873de87bade253: Wolfgang Denk (1): Coding style cleanup, update CHANGELOG
are found in the git repository at:
git://denx.de/git/u-boot-arm.git
Peter Pearse (6): Add MACH_TYPE records for several AT91 boards. Add the files. Delete the merged files. Update Makefiles for merged and split at45.c. Replace lost end of at45.c. Supply spi interface in at45.c
board/at91rm9200dk/Makefile | 2 +- board/at91rm9200dk/at45.c | 621 -------------------------- board/at91rm9200dk/led.c | 80 ++++ board/at91rm9200dk/mux.c | 39 ++ board/cmc_pu2/Makefile | 2 +- board/cmc_pu2/at45.c | 621 -------------------------- common/soft_i2c.c | 2 +- cpu/arm920t/at91rm9200/Makefile | 2 +- cpu/arm920t/at91rm9200/dm9161.c | 11 +- cpu/arm920t/at91rm9200/spi.c | 153 +++++++ cpu/arm920t/start.S | 93 ++++- drivers/Makefile | 2 +- drivers/at45.c | 569 +++++++++++++++++++++++ drivers/dataflash.c | 279 +++++++++--- include/asm-arm/arch-at91rm9200/AT91RM9200.h | 139 ++++--- include/asm-arm/mach-types.h | 69 +++ include/at45.h | 69 +++ include/config_cmd_all.h | 1 + include/configs/at91rm9200dk.h | 5 + include/dataflash.h | 43 ++- include/dm9161.h | 4 +- include/flash.h | 5 + include/led.h | 46 ++ 23 files changed, 1474 insertions(+), 1383 deletions(-) mode change 100644 => 100755 board/at91rm9200dk/Makefile delete mode 100644 board/at91rm9200dk/at45.c create mode 100644 board/at91rm9200dk/led.c create mode 100644 board/at91rm9200dk/mux.c mode change 100644 => 100755 board/cmc_pu2/Makefile delete mode 100644 board/cmc_pu2/at45.c create mode 100644 cpu/arm920t/at91rm9200/spi.c mode change 100644 => 100755 drivers/Makefile create mode 100755 drivers/at45.c create mode 100644 include/at45.h create mode 100644 include/led.h
---
Dear Peter,
in message 000001c7de84$ee21a460$821ba8c0@Emea.Arm.com you wrote:
Please pull from git://denx.de/git/u-boot-arm.git
It (actually) contains the following changes from Ulf - next time the commit messages will match the CHANGELOG entries......
Add MACH_TYPE records for several AT91 boards Merge to two at45.c files into a common file and split to at45.c and spi.c Fix spelling error in DM9161 PHY Support Initialize at91rm9200 board (and set LED) Add PIO control for at91rm9200dk LEDs and Mux. Change dataflash partition boundaries to be compatible with Linux 2.6
Signed-off-by: Peter Pearse peter.pearse@arm.com Signed-off-by: Ulf Samuelsson ulf@atmel.com
Unfortunately I shall not have time to request any more pulls before 17th August 2007.
thanks, done.
Here a few comments regarding the code changes:
There were coding style issues in the following files:
* Trailing white space: board/at91rm9200dk/mux.c, drivers/at45.c, drivers/dataflash.c, include/at45.h, include/dataflash.h * Trailing empty lines: board/at91rm9200dk/mux.c, cpu/arm920t/at91rm9200/spi.c, drivers/at45.c, include/led.h
I cleaned this up already.
The following issues need to be addressed:
include/led.h => seems specific to the at91rm9200dk board only, please move to board directory. Also, the calls to these functions should be handled without #ifdef's in common code. Either use existing functions hooks, or weak references.
drivers/at45.c => has coding style issues - indentation of long, wrapped around lines is broken.
cpu/arm920t/start.S => I'm surprised to see a lot of intialization code has now been added to the "reset" entry point. Is this by accident, or am I missing something?
include/config_cmd_all.h: "CONFIG_CMD_MUX" is a pretty generic name, while here it is used for a board or CPU specific feature only. I suggest you rename this into something which has a "AT91" in the name, like "CONFIG_CMD_AT91_SPIMUX" or so.
Thanks again.
Best regards,
Wolfgang Denk
Will address when I return from holiday (Wednesday 21st August)
-----Original Message----- From: wd@denx.de [mailto:wd@denx.de] Sent: 14 August 2007 17:10 To: Peter Pearse Cc: u-boot-users@lists.sourceforge.net; Ulf Samuelsson Subject: Re: [GIT PULL] [ARM] Please pull from git://denx.de/git/u-boot-arm.git
Dear Peter,
in message 000001c7de84$ee21a460$821ba8c0@Emea.Arm.com you wrote:
Please pull from git://denx.de/git/u-boot-arm.git
It (actually) contains the following changes from Ulf -
next time the
commit messages will match the CHANGELOG entries......
Add MACH_TYPE records for several AT91 boards Merge to two at45.c files into a common file and split to
at45.c and spi.c
Fix spelling error in DM9161 PHY Support Initialize at91rm9200 board (and set LED) Add PIO control for at91rm9200dk LEDs and Mux. Change dataflash partition boundaries to be compatible with Linux 2.6
Signed-off-by: Peter Pearse peter.pearse@arm.com Signed-off-by: Ulf Samuelsson ulf@atmel.com
Unfortunately I shall not have time to request any more
pulls before
17th August 2007.
thanks, done.
Here a few comments regarding the code changes:
There were coding style issues in the following files:
* Trailing white space: board/at91rm9200dk/mux.c,drivers/at45.c, drivers/dataflash.c, include/at45.h, include/dataflash.h * Trailing empty lines: board/at91rm9200dk/mux.c, cpu/arm920t/at91rm9200/spi.c, drivers/at45.c, include/led.h
I cleaned this up already.
The following issues need to be addressed:
include/led.h => seems specific to the at91rm9200dk board only, please move to board directory. Also, the calls to these functions should be handled without #ifdef's in common code. Either use existing functions hooks, or weak references.
drivers/at45.c => has coding style issues - indentation of long, wrapped around lines is broken.
cpu/arm920t/start.S => I'm surprised to see a lot of intialization code has now been added to the "reset" entry point. Is this by accident, or am I missing something?
include/config_cmd_all.h: "CONFIG_CMD_MUX" is a pretty generic name, while here it is used for a board or CPU specific feature only. I suggest you rename this into something which has a "AT91" in the name, like "CONFIG_CMD_AT91_SPIMUX" or so.
Thanks again.
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 Q: How do you spell "onomatopoeia"? A: The way it sounds.
In message 000101c7de92$7fb2e350$821ba8c0@Emea.Arm.com you wrote:
Will address when I return from holiday (Wednesday 21st August)
Sure. Have a nice vacation - and thanks again for making it in time for the current merge window.
Best regards,
Wolfgang Denk
-----Original Message----- From: wd@denx.de [mailto:wd@denx.de] Sent: 14 August 2007 17:10 To: Peter Pearse Cc: u-boot-users@lists.sourceforge.net; Ulf Samuelsson Subject: Re: [GIT PULL] [ARM] Please pull from git://denx.de/git/u-boot-arm.git
Dear Peter,
in message 000001c7de84$ee21a460$821ba8c0@Emea.Arm.com you wrote:
Please pull from git://denx.de/git/u-boot-arm.git
It (actually) contains the following changes from Ulf -
next time the
commit messages will match the CHANGELOG entries......
Add MACH_TYPE records for several AT91 boards Merge to two at45.c files into a common file and split to
at45.c and spi.c
Fix spelling error in DM9161 PHY Support Initialize at91rm9200 board (and set LED) Add PIO control for at91rm9200dk LEDs and Mux. Change dataflash partition boundaries to be compatible with Linux 2.6
Signed-off-by: Peter Pearse peter.pearse@arm.com Signed-off-by: Ulf Samuelsson ulf@atmel.com
<snip>
The following issues need to be addressed:
include/led.h => seems specific to the at91rm9200dk board only, please move to board directory. Also, the calls to these functions should be handled without #ifdef's in common code. Either use existing functions hooks, or weak references.
Please explain further. What is a weak reference? Which hooks?
drivers/at45.c => has coding style issues - indentation of long, wrapped around lines is broken.
cpu/arm920t/start.S => I'm surprised to see a lot of intialization code has now been added to the "reset" entry point. Is this by accident, or am I missing something?
The changes are there to allow you to boot from a dataflash. A bootstrap will initialize the SDRAM and copy from the dataflash (or NAND flash) IIRC: The code avoids reinitialization of stuff which will crash U-Boot. (And sets the LED)
include/config_cmd_all.h: "CONFIG_CMD_MUX" is a pretty generic name, while here it is used for a board or CPU specific feature only. I suggest you rename this into something which has a "AT91" in the name, like "CONFIG_CMD_AT91_SPIMUX" or so.
Thanks again.
Best regards,
Wolfgang Denk
Best Regards Ulf Samuelsson
Dear Ulf,
in message 002c01c7dfdb$280ab250$dcc4af0a@atmel.com you wrote:
include/led.h => seems specific to the at91rm9200dk board only, please move to board directory. Also, the calls to these functions should be handled without #ifdef's in common code. Either use existing functions hooks, or weak references.
Please explain further. What is a weak reference?
See gcc.info, Node: Function Attributes:
... `weak' The `weak' attribute causes the declaration to be emitted as a weak symbol rather than a global. This is primarily useful in defining library functions which can be overridden in user code, though it can also be used with non-function declarations. ...
See for example the implementation of the declaration of show_boot_progress() in common/main.c and the use of this function all over the place.
Which hooks?
You could probably avoid all your own code if you had used show_boot_progress() instead. Alternatively or in combination, you could and should have used drivers/status_led.c
cpu/arm920t/start.S => I'm surprised to see a lot of intialization code has now been added to the "reset" entry point. Is this by accident, or am I missing something?
The changes are there to allow you to boot from a dataflash. A bootstrap will initialize the SDRAM and copy from the dataflash (or > NAND flash) IIRC: The code avoids reinitialization of stuff which will crash U-Boot. (And sets the LED)
I'm afraid I don't understand. We are not talking about any startup code, but about the code of the "reset" function, i. e. what casues the board to reset.
Maybe the code was added to the wrong place in the file?
Best regards,
Wolfgang Denk
-----Original Message----- From: wd@denx.de [mailto:wd@denx.de] Sent: 16 August 2007 10:41 To: Ulf Samuelsson Cc: Peter Pearse; u-boot-users@lists.sourceforge.net Subject: Re: [U-Boot-Users] [GIT PULL] [ARM] Please pull fromgit://denx.de/git/u-boot-arm.git
Dear Ulf,
cpu/arm920t/start.S => I'm surprised to see a lot of
intialization
code has now been added to the "reset"entry point.
Is this by accident, or am I missing something?The changes are there to allow you to boot from a dataflash. A bootstrap will initialize the SDRAM and copy from the
dataflash (or
NAND flash)
IIRC: The code avoids reinitialization of stuff which will
crash U-Boot.
(And sets the LED)
I'm afraid I don't understand. We are not talking about any startup code, but about the code of the "reset" function, i. e. what casues the board to reset.
The writers (and cut & pasters) of ARM code seem to have assumed that
reset:
is the entry to the code which runs immediately after the processor is reset, rather than the code which makes the processor reset.
Could we accept Ulf's code whilst I give this more consideration?
Peter
Maybe the code was added to the wrong place in the file?
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 is easier to port a shell than a shell script." - Larry Wall
Dear Peter,
in message 000a01c7e98e$949af6c0$821ba8c0@Emea.Arm.com you wrote:
cpu/arm920t/start.S => I'm surprised to see a lot of intialization code has now been added to the "reset" entry point. Is this by accident, or am I missing something?
The changes are there to allow you to boot from a dataflash. A bootstrap will initialize the SDRAM and copy from the
dataflash (or
NAND flash)
IIRC: The code avoids reinitialization of stuff which will
crash U-Boot.
(And sets the LED)
I'm afraid I don't understand. We are not talking about any startup code, but about the code of the "reset" function, i. e. what casues the board to reset.
The writers (and cut & pasters) of ARM code seem to have assumed that
reset:
is the entry to the code which runs immediately after the processor is reset, rather than the code which makes the processor reset.
Well, if that was the assumption, that all this code has obviously never been tested on any real hardware. Which means it should have never been submitted to the list.
Could we accept Ulf's code whilst I give this more consideration?
It's so utterly wrong that I think we have to clean this up before the next release. If nobody submits a better patch, we will have to remove the bogus code.
Best regards,
Wolfgang Denk
participants (3)
-
Peter Pearse -
Ulf Samuelsson -
Wolfgang Denk