[RESEND] [PATCH v2] [0/5] pasemi_mac: fixes and enhancements

[RESEND] [PATCH v2] [0/5] pasemi_mac: fixes and enhancements

[Olof Johansson]

Hi,

The five following patches contain a number of fixes and improvements
of the pasemi_mac driver:

1/5: A couple of minor bugfixes.
2/5: Move the IRQ mapping from the PCI layer under our platform, to
     the driver.
3/5: A rather large patch with various NAPI/performance-related fixes
     and enhancements.
4/5: phy support
5/5: use local-mac-address instead of mac-address if available.

(Changes from last time: Added 5/5, changes to 2/5 to use virq_to_hw()).


-Olof

[RESEND] [PATCH v2] [1/5] pasemi_mac: minor bugfixes

[Olof Johansson]

Ethernet bugfixes:

* Move the was_full/wake_queue logic from tx_intr to clean_tx
* Fix polarity in checks in pasemi_mac_close


Signed-off-by: Olof Johansson <olof@lixom.net>


Index: linux-2.6/drivers/net/pasemi_mac.c
===================================================================
--- linux-2.6.orig/drivers/net/pasemi_mac.c
+++ linux-2.6/drivers/net/pasemi_mac.c
@@ -451,9 +451,12 @@ static int pasemi_mac_clean_tx(struct pa
 	struct pas_dma_xct_descr *dp;
 	int start, count;
 	int flags;
+	int was_full;
 
 	spin_lock_irqsave(&mac->tx->lock, flags);
 
+	was_full = mac->tx->next_to_clean - mac->tx->next_to_use == TX_RING_SIZE;
+
 	start = mac->tx->next_to_clean;
 	count = 0;
 
@@ -478,6 +481,9 @@ static int pasemi_mac_clean_tx(struct pa
 	mac->tx->next_to_clean += count;
 	spin_unlock_irqrestore(&mac->tx->lock, flags);
 
+	if (was_full)
+		netif_wake_queue(mac->netdev);
+
 	return count;
 }
 
@@ -512,9 +518,6 @@ static irqreturn_t pasemi_mac_tx_intr(in
 	struct net_device *dev = data;
 	struct pasemi_mac *mac = netdev_priv(dev);
 	unsigned int reg;
-	int was_full;
-
-	was_full = mac->tx->next_to_clean - mac->tx->next_to_use == TX_RING_SIZE;
 
 	if (!(*mac->tx_status & PAS_STATUS_INT))
 		return IRQ_NONE;
@@ -528,9 +531,6 @@ static irqreturn_t pasemi_mac_tx_intr(in
 	pci_write_config_dword(mac->iob_pdev, PAS_IOB_DMA_TXCH_RESET(mac->dma_txch),
 			       reg);
 
-	if (was_full)
-		netif_wake_queue(dev);
-
 	return IRQ_HANDLED;
 }
 
@@ -662,40 +665,37 @@ static int pasemi_mac_close(struct net_d
 		pci_read_config_dword(mac->dma_pdev,
 				      PAS_DMA_TXCHAN_TCMDSTA(mac->dma_txch),
 				      &stat);
-		if (stat & PAS_DMA_TXCHAN_TCMDSTA_ACT)
+		if (!(stat & PAS_DMA_TXCHAN_TCMDSTA_ACT))
 			break;
 		cond_resched();
 	}
 
-	if (!(stat & PAS_DMA_TXCHAN_TCMDSTA_ACT)) {
+	if (stat & PAS_DMA_TXCHAN_TCMDSTA_ACT)
 		dev_err(&mac->dma_pdev->dev, "Failed to stop tx channel\n");
-	}
 
 	for (retries = 0; retries < MAX_RETRIES; retries++) {
 		pci_read_config_dword(mac->dma_pdev,
 				      PAS_DMA_RXCHAN_CCMDSTA(mac->dma_rxch),
 				      &stat);
-		if (stat & PAS_DMA_RXCHAN_CCMDSTA_ACT)
+		if (!(stat & PAS_DMA_RXCHAN_CCMDSTA_ACT))
 			break;
 		cond_resched();
 	}
 
-	if (!(stat & PAS_DMA_RXCHAN_CCMDSTA_ACT)) {
+	if (stat & PAS_DMA_RXCHAN_CCMDSTA_ACT)
 		dev_err(&mac->dma_pdev->dev, "Failed to stop rx channel\n");
-	}
 
 	for (retries = 0; retries < MAX_RETRIES; retries++) {
 		pci_read_config_dword(mac->dma_pdev,
 				      PAS_DMA_RXINT_RCMDSTA(mac->dma_if),
 				      &stat);
-		if (stat & PAS_DMA_RXINT_RCMDSTA_ACT)
+		if (!(stat & PAS_DMA_RXINT_RCMDSTA_ACT))
 			break;
 		cond_resched();
 	}
 
-	if (!(stat & PAS_DMA_RXINT_RCMDSTA_ACT)) {
+	if (stat & PAS_DMA_RXINT_RCMDSTA_ACT)
 		dev_err(&mac->dma_pdev->dev, "Failed to stop rx interface\n");
-	}
 
 	/* Then, disable the channel. This must be done separately from
 	 * stopping, since you can't disable when active.

Re: [RESEND] [PATCH v2] [1/5] pasemi_mac: minor bugfixes

[Jeff Garzik]

Olof Johansson wrote:
> Ethernet bugfixes:
> 
> * Move the was_full/wake_queue logic from tx_intr to clean_tx
> * Fix polarity in checks in pasemi_mac_close
> 
> 
> Signed-off-by: Olof Johansson <olof@lixom.net>
> 
> 
> Index: linux-2.6/drivers/net/pasemi_mac.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/pasemi_mac.c
> +++ linux-2.6/drivers/net/pasemi_mac.c
> @@ -451,9 +451,12 @@ static int pasemi_mac_clean_tx(struct pa
>  	struct pas_dma_xct_descr *dp;
>  	int start, count;
>  	int flags;
> +	int was_full;
>  
>  	spin_lock_irqsave(&mac->tx->lock, flags);
>  
> +	was_full = mac->tx->next_to_clean - mac->tx->next_to_use == TX_RING_SIZE;
> +
>  	start = mac->tx->next_to_clean;
>  	count = 0;
>  
> @@ -478,6 +481,9 @@ static int pasemi_mac_clean_tx(struct pa
>  	mac->tx->next_to_clean += count;
>  	spin_unlock_irqrestore(&mac->tx->lock, flags);
>  
> +	if (was_full)
> +		netif_wake_queue(mac->netdev);
> +

Isn't this was_full check redundant?  Using standard test-and-clear 
atomic logic, netif_wake_queue() will not issue spurious wakeups.  Take 
a look at its implementation in include/linux/netdevice.h.

	Jeff

Re: [RESEND] [PATCH v2] [1/5] pasemi_mac: minor bugfixes

[Olof Johansson]

On Sat, Apr 28, 2007 at 11:20:17AM -0400, Jeff Garzik wrote:

> >+	if (was_full)
> >+		netif_wake_queue(mac->netdev);
> >+
> 
> Isn't this was_full check redundant?  Using standard test-and-clear 
> atomic logic, netif_wake_queue() will not issue spurious wakeups.  Take 
> a look at its implementation in include/linux/netdevice.h.

It's not needed to avoid spurious wakeups, but test_and_clear_bit()
results in an atomic op even if the bit is already clear. I.e. it's a
bit heavy-handed to do if you can already know that it isn't set.

Either way it's no big deal, I'll take out the test for now. It can always
be added back in if it starts to climb on profiles when benchmarking.


-Olof

[RESEND] [PATCH v2] [2/5] pasemi_mac: irq mapping changes

[Olof Johansson]

Fixes for ethernet IRQ mapping, to be done in the driver instead of in
the platform setup code.


Signed-off-by: Olof Johansson <olof@lixom.net>


Index: powerpc/arch/powerpc/platforms/pasemi/pci.c
===================================================================
--- powerpc.orig/arch/powerpc/platforms/pasemi/pci.c
+++ powerpc/arch/powerpc/platforms/pasemi/pci.c
@@ -163,19 +163,6 @@ static void __init pas_fixup_phb_resourc
 }
 
 
-void __devinit pas_pci_irq_fixup(struct pci_dev *dev)
-{
-	/* DMA is special, 84 interrupts (128 -> 211), all but 128
-	 * need to be mapped by hand here.
-	 */
-	if (dev->vendor == 0x1959 && dev->device == 0xa007) {
-		int i;
-		for (i = 129; i < 212; i++)
-			irq_create_mapping(NULL, i);
-	}
-}
-
-
 void __init pas_pci_init(void)
 {
 	struct device_node *np, *root;
Index: powerpc/arch/powerpc/platforms/pasemi/setup.c
===================================================================
--- powerpc.orig/arch/powerpc/platforms/pasemi/setup.c
+++ powerpc/arch/powerpc/platforms/pasemi/setup.c
@@ -240,5 +240,4 @@ define_machine(pas) {
 	.check_legacy_ioport    = pas_check_legacy_ioport,
 	.progress		= pas_progress,
 	.machine_check_exception = pas_machine_check_handler,
-	.pci_irq_fixup		= pas_pci_irq_fixup,
 };
Index: powerpc/drivers/net/pasemi_mac.c
===================================================================
--- powerpc.orig/drivers/net/pasemi_mac.c
+++ powerpc/drivers/net/pasemi_mac.c
@@ -33,6 +33,8 @@
 #include <linux/tcp.h>
 #include <net/checksum.h>
 
+#include <asm/irq.h>
+
 #include "pasemi_mac.h"
 
 
@@ -537,6 +539,7 @@ static irqreturn_t pasemi_mac_tx_intr(in
 static int pasemi_mac_open(struct net_device *dev)
 {
 	struct pasemi_mac *mac = netdev_priv(dev);
+	int base_irq;
 	unsigned int flags;
 	int ret;
 
@@ -600,28 +603,37 @@ static int pasemi_mac_open(struct net_de
 	netif_start_queue(dev);
 	netif_poll_enable(dev);
 
-	ret = request_irq(mac->dma_pdev->irq + mac->dma_txch,
-			  &pasemi_mac_tx_intr, IRQF_DISABLED,
+	/* Interrupts are a bit different for our DMA controller: While
+	 * it's got one a regular PCI device header, the interrupt there
+	 * is really the base of the range it's using. Each tx and rx
+	 * channel has it's own interrupt source.
+	 */
+
+	base_irq = virq_to_hw(mac->dma_pdev->irq);
+
+	mac->tx_irq = irq_create_mapping(NULL, base_irq + mac->dma_txch);
+	mac->rx_irq = irq_create_mapping(NULL, base_irq + 20 + mac->dma_txch);
+
+	ret = request_irq(mac->tx_irq, &pasemi_mac_tx_intr, IRQF_DISABLED,
 			  mac->tx->irq_name, dev);
 	if (ret) {
 		dev_err(&mac->pdev->dev, "request_irq of irq %d failed: %d\n",
-		       mac->dma_pdev->irq + mac->dma_txch, ret);
+			base_irq + mac->dma_txch, ret);
 		goto out_tx_int;
 	}
 
-	ret = request_irq(mac->dma_pdev->irq + 20 + mac->dma_rxch,
-			  &pasemi_mac_rx_intr, IRQF_DISABLED,
+	ret = request_irq(mac->rx_irq, &pasemi_mac_rx_intr, IRQF_DISABLED,
 			  mac->rx->irq_name, dev);
 	if (ret) {
 		dev_err(&mac->pdev->dev, "request_irq of irq %d failed: %d\n",
-		       mac->dma_pdev->irq + 20 + mac->dma_rxch, ret);
+			base_irq + 20 + mac->dma_rxch, ret);
 		goto out_rx_int;
 	}
 
 	return 0;
 
 out_rx_int:
-	free_irq(mac->dma_pdev->irq + mac->dma_txch, dev);
+	free_irq(mac->tx_irq, dev);
 out_tx_int:
 	netif_poll_disable(dev);
 	netif_stop_queue(dev);
@@ -705,8 +717,8 @@ static int pasemi_mac_close(struct net_d
 	pci_write_config_dword(mac->dma_pdev,
 			       PAS_DMA_RXINT_RCMDSTA(mac->dma_if), 0);
 
-	free_irq(mac->dma_pdev->irq + mac->dma_txch, dev);
-	free_irq(mac->dma_pdev->irq + 20 + mac->dma_rxch, dev);
+	free_irq(mac->tx_irq, dev);
+	free_irq(mac->rx_irq, dev);
 
 	/* Free resources */
 	pasemi_mac_free_rx_resources(dev);
Index: powerpc/drivers/net/pasemi_mac.h
===================================================================
--- powerpc.orig/drivers/net/pasemi_mac.h
+++ powerpc/drivers/net/pasemi_mac.h
@@ -73,6 +73,8 @@ struct pasemi_mac {
 
 	struct pasemi_mac_txring *tx;
 	struct pasemi_mac_rxring *rx;
+	unsigned long	tx_irq;
+	unsigned long	rx_irq;
 };
 
 /* Software status descriptor (desc_info) */

[RESEND] [PATCH v2] [3/5] pasemi_mac: cleanups and rx performance improvements

[Olof Johansson]

NAPI fixes and cleanups for pasemi_mac:

* Timer changes/fixes
* Abstract out the rx intr restart to a separate function
* Similar function for tx intr to reset to a known clear state even if
  firmware used the same interface
* Add a copy-break and recycle the SKB in the driver for small
  packets
* Other minor changes to rx path

Signed-off-by: Olof Johansson <olof@lixom.net>

Index: powerpc/drivers/net/pasemi_mac.c
===================================================================
--- powerpc.orig/drivers/net/pasemi_mac.c
+++ powerpc/drivers/net/pasemi_mac.c
@@ -61,12 +61,6 @@
 
 #define BUF_SIZE 1646 /* 1500 MTU + ETH_HLEN + VLAN_HLEN + 2 64B cachelines */
 
-/* XXXOJN these should come out of the device tree some day */
-#define PAS_DMA_CAP_BASE   0xe00d0040
-#define PAS_DMA_CAP_SIZE   0x100
-#define PAS_DMA_COM_BASE   0xe00d0100
-#define PAS_DMA_COM_SIZE   0x100
-
 static struct pasdma_status *dma_status;
 
 static int pasemi_get_mac_addr(struct pasemi_mac *mac)
@@ -279,8 +273,8 @@ static void pasemi_mac_free_rx_resources
 	for (i = 0; i < RX_RING_SIZE; i++) {
 		info = &RX_DESC_INFO(mac, i);
 		dp = &RX_DESC(mac, i);
-		if (info->dma) {
-			if (info->skb) {
+		if (info->skb) {
+			if (info->dma) {
 				pci_unmap_single(mac->dma_pdev,
 						 info->dma,
 						 info->skb->len,
@@ -311,84 +305,122 @@ static void pasemi_mac_replenish_rx_ring
 	struct pasemi_mac *mac = netdev_priv(dev);
 	unsigned int i;
 	int start = mac->rx->next_to_fill;
-	unsigned int count;
+	unsigned int limit, count;
 
-	count = (mac->rx->next_to_clean + RX_RING_SIZE -
+	limit = (mac->rx->next_to_clean + RX_RING_SIZE -
 		 mac->rx->next_to_fill) & (RX_RING_SIZE - 1);
 
 	/* Check to see if we're doing first-time setup */
 	if (unlikely(mac->rx->next_to_clean == 0 && mac->rx->next_to_fill == 0))
-		count = RX_RING_SIZE;
+		limit = RX_RING_SIZE;
 
-	if (count <= 0)
+	if (limit <= 0)
 		return;
 
-	for (i = start; i < start + count; i++) {
+	i = start;
+
+	for (count = limit; count; count--) {
 		struct pasemi_mac_buffer *info = &RX_DESC_INFO(mac, i);
 		u64 *buff = &RX_BUFF(mac, i);
 		struct sk_buff *skb;
 		dma_addr_t dma;
 
-		skb = dev_alloc_skb(BUF_SIZE);
+		/* skb might still be in there for recycle on short receives */
+		if (info->skb)
+			skb = info->skb;
+		else
+			skb = dev_alloc_skb(BUF_SIZE);
 
-		if (!skb) {
-			count = i - start;
+		if (unlikely(!skb))
 			break;
-		}
 
 		skb->dev = dev;
 
 		dma = pci_map_single(mac->dma_pdev, skb->data, skb->len,
 				     PCI_DMA_FROMDEVICE);
 
-		if (dma_mapping_error(dma)) {
+		if (unlikely(dma_mapping_error(dma))) {
 			dev_kfree_skb_irq(info->skb);
-			count = i - start;
 			break;
 		}
 
 		info->skb = skb;
 		info->dma = dma;
 		*buff = XCT_RXB_LEN(BUF_SIZE) | XCT_RXB_ADDR(dma);
+		i++;
 	}
 
 	wmb();
 
 	pci_write_config_dword(mac->dma_pdev,
 			       PAS_DMA_RXCHAN_INCR(mac->dma_rxch),
-			       count);
+			       limit - count);
 	pci_write_config_dword(mac->dma_pdev,
 			       PAS_DMA_RXINT_INCR(mac->dma_if),
-			       count);
+			       limit - count);
+
+	mac->rx->next_to_fill += limit - count;
+}
+
+static void pasemi_mac_restart_rx_intr(struct pasemi_mac *mac)
+{
+	unsigned int reg, stat;
+	/* Re-enable packet count interrupts: finally
+	 * ack the packet count interrupt we got in rx_intr.
+	 */
+
+	pci_read_config_dword(mac->iob_pdev,
+			      PAS_IOB_DMA_RXCH_STAT(mac->dma_rxch),
+			      &stat);
+
+	reg = PAS_IOB_DMA_RXCH_RESET_PCNT(stat & PAS_IOB_DMA_RXCH_STAT_CNTDEL_M) |
+	      PAS_IOB_DMA_RXCH_RESET_PINTC;
+
+	pci_write_config_dword(mac->iob_pdev,
+			       PAS_IOB_DMA_RXCH_RESET(mac->dma_rxch),
+			       reg);
+}
+
+static void pasemi_mac_restart_tx_intr(struct pasemi_mac *mac)
+{
+	unsigned int reg, stat;
 
-	mac->rx->next_to_fill += count;
+	/* Re-enable packet count interrupts */
+	pci_read_config_dword(mac->iob_pdev,
+			      PAS_IOB_DMA_TXCH_STAT(mac->dma_txch), &stat);
+
+	reg = PAS_IOB_DMA_TXCH_RESET_PCNT(stat & PAS_IOB_DMA_TXCH_STAT_CNTDEL_M) |
+	      PAS_IOB_DMA_TXCH_RESET_PINTC;
+
+	pci_write_config_dword(mac->iob_pdev,
+				PAS_IOB_DMA_TXCH_RESET(mac->dma_txch), reg);
 }
 
+
 static int pasemi_mac_clean_rx(struct pasemi_mac *mac, int limit)
 {
-	unsigned int i;
-	int start, count;
+	unsigned int n;
+	int count;
+	struct pas_dma_xct_descr *dp;
+	struct pasemi_mac_buffer *info;
+	struct sk_buff *skb;
+	unsigned int i, len;
+	u64 macrx;
+	dma_addr_t dma;
 
 	spin_lock(&mac->rx->lock);
 
-	start = mac->rx->next_to_clean;
-	count = 0;
-
-	for (i = start; i < (start + RX_RING_SIZE) && count < limit; i++) {
-		struct pas_dma_xct_descr *dp;
-		struct pasemi_mac_buffer *info;
-		struct sk_buff *skb;
-		unsigned int j, len;
-		dma_addr_t dma;
+	n = mac->rx->next_to_clean;
 
+	for (count = limit; count; count--) {
 		rmb();
 
-		dp = &RX_DESC(mac, i);
+		dp = &RX_DESC(mac, n);
 
-		if (!(dp->macrx & XCT_MACRX_O))
-			break;
+		macrx = dp->macrx;
 
-		count++;
+		if (!(macrx & XCT_MACRX_O))
+			break;
 
 		info = NULL;
 
@@ -400,29 +432,42 @@ static int pasemi_mac_clean_rx(struct pa
 		 */
 
 		dma = (dp->ptr & XCT_PTR_ADDR_M);
-		for (j = start; j < (start + RX_RING_SIZE); j++) {
-			info = &RX_DESC_INFO(mac, j);
+		for (i = n; i < (n + RX_RING_SIZE); i++) {
+			info = &RX_DESC_INFO(mac, i);
 			if (info->dma == dma)
 				break;
 		}
 
-		BUG_ON(!info);
-		BUG_ON(info->dma != dma);
+		skb = info->skb;
+		info->dma = 0;
 
-		pci_unmap_single(mac->dma_pdev, info->dma, info->skb->len,
+		pci_unmap_single(mac->dma_pdev, dma, skb->len,
 				 PCI_DMA_FROMDEVICE);
 
-		skb = info->skb;
+		len = (macrx & XCT_MACRX_LLEN_M) >> XCT_MACRX_LLEN_S;
 
-		len = (dp->macrx & XCT_MACRX_LLEN_M) >> XCT_MACRX_LLEN_S;
+		if (len < 256) {
+			struct sk_buff *new_skb =
+			    netdev_alloc_skb(mac->netdev, len + NET_IP_ALIGN);
+			if (new_skb) {
+				skb_reserve(new_skb, NET_IP_ALIGN);
+				memcpy(new_skb->data - NET_IP_ALIGN,
+					skb->data - NET_IP_ALIGN,
+					len + NET_IP_ALIGN);
+				/* save the skb in buffer_info as good */
+				skb = new_skb;
+			}
+			/* else just continue with the old one */
+		} else
+			info->skb = NULL;
 
 		skb_put(skb, len);
 
 		skb->protocol = eth_type_trans(skb, mac->netdev);
 
-		if ((dp->macrx & XCT_MACRX_HTY_M) == XCT_MACRX_HTY_IPV4_OK) {
+		if ((macrx & XCT_MACRX_HTY_M) == XCT_MACRX_HTY_IPV4_OK) {
 			skb->ip_summed = CHECKSUM_COMPLETE;
-			skb->csum = (dp->macrx & XCT_MACRX_CSUM_M) >>
+			skb->csum = (macrx & XCT_MACRX_CSUM_M) >>
 					   XCT_MACRX_CSUM_S;
 		} else
 			skb->ip_summed = CHECKSUM_NONE;
@@ -432,13 +477,13 @@ static int pasemi_mac_clean_rx(struct pa
 
 		netif_receive_skb(skb);
 
-		info->dma = 0;
-		info->skb = NULL;
 		dp->ptr = 0;
 		dp->macrx = 0;
+
+		n++;
 	}
 
-	mac->rx->next_to_clean += count;
+	mac->rx->next_to_clean += limit - count;
 	pasemi_mac_replenish_rx_ring(mac->netdev);
 
 	spin_unlock(&mac->rx->lock);
@@ -496,18 +541,28 @@ static irqreturn_t pasemi_mac_rx_intr(in
 	struct pasemi_mac *mac = netdev_priv(dev);
 	unsigned int reg;
 
-	if (!(*mac->rx_status & PAS_STATUS_INT))
+	if (!(*mac->rx_status & PAS_STATUS_CAUSE_M))
 		return IRQ_NONE;
 
-	netif_rx_schedule(dev);
-	pci_write_config_dword(mac->iob_pdev, PAS_IOB_DMA_COM_TIMEOUTCFG,
-			       PAS_IOB_DMA_COM_TIMEOUTCFG_TCNT(0));
+	if (*mac->rx_status & PAS_STATUS_ERROR)
+		printk("rx_status reported error\n");
 
-	reg = PAS_IOB_DMA_RXCH_RESET_PINTC | PAS_IOB_DMA_RXCH_RESET_SINTC |
-	      PAS_IOB_DMA_RXCH_RESET_DINTC;
+	/* Don't reset packet count so it won't fire again but clear
+	 * all others.
+	 */
+
+	pci_read_config_dword(mac->dma_pdev, PAS_DMA_RXINT_RCMDSTA(mac->dma_if), &reg);
+
+	reg = 0;
+	if (*mac->rx_status & PAS_STATUS_SOFT)
+		reg |= PAS_IOB_DMA_RXCH_RESET_SINTC;
+	if (*mac->rx_status & PAS_STATUS_ERROR)
+		reg |= PAS_IOB_DMA_RXCH_RESET_DINTC;
 	if (*mac->rx_status & PAS_STATUS_TIMER)
 		reg |= PAS_IOB_DMA_RXCH_RESET_TINTC;
 
+	netif_rx_schedule(dev);
+
 	pci_write_config_dword(mac->iob_pdev,
 			       PAS_IOB_DMA_RXCH_RESET(mac->dma_rxch), reg);
 
@@ -521,14 +576,17 @@ static irqreturn_t pasemi_mac_tx_intr(in
 	struct pasemi_mac *mac = netdev_priv(dev);
 	unsigned int reg;
 
-	if (!(*mac->tx_status & PAS_STATUS_INT))
+	if (!(*mac->tx_status & PAS_STATUS_CAUSE_M))
 		return IRQ_NONE;
 
 	pasemi_mac_clean_tx(mac);
 
-	reg = PAS_IOB_DMA_TXCH_RESET_PINTC | PAS_IOB_DMA_TXCH_RESET_SINTC;
-	if (*mac->tx_status & PAS_STATUS_TIMER)
-		reg |= PAS_IOB_DMA_TXCH_RESET_TINTC;
+	reg = PAS_IOB_DMA_TXCH_RESET_PINTC;
+
+	if (*mac->tx_status & PAS_STATUS_SOFT)
+		reg |= PAS_IOB_DMA_TXCH_RESET_SINTC;
+	if (*mac->tx_status & PAS_STATUS_ERROR)
+		reg |= PAS_IOB_DMA_TXCH_RESET_DINTC;
 
 	pci_write_config_dword(mac->iob_pdev, PAS_IOB_DMA_TXCH_RESET(mac->dma_txch),
 			       reg);
@@ -564,10 +622,18 @@ static int pasemi_mac_open(struct net_de
 	flags |= PAS_MAC_CFG_PCFG_TSR_1G | PAS_MAC_CFG_PCFG_SPD_1G;
 
 	pci_write_config_dword(mac->iob_pdev, PAS_IOB_DMA_RXCH_CFG(mac->dma_rxch),
-			       PAS_IOB_DMA_RXCH_CFG_CNTTH(30));
+			       PAS_IOB_DMA_RXCH_CFG_CNTTH(1));
+
+	pci_write_config_dword(mac->iob_pdev, PAS_IOB_DMA_TXCH_CFG(mac->dma_txch),
+			       PAS_IOB_DMA_TXCH_CFG_CNTTH(32));
 
+	/* Clear out any residual packet count state from firmware */
+	pasemi_mac_restart_rx_intr(mac);
+	pasemi_mac_restart_tx_intr(mac);
+
+	/* 0xffffff is max value, about 16ms */
 	pci_write_config_dword(mac->iob_pdev, PAS_IOB_DMA_COM_TIMEOUTCFG,
-			       PAS_IOB_DMA_COM_TIMEOUTCFG_TCNT(1000000));
+			       PAS_IOB_DMA_COM_TIMEOUTCFG_TCNT(0xffffff));
 
 	pci_write_config_dword(mac->pdev, PAS_MAC_CFG_PCFG, flags);
 
@@ -586,7 +652,7 @@ static int pasemi_mac_open(struct net_de
 	/* enable rx if */
 	pci_write_config_dword(mac->dma_pdev,
 			       PAS_DMA_RXINT_RCMDSTA(mac->dma_if),
-			       PAS_DMA_RXINT_RCMDSTA_EN);
+			       PAS_DMA_RXINT_RCMDSTA_EN|PAS_DMA_RXINT_RCMDSTA_MBT);
 
 	/* enable rx channel */
 	pci_write_config_dword(mac->dma_pdev,
@@ -836,18 +902,18 @@ static int pasemi_mac_poll(struct net_de
 
 	pkts = pasemi_mac_clean_rx(mac, limit);
 
+	dev->quota -= pkts;
+	*budget -= pkts;
+
 	if (pkts < limit) {
 		/* all done, no more packets present */
 		netif_rx_complete(dev);
 
-		/* re-enable receive interrupts */
-		pci_write_config_dword(mac->iob_pdev, PAS_IOB_DMA_COM_TIMEOUTCFG,
-				       PAS_IOB_DMA_COM_TIMEOUTCFG_TCNT(1000000));
+		pasemi_mac_restart_rx_intr(mac);
+
 		return 0;
 	} else {
 		/* used up our quantum, so reschedule */
-		dev->quota -= pkts;
-		*budget -= pkts;
 		return 1;
 	}
 }
Index: powerpc/drivers/net/pasemi_mac.h
===================================================================
--- powerpc.orig/drivers/net/pasemi_mac.h
+++ powerpc/drivers/net/pasemi_mac.h
@@ -195,11 +195,15 @@ enum {
 #define PAS_DMA_RXINT_RCMDSTA(i)	(0x200+(i)*_PAS_DMA_RXINT_STRIDE)
 #define    PAS_DMA_RXINT_RCMDSTA_EN	0x00000001
 #define    PAS_DMA_RXINT_RCMDSTA_ST	0x00000002
-#define    PAS_DMA_RXINT_RCMDSTA_OO	0x00000100
-#define    PAS_DMA_RXINT_RCMDSTA_BP	0x00000200
-#define    PAS_DMA_RXINT_RCMDSTA_DR	0x00000400
+#define    PAS_DMA_RXINT_RCMDSTA_MBT	0x00000008
+#define    PAS_DMA_RXINT_RCMDSTA_MDR	0x00000010
+#define    PAS_DMA_RXINT_RCMDSTA_MOO	0x00000020
+#define    PAS_DMA_RXINT_RCMDSTA_MBP	0x00000040
 #define    PAS_DMA_RXINT_RCMDSTA_BT	0x00000800
-#define    PAS_DMA_RXINT_RCMDSTA_TB	0x00001000
+#define    PAS_DMA_RXINT_RCMDSTA_DR	0x00001000
+#define    PAS_DMA_RXINT_RCMDSTA_OO	0x00002000
+#define    PAS_DMA_RXINT_RCMDSTA_BP	0x00004000
+#define    PAS_DMA_RXINT_RCMDSTA_TB	0x00008000
 #define    PAS_DMA_RXINT_RCMDSTA_ACT	0x00010000
 #define    PAS_DMA_RXINT_RCMDSTA_DROPS_M	0xfffe0000
 #define    PAS_DMA_RXINT_RCMDSTA_DROPS_S	17
@@ -299,6 +303,7 @@ enum {
 #define    PAS_STATUS_DCNT_S		16
 #define    PAS_STATUS_BPCNT_M		0x0000ffff00000000ull
 #define    PAS_STATUS_BPCNT_S		32
+#define    PAS_STATUS_CAUSE_M		0xf000000000000000ull
 #define    PAS_STATUS_TIMER		0x1000000000000000ull
 #define    PAS_STATUS_ERROR		0x2000000000000000ull
 #define    PAS_STATUS_SOFT		0x4000000000000000ull

Re: [RESEND] [PATCH v2] [3/5] pasemi_mac: cleanups and rx performance improvements

[Jeff Garzik]

Olof Johansson wrote:
> NAPI fixes and cleanups for pasemi_mac:
> 
> * Timer changes/fixes
> * Abstract out the rx intr restart to a separate function
> * Similar function for tx intr to reset to a known clear state even if
>   firmware used the same interface
> * Add a copy-break and recycle the SKB in the driver for small
>   packets
> * Other minor changes to rx path

Split out the "abstract out function foo" style changes into a separate 
patch.

This creates a smaller and much more reviewable second patch as a 
result.  Doing so also creates a nice, clean git-bisect point that can 
be used to guarantee the validity of the first patch.

	Jeff

Re: [RESEND] [PATCH v2] [3/5] pasemi_mac: cleanups and rx performance improvements

[Olof Johansson]

On Sat, Apr 28, 2007 at 11:22:43AM -0400, Jeff Garzik wrote:
> Olof Johansson wrote:
> >NAPI fixes and cleanups for pasemi_mac:
> >
> >* Timer changes/fixes
> >* Abstract out the rx intr restart to a separate function
> >* Similar function for tx intr to reset to a known clear state even if
> >  firmware used the same interface
> >* Add a copy-break and recycle the SKB in the driver for small
> >  packets
> >* Other minor changes to rx path
> 
> Split out the "abstract out function foo" style changes into a separate 
> patch.
> 
> This creates a smaller and much more reviewable second patch as a 
> result.  Doing so also creates a nice, clean git-bisect point that can 
> be used to guarantee the validity of the first patch.

Ok, I was really asking for this one...

Even that didn't result in that much of a more readible patch.

I've split it up in a total of 5. I'll repost the series shortly.


Thanks,

-Olof

[RESEND] [PATCH v2] [4/5] pasemi_mac: phy support

[Olof Johansson]

PHY support for pasemi_mac. Also add msg_enable flags for future
disablement of the link messages.

Signed-off-by: Olof Johansson <olof@lixom.net>


Index: powerpc/drivers/net/pasemi_mac.c
===================================================================
--- powerpc.orig/drivers/net/pasemi_mac.c
+++ powerpc/drivers/net/pasemi_mac.c
@@ -594,6 +592,110 @@ static irqreturn_t pasemi_mac_tx_intr(in
 	return IRQ_HANDLED;
 }
 
+static void pasemi_adjust_link(struct net_device *dev)
+{
+	struct pasemi_mac *mac = netdev_priv(dev);
+	int msg;
+	unsigned int flags;
+	unsigned int new_flags;
+
+	if (!mac->phydev->link) {
+		/* If no link, MAC speed settings don't matter. Just report
+		 * link down and return.
+		 */
+		if (mac->link && netif_msg_link(mac))
+			printk(KERN_INFO "%s: Link is down.\n", dev->name);
+
+		netif_carrier_off(dev);
+		mac->link = 0;
+
+		return;
+	} else
+		netif_carrier_on(dev);
+
+	pci_read_config_dword(mac->pdev, PAS_MAC_CFG_PCFG, &flags);
+	new_flags = flags & ~(PAS_MAC_CFG_PCFG_HD | PAS_MAC_CFG_PCFG_SPD_M);
+
+	if (!mac->phydev->duplex)
+		new_flags |= PAS_MAC_CFG_PCFG_HD;
+
+	switch (mac->phydev->speed) {
+	case 1000:
+		new_flags |= PAS_MAC_CFG_PCFG_SPD_1G;
+		break;
+	case 100:
+		new_flags |= PAS_MAC_CFG_PCFG_SPD_100M;
+		break;
+	case 10:
+		new_flags |= PAS_MAC_CFG_PCFG_SPD_10M;
+		break;
+	default:
+		printk("Unsupported speed %d\n", mac->phydev->speed);
+	}
+
+	/* Print on link or speed/duplex change */
+	msg = mac->link != mac->phydev->link || flags != new_flags;
+
+	mac->duplex = mac->phydev->duplex;
+	mac->speed = mac->phydev->speed;
+	mac->link = mac->phydev->link;
+
+	if (new_flags != flags)
+		pci_write_config_dword(mac->pdev, PAS_MAC_CFG_PCFG, new_flags);
+
+	if (msg && netif_msg_link(mac))
+		printk(KERN_INFO "%s: Link is up at %d Mbps, %s duplex.\n",
+		       dev->name, mac->speed, mac->duplex ? "full" : "half");
+}
+
+static int pasemi_mac_phy_init(struct net_device *dev)
+{
+	struct pasemi_mac *mac = netdev_priv(dev);
+	struct device_node *dn, *phy_dn;
+	struct phy_device *phydev;
+	unsigned int phy_id;
+	const phandle *ph;
+	const unsigned int *prop;
+	struct resource r;
+	int ret;
+
+	dn = pci_device_to_OF_node(mac->pdev);
+	ph = get_property(dn, "phy-handle", NULL);
+	if (!ph)
+		return -ENODEV;
+	phy_dn = of_find_node_by_phandle(*ph);
+
+	prop = get_property(phy_dn, "reg", NULL);
+	ret = of_address_to_resource(phy_dn->parent, 0, &r);
+	if (ret)
+		goto err;
+
+	phy_id = *prop;
+	snprintf(mac->phy_id, BUS_ID_SIZE, PHY_ID_FMT, (int)r.start, phy_id);
+
+	of_node_put(phy_dn);
+
+	mac->link = 0;
+	mac->speed = 0;
+	mac->duplex = -1;
+
+	phydev = phy_connect(dev, mac->phy_id, &pasemi_adjust_link, 0, PHY_INTERFACE_MODE_SGMII);
+
+	if (IS_ERR(phydev)) {
+		printk(KERN_ERR "%s: Could not attach to phy\n", dev->name);
+		return PTR_ERR(phydev);
+	}
+
+	mac->phydev = phydev;
+
+	return 0;
+
+err:
+	of_node_put(phy_dn);
+	return -ENODEV;
+}
+
+
 static int pasemi_mac_open(struct net_device *dev)
 {
 	struct pasemi_mac *mac = netdev_priv(dev);
@@ -667,6 +769,13 @@ static int pasemi_mac_open(struct net_de
 
 	pasemi_mac_replenish_rx_ring(dev);
 
+	ret = pasemi_mac_phy_init(dev);
+	/* Some configs don't have PHYs (XAUI etc), so don't complain about
+	 * failed init due to -ENODEV.
+	 */
+	if (ret && ret != -ENODEV)
+		dev_warn(&mac->pdev->dev, "phy init failed: %d\n", ret);
+
 	netif_start_queue(dev);
 	netif_poll_enable(dev);
 
@@ -697,6 +806,9 @@ static int pasemi_mac_open(struct net_de
 		goto out_rx_int;
 	}
 
+	if (mac->phydev)
+		phy_start(mac->phydev);
+
 	return 0;
 
 out_rx_int:
@@ -720,6 +832,11 @@ static int pasemi_mac_close(struct net_d
 	unsigned int stat;
 	int retries;
 
+	if (mac->phydev) {
+		phy_stop(mac->phydev);
+		phy_disconnect(mac->phydev);
+	}
+
 	netif_stop_queue(dev);
 
 	/* Clean out any pending buffers */
@@ -1013,6 +1130,9 @@ pasemi_mac_probe(struct pci_dev *pdev, c
 	mac->rx_status = &dma_status->rx_sta[mac->dma_rxch];
 	mac->tx_status = &dma_status->tx_sta[mac->dma_txch];
 
+	/* Enable most messages by default */
+	mac->msg_enable = (NETIF_MSG_IFUP << 1 ) - 1;
+
 	err = register_netdev(dev);
 
 	if (err) {
Index: powerpc/drivers/net/pasemi_mac.h
===================================================================
--- powerpc.orig/drivers/net/pasemi_mac.h
+++ powerpc/drivers/net/pasemi_mac.h
@@ -24,6 +24,7 @@
 #include <linux/ethtool.h>
 #include <linux/netdevice.h>
 #include <linux/spinlock.h>
+#include <linux/phy.h>
 
 struct pasemi_mac_txring {
 	spinlock_t	 lock;
@@ -54,6 +55,7 @@ struct pasemi_mac {
 	struct pci_dev *pdev;
 	struct pci_dev *dma_pdev;
 	struct pci_dev *iob_pdev;
+	struct phy_device *phydev;
 	struct net_device_stats stats;
 
 	/* Pointer to the cacheable per-channel status registers */
@@ -75,6 +77,12 @@ struct pasemi_mac {
 	struct pasemi_mac_rxring *rx;
 	unsigned long	tx_irq;
 	unsigned long	rx_irq;
+	int	link;
+	int	speed;
+	int	duplex;
+
+	unsigned int	msg_enable;
+	char	phy_id[BUS_ID_SIZE];
 };
 
 /* Software status descriptor (desc_info) */
Index: powerpc/drivers/net/Kconfig
===================================================================
--- powerpc.orig/drivers/net/Kconfig
+++ powerpc/drivers/net/Kconfig
@@ -2490,6 +2490,7 @@ config NETXEN_NIC
 config PASEMI_MAC
 	tristate "PA Semi 1/10Gbit MAC"
 	depends on PPC64 && PCI
+	select PHYLIB
 	help
 	  This driver supports the on-chip 1/10Gbit Ethernet controller on
 	  PA Semi's PWRficient line of chips.

Re: [RESEND] [PATCH v2] [4/5] pasemi_mac: phy support

[Jeff Garzik]

Olof Johansson wrote:
> PHY support for pasemi_mac. Also add msg_enable flags for future
> disablement of the link messages.
> 
> Signed-off-by: Olof Johansson <olof@lixom.net>
> 
> 
> Index: powerpc/drivers/net/pasemi_mac.c
> ===================================================================
> --- powerpc.orig/drivers/net/pasemi_mac.c
> +++ powerpc/drivers/net/pasemi_mac.c
> @@ -594,6 +592,110 @@ static irqreturn_t pasemi_mac_tx_intr(in
>  	return IRQ_HANDLED;
>  }
>  
> +static void pasemi_adjust_link(struct net_device *dev)
> +{
> +	struct pasemi_mac *mac = netdev_priv(dev);
> +	int msg;
> +	unsigned int flags;
> +	unsigned int new_flags;
> +
> +	if (!mac->phydev->link) {
> +		/* If no link, MAC speed settings don't matter. Just report
> +		 * link down and return.
> +		 */
> +		if (mac->link && netif_msg_link(mac))
> +			printk(KERN_INFO "%s: Link is down.\n", dev->name);
> +
> +		netif_carrier_off(dev);
> +		mac->link = 0;
> +
> +		return;
> +	} else
> +		netif_carrier_on(dev);
> +
> +	pci_read_config_dword(mac->pdev, PAS_MAC_CFG_PCFG, &flags);
> +	new_flags = flags & ~(PAS_MAC_CFG_PCFG_HD | PAS_MAC_CFG_PCFG_SPD_M);
> +
> +	if (!mac->phydev->duplex)
> +		new_flags |= PAS_MAC_CFG_PCFG_HD;
> +
> +	switch (mac->phydev->speed) {
> +	case 1000:
> +		new_flags |= PAS_MAC_CFG_PCFG_SPD_1G;
> +		break;
> +	case 100:
> +		new_flags |= PAS_MAC_CFG_PCFG_SPD_100M;
> +		break;
> +	case 10:
> +		new_flags |= PAS_MAC_CFG_PCFG_SPD_10M;
> +		break;
> +	default:
> +		printk("Unsupported speed %d\n", mac->phydev->speed);
> +	}
> +
> +	/* Print on link or speed/duplex change */
> +	msg = mac->link != mac->phydev->link || flags != new_flags;
> +
> +	mac->duplex = mac->phydev->duplex;
> +	mac->speed = mac->phydev->speed;
> +	mac->link = mac->phydev->link;
> +
> +	if (new_flags != flags)
> +		pci_write_config_dword(mac->pdev, PAS_MAC_CFG_PCFG, new_flags);
> +
> +	if (msg && netif_msg_link(mac))
> +		printk(KERN_INFO "%s: Link is up at %d Mbps, %s duplex.\n",
> +		       dev->name, mac->speed, mac->duplex ? "full" : "half");
> +}
> +
> +static int pasemi_mac_phy_init(struct net_device *dev)
> +{
> +	struct pasemi_mac *mac = netdev_priv(dev);
> +	struct device_node *dn, *phy_dn;
> +	struct phy_device *phydev;
> +	unsigned int phy_id;
> +	const phandle *ph;
> +	const unsigned int *prop;
> +	struct resource r;
> +	int ret;
> +
> +	dn = pci_device_to_OF_node(mac->pdev);
> +	ph = get_property(dn, "phy-handle", NULL);
> +	if (!ph)
> +		return -ENODEV;
> +	phy_dn = of_find_node_by_phandle(*ph);
> +
> +	prop = get_property(phy_dn, "reg", NULL);
> +	ret = of_address_to_resource(phy_dn->parent, 0, &r);
> +	if (ret)
> +		goto err;
> +
> +	phy_id = *prop;
> +	snprintf(mac->phy_id, BUS_ID_SIZE, PHY_ID_FMT, (int)r.start, phy_id);
> +
> +	of_node_put(phy_dn);
> +
> +	mac->link = 0;
> +	mac->speed = 0;
> +	mac->duplex = -1;
> +
> +	phydev = phy_connect(dev, mac->phy_id, &pasemi_adjust_link, 0, PHY_INTERFACE_MODE_SGMII);
> +
> +	if (IS_ERR(phydev)) {
> +		printk(KERN_ERR "%s: Could not attach to phy\n", dev->name);
> +		return PTR_ERR(phydev);
> +	}
> +
> +	mac->phydev = phydev;
> +
> +	return 0;
> +
> +err:
> +	of_node_put(phy_dn);
> +	return -ENODEV;
> +}
> +
> +
>  static int pasemi_mac_open(struct net_device *dev)
>  {
>  	struct pasemi_mac *mac = netdev_priv(dev);
> @@ -667,6 +769,13 @@ static int pasemi_mac_open(struct net_de
>  
>  	pasemi_mac_replenish_rx_ring(dev);
>  
> +	ret = pasemi_mac_phy_init(dev);
> +	/* Some configs don't have PHYs (XAUI etc), so don't complain about
> +	 * failed init due to -ENODEV.
> +	 */
> +	if (ret && ret != -ENODEV)
> +		dev_warn(&mac->pdev->dev, "phy init failed: %d\n", ret);
> +
>  	netif_start_queue(dev);
>  	netif_poll_enable(dev);
>  
> @@ -697,6 +806,9 @@ static int pasemi_mac_open(struct net_de
>  		goto out_rx_int;
>  	}
>  
> +	if (mac->phydev)
> +		phy_start(mac->phydev);
> +
>  	return 0;
>  
>  out_rx_int:
> @@ -720,6 +832,11 @@ static int pasemi_mac_close(struct net_d
>  	unsigned int stat;
>  	int retries;
>  
> +	if (mac->phydev) {
> +		phy_stop(mac->phydev);
> +		phy_disconnect(mac->phydev);
> +	}
> +
>  	netif_stop_queue(dev);
>  
>  	/* Clean out any pending buffers */
> @@ -1013,6 +1130,9 @@ pasemi_mac_probe(struct pci_dev *pdev, c
>  	mac->rx_status = &dma_status->rx_sta[mac->dma_rxch];
>  	mac->tx_status = &dma_status->tx_sta[mac->dma_txch];
>  
> +	/* Enable most messages by default */
> +	mac->msg_enable = (NETIF_MSG_IFUP << 1 ) - 1;

msg_enable should be initialized in a standard fashion, from your debug 
module parameter.  grep for 'msg_enable' in other drivers.

	Jeff

Re: [RESEND] [PATCH v2] [4/5] pasemi_mac: phy support

[Olof Johansson]

On Sat, Apr 28, 2007 at 11:23:42AM -0400, Jeff Garzik wrote:

> msg_enable should be initialized in a standard fashion, from your debug 
> module parameter.  grep for 'msg_enable' in other drivers.

Ah, good point. Done, and broken out as a separate patch.


Thanks,

-Olof

[RESEND] [PATCH v2] [5/5] pasemi_mac: use local-mac-address

[Olof Johansson]

Use local-mac-address in the device tree instead. Fall back to mac-address
for older firmware.


Signed-off-by: Olof Johansson <olof@lixom.net>

Index: powerpc/drivers/net/pasemi_mac.c
===================================================================
--- powerpc.orig/drivers/net/pasemi_mac.c
+++ powerpc/drivers/net/pasemi_mac.c
@@ -74,7 +74,12 @@ static int pasemi_get_mac_addr(struct pa
 		return -ENOENT;
 	}
 
-	maddr = get_property(dn, "mac-address", NULL);
+	maddr = get_property(dn, "local-mac-address", NULL);
+
+	/* Fall back to mac-address for older firmware */
+	if (maddr == NULL)
+		maddr = get_property(dn, "mac-address", NULL);
+
 	if (maddr == NULL) {
 		dev_warn(&pdev->dev,
 			 "no mac address in device tree, not configuring\n");