In message 1164940274.5921.26.camel@localhost.localdomain you wrote:
- it does not merge cleanly with the current top of tree in the git repo; there are conflicts with common/cfi_flash.c
Fixed.
Thanks.
- there are lots of coding style violations:
I checked all my commit files and fixed all the coding style violations as far as my understand :-). Sorry for some garbage codes :-).
Thanks.
- board/mpc7448hpc2/mpc7448hpc2.c contains yet another memory test. Do we really need another copy of this code?
Do you mean testdram () function? I can see this function in many other boards. memtest command can do dram test. While I still think ,this function can benefit end users when testing or debugging.
As far as I can tell your code is mostly a verbatim copy of post/memory.c - if you need a good memory test then please configure POST on your system and use the POST code instead of copying it.
- The login here looks weird to me - is this correct? cpu/74xx_7xx/speed.c: ... +#ifdef CFG_CONFIG_BUS_CLK + gd->bus_clk = get_board_bus_clk(); +#else + gd->bus_clk = CFG_BUS_CLK; +#endif
#ifdef CFG_CONFIG_BUS_CLK
the bus clock can be configured by external switch, just as the mpc7448hpc2 board
#else the bus clock is a fixed one.
But why do we need both CFG_CONFIG_BUS_CLK and CFG_BUS_CLK ?
- Please don't define CONFIG_ETHADDR / CONFIG_ETH1ADDR in your board config file. It is really evil when all boards have the same MAC addresses. Also, are the addresses you used officially assigned ones?
00:06:D2 is officially assigned to Tundra :-).
Anyway: please don;t define MAC addresses in the board config file. It will only cause harm.
Same is for CONFIG_IPADDR, CONFIG_SERVERIP, CONFIG_NETMASK, CONFIG_GATEWAYIP - it may save some time to have these set during development, but for a public source version I don't ever want to see these.Can you make sure all of these define are not necessary? I can see them in many other board.
There may be a *few* boards that do this, but in general it's a very bad idea: it works fine for the guy who is eorking on the U-Boot port, because he usually uses just a single board. But as soon as you have a second board in the net it becomes a major PITA. Please don't do it.
If needed, you can set up a valid per-board network setup as part of the software initialization - a simple expect script can do magic.
- In lib_ppc/extable.c you add code with a "#ifdef CFG_EXCEPTION_AFTER_RELOCATE; there is absolutely no explanation nor comment anywhere why you think this is necessary.
I need to deal with exception after the code relocation. I need add the
Why do you need to do this?
gd->reloc_off to search the exception table. If I do not find better method for this, I will add a detailed comment here.
I think this part of the code is pretty generic. I would like to understand why on your board such a change is necessary which is not needed on any other system.
Thanks for your effort to review my code :-).
You are welcome.
Best regards,
Wolfgang Denk