
Dear Wolfgang,
Thanks for the feedback.
...
+int misc_init_r(void) +{
...
- /* reset all SGMII interfaces */
- mfsdr(SDR0_SRST1, reg);
- reg |= (SDR0_SRST1_SGMII0 | SDR0_SRST1_SGMII1 | SDR0_SRST1_SGMII2);
- mtsdr(SDR0_SRST1, reg);
- mtsdr(SDR0_ETH_STS, 0xFFFFFFFF);
- mtsdr(SDR0_SRST1, 0x00000000);
- for (timeout = 60; timeout > 0; timeout--) {
udelay(1000);
mfsdr(SDR0_ETH_PLL, eth_pll);
if ((eth_pll & SDR0_ETH_PLL_PLLLOCK) != 0)
break;
- }
Where is the 60 ms timeout coming from?
This is a very high mark number we came up with when running some test while waiting for SDR0_ETH_PLL_PLLLLOCK. Actually a timeout is really not needed PLL LOCK is always achieved. No initialization of the SDR0_ETH_PLL required. The SDR0_ETH_PLL is configured by default after reset. The code should look more like this.
do { mfsdr(SDR0_ETH_PLL, eth_pll); } while (!(eth_pll & SDR0_ETH_PLL_PLLLOCK));
--- a/include/configs/amcc-common.h +++ b/include/configs/amcc-common.h @@ -55,6 +55,13 @@ #endif
/*
- Only very few boards have default netdev not set to eth0 (like Arches)
- */
+#if !defined(CONFIG_NETDEV) +#define CONFIG_NETDEV eth0 +#endif
Please don't add this to common code; please handle it locally in the arches configuration.
We do handle this locally in arches config section (canyonlands.h). The above is the default. We need this because eth0 in Arches is used for CPU-to-CPU Communcation via ethernet. Eth1 is used for normal RJ45 connection.
@@ -147,9 +154,11 @@ /*
- Booting and default environment
*/ +#if !defined(CONFIG_PREBOOT) #define CONFIG_PREBOOT "echo;" \ "echo Type "run flash_nfs" to mount root filesystem over NFS;" \ "echo" +#endif
Ditto. [Also: what's the reason for this change? ]
Once again. This is the default, we overwrite this in our arches Config section (canyonlands.h). We modify CONFIG_PREBOOT to Set the default ethact to ppc_4xx_eth1. See below
- "net_self_load=tftp ${kernel_addr_r} ${bootfile};" \
"tftp ${fdt_addr_r} ${fdt_file};" \
"tftp ${ramdisk_addr_r} ${ramdisk_file};\0" \
What is this needed for?
net_self_load is used by net_self to load linux, fdt blob, ramdisk via tftp. this is very similar to net_nfs, flash_nfs and flash_self
diff --git a/include/configs/canyonlands.h b/include/configs/canyonlands.h index 2f162e1..acad9b3 100644 --- a/include/configs/canyonlands.h +++ b/include/configs/canyonlands.h
...
+#define CONFIG_PREBOOT \
- "setenv ethact ppc_4xx_eth1;" \
This should not be done in the preboot command, but as part of the default config instead.
Yes, you are correct. I will fix this.
-#define CFG_AHB_BASE 0xE2000000 /* internal AHB peripherals */ +#define CFG_AHB_BASE 0xE2000000 /* internal AHB peripherals */
Is this change an improvement?
No
@@ -223,6 +268,7 @@
- DDR SDRAM
*----------------------------------------------------------------------------*/ #if !defined(CONFIG_NAND_U_BOOT) +#if !defined(CONFIG_ARCHES) /*
- NAND booting U-Boot version uses a fixed initialization, since the whole
- I2C SPD DIMM autodetection/calibration doesn't fit into the 4k of boot
Are you sure this is correct?
I will investigate.
+#if !defined(CONFIG_ARCHES) /* Only Canyonlands (460EX) has USB */ #ifdef CONFIG_460EX
I think it would make more sense to move the "!defined(CONFIG_ARCHES)" _after_ the "#ifdef CONFIG_460EX".
Will double check
+#else /* defined(CONFIG_ARCHES) */
+/* Arches board does not have USB connectivity */
+/*
- Default environment variables
- */
+#define CONFIG_EXTRA_ENV_SETTINGS \
Ah... but these definitely do not belong into the "USB connectivity" block...
OK. Will fix.
*/ +#if !defined(CONFIG_ARCHES) #define CONFIG_CMD_DATE -#define CONFIG_CMD_DTT #define CONFIG_CMD_NAND +#define CONFIG_CMD_SNTP +#endif +#define CONFIG_CMD_DTT #define CONFIG_CMD_PCI #define CONFIG_CMD_SDRAM -#define CONFIG_CMD_SNTP #ifdef CONFIG_460EX #define CONFIG_CMD_EXT2 #define CONFIG_CMD_FAT
I think we should separate these tlistrs of commands, so we can keep the lists sorted.
OK.
+/*
- Arches has 32MBytes of NOR FLASH (Spansion 29GL256) so remap FLASH to
- EBC address which accepts bigger regions:
- 0xfe00.0000 -> 4.ce00.0000
- */
Cannot we do this automatically, dependent on the actual size of flash as determined by the code?
Will investigate.
Best Regards,
Victor Gallardo