* [PATCH 0/4] drivers/net/fec: Some cleanups and fix for DUAL MAC mode
@ 2011-03-21 16:37 Lothar Waßmann
2011-03-21 16:37 ` [PATCH 1/4] drivers/net/fec: Cleanup Lothar Waßmann
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: Lothar Waßmann @ 2011-03-21 16:37 UTC (permalink / raw)
To: netdev; +Cc: u.kleine-koenig, Shawn Guo, Lothar Waßmann
patch 1/4: whitespace cleanup and spelling fixes
patch 2/4: use constants instead of bare numbers for register writes
patch 3/4: make dual MAC mode actually work
patch 4/4: take MAC address for second device from platform_data
instead of using first MAC address + 1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/4] drivers/net/fec: Cleanup
2011-03-21 16:37 [PATCH 0/4] drivers/net/fec: Some cleanups and fix for DUAL MAC mode Lothar Waßmann
@ 2011-03-21 16:37 ` Lothar Waßmann
2011-03-21 16:37 ` [PATCH 2/4] drivers/net/fec: Use constants instead of magic numbers Lothar Waßmann
2011-03-22 2:49 ` [PATCH 1/4] drivers/net/fec: Cleanup Shawn Guo
2011-03-22 10:00 ` [PATCH 3/4] drivers/net/fec: Fix dual MAC support on i.MX28 Lothar Waßmann
2011-03-22 10:00 ` [PATCH 4/4] drivers/net/fec: Don't mess with configured MAC addresses Lothar Waßmann
2 siblings, 2 replies; 17+ messages in thread
From: Lothar Waßmann @ 2011-03-21 16:37 UTC (permalink / raw)
To: netdev; +Cc: u.kleine-koenig, Shawn Guo, Lothar Waßmann
- Whitespace cleanup
- Convert indentation spaces to TABs
- Spelling fixes
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
drivers/net/fec.c | 9 +++---
drivers/net/fec.h | 78 ++++++++++++++++++++++++++--------------------------
2 files changed, 43 insertions(+), 44 deletions(-)
diff --git a/drivers/net/fec.c b/drivers/net/fec.c
index 885d8ba..f00ee7f 100644
--- a/drivers/net/fec.c
+++ b/drivers/net/fec.c
@@ -96,7 +96,7 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
#elif defined (CONFIG_M5272C3)
#define FEC_FLASHMAC (0xffe04000 + 4)
#elif defined(CONFIG_MOD5272)
-#define FEC_FLASHMAC 0xffc0406b
+#define FEC_FLASHMAC 0xffc0406b
#else
#define FEC_FLASHMAC 0
#endif
@@ -210,10 +210,10 @@ struct fec_enet_private {
#define FEC_MMFR_ST (1 << 30)
#define FEC_MMFR_OP_READ (2 << 28)
#define FEC_MMFR_OP_WRITE (1 << 28)
-#define FEC_MMFR_PA(v) ((v & 0x1f) << 23)
-#define FEC_MMFR_RA(v) ((v & 0x1f) << 18)
+#define FEC_MMFR_PA(v) (((v) & 0x1f) << 23)
+#define FEC_MMFR_RA(v) (((v) & 0x1f) << 18)
#define FEC_MMFR_TA (2 << 16)
-#define FEC_MMFR_DATA(v) (v & 0xffff)
+#define FEC_MMFR_DATA(v) ((v) & 0xffff)
#define FEC_MII_TIMEOUT 1000 /* us */
@@ -472,7 +472,6 @@ fec_stop(struct net_device *ndev)
writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
}
-
static void
fec_timeout(struct net_device *ndev)
{
diff --git a/drivers/net/fec.h b/drivers/net/fec.h
index ace318d..8697556 100644
--- a/drivers/net/fec.h
+++ b/drivers/net/fec.h
@@ -97,51 +97,51 @@ struct bufdesc {
* The following definitions courtesy of commproc.h, which where
* Copyright (c) 1997 Dan Malek (dmalek@jlc.net).
*/
-#define BD_SC_EMPTY ((ushort)0x8000) /* Recieve is empty */
-#define BD_SC_READY ((ushort)0x8000) /* Transmit is ready */
-#define BD_SC_WRAP ((ushort)0x2000) /* Last buffer descriptor */
-#define BD_SC_INTRPT ((ushort)0x1000) /* Interrupt on change */
-#define BD_SC_CM ((ushort)0x0200) /* Continous mode */
-#define BD_SC_ID ((ushort)0x0100) /* Rec'd too many idles */
-#define BD_SC_P ((ushort)0x0100) /* xmt preamble */
-#define BD_SC_BR ((ushort)0x0020) /* Break received */
-#define BD_SC_FR ((ushort)0x0010) /* Framing error */
-#define BD_SC_PR ((ushort)0x0008) /* Parity error */
-#define BD_SC_OV ((ushort)0x0002) /* Overrun */
-#define BD_SC_CD ((ushort)0x0001) /* ?? */
+#define BD_SC_EMPTY ((ushort)0x8000) /* Receive is empty */
+#define BD_SC_READY ((ushort)0x8000) /* Transmit is ready */
+#define BD_SC_WRAP ((ushort)0x2000) /* Last buffer descriptor */
+#define BD_SC_INTRPT ((ushort)0x1000) /* Interrupt on change */
+#define BD_SC_CM ((ushort)0x0200) /* Continuous mode */
+#define BD_SC_ID ((ushort)0x0100) /* Rec'd too many idles */
+#define BD_SC_P ((ushort)0x0100) /* xmt preamble */
+#define BD_SC_BR ((ushort)0x0020) /* Break received */
+#define BD_SC_FR ((ushort)0x0010) /* Framing error */
+#define BD_SC_PR ((ushort)0x0008) /* Parity error */
+#define BD_SC_OV ((ushort)0x0002) /* Overrun */
+#define BD_SC_CD ((ushort)0x0001) /* ?? */
/* Buffer descriptor control/status used by Ethernet receive.
*/
-#define BD_ENET_RX_EMPTY ((ushort)0x8000)
-#define BD_ENET_RX_WRAP ((ushort)0x2000)
-#define BD_ENET_RX_INTR ((ushort)0x1000)
-#define BD_ENET_RX_LAST ((ushort)0x0800)
-#define BD_ENET_RX_FIRST ((ushort)0x0400)
-#define BD_ENET_RX_MISS ((ushort)0x0100)
-#define BD_ENET_RX_LG ((ushort)0x0020)
-#define BD_ENET_RX_NO ((ushort)0x0010)
-#define BD_ENET_RX_SH ((ushort)0x0008)
-#define BD_ENET_RX_CR ((ushort)0x0004)
-#define BD_ENET_RX_OV ((ushort)0x0002)
-#define BD_ENET_RX_CL ((ushort)0x0001)
-#define BD_ENET_RX_STATS ((ushort)0x013f) /* All status bits */
+#define BD_ENET_RX_EMPTY ((ushort)0x8000)
+#define BD_ENET_RX_WRAP ((ushort)0x2000)
+#define BD_ENET_RX_INTR ((ushort)0x1000)
+#define BD_ENET_RX_LAST ((ushort)0x0800)
+#define BD_ENET_RX_FIRST ((ushort)0x0400)
+#define BD_ENET_RX_MISS ((ushort)0x0100)
+#define BD_ENET_RX_LG ((ushort)0x0020)
+#define BD_ENET_RX_NO ((ushort)0x0010)
+#define BD_ENET_RX_SH ((ushort)0x0008)
+#define BD_ENET_RX_CR ((ushort)0x0004)
+#define BD_ENET_RX_OV ((ushort)0x0002)
+#define BD_ENET_RX_CL ((ushort)0x0001)
+#define BD_ENET_RX_STATS ((ushort)0x013f) /* All status bits */
/* Buffer descriptor control/status used by Ethernet transmit.
*/
-#define BD_ENET_TX_READY ((ushort)0x8000)
-#define BD_ENET_TX_PAD ((ushort)0x4000)
-#define BD_ENET_TX_WRAP ((ushort)0x2000)
-#define BD_ENET_TX_INTR ((ushort)0x1000)
-#define BD_ENET_TX_LAST ((ushort)0x0800)
-#define BD_ENET_TX_TC ((ushort)0x0400)
-#define BD_ENET_TX_DEF ((ushort)0x0200)
-#define BD_ENET_TX_HB ((ushort)0x0100)
-#define BD_ENET_TX_LC ((ushort)0x0080)
-#define BD_ENET_TX_RL ((ushort)0x0040)
-#define BD_ENET_TX_RCMASK ((ushort)0x003c)
-#define BD_ENET_TX_UN ((ushort)0x0002)
-#define BD_ENET_TX_CSL ((ushort)0x0001)
-#define BD_ENET_TX_STATS ((ushort)0x03ff) /* All status bits */
+#define BD_ENET_TX_READY ((ushort)0x8000)
+#define BD_ENET_TX_PAD ((ushort)0x4000)
+#define BD_ENET_TX_WRAP ((ushort)0x2000)
+#define BD_ENET_TX_INTR ((ushort)0x1000)
+#define BD_ENET_TX_LAST ((ushort)0x0800)
+#define BD_ENET_TX_TC ((ushort)0x0400)
+#define BD_ENET_TX_DEF ((ushort)0x0200)
+#define BD_ENET_TX_HB ((ushort)0x0100)
+#define BD_ENET_TX_LC ((ushort)0x0080)
+#define BD_ENET_TX_RL ((ushort)0x0040)
+#define BD_ENET_TX_RCMASK ((ushort)0x003c)
+#define BD_ENET_TX_UN ((ushort)0x0002)
+#define BD_ENET_TX_CSL ((ushort)0x0001)
+#define BD_ENET_TX_STATS ((ushort)0x03ff) /* All status bits */
/****************************************************************************/
--
1.5.6.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/4] drivers/net/fec: Use constants instead of magic numbers
2011-03-21 16:37 ` [PATCH 1/4] drivers/net/fec: Cleanup Lothar Waßmann
@ 2011-03-21 16:37 ` Lothar Waßmann
2011-03-21 16:37 ` [PATCH 3/4] drivers/net/fec: Fix dual MAC support on i.MX28 Lothar Waßmann
2011-03-22 2:50 ` [PATCH 2/4] drivers/net/fec: Use constants instead of magic numbers Shawn Guo
2011-03-22 2:49 ` [PATCH 1/4] drivers/net/fec: Cleanup Shawn Guo
1 sibling, 2 replies; 17+ messages in thread
From: Lothar Waßmann @ 2011-03-21 16:37 UTC (permalink / raw)
To: netdev; +Cc: u.kleine-koenig, Shawn Guo, Lothar Waßmann
- No functional change.
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
drivers/net/fec.c | 29 +++++++++++++++--------------
drivers/net/fec.h | 15 +++++++++++++++
2 files changed, 30 insertions(+), 14 deletions(-)
diff --git a/drivers/net/fec.c b/drivers/net/fec.c
index f00ee7f..2436db8 100644
--- a/drivers/net/fec.c
+++ b/drivers/net/fec.c
@@ -341,10 +341,10 @@ fec_restart(struct net_device *ndev, int duplex)
platform_get_device_id(fep->pdev);
int i;
u32 temp_mac[2];
- u32 rcntl = OPT_FRAME_SIZE | 0x04;
+ u32 rcntl = OPT_FRAME_SIZE | FEC_R_CNTRL_MII_MODE;
/* Whack a reset. We should wait for this. */
- writel(1, fep->hwp + FEC_ECNTRL);
+ writel(FEC_ECNTRL_RESET, fep->hwp + FEC_ECNTRL);
udelay(10);
/*
@@ -391,10 +391,10 @@ fec_restart(struct net_device *ndev, int duplex)
/* Enable MII mode */
if (duplex) {
/* FD enable */
- writel(0x04, fep->hwp + FEC_X_CNTRL);
+ writel(FEC_X_CNTRL_FDEN, fep->hwp + FEC_X_CNTRL);
} else {
/* No Rcv on Xmit */
- rcntl |= 0x02;
+ rcntl |= FEC_R_CNTRL_DRT;
writel(0x0, fep->hwp + FEC_X_CNTRL);
}
@@ -409,19 +409,19 @@ fec_restart(struct net_device *ndev, int duplex)
*/
if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) {
/* Enable flow control and length check */
- rcntl |= 0x40000000 | 0x00000020;
+ rcntl |= FEC_R_CNTRL_NO_LEN_CHK | FEC_R_CNTRL_FCE;
/* MII or RMII */
if (fep->phy_interface == PHY_INTERFACE_MODE_RMII)
- rcntl |= (1 << 8);
+ rcntl |= FEC_R_CNTRL_RMII_MODE;
else
- rcntl &= ~(1 << 8);
+ rcntl &= ~FEC_R_CNTRL_RMII_MODE;
/* 10M or 100M */
if (fep->phy_dev && fep->phy_dev->speed == SPEED_100)
- rcntl &= ~(1 << 9);
+ rcntl &= ~FEC_R_CNTRL_RMII_10T;
else
- rcntl |= (1 << 9);
+ rcntl |= FEC_R_CNTRL_RMII_10T;
} else {
#ifdef FEC_MIIGSK_ENR
@@ -445,7 +445,7 @@ fec_restart(struct net_device *ndev, int duplex)
writel(rcntl, fep->hwp + FEC_R_CNTRL);
/* And last, enable the transmit and receive processing */
- writel(2, fep->hwp + FEC_ECNTRL);
+ writel(FEC_ECNTRL_ETHER_EN, fep->hwp + FEC_ECNTRL);
writel(0, fep->hwp + FEC_R_DES_ACTIVE);
/* Enable interrupts we wish to service */
@@ -459,14 +459,15 @@ fec_stop(struct net_device *ndev)
/* We cannot expect a graceful transmit stop without link !!! */
if (fep->link) {
- writel(1, fep->hwp + FEC_X_CNTRL); /* Graceful transmit stop */
+ /* Graceful transmit stop */
+ writel(FEC_X_CNTRL_GTS, fep->hwp + FEC_X_CNTRL);
udelay(10);
if (!(readl(fep->hwp + FEC_IEVENT) & FEC_ENET_GRA))
printk("fec_stop : Graceful transmit stop did not complete !\n");
}
/* Whack a reset. We should wait for this. */
- writel(1, fep->hwp + FEC_ECNTRL);
+ writel(FEC_ECNTRL_RESET, fep->hwp + FEC_ECNTRL);
udelay(10);
writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
@@ -1198,13 +1199,13 @@ static void set_multicast_list(struct net_device *ndev)
if (ndev->flags & IFF_PROMISC) {
tmp = readl(fep->hwp + FEC_R_CNTRL);
- tmp |= 0x8;
+ tmp |= FEC_R_CNTRL_PROM;
writel(tmp, fep->hwp + FEC_R_CNTRL);
return;
}
tmp = readl(fep->hwp + FEC_R_CNTRL);
- tmp &= ~0x8;
+ tmp &= ~FEC_R_CNTRL_PROM;
writel(tmp, fep->hwp + FEC_R_CNTRL);
if (ndev->flags & IFF_ALLMULTI) {
diff --git a/drivers/net/fec.h b/drivers/net/fec.h
index 8697556..68f4049 100644
--- a/drivers/net/fec.h
+++ b/drivers/net/fec.h
@@ -144,5 +144,20 @@ struct bufdesc {
#define BD_ENET_TX_STATS ((ushort)0x03ff) /* All status bits */
+/* register bit assignments */
+#define FEC_ECNTRL_ETHER_EN (1 << 1)
+#define FEC_ECNTRL_RESET (1 << 0)
+
+#define FEC_X_CNTRL_GTS (1 << 0)
+#define FEC_X_CNTRL_FDEN (1 << 2)
+
+#define FEC_R_CNTRL_DRT (1 << 1)
+#define FEC_R_CNTRL_MII_MODE (1 << 2)
+#define FEC_R_CNTRL_PROM (1 << 3)
+#define FEC_R_CNTRL_FCE (1 << 5)
+#define FEC_R_CNTRL_RMII_MODE (1 << 8)
+#define FEC_R_CNTRL_RMII_10T (1 << 9)
+#define FEC_R_CNTRL_NO_LEN_CHK (1 << 30)
+
/****************************************************************************/
#endif /* FEC_H */
--
1.5.6.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/4] drivers/net/fec: Fix dual MAC support on i.MX28
2011-03-21 16:37 ` [PATCH 2/4] drivers/net/fec: Use constants instead of magic numbers Lothar Waßmann
@ 2011-03-21 16:37 ` Lothar Waßmann
2011-03-21 16:37 ` [PATCH 4/4] drivers/net/fec: Don't mess with configured MAC addresses Lothar Waßmann
2011-03-22 7:28 ` [PATCH 3/4] drivers/net/fec: Fix dual MAC support on i.MX28 Shawn Guo
2011-03-22 2:50 ` [PATCH 2/4] drivers/net/fec: Use constants instead of magic numbers Shawn Guo
1 sibling, 2 replies; 17+ messages in thread
From: Lothar Waßmann @ 2011-03-21 16:37 UTC (permalink / raw)
To: netdev; +Cc: u.kleine-koenig, Shawn Guo, Lothar Waßmann
The variant of the FEC chip found on i.MX28 features two distinct
ethernet MACs that both share a single MII interface. When
deconfiguring the eth0 interface, the MII interface is currently being
switched off, so that the second MAC won't work any more.
Make sure, the first MAC is never disabled when the second FEC device
is registered.
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
drivers/net/fec.c | 38 +++++++++++++++++++++++++++++++++-----
1 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/drivers/net/fec.c b/drivers/net/fec.c
index 2436db8..3666524 100644
--- a/drivers/net/fec.c
+++ b/drivers/net/fec.c
@@ -82,6 +82,9 @@ static unsigned char macaddr[ETH_ALEN];
module_param_array(macaddr, byte, NULL, 0);
MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
+static int dual_mac;
+module_param(dual_mac, int, S_IRUGO);
+
#if defined(CONFIG_M5272)
/*
* Some hardware gets it MAC address out of local flash memory.
@@ -456,6 +459,8 @@ static void
fec_stop(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
+ const struct platform_device_id *id_entry =
+ platform_get_device_id(fep->pdev);
/* We cannot expect a graceful transmit stop without link !!! */
if (fep->link) {
@@ -466,11 +471,22 @@ fec_stop(struct net_device *ndev)
printk("fec_stop : Graceful transmit stop did not complete !\n");
}
- /* Whack a reset. We should wait for this. */
- writel(FEC_ECNTRL_RESET, fep->hwp + FEC_ECNTRL);
- udelay(10);
- writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
- writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
+ if ((id_entry->driver_data & FEC_QUIRK_ENET_MAC) && dual_mac &&
+ fep->pdev->id == 0) {
+ /* clear the RDAR/TDAR bits to stop recv/xmit,
+ * but do not reset the controller since the second MAC
+ * may still be using the MII interface and requires ETHER_EN
+ * on the first MAC to be asserted for MII interrupts!
+ */
+ writel(0, fep->hwp + FEC_ECNTRL);
+ writel(FEC_ECNTRL_ETHER_EN, fep->hwp + FEC_ECNTRL);
+ } else {
+ /* Whack a reset. We should wait for this. */
+ writel(FEC_ECNTRL_RESET, fep->hwp + FEC_ECNTRL);
+ udelay(10);
+ writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
+ writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
+ }
}
static void
@@ -1135,6 +1151,8 @@ static int
fec_enet_open(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
+ const struct platform_device_id *id_entry =
+ platform_get_device_id(fep->pdev);
int ret;
/* I should reset the ring buffers here, but I don't yet know
@@ -1145,6 +1163,14 @@ fec_enet_open(struct net_device *ndev)
if (ret)
return ret;
+ if ((id_entry->driver_data & FEC_QUIRK_ENET_MAC) &&
+ fep->pdev->id == 0) {
+ /* ETHER_EN on first MAC has to be asserted
+ * in order to generate MII interrupts
+ */
+ fec_restart(ndev, 0);
+ }
+
/* Probe and connect to PHY when open the interface */
ret = fec_enet_mii_probe(ndev);
if (ret) {
@@ -1435,6 +1461,8 @@ fec_probe(struct platform_device *pdev)
if (ret)
goto failed_register;
+ if (pdev->id > 0)
+ dual_mac = 1;
return 0;
failed_register:
--
1.5.6.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/4] drivers/net/fec: Don't mess with configured MAC addresses.
2011-03-21 16:37 ` [PATCH 3/4] drivers/net/fec: Fix dual MAC support on i.MX28 Lothar Waßmann
@ 2011-03-21 16:37 ` Lothar Waßmann
2011-03-22 7:43 ` Shawn Guo
2011-03-22 7:28 ` [PATCH 3/4] drivers/net/fec: Fix dual MAC support on i.MX28 Shawn Guo
1 sibling, 1 reply; 17+ messages in thread
From: Lothar Waßmann @ 2011-03-21 16:37 UTC (permalink / raw)
To: netdev; +Cc: u.kleine-koenig, Shawn Guo, Lothar Waßmann
The FEC driver currently uses the MAC address assigned to the first
interface incremented by one for the second interface.
Change this to be able to configure distinct MAC addresses via
platform_data.
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
drivers/net/fec.c | 9 ++-------
1 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/net/fec.c b/drivers/net/fec.c
index 3666524..9d89e99 100644
--- a/drivers/net/fec.c
+++ b/drivers/net/fec.c
@@ -750,13 +750,12 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
/*
* 2) from flash or fuse (via platform data)
*/
+ if (pdata)
+ memcpy(iap, pdata->mac, ETH_ALEN);
if (!is_valid_ether_addr(iap)) {
#ifdef CONFIG_M5272
if (FEC_FLASHMAC)
iap = (unsigned char *)FEC_FLASHMAC;
-#else
- if (pdata)
- memcpy(iap, pdata->mac, ETH_ALEN);
#endif
}
@@ -772,10 +771,6 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
}
memcpy(ndev->dev_addr, iap, ETH_ALEN);
-
- /* Adjust MAC if using macaddr */
- if (iap == macaddr)
- ndev->dev_addr[ETH_ALEN-1] = macaddr[ETH_ALEN-1] + fep->pdev->id;
}
/* ------------------------------------------------------------------------- */
--
1.5.6.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] drivers/net/fec: Cleanup
2011-03-21 16:37 ` [PATCH 1/4] drivers/net/fec: Cleanup Lothar Waßmann
2011-03-21 16:37 ` [PATCH 2/4] drivers/net/fec: Use constants instead of magic numbers Lothar Waßmann
@ 2011-03-22 2:49 ` Shawn Guo
1 sibling, 0 replies; 17+ messages in thread
From: Shawn Guo @ 2011-03-22 2:49 UTC (permalink / raw)
To: Lothar Waßmann; +Cc: netdev, u.kleine-koenig
On Mon, Mar 21, 2011 at 05:37:33PM +0100, Lothar Waßmann wrote:
> - Whitespace cleanup
> - Convert indentation spaces to TABs
> - Spelling fixes
>
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
>
Acked-by: Shawn Guo <shawn.guo@freescale.com>
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] drivers/net/fec: Use constants instead of magic numbers
2011-03-21 16:37 ` [PATCH 2/4] drivers/net/fec: Use constants instead of magic numbers Lothar Waßmann
2011-03-21 16:37 ` [PATCH 3/4] drivers/net/fec: Fix dual MAC support on i.MX28 Lothar Waßmann
@ 2011-03-22 2:50 ` Shawn Guo
1 sibling, 0 replies; 17+ messages in thread
From: Shawn Guo @ 2011-03-22 2:50 UTC (permalink / raw)
To: Lothar Waßmann; +Cc: netdev, u.kleine-koenig
On Mon, Mar 21, 2011 at 05:37:34PM +0100, Lothar Waßmann wrote:
> - No functional change.
>
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
Acked-by: Shawn Guo <shawn.guo@freescale.com>
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] drivers/net/fec: Fix dual MAC support on i.MX28
2011-03-21 16:37 ` [PATCH 3/4] drivers/net/fec: Fix dual MAC support on i.MX28 Lothar Waßmann
2011-03-21 16:37 ` [PATCH 4/4] drivers/net/fec: Don't mess with configured MAC addresses Lothar Waßmann
@ 2011-03-22 7:28 ` Shawn Guo
2011-03-22 8:04 ` Lothar Waßmann
1 sibling, 1 reply; 17+ messages in thread
From: Shawn Guo @ 2011-03-22 7:28 UTC (permalink / raw)
To: Lothar Waßmann; +Cc: netdev, u.kleine-koenig
Hi Lothar,
On Mon, Mar 21, 2011 at 05:37:35PM +0100, Lothar Waßmann wrote:
> The variant of the FEC chip found on i.MX28 features two distinct
> ethernet MACs that both share a single MII interface. When
> deconfiguring the eth0 interface, the MII interface is currently being
> switched off, so that the second MAC won't work any more.
>
I'm wondering what the use case is you intend to support with this
patch.
> Make sure, the first MAC is never disabled when the second FEC device
> is registered.
>
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> ---
> drivers/net/fec.c | 38 +++++++++++++++++++++++++++++++++-----
> 1 files changed, 33 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/fec.c b/drivers/net/fec.c
> index 2436db8..3666524 100644
> --- a/drivers/net/fec.c
> +++ b/drivers/net/fec.c
> @@ -82,6 +82,9 @@ static unsigned char macaddr[ETH_ALEN];
> module_param_array(macaddr, byte, NULL, 0);
> MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
>
> +static int dual_mac;
> +module_param(dual_mac, int, S_IRUGO);
> +
As you are detecting dual_mac in probe function, is this necessary?
> #if defined(CONFIG_M5272)
> /*
> * Some hardware gets it MAC address out of local flash memory.
> @@ -456,6 +459,8 @@ static void
> fec_stop(struct net_device *ndev)
> {
> struct fec_enet_private *fep = netdev_priv(ndev);
> + const struct platform_device_id *id_entry =
> + platform_get_device_id(fep->pdev);
>
> /* We cannot expect a graceful transmit stop without link !!! */
> if (fep->link) {
> @@ -466,11 +471,22 @@ fec_stop(struct net_device *ndev)
> printk("fec_stop : Graceful transmit stop did not complete !\n");
> }
>
> - /* Whack a reset. We should wait for this. */
> - writel(FEC_ECNTRL_RESET, fep->hwp + FEC_ECNTRL);
> - udelay(10);
> - writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
> - writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
> + if ((id_entry->driver_data & FEC_QUIRK_ENET_MAC) && dual_mac &&
> + fep->pdev->id == 0) {
> + /* clear the RDAR/TDAR bits to stop recv/xmit,
> + * but do not reset the controller since the second MAC
> + * may still be using the MII interface and requires ETHER_EN
> + * on the first MAC to be asserted for MII interrupts!
> + */
> + writel(0, fep->hwp + FEC_ECNTRL);
> + writel(FEC_ECNTRL_ETHER_EN, fep->hwp + FEC_ECNTRL);
> + } else {
> + /* Whack a reset. We should wait for this. */
> + writel(FEC_ECNTRL_RESET, fep->hwp + FEC_ECNTRL);
> + udelay(10);
> + writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
> + writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
> + }
> }
>
> static void
> @@ -1135,6 +1151,8 @@ static int
> fec_enet_open(struct net_device *ndev)
> {
> struct fec_enet_private *fep = netdev_priv(ndev);
> + const struct platform_device_id *id_entry =
> + platform_get_device_id(fep->pdev);
> int ret;
>
> /* I should reset the ring buffers here, but I don't yet know
> @@ -1145,6 +1163,14 @@ fec_enet_open(struct net_device *ndev)
> if (ret)
> return ret;
>
> + if ((id_entry->driver_data & FEC_QUIRK_ENET_MAC) &&
> + fep->pdev->id == 0) {
> + /* ETHER_EN on first MAC has to be asserted
> + * in order to generate MII interrupts
> + */
> + fec_restart(ndev, 0);
> + }
> +
Is this relevant to the use case you intend to support? Or something
is broken without these codes?
> /* Probe and connect to PHY when open the interface */
> ret = fec_enet_mii_probe(ndev);
> if (ret) {
> @@ -1435,6 +1461,8 @@ fec_probe(struct platform_device *pdev)
> if (ret)
> goto failed_register;
>
> + if (pdev->id > 0)
> + dual_mac = 1;
> return 0;
>
Blank line above 'return 0;'?
--
Regards,
Shawn
> failed_register:
> --
> 1.5.6.5
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] drivers/net/fec: Don't mess with configured MAC addresses.
2011-03-21 16:37 ` [PATCH 4/4] drivers/net/fec: Don't mess with configured MAC addresses Lothar Waßmann
@ 2011-03-22 7:43 ` Shawn Guo
2011-03-22 7:49 ` Lothar Waßmann
0 siblings, 1 reply; 17+ messages in thread
From: Shawn Guo @ 2011-03-22 7:43 UTC (permalink / raw)
To: Lothar Waßmann; +Cc: netdev, u.kleine-koenig
On Mon, Mar 21, 2011 at 05:37:36PM +0100, Lothar Waßmann wrote:
> The FEC driver currently uses the MAC address assigned to the first
> interface incremented by one for the second interface.
>
> Change this to be able to configure distinct MAC addresses via
> platform_data.
>
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> ---
> drivers/net/fec.c | 9 ++-------
> 1 files changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/fec.c b/drivers/net/fec.c
> index 3666524..9d89e99 100644
> --- a/drivers/net/fec.c
> +++ b/drivers/net/fec.c
> @@ -750,13 +750,12 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
> /*
> * 2) from flash or fuse (via platform data)
> */
> + if (pdata)
> + memcpy(iap, pdata->mac, ETH_ALEN);
> if (!is_valid_ether_addr(iap)) {
> #ifdef CONFIG_M5272
> if (FEC_FLASHMAC)
> iap = (unsigned char *)FEC_FLASHMAC;
> -#else
> - if (pdata)
> - memcpy(iap, pdata->mac, ETH_ALEN);
> #endif
> }
>
> @@ -772,10 +771,6 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
> }
>
> memcpy(ndev->dev_addr, iap, ETH_ALEN);
> -
> - /* Adjust MAC if using macaddr */
> - if (iap == macaddr)
> - ndev->dev_addr[ETH_ALEN-1] = macaddr[ETH_ALEN-1] + fep->pdev->id;
> }
>
> /* ------------------------------------------------------------------------- */
> --
> 1.5.6.5
>
>
NAK.
The 'mac' was designed as an optional member of fec
platform_data, as some platforms may not be able to provide it. This
patch breaks the order of getting mac, and will not work on the
platform that does not provide an valid mac with either platform_data
or controller mac registers, even there may be one in the kernel
command line.
The existing code takes mac address from kernel command line as the
first choice, and only falls through on other options when it's
invalid/unavailable. So an valid mac from kernel command line will
always get fec work anyway.
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] drivers/net/fec: Don't mess with configured MAC addresses.
2011-03-22 7:43 ` Shawn Guo
@ 2011-03-22 7:49 ` Lothar Waßmann
2011-03-22 8:55 ` Shawn Guo
0 siblings, 1 reply; 17+ messages in thread
From: Lothar Waßmann @ 2011-03-22 7:49 UTC (permalink / raw)
To: Shawn Guo; +Cc: netdev, u.kleine-koenig
Hi,
Shawn Guo writes:
> On Mon, Mar 21, 2011 at 05:37:36PM +0100, Lothar Waßmann wrote:
> > The FEC driver currently uses the MAC address assigned to the first
> > interface incremented by one for the second interface.
> >
> > Change this to be able to configure distinct MAC addresses via
> > platform_data.
> >
> > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> > ---
> > drivers/net/fec.c | 9 ++-------
> > 1 files changed, 2 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/fec.c b/drivers/net/fec.c
> > index 3666524..9d89e99 100644
> > --- a/drivers/net/fec.c
> > +++ b/drivers/net/fec.c
> > @@ -750,13 +750,12 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
> > /*
> > * 2) from flash or fuse (via platform data)
> > */
> > + if (pdata)
> > + memcpy(iap, pdata->mac, ETH_ALEN);
> > if (!is_valid_ether_addr(iap)) {
> > #ifdef CONFIG_M5272
> > if (FEC_FLASHMAC)
> > iap = (unsigned char *)FEC_FLASHMAC;
> > -#else
> > - if (pdata)
> > - memcpy(iap, pdata->mac, ETH_ALEN);
> > #endif
> > }
> >
> > @@ -772,10 +771,6 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
> > }
> >
> > memcpy(ndev->dev_addr, iap, ETH_ALEN);
> > -
> > - /* Adjust MAC if using macaddr */
> > - if (iap == macaddr)
> > - ndev->dev_addr[ETH_ALEN-1] = macaddr[ETH_ALEN-1] + fep->pdev->id;
> > }
> >
> > /* ------------------------------------------------------------------------- */
> > --
> > 1.5.6.5
> >
> >
> NAK.
>
> The 'mac' was designed as an optional member of fec
> platform_data, as some platforms may not be able to provide it. This
> patch breaks the order of getting mac, and will not work on the
> platform that does not provide an valid mac with either platform_data
> or controller mac registers, even there may be one in the kernel
> command line.
>
> The existing code takes mac address from kernel command line as the
> first choice, and only falls through on other options when it's
> invalid/unavailable. So an valid mac from kernel command line will
> always get fec work anyway.
>
The problem is that you can only assign one mac address on the kernel
cmdline and the second one will be derived from the first one.
And even if the MAC address is set via platform_data it will not be
used for the second interface.
IMO an ethernet driver has no business in inventing arbitrary MAC
addresses. Either the MAC address is supplied by the boot loader or
the platform code or it has to be assigned from userspace via
ifconfig.
Lothar Waßmann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] drivers/net/fec: Fix dual MAC support on i.MX28
2011-03-22 7:28 ` [PATCH 3/4] drivers/net/fec: Fix dual MAC support on i.MX28 Shawn Guo
@ 2011-03-22 8:04 ` Lothar Waßmann
2011-03-22 8:07 ` David Miller
2011-03-22 9:22 ` Shawn Guo
0 siblings, 2 replies; 17+ messages in thread
From: Lothar Waßmann @ 2011-03-22 8:04 UTC (permalink / raw)
To: Shawn Guo; +Cc: netdev, u.kleine-koenig
Hi,
Shawn Guo writes:
> Hi Lothar,
>
> On Mon, Mar 21, 2011 at 05:37:35PM +0100, Lothar Waßmann wrote:
> > The variant of the FEC chip found on i.MX28 features two distinct
> > ethernet MACs that both share a single MII interface. When
> > deconfiguring the eth0 interface, the MII interface is currently being
> > switched off, so that the second MAC won't work any more.
> >
> I'm wondering what the use case is you intend to support with this
> patch.
>
The patch enables the use of both MACs independently from each other.
Without this patch deconfiguring eth0 would leave eth1 in an unusable
state.
> > Make sure, the first MAC is never disabled when the second FEC device
> > is registered.
> >
> > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> > ---
> > drivers/net/fec.c | 38 +++++++++++++++++++++++++++++++++-----
> > 1 files changed, 33 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/fec.c b/drivers/net/fec.c
> > index 2436db8..3666524 100644
> > --- a/drivers/net/fec.c
> > +++ b/drivers/net/fec.c
> > @@ -82,6 +82,9 @@ static unsigned char macaddr[ETH_ALEN];
> > module_param_array(macaddr, byte, NULL, 0);
> > MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
> >
> > +static int dual_mac;
> > +module_param(dual_mac, int, S_IRUGO);
> > +
>
> As you are detecting dual_mac in probe function, is this necessary?
>
Not really. I added this is only for debugging.
> > #if defined(CONFIG_M5272)
> > /*
> > * Some hardware gets it MAC address out of local flash memory.
> > @@ -456,6 +459,8 @@ static void
> > fec_stop(struct net_device *ndev)
> > {
> > struct fec_enet_private *fep = netdev_priv(ndev);
> > + const struct platform_device_id *id_entry =
> > + platform_get_device_id(fep->pdev);
> >
> > /* We cannot expect a graceful transmit stop without link !!! */
> > if (fep->link) {
> > @@ -466,11 +471,22 @@ fec_stop(struct net_device *ndev)
> > printk("fec_stop : Graceful transmit stop did not complete !\n");
> > }
> >
> > - /* Whack a reset. We should wait for this. */
> > - writel(FEC_ECNTRL_RESET, fep->hwp + FEC_ECNTRL);
> > - udelay(10);
> > - writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
> > - writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
> > + if ((id_entry->driver_data & FEC_QUIRK_ENET_MAC) && dual_mac &&
> > + fep->pdev->id == 0) {
> > + /* clear the RDAR/TDAR bits to stop recv/xmit,
> > + * but do not reset the controller since the second MAC
> > + * may still be using the MII interface and requires ETHER_EN
> > + * on the first MAC to be asserted for MII interrupts!
> > + */
> > + writel(0, fep->hwp + FEC_ECNTRL);
> > + writel(FEC_ECNTRL_ETHER_EN, fep->hwp + FEC_ECNTRL);
> > + } else {
> > + /* Whack a reset. We should wait for this. */
> > + writel(FEC_ECNTRL_RESET, fep->hwp + FEC_ECNTRL);
> > + udelay(10);
> > + writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
> > + writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
> > + }
> > }
> >
> > static void
> > @@ -1135,6 +1151,8 @@ static int
> > fec_enet_open(struct net_device *ndev)
> > {
> > struct fec_enet_private *fep = netdev_priv(ndev);
> > + const struct platform_device_id *id_entry =
> > + platform_get_device_id(fep->pdev);
> > int ret;
> >
> > /* I should reset the ring buffers here, but I don't yet know
> > @@ -1145,6 +1163,14 @@ fec_enet_open(struct net_device *ndev)
> > if (ret)
> > return ret;
> >
> > + if ((id_entry->driver_data & FEC_QUIRK_ENET_MAC) &&
> > + fep->pdev->id == 0) {
> > + /* ETHER_EN on first MAC has to be asserted
> > + * in order to generate MII interrupts
> > + */
> > + fec_restart(ndev, 0);
> > + }
> > +
>
> Is this relevant to the use case you intend to support? Or something
> is broken without these codes?
>
Actually this is not necessary any more, since with this patch the
first MAC is not being disabled on close any more. Looks like a single
problem fixed in two different places...
> > /* Probe and connect to PHY when open the interface */
> > ret = fec_enet_mii_probe(ndev);
> > if (ret) {
> > @@ -1435,6 +1461,8 @@ fec_probe(struct platform_device *pdev)
> > if (ret)
> > goto failed_register;
> >
> > + if (pdev->id > 0)
> > + dual_mac = 1;
> > return 0;
> >
> Blank line above 'return 0;'?
>
OK.
Lothar Waßmann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] drivers/net/fec: Fix dual MAC support on i.MX28
2011-03-22 8:04 ` Lothar Waßmann
@ 2011-03-22 8:07 ` David Miller
2011-03-22 9:22 ` Shawn Guo
1 sibling, 0 replies; 17+ messages in thread
From: David Miller @ 2011-03-22 8:07 UTC (permalink / raw)
To: LW; +Cc: shawn.guo, netdev, u.kleine-koenig
From: Lothar Waßmann <LW@KARO-electronics.de>
Date: Tue, 22 Mar 2011 09:04:00 +0100
> Shawn Guo writes:
>> > +static int dual_mac;
>> > +module_param(dual_mac, int, S_IRUGO);
>> > +
>>
>> As you are detecting dual_mac in probe function, is this necessary?
>>
> Not really. I added this is only for debugging.
Please do not add driver-specific module parameters.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] drivers/net/fec: Don't mess with configured MAC addresses.
2011-03-22 7:49 ` Lothar Waßmann
@ 2011-03-22 8:55 ` Shawn Guo
0 siblings, 0 replies; 17+ messages in thread
From: Shawn Guo @ 2011-03-22 8:55 UTC (permalink / raw)
To: Lothar Waßmann; +Cc: netdev, u.kleine-koenig
On Tue, Mar 22, 2011 at 08:49:41AM +0100, Lothar Waßmann wrote:
> Hi,
>
> Shawn Guo writes:
> > On Mon, Mar 21, 2011 at 05:37:36PM +0100, Lothar Waßmann wrote:
> > > The FEC driver currently uses the MAC address assigned to the first
> > > interface incremented by one for the second interface.
> > >
> > > Change this to be able to configure distinct MAC addresses via
> > > platform_data.
> > >
> > > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> > > ---
> > > drivers/net/fec.c | 9 ++-------
> > > 1 files changed, 2 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/net/fec.c b/drivers/net/fec.c
> > > index 3666524..9d89e99 100644
> > > --- a/drivers/net/fec.c
> > > +++ b/drivers/net/fec.c
> > > @@ -750,13 +750,12 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
> > > /*
> > > * 2) from flash or fuse (via platform data)
> > > */
> > > + if (pdata)
> > > + memcpy(iap, pdata->mac, ETH_ALEN);
> > > if (!is_valid_ether_addr(iap)) {
> > > #ifdef CONFIG_M5272
> > > if (FEC_FLASHMAC)
> > > iap = (unsigned char *)FEC_FLASHMAC;
> > > -#else
> > > - if (pdata)
> > > - memcpy(iap, pdata->mac, ETH_ALEN);
> > > #endif
> > > }
> > >
> > > @@ -772,10 +771,6 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
> > > }
> > >
> > > memcpy(ndev->dev_addr, iap, ETH_ALEN);
> > > -
> > > - /* Adjust MAC if using macaddr */
> > > - if (iap == macaddr)
> > > - ndev->dev_addr[ETH_ALEN-1] = macaddr[ETH_ALEN-1] + fep->pdev->id;
> > > }
> > >
> > > /* ------------------------------------------------------------------------- */
> > > --
> > > 1.5.6.5
> > >
> > >
> > NAK.
> >
> > The 'mac' was designed as an optional member of fec
> > platform_data, as some platforms may not be able to provide it. This
> > patch breaks the order of getting mac, and will not work on the
> > platform that does not provide an valid mac with either platform_data
> > or controller mac registers, even there may be one in the kernel
> > command line.
> >
> > The existing code takes mac address from kernel command line as the
> > first choice, and only falls through on other options when it's
> > invalid/unavailable. So an valid mac from kernel command line will
> > always get fec work anyway.
> >
> The problem is that you can only assign one mac address on the kernel
> cmdline and the second one will be derived from the first one.
> And even if the MAC address is set via platform_data it will not be
> used for the second interface.
>
Yes, these are problems. But we can fix them like the following while
still make fec0 work with fec.macaddr. This is useful on the platform
that neither platform code nor boot loader gives an valid mac address.
@@ -721,7 +721,7 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
struct fec_platform_data *pdata = fep->pdev->dev.platform_data;
- unsigned char *iap, tmpaddr[ETH_ALEN];
+ unsigned char iap[ETH_ALEN], tmpaddr[ETH_ALEN];
/*
* try to get mac address in following order:
@@ -729,7 +729,10 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
* 1) module parameter via kernel command line in form
* fec.macaddr=0x00,0x04,0x9f,0x01,0x30,0xe0
*/
- iap = macaddr;
+ if (fep->pdev->id == 0)
+ memcpy(iap, macaddr, ETH_ALEN);
+ else
+ memset(iap, 0, ETH_ALEN);
/*
* 2) from flash or fuse (via platform data)
@@ -752,14 +755,10 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
be32_to_cpu(readl(fep->hwp + FEC_ADDR_LOW));
*((unsigned short *) &tmpaddr[4]) =
be16_to_cpu(readl(fep->hwp + FEC_ADDR_HIGH) >> 16);
- iap = &tmpaddr[0];
+ memcpy(iap, tmpaddr, ETH_ALEN);
}
memcpy(ndev->dev_addr, iap, ETH_ALEN);
-
- /* Adjust MAC if using macaddr */
- if (iap == macaddr)
- ndev->dev_addr[ETH_ALEN-1] = macaddr[ETH_ALEN-1] + fep->pdev->id;
}
> IMO an ethernet driver has no business in inventing arbitrary MAC
> addresses. Either the MAC address is supplied by the boot loader or
> the platform code or it has to be assigned from userspace via
> ifconfig.
>
ifconfig will not be an option if we need to mount nfs as rootfs.
So we at lease need to have kernel command line for fec0 mac address
for the last chance, if neither platform data nor boot loader provides
mac address. And let ifconfig provide mac for fec1 from user space.
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] drivers/net/fec: Fix dual MAC support on i.MX28
2011-03-22 8:04 ` Lothar Waßmann
2011-03-22 8:07 ` David Miller
@ 2011-03-22 9:22 ` Shawn Guo
1 sibling, 0 replies; 17+ messages in thread
From: Shawn Guo @ 2011-03-22 9:22 UTC (permalink / raw)
To: Lothar Waßmann; +Cc: netdev, u.kleine-koenig
On Tue, Mar 22, 2011 at 09:04:00AM +0100, Lothar Waßmann wrote:
> Hi,
>
> Shawn Guo writes:
> > Hi Lothar,
> >
> > On Mon, Mar 21, 2011 at 05:37:35PM +0100, Lothar Waßmann wrote:
> > > The variant of the FEC chip found on i.MX28 features two distinct
> > > ethernet MACs that both share a single MII interface. When
> > > deconfiguring the eth0 interface, the MII interface is currently being
> > > switched off, so that the second MAC won't work any more.
> > >
> > I'm wondering what the use case is you intend to support with this
> > patch.
> >
> The patch enables the use of both MACs independently from each other.
> Without this patch deconfiguring eth0 would leave eth1 in an unusable
> state.
>
Ah, I see. Just tested it. It really helps. Thanks for the patch.
With the patch cleaned up, please add:
Acked-by: <shawn.guo@freescale.com>
Tested-by: <shawn.guo@freescale.com>
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/4] drivers/net/fec: Fix dual MAC support on i.MX28
2011-03-21 16:37 [PATCH 0/4] drivers/net/fec: Some cleanups and fix for DUAL MAC mode Lothar Waßmann
2011-03-21 16:37 ` [PATCH 1/4] drivers/net/fec: Cleanup Lothar Waßmann
@ 2011-03-22 10:00 ` Lothar Waßmann
2011-03-22 10:00 ` [PATCH 4/4] drivers/net/fec: Don't mess with configured MAC addresses Lothar Waßmann
2 siblings, 0 replies; 17+ messages in thread
From: Lothar Waßmann @ 2011-03-22 10:00 UTC (permalink / raw)
To: netdev; +Cc: Shawn Guo, u.kleine-koenig, Lothar Waßmann
The variant of the FEC chip found on i.MX28 features two distinct
ethernet MACs that both share a single MII interface. When
deconfiguring the eth0 interface, the MII interface is currently being
switched off, so that the second MAC won't work any more.
Make sure, the first MAC is never disabled when the second FEC device
is registered.
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
Acked-by: <shawn.guo@freescale.com>
Tested-by: <shawn.guo@freescale.com>
---
drivers/net/fec.c | 38 +++++++++++++++++++++++++++++++++-----
1 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/drivers/net/fec.c b/drivers/net/fec.c
index 2436db8..3666524 100644
--- a/drivers/net/fec.c
+++ b/drivers/net/fec.c
@@ -82,6 +82,8 @@ static unsigned char macaddr[ETH_ALEN];
module_param_array(macaddr, byte, NULL, 0);
MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
+static int dual_mac;
+
#if defined(CONFIG_M5272)
/*
* Some hardware gets it MAC address out of local flash memory.
@@ -456,6 +459,8 @@ static void
fec_stop(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
+ const struct platform_device_id *id_entry =
+ platform_get_device_id(fep->pdev);
/* We cannot expect a graceful transmit stop without link !!! */
if (fep->link) {
@@ -466,11 +471,22 @@ fec_stop(struct net_device *ndev)
printk("fec_stop : Graceful transmit stop did not complete !\n");
}
- /* Whack a reset. We should wait for this. */
- writel(FEC_ECNTRL_RESET, fep->hwp + FEC_ECNTRL);
- udelay(10);
- writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
- writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
+ if ((id_entry->driver_data & FEC_QUIRK_ENET_MAC) && dual_mac &&
+ fep->pdev->id == 0) {
+ /* clear the RDAR/TDAR bits to stop recv/xmit,
+ * but do not reset the controller since the second MAC
+ * may still be using the MII interface and requires ETHER_EN
+ * on the first MAC to be asserted for MII interrupts!
+ */
+ writel(0, fep->hwp + FEC_ECNTRL);
+ writel(FEC_ECNTRL_ETHER_EN, fep->hwp + FEC_ECNTRL);
+ } else {
+ /* Whack a reset. We should wait for this. */
+ writel(FEC_ECNTRL_RESET, fep->hwp + FEC_ECNTRL);
+ udelay(10);
+ writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
+ writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
+ }
}
static void
@@ -1435,6 +1461,9 @@ fec_probe(struct platform_device *pdev)
if (ret)
goto failed_register;
+ if (pdev->id > 0)
+ dual_mac = 1;
+
return 0;
failed_register:
--
1.5.6.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/4] drivers/net/fec: Don't mess with configured MAC addresses.
2011-03-21 16:37 [PATCH 0/4] drivers/net/fec: Some cleanups and fix for DUAL MAC mode Lothar Waßmann
2011-03-21 16:37 ` [PATCH 1/4] drivers/net/fec: Cleanup Lothar Waßmann
2011-03-22 10:00 ` [PATCH 3/4] drivers/net/fec: Fix dual MAC support on i.MX28 Lothar Waßmann
@ 2011-03-22 10:00 ` Lothar Waßmann
2011-03-22 12:33 ` Shawn Guo
2 siblings, 1 reply; 17+ messages in thread
From: Lothar Waßmann @ 2011-03-22 10:00 UTC (permalink / raw)
To: netdev; +Cc: Shawn Guo, u.kleine-koenig, Lothar Waßmann
The FEC driver currently uses the MAC address assigned to the first
interface incremented by one for the second interface.
Change this to be able to configure distinct MAC addresses via
platform_data.
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
drivers/net/fec.c | 9 ++-------
1 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/net/fec.c b/drivers/net/fec.c
index 3666524..9d89e99 100644
--- a/drivers/net/fec.c
+++ b/drivers/net/fec.c
@@ -736,7 +736,7 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
struct fec_platform_data *pdata = fep->pdev->dev.platform_data;
- unsigned char *iap, tmpaddr[ETH_ALEN];
+ unsigned char iap[ETH_ALEN], tmpaddr[ETH_ALEN];
/*
* try to get mac address in following order:
@@ -744,7 +744,10 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
* 1) module parameter via kernel command line in form
* fec.macaddr=0x00,0x04,0x9f,0x01,0x30,0xe0
*/
- iap = macaddr;
+ if (fep->pdev->id == 0)
+ memcpy(iap, macaddr, ETH_ALEN);
+ else
+ memset(iap, 0, ETH_ALEN);
/*
* 2) from flash or fuse (via platform data)
@@ -767,14 +770,10 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
be32_to_cpu(readl(fep->hwp + FEC_ADDR_LOW));
*((unsigned short *) &tmpaddr[4]) =
be16_to_cpu(readl(fep->hwp + FEC_ADDR_HIGH) >> 16);
- iap = &tmpaddr[0];
+ memcpy(iap, tmpaddr, ETH_ALEN);
}
memcpy(ndev->dev_addr, iap, ETH_ALEN);
-
- /* Adjust MAC if using macaddr */
- if (iap == macaddr)
- ndev->dev_addr[ETH_ALEN-1] = macaddr[ETH_ALEN-1] + fep->pdev->id;
}
/* ------------------------------------------------------------------------- */
--
1.5.6.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] drivers/net/fec: Don't mess with configured MAC addresses.
2011-03-22 10:00 ` [PATCH 4/4] drivers/net/fec: Don't mess with configured MAC addresses Lothar Waßmann
@ 2011-03-22 12:33 ` Shawn Guo
0 siblings, 0 replies; 17+ messages in thread
From: Shawn Guo @ 2011-03-22 12:33 UTC (permalink / raw)
To: Lothar Waßmann; +Cc: netdev, u.kleine-koenig
On Tue, Mar 22, 2011 at 11:00:02AM +0100, Lothar Waßmann wrote:
> The FEC driver currently uses the MAC address assigned to the first
> interface incremented by one for the second interface.
>
> Change this to be able to configure distinct MAC addresses via
> platform_data.
>
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
Acked-by: <shawn.guo@freescale.com>
--
Regards,
Shawn
> ---
> drivers/net/fec.c | 9 ++-------
> 1 files changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/fec.c b/drivers/net/fec.c
> index 3666524..9d89e99 100644
> --- a/drivers/net/fec.c
> +++ b/drivers/net/fec.c
> @@ -736,7 +736,7 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
> {
> struct fec_enet_private *fep = netdev_priv(ndev);
> struct fec_platform_data *pdata = fep->pdev->dev.platform_data;
> - unsigned char *iap, tmpaddr[ETH_ALEN];
> + unsigned char iap[ETH_ALEN], tmpaddr[ETH_ALEN];
>
> /*
> * try to get mac address in following order:
> @@ -744,7 +744,10 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
> * 1) module parameter via kernel command line in form
> * fec.macaddr=0x00,0x04,0x9f,0x01,0x30,0xe0
> */
> - iap = macaddr;
> + if (fep->pdev->id == 0)
> + memcpy(iap, macaddr, ETH_ALEN);
> + else
> + memset(iap, 0, ETH_ALEN);
>
> /*
> * 2) from flash or fuse (via platform data)
> @@ -767,14 +770,10 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
> be32_to_cpu(readl(fep->hwp + FEC_ADDR_LOW));
> *((unsigned short *) &tmpaddr[4]) =
> be16_to_cpu(readl(fep->hwp + FEC_ADDR_HIGH) >> 16);
> - iap = &tmpaddr[0];
> + memcpy(iap, tmpaddr, ETH_ALEN);
> }
>
> memcpy(ndev->dev_addr, iap, ETH_ALEN);
> -
> - /* Adjust MAC if using macaddr */
> - if (iap == macaddr)
> - ndev->dev_addr[ETH_ALEN-1] = macaddr[ETH_ALEN-1] + fep->pdev->id;
> }
>
> /* ------------------------------------------------------------------------- */
> --
> 1.5.6.5
>
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2011-03-22 12:32 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-03-21 16:37 [PATCH 0/4] drivers/net/fec: Some cleanups and fix for DUAL MAC mode Lothar Waßmann
2011-03-21 16:37 ` [PATCH 1/4] drivers/net/fec: Cleanup Lothar Waßmann
2011-03-21 16:37 ` [PATCH 2/4] drivers/net/fec: Use constants instead of magic numbers Lothar Waßmann
2011-03-21 16:37 ` [PATCH 3/4] drivers/net/fec: Fix dual MAC support on i.MX28 Lothar Waßmann
2011-03-21 16:37 ` [PATCH 4/4] drivers/net/fec: Don't mess with configured MAC addresses Lothar Waßmann
2011-03-22 7:43 ` Shawn Guo
2011-03-22 7:49 ` Lothar Waßmann
2011-03-22 8:55 ` Shawn Guo
2011-03-22 7:28 ` [PATCH 3/4] drivers/net/fec: Fix dual MAC support on i.MX28 Shawn Guo
2011-03-22 8:04 ` Lothar Waßmann
2011-03-22 8:07 ` David Miller
2011-03-22 9:22 ` Shawn Guo
2011-03-22 2:50 ` [PATCH 2/4] drivers/net/fec: Use constants instead of magic numbers Shawn Guo
2011-03-22 2:49 ` [PATCH 1/4] drivers/net/fec: Cleanup Shawn Guo
2011-03-22 10:00 ` [PATCH 3/4] drivers/net/fec: Fix dual MAC support on i.MX28 Lothar Waßmann
2011-03-22 10:00 ` [PATCH 4/4] drivers/net/fec: Don't mess with configured MAC addresses Lothar Waßmann
2011-03-22 12:33 ` Shawn Guo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).