[U-Boot] [POWERPC] mgcoge, mgsuvd: add I2C support.

[Wolfgang Denk]

Dear Heiko Schocher,

In message <48E08972.2040005 at denx.de> you wrote:
> Also support CONFIG_I2C_MULTI_BUS for the
> soft_i2c and the cpu/mpc8260/i2c.c driver.
> 
> Signed-off-by: Heiko Schocher <hs at denx.de>
> ---
>  board/keymile/mgcoge/mgcoge.c |   93 ++++++++++++++++++++++-------------------
>  board/keymile/mgsuvd/mgsuvd.c |   13 ++++++
>  cpu/mpc8260/i2c.c             |   36 ++++++++++++++++
>  drivers/i2c/soft_i2c.c        |   36 ++++++++++++++++
>  include/configs/mgcoge.h      |   25 +++++++++++
>  include/configs/mgsuvd.h      |   42 ++++++++++++++++++
>  6 files changed, 202 insertions(+), 43 deletions(-)
> 
> diff --git a/board/keymile/mgcoge/mgcoge.c b/board/keymile/mgcoge/mgcoge.c
> index 51b6dc6..9b37959 100644
> --- a/board/keymile/mgcoge/mgcoge.c
> +++ b/board/keymile/mgcoge/mgcoge.c
> @@ -1,5 +1,5 @@
>  /*
> - * (C) Copyright 2007
> + * (C) Copyright 2007 - 2008
>   * Heiko Schocher, DENX Software Engineering, hs at denx.de.

You send a number of patches here,. and from reviewing it it becomes
clear that these should be applied in a certain order. But since you
omitted using the "-n" (aka "--numbered") option, it is impossible to
know which order is required.

Please add at least a description in which order to apply the patches.
Thanks.

>   * See file CREDITS for list of people who contributed to this
> @@ -24,11 +24,16 @@
>  #include <common.h>
>  #include <mpc8260.h>
>  #include <ioports.h>
> +#include <malloc.h>
> 
>  #if defined(CONFIG_OF_BOARD_SETUP) && defined(CONFIG_OF_LIBFDT)
>  #include <libfdt.h>
>  #endif
> 
> +#if defined(CONFIG_HARD_I2C) || defined(CONFIG_SOFT_I2C)
> +#include <i2c.h>
> +#endif
> +
>  /*
>   * I/O Port configuration table
>   *
> @@ -163,8 +168,13 @@ const iop_conf_t iop_conf_tab[4][32] = {
>  	/* PD18 */ { 0,          0,   0,   0,   0,   0 }, /* PD18            */
>  	/* PD17 */ { 0,          0,   0,   0,   0,   0 }, /* PD17            */
>  	/* PD16 */ { 0,          0,   0,   0,   0,   0 }, /* PD16            */
> -	/* PD15 */ { 0,          0,   0,   0,   0,   0 }, /* PD15            */
> -	/* PD14 */ { 0,          0,   0,   0,   0,   0 }, /* PD14            */
> +#if defined(CONFIG_HARD_I2C)
> +	/* PD15 */ { 1,          1,   1,   0,   1,   0 }, /* I2C SDA         */
> +	/* PD14 */ { 1,          1,   1,   0,   1,   0 }, /* I2C SCL         */
> +#else
> +	/* PD15 */ { 1,          0,   0,   0,   1,   1 }, /* PD15            */
> +	/* PD14 */ { 1,          0,   0,   1,   1,   1 }, /* PD14            */
> +#endif
>  	/* PD13 */ { 0,          0,   0,   0,   0,   0 }, /* PD13            */
>  	/* PD12 */ { 0,          0,   0,   0,   0,   0 }, /* PD12            */
>  	/* PD11 */ { 0,          0,   0,   0,   0,   0 }, /* PD11            */
> @@ -243,13 +253,13 @@ static long int try_init (volatile memctl8260_t * memctl, ulong sdmr,
>  	*sdmr_ptr = sdmr | PSDMR_OP_NORM | PSDMR_RFEN;
>  	*base = c;
> 
> -	size = get_ram_size((long *)base, maxsize);
> +	size = get_ram_size ((long *)base, maxsize);
>  	*orx_ptr = orx | ~(size - 1);

Please do not mix adding code and code cleanup in one  patch.  Please
split in separate patches. Also note that this space is not mandatory
(and  actually  many  people  think  it  should  not  be used) - it's
sufficient if your source file is consistent in this respect. I  will
not enforce one or the other style here.


...
> -	else {
> +		printf ("ft_blob_update(): cannot set /localbus/ranges "
> +			"property err:%s\n", fdt_strerror (ret));
> +	} else {
>  		/* memory node is required in dts */
> -		printf("ft_blob_update(): cannot find /localbus node "
> -		"err:%s\n", fdt_strerror(nodeoffset));
> +		printf ("ft_blob_update(): cannot find /localbus node "
> +		"err:%s\n", fdt_strerror (nodeoffset));

Indentation is not correct here.

>  		/* memory node is required in dts */
> -		printf("ft_blob_update(): cannot find /soc/cpm/ethernet node "
> -		"err:%s\n", fdt_strerror(nodeoffset));
> +		printf ("ft_blob_update(): cannot find /soc/cpm/ethernet node "
> +		"err:%s\n", fdt_strerror (nodeoffset));

Ditto.

> diff --git a/cpu/mpc8260/i2c.c b/cpu/mpc8260/i2c.c
> index c3af7b6..335177f 100644
> --- a/cpu/mpc8260/i2c.c
> +++ b/cpu/mpc8260/i2c.c
> @@ -36,6 +36,10 @@
> 
>  DECLARE_GLOBAL_DATA_PTR;
> 
> +#if defined(CONFIG_I2C_MULTI_BUS)
> +static unsigned int i2c_bus_num __attribute__ ((section ("data"))) = 0;
> +#endif /* CONFIG_I2C_MULTI_BUS */
> +

Please make MULTI_BUS support a separate patch, too.


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 the  good  computer  designs  are  bootlegged;  the  formally
planned  products,  if  they  are built at all, are dogs!" - David E.
Lundstrom, "A Few Good Men From Univac", MIT Press, 1987