Re: [PATCH 09/26] atl1: refactor tx processing

Previous thread: none

Next thread: [Patch 0/8] Remove 'TOPDIR' from Makefiles by WANG Cong on Tuesday, January 1, 2008 - 12:13 am. (43 messages)
From: jacliburn
Date: Monday, December 31, 2007 - 6:59 pm

Hello Jeff,

Happy New Year to you and all.

In preparation for a future atl2 driver for the Atheros L2 10/100 chip,
we propose to move the existing atl1 driver to a new directory
(drivers/net/atlx), then split out functions and definitions that both
atl1 and atl2 can share.  The final structure will look like this:

directory or file		status
=======================		==========================
drivers/net/atl1/		deleted
drivers/net/atlx/		new
drivers/net/atlx/atl1.c		atl1-specific functions
drivers/net/atlx/atl1.h		atl1-specific definitions
drivers/net/atlx/atlx.c		atl1-atl2 shared functions
drivers/net/atlx/atlx.h		atl1-atl2 shared definitions

The first two patches submitted in this patchset accomplish the relocation
by movng the atl1 driver -- lock, stock, and barrel -- over to
drivers/net/atlx, then splitting out shareable functions and definitions.
Some transitory hackery will be present until the atl2 merge.  Please
overlook it for now.

The remaining 24 patches bring the atl1 driver up to par with the current
vendor driver version 1.2.40.2.  NAPI support is included and it seems
to work, but it needs to be scrutinized by an experienced eye.  I had a
hard time finding much current NAPI documentation, so I just hacked at
it by looking at the e1000 driver.

Patch 02/26 is too large for LKML, so it's available at:

ftp://ftp.hogchain.net/pub/linux/attansic/atlx

Or, alternatively, the whole shebang can be pulled from:

git://git.hogchain.net/home/jcliburn/netdev-2.6.git atl1-for-jeff


Table of ...
From: jacliburn
Date: Monday, December 31, 2007 - 6:59 pm

From: Jay Cliburn <jacliburn@bellsouth.net>

Refactor tx processing to use a less convoluted tx packet descriptor and
to conform generally with the vendor's current version 1.2.40.2.

Signed-off-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/atlx/atl1.c |  265 +++++++++++++++++++++++++----------------------
 drivers/net/atlx/atl1.h |  201 +++++++++++++++++++-----------------
 2 files changed, 246 insertions(+), 220 deletions(-)

diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index fb0a0af..b0c3273 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -1288,8 +1288,6 @@ static void atl1_intr_tx(struct atl1_adapter *adapter)
 			dev_kfree_skb_irq(buffer_info->skb);
 			buffer_info->skb = NULL;
 		}
-		tpd->buffer_addr = 0;
-		tpd->desc.data = 0;
 
 		if (++sw_tpd_next_to_clean == tpd_ring->count)
 			sw_tpd_next_to_clean = 0;
