From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefano Babic <sbabic@denx.de>
Date: Wed, 02 Feb 2011 19:02:11 +0100
Subject: [U-Boot] [PATCH] Add support for ASIX's AX88783 ethernet chip
In-Reply-To: <1296495758-17784-1-git-send-email-lgxue@hotmail.com>
References: <Joe Xue> <1296495758-17784-1-git-send-email-lgxue@hotmail.com>
Message-ID: <4D499C23.40500@denx.de>
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

On 01/31/2011 06:42 PM, Joe Xue wrote:
> for more information about this chip, please check:
> http://www.asix.com.tw/products.php?op=pItemdetail&PItemID=98;65;86&PLine=65
> 
> Signed-off-by: Joe Xue <lgxue@hotmail.com>
> 

Please add a version number to your patch to make easier tracking which
is your last version.

Do not forget to add always the net Maintainer to CC (Wolfgang Denk), I
added him now.

> --- /dev/null
> +++ b/drivers/net/ax88783.c
> @@ -0,0 +1,297 @@
> +/*
> + *

You should drop this line

> +
> +static int ax88183_phy_initial(struct eth_device *dev)

You forget to replace the name of the function. It has still ax88183_

> +	/* phy init */
> +	tmp = readl(&reg->pcr);
> +	tmp |= PCR_PHY0_RESET_CLEAR;
> +
> +	writel(tmp, &reg->pcr);
> +	udelay(100000);

you already explained why you need such a long delay. It is not bad to
add your explanation as comment here, so everyone knows your answer.

> +static void ax88783_halt(struct eth_device *dev)
> +{
> +	unsigned int tmp;
> +	struct ax88783_reg *reg = (struct ax88783_reg *)dev->iobase;
> +	tmp = readl(&reg->pcr);
> +	writel((tmp | PCR_LOOP_BACK), &reg->pcr);
> +}