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.
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(®->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.
+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 ?
- 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.
- res = ax88183_phy_initial(dev);
Name must be changed.
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.
Best regards, Stefano Babic