Add support for KMC KZM-ARM11-01 board
PATCH v2 1/1 Add support for KMC KZM-ARM11-01 board
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]>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: kzm_arm11fix.diff
Type: text/x-diff
Size: 23939 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080909/76b1b36d/attachment-0001.diff
PATCH v2 1/1 Add support for KMC KZM-ARM11-01 board
Dear Atsuo Igarashi,
Quoted 11 lines · 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.
...
Quoted 3 lines
(quoted diff) +3
-0+int board_init (void) +{ + int i;
...
Review
Quoted 2 lines
(quoted diff) +2
-0+ /* RedBoot also has an empty loop with 100000 iterations here - + * clock doesn't run yet */
Incorrect multiline comment.
Quoted 2 lines
(quoted diff) +2
-0+ for (i = 0; i < 100000; i++) + ;
...
Review
Quoted 3 lines
(quoted diff) +3
-0+ /* 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?
Review
Quoted 11 lines
(quoted diff) +11
-0+/* + * 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.
Quoted 1 line
+#define
CFG_HZCONFIG_MX31_CLK32/* use 32kHz clock as source */
This is wrong. CFG_HZ must always be defined as 1000; it is a constant.
Review
Quoted 9 lines
(quoted diff) +9
-0+/* 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 ...
?
Best regards,
Wolfgang Denk
Signature · 5 lines
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 Every living thing wants to survive. -- Spock, "The Ultimate Computer", stardate 4731.3
PATCH v2 1/1 Add support for KMC KZM-ARM11-01 board
Dear Wolfgang Denk,
Quoted 12 lines
Dear Atsuo Igarashi,
Quoted 9 lines · 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.
Quoted 8 lines
Quoted 1 line
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.
Quoted 14 lines
...
Quoted 3 lines
(quoted diff) +3
-0+int board_init (void) +{ + int i;...
Review
Quoted 2 lines
(quoted diff) +2
-0+ /* RedBoot also has an empty loop with 100000 iterations here - + * clock doesn't run yet */Incorrect multiline comment.
Quoted 2 lines
(quoted diff) +2
-0+ for (i = 0; i < 100000; i++) + ;...
Quoted 9 lines
Review
Quoted 3 lines
(quoted diff) +3
-0+ /* 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.
Quoted 6 lines
Quoted 6 lines
+/*
+ * 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
Quoted 9 lines
Quoted 5 lines
+ * 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.
+ */
+#defineCONFIG_ARP_TIMEOUT200ULIs this really needed on your hardware, too? I doubt it.
Probably, this is not needed.
I'll remove this.
Quoted 5 lines
Quoted 1 line
+#define
CFG_HZCONFIG_MX31_CLK32/* use 32kHz clock as source */This is wrong.
CFG_HZmust always be defined as 1000; it is a constant.
Okay, and it seems that I should fix something else too.
Quoted 15 lines
Review
Quoted 9 lines
(quoted diff) +9
-0+/* 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"):
Quoted 8 lines
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
Quoted 5 lines
Best regards,
Wolfgang Denk
PATCH v2 1/1 Add support for KMC KZM-ARM11-01 board
Dear Wolfgang Denk,
Quoted 22 lines
Quoted 12 lines
Quoted 12 lines
+/*
+ * 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. ReducingQuoted 9 lines
Quoted 5 lines
the ARP
+ * timeout from default 5 seconds to 200ms we speed up the initial TFTP
+ * transfer, should the user wish one, significantly.
+ */
+#defineCONFIG_ARP_TIMEOUT200ULIs this really needed on your hardware, too? I doubt it.
Quoted 3 lines
Probably, this is not needed.
I'll remove this.
In our board which has SMC9118 controller, it seems that it needs a few hundred milliseconds wait after the initialization. So this is needed. I'll modify only these comment.
Quoted 25 lines
Quoted 25 lines
Quoted 5 lines
(quoted diff) +5
-0+/* 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 beginningQuoted 7 lines
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. */
+#defineCFG_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) copysector 5: 128 KiB - more U-Boot code
...
Quoted 5 lines
Quoted 1 line
?
Indeed. I think so too.
I'll rework.
In our board which has S29GL512N NOR flash, it has 512 128KiB big sector only. So I leave this, and I'll modify only these comment.
sector 1: 128 KiB - U-Boot code
sector 2: 128 KiB - environment, first copy sector 3: 128 KiB - environment, second (redundant) copy sector 4: 128 KiB - more U-Boot code
Regards,
Atsuo Igarashi