[PATCH 01/10] GRETH: added raw AMBA vendor/device number to match against.

[PATCH 01/10] GRETH: added raw AMBA vendor/device number to match against.

[Daniel Hellstrom]

Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
 drivers/net/greth.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/net/greth.c b/drivers/net/greth.c
index 27d6960..1c2dbdb 100644
--- a/drivers/net/greth.c
+++ b/drivers/net/greth.c
@@ -1600,6 +1600,9 @@ static struct of_device_id greth_of_match[] = {
 	{
 	 .name = "GAISLER_ETHMAC",
 	 },
+	{
+	 .name = "01_01d",
+	 },
 	{},
 };
 
-- 
1.5.4

[PATCH 02/10] GRETH: added option to disable a device node from bootloader.

[Daniel Hellstrom]

Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
 drivers/net/greth.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/net/greth.c b/drivers/net/greth.c
index 1c2dbdb..1b10186 100644
--- a/drivers/net/greth.c
+++ b/drivers/net/greth.c
@@ -1383,6 +1383,12 @@ static int __devinit greth_of_probe(struct platform_device *ofdev, const struct
 	int err;
 	int tmp;
 	unsigned long timeout;
+	int *ampopts;
+
+	/* Skip device if used by another OS instance */
+	ampopts = (int *) of_get_property(ofdev->dev.of_node, "ampopts", NULL);
+	if (ampopts && (*ampopts == 0))
+		return -EIO;
 
 	dev = alloc_etherdev(sizeof(struct greth_private));
 
-- 
1.5.4

[PATCH 03/10] GRETH: added no_gbit option

[Daniel Hellstrom]

For debug only. The driver does not report that it is GBit capable, instead
it will report 10/100 mode to the generic PHY layer.

Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
 drivers/net/greth.c |   15 +++++++++++++--
 drivers/net/greth.h |    1 +
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/net/greth.c b/drivers/net/greth.c
index 1b10186..ef8da22 100644
--- a/drivers/net/greth.c
+++ b/drivers/net/greth.c
@@ -66,6 +66,10 @@ static int greth_edcl = 1;
 module_param(greth_edcl, int, 0);
 MODULE_PARM_DESC(greth_edcl, "GRETH EDCL usage indicator. Set to 1 if EDCL is used.");
 
+static int no_gbit = 0;
+module_param(no_gbit, int, S_IRUGO);
+MODULE_PARM_DESC(no_gbit, "GRETH reports only 10/100 support to PHY layer if set to 1. Only affects GRETH GBit MAC, default 0 (off).");
+
 static int greth_open(struct net_device *dev);
 static netdev_tx_t greth_start_xmit(struct sk_buff *skb,
 	   struct net_device *dev);
@@ -1284,7 +1288,7 @@ static int greth_mdio_probe(struct net_device *dev)
 	}
 
 	ret = phy_connect_direct(dev, phy, &greth_link_change,
-			0, greth->gbit_mac ?
+			0, greth->gbit_phy_support ?
 			PHY_INTERFACE_MODE_GMII :
 			PHY_INTERFACE_MODE_MII);
 	if (ret) {
@@ -1293,7 +1297,7 @@ static int greth_mdio_probe(struct net_device *dev)
 		return ret;
 	}
 
-	if (greth->gbit_mac)
+	if (greth->gbit_phy_support)
 		phy->supported &= PHY_GBIT_FEATURES;
 	else
 		phy->supported &= PHY_BASIC_FEATURES;
