[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