Skip to contentSkip to conversation

net: Adds Fast Ethernet Controller driver for Armada100

PATCH Superseded Ajay Bhargav 2 participants 2 messages Aug 25, 2011 1 message not in the archive

v2 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

#1 lore raw

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.

---

[...]

[...]

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(&regs->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)&regs->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, &regs->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)&regs->smi, SMI_R_VALID, TRUE)) {
+		val = readl(&regs->smi);
+		printf("Err (%s) PHY Read timeout, val=0x%x\n", __func__, val);
+		return -1;
+	}
+	val = readl(&regs->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(&regs->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)&regs->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
+			&regs->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, &regs->sdma_cmd);
+		udelay(100);
+
+		delay = 10;
Quoted 9 lines

Review

Quoted 6 lines
(quoted diff) +6 -0
+		while ((readl(&regs->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

#2 lore raw

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.

---

[...]

[...]

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