Re: [U-Boot] Subject: [PATCH v3] mx27ads: add support for iMX27ADS board from Freescale

2009-09-22 Thread Alan Carvalho de Assis 
Hi Fred,

Thank you for your fast reply, please see my comments below:

On 9/21/09, Fred Fan fanyef...@gmail.com wrote:
 Dear Alan Carvalho de Assis,

 2009/9/16 Alan Carvalho de Assis acas...@gmail.com

 +   /*
 +* DDR on CSD0
 +*/
 +   write32 0xD8001010, 0x0008
 +   write32 0x10027828, 0x
 +   write32 0x10027830, 0x
 +   write32 0x10027834, 0x
 +   write32 0x10027838, 0x5005
 +   write32 0x1002783C, 0x1555
 +   write32 0xD8001010, 0x0004
 +   write32 0xD8001004, 0x006ac73a
 +   write32 0xD8001000, 0x9210
 +   write32 0xAF00, 0x
 +   write32 0xD8001000, 0xA210
 +   write32 0xAF00, 0x
 +   write32 0xAF00, 0x
 +   write32 0xAF00, 0x
 +   write32 0xAF00, 0x
 +   write32 0xD8001000, 0xA220
 +   write32 0xAF00, 0x
 +   write32 0xAF00, 0x
 +   write32 0xAF00, 0x
 +   write32 0xAF00, 0x
 +   write32 0xD8001000, 0xb210

 +   ldr r0, =0xA033
 +   mov r1, #0xda
 +   strbr1, [r0]
 +   ldr r0, =0xA100
 +   mov r1, #0xff
 +   strbr1, [r0]
 +   write32 0xD8001000, 0x82226080

 Please use the friendly register definition.

I based on U-Boot-v2 start-up code.
Where should I place these registers definition? (could be in
include/asm-arm/arch-mx27/imx-regs.h ?)

 +*
 +* FIXME: Using the 399*2 MHz values from table 3-8 doens't work
 +*with 1.2 V core voltage! Find out if this is
 +*documented somewhere.
 +*/
 +   write32 MPCTL0, 0x1EF15D5   /* MPLL = 199.5*2 MHz
 */
 +   write32 SPCTL0, 0x043A1C09  /* SPLL = FIXME (needs review)
  */

  I will check the setting in another mail.


Sure, I will wait for it.

 +   /* clock gating enable */
 +   write32 0x10027818, 0x00050f08
 +

 ditto


ACK

 +
 +   /* skip sdram initialization if we run from ram */
 +   cmp pc, #0xa000
 +   bls 1f
 +   cmp pc, #0xc000
 +   bhi 1f

 Please use the definitions of ddr area.


Ok

 +int board_init(void)
 +{
 +   struct gpio_regs *regs = (struct gpio_regs *)IMX_GPIO_BASE;
 +
 +   gd-bd-bi_arch_number = MACH_TYPE_MX27ADS;
 +   gd-bd-bi_boot_params = PHYS_SDRAM_1 + 0x100;

 move bi_boot_params to dram_init.


Ok

 +/*
 + * Ethernet
 + */
 +#define CONFIG_FEC_MXC
 +#define CONFIG_FEC_MXC_PHYADDR 0x1f
 +#define CONFIG_MII
 +#define CONFIG_NET_MULTI
 +
 +/*#define CONFIG_DRIVER_CS89001
 +#define CS8900_BASE 0xD4020300
 +#define CS8900_BUS161*/

 Remove commented code.


I will active this commented lines, I think it just need another
ETHADDR to work.

 +#define CONFIG_ETHADDR 02:80:ad:20:31:e8

Thank you Fred, I am glad to see more Freescale guys helping
open-source projects.

Best Regards,

Alan
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] Subject: [PATCH v3] mx27ads: add support for iMX27ADS board from Freescale

2009-09-22 Thread Wolfgang Denk 
Dear Alan Carvalho de Assis,

In message 37367b3a0909151429h317066ax2efa504d83dbf...@mail.gmail.com you 
wrote:
 This patch adds support to iMX27ADS development board. This board has
 128MB RAM, 32MB NOR Flash and 128MB NAND Flash. Currently only
 booting from NOR is supported.
 
 Signed-off-by: Alan Carvalho de Assis acas...@gmail.com