@@ -1311,48 +1309,69 @@ static u16 atl1_tpd_avail(struct atl1_tpd_ring *tpd_ring)
 }
 
 static int atl1_tso(struct atl1_adapter *adapter, struct sk_buff *skb,
-			 struct tso_param *tso)
+	struct tx_packet_desc *ptpd)
 {
-	/* We enter this function holding a spinlock. */
-	u8 ipofst;
+	/* spinlock held */
+	u8 hdr_len, ip_off;
+	u32 real_len;
 	int err;
 
 	if (skb_shinfo(skb)->gso_size) {
 		if (skb_header_cloned(skb)) {
 			err = pskb_expand_head(skb, 0, 0, GFP_ATOMIC);
 			if (unlikely(err))
-				return err;
+				return -1;
 		}
 
 		if (skb->protocol == ntohs(ETH_P_IP)) {
 			struct iphdr *iph = ip_hdr(skb);
 
-			iph->tot_len = 0;
+			real_len = (((unsigned char *)iph - skb->data) +
+				ntohs(iph->tot_len));
+			if (real_len < skb->len)
+				pskb_trim(skb, real_len);
+			hdr_len = (skb_transport_offset(skb) + tcp_hdrlen(skb));
+			if (skb->len == hdr_len) {
+				iph->check = 0;
+				tcp_hdr(skb)->check =
+					~csum_tcpudp_magic(iph->saddr,
+					iph->daddr, tcp_hdrlen(skb),
+					IPPROTO_TCP, 0);
+				ptpd->word3 |= (iph->ihl & TPD_IPHL_MASK) ...
From: Jeff Garzik
Date: Tuesday, January 22, 2008 - 2:58 am

for such a huge patch, this description is very tiny.  [describe] what 
is refactored, and why.

what does "less convoluted" mean?


--

From: Jay Cliburn
Date: Tuesday, January 22, 2008 - 5:31 pm

On Tue, 22 Jan 2008 04:58:17 -0500

Okay, I'll go back and rework the offending descriptions for this and

I should have written "simpler," I suppose.

Before:
=======
struct tso_param {
	u32 tsopu;      /* tso_param upper word */
	u32 tsopl;      /* tso_param lower word */
};

struct csum_param {
	u32 csumpu;     /* csum_param upper word */
	u32 csumpl;     /* csum_param lower word */
};

union tpd_descr {
	u64 data;
	struct csum_param csum;
	struct tso_param tso;
};

struct tx_packet_desc {
	__le64 buffer_addr;
	union tpd_descr desc;
};


After:
======
struct tx_packet_desc {
        __le64 buffer_addr;
        __le32 word2;
        __le32 word3;
};

--

From: Jay Cliburn
Date: Thursday, January 24, 2008 - 6:00 pm

On Tue, 22 Jan 2008 18:31:09 -0600

Is this one any better?



From df475e2eea401f9dc18ca23dab538b99fb9e710c Mon Sep 17 00:00:00 2001
From: Jay Cliburn <jacliburn@bellsouth.net>
Date: Wed, 23 Jan 2008 21:36:36 -0600
Subject: [PATCH] atl1: simplify tx packet descriptor

The transmit packet descriptor consists of four 32-bit words, with word 3
upper bits overloaded depending upon the condition of its bits 3 and 4.
The driver currently duplicates all word 2 and some word 3 register bit
definitions unnecessarily and also uses a set of nested structures in its
definition of the TPD without good cause. This patch adds a lengthy
comment describing the TPD, eliminates duplicate TPD bit definitions,
and simplifies the TPD structure itself. It also expands the TSO check
to correctly handle custom checksum versus TSO processing using the revised
TPD definitions. Finally, shorten some variable names in the transmit
processing path to reduce line lengths, rename some variables to better
describe their purpose (e.g., nseg versus m), and add a comment or two
to better describe what the code is doing.

Signed-off-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/atlx/atl1.c |  265 +++++++++++++++++++++++++----------------------
 drivers/net/atlx/atl1.h |  201 +++++++++++++++++++-----------------
 2 files changed, 246 insertions(+), 220 deletions(-)

diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index 1f564f0..f4add3c 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -1259,8 +1259,6 @@ static void atl1_intr_tx(struct atl1_adapter *adapter)
 			dev_kfree_skb_irq(buffer_info->skb);
 			buffer_info->skb = NULL;
 		}
-		tpd->buffer_addr = 0;
-		tpd->desc.data = 0;
 
 		if (++sw_tpd_next_to_clean == tpd_ring->count)
 			sw_tpd_next_to_clean = 0;
@@ -1282,48 +1280,69 @@ static u16 atl1_tpd_avail(struct atl1_tpd_ring *tpd_ring)
 }
 
 static int atl1_tso(struct atl1_adapter *adapter, struct sk_buff *skb,
-			 struct tso_param *tso)
+	struct ...
From: Chris Snook
Date: Thursday, January 24, 2008 - 6:08 pm

This satisfies me.

--

From: Jeff Garzik
Date: Thursday, January 24, 2008 - 8:01 pm

Yep, better.

	Jeff



--

From: jacliburn
Date: Monday, December 31, 2007 - 6:59 pm

From: Jay Cliburn <jacliburn@bellsouth.net>

Refactor atl1 initialization and startup to conform with the current
vendor driver version 1.2.40.2.

Signed-off-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/atlx/atl1.c |  507 ++++++++++++++++++++++++-----------------------
 drivers/net/atlx/atl1.h |   10 +-
 drivers/net/atlx/atlx.c |    1 +
 drivers/net/atlx/atlx.h |   21 ++-
 4 files changed, 284 insertions(+), 255 deletions(-)

diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index 31aad9f..e96f706 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -568,39 +568,6 @@ static u32 atl1_check_link(struct atl1_adapter *adapter)
 	return 0;
 }
 
-/*
- * atl1_change_mtu - Change the Maximum Transfer Unit
- * @netdev: network interface device structure
- * @new_mtu: new value for maximum frame size
- *
- * Returns 0 on success, negative on failure
- */
-static int atl1_change_mtu(struct net_device *netdev, int new_mtu)
-{
-	struct atl1_adapter *adapter = netdev_priv(netdev);
-	int old_mtu = netdev->mtu;
-	int max_frame = new_mtu + ETH_HLEN + ETH_FCS_LEN;
-
-	if ((max_frame < ETH_ZLEN + ETH_FCS_LEN) ||
-	    (max_frame > MAX_JUMBO_FRAME_SIZE)) {
-		dev_warn(&adapter->pdev->dev, "invalid MTU setting\n");
-		return -EINVAL;
-	}
-
-	adapter->hw.max_frame_size = max_frame;
-	adapter->hw.tx_jumbo_task_th = (max_frame + 7) >> 3;
-	adapter->rx_buffer_len = (max_frame + 7) & ~7;
-	adapter->hw.rx_jumbo_th = adapter->rx_buffer_len / 8;
-
-	netdev->mtu = new_mtu;
-	if ((old_mtu != new_mtu) && netif_running(netdev)) {
-		atl1_down(adapter);
-		atl1_up(adapter);
-	}
-
-	return 0;
-}
-
 static void set_flow_ctrl_old(struct atl1_adapter *adapter)
 {
 	u32 hi, lo, value;
@@ -1043,6 +1010,7 @@ static void atl1_rx_checksum(struct atl1_adapter *adapter,
 static u16 atl1_alloc_rx_buffers(struct atl1_adapter *adapter)
 {
 	struct atl1_rfd_ring *rfd_ring = &adapter->rfd_ring;
+	struct net_device *netdev = adapter->netdev;
 	struct ...
From: jacliburn
Date: Monday, December 31, 2007 - 6:59 pm

From: Jay Cliburn <jacliburn@bellsouth.net>

Refactor atl1_probe to conform with current vendor driver version 1.2.40.2.
Also reorder functions to minimize the need for forward declarations.

Signed-off-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/atlx/atl1.c | 1397 +++++++++++++++++++++++------------------------
 drivers/net/atlx/atl1.h |   21 +-
 2 files changed, 691 insertions(+), 727 deletions(-)

diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index e96f706..d38f26f 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -101,17 +101,681 @@ static const struct pci_device_id atl1_pci_tbl[] = {
 MODULE_DEVICE_TABLE(pci, atl1_pci_tbl);
 
 /*
+ * Reset the transmit and receive units; mask and clear all interrupts.
+ * hw - Struct containing variables accessed by shared code
+ * return : 0  or  idle status (if error)
+ */
+static s32 atl1_reset_hw(struct atl1_hw *hw)
+{
+	struct pci_dev *pdev = hw->back->pdev;
+	u32 icr;
+	int i;
+
+	/*
+	 * Issue Soft Reset to the MAC.  This will reset the chip's
+	 * transmit, receive, DMA.  It will not effect
+	 * the current PCI configuration.  The global reset bit is self-
+	 * clearing, and should clear within a microsecond.
+	 */
+	iowrite32(MASTER_CTRL_SOFT_RST, hw->hw_addr + REG_MASTER_CTRL);
+	ioread32(hw->hw_addr + REG_MASTER_CTRL);
+
+	/* delay about 1ms */
+	msleep(1);
+
+	/* wait at least 10ms for idle */
+	for (i = 0; i < 10; i++) {
+		icr = ioread32(hw->hw_addr + REG_IDLE_STATUS);
+		if (!icr)
+			break;
+		/* delay 1 ms */
+		msleep(1);
+		cpu_relax();
+	}
+
+	if (icr) {
+		dev_dbg(&pdev->dev, "ICR = 0x%x\n", icr);
+		return icr;
+	}
+
+	return 0;
+}
+
+/* function about EEPROM
+ *
+ * check_eeprom_exist
+ * return 0 if eeprom exist
+ */
+static int atl1_check_eeprom_exist(struct atl1_hw *hw)
+{
+	u32 value;
+
+	value = ioread32(hw->hw_addr + REG_SPI_FLASH_CTRL);
+	if (value & SPI_FLASH_CTRL_EN_VPD) {
+		value &= ...
From: jacliburn
Date: Monday, December 31, 2007 - 6:59 pm

From: Jay Cliburn <jacliburn@bellsouth.net>

Refactor interrupt handling to conform with the current vendor driver
version 1.2.40.2.

Signed-off-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/atlx/atl1.c |  196 ++++++++++++++++++++++-------------------------
 drivers/net/atlx/atl1.h |   25 +++++-
 drivers/net/atlx/atlx.c |    2 +-
 3 files changed, 114 insertions(+), 109 deletions(-)

diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index d38f26f..9c86ef4 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -1740,12 +1740,11 @@ next:
 	return num_alloc;
 }
 
-static void atl1_intr_rx(struct atl1_adapter *adapter)
+static void atl1_clean_rx_irq(struct atl1_adapter *adapter)
 {
+	struct net_device *netdev = adapter->netdev;
 	int i, count;
-	u16 length;
-	u16 rrd_next_to_clean;
-	u32 value;
+	u16 length, rrd_next_to_clean;
 	struct atl1_rfd_ring *rfd_ring = &adapter->rfd_ring;
 	struct atl1_rrd_ring *rrd_ring = &adapter->rrd_ring;
 	struct atl1_buffer *buffer_info;
@@ -1754,7 +1753,7 @@ static void atl1_intr_rx(struct atl1_adapter *adapter)
 
 	count = 0;
 
-	rrd_next_to_clean = atomic_read(&rrd_ring->next_to_clean);
+	rrd_next_to_clean = (u16) atomic_read(&rrd_ring->next_to_clean);
 
 	while (1) {
 		rrd = ATL1_RRD_DESC(rrd_ring, rrd_next_to_clean);
@@ -1795,6 +1794,7 @@ chk_rrd:
 			/* bad rrd */
 			dev_printk(KERN_DEBUG, &adapter->pdev->dev,
 				"bad RRD\n");
+
 			/* see if update RFD index */
 			if (rrd->num_buf > 1)
 				atl1_update_rfd_index(adapter, rrd);
@@ -1823,13 +1823,18 @@ rrd_ok:
 		count++;
 
 		if (unlikely(rrd->pkt_flg & PACKET_FLAG_ERR)) {
-			if (!(rrd->err_flg &
-				(ERR_FLAG_IP_CHKSUM | ERR_FLAG_L4_CHKSUM
-				| ERR_FLAG_LEN))) {
-				/* packet error, don't need upstream */
-				buffer_info->alloced = 0;
-				rrd->xsz.valid = 0;
-				continue;
+			if (rrd->err_flg & (ERR_FLAG_CRC | ERR_FLAG_TRUNC |
+				ERR_FLAG_CODE | ERR_FLAG_OV)) {
+				if (!(netdev->flags & IFF_PROMISC)) ...
From: Jeff Garzik
Date: Tuesday, January 22, 2008 - 2:59 am

same comments... "refactor" should be explained further in the changelog

--

From: jacliburn
Date: Monday, December 31, 2007 - 6:59 pm

From: Jay Cliburn <jacliburn@bellsouth.net>

Move some stray defines out to a header file.  Improve indentation from
ghastly to horrid.

Signed-off-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/atlx/atl1.c |   29 ++++++++++++-----------------
 drivers/net/atlx/atl1.h |    6 ++++++
 2 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index 9c86ef4..f40cc6e 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -217,12 +217,6 @@ static s32 atl1_read_phy_reg(struct atl1_hw *hw, u16 reg_addr, u16 *phy_data)
 	return ATLX_ERR_PHY;
 }
 
-#define CUSTOM_SPI_CS_SETUP	2
-#define CUSTOM_SPI_CLK_HI	2
-#define CUSTOM_SPI_CLK_LO	2
-#define CUSTOM_SPI_CS_HOLD	2
-#define CUSTOM_SPI_CS_HI	3
-
 static bool atl1_spi_read(struct atl1_hw *hw, u32 addr, u32 *buf)
 {
 	int i;
@@ -232,17 +226,18 @@ static bool atl1_spi_read(struct atl1_hw *hw, u32 addr, u32 *buf)
 	iowrite32(addr, hw->hw_addr + REG_SPI_ADDR);
 
 	value = SPI_FLASH_CTRL_WAIT_READY |
-	    (CUSTOM_SPI_CS_SETUP & SPI_FLASH_CTRL_CS_SETUP_MASK) <<
-	    SPI_FLASH_CTRL_CS_SETUP_SHIFT | (CUSTOM_SPI_CLK_HI &
-					     SPI_FLASH_CTRL_CLK_HI_MASK) <<
-	    SPI_FLASH_CTRL_CLK_HI_SHIFT | (CUSTOM_SPI_CLK_LO &
-					   SPI_FLASH_CTRL_CLK_LO_MASK) <<
-	    SPI_FLASH_CTRL_CLK_LO_SHIFT | (CUSTOM_SPI_CS_HOLD &
-					   SPI_FLASH_CTRL_CS_HOLD_MASK) <<
-	    SPI_FLASH_CTRL_CS_HOLD_SHIFT | (CUSTOM_SPI_CS_HI &
-					    SPI_FLASH_CTRL_CS_HI_MASK) <<
-	    SPI_FLASH_CTRL_CS_HI_SHIFT | (1 & SPI_FLASH_CTRL_INS_MASK) <<
-	    SPI_FLASH_CTRL_INS_SHIFT;
+		(CUSTOM_SPI_CS_SETUP & SPI_FLASH_CTRL_CS_SETUP_MASK) <<
+		SPI_FLASH_CTRL_CS_SETUP_SHIFT |
+		(CUSTOM_SPI_CLK_HI & SPI_FLASH_CTRL_CLK_HI_MASK) <<
+		SPI_FLASH_CTRL_CLK_HI_SHIFT |
+		(CUSTOM_SPI_CLK_LO & SPI_FLASH_CTRL_CLK_LO_MASK) <<
+		SPI_FLASH_CTRL_CLK_LO_SHIFT |
+		(CUSTOM_SPI_CS_HOLD & SPI_FLASH_CTRL_CS_HOLD_MASK) <<
+		SPI_FLASH_CTRL_CS_HOLD_SHIFT |
+		(CUSTOM_SPI_CS_HI & ...
From: jacliburn
Date: Monday, December 31, 2007 - 6:59 pm

From: Jay Cliburn <jacliburn@bellsouth.net>

Check for null pointers and such in ring handling functions.  Make
needlessly global functions static.  Clean up some comments and indentation.

Signed-off-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/atlx/atl1.c |   20 +++++++++++---------
 1 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index f40cc6e..db6e76c 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -141,8 +141,7 @@ static s32 atl1_reset_hw(struct atl1_hw *hw)
 	return 0;
 }
 
-/* function about EEPROM
- *
+/*
  * check_eeprom_exist
  * return 0 if eeprom exist
  */
@@ -166,7 +165,7 @@ static bool atl1_read_eeprom(struct atl1_hw *hw, u32 offset, u32 *p_value)
 	u32 control;
 
 	if (offset & 3)
-		/* address do not align */
+		/* unaligned address */
 		return false;
 
 	iowrite32(0, hw->hw_addr + REG_VPD_DATA);
@@ -417,7 +416,7 @@ static void atl1_hash_set(struct atl1_hw *hw, u32 hash_value)
 	 * bit BitArray[hash_value]. So we figure out what register
 	 * the bit is in, read it, OR in the new bit, then write
 	 * back the new value.  The register is determined by the
-	 * upper 7 bits of the hash value and the bit within that
+	 * upper bit of the hash value, and the bits within that
 	 * register are determined by the lower 5 bits of the value.
 	 */
 	hash_reg = (hash_value >> 31) & 0x1;
@@ -439,9 +438,9 @@ static s32 atl1_write_phy_reg(struct atl1_hw *hw, u32 reg_addr, u16 phy_data)
 	u32 val;
 
 	val = ((u32) (phy_data & MDIO_DATA_MASK)) << MDIO_DATA_SHIFT |
-	    (reg_addr & MDIO_REG_ADDR_MASK) << MDIO_REG_ADDR_SHIFT |
-	    MDIO_SUP_PREAMBLE |
-	    MDIO_START | MDIO_CLK_25_4 << MDIO_CLK_SEL_SHIFT;
+		(reg_addr & MDIO_REG_ADDR_MASK) << MDIO_REG_ADDR_SHIFT |
+		MDIO_SUP_PREAMBLE | MDIO_START |
+		MDIO_CLK_25_4 << MDIO_CLK_SEL_SHIFT;
 	iowrite32(val, hw->hw_addr + REG_MDIO_CTRL);
 	ioread32(hw->hw_addr + REG_MDIO_CTRL);
 
@@ -872,7 +871,7 @@ ...
From: jacliburn
Date: Monday, December 31, 2007 - 6:59 pm

From: Jay Cliburn <jacliburn@bellsouth.net>

Update atl1_check_link() to conform with the current vendor driver version
1.2.40.2.  Clean up vertical spacing, indentation, and remove dead code.

Signed-off-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/atlx/atl1.c |   65 +++++++++++++++-------------------------------
 drivers/net/atlx/atl1.h |    1 +
 2 files changed, 22 insertions(+), 44 deletions(-)

diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index db6e76c..abed547 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -1094,6 +1094,7 @@ static void atl1_setup_mac_ctrl(struct atl1_adapter *adapter)
 	u32 value;
 	struct atl1_hw *hw = &adapter->hw;
 	struct net_device *netdev = adapter->netdev;
+
 	/* Config MAC CTRL Register */
 	value = MAC_CTRL_TX_EN | MAC_CTRL_RX_EN;
 	/* duplex */
@@ -1101,8 +1102,8 @@ static void atl1_setup_mac_ctrl(struct atl1_adapter *adapter)
 		value |= MAC_CTRL_DUPLX;
 	/* speed */
 	value |= ((u32) ((SPEED_1000 == adapter->link_speed) ?
-			 MAC_CTRL_SPEED_1000 : MAC_CTRL_SPEED_10_100) <<
-		  MAC_CTRL_SPEED_SHIFT);
+		MAC_CTRL_SPEED_1000 : MAC_CTRL_SPEED_10_100) <<
+		MAC_CTRL_SPEED_SHIFT);
 	/* flow control */
 	value |= (MAC_CTRL_TX_FLOW | MAC_CTRL_RX_FLOW);
 	/* PAD & CRC */
@@ -1113,10 +1114,6 @@ static void atl1_setup_mac_ctrl(struct atl1_adapter *adapter)
 	/* vlan */
 	if (adapter->vlgrp)
 		value |= MAC_CTRL_RMV_VLAN;
-	/* rx checksum
-	   if (adapter->rx_csum)
-	   value |= MAC_CTRL_RX_CHKSUM_EN;
-	 */
 	/* filter mode */
 	value |= MAC_CTRL_BC_EN;
 	if (netdev->flags & IFF_PROMISC)
@@ -1131,7 +1128,7 @@ static u32 atl1_check_link(struct atl1_adapter *adapter)
 {
 	struct atl1_hw *hw = &adapter->hw;
 	struct net_device *netdev = adapter->netdev;
-	u32 ret_val;
+	u32 val, retval;
 	u16 speed, duplex, phy_data;
 	int reconfig = 0;
 
@@ -1143,6 +1140,10 @@ static u32 atl1_check_link(struct atl1_adapter *adapter)
 		if (netif_carrier_ok(netdev)) {
 			/* old link state: Up ...
From: jacliburn
Date: Monday, December 31, 2007 - 6:59 pm

From: Jay Cliburn <jacliburn@bellsouth.net>

Update atl1_phy_config() to conform with current vendor driver version
1.2.40.2.

Signed-off-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/atlx/atl1.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index abed547..6432956 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -2344,11 +2344,11 @@ static void atl1_phy_config(unsigned long data)
 	unsigned long flags;
 
 	spin_lock_irqsave(&adapter->lock, flags);
-	adapter->phy_timer_pending = false;
 	atl1_write_phy_reg(hw, MII_ADVERTISE, hw->mii_autoneg_adv_reg);
 	atl1_write_phy_reg(hw, MII_ATLX_CR, hw->mii_1000t_ctrl_reg);
 	atl1_write_phy_reg(hw, MII_BMCR, MII_CR_RESET | MII_CR_AUTO_NEG_EN);
 	spin_unlock_irqrestore(&adapter->lock, flags);
+	clear_bit(0, &adapter->cfg_phy);
 }
 
 int atl1_reset(struct atl1_adapter *adapter)
-- 
1.5.3.3

--

From: jacliburn
Date: Monday, December 31, 2007 - 6:59 pm

From: Jay Cliburn <jacliburn@bellsouth.net>

Make atl1_reset() a static function.

Signed-off-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/atlx/atl1.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index 6432956..7697e80 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -2351,12 +2351,13 @@ static void atl1_phy_config(unsigned long data)
 	clear_bit(0, &adapter->cfg_phy);
 }
 
-int atl1_reset(struct atl1_adapter *adapter)
+static int atl1_reset(struct atl1_adapter *adapter)
 {
-	int ret;
-	ret = atl1_reset_hw(&adapter->hw);
-	if (ret)
-		return ret;
+	int retval;
+
+	retval = atl1_reset_hw(&adapter->hw);
+	if (retval)
+		return retval;
 	return atl1_init_hw(&adapter->hw);
 }
 
-- 
1.5.3.3

--

From: jacliburn
Date: Monday, December 31, 2007 - 6:59 pm

From: Jay Cliburn <jacliburn@bellsouth.net>

In preparation for a future Atheros L2 NIC driver (called atl2), relocate
the atl1 driver into a new /drivers/net/atlx directory that will ultimately
be shared with the future atl2 driver.

Signed-off-by: Chris Snook <csnook@redhat.com>
Signed-off-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/Makefile                      |    2 +-
 drivers/net/{atl1 => atlx}/Makefile       |    0 
 drivers/net/{atl1 => atlx}/atl1.h         |    0 
 drivers/net/{atl1 => atlx}/atl1_ethtool.c |    0 
 drivers/net/{atl1 => atlx}/atl1_hw.c      |    0 
 drivers/net/{atl1 => atlx}/atl1_hw.h      |    0 
 drivers/net/{atl1 => atlx}/atl1_main.c    |    0 
 drivers/net/{atl1 => atlx}/atl1_param.c   |    0 
 8 files changed, 1 insertions(+), 1 deletions(-)
 rename drivers/net/{atl1 => atlx}/Makefile (100%)
 rename drivers/net/{atl1 => atlx}/atl1.h (100%)
 rename drivers/net/{atl1 => atlx}/atl1_ethtool.c (100%)
 rename drivers/net/{atl1 => atlx}/atl1_hw.c (100%)
 rename drivers/net/{atl1 => atlx}/atl1_hw.h (100%)
 rename drivers/net/{atl1 => atlx}/atl1_main.c (100%)
 rename drivers/net/{atl1 => atlx}/atl1_param.c (100%)

diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 0e5fde4..14acf84 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -13,7 +13,7 @@ obj-$(CONFIG_CHELSIO_T1) += chelsio/
 obj-$(CONFIG_CHELSIO_T3) += cxgb3/
 obj-$(CONFIG_EHEA) += ehea/
 obj-$(CONFIG_BONDING) += bonding/
-obj-$(CONFIG_ATL1) += atl1/
+obj-$(CONFIG_ATL1) += atlx/
 obj-$(CONFIG_GIANFAR) += gianfar_driver.o
 obj-$(CONFIG_TEHUTI) += tehuti.o
 
diff --git a/drivers/net/atl1/Makefile b/drivers/net/atlx/Makefile
similarity index 100%
rename from drivers/net/atl1/Makefile
rename to drivers/net/atlx/Makefile
diff --git a/drivers/net/atl1/atl1.h b/drivers/net/atlx/atl1.h
similarity index 100%
rename from drivers/net/atl1/atl1.h
rename to drivers/net/atlx/atl1.h
diff --git a/drivers/net/atl1/atl1_ethtool.c ...
From: jacliburn
Date: Monday, December 31, 2007 - 6:59 pm

From: Jay Cliburn <jacliburn@bellsouth.net>

Update atl1_down() and atl1_up() to conform with the current vendor driver
version 1.2.40.2.

Signed-off-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/atlx/atl1.c |   21 ++++++++-------------
 drivers/net/atlx/atl1.h |    1 -
 drivers/net/atlx/atlx.c |    2 +-
 3 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index 7697e80..972de34 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -2388,7 +2388,6 @@ static s32 atl1_up(struct atl1_adapter *adapter)
 	}
 
 	clear_bit(__ATL1_DOWN, &adapter->flags);
-	mod_timer(&adapter->watchdog_timer, jiffies + (4 * HZ));
 	retval = ioread32(&adapter->hw + REG_MASTER_CTRL);
 	retval |= MASTER_CTRL_MANUAL_INT;
 	iowrite32(retval, &adapter->hw + REG_MASTER_CTRL);
@@ -2401,21 +2400,18 @@ void atl1_down(struct atl1_adapter *adapter)
 {
 	struct net_device *netdev = adapter->netdev;
 
-	del_timer_sync(&adapter->watchdog_timer);
-	del_timer_sync(&adapter->phy_config_timer);
-	adapter->phy_timer_pending = false;
-
-	atlx_irq_disable(adapter);
-	free_irq(adapter->pdev->irq, netdev);
-	pci_disable_msi(adapter->pdev);
+	set_bit(__ATL1_DOWN, &adapter->flags);
+	netif_stop_queue(netdev);
 	atl1_reset_hw(&adapter->hw);
 	adapter->cmb.cmb->int_stats = 0;
-
+	msleep(1);
+	atlx_irq_disable(adapter);
+	del_timer_sync(&adapter->watchdog_timer);
+	del_timer_sync(&adapter->phy_config_timer);
+	clear_bit(0, &adapter->cfg_phy);
+	netif_carrier_off(netdev);
 	adapter->link_speed = SPEED_0;
 	adapter->link_duplex = -1;
-	netif_carrier_off(netdev);
-	netif_stop_queue(netdev);
-
 	atl1_clean_tx_ring(adapter);
 	atl1_clean_rx_ring(adapter);
 }
@@ -2821,7 +2817,6 @@ static int __devinit atl1_probe(struct pci_dev *pdev,
 	init_timer(&adapter->phy_config_timer);
 	adapter->phy_config_timer.function = &atl1_phy_config;
 	adapter->phy_config_timer.data = (unsigned long)adapter;
-	adapter->phy_timer_pending = ...
From: Jeff Garzik
Date: Tuesday, January 22, 2008 - 3:01 am

same comments continue...  these are software changes that need to be 
explained.

"conform with vendor driver" does not excuse failure to document changes 
in init and shutdown sequencing, for example.

--

From: jacliburn
Date: Monday, December 31, 2007 - 6:59 pm

From: Jay Cliburn <jacliburn@bellsouth.net>

Update atl1_change_mtu() to conform with the current vendor driver version
1.2.40.2.

Signed-off-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/atlx/atl1.c |   39 ++++++++++++++++++++++-----------------
 1 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index 972de34..7d84a51 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -2438,32 +2438,37 @@ static void atl1_reset_task(struct work_struct *work)
  * atl1_change_mtu - Change the Maximum Transfer Unit
  * @netdev: network interface device structure
  * @new_mtu: new value for maximum frame size
- *
- * Returns 0 on success, negative on failure
  */
 static int atl1_change_mtu(struct net_device *netdev, int new_mtu)
 {
 	struct atl1_adapter *adapter = netdev_priv(netdev);
-	int old_mtu = netdev->mtu;
-	int max_frame = new_mtu + ETH_HLEN + ETH_FCS_LEN;
+	struct atl1_hw *hw = &adapter->hw;
 
-	if ((max_frame < ETH_ZLEN + ETH_FCS_LEN) ||
-	    (max_frame > MAX_JUMBO_FRAME_SIZE)) {
-		dev_warn(&adapter->pdev->dev, "invalid MTU setting\n");
+	if ((new_mtu < 40) || (new_mtu > (ETH_DATA_LEN + VLAN_HLEN)))
 		return -EINVAL;
-	}
 
-	adapter->hw.max_frame_size = max_frame;
-	adapter->hw.tx_jumbo_task_th = (max_frame + 7) >> 3;
-	adapter->rx_buffer_len = (max_frame + 7) & ~7;
-	adapter->hw.rx_jumbo_th = adapter->rx_buffer_len / 8;
+	if (hw->max_frame_size != new_mtu) {
+		while (test_and_set_bit(__ATL1_RESETTING, &adapter->flags))
+			msleep(1);
 
-	netdev->mtu = new_mtu;
-	if ((old_mtu != new_mtu) && netif_running(netdev)) {
-		atl1_down(adapter);
-		atl1_up(adapter);
-	}
+		if (netif_running(netdev))
+			atl1_down(adapter);
+
+		netdev->mtu = new_mtu;
+		adapter->hw.max_frame_size = new_mtu;
+		adapter->hw.tx_jumbo_task_th = (new_mtu + ETH_HLEN +
+			ETH_FCS_LEN + VLAN_HLEN + 7) >> 3;
+		adapter->rx_buffer_len = (new_mtu + ETH_HLEN +
+			ETH_FCS_LEN + VLAN_HLEN + 7) & ...
From: jacliburn
Date: Monday, December 31, 2007 - 6:59 pm

From: Jay Cliburn <jacliburn@bellsouth.net>

Update atl1_close() to conform with current vendor driver version 1.2.40.2.

Signed-off-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/atlx/atl1.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index 7d84a51..997c83c 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -2558,6 +2558,18 @@ err_open:
 	return err;
 }
 
+static void atl1_free_irq(struct atl1_adapter *adapter)
+{
+	struct net_device *netdev = adapter->netdev;
+
+	free_irq(adapter->pdev->irq, netdev);
+
+#ifdef CONFIG_PCI_MSI
+	if (adapter->have_msi)
+		pci_disable_msi(adapter->pdev);
+#endif
+}
+
 /*
  * atl1_close - Disables a network interface
  * @netdev: network interface device structure
@@ -2572,7 +2584,9 @@ err_open:
 static int atl1_close(struct net_device *netdev)
 {
 	struct atl1_adapter *adapter = netdev_priv(netdev);
+
 	atl1_down(adapter);
+	atl1_free_irq(adapter);
 	atl1_free_ring_resources(adapter);
 	return 0;
 }
-- 
1.5.3.3

--

From: jacliburn
Date: Monday, December 31, 2007 - 7:00 pm

From: Jay Cliburn <jacliburn@bellsouth.net>

Rename atl1_poll_controller() to atl1_netpoll() and update to conform with
the current vendor driver version 1.2.40.2.

Signed-off-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/atlx/atl1.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index 997c83c..f09928d 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -2691,11 +2691,14 @@ static int atl1_resume(struct pci_dev *pdev)
 #endif
 
 #ifdef CONFIG_NET_POLL_CONTROLLER
-static void atl1_poll_controller(struct net_device *netdev)
+static void atl1_netpoll(struct net_device *netdev)
 {
-	disable_irq(netdev->irq);
+	struct atl1_adapter *adapter = netdev_priv(netdev);
+
+	disable_irq(adapter->pdev->irq);
 	atl1_intr(netdev->irq, netdev);
-	enable_irq(netdev->irq);
+	atl1_clean_tx_irq(adapter);
+	enable_irq(adapter->pdev->irq);
 }
 #endif
 
@@ -2796,7 +2799,7 @@ static int __devinit atl1_probe(struct pci_dev *pdev,
 	netdev->tx_timeout = &atlx_tx_timeout;
 	netdev->watchdog_timeo = 5 * HZ;
 #ifdef CONFIG_NET_POLL_CONTROLLER
-	netdev->poll_controller = atl1_poll_controller;
+	netdev->poll_controller = atl1_netpoll;
 #endif
 	netdev->vlan_rx_register = atlx_vlan_rx_register;
 
-- 
1.5.3.3

--

From: jacliburn
Date: Monday, December 31, 2007 - 7:00 pm

From: Jay Cliburn <jacliburn@bellsouth.net>

Update shutdown and remove functions to conform with the current vendor
driver version 1.2.40.2.

Signed-off-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/atlx/atl1.c |   31 +++++++++++++++++++++++--------
 1 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index f09928d..b89201e 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -2591,6 +2591,16 @@ static int atl1_close(struct net_device *netdev)
 	return 0;
 }
 
+static void atl1_force_ps(struct atl1_hw *hw)
+{
+	atl1_write_phy_reg(hw, MII_DBG_ADDR, 0);
+	atl1_write_phy_reg(hw, MII_DBG_DATA, 0x124E);
+	atl1_write_phy_reg(hw, MII_DBG_ADDR, 2);
+	atl1_write_phy_reg(hw, MII_DBG_DATA, 0x3000);
+	atl1_write_phy_reg(hw, MII_DBG_ADDR, 3);
+	atl1_write_phy_reg(hw, MII_DBG_DATA, 0);
+}
+
 #ifdef CONFIG_PM
 static int atl1_suspend(struct pci_dev *pdev, pm_message_t state)
 {
@@ -2878,12 +2888,7 @@ err_request_regions:
 static void __devexit atl1_remove(struct pci_dev *pdev)
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
-	struct atl1_adapter *adapter;
-	/* Device not available. Return. */
-	if (!netdev)
-		return;
-
-	adapter = netdev_priv(netdev);
+	struct atl1_adapter *adapter = netdev_priv(netdev);
 
 	/*
 	 * Some atl1 boards lack persistent storage for their MAC, and get it
@@ -2896,21 +2901,31 @@ static void __devexit atl1_remove(struct pci_dev *pdev)
 		atl1_set_mac_addr(&adapter->hw);
 	}
 
-	iowrite16(0, adapter->hw.hw_addr + REG_PHY_ENABLE);
+	set_bit(__ATL1_DOWN, &adapter->flags);
+	del_timer_sync(&adapter->watchdog_timer);
+	del_timer_sync(&adapter->phy_config_timer);
+	flush_scheduled_work();
 	unregister_netdev(netdev);
+	atl1_force_ps(&adapter->hw);
 	pci_iounmap(pdev, adapter->hw.hw_addr);
 	pci_release_regions(pdev);
 	free_netdev(netdev);
 	pci_disable_device(pdev);
 }
 
+static void atl1_shutdown(struct pci_dev ...
From: jacliburn
Date: Monday, December 31, 2007 - 7:00 pm

From: Jay Cliburn <jacliburn@bellsouth.net>

Update wake-on-lan to conform with the current vendor driver version
1.2.40.2.

Signed-off-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/atlx/atl1.c |  140 ++++++++++++++++++++++++++++-------------------
 1 files changed, 84 insertions(+), 56 deletions(-)

diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index b89201e..237622e 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -1457,22 +1457,6 @@ static u32 atl1_configure(struct atl1_adapter *adapter)
 	return value;
 }
 
-/*
- * When ACPI resume on some VIA MotherBoard, the Interrupt Disable bit/0x400
- * on PCI Command register is disable.
- * The function enable this bit.
- * Brackett, 2006/03/15
- */
-static void atl1_via_workaround(struct atl1_adapter *adapter)
-{
-	unsigned long value;
-
-	value = ioread16(adapter->hw.hw_addr + PCI_COMMAND);
-	if (value & PCI_COMMAND_INTX_DISABLE)
-		value &= ~PCI_COMMAND_INTX_DISABLE;
-	iowrite32(value, adapter->hw.hw_addr + PCI_COMMAND);
-}
-
 static void atl1_inc_smb(struct atl1_adapter *adapter)
 {
 	struct stats_msg_block *smb = adapter->smb.smb;
@@ -2607,65 +2591,97 @@ static int atl1_suspend(struct pci_dev *pdev, pm_message_t state)
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct atl1_adapter *adapter = netdev_priv(netdev);
 	struct atl1_hw *hw = &adapter->hw;
+	u16 speed, duplex;
 	u32 ctrl = 0;
 	u32 wufc = adapter->wol;
+	int retval;
 
 	netif_device_detach(netdev);
-	if (netif_running(netdev))
+	if (netif_running(netdev)) {
+		WARN_ON(test_bit(__ATL1_RESETTING, &adapter->flags));
 		atl1_down(adapter);
+	}
+
+	retval = pci_save_state(pdev);
+	if (retval)
+		return retval;
 
-	atl1_read_phy_reg(hw, MII_BMSR, (u16 *) & ctrl);
-	atl1_read_phy_reg(hw, MII_BMSR, (u16 *) & ctrl);
+	atl1_read_phy_reg(hw, MII_BMSR, (u16 *) &ctrl);
+	atl1_read_phy_reg(hw, MII_BMSR, (u16 *) &ctrl);
 	if (ctrl & BMSR_LSTATUS)
 		wufc &= ~ATLX_WUFC_LNKC;
 
-	/* reduce ...
From: jacliburn
Date: Monday, December 31, 2007 - 7:00 pm

From: Jay Cliburn <jacliburn@bellsouth.net>

Add support for NAPI, styled after the e1000 NAPI implementation.  That we
follow the e1000 for NAPI shouldn't come as much of a surprise, since the
entire atl1 driver is based heavily upon it.

Signed-off-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/Kconfig     |   14 ++++
 drivers/net/atlx/atl1.c |  151 +++++++++++++++++++++++++++++++++++++++++++++--
 drivers/net/atlx/atl1.h |   20 ++++++
 drivers/net/atlx/atlx.h |    7 ++-
 4 files changed, 186 insertions(+), 6 deletions(-)

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index d9107e5..095629f 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -2371,6 +2371,20 @@ config ATL1
 	  To compile this driver as a module, choose M here.  The module
 	  will be called atl1.
 
+config ATL1_NAPI
+	bool "Use Rx Polling (NAPI) (EXPERIMENTAL)"
+	depends on ATL1 && EXPERIMENTAL
+	help
+	  NAPI is a new driver API designed to reduce CPU and interrupt load
+	  when the driver is receiving lots of packets from the card. It is
+	  still somewhat experimental and thus not yet enabled by default.
+
+	  If your estimated Rx load is 10kpps or more, or if the card will be
+	  deployed on potentially unfriendly networks (e.g. in a firewall),
+	  then say Y here.
+
+	  If in doubt, say N.
+
 endif # NETDEV_1000
 
 #
diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index 237622e..10bccf6 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -756,7 +756,16 @@ static void atl1_set_mac_addr(struct atl1_hw *hw)
 
 static int atl1_alloc_queues(struct atl1_adapter *adapter)
 {
-	/* temporary placeholder function for NAPI */
+#ifdef CONFIG_ATL1_NAPI
+	int size;
+
+	size = sizeof(struct net_device) * adapter->num_rx_queues;
+	adapter->polling_netdev = kmalloc(size, GFP_KERNEL);
+	if (!adapter->polling_netdev)
+		return -ENOMEM;
+
+	memset(adapter->polling_netdev, 0, size);
+#endif
 
 	return 0;
 }
@@ -770,6 +779,9 @@ static ...
From: Jay Cliburn
Date: Tuesday, January 1, 2008 - 11:15 am

Thanks for your comments Stephen and Joonwoo.  Here's the revised
version of the atl1 NAPI patch.



From 9c3a8944220287671f983557099bc329f02fda9b Mon Sep 17 00:00:00 2001
From: Jay Cliburn <jacliburn@bellsouth.net>
Date: Tue, 1 Jan 2008 11:55:24 -0600
Subject: [PATCH 25/26] atl1: add NAPI support

Add support for NAPI.

Signed-off-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/Kconfig     |   14 +++++
 drivers/net/atlx/atl1.c |  125 +++++++++++++++++++++++++++++++++++------------
 drivers/net/atlx/atl1.h |   19 +++++++
 drivers/net/atlx/atlx.h |    7 ++-
 4 files changed, 132 insertions(+), 33 deletions(-)

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index d9107e5..095629f 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -2371,6 +2371,20 @@ config ATL1
 	  To compile this driver as a module, choose M here.  The module
 	  will be called atl1.
 
+config ATL1_NAPI
+	bool "Use Rx Polling (NAPI) (EXPERIMENTAL)"
+	depends on ATL1 && EXPERIMENTAL
+	help
+	  NAPI is a new driver API designed to reduce CPU and interrupt load
+	  when the driver is receiving lots of packets from the card. It is
+	  still somewhat experimental and thus not yet enabled by default.
+
+	  If your estimated Rx load is 10kpps or more, or if the card will be
+	  deployed on potentially unfriendly networks (e.g. in a firewall),
+	  then say Y here.
+
+	  If in doubt, say N.
+
 endif # NETDEV_1000
 
 #
diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index 8b4aa94..88ff000 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -754,13 +754,6 @@ static void atl1_set_mac_addr(struct atl1_hw *hw)
 	iowrite32(value, (hw->hw_addr + REG_MAC_STA_ADDR) + (1 << 2));
 }
 
-static int atl1_alloc_queues(struct atl1_adapter *adapter)
-{
-	/* temporary placeholder function for NAPI */
-
-	return 0;
-}
-
 /*
  * atl1_sw_init - Initialize general software structures (struct atl1_adapter)
  * @adapter: board private structure to ...
From: Joonwoo Park
Date: Tuesday, January 1, 2008 - 7:56 pm

Hi Jay,

+	if ((work_done < budget) || !netif_running(poll_dev)) {
+quit_polling:
+		netif_rx_complete(poll_dev, napi);
+
+		if (!test_bit(__ATL1_DOWN, &adapter->flags))
+			atlx_irq_enable(adapter);
+	}

Not enough :)
If netif_running() is false, it can make problem.
The problem occurs calling netif_rx_complete with work_done == budget.
If do that, net_rx_action would do poll list double deletion.

Since we had reached a consensus on fixing it without each drivers modifications, there is no best solution for that problem for
now. I'm expecting Dave or others work for net-core.
(http://lkml.org/lkml/2007/12/20/600)

IMHO, atl1_clean should wait for work_done != budget even though netif_running is false at this time.
At least, It would not make oops.

Thanks,
Joonwoo

--

From: David Miller
Date: Tuesday, January 1, 2008 - 8:07 pm

From: "Joonwoo Park" <joonwpark81@gmail.com>

Indeed, I really hope to try and close this out very soon.
Sorry for taking so long.
--

From: Joonwoo Park
Date: Monday, December 31, 2007 - 11:09 pm

Hi Jay,
if buget == work_done is true, you should not call netif_rx_complete.
Searching recent netdev mailbox would be help.

Thanks.
Joonwoo
--

From: jacliburn
Date: Monday, December 31, 2007 - 6:59 pm

From: Jay Cliburn <jacliburn@bellsouth.net>

The future atl2 driver and the existing atl1 driver can share certain
functions and definitions.  Move these shareable functions and definitions
out of atl1-specific files and into atlx.c and atlx.h.  Some transitory
hackery will be present until atl2 is merged.

Reduce the number of source files by moving ethtool, hw, and param
functions from separate files into atl1_main.c, then rename it to just
atl1.c.

Move all atl1-specific definitions from atl1_hw.h to atl1.h.

Finally, clean up to make checkpatch.pl happy.

Signed-off-by: Chris Snook <csnook@redhat.com>
Signed-off-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/atlx/Makefile                |    1 -
 drivers/net/atlx/{atl1_main.c => atl1.c} | 1544 ++++++++++++++++++++++++------
 drivers/net/atlx/atl1.h                  |  603 +++++++++++-
 drivers/net/atlx/atl1_ethtool.c          |  505 ----------
 drivers/net/atlx/atl1_hw.c               |  720 --------------
 drivers/net/atlx/atl1_hw.h               |  946 ------------------
 drivers/net/atlx/atl1_param.c            |  203 ----
 drivers/net/atlx/atlx.c                  |  433 +++++++++
 drivers/net/atlx/atlx.h                  |  506 ++++++++++
 9 files changed, 2751 insertions(+), 2710 deletions(-)
 rename drivers/net/atlx/{atl1_main.c => atl1.c} (65%)
 delete mode 100644 drivers/net/atlx/atl1_ethtool.c
 delete mode 100644 drivers/net/atlx/atl1_hw.c
 delete mode 100644 drivers/net/atlx/atl1_hw.h
 delete mode 100644 drivers/net/atlx/atl1_param.c
 create mode 100644 drivers/net/atlx/atlx.c
 create mode 100644 drivers/net/atlx/atlx.h

diff --git a/drivers/net/atlx/Makefile b/drivers/net/atlx/Makefile
index a6b707e..ca45553 100644
--- a/drivers/net/atlx/Makefile
+++ b/drivers/net/atlx/Makefile
@@ -1,2 +1 @@
 obj-$(CONFIG_ATL1)	+= atl1.o
-atl1-y			+= atl1_main.o atl1_hw.o atl1_ethtool.o atl1_param.o
diff --git a/drivers/net/atlx/atl1_main.c b/drivers/net/atlx/atl1.c
similarity index 65%
rename from ...
From: jacliburn
Date: Monday, December 31, 2007 - 6:59 pm

From: Jay Cliburn <jacliburn@bellsouth.net>

The L1 tx packet descriptor expects TCP Header Length to be expressed as a
number of 32-bit dwords.  The atl1 driver uses tcp_hdrlen() to populate the
field, but tcp_hdrlen() returns the header length in bytes, not in dwords.
Add a shift to convert tcp_hdrlen() to dwords when we write it to the tpd.

Also, some of our bit assignments are made to the wrong tpd words.  Change
those to the correct words.

Finally, since all this fixes TSO, enable TSO by default.

Signed-off-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/atlx/atl1.c |   29 +++++++++++------------------
 1 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index 0df3f32..4e98c16 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -36,7 +36,6 @@
  * A very incomplete list of things that need to be dealt with:
  *
  * TODO:
- * Fix TSO; tx performance is horrible with TSO enabled.
  * Wake on LAN.
  * Add more ethtool functions.
  * Fix abstruse irq enable/disable condition described here:
@@ -1308,8 +1307,8 @@ static int atl1_tso(struct atl1_adapter *adapter, struct sk_buff *skb,
 				tso->tsopl |= 1 << TSO_PARAM_ETHTYPE_SHIFT;
 
 			tso->tsopl |= (iph->ihl &
-				CSUM_PARAM_IPHL_MASK) << CSUM_PARAM_IPHL_SHIFT;
-			tso->tsopl |= (tcp_hdrlen(skb) &
+				TSO_PARAM_IPHL_MASK) << TSO_PARAM_IPHL_SHIFT;
+			tso->tsopl |= ((tcp_hdrlen(skb) >> 2) &
 				TSO_PARAM_TCPHDRLEN_MASK) <<
 				TSO_PARAM_TCPHDRLEN_SHIFT;
 			tso->tsopl |= (skb_shinfo(skb)->gso_size &
@@ -1472,8 +1471,8 @@ static void atl1_tx_queue(struct atl1_adapter *adapter, int count,
 		tpd->desc.tso.tsopl = descr->tso.tsopl;
 		tpd->buffer_addr = cpu_to_le64(buffer_info->dma);
 		tpd->desc.data = descr->data;
-		tpd->desc.csum.csumpu |= (cpu_to_le16(buffer_info->length) &
-			CSUM_PARAM_BUFLEN_MASK) << CSUM_PARAM_BUFLEN_SHIFT;
+		tpd->desc.tso.tsopu |= (cpu_to_le16(buffer_info->length) ...
From: jacliburn
Date: Monday, December 31, 2007 - 6:59 pm

From: Jay Cliburn <jacliburn@bellsouth.net>

Add the ethtool register dump option to the atl1 driver.

Signed-off-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/atlx/atl1.c |   53 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/net/atlx/atl1.h |    1 +
 2 files changed, 54 insertions(+), 0 deletions(-)

diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index 4e98c16..239641f 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -2513,6 +2513,57 @@ static int atl1_set_wol(struct net_device *netdev,
 	return 0;
 }
 
+static int atl1_get_regs_len(struct net_device *netdev)
+{
+	return ATL1_REG_COUNT * sizeof(u32);
+}
+
+static void atl1_get_regs(struct net_device *netdev, struct ethtool_regs *regs,
+	void *p)
+{
+	struct atl1_adapter *adapter = netdev_priv(netdev);
+	struct atl1_hw *hw = &adapter->hw;
+	unsigned int i;
+	u32 *regbuf = p;
+
+	for (i = 0; i < ATL1_REG_COUNT; i++) {
+		/*
+		 * This switch statement avoids reserved regions
+		 * of register space.
+		 */
+		switch (i) {
+		case 6 ... 9:
+		case 14:
+		case 29 ... 31:
+		case 34 ... 63:
+		case 75 ... 127:
+		case 136 ... 1023:
+		case 1027 ... 1087:
+		case 1091 ... 1151:
+		case 1194 ... 1195:
+		case 1200 ... 1201:
+		case 1206 ... 1213:
+		case 1216 ... 1279:
+		case 1290 ... 1311:
+		case 1323 ... 1343:
+		case 1358 ... 1359:
+		case 1368 ... 1375:
+		case 1378 ... 1383:
+		case 1388 ... 1391:
+		case 1393 ... 1395:
+		case 1402 ... 1403:
+		case 1410 ... 1471:
+		case 1522 ... 1535:
+			/* reserved region; don't read it */
+			regbuf[i] = 0;
+			break;
+		default:
+			/* unreserved region */
+			regbuf[i] = ioread32(hw->hw_addr + (i * sizeof(u32)));
+		}
+	}
+}
+
 static void atl1_get_ringparam(struct net_device *netdev,
 	struct ethtool_ringparam *ring)
 {
@@ -2703,6 +2754,8 @@ const struct ethtool_ops atl1_ethtool_ops = {
 	.get_drvinfo		= atl1_get_drvinfo,
 	.get_wol		= atl1_get_wol,
 	.set_wol		= ...
From: Jeff Garzik
Date: Tuesday, January 22, 2008 - 2:54 am

ACK patches 1-4


--

From: jacliburn
Date: Monday, December 31, 2007 - 6:59 pm

From: Jay Cliburn <jacliburn@bellsouth.net>

Add some debug dev_printks if we encounter a bad receive return descriptor.

Signed-off-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/atlx/atl1.c |   21 ++++++++++++++++++++-
 1 files changed, 20 insertions(+), 1 deletions(-)

diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index 239641f..262d3ca 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -1127,7 +1127,26 @@ static void atl1_intr_rx(struct atl1_adapter *adapter)
 		if (likely(rrd->xsz.valid)) {	/* packet valid */
 chk_rrd:
 			/* check rrd status */
-			if (likely(rrd->num_buf == 1))
+			if (rrd->num_buf != 1) {
+				dev_printk(KERN_DEBUG, &adapter->pdev->dev,
+					"rx_buf_len = %d\n",
+					adapter->rx_buffer_len);
+				dev_printk(KERN_DEBUG, &adapter->pdev->dev,
+					"RRD num_buf = %d\n",
+					rrd->num_buf);
+				dev_printk(KERN_DEBUG, &adapter->pdev->dev,
+					"RRD pkt_len = %d\n",
+					rrd->xsz.xsum_sz.pkt_size);
+				dev_printk(KERN_DEBUG, &adapter->pdev->dev,
+					"RRD pkt_flg = 0x%08X\n",
+					rrd->pkt_flg);
+				dev_printk(KERN_DEBUG, &adapter->pdev->dev,
+					"RRD err_flg = 0x%08X\n",
+					rrd->err_flg);
+				dev_printk(KERN_DEBUG, &adapter->pdev->dev,
+					"RRD vlan_tag = 0x%08X\n",
+					rrd->vlan_tag);
+			} else
 				goto rrd_ok;
 
 			/* rrd seems to be bad */
-- 
1.5.3.3

--

From: Jeff Garzik
Date: Tuesday, January 22, 2008 - 2:55 am

look into netif_msg_*, which enables via ethtool very fine-grained 
selection of messages.

--

From: jacliburn
Date: Monday, December 31, 2007 - 6:59 pm

From: Jay Cliburn <jacliburn@bellsouth.net>

Update initialization parameters to match the current vendor driver
version 1.2.40.2.

Signed-off-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/atlx/atl1.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index 262d3ca..695dcbc 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -136,20 +136,20 @@ static int __devinit atl1_sw_init(struct atl1_adapter *adapter)
 	hw->rfd_fetch_gap = 1;
 	hw->rx_jumbo_th = adapter->rx_buffer_len / 8;
 	hw->rx_jumbo_lkah = 1;
-	hw->rrd_ret_timer = 16;
-	hw->tpd_burst = 4;
+	hw->rrd_ret_timer = 4;	/* 8 us */
+	hw->tpd_burst = 8;
 	hw->tpd_fetch_th = 16;
-	hw->txf_burst = 0x100;
+	hw->txf_burst = 0x200;
 	hw->tx_jumbo_task_th = (hw->max_frame_size + 7) >> 3;
 	hw->tpd_fetch_gap = 1;
 	hw->rcb_value = atl1_rcb_64;
 	hw->dma_ord = atl1_dma_ord_enh;
-	hw->dmar_block = atl1_dma_req_256;
-	hw->dmaw_block = atl1_dma_req_256;
+	hw->dmar_block = atl1_dma_req_1024;
+	hw->dmaw_block = atl1_dma_req_1024;
 	hw->cmb_rrd = 4;
 	hw->cmb_tpd = 4;
-	hw->cmb_rx_timer = 1;	/* about 2us */
-	hw->cmb_tx_timer = 1;	/* about 2us */
+	hw->cmb_rx_timer = 2;	/* about 4us */
+	hw->cmb_tx_timer = 256;	/* about 512us */
 	hw->smb_timer = 100000;	/* about 200ms */
 
 	spin_lock_init(&adapter->lock);
-- 
1.5.3.3

--

From: Jeff Garzik
Date: Tuesday, January 22, 2008 - 2:56 am

ACK without any better knowledge...  but is any addition insight 
available at all?

--

From: Jay Cliburn
Date: Tuesday, January 22, 2008 - 7:13 pm

On Tue, 22 Jan 2008 04:56:11 -0500


No, sorry Jeff.  I simply took the vendor's current driver and matched
his initialization settings.  I can only assume he discovered these
values through lab testing.

For this and the other "conform to vendor driver" patches in this set, I
thought it important to have the in-tree driver match the vendor driver
as closely as possible.  The primary motivations are (1) my belief that
he's in a better position to test the NIC, and (2) to be able to go to
him for assistance occasionally and not be rejected because of
significant differences between his and our drivers.

Jay
--

From: Jeff Garzik
Date: Tuesday, January 22, 2008 - 7:19 pm

Since these changes are not simply moving code around, we really do need 
full explanations for them, and to understand their need.

Blindly copying code from an exterior driver is pointless, and no way at 
all to run an engineering process.

If the driver is not going to get the review and attention necessary, 
bug fixes and feedback attended-to, then there's not much point in 
having this driver in the kernel at all.

You will only lead yourself to frustration, if you set up a system where 
changes only flow one way.  That's not how Linux development is done at all.

	Jeff



--

From: Chris Snook
Date: Tuesday, January 22, 2008 - 7:30 pm

I don't think we should be doing this without justification.  From all the atl1 
and atl2 code I've looked at, I've gotten the impression that their driver 
development processes are extremely ad-hoc.  There is code in the Atheros 
version of atl2 that cannot *possibly* apply to that hardware and was just 
copied and pasted from atl1, just as much of atl1 was copied and pasted from 
e1000.  The fact that various versions have different magic numbers may simply 
mean they copied and pasted from different irrelevant and incorrect sources.

Our contacts at Atheros seem to be very good electrical engineers, so when they 
tell us that a certain setting should be changed to match particular properties 
of the hardware, I trust them.  They are not, however, experienced and 
disciplined kernel developers, so absent such justification I think we should 
stick with what we have, which has been improved and reviewed by people who 
*are* experienced and disciplined kernel developers.

We have at least as much to teach Atheros about writing kernel code as they have 
to teach us about their hardware.

	-- Chris
--

From: jacliburn
Date: Monday, December 31, 2007 - 6:59 pm

From: Jay Cliburn <jacliburn@bellsouth.net>

When we initially set max rx frame size, we don't explicitly allow room for
the VLAN header; it's done later in a somewhat obscure fashion.  Let's make
it clear from the top that we've allowed enough room for the VLAN header.

Signed-off-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/atlx/atl1.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index 695dcbc..c93cf19 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -113,7 +113,7 @@ static int __devinit atl1_sw_init(struct atl1_adapter *adapter)
 	struct atl1_hw *hw = &adapter->hw;
 	struct net_device *netdev = adapter->netdev;
 
-	hw->max_frame_size = netdev->mtu + ETH_HLEN + ETH_FCS_LEN;
+	hw->max_frame_size = netdev->mtu + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN;
 	hw->min_frame_size = ETH_ZLEN + ETH_FCS_LEN;
 
 	adapter->wol = 0;
@@ -744,8 +744,8 @@ static u32 atl1_configure(struct atl1_adapter *adapter)
 	/* set Interrupt Clear Timer */
 	iowrite16(adapter->ict, hw->hw_addr + REG_CMBDISDMA_TIMER);
 
-	/* set MTU, 4 : VLAN */
-	iowrite32(hw->max_frame_size + 4, hw->hw_addr + REG_MTU);
+	/* set MTU size */
+	iowrite32(hw->max_frame_size, hw->hw_addr + REG_MTU);
 
 	/* jumbo size & rrd retirement timer */
 	value = (((u32) hw->rx_jumbo_th & RXQ_JMBOSZ_TH_MASK)
-- 
1.5.3.3

--

Previous thread: none

Next thread: [Patch 0/8] Remove 'TOPDIR' from Makefiles by WANG Cong on Tuesday, January 1, 2008 - 12:13 am. (43 messages)