
Dear Jens Scharsig,
In message 4AE6B186.9030301@bus-elektronik.de you wrote:
This patch adds a new ARM AT91RM9200 board, named EB+CPUx9K2.
- support for EB+CPUx9K2 board by BuS Elektronik GmbH & Co. KG
- select via make EB_CPUx9K2_config
Signed-off-by: Jens Scharsig esw@bus-elektronik.de
This patch needs the [PATCH] AT91RM9200 BGA port D defines (http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/50667)
Please use shorter lines.
...
diff --git a/board/BuS/EB+CPUx9K2/cpux9k2.c b/board/BuS/EB+CPUx9K2/cpux9k2.c new file mode 100644 index 0000000..6c29d91 --- /dev/null +++ b/board/BuS/EB+CPUx9K2/cpux9k2.c
...
+int board_init(void) +{
- /* Enable Ctrlc */
- console_init_f();
- /* Correct IRDA resistor problem / Set PA23_TXD in Output */
- ((AT91PS_PIO) AT91C_BASE_PIOA)->PIO_OER = AT91C_PA23_TXD2;
Please use I/O accessors to access device registers. Please fix globally.
+int misc_init_r(void) +{
...
if (tm) {
sprintf(str, "%02x:%02x:%02x:%02x:%02x:%02x",
mac[0], mac[1], mac[2],
mac[3], mac[4], mac[5]);
setenv("ethaddr", str);
Please don't reinvent the wheel. Use eth_setenv_enetaddr().
+/*
- DRAM initialisations
- */
+int dram_init(void) +{
- gd->bd->bi_dram[0].start = PHYS_SDRAM;
- gd->bd->bi_dram[0].size = PHYS_SDRAM_SIZE;
- return 0;
Please consider using get_ram_size() for auto-sizing the memor and perfoming at least a basic RAM test.
+unsigned int lxt972_IsPhyConnected(AT91PS_EMAC p_mac); +UCHAR lxt972_GetLinkSpeed(AT91PS_EMAC p_mac); +UCHAR lxt972_InitPhy(AT91PS_EMAC p_mac); +UCHAR lxt972_AutoNegotiate(AT91PS_EMAC p_mac, int *status);
Please don't use non-stanrard data types like UCHAR.
+void cpux9k2_nand_hw_init(void) +{
- /* Setup Smart Media, fitst enable the address range of CS3 */
- *AT91C_EBI_CSA |= AT91C_EBI_CS3A_SMC_SmartMedia;
- /* set the bus interface characteristics based on
tDS Data Set up Time 30 - ns
tDH Data Hold Time 20 - ns
tALS ALE Set up Time 20 - ns
16ns at 60 MHz ~= 3 */
- /*memory mapping structures */
+#define SM_ID_RWH (5 << 28) +#define SM_RWH (1 << 28) +#define SM_RWS (0 << 24) +#define SM_TDF (1 << 8) +#define SM_NWS (3)
Please don't add #defines right in the middle of the code. Move to header file, or at least to head of file.
And see comment above - use I/O accessors here and everywhere.
+#if defined(CONFIG_VIDEO) +/*
- ****h* EB+CPUx9K2/drv_video_init
- FUNCTION
- */
Incorrect multiline comment style.
+int drv_video_init(void) +{
- char *s;
- unsigned long splash;
- printf("Init Video as ");
- s = getenv("displaywidth");
- if (s != NULL)
display_width = simple_strtoul(s, NULL, 10);
- else
display_width = 256;
- s = getenv("displayheight");
- if (s != NULL)
display_height = simple_strtoul(s, NULL, 10);
- else
display_height = 256;
- printf("%ld x %ld pixel matrix\n", display_width, display_height);
- #define SM2_RWH (0x7 << 28)
- #define SM2_RWS (0x7 << 24)
- #define SM2_TDF (15 << 8)
- #define SM2_NWS (0x7F)
Please don't add #defines right in the middle of the code. Move to header file, or at least to head of file.
diff --git a/board/BuS/EB+CPUx9K2/u-boot.lds b/board/BuS/EB+CPUx9K2/u-boot.lds new file mode 100644 index 0000000..f4fbf96 --- /dev/null +++ b/board/BuS/EB+CPUx9K2/u-boot.lds
Do you really need a private linker script? Doesn't look so...
diff --git a/include/configs/EB_CPUx9K2.h b/include/configs/EB_CPUx9K2.h new file mode 100644 index 0000000..9ff743d --- /dev/null +++ b/include/configs/EB_CPUx9K2.h
...
+/*--------------------------------------------------------------------------*/
+#undef DEBUG
+/*--------------------------------------------------------------------------*/
Get rid of all this stuff. Don't undef what is not defined in the beginning, and don't meddle with command line options.
+#include <config_cmd_default.h>
+#define CONFIG_CMD_DHCP +#define CONFIG_CMD_PING +#define CONFIG_CMD_NAND +#define CONFIG_CMD_BMP +#define CONFIG_CMD_I2C +#define CONFIG_I2C_CMD_TREE +#define CONFIG_I2C_CMD_NO_FLAT +#define CONFIG_CMD_DATE +#define CONFIG_CMD_JFFS2
You may want to keep such lists sorted. And please use a consistent style - either always follow #define with a TAB, or (probably better) always with a space.
+#define CONFIG_BAUDRATE 115200 +#define CONFIG_AT91RM9200_USART +#define CONFIG_DBGU /* define DBGU as console */ +#undef CONFIG_USART0 +#undef CONFIG_USART1
+#undef CONFIG_HWFLOW +#undef CONFIG_MODEM_SUPPORT
Don;t undefine what is not defined.
+/*
- network
- */
+#if 0 +#define CONFIG_NET_MULTI 1 +#define CONFIG_DRIVER_AT91EMAC 1 +#define CONFIG_DRIVER_AT91EMAC_QUIET 1 +#define CONFIG_SYS_RX_ETH_BUFFER 8 +#undef CONFIG_RMII +#define CONFIG_MII 1 +#define CONFIG_CMD_MII 1
Don't add dead code.
+#define CONFIG_SYS_FLASH_ERASE_TOUT (6*CONFIG_SYS_HZ) +#define CONFIG_SYS_FLASH_WRITE_TOUT (2*CONFIG_SYS_HZ)
This looks wrong to me. A timeout is a time, but CONFIG_SYS_HZ is a frequency, i. e. the inverse of a time. These don't mix.
Best regards,
Wolfgang Denk