@@ -1447,6 +1451,13 @@ static int __devinit greth_of_probe(struct platform_device *ofdev, const struct
 	tmp = GRETH_REGLOAD(regs->control);
 	greth->gbit_mac = (tmp >> 27) & 1;
 
+	/* Let user skip GBit link mode by telling MDIO layer that MAC does
+	 * not support GBIT (for debug) */
+	if (greth->gbit_mac && !no_gbit)
+		greth->gbit_phy_support = 1;
+	else
+		greth->gbit_phy_support = 0;
+
 	/* Check for multicast capability */
 	greth->multicast = (tmp >> 25) & 1;
 
diff --git a/drivers/net/greth.h b/drivers/net/greth.h
index 03ad903..9414169 100644
--- a/drivers/net/greth.h
+++ b/drivers/net/greth.h
@@ -138,6 +138,7 @@ struct greth_private {
 	u8 gbit_mac;
 	u8 mdio_int_en;
 	u8 edcl;
+	u8 gbit_phy_support;
 };
 
 #endif
-- 
1.5.4

[PATCH 04/10] GRETH: added greth_compat_mode module parameter

[Daniel Hellstrom]

The greth_compat_mode option can be used to set a GRETH GBit capable MAC
in operate as if the GRETH 10/100 device was found. The GRETH GBit supports
TCP/UDP checksum offloading, unaligned frame buffers, scatter gather etc.
Enabling this mode allows the developer to test the GRETH 10/100 device
without all features mentioned above on a GBit MAC capable of the above.

Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
 drivers/net/greth.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/net/greth.c b/drivers/net/greth.c
index ef8da22..775dc24 100644
--- a/drivers/net/greth.c
+++ b/drivers/net/greth.c
@@ -70,6 +70,12 @@ static int no_gbit = 0;
 module_param(no_gbit, int, S_IRUGO);
 MODULE_PARM_DESC(no_gbit, "GRETH reports only 10/100 support to PHY layer if set to 1. Only affects GRETH GBit MAC, default 0 (off).");
 
+/* Use this option to enable GRETH 10/100 code on GRETH_GBIT hardware
+ * (debug legacy code option) */
+static int compat_mode = 0;
+module_param(compat_mode, int, S_IRUGO);
+MODULE_PARM_DESC(compat_mode, "GRETH 10/100 legacy mode enable. Only affects GRETH GBit MAC, default 0 (off).");
+
 static int greth_open(struct net_device *dev);
 static netdev_tx_t greth_start_xmit(struct sk_buff *skb,
 	   struct net_device *dev);
@@ -1458,6 +1464,10 @@ static int __devinit greth_of_probe(struct platform_device *ofdev, const struct
 	else
 		greth->gbit_phy_support = 0;
 
+	/* Force GBit MAC in legacy 10/100 mode (no offloading etc.) */
+	if (compat_mode == 1)
+		greth->gbit_mac = 0;
+
 	/* Check for multicast capability */
 	greth->multicast = (tmp >> 25) & 1;
 
-- 
1.5.4

[PATCH 05/10] GRETH: fix opening/closing

[Daniel Hellstrom]

When NAPI is disabled there is no point in having IRQs enabled, TX/RX
should be off before clearing the TX/RX descriptor rings.

Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
 drivers/net/greth.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/net/greth.c b/drivers/net/greth.c
index 775dc24..27578c9 100644
--- a/drivers/net/greth.c
+++ b/drivers/net/greth.c
@@ -366,6 +366,8 @@ static int greth_open(struct net_device *dev)
 		dev_dbg(&dev->dev, " starting queue\n");
 	netif_start_queue(dev);
 
+	GRETH_REGSAVE(greth->regs->status, 0xFF);
+
 	napi_enable(&greth->napi);
 
 	greth_enable_irqs(greth);
@@ -381,7 +383,9 @@ static int greth_close(struct net_device *dev)
 
 	napi_disable(&greth->napi);
 
+	greth_disable_irqs(greth);
 	greth_disable_tx(greth);
+	greth_disable_rx(greth);
 
 	netif_stop_queue(dev);
 
-- 
1.5.4

[PATCH 06/10] GRETH: GBit transmit descriptor handling optimization

[Daniel Hellstrom]

It is safe to enable all fragments before enabling the first descriptor,
this way all descriptors don't have to be processed twice, added extra
memory barrier.

Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
 drivers/net/greth.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/net/greth.c b/drivers/net/greth.c
index 27578c9..72a4317 100644
--- a/drivers/net/greth.c
+++ b/drivers/net/greth.c
@@ -513,7 +513,7 @@ greth_start_xmit_gbit(struct sk_buff *skb, struct net_device *dev)
 		greth->tx_skbuff[curr_tx] = NULL;
 		bdp = greth->tx_bd_base + curr_tx;
 
-		status = GRETH_TXBD_CSALL;
+		status = GRETH_TXBD_CSALL | GRETH_BD_EN;
 		status |= frag->size & GRETH_BD_LEN;
 
 		/* Wrap around descriptor ring */
@@ -550,26 +550,27 @@ greth_start_xmit_gbit(struct sk_buff *skb, struct net_device *dev)
 
 	wmb();
 
-	/* Enable the descriptors that we configured ...  */
-	for (i = 0; i < nr_frags + 1; i++) {
-		bdp = greth->tx_bd_base + greth->tx_next;
-		greth_write_bd(&bdp->stat, greth_read_bd(&bdp->stat) | GRETH_BD_EN);
-		greth->tx_next = NEXT_TX(greth->tx_next);
-		greth->tx_free--;
-	}
+	/* Enable the descriptor chain by enabling the first descriptor */
+	bdp = greth->tx_bd_base + greth->tx_next;
+	greth_write_bd(&bdp->stat, greth_read_bd(&bdp->stat) | GRETH_BD_EN);
+	greth->tx_next = curr_tx;
+	greth->tx_free -= nr_frags + 1;
+
+	wmb();
 
 	greth_enable_tx(greth);
 
 	return NETDEV_TX_OK;
 
 frag_map_error:
-	/* Unmap SKB mappings that succeeded */
+	/* Unmap SKB mappings that succeeded and disable descriptor */
 	for (i = 0; greth->tx_next + i != curr_tx; i++) {
 		bdp = greth->tx_bd_base + greth->tx_next + i;
 		dma_unmap_single(greth->dev,
 				 greth_read_bd(&bdp->addr),
 				 greth_read_bd(&bdp->stat) & GRETH_BD_LEN,
 				 DMA_TO_DEVICE);
+		greth_write_bd(&bdp->stat, 0);
 	}
 map_error:
 	if (net_ratelimit())
-- 
1.5.4

[PATCH 07/10] GRETH: fixed skb buffer memory leak on frame errors

[Daniel Hellstrom]

A new SKB buffer should not be allocated when the old SKB is reused.

Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
 drivers/net/greth.c |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/net/greth.c b/drivers/net/greth.c
index 72a4317..888dc65 100644
--- a/drivers/net/greth.c
+++ b/drivers/net/greth.c
@@ -880,10 +880,9 @@ static int greth_rx_gbit(struct net_device *dev, int limit)
 			}
 		}
 
