[U-Boot] [PATCH] stx: create common vendor hierarchy for Silicon Turnkey boards

Wolfgang Denk wd at denx.de
Tue Aug 4 22:54:39 CEST 2009


Dear oakad at yahoo.com,

In message <1249383697-28141-1-git-send-email-oakad at yahoo.com> you wrote:
> From: Alex Dubov <oakad at yahoo.com>
> 
> Move board definition files for STx XTC, GP3 and SSA boards into
> common subdirectory and factor out common code.
> 
> "-mno-spe" flag common to all MPC85xx configurations does not work
> so change it to "-mspe=no" which does (GCC bug 37759).
> 
> Signed-off-by: Alex Dubov <oakad at yahoo.com>
...
> --- a/board/stxgp3/Makefile
> +++ b/board/stx/common/Makefile
> @@ -1,5 +1,5 @@
>  #
> -# (C) Copyright 2001-2006
> +# (C) Copyright 2006
>  # Wolfgang Denk, DENX Software Engineering, wd at denx.de.

How comes?

> --- a/board/stxssa/ddr.c
> +++ b/board/stx/common/ddr.c
> @@ -1,5 +1,6 @@
>  /*
>   * Copyright 2008 Freescale Semiconductor, Inc.
> + * Copyright 2009 Alex Dubov <oakad at yahoo.com>
>   *
>   * This program is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU General Public License

Please do not add a Copyright for such minor changes. Attribution for
your work by the Signed-off-by: messages in the git repository should
be sufficient here.

...
> +	 * Factors to consider for clock adjust:
> +	 *	- number of chips on bus
> +	 *	- position of slot
> +	 *	- DDR1 vs. DDR2?
> +	 *	- ???
> +	 *
> +	 * This needs to be determined on a board-by-board basis.
> +	 *	0110	3/4 cycle late
> +	 *	0111	7/8 cycle late
> +	 */
> +	popts->clk_adjust = 6;

If this "needs to be determined on a board-by-board basis" i would
expect some CONFIG_SYS_* variable here, which is defined in the
respective board config files?

> +#if defined(CONFIG_FSL_DDR2)
> +	popts->cpo_override = 7;
> +#else
>  	popts->cpo_override = 0;
> +#endif

Ditto - this should go to the respective board config files.

> diff --git a/board/stxssa/law.c b/board/stx/common/law.c
> similarity index 74%
> rename from board/stxssa/law.c
> rename to board/stx/common/law.c
> index 55dde66..a82c99f 100644
> --- a/board/stxssa/law.c
> +++ b/board/stx/common/law.c
> @@ -1,9 +1,9 @@
>  /*
> - * Copyright 2008 Freescale Semiconductor, Inc.
> - *
>   * (C) Copyright 2000
>   * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
>   *
> + * Copyright 2009 Alex Dubov <oakad at yahoo.com>
> + *
>   * See file CREDITS for list of people who contributed to this
>   * project.

NAK. Never ever delete any Copyrights. And don't add yoru own just for
minor changes.

> @@ -31,30 +31,34 @@
>   * LAW(Local Access Window) configuration:
>   *
>   * 0x0000_0000     0x7fff_ffff     DDR                     2G
> - * 0x8000_0000     0x9fff_ffff     PCI1 MEM                512M
> - * 0xa000_0000     0xbfff_ffff     PCI2 MEM                512M
> + * 0x8000_0000     0x9fff_ffff     PCI1                    512M
> + * 0xa000_0000     0xbfff_ffff     PCI2                    512M
> + * 0xc000_0000     0xdfff_ffff     RapidIO                 512M
>   * 0xe000_0000     0xe000_ffff     CCSR                    1M
>   * 0xe200_0000     0xe2ff_ffff     PCI1 IO                 16M
>   * 0xe300_0000     0xe3ff_ffff     PCI2 IO                 16M
> - * 0xf000_0000     0xfaff_ffff     Local bus               128M
> - * 0xfb00_0000     0xfb00_ffff     Config Latch            64K
> - * 0xfc00_0000     0xffff_ffff     FLASH (boot bank)       64M
> + * 0xf000_0000     0xffff_ffff     LBC options + FLASH     256M

Are you sure this is correct?

>  struct law_entry law_table[] = {
> -#ifndef CONFIG_SPD_EEPROM
> -	SET_LAW(CONFIG_SYS_DDR_SDRAM_BASE, LAW_SIZE_128M, LAW_TRGT_IF_DDR),
> +#ifdef CONFIG_SYS_PCI1_MEM_PHYS
> +	SET_LAW(CONFIG_SYS_PCI1_MEM_PHYS, LAW_SIZE_512M, LAW_TRGT_IF_PCI),
> +	SET_LAW(CONFIG_SYS_PCI1_IO_PHYS, LAW_SIZE_16M, LAW_TRGT_IF_PCI),
>  #endif
> -	SET_LAW(CONFIG_SYS_PCI1_MEM_PHYS, LAW_SIZE_512M, LAW_TRGT_IF_PCI_1),
> +#ifdef CONFIG_SYS_PCI2_MEM_PHYS
>  	SET_LAW(CONFIG_SYS_PCI2_MEM_PHYS, LAW_SIZE_512M, LAW_TRGT_IF_PCI_2),
> -	SET_LAW(CONFIG_SYS_PCI1_IO_PHYS, LAW_SIZE_16M, LAW_TRGT_IF_PCI_1),
>  	SET_LAW(CONFIG_SYS_PCI2_IO_PHYS, LAW_SIZE_16M, LAW_TRGT_IF_PCI_2),
> -	/* Map the whole localbus, including flash and reset latch. */
> -	SET_LAW(CONFIG_SYS_LBC_OPTION_BASE, LAWAR_SIZE_256M, LAW_TRGT_IF_LBC),
> +#endif
> +#ifdef CONFIG_SYS_RIO_MEM_PHYS
> +	SET_LAW(CONFIG_SYS_RIO_MEM_PHYS, LAW_SIZE_512M, LAW_TRGT_IF_RIO),
> +#endif
> +	SET_LAW(CONFIG_SYS_LBC_OPTION_BASE, LAWAR_SIZE_256M, LAW_TRGT_IF_LBC)
>  };

