[PATCH 0/3] pch_gbe: fix CRC errors and improve speed

[PATCH 0/3] pch_gbe: fix CRC errors and improve speed

[Veaceslav Falico]

This patch series tries to get rid of CRC errors and other instability
caused by wrong RX_FIFO overrun processing. It also cleans the code a bit.

Now (without the patch applied) on high number of incoming packets and/or
stressed system pch_gbe driver reports CRC errors and huge numbers of
RX_FIFO_ERR (overruns). It also sometimes just freezes (caused by not
waiting enough on DMA_RX to stop before issuing a reset on MAC_RX logic).

This patchset removes the unneeded (as per datasheet) MAC_RX reset and
corrects the processing of RX_FIFO_ERR in pch_gbe_napi_poll().

With the patchset applied there are no more CRC errors, sane number of
RX_FIFO_ERR and improved speed/stability. The patchset was tested on
Informix SYS940X and Kontron Tunnel Creek EG20T on 1gbps direct-connected
link and different rx/tx descriptors count.

 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   |   99 +++++--------------
 1 files changed, 26 insertions(+), 73 deletions(-)

[PATCH 1/3] pch_gbe: create functions for MAC_RX {en,dis}able

[Veaceslav Falico]

Move MAC_RX-related bits into separate functions.

Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   |   26 +++++++++++++++----
 1 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index b2a94d0..d5190bf 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -420,6 +420,22 @@ static void pch_gbe_mac_reset_rx(struct pch_gbe_hw *hw)
 	return;
 }
 
+static void pch_gbe_disable_mac_rx(struct pch_gbe_hw *hw)
+{
+	u32 rctl;
+	/* Disables Receive MAC */
+	rctl = ioread32(&hw->reg->MAC_RX_EN);
+	iowrite32((rctl & ~PCH_GBE_MRE_MAC_RX_EN), &hw->reg->MAC_RX_EN);
+}
+
+static void pch_gbe_enable_mac_rx(struct pch_gbe_hw *hw)
+{
+	u32 rctl;
+	/* Enables Receive MAC */
+	rctl = ioread32(&hw->reg->MAC_RX_EN);
+	iowrite32((rctl | PCH_GBE_MRE_MAC_RX_EN), &hw->reg->MAC_RX_EN);
+}
+
 /**
  * pch_gbe_mac_init_rx_addrs - Initialize receive address's
  * @hw:	Pointer to the HW structure
@@ -913,7 +929,7 @@ static void pch_gbe_setup_rctl(struct pch_gbe_adapter *adapter)
 static void pch_gbe_configure_rx(struct pch_gbe_adapter *adapter)
 {
 	struct pch_gbe_hw *hw = &adapter->hw;
-	u32 rdba, rdlen, rctl, rxdma;
+	u32 rdba, rdlen, rxdma;
 
 	pr_debug("dma adr = 0x%08llx  size = 0x%08x\n",
 		 (unsigned long long)adapter->rx_ring->dma,
@@ -921,9 +937,7 @@ static void pch_gbe_configure_rx(struct pch_gbe_adapter *adapter)
 
 	pch_gbe_mac_force_mac_fc(hw);
 
-	/* Disables Receive MAC */
-	rctl = ioread32(&hw->reg->MAC_RX_EN);
-	iowrite32((rctl & ~PCH_GBE_MRE_MAC_RX_EN), &hw->reg->MAC_RX_EN);
+	pch_gbe_disable_mac_rx(hw);
 
 	/* Disables Receive DMA */
 	rxdma = ioread32(&hw->reg->DMA_CTRL);
@@ -1355,8 +1369,8 @@ static void pch_gbe_start_receive(struct pch_gbe_hw *hw)
 	rxdma = ioread32(&hw->reg->DMA_CTRL);
 	rxdma |= PCH_GBE_RX_DMA_EN;
 	iowrite32(rxdma, &hw->reg->DMA_CTRL);
-	/* Enables Receive */
-	iowrite32(PCH_GBE_MRE_MAC_RX_EN, &hw->reg->MAC_RX_EN);
+
+	pch_gbe_enable_mac_rx(hw);
 	return;
 }
 
-- 
1.7.1

[PATCH 2/3] pch_gbe: don't re-set RX_FIFO_ERR flag in napi_poll

[Veaceslav Falico]

If we were in RX_FIFO_ERR state and entered pch_gbe_napi_poll(), we'll
anyway clean some rx space and thus can continue to receive more packets.
Currently, we re-set the RX_FIFO_ERR in situations when we've exhausted our
budget for RX cleaning or cleaned some TX packets. Removing it gives us
+20%-40% speed increase and a lot less of RX_FIFO_ERRors reported.

Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   |   19 ++++++-------------
 1 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index d5190bf..4ffad78 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -2419,7 +2419,6 @@ static int pch_gbe_napi_poll(struct napi_struct *napi, int budget)
 	int work_done = 0;
 	bool poll_end_flag = false;
 	bool cleaned = false;
