Dear Wolfgang Denk, > Dear Atsuo Igarashi, > > In message <[EMAIL PROTECTED]> you wrote: >> Hi, >> >> I've fixed which Jean-Christophe pointed out. >> >> Regards, >> Atsuo >> --- >> This patch adds support for KMC KZM-ARM11-01 board. >> >> Signed-off-by Atsuo Igarashi <[EMAIL PROTECTED]> > > The you have the content of the message swapped. The "Hi, I've > ..Regards, Atsuo" part belongs *below the '---' line, and the "This > patch ... Signed-off-by:" part belongs *above* it. > > The text above the '---' line will become the commit message, the text > below it is just comment.
Sorry, I'll do so next time. > > ... >> +int board_init (void) >> +{ >> + int i; > ... >> + /* RedBoot also has an empty loop with 100000 iterations here - >> + * clock doesn't run yet */ > > Incorrect multiline comment. > >> + for (i = 0; i < 100000; i++) >> + ; > ... >> + /* clock still doesn't run */ >> + for (i = 0; i < 100000; i++) >> + ; > > Be careful here, this might not do what you think it does. The > compiler might optimize these loops away. > > Why are they needed in the first place? > Indeed. Since these are based on Freescale's ADS board, I'm not sure what means these loops, but these not necessary for my board. I'll remove these code of the CPLD initializing. >> +/* >> + * The MX31ADS board seems to have a hardware "peculiarity" confirmed under >> + * U-Boot, RedBoot and Linux: the ethernet Rx signal is reaching the CS8900A >> + * controller inverted. The controller is capable of detecting and >> correcting >> + * this, but it needs 4 network packets for that. Which means, at startup, >> you >> + * will not receive answers to the first 4 packest, unless there have been >> some >> + * broadcasts on the network, or your board is on a hub. Reducing the ARP >> + * timeout from default 5 seconds to 200ms we speed up the initial TFTP >> + * transfer, should the user wish one, significantly. >> + */ >> +#define CONFIG_ARP_TIMEOUT 200UL > > > Is this really needed on your hardware, too? I doubt it. > Probably, this is not needed. I'll remove this. >> +#define CFG_HZ CONFIG_MX31_CLK32 /* use 32kHz clock as >> source */ > > This is wrong. CFG_HZ must always be defined as 1000; it is a > constant. > Okay, and it seems that I should fix something else too. >> +/* Address and size of Redundant Environment Sector */ >> +#define CFG_ENV_OFFSET_REDUND (CFG_ENV_OFFSET + CFG_ENV_SIZE) >> +#define CFG_ENV_SIZE_REDUND CFG_ENV_SIZE >> + >> +/* S29WS256N NOR flash has 4 32KiB small sectors at the beginning and at >> the end. >> + * The rest of 32MiB is in 128KiB big sectors. U-Boot occupies the low 4 >> sectors, >> + * if we put environment next to it, we will have to occupy 128KiB for it. >> + * Putting it at the top of flash we use only 32KiB. */ >> +#define CFG_ENV_ADDR (CFG_MONITOR_BASE + CFG_ENV_SECT_SIZE) > > This is a very ugly set up, because you split the U-Boot related > informatiomn, and you block both ends of your flash memory. Why don;t > you locate the environment for example in the third and fourts > sectors, i. e. use such a setup (called "embedded environment"): > > sector 1: 32 KiB - U-Boot code > sector 2: 32 KiB - more U-Boot code > sector 3: 32 KiB - environment, first copy > sector 4: 32 KiB - environment, second (redundant) copy > sector 5: 128 KiB - more U-Boot code > ... > > ? Indeed. I think so too. I'll rework. Regards, Atsuo Igarashi > > Best regards, > > Wolfgang Denk > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot