net: Adds Fast Ethernet Controller driver for Armada100
Subsystems (2)
PATCH v3 1/3 net: Adds Fast Ethernet Controller driver for Armada100
This patch adds support for Fast Ethernet Controller driver for Armada100 series.
---
Changes for v2:
- removed random MAC generation
- driver init function changed to register as per new naming convention
- code cleanup (Thanks to Wolfgang, Marek & Mike for tips)
Changes for v3:
- code cleanup
arch/arm/include/asm/arch-armada100/armada100.h | 1 + drivers/net/Makefile | 1 + drivers/net/armada100_fec.c | 759 +++++++++++++++++++++++ drivers/net/armada100_fec.h | 232 +++++++ include/netdev.h | 1 + 5 files changed, 994 insertions(+), 0 deletions(-) create mode 100644 drivers/net/armada100_fec.c create mode 100644 drivers/net/armada100_fec.h
arch/arm/include/asm/arch-armada100/armada100.h +1 -0
@@ -59,6 +59,7 @@#define ARMD1_MPMU_BASE 0xD4050000#define ARMD1_APMU_BASE 0xD4282800#define ARMD1_CPU_BASE 0xD4282C00 +#define ARMD1_FEC_BASE 0xC0800000/* * Main Power Management (MPMU) Registers
drivers/net/Makefile +1 -0
@@ -28,6 +28,7 @@ LIB := $(obj)libnet.o COBJS-$(CONFIG_DRIVER_3C589) += 3c589.o COBJS-$(CONFIG_PPC4xx_EMAC) += 4xx_enet.o COBJS-$(CONFIG_ALTERA_TSE) += altera_tse.o +COBJS-$(CONFIG_ARMADA100_FEC) += armada100_fec.o COBJS-$(CONFIG_DRIVER_AT91EMAC) += at91_emac.o COBJS-$(CONFIG_DRIVER_AX88180) += ax88180.o COBJS-$(CONFIG_BCM570x) += bcm570x.o
drivers/net/armada100_fec.c new file +759 -0
759 lines not on this page · raw message
drivers/net/armada100_fec.h new file +232 -0
232 lines not on this page · raw message
include/netdev.h +1 -0
@@ -94,6 +94,7 @@ int xilinx_emaclite_initialize (bd_t *bis, int base_addr);int sh_eth_initialize (bd_t *bis);int dm9000_initialize (bd_t *bis);int fecmxc_initialize (bd_t *bis); +int armada100_fec_register (int base_addr);/* Boards with PCI network controllers can call this from their board_eth_init() * function to initialize whatever's on board.
Signature · 1 line
1.7.0.4
PATCH v3 1/3 net: Adds Fast Ethernet Controller driver for Armada100
Quoted 4 lines · Ajay Bhargav
This patch adds support for Fast Ethernet Controller driver for Armada100 series.
Signed-off-by Ajay Bhargav <[email protected]>
Hi, please don't forget to CC me next time ;-)
[...]
drivers/net/armada100_fec.c line 86
Quoted 11 lines
drivers/net/armada100_fec.c new file +11
-0+static int smi_reg_read (const char *devname,u8 phy_addr,u8 phy_reg, +u16 *value) +{ +struct eth_device *dev =eth_get_dev_by_name (devname); +struct armdfec_device *darmdfec =to_darmdfec (dev); +struct armdfec_reg *regs = darmdfec->regs; +u32 val, reg_data; + +if (phy_addr ==PHY_ADR_REQ && phy_reg ==PHY_ADR_REQ ) { + reg_data =readl (®s->phyadr); + *value = (u16) (reg_data &0x1f );
Do you need this cast?
drivers/net/armada100_fec.c line 93
Quoted 7 lines
drivers/net/armada100_fec.c new file +7
-0+return 0 ; + } + +/* check parameters */ +if (phy_addr >PHY_MASK ) { +printf ("Err..(%s) Invalid phy address: 0x%X\n" , + __func__, phy_addr);
Maybe a firm "A100 FEC: Invalid phy address (address = 0x%x)\n" would be better, fix that "Err.." and "Err " globally please.
Quoted 64 lines
drivers/net/armada100_fec.c new file +64
-0+return -EINVAL ; + } +if (phy_reg >PHY_MASK ) { +printf ("Err..(%s) Invalid register offset: 0x%X\n" , + __func__, phy_reg); +return -EINVAL ; + } + +/* wait for the SMI register to become available */ +if (armdfec_phy_timeout (®s->smi,SMI_BUSY , FALSE)) { +printf ("Error (%s) PHY busy timeout\n" , __func__); +return -1 ; + } + +writel ((phy_addr <<16 ) | (phy_reg <<21 ) |SMI_OP_R , ®s->smi); + +/* now wait for the data to be valid */ +if (armdfec_phy_timeout (®s->smi,SMI_R_VALID , TRUE)) { + val =readl (®s->smi); +printf ("Err (%s) PHY Read timeout, val=0x%x\n" , __func__, val); +return -1 ; + } + val =readl (®s->smi); + *value = val &0xffff ; + +return 0 ; +} + +static int smi_reg_write (const char *devname, +u8 phy_addr,u8 phy_reg,u16 value) +{ +struct eth_device *dev =eth_get_dev_by_name (devname); +struct armdfec_device *darmdfec =to_darmdfec (dev); +struct armdfec_reg *regs = darmdfec->regs; + +if (phy_addr ==PHY_ADR_REQ && phy_reg ==PHY_ADR_REQ ) { +clrsetbits_le32 (®s->phyadr,0x1f , value &0x1f ); +return 0 ; + } + +/* check parameters */ +if (phy_addr >PHY_MASK ) { +printf ("Err..(%s) Invalid phy address\n" , __func__); +return -EINVAL ; + } +if (phy_reg >PHY_MASK ) { +printf ("Err..(%s) Invalid register offset\n" , __func__); +return -EINVAL ; + } + +/* wait for the SMI register to become available */ +if (armdfec_phy_timeout (®s->smi,SMI_BUSY , FALSE)) { +printf ("Error (%s) PHY busy timeout\n" , __func__); +return -1 ; + } + +writel ((phy_addr <<16 ) | (phy_reg <<21 ) |SMI_OP_W | (value &0xffff ), + ®s->smi); +return 0 ; +} + +/* +* Abort any transmit and receive operations and put DMA +* in idle state. AT and AR bits are cleared upon entering
drivers/net/armada100_fec.c line 167
Quoted 10 lines
drivers/net/armada100_fec.c new file +10
-0+* in IDLE state. So poll those bits to verify operation. +*/ +static void abortdma (struct eth_device *dev) +{ +struct armdfec_device *darmdfec =to_darmdfec (dev); +struct armdfec_reg *regs = darmdfec->regs; +int delay; +int maxretries =40 ; +u32 tmp; +
while (--maxretries) {
That way it won't get negative and you can check below for if (!maxretries)
drivers/net/armada100_fec.c line 173
Quoted 6 lines
drivers/net/armada100_fec.c new file +6
-0+while (maxretries--) { +writel (SDMA_CMD_AR |SDMA_CMD_AT , ®s->sdma_cmd); +udelay (100 ); + + delay =10 ; +while (delay--) {
This will go negative and the check below will be true for -1.Fix this to while (--delay) {
drivers/net/armada100_fec.c line 185
Quoted 12 lines
drivers/net/armada100_fec.c new file +12
-0+ tmp =readl (®s->sdma_cmd); +if (!(tmp & (SDMA_CMD_AR |SDMA_CMD_AT ))) +break ; +udelay (10 ); + } +if (delay) +break ; + } + +if (maxretries <=0 ) +printf ("%s : DMA Stuck\n" , __func__); +}
See two comments above.
Quoted 125 lines
drivers/net/armada100_fec.c new file +125
-0+ +static inline u32 nibble_swapping_32_bit (u32 x) +{ +return (((x) &0xf0f0f0f0 ) >>4 ) | (((x) &0x0f0f0f0f ) <<4 ); +} + +static inline u32 nibble_swapping_16_bit (u32 x) +{ +return (((x) &0x0000f0f0 ) >>4 ) | (((x) &0x00000f0f ) <<4 ); +} + +static inline u32 flip_4_bits (u32 x) +{ +return (((x) &0x01 ) <<3 ) | (((x) &0x002 ) <<1 ) + | (((x) &0x04 ) >>1 ) | (((x) &0x008 ) >>3 ); +} + +/* +* This function will calculate the hash function of the address. +* depends on the hash mode and hash size. +* Inputs +* mach - the 2 most significant bytes of the MAC address. +* macl - the 4 least significant bytes of the MAC address. +* Outputs +* return the calculated entry. +*/ +static u32 hash_function (u32 mach,u32 macl) +{ +u32 hashresult; +u32 addrh; +u32 addrl; +u32 addr0; +u32 addr1; +u32 addr2; +u32 addr3; +u32 addrhswapped; +u32 addrlswapped; + + addrh =nibble_swapping_16_bit (mach); + addrl =nibble_swapping_32_bit (macl); + + addrhswapped =flip_4_bits (addrh &0xf ) + + ((flip_4_bits ((addrh >>4 ) &0xf )) <<4 ) + + ((flip_4_bits ((addrh >>8 ) &0xf )) <<8 ) + + ((flip_4_bits ((addrh >>12 ) &0xf )) <<12 ); + + addrlswapped =flip_4_bits (addrl &0xf ) + + ((flip_4_bits ((addrl >>4 ) &0xf )) <<4 ) + + ((flip_4_bits ((addrl >>8 ) &0xf )) <<8 ) + + ((flip_4_bits ((addrl >>12 ) &0xf )) <<12 ) + + ((flip_4_bits ((addrl >>16 ) &0xf )) <<16 ) + + ((flip_4_bits ((addrl >>20 ) &0xf )) <<20 ) + + ((flip_4_bits ((addrl >>24 ) &0xf )) <<24 ) + + ((flip_4_bits ((addrl >>28 ) &0xf )) <<28 ); + + addrh = addrhswapped; + addrl = addrlswapped; + + addr0 = (addrl >>2 ) &0x03f ; + addr1 = (addrl &0x003 ) | (((addrl >>8 ) &0x7f ) <<2 ); + addr2 = (addrl >>15 ) &0x1ff ; + addr3 = ((addrl >>24 ) &0x0ff ) | ((addrh &1 ) <<8 ); + + hashresult = (addr0 <<9 ) | (addr1 ^ addr2 ^ addr3); + hashresult = hashresult &0x07ff ; +return hashresult; +} + +/* +* This function will add an entry to the address table. +* depends on the hash mode and hash size that was initialized. +* Inputs +* mach - the 2 most significant bytes of the MAC address. +* macl - the 4 least significant bytes of the MAC address. +* skip - if 1, skip this address. +* rd - the RD field in the address table. +* Outputs +* address table entry is added. +* 0 if success. +* -ENOSPC if table full +*/ +static int add_del_hash_entry (struct armdfec_device *darmdfec,u32 mach, +u32 macl,u32 rd,u32 skip,int del) +{ +struct addr_table_entry_t *entry, *start; +u32 newhi; +u32 newlo; +u32 i; +u8 *last; + + newlo = (((mach >>4 ) &0xf ) <<15 ) + | (((mach >>0 ) &0xf ) <<11 ) + | (((mach >>12 ) &0xf ) <<7 ) + | (((mach >>8 ) &0xf ) <<3 ) + | (((macl >>20 ) &0x1 ) <<31 ) + | (((macl >>16 ) &0xf ) <<27 ) + | (((macl >>28 ) &0xf ) <<23 ) + | (((macl >>24 ) &0xf ) <<19 ) + | (skip <<HTESKIP ) | (rd <<HTERDBIT ) + |HTEVALID ; + + newhi = (((macl >>4 ) &0xf ) <<15 ) + | (((macl >>0 ) &0xf ) <<11 ) + | (((macl >>12 ) &0xf ) <<7 ) + | (((macl >>8 ) &0xf ) <<3 ) + | (((macl >>21 ) &0x7 ) <<0 ); + +/* +* Pick the appropriate table, start scanning for free/reusable +* entries at the index obtained by hashing the specified MAC address +*/ + start = (struct addr_table_entry_t *) (darmdfec->htpr); + entry = start +hash_function (mach, macl); +for (i =0 ; i <HOP_NUMBER ; i++) { +if (!(entry->lo &HTEVALID )) { +break ; + }else { +/* if same address put in same position */ +if (((entry->lo &0xfffffff8 ) == (newlo &0xfffffff8 )) + && (entry->hi == newhi)) +break ; + } +if (entry == start +0x7ff ) + entry = start; +else
drivers/net/armada100_fec.c line 320
Quoted 10 lines
drivers/net/armada100_fec.c new file +10
-0+ entry++; + } + +if (((entry->lo &0xfffffff8 ) != (newlo &0xfffffff8 )) && + (entry->hi != newhi) && del) +return 0 ; + +if (i ==HOP_NUMBER ) { +if (!del) { +printf ("%s: table section is full\n" , __FILE__);
Unify the error reporting please.
It looks good, just a few nits
Cheers!
PATCH v3 1/3 net: Adds Fast Ethernet Controller driver for Armada100
In reply to Ajay Bhargavdrivers/net/armada100_fec.c line 274
Quoted 4 lines · On Friday, August 26, 2011 02:36:51 Ajay Bhargav wrote:
drivers/net/armada100_fec.c new file +4
-0+static int add_del_hash_entry (struct armdfec_device *darmdfec,u32 mach, +u32 macl,u32 rd,u32 skip,int del) +{ +u8 *last;
local var ...
drivers/net/armada100_fec.c line 342
Quoted 5 lines
drivers/net/armada100_fec.c new file +5
-0+ last = (u8 *) entry; + last = last +sizeof (*entry); + +return 0 ; +}
so what's the point of these two assignments to "last" ?
Quoted 1 line
+int
armada100_fec_register(int base_addr)
when it comes to addresses for memory mapped registers, we typically use "unsigned long" rather than "int"
drivers/net/armada100_fec.c line 706
Quoted 3 lines
drivers/net/armada100_fec.c new file +3
-0+ darmdfec =malloc (sizeof (struct armdfec_device )); +if (!darmdfec) + goto error;
if this first one fails, we jump to:
drivers/net/armada100_fec.c line 755
Quoted 5 lines
drivers/net/armada100_fec.c new file +5
-0+error: +free (darmdfec->p_aligned_txbuf); +free (darmdfec->p_rxbuf); +free (darmdfec->p_rxdesc); +free (darmdfec->htpr);
looks like 4 NULL pointer derefs. so you'll need one specific path for the first malloc(), but the rest are fine.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110826/c1c15b94/attachment.pgp