I will try and mention only the issues not yet covered.

...
 --- a/Makefile
 +++ b/Makefile
 @@ -2965,6 +2965,9 @@ davinci_dm365evm_config :   unconfig
  imx27lite_config:unconfig
   @$(MKCONFIG) $(@:_config=) arm arm926ejs imx27lite logicpd mx27
 
 +mx27ads_config : unconfig
 + @$(MKCONFIG) $(@:_config=) arm arm926ejs mx27ads freescale mx27
 +
  lpd7a400_config \
  lpd7a404_config: unconfig

Please keep list sorted.

 diff --git a/board/freescale/mx27ads/Makefile 
 b/board/freescale/mx27ads/Makefile
 new file mode 100644
 index 000..d142a9e
 --- /dev/null
 +++ b/board/freescale/mx27ads/Makefile
...
 +#
 +

Please don;t add trailing empty lines.

 diff --git a/board/freescale/mx27ads/config.mk
 b/board/freescale/mx27ads/config.mk
 new file mode 100644
 index 000..a2e7768
 --- /dev/null
 +++ b/board/freescale/mx27ads/config.mk
...
 +/*
 + * For clock initialization, see chapter 3 of the MCIMX27 Multimedia
 + * Applications Processor Reference Manual, Rev. 0.2.
 + *
 + */

Please don't add random empty comment lines.

...
 + write32 0xAF00, 0x
 + write32 0xD8001000, 0xb210
 + ldr r0, =0xA033
 + mov r1, #0xda
 + strbr1, [r0]
 + ldr r0, =0xA100
 + mov r1, #0xff
 + strbr1, [r0]
 + write32 0xD8001000, 0x82226080
...
 + mov r10, lr
...
 + ldr r0, =CSCR
 + ldr r1, [r0]
 + bic r1, r1, #0x03
 + str r1, [r0]

Please use a consistent style for indentation. I suggest you use the
insn name followed by a single TAB character followed by args.


 +++ b/board/freescale/mx27ads/mx27ads.c
 @@ -0,0 +1,93 @@
...
...
 +# define __REG(x)  (*((volatile u32 *)(x)))
 +
 +#define IMX_CS4_BASE   0xD400
 +#define CS4U __REG(IMX_WEIM_BASE + 0x40) /* Chip Select 4 Upper Register
 */
 +#define CS4L __REG(IMX_WEIM_BASE + 0x44) /* Chip Select 4 Lower Register
 */
 +#define CS4A __REG(IMX_WEIM_BASE + 0x48) /* Chip Select 4 Addition Register 
 */

Please use proper I/O accessors. Direct register accesses like here
are forbidden. Also, you probably want to define a C structure
describing the register map.

...
 + /* Configure CPLD on CS4 */
 + CS4U = 0xDCF6;
 + CS4L = 0x444A4541;
 + CS4A = 0x3302;

Please never use suchmagic numbers in the code. Provide some #defines
for these, together with sufficient comments to explain what the
numbers mean.

 + /* Select FEC data through data path */
 + writew(0x0020, IMX_CS4_BASE + 0x10);
 +
 + /* Enable CPLD FEC data path */
 + writew(0x0010, IMX_CS4_BASE + 0x14);

