From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ben Warren <biggerbadderben@gmail.com>
Date: Mon, 25 May 2009 23:34:43 -0700
Subject: [U-Boot] [PATCH v4] Gbe Controller driver support for kirkwood
 SOCs
In-Reply-To: <1242937472-30778-4-git-send-email-prafulla@marvell.com>
References: <1242937472-30778-1-git-send-email-prafulla@marvell.com>	<1242937472-30778-2-git-send-email-prafulla@marvell.com>	<1242937472-30778-3-git-send-email-prafulla@marvell.com>
	<1242937472-30778-4-git-send-email-prafulla@marvell.com>
Message-ID: <4A1B8D83.3010407@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 Prafulla,

Prafulla Wadaskar wrote:
> Contributors:
> Yotam Admon <yotam@marvell.com>
> Michael Blostein <michaelbl@marvell.com
>
> Reviewed by: Ronen Shitrit <rshitrit@marvell.com>
> Signed-off-by: Prafulla Wadaskar <prafulla@marvell.com>
> ---
> Change log:
> v2: entire rewrite/restructure of v1
> used small names for variable/function names
> readl/writel used to access SoC registers
> Soc registers accssed using pointers through net device struct
> miiphy registration done for external smi read/write access
> miiphy_link used to detect phy link presence
> cleaned for cosmetic changes
>
> v3: asm/arch/kirkwood.h included in c file
>
> v4: bugfix for phy address read
>
>  cpu/arm926ejs/kirkwood/cpu.c |    8 +
>  drivers/net/Makefile         |    1 +
>  drivers/net/kirkwood_egiga.c | 1670 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/net/kirkwood_egiga.h |  828 +++++++++++++++++++++
>  include/netdev.h             |    1 +
>  5 files changed, 2508 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/net/kirkwood_egiga.c
>  create mode 100644 drivers/net/kirkwood_egiga.h
>
> diff --git a/cpu/arm926ejs/kirkwood/cpu.c b/cpu/arm926ejs/kirkwood/cpu.c
> index 1286cac..d37c2e2 100644
> --- a/cpu/arm926ejs/kirkwood/cpu.c
> +++ b/cpu/arm926ejs/kirkwood/cpu.c
> @@ -314,3 +314,11 @@ int arch_misc_init(void)
>  	return 0;
>  }
>  #endif /* CONFIG_ARCH_MISC_INIT */
> +
> +#ifdef CONFIG_KIRKWOOD_EGIGA
> +int cpu_eth_init(bd_t *bis)
> +{
> +	kirkwood_egiga_initialize(bis);
> +	return 0;
> +}
> +#endif
>   
Please move this part (cpu.c) to a different patch.  This one should 
only include the driver itself, and I can't apply it since the cpu.c 
file doesn't exist yet...

I don't have any technical problems with this patch, but please consider 
the following:

1. Statistics are great to have, but most people won't look at them and 
all the associated logic makes the driver very big.  Please 
conditionally compile that stuff in.

2. Please ensure that all lines are <= 78 characters long (80 minus the 
initial character that diff puts in)

regards,
Ben