From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ben Warren <biggerbadderben@gmail.com>
Date: Wed, 19 Nov 2008 10:02:25 -0800
Subject: [U-Boot] [PATCH 4/5 v2] powerpc: keymile: Add a check for the
 PIGGY debug board
In-Reply-To: <4923D8B1.6050204@denx.de>
References: <4923D8B1.6050204@denx.de>
Message-ID: <492454B1.8070002@gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Heiko,

Heiko Schocher wrote:
> Check the presence of the PIGGY on the keymile boards mgcoge,
> mgsuvd and kmeter1. If the PIGGY is not present, dont register
> this Ethernet device.
>
> Signed-off-by: Heiko Schocher <hs@denx.de>
> ---
>
> changes since v1:
>
> - rebased against current HEAD
>   commit e0b0ec843085f96f4fe0738424835ee90e58bc00
>
> - use board_eth_init () for the Piggy Test
>
>  board/keymile/common/common.c   |   24 ++++++++++++++++++++++--
>  board/keymile/kmeter1/Makefile  |    6 ++++--
>  board/keymile/kmeter1/kmeter1.c |   11 ++++++++++-
>  board/keymile/mgcoge/mgcoge.c   |   12 ++++++++++--
>  board/keymile/mgsuvd/mgsuvd.c   |   11 ++++++++++-
>  include/configs/kmeter1.h       |   10 ++++++++++
>  include/configs/mgcoge.h        |   11 +++++++++++
>  include/configs/mgsuvd.h        |   10 ++++++++++
>  8 files changed, 87 insertions(+), 8 deletions(-)
>
> diff --git a/board/keymile/common/common.c b/board/keymile/common/common.c
> index a4cf24c..4990f00 100644
> --- a/board/keymile/common/common.c
> +++ b/board/keymile/common/common.c
> @@ -22,10 +22,14 @@
>   */
>
>  #include <common.h>
> +#if defined(CONFIG_MGCOGE)
>  #include <mpc8260.h>
> +#endif
>  #include <ioports.h>
>  #include <malloc.h>
>  #include <hush.h>
> +#include <net.h>
> +#include <asm/io.h>
>
>  #if defined(CONFIG_OF_BOARD_SETUP) && defined(CONFIG_OF_LIBFDT)
>  #include <libfdt.h>
> @@ -33,8 +37,6 @@
>
>  #if defined(CONFIG_HARD_I2C) || defined(CONFIG_SOFT_I2C)
>  #include <i2c.h>
> -#endif
> -#include <asm/io.h>
>
>  extern int i2c_soft_read_pin (void);
>
> @@ -495,6 +497,7 @@ void i2c_init_board(void)
>  #endif
>  }
>  #endif
> +#endif
>
>  #if defined(CONFIG_OF_BOARD_SETUP) && defined(CONFIG_OF_LIBFDT)
>  int fdt_set_node_and_value (void *blob,
> @@ -521,3 +524,20 @@ int fdt_set_node_and_value (void *blob,
>  	return ret;
>  }
>  #endif
> +
> +#if defined(CONFIG_CHECK_ETHERNET_PRESENT)
> +int ethernet_present (int index)
> +{
> +	int	ret;
> +	ret = in_8((u8 *)CONFIG_SYS_PIGGY_BASE + CONFIG_SYS_SLOT_ID_OFF) & 0x80;
> +	return ret;
>   
A nit, but this could be a one liner (ret isn't really needed).  Also, 
you don't use index...
> +}
> +
> +int board_eth_init(bd_t *bis)
> +{
> +	if (ethernet_present (0)) {
> +		return -1;
> +	}
> +	return 0;
> +}
>   
As mentioned above, do you need to pass a parameter?
> +#endif
> diff --git a/board/keymile/kmeter1/Makefile b/board/keymile/kmeter1/Makefile
> index 88b79f3..12a1518 100644
> --- a/board/keymile/kmeter1/Makefile
> +++ b/board/keymile/kmeter1/Makefile
> @@ -22,12 +22,14 @@
>  #
>
>  include $(TOPDIR)/config.mk
> +ifneq ($(OBJTREE),$(SRCTREE))
> +$(shell mkdir -p $(obj)../common)
> +endif
>
>  LIB	= $(obj)lib$(BOARD).a
>
> -COBJS-y += $(BOARD).o
> +COBJS	+= $(BOARD).o ../common/common.o
>
> -COBJS	:= $(COBJS-y)
>  SRCS	:= $(SOBJS:.o=.S) $(COBJS:.o=.c)
>  OBJS	:= $(addprefix $(obj),$(COBJS))
>  SOBJS	:= $(addprefix $(obj),$(SOBJS))
> diff --git a/board/keymile/kmeter1/kmeter1.c b/board/keymile/kmeter1/kmeter1.c
> index f9a59a6..0c4dbe7 100644
> --- a/board/keymile/kmeter1/kmeter1.c
> +++ b/board/keymile/kmeter1/kmeter1.c
> @@ -27,6 +27,10 @@
>  #include <pci.h>
>  #include <libfdt.h>
>
> +#if defined(CONFIG_CHECK_ETHERNET_PRESENT)
> +extern int ethernet_present (int index);
> +#endif
> +
>   
I'd prefer to see the function prototype in a header file.  Even if you 
just 'extern' it here, I'm not sure you need to wrap it in an #ifdef.  
This comment applies to other places in this patch.

A bigger question:  do you really need CONFIG_CHECK_ETHERNET_PRESENT, 
since all the logic is in your board code?

<snip>


regards,
Ben