This looks fishy, too.

> --- a/board/stxssa/tlb.c
> +++ b/board/stx/common/tlb.c
> @@ -1,9 +1,9 @@
>  /*
> - * Copyright 2008 Freescale Semiconductor, Inc.
> - *
>   * (C) Copyright 2000
>   * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
>   *
> + * Copyright 2009 Alex Dubov <oakad at yahoo.com>
> + *
>   * See file CREDITS for list of people who contributed to this
>   * project.

NAK. See above.

> @@ -31,76 +31,90 @@ struct fsl_e_tlb_entry tlb_table[] = {
>  	SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR, CONFIG_SYS_INIT_RAM_ADDR,
>  		      MAS3_SX|MAS3_SW|MAS3_SR, 0,
>  		      0, 0, BOOKE_PAGESZ_4K, 0),
> -	SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR + 4 * 1024 , CONFIG_SYS_INIT_RAM_ADDR + 4 * 1024,
> +	SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR + 4 * 1024,
> +		      CONFIG_SYS_INIT_RAM_ADDR + 4 * 1024,
>  		      MAS3_SX|MAS3_SW|MAS3_SR, 0,
>  		      0, 0, BOOKE_PAGESZ_4K, 0),
> -	SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR + 8 * 1024 , CONFIG_SYS_INIT_RAM_ADDR + 8 * 1024,
> +	SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR + 8 * 1024,
> +		      CONFIG_SYS_INIT_RAM_ADDR + 8 * 1024,
>  		      MAS3_SX|MAS3_SW|MAS3_SR, 0,
>  		      0, 0, BOOKE_PAGESZ_4K, 0),
> -	SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR + 12 * 1024 , CONFIG_SYS_INIT_RAM_ADDR + 12 * 1024,
> +	SET_TLB_ENTRY(0, CONFIG_SYS_INIT_RAM_ADDR + 12 * 1024,
> +		      CONFIG_SYS_INIT_RAM_ADDR + 12 * 1024,
>  		      MAS3_SX|MAS3_SW|MAS3_SR, 0,
>  		      0, 0, BOOKE_PAGESZ_4K, 0),

Better split code beautifying into separate patch.

>  	/*
> -	 * TLB 0:	64M	Non-cacheable, guarded
> -	 * 0xfc000000	6M4	FLASH
> +	 * TLB 0:	256M	Non-cacheable, guarded
> +	 * 0xf0000000	256M	LBC (FLASH included)
>  	 * Out of reset this entry is only 4K.
>  	 */

