[U-Boot] [PATCH 2/2] Add board support for hawkboard

Sughosh Ganu urwithsughosh at gmail.com
Fri Oct 22 09:50:16 CEST 2010


hi Wolfgang,
On Thu Oct 21, 2010 at 10:44:35PM +0200, Wolfgang Denk wrote:
> Dear Sughosh Ganu,
> 
> In message <1287690234-6109-1-git-send-email-urwithsughosh at gmail.com> you wrote:

<snip>

> > Signed-off-by: Sughosh Ganu <urwithsughosh at gmail.com>
> > ---
> >  Makefile                                     |   17 +++
> >  arch/arm/cpu/arm926ejs/davinci/cpu.c         |    2 +
> >  arch/arm/include/asm/arch-davinci/hardware.h |    5 +-
> >  board/davinci/common/misc.c                  |    2 +
> >  board/davinci/da8xxevm/Makefile              |    1 +
> >  board/davinci/da8xxevm/config.mk             |    3 +
> >  board/davinci/da8xxevm/hawkboard.c           |  201 ++++++++++++++++++++++++++
> >  drivers/mtd/nand/davinci_nand.c              |    1 +
> >  include/configs/hawkboard.h                  |  200 +++++++++++++++++++++++++
> >  nand_spl/board/davinci/da8xxevm/Makefile     |  141 ++++++++++++++++++
> >  nand_spl/board/davinci/da8xxevm/config.mk    |   40 +++++
> >  nand_spl/board/davinci/da8xxevm/u-boot.lds   |   75 ++++++++++
> >  nand_spl/nand_boot.c                         |    3 +-
> >  13 files changed, 689 insertions(+), 2 deletions(-)
> >  create mode 100644 board/davinci/da8xxevm/hawkboard.c
> >  create mode 100644 include/configs/hawkboard.h
> >  create mode 100644 nand_spl/board/davinci/da8xxevm/Makefile
> >  create mode 100644 nand_spl/board/davinci/da8xxevm/config.mk
> >  create mode 100644 nand_spl/board/davinci/da8xxevm/u-boot.lds
> 
> Entries to MAINTAINERS and boards.cfg missing.

  Will add.

> 
> > diff --git a/Makefile b/Makefile
> > index 06c71a2..7b8152c 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -911,6 +911,23 @@ cp922_XA10_config	\
> >  cp1026_config: unconfig
> >  	@board/armltd/integrator/split_by_variant.sh cp $@
> >  
> > +hawkboard_config \
> > +hawkboard_uart_config \
> > +hawkboard_nand_config	: unconfig
> > +	@mkdir -p $(obj)include
> > +	@mkdir -p $(obj)board/davinci/da8xxevm
> > +	@if [ -n "$(findstring _nand_,$@)" ]; then							\
> > +		echo "#define CONFIG_NAND_U_BOOT" >> $(obj)include/config.h;				\
> > +		echo "CONFIG_NAND_U_BOOT = y" >> $(obj)include/config.mk;				\
> > +		echo "CONFIG_SYS_TEXT_BASE = 0xc1080000" >$(obj)board/davinci/da8xxevm/config.tmp;	\
> > +	elif [ -n "$(findstring _uart_,$@)" ]; then							\
> > +		echo "CONFIG_SYS_TEXT_BASE = 0xc1080000" >$(obj)board/davinci/da8xxevm/config.tmp;	\
> > +	else 												\
> > +		echo "CONFIG_SYS_TEXT_BASE = 0xc1180000" >$(obj)board/davinci/da8xxevm/config.tmp;	\
> > +	fi
> 
> We don;t allow and don;t need any such scripting in the Makefile any
> more. Please add your board to boards.cfg instead..

  I had tried adding the entries in the boards.cfg file but could not
  get nand_spl to build. Will check this out and get back.


> > diff --git a/arch/arm/cpu/arm926ejs/davinci/cpu.c b/arch/arm/cpu/arm926ejs/davinci/cpu.c
> > index fc3551c..b4a7382 100644
> > --- a/arch/arm/cpu/arm926ejs/davinci/cpu.c
> > +++ b/arch/arm/cpu/arm926ejs/davinci/cpu.c
> > @@ -109,6 +109,7 @@ out:
> >  }
> >  #endif /* CONFIG_SOC_DA8XX */
> >  
> > +#if !defined(CONFIG_NAND_SPL)
> >  #ifdef CONFIG_DISPLAY_CPUINFO
> >  
> >  static unsigned pll_div(volatile void *pllbase, unsigned offset)
> > @@ -189,3 +190,4 @@ int cpu_eth_init(bd_t *bis)
> >  #endif
> >  	return 0;
> >  }
> > +#endif /* !CONFIG_NAND_SPL */
> 
> Why exactly is this change needed?  What happens if you just do not
> define CONFIG_DISPLAY_CPUINFO in your NAND config?

  I was getting an error with the davinci_emac_initialize function. I
  guess i can also not define CONFIG_DRIVER_TI_EMAC for the nand spl
  case and get rid of the check completely.


