Than you Stefano.
Date: Wed, 2 Feb 2011 19:02:11 +0100 From: sbabic@denx.de To: lgxue@hotmail.com CC: u-boot@lists.denx.de; wd@denx.de Subject: Re: [U-Boot] [PATCH] Add support for ASIX's AX88783 ethernet chip
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&...
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.
Will add it.
Do not forget to add always the net Maintainer to CC (Wolfgang Denk), I added him now.
Not exactly understand your meaning. You mean I should add wd as maintainer to my code or just add him in mail.
--- /dev/null +++ b/drivers/net/ax88783.c @@ -0,0 +1,297 @@ +/*
You should drop this line
will drop it.
+static int ax88183_phy_initial(struct eth_device *dev)
You forget to replace the name of the function. It has still ax88183_
will change, sorry for it.
- /* phy init */
- tmp = readl(®->pcr);
- tmp |= PCR_PHY0_RESET_CLEAR;
- writel(tmp, ®->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.
will do.
+static void ax88783_halt(struct eth_device *dev) +{
- unsigned int tmp;
- struct ax88783_reg *reg = (struct ax88783_reg *)dev->iobase;
- tmp = readl(®->pcr);
- writel((tmp | PCR_LOOP_BACK), ®->pcr);
+}
From the name it seems you set the controller in loopback, instead of disabling it. Is it correct ?
mmn. I just make it can't receive the data outside.The other way is make it into sleep mode.
- res = eth_getenv_enetaddr("ethaddr", dev->enetaddr);
- if (!res) {
puts("Please set your MAC address!");free(dev);return 0;- }
This is wrong. A network driver should not call directly eth_getenv_enetaddr, and you do not need. If you want to check the mac addrsss, use is_valid_ether_addr() inside ax88783_init() before copying the mac to the hardware.
I refer to the newest net driver patch ftgmac100.c, it uses this function to getmac address from environmental setting. and I checked the code eth_getenv_enetaddralso call the is_valid_ether_addr(). Anyway, will change it according to your advice.
- res = ax88183_phy_initial(dev);
Name must be changed.
will change.
diff --git a/drivers/net/ax88783.h b/drivers/net/ax88783.h new file mode 100644 index 0000000..09ac9ed --- /dev/null +++ b/drivers/net/ax88783.h @@ -0,0 +1,100 @@ +/*
As explained, all headers start with copyright on the second line. Drop this line.
Yes, thank you for your patience :-)
Best regards, Stefano Babic
Best wishes, Joe
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================