You mean you change the mapping for soem (or all) boards?

> -	SET_TLB_ENTRY(1, CONFIG_SYS_FLASH_BASE, CONFIG_SYS_FLASH_BASE,
> +	SET_TLB_ENTRY(1, CONFIG_SYS_LBC_OPTION_BASE,
> +		      CONFIG_SYS_LBC_OPTION_BASE,
>  		      MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
> -		      0, 0, BOOKE_PAGESZ_64M, 1),
> +		      0, 0, BOOKE_PAGESZ_256M, 1),
>  
>  	/*
> -	 * TLB 1:	256M	Non-cacheable, guarded
> -	 * 0x80000000	256M	PCI1 MEM First half
> +	 * TLB 1:	64M	Non-cacheable, guarded
> +	 * 0xe000_0000	1M	CCSRBAR
> +	 * 0xe200_0000	1M	PCI1 IO
> +	 * 0xe210_0000	1M	PCI2 IO
> +	 * 0xe300_0000	1M	PCIe IO
>  	 */
> -	SET_TLB_ENTRY(1, CONFIG_SYS_PCI1_MEM_PHYS, CONFIG_SYS_PCI1_MEM_PHYS,
> +	SET_TLB_ENTRY(1, CONFIG_SYS_CCSRBAR, CONFIG_SYS_CCSRBAR_PHYS,
>  		      MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
> -		      0, 1, BOOKE_PAGESZ_256M, 1),
> +		      0, 1, BOOKE_PAGESZ_64M, 1),

Hm... did you actually test these changes on all 4 boards?

> diff --git a/cpu/mpc85xx/config.mk b/cpu/mpc85xx/config.mk
> index 9e574a2..a7d948d 100644
> --- a/cpu/mpc85xx/config.mk
> +++ b/cpu/mpc85xx/config.mk
> @@ -25,4 +25,4 @@ PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi
>  
>  PLATFORM_CPPFLAGS += -DCONFIG_MPC85xx -DCONFIG_E500 -ffixed-r2 \
>  			-Wa,-me500 -msoft-float -mno-string
> -PLATFORM_CPPFLAGS +=$(call cc-option,-mno-spe)
> +PLATFORM_CPPFLAGS +=$(call cc-option,-mspe=no)

This is an unrelated change which must be submitted as a separate
patch.

> diff --git a/include/configs/stxgp3.h b/include/configs/stxgp3.h
> index 0424e29..12df277 100644
> --- a/include/configs/stxgp3.h
> +++ b/include/configs/stxgp3.h
> @@ -78,6 +78,7 @@
>   */
>  #define CONFIG_SYS_LBC_SDRAM_BASE      0xf0000000      /* Localbus SDRAM */
>  #define CONFIG_SYS_LBC_SDRAM_SIZE	256		/* LBC SDRAM is 64MB	*/
> +#define CONFIG_SYS_LBC_OPTION_BASE     0xf0000000
>  
>  #define CONFIG_SYS_FLASH_BASE        0xff000000      /* start of FLASH 16M    */
>  #define CONFIG_SYS_BR0_PRELIM        0xff001801      /* port size 32bit      */
> @@ -195,18 +196,20 @@
>  
>  /* RapdIO Map configuration, mapped 1:1.
>  */
> -#define CONFIG_SYS_RIO_MEM_BASE	0xc0000000
> -#define CONFIG_SYS_RIO_MEM_PHYS	CONFIG_SYS_RIO_MEM_BASE
> -#define CONFIG_SYS_RIO_MEM_SIZE	0x200000000	/* 512 M */
> +#define CONFIG_SYS_RIO_MEM_BASE 0xc0000000
> +#define CONFIG_SYS_RIO_MEM_PHYS CONFIG_SYS_RIO_MEM_BASE
> +#define CONFIG_SYS_RIO_MEM_VIRT CONFIG_SYS_RIO_MEM_BASE
> +#define CONFIG_SYS_RIO_MEM_SIZE 0x200000000	/* 512 M */

Please submit code cleanup as separate patch(es).

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: wd at denx.de
All a hacker needs is a tight PUSHJ, a loose pair of UUOs, and a warm
place to shift.


More information about the U-Boot mailing list