net: Adds Fast Ethernet Controller driver for Armada100
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH v4 1/3 net: Adds Fast Ethernet Controller driver for Armada100
Quoted 10 lines · ----- "Marek Vasut
Quoted 4 lines · Ajay Bhargav
This patch adds support for Fast Ethernet Controller driver for Armada100 series.
Signed-off-by Ajay Bhargav <[email protected]>[...]
Quoted 1 line
+static int smi_reg_read(const char *devname, u8
phy_addr, u8
phy_reg,
Quoted 13 lines
Review
Quoted 9 lines
(quoted diff) +9
-0+ 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);You use "
reg_data" here and "val" below ... can't you use just one variable?
Yes.. sorry for mistake.
Quoted 48 lines
[...]
Quoted 1 line
+static int add_del_hash_entry(struct
armdfec_device*darmdfec, u32mach,
Quoted 17 lines
(quoted diff) +17
-0+ u32 macl, u32 rd, u32 skip, int del) +{ + struct addr_table_entry_t *entry, *start; + u32 newhi; + u32 newlo; + u32 i; + + 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;Review
Quoted 10 lines
(quoted diff) +10
-0+ + 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 MACaddress
Review
Quoted 12 lines
(quoted diff) +12
-0+ */ + 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; + }What about
if (!(entry->lo & HTEVALID))
Quoted 9 lines
break;if (((
entry->lo&0xfffffff8) == (newlo &0xfffffff8))&& (entry->hi == newhi)) { break; }? :-)
No.. if entry is valid, we should not modify it anyways. Deleting a valid entry might break the hash chain.
Quoted 5 lines
Quoted 5 lines
(quoted diff) +5
-0+ if (entry == start + 0x7ff) + entry = start; + else + entry++; + }
Quoted 9 lines
Review
Quoted 4 lines
(quoted diff) +4
-0+ + if (((entry->lo & 0xfffffff8) != (newlo & 0xfffffff8)) && + (entry->hi != newhi) && del) + return 0;Now thinking of it, are you sure about this condition ? Shouldn't the first && be
|| instead ? And there should be parenthesis around that ?
if delete is requested for an address which is not present in chain, we should return immideatly. so && is fine there.
Quoted 26 lines
Quoted 10 lines
(quoted diff) +10
-0+ + if (i == HOP_NUMBER) { + if (!del) { + printf("ARMD100 FEC: (%s) table section is full\n", + __func__); + return -ENOSPC; + } else { + return 0; + } + }[...]
Review
Quoted 2 lines
(quoted diff) +2
-0+ + /* 64 should work but does not -- dhcp packets NEVER gettransmitted. */
What's this about ?
Quoted 2 lines
(quoted diff) +2
-0+ if ((mtu > MAX_PKT_SIZE) || (mtu < 64)) + return -EINVAL;[...]
Quoted 1 line
+static int armdfec_send(struct
eth_device*dev, volatile void*dataptr,
Quoted 9 lines
Review
Quoted 6 lines
(quoted diff) +6
-0+ int datasize) +{ + struct armdfec_device *darmdfec = to_darmdfec(dev); + struct armdfec_reg *regs = darmdfec->regs; + struct tx_desc *p_txdesc = darmdfec->p_txdesc; + void *p = (void *) dataptr;Is this needed?
If I use dataptr directly, sending doesnt work...
Quoted 8 lines
Review
Quoted 5 lines
(quoted diff) +5
-0+ int retry = PHY_WAIT_ITERATIONS * PHY_WAIT_MICRO_SECONDS; + u32 cmd_sts; + + /* Copy buffer if it's misaligned */ + if ((u32) dataptr & 0x07) {Space after typecast. Please fix globally.
will do that..
Quoted 11 lines
Review
Quoted 8 lines
(quoted diff) +8
-0+ if (datasize > PKTSIZE_ALIGN) { + printf("ARMD100 FEC: Non-aligned data too large (%d)\n", + datasize); + return -1; + } + memcpy(darmdfec->p_aligned_txbuf, p, datasize); + p = darmdfec->p_aligned_txbuf; + }The rest is really good !
Quoted 4 lines
Thanks!
Cheers
Thanks,
Ajay Bhargav