Hi Guennadi,
Guennadi Liakhovetski wrote:
From: Sascha Hauer s.hauer@pengutronix.de
This patch adds a driver for the following smsc network controllers: LAN9115 LAN9116 LAN9117 LAN9215 LAN9216 LAN9217
How many of these have been tested, and on what platforms. I'm asking because the code seems to assume a 32-bit interface and these aren't all 32-bit chips.
Signed-off-by: Sascha Hauer s.hauer@pengutronix.de Signed-off-by: Guennadi Liakhovetski lg@denx.de
Changes since v1: Removed C++ style comments
drivers/net/Makefile | 1 + drivers/net/smc911x.c | 668 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 669 insertions(+), 0 deletions(-) create mode 100644 drivers/net/smc911x.c
diff --git a/drivers/net/Makefile b/drivers/net/Makefile index 320dc3e..9482398 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -54,6 +54,7 @@ COBJS-y += rtl8139.o COBJS-y += rtl8169.o COBJS-y += s3c4510b_eth.o COBJS-y += smc91111.o +COBJS-y += smc911x.o COBJS-y += tigon3.o COBJS-y += tsec.o COBJS-y += tsi108_eth.o diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c new file mode 100644 index 0000000..5830368 --- /dev/null +++ b/drivers/net/smc911x.c @@ -0,0 +1,667 @@ +/*
- SMSC LAN9[12]1[567] Network driver
- (c) 2007 Pengutronix, Sascha Hauer s.hauer@pengutronix.de
- See file CREDITS for list of people who contributed to this
- project.
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License as
- published by the Free Software Foundation; either version 2 of
- the License, or (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program; if not, write to the Free Software
- Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- MA 02111-1307 USA
- */
+#include <common.h>
+#ifdef CONFIG_DRIVER_SMC911X
This should be moved to the Makefile. Looks like I beat J-C to this one...
+#include <command.h> +#include <net.h> +#include <miiphy.h>
+#define mdelay(n) udelay((n)*1000)
+#define __REG(x) (*((volatile u32 *)(x)))
See, you're assuming 32-bit accesses. That should be configurable, I think
+/* Below are the register offsets and bit definitions
- of the Lan911x memory space
- */
+#define RX_DATA_FIFO __REG(CONFIG_DRIVER_SMC911X_BASE + 0x00)
+#define TX_DATA_FIFO __REG(CONFIG_DRIVER_SMC911X_BASE + 0x20) +#define TX_CMD_A_INT_ON_COMP (0x80000000) +#define TX_CMD_A_INT_BUF_END_ALGN (0x03000000) +#define TX_CMD_A_INT_4_BYTE_ALGN (0x00000000) +#define TX_CMD_A_INT_16_BYTE_ALGN (0x01000000) +#define TX_CMD_A_INT_32_BYTE_ALGN (0x02000000) +#define TX_CMD_A_INT_DATA_OFFSET (0x001F0000) +#define TX_CMD_A_INT_FIRST_SEG (0x00002000) +#define TX_CMD_A_INT_LAST_SEG (0x00001000) +#define TX_CMD_A_BUF_SIZE (0x000007FF) +#define TX_CMD_B_PKT_TAG (0xFFFF0000) +#define TX_CMD_B_ADD_CRC_DISABLE (0x00002000) +#define TX_CMD_B_DISABLE_PADDING (0x00001000) +#define TX_CMD_B_PKT_BYTE_LENGTH (0x000007FF)
Register and bitfield definitions should be in a header file. More generally, only register addresses and bitfields should be defined. Using macros to encapsulate both address and function is bad form, IMHO. More on that later. <snip>
+#define DRIVERNAME "smc911x"
+u32 smc911x_get_mac_csr(u8 reg) +{
- while (MAC_CSR_CMD & MAC_CSR_CMD_CSR_BUSY);
Using macros like this is both unreadable and hard to debug. Instead, consider something like:
while (reg_read(MAC_CSR) & MAC_CSR_BUSY));
IMHO, one-liner while loops are bad too, but that's debatable.
I haven't even gotten into the functionality, because I think there's a lot of work to be done just in coding style before we look at substance.
regards, Ben