Dear Ilya Yanok,
In message <1233589490-14293-4-git-send-email-ya...@emcraft.com> you wrote:
> This patch adds support for Dave/DENX QongEVB-LITE i.MX31-based board.
>
> Signed-off-by: Ilya Yanok <ya...@emcraft.com>
...
> board/dave/qong/Makefile | 51 +++++++++
> board/dave/qong/config.mk | 1 +
> board/dave/qong/lowlevel_init.S | 170 +++++++++++++++++++++++++++++++
> board/dave/qong/qong.c | 167 ++++++++++++++++++++++++++++++
> board/dave/qong/qong_fpga.h | 41 ++++++++
> board/dave/qong/u-boot.lds | 59 +++++++++++
The vendor name should be "davedenx", thus the directory name should
be board/davedenx/qong/, please.
> diff --git a/board/dave/qong/qong.c b/board/dave/qong/qong.c
> new file mode 100644
> index 0000000..5d12a13
> --- /dev/null
> +++ b/board/dave/qong/qong.c
> @@ -0,0 +1,167 @@
...
> +int dram_init (void)
> +{
> + gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
> + gd->bd->bi_dram[0].size = PHYS_SDRAM_1_SIZE;
> +
> + return 0;
> +}
Why do we not auto-size the RAM here like U-Boot is supposed to do?
> diff --git a/board/dave/qong/u-boot.lds b/board/dave/qong/u-boot.lds
> new file mode 100644
> index 0000000..1460adc
> --- /dev/null
> +++ b/board/dave/qong/u-boot.lds
> @@ -0,0 +1,59 @@
> +/*
> + * January 2004 - Changed to support H4 device
> + * Copyright (c) 2004 Texas Instruments
Are you sure this is an appropriate comment for this file?
> diff --git a/include/configs/qong.h b/include/configs/qong.h
> new file mode 100644
> index 0000000..8fae342
> --- /dev/null
> +++ b/include/configs/qong.h
...
> +/*
> + * 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 necessary on this hardware?
> +/* allow to overwrite serial and ethaddr */
> +#define CONFIG_ENV_OVERWRITE
Please don't.
> +/***********************************************************
> + * Command definition
> + ***********************************************************/
> +
> +#include <config_cmd_default.h>
> +
> +#define CONFIG_CMD_PING
> +#define CONFIG_CMD_DHCP
> +#define CONFIG_CMD_NET
> +#define CONFIG_CMD_MII
> +
> +#define CONFIG_ETHADDR 00:50:c2:1e:af:e7
> +#define CONFIG_NETMASK 255.255.0.0
> +#define CONFIG_IPADDR 192.168.0.81
> +#define CONFIG_SERVERIP 192.168.1.1
> +#define CONFIG_GATEWAYIP 192.168.1.1
Please never, never ever hardcode any network configuration into
U-Boot. Delete this, please.
And please add image timestamp support.
> +#define CONFIG_BOOTDELAY 3
Make this standard 5 seconds, please.
> +#define CONFIG_EXTRA_ENV_SETTINGS
> \
> + "netdev=eth0\0" \
> + "nfsargs=setenv bootargs root=/dev/nfs rw " \
> + "nfsroot=${serverip}:${rootpath}\0" \
> + "ramargs=setenv bootargs root=/dev/ram rw\0" \
> + "addip=setenv bootargs ${bootargs} " \
> + "ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}" \
> + ":${hostname}:${netdev}:off panic=1\0" \
> + "addtty=setenv bootargs ${bootargs}" \
> + " console=ttymxc0,${baudrate}\0" \
> + "addmisc=setenv bootargs ${bootargs}\0" \
> + "uboot_addr=0xa0000000\0" \
--------------------^^
> + "uboot=qong/u-boot.bin\0" \
> + "kernel_addr_r=0x80800000\0" \
-----------------------^^
No need for 0x prefix.
> + "hostname=qong\0" \
> + "bootfile=qong/uImage\0" \
> + "rootpath=/opt/eldk-4.2-arm/armVFP\0" \
> + "flash_self=run ramargs addip addtty addmisc;" \
> + "bootm ${kernel_addr} ${ramdisk_addr}\0" \
> + "flash_nfs=run nfsargs addip addtty addmisc;" \
> + "bootm ${kernel_addr}\0" \
> + "net_nfs=tftp ${kernel_addr_r} ${bootfile};" \
> + "run nfsargs addip addtty addmisc;" \
> + "bootm ${kernel_addr_r}\0" \
Pleain "bootm" would do as well.
> + "load=tftp ${loadaddr} ${uboot}\0" \
> + "update=protect off " xstr(CONFIG_SYS_MONITOR_BASE) " +" \
> + xstr(CONFIG_SYS_MONITOR_LEN)";" \
----------------^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> + "era " xstr(CONFIG_SYS_MONITOR_BASE) " +" \
> + xstr(CONFIG_SYS_MONITOR_LEN)";" \
----------------^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Please use "+${filesize}" instead.
> +#define CONFIG_SYS_PBSIZE (CONFIG_SYS_CBSIZE + \
> + sizeof(CONFIG_SYS_PROMPT) + 16)
> +#define CONFIG_SYS_MAXARGS 16 /* max number of command args */
32 ?
> +#define CONFIG_SYS_MEMTEST_START 0 /* memtest works on */
> +#define CONFIG_SYS_MEMTEST_END 0x10000
Hm... tha RAM test operates on the first 64 kB of flash memory? Or am
I missing something?
> +#define CONFIG_ENV_IS_IN_FLASH 1
> +#define CONFIG_ENV_SECT_SIZE SZ_128K
Please do not use any of these funny "SZ_" definitions. These shall be
removed ASAP.
> +#define CONFIG_ENV_SIZE CONFIG_ENV_SECT_SIZE
> +#define CONFIG_ENV_ADDR (CONFIG_SYS_FLASH_BASE+(2*SZ_128K))
Ditto.
> +/*
> + * JFFS2 partitions
> + */
> +#undef CONFIG_JFFS2_CMDLINE
Why?
Best regards,
Wolfgang Denk
--
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: w...@denx.de
Of all the things I've lost, I miss my mind the most.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