Please use C structs for the register mapping; do not use base address
plus offsets.

 +int dram_init(void)
 +{
 +

No empty line here.

 +#if CONFIG_NR_DRAM_BANKS  0
 + gd-bd-bi_dram[0].start = PHYS_SDRAM_1;
 + gd-bd-bi_dram[0].size = get_ram_size((volatile void *)PHYS_SDRAM_1,
 + PHYS_SDRAM_1_SIZE);
 +#endif

Um... is CONFIG_NR_DRAM_BANKS = 0 any configuration that is supposed
to work? 

 +#if CONFIG_NR_DRAM_BANKS  1
 + gd-bd-bi_dram[1].start = PHYS_SDRAM_2;
 + gd-bd-bi_dram[1].size = get_ram_size((volatile void *)PHYS_SDRAM_2,
 + PHYS_SDRAM_2_SIZE);
 +#endif
 +
 + return 0;

You unconditionally return OK even in case of memory errors? This
seems wrong to me.

 diff --git a/board/freescale/mx27ads/u-boot.lds
 b/board/freescale/mx27ads/u-boot.lds
 new file mode 100644
 index 000..f66f20e
 --- /dev/null
 +++ b/board/freescale/mx27ads/u-boot.lds

Does this board need a private linke rscript? Why cannot you use the
common script in cpu/arm926ejs/u-boot.lds ?

 diff --git a/include/configs/mx27ads.h b/include/configs/mx27ads.h
 new file mode 100644
 index 000..3360d76
 --- /dev/null
 +++ b/include/configs/mx27ads.h
...
 +/* memtest start address */
 +#define CONFIG_SYS_MEMTEST_START 0xA000
 +#define CONFIG_SYS_MEMTEST_END   0xA100  /* 16MB RAM 
 test */

Did you ever test this? Overwriting low memory where exception vectors
and such is located is probably a bad idea.

...
 +/* Use hardware sector protection */
 +#define CONFIG_SYS_FLASH_PROTECTION  1
 +#define CONFIG_SYS_MAX_FLASH_BANKS   1   /* max number of flash banks */
 +#define CONFIG_SYS_FLASH_SECT_SZ 0x2000  /* 8KB sect size Intel Flash */
 +/* end of flash */
 +#define CONFIG_ENV_OFFSET(PHYS_FLASH_SIZE - 0x2)
 +/* CS2 Base address */
 +#define PHYS_FLASH_1 0xc000
 +/* Flash Base for U-Boot */

Re: [U-Boot] Subject: [PATCH v3] mx27ads: add support for iMX27ADS board from Freescale

2009-09-21 Thread Fred Fan 
Dear Alan Carvalho de Assis,

2009/9/16 Alan Carvalho de Assis acas...@gmail.com

 This patch adds support to iMX27ADS development board. This board has
 128MB RAM, 32MB NOR Flash and 128MB NAND Flash. Currently only
 booting from NOR is supported.

 Signed-off-by: Alan Carvalho de Assis acas...@gmail.com
 ---
  MAINTAINERS |3 +
  MAKEALL |1 +
  Makefile|3 +
  board/freescale/mx27ads/Makefile|   51 
  board/freescale/mx27ads/config.mk   |1 +
  board/freescale/mx27ads/lowlevel_init.S |  127 +++
  board/freescale/mx27ads/mx27ads.c   |   93 ++
  board/freescale/mx27ads/u-boot.lds  |   56 +
  include/configs/mx27ads.h   |  202
 +++
  9 files changed, 537 insertions(+), 0 deletions(-)
  create mode 100644 board/freescale/mx27ads/Makefile
  create mode 100644 board/freescale/mx27ads/config.mk
  create mode 100644 board/freescale/mx27ads/lowlevel_init.S
  create mode 100644 board/freescale/mx27ads/mx27ads.c
  create mode 100644 board/freescale/mx27ads/u-boot.lds
  create mode 100644 include/configs/mx27ads.h

 diff --git a/MAINTAINERS b/MAINTAINERS
 index e9db278..7ff3160 100644
 --- a/MAINTAINERS
 +++ b/MAINTAINERS
 @@ -512,6 +512,9 @@ Unknown / orphaned boards:
  #  Board   CPU #
  #

 +Alan Carvalho de Assis acas...@gmail.com
 +   mx27ads i.MX27
 +
  Rowel Atienza ro...@diwalabs.com

armadillo   ARM720T
 diff --git a/MAKEALL b/MAKEALL
 index f0ed8ea..56b4446 100755
 --- a/MAKEALL
 +++ b/MAKEALL
 @@ -525,6 +525,7 @@ LIST_ARM9= \
mv88f6281gtw_ge \
mx1ads  \
mx1fs2  \
 +   mx27ads \
netstar \
nhk8815 \
nhk8815_onenand \
 diff --git a/Makefile b/Makefile
 index 9764cea..4f2dcef 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -2965,6 +2965,9 @@ davinci_dm365evm_config : unconfig
  imx27lite_config:  unconfig
@$(MKCONFIG) $(@:_config=) arm arm926ejs imx27lite logicpd mx27

 +mx27ads_config :   unconfig
 +   @$(MKCONFIG) $(@:_config=) arm arm926ejs mx27ads freescale mx27
 +
  lpd7a400_config \
  lpd7a404_config:   unconfig
@$(MKCONFIG) $(@:_config=) arm lh7a40x lpd7a40x
 diff --git a/board/freescale/mx27ads/Makefile
 b/board/freescale/mx27ads/Makefile
 new file mode 100644
 index 000..d142a9e
 --- /dev/null
 +++ b/board/freescale/mx27ads/Makefile
 @@ -0,0 +1,51 @@
 +#
 +# (C) Copyright 2000-2004
 +# Wolfgang Denk, DENX Software Engineering, w...@denx.de.
 +#
 +# See file CREDITS for list of people who contributed to this
 +# project.
 +#
 +# This program is free software; you can redistribute it and/or
 +# modify it under the terms of the GNU General Public License as
 +# published by the Free Software Foundation; either version 2 of
 +# the License, or (at your option) any later version.
 +#
 +# This program is distributed in the hope that it will be useful,
 +# but WITHOUT ANY WARRANTY; without even the implied warranty of
 +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 +# GNU General Public License for more details.
 +#
 +# You should have received a copy of the GNU General Public License
 +# along with this program; if not, write to the Free Software
 +# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
 +# MA 02111-1307 USA
 +#
 +
 +include $(TOPDIR)/config.mk
 +
 +LIB= $(obj)lib$(BOARD).a
 +
 +COBJS  := mx27ads.o
 +SOBJS  := lowlevel_init.o
 +
 +SRCS   := $(SOBJS:.o=.S) $(COBJS:.o=.c)
 +OBJS   := $(addprefix $(obj),$(COBJS))
 +SOBJS  := $(addprefix $(obj),$(SOBJS))
 +
 +$(LIB):$(obj).depend $(OBJS) $(SOBJS)
 +   $(AR) $(ARFLAGS) $@ $(OBJS) $(SOBJS)
 +
 +clean:
 +   rm -f $(SOBJS) $(OBJS)
 +
 +distclean: clean
 +   rm -f $(LIB) core *.bak $(obj).depend
 +
 +#
 +
 +include $(SRCTREE)/rules.mk
 +
 +sinclude $(obj).depend
 +
 +#
 +
 diff --git a/board/freescale/mx27ads/config.mk
 b/board/freescale/mx27ads/config.mk
 new file mode 100644
 index 000..a2e7768
 --- /dev/null
 +++ b/board/freescale/mx27ads/config.mk
 @@ -0,0 +1 @@
 +TEXT_BASE = 0xA7F0
 diff --git a/board/freescale/mx27ads/lowlevel_init.S
 b/board/freescale/mx27ads/lowlevel_init.S
 new file mode 100644
 index 000..dc62a93
 --- /dev/null
 +++ b/board/freescale/mx27ads/lowlevel_init.S
 @@ -0,0 +1,127 @@
 +/*
 + * Copyright (C) 2008, Guennadi Liakhovetski l...@denx.de
 + *
 + * This program is free software; you can redistribute it and/or
 + * modify it under the terms of the GNU General Public License as

[U-Boot] Subject: [PATCH v3] mx27ads: add support for iMX27ADS board from Freescale

2009-09-15 Thread Alan Carvalho de Assis 
This patch adds support to iMX27ADS development board. This board has
128MB RAM, 32MB NOR Flash and 128MB NAND Flash. Currently only
booting from NOR is supported.

Signed-off-by: Alan Carvalho de Assis acas...@gmail.com
---
 MAINTAINERS |3 +
 MAKEALL |1 +
 Makefile|3 +
 board/freescale/mx27ads/Makefile|   51 
 board/freescale/mx27ads/config.mk   |1 +
 board/freescale/mx27ads/lowlevel_init.S |  127 +++
 board/freescale/mx27ads/mx27ads.c   |   93 ++
 board/freescale/mx27ads/u-boot.lds  |   56 +
 include/configs/mx27ads.h   |  202 +++
 9 files changed, 537 insertions(+), 0 deletions(-)
 create mode 100644 board/freescale/mx27ads/Makefile
 create mode 100644 board/freescale/mx27ads/config.mk
 create mode 100644 board/freescale/mx27ads/lowlevel_init.S
 create mode 100644 board/freescale/mx27ads/mx27ads.c
 create mode 100644 board/freescale/mx27ads/u-boot.lds
 create mode 100644 include/configs/mx27ads.h

diff --git a/MAINTAINERS b/MAINTAINERS
index e9db278..7ff3160 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -512,6 +512,9 @@ Unknown / orphaned boards:
 #  Board   CPU #
 #

+Alan Carvalho de Assis acas...@gmail.com
+   mx27ads i.MX27
+
 Rowel Atienza ro...@diwalabs.com

armadillo   ARM720T
diff --git a/MAKEALL b/MAKEALL
index f0ed8ea..56b4446 100755
--- a/MAKEALL
+++ b/MAKEALL
@@ -525,6 +525,7 @@ LIST_ARM9= \
mv88f6281gtw_ge \
mx1ads  \
mx1fs2  \
+   mx27ads \
netstar \
nhk8815 \
nhk8815_onenand \
diff --git a/Makefile b/Makefile
index 9764cea..4f2dcef 100644
--- a/Makefile
+++ b/Makefile
@@ -2965,6 +2965,9 @@ davinci_dm365evm_config : unconfig
 imx27lite_config:  unconfig
@$(MKCONFIG) $(@:_config=) arm arm926ejs imx27lite logicpd mx27

+mx27ads_config :   unconfig
+   @$(MKCONFIG) $(@:_config=) arm arm926ejs mx27ads freescale mx27
+
 lpd7a400_config \
 lpd7a404_config:   unconfig
@$(MKCONFIG) $(@:_config=) arm lh7a40x lpd7a40x
diff --git a/board/freescale/mx27ads/Makefile b/board/freescale/mx27ads/Makefile
new file mode 100644
index 000..d142a9e
--- /dev/null
+++ b/board/freescale/mx27ads/Makefile
@@ -0,0 +1,51 @@
+#
+# (C) Copyright 2000-2004
+# Wolfgang Denk, DENX Software Engineering, w...@denx.de.
+#
+# See file CREDITS for list of people who contributed to this
+# project.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+# MA 02111-1307 USA
+#
+
+include $(TOPDIR)/config.mk
+
+LIB= $(obj)lib$(BOARD).a
+
+COBJS  := mx27ads.o
+SOBJS  := lowlevel_init.o
+
+SRCS   := $(SOBJS:.o=.S) $(COBJS:.o=.c)
+OBJS   := $(addprefix $(obj),$(COBJS))
+SOBJS  := $(addprefix $(obj),$(SOBJS))
+
+$(LIB):$(obj).depend $(OBJS) $(SOBJS)
+   $(AR) $(ARFLAGS) $@ $(OBJS) $(SOBJS)
+
+clean:
+   rm -f $(SOBJS) $(OBJS)
+
+distclean: clean
+   rm -f $(LIB) core *.bak $(obj).depend
+
+#
+
+include $(SRCTREE)/rules.mk
+
+sinclude $(obj).depend
+
+#
+
diff --git a/board/freescale/mx27ads/config.mk
b/board/freescale/mx27ads/config.mk
new file mode 100644
index 000..a2e7768
--- /dev/null
+++ b/board/freescale/mx27ads/config.mk
@@ -0,0 +1 @@
+TEXT_BASE = 0xA7F0
diff --git a/board/freescale/mx27ads/lowlevel_init.S
b/board/freescale/mx27ads/lowlevel_init.S
new file mode 100644
index 000..dc62a93
--- /dev/null
+++ b/board/freescale/mx27ads/lowlevel_init.S
@@ -0,0 +1,127 @@
+/*
+ * Copyright (C) 2008, Guennadi Liakhovetski l...@denx.de
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but