Re: [U-Boot] [PATCH] Driver for AX88183 ethernet chip

29 Jan 2011

Hi Stefano,
Thank you for your advice.It has been changed according to you suggestion and passed the checking of checkpatch.pl (two warnings , one is asm/io.h one is struct eth_device's member function send has a volatile parameter).
Although AX88183 has the similar name with AX88180, they are very different.
It works well on my board.I have submitted it again.
Best wishes.
Joe
...
Date: Fri, 28 Jan 2011 09:02:38 +0100
From: sbabic@denx.de
To: lgxue@hotmail.com
CC: u-boot@lists.denx.de
Subject: Re: [U-Boot] [PATCH] Driver for AX88183 ethernet chip
On 01/27/2011 07:55 PM, root wrote:
...
From: Joe Xue lgxue@hotmail.com
You have to set you Signe-off-by in your patch. Please add also
a commit message to explain what your patch is doing, and on which board
you tested this driver. And set the net custodian as CC, this helps to
get faster review.
...

--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -83,6 +83,7 @@ COBJS-$(CONFIG_TSI108_ETH) += tsi108_eth.o
 COBJS-$(CONFIG_ULI526X) += uli526x.o
 COBJS-$(CONFIG_VSC7385_ENET) += vsc7385.o
 COBJS-$(CONFIG_XILINX_EMACLITE) += xilinx_emaclite.o
+COBJS-$(CONFIG_DRIVER_AX88183) += ax88183.o
Please maintain the list sorted.
...
@@ -0,0 +1,265 @@



+/*


(C) Copyright 2010  Joe Xue lgxue@hotmail.com







This file is released under the terms of GPL v2 and any later version.



See the file COPYING in the root directory of the source tree for details.



+*/
The header is not enough. Please add coherent header with license, as
you can see for other drivers. This must be fixed globally.
...



+/*


This driver works on 32bit mode



AX88783 has two ethernet ports, we only use port 0 in u-boot


*/

I suggest you add this comment to the README file, too,
...
+#include <common.h>
+#include <command.h>
+#include <net.h>
+#include <linux/mii.h>
+#include <miiphy.h>
+#include <config.h>
+#include "ax88183.h"



+static int ax88183_init(bd_t *bd)
+{

volatile unsigned int tmp;
int i;

/* reset chip */
tmp = CR;
CR = (tmp & ~CR_CHIP_RESET);

u-boot does not not accept anymore to write/read into registers using
offsets and volatile variables. Please define an
appropriate structure for your chip, and use read/write accessors to
work with registers. This must be fixed globally in all your code.
I do not know this chip. However, u-boot has already a ax88180 driver.
Would be not possible to modify this driver to make it suitable for your
chip, too ?
Other remarks: it seems to me your driver does not support CONFIG_MULTI,
as it should do. And it must call eth_register() to be inserted as
device at runtime. Please check with other network drivers.
...

udelay(1000);
CR = (tmp | CR_CHIP_RESET);

/* disable interrupt */
IMSR = IMSR_MASK_ALL;

/* set mac address*/
unsigned char mactmp[4];
unsigned int * mac = (unsigned int *)mactmp;
mactmp[0] = bd->bi_enetaddr[5];
mactmp[1] = bd->bi_enetaddr[4];
mactmp[2] = bd->bi_enetaddr[3];
mactmp[3] = bd->bi_enetaddr[2];

This is not correct anymore. You do not have to set the mac from the bd.
Instead of that, you have to provide in your initialization function a
struct eth_device that should be passed to eth_register().
...



+int eth_init (bd_t * bd)
+{


char *s, *e;
int i;

s = getenv ("ethaddr");

The driver must not call getenv explicitely.
...
+#if 0

printf ("\tHW MAC address:  "
"%02X:%02X:%02X:%02X:%02X:%02X\n",


bd->bi_enetaddr[0], bd->bi_enetaddr[1],


bd->bi_enetaddr[2], bd->bi_enetaddr[3],


bd->bi_enetaddr[4], bd->bi_enetaddr[5] );



+#endif
do not add dead code. Instead of that, you can use the debug() function
to trace.
...
+/*

*-------------------------------------------------------------------------

Registers


*-------------------------------------------------------------------------
*/


+#define PCR  (*(volatile unsigned int *)(CONFIG_AX88183_BASE + 0x0004))
Replace all defines with a C structure describing your chip.
Best regards,
Stefano Babic
--
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
=====================================================================

    

Re: [U-Boot] [PATCH] Driver for AX88183 ethernet chip

Joe XUE

--