-	u32 int_en;
 
 	pr_debug("budget : %d\n", budget);
 
@@ -2436,19 +2435,13 @@ static int pch_gbe_napi_poll(struct napi_struct *napi, int budget)
 
 	if (poll_end_flag) {
 		napi_complete(napi);
-		if (adapter->rx_stop_flag) {
-			adapter->rx_stop_flag = false;
-			pch_gbe_start_receive(&adapter->hw);
-		}
 		pch_gbe_irq_enable(adapter);
-	} else
-		if (adapter->rx_stop_flag) {
-			adapter->rx_stop_flag = false;
-			pch_gbe_start_receive(&adapter->hw);
-			int_en = ioread32(&adapter->hw.reg->INT_EN);
-			iowrite32((int_en | PCH_GBE_INT_RX_FIFO_ERR),
-				&adapter->hw.reg->INT_EN);
-		}
+	}
+
+	if (adapter->rx_stop_flag) {
+		adapter->rx_stop_flag = false;
+		pch_gbe_start_receive(&adapter->hw);
+	}
 
 	pr_debug("poll_end_flag : %d  work_done : %d  budget : %d\n",
 		 poll_end_flag, work_done, budget);
-- 
1.7.1

[PATCH 3/3] pch_gbe: don't reset MAC_RX on FIFO overflow

[Veaceslav Falico]

Currently, when FIFO_ERR happens, we stop the dma, wait for it to become
idle and then reset the whole MAC_RX logic (and after that we must re-set
multicast addresses and also re-enable MAC_RX when we're finally ready to
accept new packets). This leads to CRC errors on high number of incoming
packets and is not needed according to the datasheet.

This patch fixes it by the following steps:

1) remove this reset in pch_gbe_stop_receive(), which causes some functions
to not be used anywhere
2) remove already unused functions pch_gbe_wait_clr_bit_irq() and
pch_gbe_mac_reset_rx() to correctly build
3) move pch_gbe_enable_mac_rx() out of pch_gbe_start_receive() to
pch_gbe_up() where it's only needed after we've removed the MAC_RX reset
4) rename pch_gbe_start/stop_receive() to pch_gbe_enable/disable_dma_rx()
to more precisely reflect what the functions are now doing.

After these changes we already don't see the CRC errors and gain some
increase in RX processing speed.

Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   |   66 ++------------------
 1 files changed, 6 insertions(+), 60 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 4ffad78..a8854d0 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -339,26 +339,6 @@ static void pch_gbe_wait_clr_bit(void *reg, u32 bit)
 }
 
 /**
- * pch_gbe_wait_clr_bit_irq - Wait to clear a bit for interrupt context
- * @reg:	Pointer of register
- * @busy:	Busy bit
- */
-static int pch_gbe_wait_clr_bit_irq(void *reg, u32 bit)
-{
-	u32 tmp;
-	int ret = -1;
-	/* wait busy */
-	tmp = 20;
-	while ((ioread32(reg) & bit) && --tmp)
-		udelay(5);
-	if (!tmp)
-		pr_err("Error: busy bit is not cleared\n");
-	else
-		ret = 0;
-	return ret;
-}
-
-/**
  * pch_gbe_mac_mar_set - Set MAC address register
  * @hw:	    Pointer to the HW structure
  * @addr:   Pointer to the MAC address
@@ -409,17 +389,6 @@ static void pch_gbe_mac_reset_hw(struct pch_gbe_hw *hw)
 	return;
 }
 
-static void pch_gbe_mac_reset_rx(struct pch_gbe_hw *hw)
-{
-	/* Read the MAC addresses. and store to the private data */
-	pch_gbe_mac_read_mac_addr(hw);
-	iowrite32(PCH_GBE_RX_RST, &hw->reg->RESET);
-	pch_gbe_wait_clr_bit_irq(&hw->reg->RESET, PCH_GBE_RX_RST);
-	/* Setup the MAC addresses */
-	pch_gbe_mac_mar_set(hw, hw->mac.addr, 0);
-	return;
-}
-
 static void pch_gbe_disable_mac_rx(struct pch_gbe_hw *hw)
 {
 	u32 rctl;
@@ -1330,38 +1299,17 @@ void pch_gbe_update_stats(struct pch_gbe_adapter *adapter)
 	spin_unlock_irqrestore(&adapter->stats_lock, flags);
 }
 