> > index 28750b1..546fd93 100644
> > --- a/board/davinci/common/misc.c
> > +++ b/board/davinci/common/misc.c
> > @@ -33,6 +33,7 @@
> >  
> >  DECLARE_GLOBAL_DATA_PTR;
> >  
> > +#if !defined(CONFIG_NAND_SPL)
> >  #if defined(CONFIG_SYS_ARM_WITHOUT_RELOC)
> >  int dram_init(void)
> >  {
> > @@ -105,6 +106,7 @@ void davinci_sync_env_enetaddr(uint8_t *rom_enetaddr)
> >  }
> >  
> >  #endif	/* DAVINCI_EMAC */
> > +#endif	/* !CONFIG_NAND_SPL */
> 
> Is there not a better way than adding such a huge #ifdef block?

  Not sure about this one. I need the pinmux configuration functions
  from this file. 

> 
> > index e176f7d..9fc7ad9 100644
> > --- a/board/davinci/da8xxevm/config.mk
> > +++ b/board/davinci/da8xxevm/config.mk
> > @@ -38,6 +38,9 @@
> >  #
> >  # we load ourself to C108 '0000
> >  
> > +sinclude $(OBJTREE)/board/$(BOARDDIR)/config.tmp
> >  
> >  #Provide at least 16MB spacing between us and the Linux Kernel image
> > +ifndef CONFIG_SYS_TEXT_BASE
> >  CONFIG_SYS_TEXT_BASE = 0xC1080000
> > +endif
> 
> No. We don't need config.mk files for this any more. Please move
> settings to board config file.

  This is currently being used by da830evm and da850evm too. If ok, i
  can cleanup for those files too, although i cannot test it on these
  boards.

> 
> > diff --git a/board/davinci/da8xxevm/hawkboard.c b/board/davinci/da8xxevm/hawkboard.c
> > new file mode 100644
> ....
> > +#if !defined(CONFIG_NAND_SPL)
> > +int board_init(void)
> 
> Again: can we not avoid such a huge #ifdef block here?


  We need this initialisation for the nand spl part alone, and this is
  board specific. Can you please suggest any better way of handling
  this.

> 
> > +int misc_init_r (void)
> > +{
> > +	printf ("ARM Clock : %d Hz\n", clk_get(DAVINCI_ARM_CLKID));
> 
> Would it make sense to use strmhz() here?


  Will check on this.

> 
> > diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
> > index d41579c..ad70dd9 100644
> > --- a/drivers/mtd/nand/davinci_nand.c
> > +++ b/drivers/mtd/nand/davinci_nand.c
> > @@ -635,6 +635,7 @@ void davinci_nand_init(struct nand_chip *nand)
> >  	nand->write_buf = nand_davinci_write_buf;
> >  
> >  	nand->dev_ready = nand_davinci_dev_ready;
> > +	nand->select_chip = NULL;
> 
> Hm... maybe we should rather use memset() to set the whole struct
> nand_chip to zero?


  Will initialise the nand chip structure.

> 
> ...
> > +#define CONFIG_SYS_NAND_ECCBYTES	10
> > +#define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE/ CONFIG_SYS_NAND_ECCSIZE)
> > +#define CONFIG_SYS_NAND_OOBSIZE		64
> > +#define CONFIG_SYS_NAND_ECCTOTAL	(CONFIG_SYS_NAND_ECCBYTES * CONFIG_SYS_NAND_ECCSTEPS)
> 
> Lines too long; please fix globally.


  Ok, will check.

> 
> > diff --git a/nand_spl/board/davinci/da8xxevm/config.mk b/nand_spl/board/davinci/da8xxevm/config.mk
> > new file mode 100644
> > index 0000000..ea071eb
> > --- /dev/null
> > +++ b/nand_spl/board/davinci/da8xxevm/config.mk
> ...
> > +include $(TOPDIR)/board/$(BOARDDIR)/config.mk
> > +
> > +# PAD_TO used to generate a 4kByte binary needed for the combined image
> > +# -> PAD_TO = TEXT_BASE + 4096
> > +PAD_TO	:= $(shell expr $$[$(CONFIG_SYS_TEXT_BASE) + 4096])
> > +
> > +ifeq ($(debug),1)
> > +PLATFORM_CPPFLAGS += -DDEBUG
> > +endif
> 
> Is this really needed? Please get rid of this file.


  Will check.

> 
> 
> > diff --git a/nand_spl/nand_boot.c b/nand_spl/nand_boot.c
> > index ccd0af2..3cda41c 100644
> > --- a/nand_spl/nand_boot.c
> > +++ b/nand_spl/nand_boot.c
> > @@ -222,11 +222,12 @@ static int nand_load(struct mtd_info *mtd, unsigned int offs,
> >  }
> >  
> >  #if defined(CONFIG_ARM) && !defined(CONFIG_SYS_ARM_WITHOUT_RELOC)
> > -void board_init_f (ulong bootflag)
> > +void __board_init_f (ulong bootflag)
> >  {
> >  	relocate_code (CONFIG_SYS_TEXT_BASE - TOTAL_MALLOC_LEN, NULL,
> >  		       CONFIG_SYS_TEXT_BASE);
> >  }
> > +void board_init_f (ulong bootflag)__attribute__((weak, alias("__board_init_f")));
> >  #endif
> 
> This is a global change that affects all NAND booting boards. This
> must be submitted spearately, and you must explain in detail why you
> think you need that.  Also please mention on which systems this change
> has been tested.

  For hawkboard, we need to do some board specific initialisation
  which can be included in board_init_f. The freescale boards which
  use the nand spl mechanism to boot define their board specific init
  functions, but we do not do this for the arm boards. I am afraid i
  cannot test this on any other boards, but i have compile tested this
  on the tx25 board.

-sughosh


More information about the U-Boot mailing list