-		/* Allocate new skb to replace current */
-		newskb = netdev_alloc_skb(dev, MAX_FRAME_SIZE + NET_IP_ALIGN);
-
-		if (!bad && newskb) {
+		/* Allocate new skb to replace current, not needed if the
+		 * current skb can be reused */
+		if (!bad && (newskb=netdev_alloc_skb(dev, MAX_FRAME_SIZE + NET_IP_ALIGN))) {
 			skb_reserve(newskb, NET_IP_ALIGN);
 
 			dma_addr = dma_map_single(greth->dev,
@@ -920,11 +919,22 @@ static int greth_rx_gbit(struct net_device *dev, int limit)
 				if (net_ratelimit())
 					dev_warn(greth->dev, "Could not create DMA mapping, dropping packet\n");
 				dev_kfree_skb(newskb);
+				/* reusing current skb, so it is a drop */
 				dev->stats.rx_dropped++;
 			}
+		} else if (bad) {
+			/* Bad Frame transfer, the skb is reused */
+			dev->stats.rx_dropped++;
 		} else {
+			/* Failed Allocating a new skb. This is rather stupid
+			 * but the current "filled" skb is reused, as if
+			 * transfer failure. One could argue that RX descriptor
+			 * table handling should be divided into cleaning and
+			 * filling as the TX part of the driver
+			 */
 			if (net_ratelimit())
 				dev_warn(greth->dev, "Could not allocate SKB, dropping packet\n");
+			/* reusing current skb, so it is a drop */
 			dev->stats.rx_dropped++;
 		}
 
-- 
1.5.4

[PATCH 08/10] GRETH: avoid writing bad speed/duplex when setting transfer mode

[Daniel Hellstrom]

Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
 drivers/net/greth.c |   19 ++++++++-----------
 1 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/net/greth.c b/drivers/net/greth.c
index 888dc65..fea1e20 100644
--- a/drivers/net/greth.c
+++ b/drivers/net/greth.c
@@ -1242,29 +1242,26 @@ static void greth_link_change(struct net_device *dev)
 	struct greth_private *greth = netdev_priv(dev);
 	struct phy_device *phydev = greth->phy;
 	unsigned long flags;
-
 	int status_change = 0;
+	u32 ctrl;
 
 	spin_lock_irqsave(&greth->devlock, flags);
 
 	if (phydev->link) {
 
 		if ((greth->speed != phydev->speed) || (greth->duplex != phydev->duplex)) {
-
-			GRETH_REGANDIN(greth->regs->control,
-				       ~(GRETH_CTRL_FD | GRETH_CTRL_SP | GRETH_CTRL_GB));
+			ctrl = GRETH_REGLOAD(greth->regs->control) &
+			       ~(GRETH_CTRL_FD | GRETH_CTRL_SP | GRETH_CTRL_GB);
 
 			if (phydev->duplex)
-				GRETH_REGORIN(greth->regs->control, GRETH_CTRL_FD);
-
-			if (phydev->speed == SPEED_100) {
-
-				GRETH_REGORIN(greth->regs->control, GRETH_CTRL_SP);
-			}
+				ctrl |= GRETH_CTRL_FD;
 
+			if (phydev->speed == SPEED_100)
+				ctrl |= GRETH_CTRL_SP;
 			else if (phydev->speed == SPEED_1000)
-				GRETH_REGORIN(greth->regs->control, GRETH_CTRL_GB);
+				ctrl |= GRETH_CTRL_GB;
 
+			GRETH_REGSAVE(greth->regs->control, ctrl);
 			greth->speed = phydev->speed;
 			greth->duplex = phydev->duplex;
 			status_change = 1;
-- 
1.5.4

[PATCH 09/10] GRETH: handle frame error interrupts

[Daniel Hellstrom]

Frame error interrupts must also be handled since the RX flag only indicates
successful reception, it is unlikely but the old code may lead to dead lock
if 128 error frames are recieved in a row.

Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
 drivers/net/greth.c |    9 +++++----
 drivers/net/greth.h |    2 ++
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/greth.c b/drivers/net/greth.c
index fea1e20..b9623d2 100644
--- a/drivers/net/greth.c
+++ b/drivers/net/greth.c
@@ -596,12 +596,13 @@ static irqreturn_t greth_interrupt(int irq, void *dev_id)
 	status = GRETH_REGLOAD(greth->regs->status);
 
 	/* Handle rx and tx interrupts through poll */
-	if (status & (GRETH_INT_RX | GRETH_INT_TX)) {
+	if (status & (GRETH_INT_RE | GRETH_INT_RX |
+		      GRETH_INT_TE | GRETH_INT_TX)) {
 
 		/* Clear interrupt status */
-		GRETH_REGORIN(greth->regs->status,
-			      status & (GRETH_INT_RX | GRETH_INT_TX));
-
+		GRETH_REGSAVE(greth->regs->status,
+			      status & (GRETH_INT_RE | GRETH_INT_RX |
+					GRETH_INT_TE | GRETH_INT_TX));
 		retval = IRQ_HANDLED;
 
 		/* Disable interrupts and schedule poll() */
diff --git a/drivers/net/greth.h b/drivers/net/greth.h
index 9414169..f97f553 100644
--- a/drivers/net/greth.h
+++ b/drivers/net/greth.h
@@ -23,6 +23,7 @@
 #define GRETH_BD_LEN 0x7FF
 
 #define GRETH_TXEN 0x1
+#define GRETH_INT_TE 0x2
 #define GRETH_INT_TX 0x8
 #define GRETH_TXI 0x4
 #define GRETH_TXBD_STATUS 0x0001C000
@@ -35,6 +36,7 @@
 #define GRETH_TXBD_ERR_UE 0x4000
 #define GRETH_TXBD_ERR_AL 0x8000
 
+#define GRETH_INT_RE         0x1
 #define GRETH_INT_RX         0x4
 #define GRETH_RXEN           0x2
 #define GRETH_RXI            0x8
-- 
1.5.4

[PATCH 10/10] GRETH: resolve SMP issues and other problems

[Daniel Hellstrom]

Fixes the following:
1. POLL should not enable IRQ when work is not completed
2. No locking between TX descriptor cleaning and XMIT descriptor handling
3. No locking between RX POLL and XMIT modifying control register
4. Since TX cleaning (called from POLL) is running in parallel with XMIT
   unnecessary locking is needed.
5. IRQ handler looks at RX frame status solely, this is wrong when IRQ is
   temporarily disabled (in POLL), and when IRQ is shared.
6. IRQ handler clears IRQ status, which is unnecessary
7. TX queue was stopped in preventing cause when not MAX_SKB_FRAGS+1
   descriptors were available after a SKB been scheduled by XMIT. Instead
   the TX queue is stopped first when not enough descriptors are available
   upon entering XMIT.

It was hard to split up this patch in smaller pieces since all are tied
together somehow.

Note the RX flag used in the interrupt handler does not signal that
interrupt was asserted, but that a frame was received. Same goes for TX.
Also, IRQ is not asserted when the RX flag is set before enabling IRQ
enable until a new frame is received. So extra care must be taken to
avoid enabling IRQ and all descriptors are already used, hence dead lock
will upon us. See new POLL implementation that enableds IRQ then look at
the RX flag to determine if one or more IRQs may have been missed. TX/RX
flags are cleared before handling previously enabled descriptors, this
ensures that the RX/TX flags are valid when determining if IRQ should be
turned on again.

By moving TX cleaning from POLL to XMIT in the standard case, removes some
locking trouble. Enabling TX cleaning from poll only when not enough TX
descriptors are available is safe because the TX queue is at the same time
stopped, thus XMIT will not be called. The TX queue is woken up again when
enough descriptrs are available.

TX Frames are always enabled with IRQ, however the TX IRQ Enable flag will
not be enabled until XMIT must wait for free descriptors.

Locking RX and XMIT parts of the driver from each other is needed because
the RX/TX enable bits share the same register.

Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
 drivers/net/greth.c |  159 +++++++++++++++++++++++++++++---------------------
 1 files changed, 92 insertions(+), 67 deletions(-)

diff --git a/drivers/net/greth.c b/drivers/net/greth.c
index b9623d2..954f65a 100644
--- a/drivers/net/greth.c
+++ b/drivers/net/greth.c
@@ -1,7 +1,7 @@
 /*
  * Aeroflex Gaisler GRETH 10/100/1G Ethernet MAC.
  *
- * 2005-2009 (c) Aeroflex Gaisler AB
+ * 2005-2010 (c) Aeroflex Gaisler AB
  *
  * This driver supports GRETH 10/100 and GRETH 10/100/1G Ethernet MACs
  * available in the GRLIB VHDL IP core library.
@@ -402,12 +402,20 @@ greth_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	struct greth_private *greth = netdev_priv(dev);
 	struct greth_bd *bdp;
 	int err = NETDEV_TX_OK;
-	u32 status, dma_addr;
+	u32 status, dma_addr, ctrl;
+	unsigned long flags;
 
-	bdp = greth->tx_bd_base + greth->tx_next;
+	/* Clean TX Ring */
+	greth_clean_tx(greth->netdev);
 
 	if (unlikely(greth->tx_free <= 0)) {
+		spin_lock_irqsave(&greth->devlock, flags);/*save from poll/irq*/
+		ctrl = GRETH_REGLOAD(greth->regs->control);
+		/* Enable TX IRQ only if not already in poll() routine */
+		if (ctrl & GRETH_RXI)
+			GRETH_REGSAVE(greth->regs->control, ctrl | GRETH_TXI);
 		netif_stop_queue(dev);
+		spin_unlock_irqrestore(&greth->devlock, flags);
 		return NETDEV_TX_BUSY;
 	}
 
@@ -420,13 +428,14 @@ greth_start_xmit(struct sk_buff *skb, struct net_device *dev)
 		goto out;
 	}
 
+	bdp = greth->tx_bd_base + greth->tx_next;
 	dma_addr = greth_read_bd(&bdp->addr);
 
 	memcpy((unsigned char *) phys_to_virt(dma_addr), skb->data, skb->len);
 
 	dma_sync_single_for_device(greth->dev, dma_addr, skb->len, DMA_TO_DEVICE);
 
-	status = GRETH_BD_EN | (skb->len & GRETH_BD_LEN);
+	status = GRETH_BD_EN | GRETH_BD_IE | (skb->len & GRETH_BD_LEN);
 
 	/* Wrap around descriptor ring */
 	if (greth->tx_next == GRETH_TXBD_NUM_MASK) {
@@ -436,22 +445,11 @@ greth_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	greth->tx_next = NEXT_TX(greth->tx_next);
 	greth->tx_free--;
 
-	/* No more descriptors */
-	if (unlikely(greth->tx_free == 0)) {
-
-		/* Free transmitted descriptors */
-		greth_clean_tx(dev);
-
-		/* If nothing was cleaned, stop queue & wait for irq */
-		if (unlikely(greth->tx_free == 0)) {
-			status |= GRETH_BD_IE;
-			netif_stop_queue(dev);
-		}
-	}
-
 	/* Write descriptor control word and enable transmission */
 	greth_write_bd(&bdp->stat, status);
+	spin_lock_irqsave(&greth->devlock, flags); /*save from poll/irq*/
 	greth_enable_tx(greth);
+	spin_unlock_irqrestore(&greth->devlock, flags);
 
 out:
 	dev_kfree_skb(skb);
@@ -464,13 +462,23 @@ greth_start_xmit_gbit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct greth_private *greth = netdev_priv(dev);
 	struct greth_bd *bdp;
-	u32 status = 0, dma_addr;
+	u32 status = 0, dma_addr, ctrl;
 	int curr_tx, nr_frags, i, err = NETDEV_TX_OK;
+	unsigned long flags;
 
 	nr_frags = skb_shinfo(skb)->nr_frags;
 
+	/* Clean TX Ring */
+	greth_clean_tx_gbit(dev);
+
 	if (greth->tx_free < nr_frags + 1) {
+		spin_lock_irqsave(&greth->devlock, flags);/*save from poll/irq*/
+		ctrl = GRETH_REGLOAD(greth->regs->control);
+		/* Enable TX IRQ only if not already in poll() routine */
+		if (ctrl & GRETH_RXI)
+			GRETH_REGSAVE(greth->regs->control, ctrl | GRETH_TXI);
 		netif_stop_queue(dev);
+		spin_unlock_irqrestore(&greth->devlock, flags);
 		err = NETDEV_TX_BUSY;
 		goto out;
 	}
@@ -523,14 +531,8 @@ greth_start_xmit_gbit(struct sk_buff *skb, struct net_device *dev)
 		/* More fragments left */
 		if (i < nr_frags - 1)
 			status |= GRETH_TXBD_MORE;
-
-		/* ... last fragment, check if out of descriptors  */
-		else if (greth->tx_free - nr_frags - 1 < (MAX_SKB_FRAGS + 1)) {
-
-			/* Enable interrupts and stop queue */
-			status |= GRETH_BD_IE;
-			netif_stop_queue(dev);
-		}
+		else
+			status |= GRETH_BD_IE; /* enable IRQ on last fragment */
 
 		greth_write_bd(&bdp->stat, status);
 
@@ -558,7 +560,9 @@ greth_start_xmit_gbit(struct sk_buff *skb, struct net_device *dev)
 
 	wmb();
 
+	spin_lock_irqsave(&greth->devlock, flags); /*save from poll/irq*/
 	greth_enable_tx(greth);
+	spin_unlock_irqrestore(&greth->devlock, flags);
 
 	return NETDEV_TX_OK;
 
@@ -580,12 +584,11 @@ out:
 	return err;
 }
 
-
 static irqreturn_t greth_interrupt(int irq, void *dev_id)
 {
 	struct net_device *dev = dev_id;
 	struct greth_private *greth;
-	u32 status;
+	u32 status, ctrl;
 	irqreturn_t retval = IRQ_NONE;
 
 	greth = netdev_priv(dev);
@@ -595,14 +598,15 @@ static irqreturn_t greth_interrupt(int irq, void *dev_id)
 	/* Get the interrupt events that caused us to be here. */
 	status = GRETH_REGLOAD(greth->regs->status);
 
-	/* Handle rx and tx interrupts through poll */
-	if (status & (GRETH_INT_RE | GRETH_INT_RX |
-		      GRETH_INT_TE | GRETH_INT_TX)) {
+	/* Must see if interrupts are enabled also, INT_TX|INT_RX flags may be
+	 * set regardless of whether IRQ is enabled or not. Especially
+	 * important when shared IRQ.
+	 */
+	ctrl = GRETH_REGLOAD(greth->regs->control);
 
-		/* Clear interrupt status */
-		GRETH_REGSAVE(greth->regs->status,
-			      status & (GRETH_INT_RE | GRETH_INT_RX |
-					GRETH_INT_TE | GRETH_INT_TX));
+	/* Handle rx and tx interrupts through poll */
+	if (((status & (GRETH_INT_RE | GRETH_INT_RX)) && (ctrl & GRETH_RXI)) ||
+	    ((status & (GRETH_INT_TE | GRETH_INT_TX)) && (ctrl & GRETH_TXI))) {
 		retval = IRQ_HANDLED;
 
 		/* Disable interrupts and schedule poll() */
@@ -626,6 +630,8 @@ static void greth_clean_tx(struct net_device *dev)
 
 	while (1) {
 		bdp = greth->tx_bd_base + greth->tx_last;
+		GRETH_REGSAVE(greth->regs->status, GRETH_INT_TE | GRETH_INT_TX);
+		mb();
 		stat = greth_read_bd(&bdp->stat);
 
 		if (unlikely(stat & GRETH_BD_EN))
@@ -686,7 +692,10 @@ static void greth_clean_tx_gbit(struct net_device *dev)
 
 		/* We only clean fully completed SKBs */
 		bdp_last_frag = greth->tx_bd_base + SKIP_TX(greth->tx_last, nr_frags);
-		stat = bdp_last_frag->stat;
+
+		GRETH_REGSAVE(greth->regs->status, GRETH_INT_TE | GRETH_INT_TX);
+		mb();
+		stat = greth_read_bd(&bdp_last_frag->stat);
 
 		if (stat & GRETH_BD_EN)
 			break;
@@ -718,21 +727,9 @@ static void greth_clean_tx_gbit(struct net_device *dev)
 		greth->tx_free += nr_frags+1;
 		dev_kfree_skb(skb);
 	}
-	if (greth->tx_free > (MAX_SKB_FRAGS + 1)) {
-		netif_wake_queue(dev);
-	}
-}
 
-static int greth_pending_packets(struct greth_private *greth)
-{
-	struct greth_bd *bdp;
-	u32 status;
-	bdp = greth->rx_bd_base + greth->rx_cur;
-	status = greth_read_bd(&bdp->stat);
-	if (status & GRETH_BD_EN)
-		return 0;
-	else
-		return 1;
+	if (netif_queue_stopped(dev) && (greth->tx_free > (MAX_SKB_FRAGS+1)))
+		netif_wake_queue(dev);
 }
 
 static int greth_rx(struct net_device *dev, int limit)
@@ -743,20 +740,24 @@ static int greth_rx(struct net_device *dev, int limit)
 	int pkt_len;
 	int bad, count;
 	u32 status, dma_addr;
+	unsigned long flags;
 
 	greth = netdev_priv(dev);
 
 	for (count = 0; count < limit; ++count) {
 
 		bdp = greth->rx_bd_base + greth->rx_cur;
+		GRETH_REGSAVE(greth->regs->status, GRETH_INT_RE | GRETH_INT_RX);
+		mb();
 		status = greth_read_bd(&bdp->stat);
-		dma_addr = greth_read_bd(&bdp->addr);
-		bad = 0;
 
 		if (unlikely(status & GRETH_BD_EN)) {
 			break;
 		}
 
+		dma_addr = greth_read_bd(&bdp->addr);
+		bad = 0;
+
 		/* Check status for errors. */
 		if (unlikely(status & GRETH_RXBD_STATUS)) {
 			if (status & GRETH_RXBD_ERR_FT) {
@@ -818,7 +819,9 @@ static int greth_rx(struct net_device *dev, int limit)
 
 		dma_sync_single_for_device(greth->dev, dma_addr, MAX_FRAME_SIZE, DMA_FROM_DEVICE);
 
+		spin_lock_irqsave(&greth->devlock, flags); /* save from XMIT */
 		greth_enable_rx(greth);
+		spin_unlock_irqrestore(&greth->devlock, flags);
 
 		greth->rx_cur = NEXT_RX(greth->rx_cur);
 	}
@@ -852,6 +855,7 @@ static int greth_rx_gbit(struct net_device *dev, int limit)
 	int pkt_len;
 	int bad, count = 0;
 	u32 status, dma_addr;
+	unsigned long flags;
 
 	greth = netdev_priv(dev);
 
@@ -859,6 +863,8 @@ static int greth_rx_gbit(struct net_device *dev, int limit)
 
 		bdp = greth->rx_bd_base + greth->rx_cur;
 		skb = greth->rx_skbuff[greth->rx_cur];
+		GRETH_REGSAVE(greth->regs->status, GRETH_INT_RE | GRETH_INT_RX);
+		mb();
 		status = greth_read_bd(&bdp->stat);
 		bad = 0;
 
@@ -946,7 +952,9 @@ static int greth_rx_gbit(struct net_device *dev, int limit)
 
 		wmb();
 		greth_write_bd(&bdp->stat, status);
+		spin_lock_irqsave(&greth->devlock, flags);
 		greth_enable_rx(greth);
+		spin_unlock_irqrestore(&greth->devlock, flags);
 		greth->rx_cur = NEXT_RX(greth->rx_cur);
 	}
 
@@ -958,15 +966,18 @@ static int greth_poll(struct napi_struct *napi, int budget)
 {
 	struct greth_private *greth;
 	int work_done = 0;
+	unsigned long flags;
+	u32 mask, ctrl;
 	greth = container_of(napi, struct greth_private, napi);
 
-	if (greth->gbit_mac) {
-		greth_clean_tx_gbit(greth->netdev);
-	} else {
-		greth_clean_tx(greth->netdev);
+restart_txrx_poll:
+	if (netif_queue_stopped(greth->netdev)) {
+		if (greth->gbit_mac)
+			greth_clean_tx_gbit(greth->netdev);
+		else
+			greth_clean_tx(greth->netdev);
 	}
 
-restart_poll:
 	if (greth->gbit_mac) {
 		work_done += greth_rx_gbit(greth->netdev, budget - work_done);
 	} else {
@@ -975,15 +986,29 @@ restart_poll:
 
 	if (work_done < budget) {
 
-		napi_complete(napi);
+		spin_lock_irqsave(&greth->devlock, flags);
 
-		if (greth_pending_packets(greth)) {
-			napi_reschedule(napi);
-			goto restart_poll;
+		ctrl = GRETH_REGLOAD(greth->regs->control);
+		if (netif_queue_stopped(greth->netdev)) {
+			GRETH_REGSAVE(greth->regs->control,
+					ctrl | GRETH_TXI | GRETH_RXI);
+			mask = GRETH_INT_RX | GRETH_INT_RE |
+			       GRETH_INT_TX | GRETH_INT_TE;
+		} else {
+			GRETH_REGSAVE(greth->regs->control, ctrl | GRETH_RXI);
+			mask = GRETH_INT_RX | GRETH_INT_RE;
+		}
+
+		if (GRETH_REGLOAD(greth->regs->status) & mask) {
+			GRETH_REGSAVE(greth->regs->control, ctrl);
+			spin_unlock_irqrestore(&greth->devlock, flags);
+			goto restart_txrx_poll;
+		} else {
+			__napi_complete(napi);
+			spin_unlock_irqrestore(&greth->devlock, flags);
 		}
 	}
 
-	greth_enable_irqs(greth);
 	return work_done;
 }
 
@@ -1178,11 +1203,11 @@ static const struct ethtool_ops greth_ethtool_ops = {
 };
 
 static struct net_device_ops greth_netdev_ops = {
-	.ndo_open = greth_open,
-	.ndo_stop = greth_close,
-	.ndo_start_xmit = greth_start_xmit,
-	.ndo_set_mac_address = greth_set_mac_add,
-	.ndo_validate_addr 	= eth_validate_addr,
+	.ndo_open		= greth_open,
+	.ndo_stop		= greth_close,
+	.ndo_start_xmit		= greth_start_xmit,
+	.ndo_set_mac_address	= greth_set_mac_add,
+	.ndo_validate_addr	= eth_validate_addr,
 };
 
 static inline int wait_for_mdio(struct greth_private *greth)
-- 
1.5.4

Re: [PATCH 02/10] GRETH: added option to disable a device node from bootloader.

[David Miller]

From: Daniel Hellstrom <daniel@gaisler.com>
Date: Thu, 13 Jan 2011 09:25:27 +0100

> Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>

This is not how you do this.

Simply not present the device in the OpenFirmware tree at all.  If you
can make this special properly appear, you can also toss the device
node away completely.

There is zero reason whatsoever to create a special hack-job
non-standardized device node properly to do this.

Re: [PATCH 03/10] GRETH: added no_gbit option

[David Miller]

From: Daniel Hellstrom <daniel@gaisler.com>
Date: Thu, 13 Jan 2011 09:25:28 +0100

> For debug only. The driver does not report that it is GBit capable, instead
> it will report 10/100 mode to the generic PHY layer.
> 
> Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>

You may not add special purpose module parameters to your ethernet driver.

Instead, users can use ethtool to control what speeds the device will
try to advertise for during auto-negotiation, or use with a fixed
link configuration.

Re: [PATCH 04/10] GRETH: added greth_compat_mode module parameter

[David Miller]

From: Daniel Hellstrom <daniel@gaisler.com>
Date: Thu, 13 Jan 2011 09:25:29 +0100

> The greth_compat_mode option can be used to set a GRETH GBit capable MAC
> in operate as if the GRETH 10/100 device was found. The GRETH GBit supports
> TCP/UDP checksum offloading, unaligned frame buffers, scatter gather etc.
> Enabling this mode allows the developer to test the GRETH 10/100 device
> without all features mentioned above on a GBit MAC capable of the above.
> 
> Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>

No special device specific module parameters please.

What if every single driver author added something like this and they
all named it something different or made it behave in slightly differing
ways?

What kind of user experience would that result in?

It would result in a sucky one, which is why we avoid adding all kinds
of hacky driver specific module options.

Find a generic way to provide this functionality.

Re: [PATCH 02/10] GRETH: added option to disable a device node from bootloader.

[Daniel Hellstrom]

David Miller wrote:

>From: Daniel Hellstrom <daniel@gaisler.com>
>Date: Thu, 13 Jan 2011 09:25:27 +0100
>
>  
>
>>Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
>>    
>>
>
>This is not how you do this.
>
>Simply not present the device in the OpenFirmware tree at all.  If you
>can make this special properly appear, you can also toss the device
>node away completely.
>
>There is zero reason whatsoever to create a special hack-job
>non-standardized device node properly to do this.
>  
>
I agree, I will remove it. The optional parameter should be a setting 
for a particular device rather that a "disable device" parameter.

Thanks,
Daniel

Re: [PATCH 03/10] GRETH: added no_gbit option

[Daniel Hellstrom]

David Miller wrote:

>From: Daniel Hellstrom <daniel@gaisler.com>
>Date: Thu, 13 Jan 2011 09:25:28 +0100
>
>  
>
>>For debug only. The driver does not report that it is GBit capable, instead
>>it will report 10/100 mode to the generic PHY layer.
>>
>>Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
>>    
>>
>
>You may not add special purpose module parameters to your ethernet driver.
>
>Instead, users can use ethtool to control what speeds the device will
>try to advertise for during auto-negotiation, or use with a fixed
>link configuration.
>  
>
Ok, I will remove this patch.

Daniel

Re: [PATCH 04/10] GRETH: added greth_compat_mode module parameter

[Daniel Hellstrom]

David Miller wrote:

>From: Daniel Hellstrom <daniel@gaisler.com>
>Date: Thu, 13 Jan 2011 09:25:29 +0100
>
>  
>
>>The greth_compat_mode option can be used to set a GRETH GBit capable MAC
>>in operate as if the GRETH 10/100 device was found. The GRETH GBit supports
>>TCP/UDP checksum offloading, unaligned frame buffers, scatter gather etc.
>>Enabling this mode allows the developer to test the GRETH 10/100 device
>>without all features mentioned above on a GBit MAC capable of the above.
>>
>>Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
>>    
>>
>
>No special device specific module parameters please.
>
>What if every single driver author added something like this and they
>all named it something different or made it behave in slightly differing
>ways?
>
>What kind of user experience would that result in?
>
>It would result in a sucky one, which is why we avoid adding all kinds
>of hacky driver specific module options.
>
>Find a generic way to provide this functionality.
>  
>
I understand, it was not ment to be a user-experience, rather a 
developer debugging option. I will probably need an option like this in 
the future to avoid searching a very long time for very old hardware, I 
will try to find an other solution for it in the future.

I will remove the 3 GRETH patches and resubmit the others.

Thanks,
Daniel