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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
