[U-Boot] [PATCH] ppc4xx: Add CATCenter Io 405EP board support

[Wolfgang Denk]

Dear Dirk Eibach,

In message <1286890387-11491-1-git-send-email-eibach at gdsys.de> you wrote:
> Board support for the Guntermann & Drunck CATCenter Io.
> 
> Signed-off-by: Dirk Eibach <eibach at gdsys.de>
> ---
>  MAKEALL                  |    1 +
>  Makefile                 |    3 +
>  board/gdsys/io/Makefile  |   51 ++++++++++
>  board/gdsys/io/config.mk |   24 +++++
>  board/gdsys/io/io.c      |  231 ++++++++++++++++++++++++++++++++++++++++++
>  include/configs/io.h     |  251 ++++++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 561 insertions(+), 0 deletions(-)
>  create mode 100644 board/gdsys/io/Makefile
>  create mode 100644 board/gdsys/io/config.mk
>  create mode 100644 board/gdsys/io/io.c
>  create mode 100644 include/configs/io.h

Entry to MAINTAINERS missing.

> diff --git a/MAKEALL b/MAKEALL
> index 1b506d6..37aeed4 100755
> --- a/MAKEALL
> +++ b/MAKEALL
> @@ -156,6 +156,7 @@ LIST_4xx="$(boards_by_cpu ppc4xx)
>  	haleakala_nand	\
>  	hcu4		\
>  	hcu5		\
> +	io		\
>  	intip		\
>  	kilauea		\
>  	kilauea_nand	\

Please keep lists sorted.

> diff --git a/Makefile b/Makefile
> index 8df60fa..86f9efa 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1004,6 +1004,9 @@ mcu25_config:  unconfig
>  	@mkdir -p $(obj)board/netstal/common
>  	@$(MKCONFIG) $@ powerpc ppc4xx $(call lcname,$@) netstal
>  
> +io_config: unconfig
> +	@$(MKCONFIG) $(@:_config=) powerpc ppc4xx io gdsys
> +

NAK. We don't accept entries to Makefile any more. Please add your
board description to boards.cfg instead.

And remember to keep lists sorted.

> +int configure_gbit_phy(unsigned char addr)
> +{
> +	unsigned short value;
> +
> +	if (miiphy_write(GBIT_PHY_NAME, addr, 0x16, 0x0002))
> +		goto err_out;
> +	if (miiphy_write(GBIT_PHY_NAME, addr, 0x1a, 0x800a))
> +		goto err_out;
> +	if (miiphy_write(GBIT_PHY_NAME, addr, 0x16, 0x0000))
> +		goto err_out;
> +	if (miiphy_read(GBIT_PHY_NAME, addr, 0x10, &value))
> +		goto err_out;
> +	if (miiphy_write(GBIT_PHY_NAME, addr, 0x10, value & ~0x0004))
> +		goto err_out;
> +	if (miiphy_write(GBIT_PHY_NAME, addr, 0x00, 0x9140))
> +		goto err_out;

You might want to replace all these magic constants with some
#defines, and add some comments what you're actually doing.

> +	while (!(in_le16((void *)(CONFIG_SYS_LATCH_BASE + 0x200)) & 0x0010));

Semicolon onnew line, please.

> +	/*
> +	 * wait for fpga out of reset
> +	 */
> +	while (1) {
> +		fpga_set_reg(REG_REFELECTION_LOW, REFLECTION_TESTPATTERN);
> +		if (fpga_get_reg(REG_REFELECTION_HIGH) ==
> +			REFLECTION_TESTPATTERN_INV)
> +			break;
> +	}

What happens if this loop does not terminate? Maybe a timeout and
error message would be helpful?

> +	char *s = getenv("serial#");
> +	u16 versions = fpga_get_reg(REG_VERSIONS);
> +	u16 fpga_version = fpga_get_reg(REG_FPGA_VERSION);
> +	u16 fpga_features = fpga_get_reg(REG_FPGA_FEATURES);
> +	unsigned unit_type = (versions & 0xf000) >> 12;
> +	unsigned hardware_version = versions & 0x000f;
> +	unsigned feature_channels = fpga_features & 0x007f;
> +	unsigned feature_expansion = fpga_features & (1<<15);

Please separate declarations and code.  A few initializations may be
ok, but this is unreadable.

> +	printf(", expansion %ssupported", feature_expansion ? "" : "un");
> +
> +	puts("\n");

Why not include the newline in the printf() format string ?

> diff --git a/include/configs/io.h b/include/configs/io.h
> new file mode 100644
> index 0000000..53dce7d
> --- /dev/null
> +++ b/include/configs/io.h
> @@ -0,0 +1,251 @@
> +/*
> + * (C) Copyright 2009
> + * Dirk Eibach,  Guntermann & Drunck GmbH, eibach at gdsys.de

2009?

+
> +#define CONFIG_PHY_ADDR		4	/* PHY address			*/
> +#define CONFIG_HAS_ETH0
> +#define CONFIG_HAS_ETH1
> +#define CONFIG_PHY1_ADDR	0xc	/* EMAC1 PHY address		*/
> +#define CONFIG_PHY_CLK_FREQ    EMAC_STACR_CLK_66MHZ

Please use TABs for vertical alignment.

> +/*
> + * I2C stuff
> + */
> +#define CONFIG_SYS_I2C_SPEED		100000	/* I2C speed and slave address*/

Comment wrong? I don't see a slave address here.

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
Unix is like a toll road on which you have to stop every 50  feet  to
pay another nickel. But hey! You only feel 5 cents poorer each time.
                 - Larry Wall in <1992Aug13.192357.15731 at netlabs.com>