Re: [PATCH 4/4] net/fec: add imx6q enet support

On Sun, Sep 18, 2011 at 08:09:12PM +0200, Francois Romieu wrote:

Shawn Guo [off-list ref] :
[...]

quoted

diff --git a/drivers/net/fec.c b/drivers/net/fec.c
index 04206e4..849cb0b 100644
--- a/drivers/net/fec.c
+++ b/drivers/net/fec.c

@@ -442,18 +453,23 @@ fec_restart(struct net_device *ndev, int duplex)
 		/* Enable flow control and length check */
 		rcntl |= 0x40000000 | 0x00000020;
 
-		/* MII or RMII */
+		/* RGMII, RMII or MII */
+		if (fep->phy_interface == PHY_INTERFACE_MODE_RGMII)
+			rcntl |= (1 << 6);
 		if (fep->phy_interface == PHY_INTERFACE_MODE_RMII)

           ^^^^ missing "else"

Yes, my bad.

[...]

quoted

+		/* 1G, 100M or 10M */
+		if (fep->phy_dev) {
+			if (fep->phy_dev->speed == SPEED_1000)
+				ecntl |= (1 << 8);

Right, this is a typo.  It should be (1 << 5);

quoted

+			else if (fep->phy_dev->speed == SPEED_100)
+				rcntl &= ~(1 << 9);
+			else
+				rcntl |= (1 << 9);
+		}

[...]

quoted

+	if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) {
+		/* enable ENET endian swap */
+		ecntl |= (1 << 8);

I do not understand why the endian swap bit of ecntl needs to be
set the same in these two different paths, especially as the latter
handles the old faulty imx28 and the former the newly fixed imx6q.
Typo ?

Nice catches.  Thanks a lot, Ueimor.

-- 
Regards,
Shawn