[U-Boot] [PATCH] stxamc8548: initial commit

[Wolfgang Denk]

Dear Alex Dubov,

In message <123712.99670.qm at web37606.mail.mud.yahoo.com> you wrote:
> 
> STx AMC8548 board is an old, AMC form factor, MPC8548 based board intended
> for RapidIO applications. It features 16MiB NAND flash, one DDR2 soDIMM
> slot, ethernet on front panel and backplane, RapidIO on backplane, USB
> controller on local bus (not currently enabled) and no PCI of any kind.

I will not repeat the comments already made by Peter Tyser.

>  board/stxamc8548/Makefile     |   54 ++++++
>  board/stxamc8548/config.mk    |   32 ++++
>  board/stxamc8548/ddr.c        |   87 +++++++++
>  board/stxamc8548/law.c        |   54 ++++++
>  board/stxamc8548/stxamc8548.c |  260 +++++++++++++++++++++++++++
>  board/stxamc8548/tlb.c        |   91 ++++++++++
>  board/stxamc8548/u-boot.lds   |  143 +++++++++++++++
>  include/configs/stxamc8548.h  |  396 +++++++++++++++++++++++++++++++++++++++++
>  8 files changed, 1117 insertions(+), 0 deletions(-)
>  create mode 100644 board/stxamc8548/Makefile
>  create mode 100644 board/stxamc8548/config.mk
>  create mode 100644 board/stxamc8548/ddr.c
>  create mode 100644 board/stxamc8548/law.c
>  create mode 100644 board/stxamc8548/stxamc8548.c
>  create mode 100644 board/stxamc8548/tlb.c
>  create mode 100644 board/stxamc8548/u-boot.lds
>  create mode 100644 include/configs/stxamc8548.h

So this is the 4th STX board we're adding (after stxgp3, stxssa, and
stxxtc). I think it's time to

- create a board/stx/ vendor directory and move the existing and the
  new code there
- factor out common code. For example, files board/stxgp3/ddr.c and
  board/stxssa/ddr.c are absolutely identical, and your new ddr.c
  seems not to be too different; same is true for the law.c files,
  etc.

...
> +	/*
> +	 * Setup SDRAM Base and Option Registers
> +	 */
> +	lbc->or2 = CONFIG_SYS_OR2_PRELIM;
> +	asm("msync");
> +
> +	lbc->br2 = CONFIG_SYS_BR2_PRELIM;
> +	asm("msync");
> +
> +	lbc->lbcr = CONFIG_SYS_LBC_LBCR;
> +	asm("msync");

As already menatione by Peter: use I/O accessors.

...
> +struct fsl_e_tlb_entry tlb_table[] = {
> +	/* TLB 0 - for temp stack in cache */
> +	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,
> +		      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,
> +		      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,

These lines are _way_ too long. Check globally!

...
> +/* The mac addresses for all ethernet interface */
> +#if defined(CONFIG_TSEC_ENET)
> +#define CONFIG_HAS_ETH0
> +#define CONFIG_ETHADDR   02:E0:0C:BA:D0:33
> +#define CONFIG_HAS_ETH1
> +#define CONFIG_ETH1ADDR  02:E0:0C:BA:D1:33
> +#define CONFIG_HAS_ETH2
> +#define CONFIG_ETH2ADDR  02:E0:0C:BA:D2:33
> +#define CONFIG_HAS_ETH3
> +#define CONFIG_ETH3ADDR  02:E0:0C:BA:D3:33
> +#endif
> +
> +#define CONFIG_IPADDR	 192.168.1.201
...
> +#define CONFIG_SERVERIP		192.168.1.14
> +#define CONFIG_GATEWAYIP	192.168.1.254
> +#define CONFIG_NETMASK		255.255.255.0

This gives you a full NAK.

> +#define	CONFIG_EXTRA_ENV_SETTINGS				\
> + "netdev=eth0\0"						\
> + "uboot=" MK_STR(CONFIG_UBOOTPATH) "\0"				\
> + "tftpflash=tftpboot $loadaddr $uboot; "			\
> +	"protect off " MK_STR(TEXT_BASE) " +$filesize; "	\
> +	"erase " MK_STR(TEXT_BASE) " +$filesize; "		\
> +	"cp.b $loadaddr " MK_STR(TEXT_BASE) " $filesize; "	\
> +	"protect on " MK_STR(TEXT_BASE) " +$filesize; "		\
> +	"cmp.b $loadaddr " MK_STR(TEXT_BASE) " $filesize\0"	\

Indentation by TAB only, please (fix globally).

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
We have phasers, I vote we blast 'em!
	-- Bailey, "The Corbomite Maneuver", stardate 1514.2