Dear Stefan Roese,
In message 1248440296-17129-1-git-send-email-sr@denx.de you wrote:
From: Reinhard Arlt reinhard.arlt@esd-electronics.com
This patch adds support for the esd VME8349 board equipped with the MPC8349. It's a VME PMC carrier board equipped with the Tundra TSI148 VME-bridge.
Signed-off-by: Reinhard Arlt reinhard.arlt@esd-electronics.com Signed-off-by: Stefan Roese sr@denx.de Cc: Kim Phillips kim.phillips@freescale.com
...
+int do_caddy(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +{
...
switch (caddy_cmd->cmd) {case CADDY_CMD_IO_READ_8:result[0] = in_8((void *)CONFIG_SYS_PCI1_IO_PHYS +(caddy_cmd->addr & 0x001fffff));generate_answer(caddy_cmd, status, &result[0]);break;case CADDY_CMD_IO_READ_16:result[0] = in_be16((void *)CONFIG_SYS_PCI1_IO_PHYS +(caddy_cmd->addr & 0x001fffff));generate_answer(caddy_cmd, status, &result[0]);
Can we get rid of these base address plus offset constructs? I would like to get rid of the casts which nullify one of the major benefits of using I/O acessors, i. e. having type checking done by the com- piler.
case CADDY_CMD_IO_READ_32:result[0] = in_be32((void *)CONFIG_SYS_PCI1_IO_PHYS +(caddy_cmd->addr & 0x001fffff));generate_answer(caddy_cmd, status, &result[0]);break;
Hm... and the code for the CADDY_CMD_IO_READ_*, CADDY_CMD_IO_WRITE_* etc. cases looks always the same, execpt for the access size. Maybe you want to use some macro? Thsi would make the code much easier to read and to maintain.
...
diff --git a/board/esd/vme8349/vme8349.c b/board/esd/vme8349/vme8349.c new file mode 100644 index 0000000..db24a7c --- /dev/null +++ b/board/esd/vme8349/vme8349.c @@ -0,0 +1,139 @@
...
- im->ddr.sdram_cfg = 0x63000000;
- im->ddr.sdram_cfg2 = 0x04061000;
- im->ddr.sdram_mode = 0x07940242;
- im->ddr.sdram_mode2 = 0x00000000;
Maybe you want to use some nice readable constants here instead of hard-coded magic numbers?
diff --git a/include/configs/vme8349.h b/include/configs/vme8349.h new file mode 100644 index 0000000..cde98ae --- /dev/null +++ b/include/configs/vme8349.h
...
+#define CONFIG_SYS_DDR_TIMING_0 0x00220802 +#define CONFIG_SYS_DDR_TIMING_1 0x39377322 +#define CONFIG_SYS_DDR_TIMING_2 0x2f9848ca /* P9-45,may need tuning */ +#define CONFIG_SYS_DDR_TIMING_3 0x00000000 +#define CONFIG_SYS_DDR_CONTROL 0xc2000000 /* unbuffered,no DYN_PWR */ +#define CONFIG_SYS_DDR_INTERVAL 0x04060100 /* autocharge,no open page */
+/* the default burst length is 4 - for 64-bit data path */ +#define CONFIG_SYS_DDR_MODE 0x00000022 /* DLL,normal,seq,4/2.5, 4 burst len */
Line too long. Please check globally.
+#if 1 /*528/264*/ +#define CONFIG_SYS_HRCW_LOW (\
- HRCWL_LCL_BUS_TO_SCB_CLK_1X1 |\
- HRCWL_DDR_TO_SCB_CLK_1X1 |\
- HRCWL_CSB_TO_CLKIN |\
- HRCWL_VCO_1X2 |\
- HRCWL_CORE_TO_CSB_2X1)
+#elif 0 /*396/132*/ +#define CONFIG_SYS_HRCW_LOW (\
- HRCWL_LCL_BUS_TO_SCB_CLK_1X1 |\
- HRCWL_DDR_TO_SCB_CLK_1X1 |\
- HRCWL_CSB_TO_CLKIN |\
- HRCWL_VCO_1X4 |\
- HRCWL_CORE_TO_CSB_3X1)
+#elif 0 /*264/132*/ +#define CONFIG_SYS_HRCW_LOW (\
- HRCWL_LCL_BUS_TO_SCB_CLK_1X1 |\
- HRCWL_DDR_TO_SCB_CLK_1X1 |\
- HRCWL_CSB_TO_CLKIN |\
- HRCWL_VCO_1X4 |\
- HRCWL_CORE_TO_CSB_2X1)
+#elif 0 /*132/132*/ +#define CONFIG_SYS_HRCW_LOW (\
- HRCWL_LCL_BUS_TO_SCB_CLK_1X1 |\
- HRCWL_DDR_TO_SCB_CLK_1X1 |\
- HRCWL_CSB_TO_CLKIN |\
- HRCWL_VCO_1X4 |\
- HRCWL_CORE_TO_CSB_1X1)
+#elif 0 /*264/264 */ +#define CONFIG_SYS_HRCW_LOW (\
- HRCWL_LCL_BUS_TO_SCB_CLK_1X1 |\
- HRCWL_DDR_TO_SCB_CLK_1X1 |\
- HRCWL_CSB_TO_CLKIN |\
- HRCWL_VCO_1X4 |\
- HRCWL_CORE_TO_CSB_1X1)
+#endif
There's a lot of dead code here. Please remove (or move to documentation).
+#define CONFIG_SERVERIP 192.168.1.1 +#define CONFIG_GATEWAYIP 192.168.1.1 +#define CONFIG_NETMASK 255.255.255.0
Please do not add network configuration data into the default board config file.
Best regards,
Wolfgang Denk