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 v2 1/3 net: Adds Fast Ethernet Controller driver for Armada100
Quoted 9 lines · ----- "Marek Vasut
Quoted 5 lines · Ajay Bhargav
This patch adds support for Fast Ethernet Controller driver for Armada100 series.
Signed-off-by Ajay Bhargav <[email protected]>---
[...]
[...]
Quoted 4 lines
Quoted 4 lines
(quoted diff) +4
-0+ +static int armdfec_phy_timeout(u32 reg, u32 flag, int cond) +{ + u32 timeout = PHY_WAIT_ITERATIONS;
Quoted 9 lines
Review
Quoted 4 lines
(quoted diff) +4
-0+ while (--timeout) { + if (cond && (readl(reg) & flag)) + break; + else if (!cond && !(readl(reg) & flag))You can read the register into some temporary variable so you don't need the
readl()at two places.
readl will be called only once... do i really need a temp var?
Quoted 29 lines
Review
Quoted 7 lines
(quoted diff) +7
-0+ break; + udelay(PHY_WAIT_MICRO_SECONDS); + } + return !timeout; +} + +static int smi_reg_read(const char *devname, u8 phy_addr, u8
phy_reg,Quoted 21 lines
(quoted diff) +21
-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); + *value = (u16) (reg_data & 0x1f); + return 0; + } + + /* check parameters */ + if (phy_addr > PHY_MASK) { + printf("Err..(%s) Invalid phy address: 0x%X\n", + __func__, phy_addr); + return -EINVAL; + } + if (phy_reg > PHY_MASK) { + printf("Err..(%s) Invalid register offset: 0x%X\n",
Quoted 9 lines
Review
Quoted 6 lines
(quoted diff) +6
-0+ __func__, phy_reg); + return -EINVAL; + } + + /* wait for the SMI register to become available */ + if (armdfec_phy_timeout((u32)®s->smi, SMI_BUSY, FALSE)) {Adjust the function so you don't need the cast.
okay...
Quoted 8 lines
Review
Quoted 5 lines
(quoted diff) +5
-0+ printf("Error (%s) PHY busy timeout\n", __func__); + return -1; + } + + writel(phy_addr << 16 | phy_reg << 21 | SMI_OP_R, ®s->smi);Parentheses missing maybe ?
:) okay..
Quoted 64 lines
Quoted 32 lines
(quoted diff) +32
-0+ + /* now wait for the data to be valid */ + if (armdfec_phy_timeout((u32)®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__);Review
Quoted 10 lines
(quoted diff) +10
-0+ return -EINVAL; + } + + /* wait for the SMI register to become available */ + if (armdfec_phy_timeout((u32)®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),Quoted 21 lines
(quoted diff) +21
-0+ ®s->smi); + return 0; +} + +/* + * Abort any transmit and receive operations and put DMA + * in idle state. AT and AR bits are cleared upon entering + * 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; + + do { + writel(SDMA_CMD_AR | SDMA_CMD_AT, ®s->sdma_cmd); + udelay(100); + + delay = 10;
Quoted 9 lines
Review
Quoted 6 lines
(quoted diff) +6
-0+ while ((readl(®s->sdma_cmd) & + (SDMA_CMD_AR | SDMA_CMD_AT)) + && delay-- > 0) { + udelay(10); + } + } while (maxretries-- > 0 && delay <= 0);Didn't I comment on this one in V1?
Yes you did...
Quoted 27 lines
Quoted 4 lines
(quoted diff) +4
-0+ + if (maxretries <= 0) + printf("%s : DMA Stuck\n", __func__); +}[...]
Quoted 20 lines
(quoted diff) +20
-0+int armada100_fec_register() +{ + struct armdfec_device *darmdfec; + struct eth_device *dev; + int phy_adr; + + darmdfec = malloc(sizeof(struct armdfec_device)); + if (!darmdfec) + goto error; + + memset(darmdfec, 0, sizeof(struct armdfec_device)); + + darmdfec->htpr = memalign(8, HASH_ADDR_TABLE_SIZE); + if (!darmdfec->htpr) + goto error; + + darmdfec->p_rxdesc = (struct rx_desc *) memalign(PKTALIGN, + ARMDFEC_RXQ_DESC_ALIGNED_SIZE * RINGSZ + 1); + + if (!darmdfec->p_rxdesc)
Quoted 9 lines
Review
Quoted 4 lines
(quoted diff) +4
-0+ goto error; + + darmdfec->p_rxbuf = (u8 *) memalign(PKTALIGN, + RINGSZ * PKTSIZE_ALIGN + 1);Is the cast needed ? You should review your casts, most of them can be solved by
using proper type!
Thats a serious issue with me I guess :)
Quoted 9 lines
Quoted 9 lines
(quoted diff) +9
-0+ if (!darmdfec->p_rxbuf) + goto error; + + darmdfec->p_aligned_txbuf = memalign(8, PKTSIZE_ALIGN); + if (!darmdfec->p_aligned_txbuf) + goto error; + + darmdfec->p_txdesc = (struct tx_desc *) + memalign(PKTALIGN, sizeof(struct tx_desc) + 1);
Quoted 9 lines
Review
Quoted 6 lines
(quoted diff) +6
-0+ if (!darmdfec->p_txdesc) + goto error; + + dev = &darmdfec->dev; + /* Assign ARMADA100 Fast Ethernet Controller Base Address */ + darmdfec->regs = (void *) ARMD1_FEC_BASE;Cheers!
Thanks,
Ajay Bhargav
PATCH v2 1/3 net: Adds Fast Ethernet Controller driver for Armada100
Quoted 25 lines · Ajay Bhargav
Quoted 8 lines · ----- "Marek Vasut
Quoted 5 lines · Ajay Bhargav
This patch adds support for Fast Ethernet Controller driver for Armada100 series.
Signed-off-by Ajay Bhargav <[email protected]>---
[...]
[...]
Quoted 4 lines
Quoted 4 lines
(quoted diff) +4
-0+ +static int armdfec_phy_timeout(u32 reg, u32 flag, int cond) +{ + u32 timeout = PHY_WAIT_ITERATIONS;Quoted 8 lines
Review
Quoted 4 lines
(quoted diff) +4
-0+ while (--timeout) { + if (cond && (readl(reg) & flag)) + break; + else if (!cond && !(readl(reg) & flag))You can read the register into some temporary variable so you don't need the
readl()at two places.
Quoted 2 lines
readl will be called only once... do i really need a temp var?
I think it'll help the readability a bit.
Cheers