----- "Marek Vasut" marek.vasut@gmail.com wrote:
On Tuesday, August 30, 2011 07:44:40 AM Ajay Bhargav wrote:
This patch adds support for Fast Ethernet Controller driver for Armada100 series.
Signed-off-by: Ajay Bhargav ajay.bhargav@einfochips.com
[...]
+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);You use "reg_data" here and "val" below ... can't you use just one variable?
Yes.. sorry for mistake.
[...]
+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;
- 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 MACaddress
*/- 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)) 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.
if (entry == start + 0x7ff)entry = start;elseentry++;- }
- 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.
- if (i == HOP_NUMBER) {
if (!del) {printf("ARMD100 FEC: (%s) table section is full\n",__func__);return -ENOSPC;} else {return 0;}- }
[...]
- /* 64 should work but does not -- dhcp packets NEVER get
transmitted. */
What's this about ?
- if ((mtu > MAX_PKT_SIZE) || (mtu < 64))
return -EINVAL;[...]
+static int armdfec_send(struct eth_device *dev, volatile void
*dataptr,
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...
- 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..
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 !
Thanks!
Cheers
Thanks, Ajay Bhargav