-static void pch_gbe_stop_receive(struct pch_gbe_adapter *adapter)
+static void pch_gbe_disable_dma_rx(struct pch_gbe_hw *hw)
 {
-	struct pch_gbe_hw *hw = &adapter->hw;
 	u32 rxdma;
-	u16 value;
-	int ret;
 
 	/* Disable Receive DMA */
 	rxdma = ioread32(&hw->reg->DMA_CTRL);
 	rxdma &= ~PCH_GBE_RX_DMA_EN;
 	iowrite32(rxdma, &hw->reg->DMA_CTRL);
-	/* Wait Rx DMA BUS is IDLE */
-	ret = pch_gbe_wait_clr_bit_irq(&hw->reg->RX_DMA_ST, PCH_GBE_IDLE_CHECK);
-	if (ret) {
-		/* Disable Bus master */
-		pci_read_config_word(adapter->pdev, PCI_COMMAND, &value);
-		value &= ~PCI_COMMAND_MASTER;
-		pci_write_config_word(adapter->pdev, PCI_COMMAND, value);
-		/* Stop Receive */
-		pch_gbe_mac_reset_rx(hw);
-		/* Enable Bus master */
-		value |= PCI_COMMAND_MASTER;
-		pci_write_config_word(adapter->pdev, PCI_COMMAND, value);
-	} else {
-		/* Stop Receive */
-		pch_gbe_mac_reset_rx(hw);
-	}
-	/* reprogram multicast address register after reset */
-	pch_gbe_set_multi(adapter->netdev);
 }
 
-static void pch_gbe_start_receive(struct pch_gbe_hw *hw)
+static void pch_gbe_enable_dma_rx(struct pch_gbe_hw *hw)
 {
 	u32 rxdma;
 
@@ -1369,9 +1317,6 @@ static void pch_gbe_start_receive(struct pch_gbe_hw *hw)
 	rxdma = ioread32(&hw->reg->DMA_CTRL);
 	rxdma |= PCH_GBE_RX_DMA_EN;
 	iowrite32(rxdma, &hw->reg->DMA_CTRL);
-
-	pch_gbe_enable_mac_rx(hw);
-	return;
 }
 
 /**
@@ -1407,7 +1352,7 @@ static irqreturn_t pch_gbe_intr(int irq, void *data)
 			int_en = ioread32(&hw->reg->INT_EN);
 			iowrite32((int_en & ~PCH_GBE_INT_RX_FIFO_ERR),
 				  &hw->reg->INT_EN);
-			pch_gbe_stop_receive(adapter);
+			pch_gbe_disable_dma_rx(&adapter->hw);
 			int_st |= ioread32(&hw->reg->INT_ST);
 			int_st = int_st & ioread32(&hw->reg->INT_EN);
 		}
@@ -2014,7 +1959,8 @@ int pch_gbe_up(struct pch_gbe_adapter *adapter)
 	pch_gbe_alloc_tx_buffers(adapter, tx_ring);
 	pch_gbe_alloc_rx_buffers(adapter, rx_ring, rx_ring->count);
 	adapter->tx_queue_len = netdev->tx_queue_len;
-	pch_gbe_start_receive(&adapter->hw);
+	pch_gbe_enable_dma_rx(&adapter->hw);
+	pch_gbe_enable_mac_rx(&adapter->hw);
 
 	mod_timer(&adapter->watchdog_timer, jiffies);
 
@@ -2440,7 +2386,7 @@ static int pch_gbe_napi_poll(struct napi_struct *napi, int budget)
 
 	if (adapter->rx_stop_flag) {
 		adapter->rx_stop_flag = false;
-		pch_gbe_start_receive(&adapter->hw);
+		pch_gbe_enable_dma_rx(&adapter->hw);
 	}
 
 	pr_debug("poll_end_flag : %d  work_done : %d  budget : %d\n",
-- 
1.7.1

Re: [PATCH 0/3] pch_gbe: fix CRC errors and improve speed

[David Miller]

From: Veaceslav Falico <vfalico@redhat.com>
Date: Mon, 22 Oct 2012 16:43:22 +0200

> This patch series tries to get rid of CRC errors and other instability
> caused by wrong RX_FIFO overrun processing. It also cleans the code a bit.
> 
> Now (without the patch applied) on high number of incoming packets and/or
> stressed system pch_gbe driver reports CRC errors and huge numbers of
> RX_FIFO_ERR (overruns). It also sometimes just freezes (caused by not
> waiting enough on DMA_RX to stop before issuing a reset on MAC_RX logic).
> 
> This patchset removes the unneeded (as per datasheet) MAC_RX reset and
> corrects the processing of RX_FIFO_ERR in pch_gbe_napi_poll().
> 
> With the patchset applied there are no more CRC errors, sane number of
> RX_FIFO_ERR and improved speed/stability. The patchset was tested on
> Informix SYS940X and Kontron Tunnel Creek EG20T on 1gbps direct-connected
> link and different rx/tx descriptors count.

All applied to 'net', thanks.