[PATCH] pch_gbe: Fix transmit queue management

[PATCH] pch_gbe: Fix transmit queue management

[Ondrej Puzman]

According to Documentation/networking/driver.txt the ndo_start_xmit method should not return NETDEV_TX_BUSY under normal circumstances.
Stop the transmit queue when tx_ring is full.

Signed-off-by: Ondrej Puzman <puzman@gmail.com>
---
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   |    4 ++++
 1 files changed, 4 insertions(+), 0 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 27ffe0e..6764e59 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
@@ -2157,6 +2157,10 @@ static int pch_gbe_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 
 	/* CRC,ITAG no support */
 	pch_gbe_tx_queue(adapter, tx_ring, skb);
+
+	if (unlikely(!PCH_GBE_DESC_UNUSED(tx_ring)))
+		netif_stop_queue(netdev);
+
 	spin_unlock_irqrestore(&tx_ring->tx_lock, flags);
 	return NETDEV_TX_OK;
 }
-- 
1.5.6.5

[PATCH] pch_gbe: Remove tx_queue_len setting from the driver

[Ondrej Puzman]

The driver should not touch netdev tx_queue_len when changing link speed.

Signed-off-by: Ondrej Puzman <puzman@gmail.com>
---
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h    |    2 --
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   |    5 -----
 2 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
index 2a90030..f450d09 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
@@ -615,7 +615,6 @@ struct pch_gbe_privdata {
  * @tx_ring:		Pointer of Tx descriptor ring structure
  * @rx_ring:		Pointer of Rx descriptor ring structure
  * @rx_buffer_len:	Receive buffer length
- * @tx_queue_len:	Transmit queue length
  * @have_msi:		PCI MSI mode flag
  * @pch_gbe_privdata:	PCI Device ID driver_data
  */
@@ -639,7 +638,6 @@ struct pch_gbe_adapter {
 	struct pch_gbe_tx_ring *tx_ring;
 	struct pch_gbe_rx_ring *rx_ring;
 	unsigned long rx_buffer_len;
-	unsigned long tx_queue_len;
 	bool have_msi;
 	bool rx_stop_flag;
 	int hwts_tx_en;
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 6764e59..d967603 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
@@ -1076,11 +1076,9 @@ static void pch_gbe_set_mode(struct pch_gbe_adapter *adapter, u16 speed,
 	switch (speed) {
 	case SPEED_10:
 		mode = PCH_GBE_MODE_MII_ETHER;
-		netdev->tx_queue_len = 10;
 		break;
 	case SPEED_100:
 		mode = PCH_GBE_MODE_MII_ETHER;
-		netdev->tx_queue_len = 100;
 		break;
 	case SPEED_1000:
 		mode = PCH_GBE_MODE_GMII_ETHER;
@@ -1108,7 +1106,6 @@ static void pch_gbe_watchdog(unsigned long data)
 	pch_gbe_update_stats(adapter);
 	if ((mii_link_ok(&adapter->mii)) && (!netif_carrier_ok(netdev))) {
 		struct ethtool_cmd cmd = { .cmd = ETHTOOL_GSET };
-		netdev->tx_queue_len = adapter->tx_queue_len;
 		/* mii library handles link maintenance tasks */
 		if (mii_ethtool_gset(&adapter->mii, &cmd)) {
 			netdev_err(netdev, "ethtool get setting Error\n");
@@ -1975,7 +1972,6 @@ 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_enable_dma_rx(&adapter->hw);
 	pch_gbe_enable_mac_rx(&adapter->hw);
 
@@ -2013,7 +2009,6 @@ void pch_gbe_down(struct pch_gbe_adapter *adapter)
 
 	del_timer_sync(&adapter->watchdog_timer);
 
-	netdev->tx_queue_len = adapter->tx_queue_len;
 	netif_carrier_off(netdev);
 	netif_stop_queue(netdev);
 
-- 
1.5.6.5

[PATCH] pch_gbe: Add DQL support to the driver

[Ondrej Puzman]

Add dynamic queue limits support to the driver.

Signed-off-by: Ondrej Puzman <puzman@gmail.com>
---
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   |    9 +++++++++
 1 files changed, 9 insertions(+), 0 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 d967603..2c14e2e 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
@@ -994,6 +994,8 @@ static void pch_gbe_clean_tx_ring(struct pch_gbe_adapter *adapter,
 	netdev_dbg(adapter->netdev,
 		   "call pch_gbe_unmap_and_free_tx_resource() %d count\n", i);
 
+	netdev_reset_queue(adapter->netdev);
+
 	size = (unsigned long)sizeof(struct pch_gbe_buffer) * tx_ring->count;
 	memset(tx_ring->buffer_info, 0, size);
 
@@ -1544,6 +1546,7 @@ pch_gbe_clean_tx(struct pch_gbe_adapter *adapter,
 	unsigned int cleaned_count = 0;
 	bool cleaned = false;
 	int unused, thresh;
+	unsigned int bytes_compl = 0, pkts_compl = 0;
 
 	netdev_dbg(adapter->netdev, "next_to_clean : %d\n",
 		   tx_ring->next_to_clean);
@@ -1622,6 +1625,8 @@ pch_gbe_clean_tx(struct pch_gbe_adapter *adapter,
 			buffer_info->mapped = false;
 		}
 		if (buffer_info->skb) {
+			bytes_compl += buffer_info->skb->len;
+			pkts_compl++;
 			netdev_dbg(adapter->netdev,
 				   "trim buffer_info->skb : %d\n", i);
 			skb_trim(buffer_info->skb, 0);
@@ -1641,6 +1646,9 @@ pch_gbe_clean_tx(struct pch_gbe_adapter *adapter,
 		   "called pch_gbe_unmap_and_free_tx_resource() %d count\n",
 		   cleaned_count);
 	if (cleaned_count > 0)  { /*skip this if nothing cleaned*/
+		netdev_completed_queue(adapter->netdev,
+				       pkts_compl, bytes_compl);
+
 		/* Recover from running out of Tx resources in xmit_frame */
 		spin_lock(&tx_ring->tx_lock);
 		if (unlikely(cleaned && (netif_queue_stopped(adapter->netdev))))
@@ -2152,6 +2160,7 @@ static int pch_gbe_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 
 	/* CRC,ITAG no support */
 	pch_gbe_tx_queue(adapter, tx_ring, skb);
+	netdev_sent_queue(netdev, skb->len);
 
 	if (unlikely(!PCH_GBE_DESC_UNUSED(tx_ring)))
 		netif_stop_queue(netdev);
-- 
1.5.6.5

[PATCH] pch_gbe: Fix: Link configuration in module parameters was ignored

[Ondrej Puzman]

PHY configuration set in pch_gbe_phy_init_setting() by mii_ethtool_sset() was immedeiately reset by two calls of pch_gbe_phy_sw_reset().
Result of this bug was that module parameters like AutoNeg were ignored silently.

Signed-off-by: Ondrej Puzman <puzman@gmail.com>
---
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c    |    4 ----
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c    |   13 +------------
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h    |    1 -
 3 files changed, 1 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
index ff3ad70..365c202 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
@@ -77,10 +77,6 @@ static s32 pch_gbe_plat_init_hw(struct pch_gbe_hw *hw)
 		return ret_val;
 	}
 	pch_gbe_phy_init_setting(hw);
-	/* Setup Mac interface option RGMII */
-#ifdef PCH_GBE_MAC_IFOP_RGMII
-	pch_gbe_phy_set_rgmii(hw);
-#endif
 	return ret_val;
 }
 
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c
index 8b7ff75..d283818 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c
@@ -249,15 +249,6 @@ void pch_gbe_phy_power_down(struct pch_gbe_hw *hw)
 }
 
 /**
- * pch_gbe_phy_set_rgmii - RGMII interface setting
- * @hw:	            Pointer to the HW structure
- */
-void pch_gbe_phy_set_rgmii(struct pch_gbe_hw *hw)
-{
-	pch_gbe_phy_sw_reset(hw);
-}
-
-/**
  * pch_gbe_phy_tx_clk_delay - Setup TX clock delay via the PHY
  * @hw:	            Pointer to the HW structure
  * Returns
@@ -321,13 +312,11 @@ void pch_gbe_phy_init_setting(struct pch_gbe_hw *hw)
 	cmd.duplex = hw->mac.link_duplex;
 	cmd.advertising = hw->phy.autoneg_advertised;
 	cmd.autoneg = hw->mac.autoneg;
-	pch_gbe_phy_write_reg_miic(hw, MII_BMCR, BMCR_RESET);
+	pch_gbe_phy_sw_reset(hw);
 	ret = mii_ethtool_sset(&adapter->mii, &cmd);
 	if (ret)
 		netdev_err(adapter->netdev, "Error: mii_ethtool_sset\n");
 
-	pch_gbe_phy_sw_reset(hw);
-
 	pch_gbe_phy_read_reg_miic(hw, PHY_PHYSP_CONTROL, &mii_reg);
 	mii_reg |= PHYSP_CTRL_ASSERT_CRS_TX;
 	pch_gbe_phy_write_reg_miic(hw, PHY_PHYSP_CONTROL, mii_reg);
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h
index 0cbe692..cda223c 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h
@@ -31,7 +31,6 @@ void pch_gbe_phy_sw_reset(struct pch_gbe_hw *hw);
 void pch_gbe_phy_hw_reset(struct pch_gbe_hw *hw);
 void pch_gbe_phy_power_up(struct pch_gbe_hw *hw);
 void pch_gbe_phy_power_down(struct pch_gbe_hw *hw);
-void pch_gbe_phy_set_rgmii(struct pch_gbe_hw *hw);
 void pch_gbe_phy_init_setting(struct pch_gbe_hw *hw);
 int pch_gbe_phy_disable_hibernate(struct pch_gbe_hw *hw);
 
-- 
1.5.6.5

Re: [PATCH] pch_gbe: Remove tx_queue_len setting from the driver

[David Miller]

When submitting a series of patches to the same files, you must number
them so that the order in which the patches need to be applied is
explicit.

Please read Documentation/SubmittingPatches in the kernel sources.

Thank you.

[PATCH 0/4] pch_gbe fixes and enhancements

[Ondrej Puzman]

Hi,
I'm sending the pch_gbe patch set again per David Miller's request.
Now the patches should be correctly numbered.

The patches fixes problems I found on our industrial computer with Intel Atom E680T CPU and EG20T chipset.

Ondrej Puzman (4):
  pch_gbe: Fix transmit queue management
  pch_gbe: Remove tx_queue_len setting from the driver
  pch_gbe: Add DQL support to the driver
  pch_gbe: Fix: Link configuration in module parameters was ignored

 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h    |    2 --
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c    |    4 ----
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   |   18 +++++++++++++-----
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c    |   13 +------------
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h    |    1 -
 5 files changed, 14 insertions(+), 24 deletions(-)

-- 
1.7.2.3

[PATCH 1/4] pch_gbe: Fix transmit queue management

[Ondrej Puzman]

According to Documentation/networking/driver.txt the ndo_start_xmit method should not return NETDEV_TX_BUSY under normal circumstances.
Stop the transmit queue when tx_ring is full.

Signed-off-by: Ondrej Puzman <puzman@gmail.com>
---
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   |    4 ++++
 1 files changed, 4 insertions(+), 0 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 27ffe0e..6764e59 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
@@ -2157,6 +2157,10 @@ static int pch_gbe_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 
 	/* CRC,ITAG no support */
 	pch_gbe_tx_queue(adapter, tx_ring, skb);
+
+	if (unlikely(!PCH_GBE_DESC_UNUSED(tx_ring)))
+		netif_stop_queue(netdev);
+
 	spin_unlock_irqrestore(&tx_ring->tx_lock, flags);
 	return NETDEV_TX_OK;
 }
-- 
1.7.2.3

[PATCH 2/4] pch_gbe: Remove tx_queue_len setting from the driver

[Ondrej Puzman]

The driver should not touch netdev tx_queue_len when changing link speed.

Signed-off-by: Ondrej Puzman <puzman@gmail.com>
---
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h    |    2 --
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   |    5 -----
 2 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
index 2a900307..f450d09 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
@@ -615,7 +615,6 @@ struct pch_gbe_privdata {
  * @tx_ring:		Pointer of Tx descriptor ring structure
  * @rx_ring:		Pointer of Rx descriptor ring structure
  * @rx_buffer_len:	Receive buffer length
- * @tx_queue_len:	Transmit queue length
  * @have_msi:		PCI MSI mode flag
  * @pch_gbe_privdata:	PCI Device ID driver_data
  */
@@ -639,7 +638,6 @@ struct pch_gbe_adapter {
 	struct pch_gbe_tx_ring *tx_ring;
 	struct pch_gbe_rx_ring *rx_ring;
 	unsigned long rx_buffer_len;
-	unsigned long tx_queue_len;
 	bool have_msi;
 	bool rx_stop_flag;
 	int hwts_tx_en;
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 6764e59..d967603 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
@@ -1076,11 +1076,9 @@ static void pch_gbe_set_mode(struct pch_gbe_adapter *adapter, u16 speed,
 	switch (speed) {
 	case SPEED_10:
 		mode = PCH_GBE_MODE_MII_ETHER;
-		netdev->tx_queue_len = 10;
 		break;
 	case SPEED_100:
 		mode = PCH_GBE_MODE_MII_ETHER;
-		netdev->tx_queue_len = 100;
 		break;
 	case SPEED_1000:
 		mode = PCH_GBE_MODE_GMII_ETHER;
@@ -1108,7 +1106,6 @@ static void pch_gbe_watchdog(unsigned long data)
 	pch_gbe_update_stats(adapter);
 	if ((mii_link_ok(&adapter->mii)) && (!netif_carrier_ok(netdev))) {
 		struct ethtool_cmd cmd = { .cmd = ETHTOOL_GSET };
-		netdev->tx_queue_len = adapter->tx_queue_len;
 		/* mii library handles link maintenance tasks */
 		if (mii_ethtool_gset(&adapter->mii, &cmd)) {
 			netdev_err(netdev, "ethtool get setting Error\n");
@@ -1975,7 +1972,6 @@ 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_enable_dma_rx(&adapter->hw);
 	pch_gbe_enable_mac_rx(&adapter->hw);
 
@@ -2013,7 +2009,6 @@ void pch_gbe_down(struct pch_gbe_adapter *adapter)
 
 	del_timer_sync(&adapter->watchdog_timer);
 
-	netdev->tx_queue_len = adapter->tx_queue_len;
 	netif_carrier_off(netdev);
 	netif_stop_queue(netdev);
 
-- 
1.7.2.3

[PATCH 3/4] pch_gbe: Add DQL support to the driver

[Ondrej Puzman]

Add dynamic queue limits support to the driver.

Signed-off-by: Ondrej Puzman <puzman@gmail.com>
---
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   |    9 +++++++++
 1 files changed, 9 insertions(+), 0 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 d967603..2c14e2e 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
@@ -994,6 +994,8 @@ static void pch_gbe_clean_tx_ring(struct pch_gbe_adapter *adapter,
 	netdev_dbg(adapter->netdev,
 		   "call pch_gbe_unmap_and_free_tx_resource() %d count\n", i);
 
+	netdev_reset_queue(adapter->netdev);
+
 	size = (unsigned long)sizeof(struct pch_gbe_buffer) * tx_ring->count;
 	memset(tx_ring->buffer_info, 0, size);
 
@@ -1544,6 +1546,7 @@ pch_gbe_clean_tx(struct pch_gbe_adapter *adapter,
 	unsigned int cleaned_count = 0;
 	bool cleaned = false;
 	int unused, thresh;
+	unsigned int bytes_compl = 0, pkts_compl = 0;
 
 	netdev_dbg(adapter->netdev, "next_to_clean : %d\n",
 		   tx_ring->next_to_clean);
@@ -1622,6 +1625,8 @@ pch_gbe_clean_tx(struct pch_gbe_adapter *adapter,
 			buffer_info->mapped = false;
 		}
 		if (buffer_info->skb) {
+			bytes_compl += buffer_info->skb->len;
+			pkts_compl++;
 			netdev_dbg(adapter->netdev,
 				   "trim buffer_info->skb : %d\n", i);
 			skb_trim(buffer_info->skb, 0);
@@ -1641,6 +1646,9 @@ pch_gbe_clean_tx(struct pch_gbe_adapter *adapter,
 		   "called pch_gbe_unmap_and_free_tx_resource() %d count\n",
 		   cleaned_count);
 	if (cleaned_count > 0)  { /*skip this if nothing cleaned*/
+		netdev_completed_queue(adapter->netdev,
+				       pkts_compl, bytes_compl);
+
 		/* Recover from running out of Tx resources in xmit_frame */
 		spin_lock(&tx_ring->tx_lock);
 		if (unlikely(cleaned && (netif_queue_stopped(adapter->netdev))))
@@ -2152,6 +2160,7 @@ static int pch_gbe_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 
 	/* CRC,ITAG no support */
 	pch_gbe_tx_queue(adapter, tx_ring, skb);
+	netdev_sent_queue(netdev, skb->len);
 
 	if (unlikely(!PCH_GBE_DESC_UNUSED(tx_ring)))
 		netif_stop_queue(netdev);
-- 
1.7.2.3

[PATCH 4/4] pch_gbe: Fix: Link configuration in module parameters was ignored

[Ondrej Puzman]

PHY configuration set in pch_gbe_phy_init_setting() by mii_ethtool_sset() was immedeiately reset by two calls of pch_gbe_phy_sw_reset().
Result of this bug was that module parameters like AutoNeg were ignored silently.

Signed-off-by: Ondrej Puzman <puzman@gmail.com>
---
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c    |    4 ----
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c    |   13 +------------
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h    |    1 -
 3 files changed, 1 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
index ff3ad70..365c202 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_api.c
@@ -77,10 +77,6 @@ static s32 pch_gbe_plat_init_hw(struct pch_gbe_hw *hw)
 		return ret_val;
 	}
 	pch_gbe_phy_init_setting(hw);
-	/* Setup Mac interface option RGMII */
-#ifdef PCH_GBE_MAC_IFOP_RGMII
-	pch_gbe_phy_set_rgmii(hw);
-#endif
 	return ret_val;
 }
 
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c
index 8b7ff75..d283818 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c
@@ -249,15 +249,6 @@ void pch_gbe_phy_power_down(struct pch_gbe_hw *hw)
 }
 
 /**
- * pch_gbe_phy_set_rgmii - RGMII interface setting
- * @hw:	            Pointer to the HW structure
- */
-void pch_gbe_phy_set_rgmii(struct pch_gbe_hw *hw)
-{
-	pch_gbe_phy_sw_reset(hw);
-}
-
-/**
  * pch_gbe_phy_tx_clk_delay - Setup TX clock delay via the PHY
  * @hw:	            Pointer to the HW structure
  * Returns
@@ -321,13 +312,11 @@ void pch_gbe_phy_init_setting(struct pch_gbe_hw *hw)
 	cmd.duplex = hw->mac.link_duplex;
 	cmd.advertising = hw->phy.autoneg_advertised;
 	cmd.autoneg = hw->mac.autoneg;
-	pch_gbe_phy_write_reg_miic(hw, MII_BMCR, BMCR_RESET);
+	pch_gbe_phy_sw_reset(hw);
 	ret = mii_ethtool_sset(&adapter->mii, &cmd);
 	if (ret)
 		netdev_err(adapter->netdev, "Error: mii_ethtool_sset\n");
 
-	pch_gbe_phy_sw_reset(hw);
-
 	pch_gbe_phy_read_reg_miic(hw, PHY_PHYSP_CONTROL, &mii_reg);
 	mii_reg |= PHYSP_CTRL_ASSERT_CRS_TX;
 	pch_gbe_phy_write_reg_miic(hw, PHY_PHYSP_CONTROL, mii_reg);
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h
index 0cbe692..cda223c 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h
@@ -31,7 +31,6 @@ void pch_gbe_phy_sw_reset(struct pch_gbe_hw *hw);
 void pch_gbe_phy_hw_reset(struct pch_gbe_hw *hw);
 void pch_gbe_phy_power_up(struct pch_gbe_hw *hw);
 void pch_gbe_phy_power_down(struct pch_gbe_hw *hw);
-void pch_gbe_phy_set_rgmii(struct pch_gbe_hw *hw);
 void pch_gbe_phy_init_setting(struct pch_gbe_hw *hw);
 int pch_gbe_phy_disable_hibernate(struct pch_gbe_hw *hw);
 
-- 
1.7.2.3

Re: [PATCH 1/4] pch_gbe: Fix transmit queue management

[David Miller]

From: Ondrej Puzman <puzman@gmail.com>
Date: Thu, 28 Nov 2013 10:44:39 +0100

> According to Documentation/networking/driver.txt the ndo_start_xmit method should not return NETDEV_TX_BUSY under normal circumstances.
> Stop the transmit queue when tx_ring is full.

Please format your commit message text to 80 columns.

> 
> Signed-off-by: Ondrej Puzman <puzman@gmail.com>

You should be instead preventing the transmit method from being invoked
when it might be possible that a request cannot be satisfied.

This means that at the end of a transmit request, you must stop the
queue if a packet with the maximum number of possible descriptors
cannot be satisfied.  Then it is impossible for the transmit function
to be invoked in a situation where it would need to fail for lack of
available transmit descriptors.

This is why drivers decided whether to stop their TX queues based upon
calculations involving MAX_SKB_FRAGS.

Re: [PATCH 0/4] pch_gbe fixes and enhancements

[David Miller]

From: Ondrej Puzman <puzman@gmail.com>
Date: Thu, 28 Nov 2013 10:44:38 +0100

> Hi,
> I'm sending the pch_gbe patch set again per David Miller's request.
> Now the patches should be correctly numbered.
> 
> The patches fixes problems I found on our industrial computer with
> Intel Atom E680T CPU and EG20T chipset.

Some of these patches are adding new features, which is not appropriate
to submit until the net-next tree is open, which it is not.

It is also not appropriate to mix bug fixes with new feature additions.

Bug fixes should be separated out and submitted targetting the
'net' tree, and then new features should be submitted targetting the
'net-next' tree but only when it is open for new changes.

Re: [PATCH 1/4] pch_gbe: Fix transmit queue management

[Ondřej Pužman]

> Please format your commit  message text to 80 columns.

Ok, no problem. I did not know that 80 columns limitation applies also
for commit messages.

> You should be instead  preventing the transmit method from being
> invoked when it might be possible that a request cannot be
> satisfied. This means that at the end of a transmit request, you must
> stop the queue if a packet with the maximum number of possible
> descriptors cannot be satisfied. Then it is impossible for the
> transmit function to be invoked in a situation where it would need to
> fail for lack of available transmit descriptors. This is why drivers
> decided whether to stop their TX queues based upon calculations
> involving MAX_SKB_FRAGS.

Hmm, correct me if I'm wrong ... but the driver and hardware does not
support scatter-gather DMA. So fragmented skbs can't be passed to
ndo_start_xmit function and each skb passed to the function represents
one packet and one descriptor in tx_ring.
I do not think that in this caseit makes sense to to involve
MAX_SKB_FRAGS in any calculation and the queue should be stopped in the
moment when it gets full.
But maybe I am missing something, I am no expert in this kind of stuff.

--
Ondrej Puzman

RE: [PATCH 1/4] pch_gbe: Fix transmit queue management

[David Laight]

> From: David Miller
> 
> > According to Documentation/networking/driver.txt the ndo_start_xmit
> > method should not return NETDEV_TX_BUSY under normal circumstances.
> > Stop the transmit queue when tx_ring is full.
... 
> You should be instead preventing the transmit method from being invoked
> when it might be possible that a request cannot be satisfied.
> 
> This means that at the end of a transmit request, you must stop the
> queue if a packet with the maximum number of possible descriptors
> cannot be satisfied.  Then it is impossible for the transmit function
> to be invoked in a situation where it would need to fail for lack of
> available transmit descriptors.

I know you like ethernet drivers to work this way, but requiring enough
descriptors for a maximally fragmented packet requires a difficult
calculation and will cause the tx queue to be stopped unnecessarily.

IIRC a normal skb can have a maximum of 17 fragments, if these end up
being transmitted over USB using xhci they might need 34 descriptors
(because descriptors can't cross 64k boundaries).
However a typical 64k TCP packet just needs 4.
xhci has a further constraint that the ring end can only be at
specific alignments, in effect thus means that the descriptors for
a single packet cannot cross the end of a ring segment.
So if the available space in the xhci ring was used to stop the
ethernet tx queue (which it isn't at the moment) it would have to
be stopped very early.

Isn't there also a new skb structure that allows lists of fragments?
That might need even more descriptors for a worst case transmit.

I would have thought it would make sense for the ethernet driver
to stop the queue when there is a reasonable expectation that there
won't be enough descriptors for the next packet, but have a reasonable
code path when a packet with a large number of fragments won't fit
in the tx ring. This either requires the driver itself to hold the
skb, or to return NETDEV_TX_BUSY.
It might make sense to monitor recent traffic to find the 'expected
maximum number of fragments'.

	David

Re: [PATCH 1/4] pch_gbe: Fix transmit queue management

[David Miller]

From: "David Laight" <David.Laight@ACULAB.COM>
Date: Mon, 2 Dec 2013 10:04:49 -0000

> I know you like ethernet drivers to work this way, but requiring enough
> descriptors for a maximally fragmented packet requires a difficult
> calculation and will cause the tx queue to be stopped unnecessarily.
 ...
> Isn't there also a new skb structure that allows lists of fragments?
> That might need even more descriptors for a worst case transmit.

Fragment lists are not to be found in the transmit function of the
driver.  Any such SKBs will be linearized or similar first.  We do
want to support them in some capacity in the future, but right now
we don't.

And here we're not talking about a USB driver, so using the rather
extreme example of the USB segmentation restrictions is not really
appropriate.

This is a pretty standard ethernet driver, which can use a one to
one correspondance between the segmentation count of the SKB and
the number of TX descriptors that will be used.

There is a large precedence for doing things this way and in this
particular case it's straightforward.

Re: [PATCH 1/4] pch_gbe: Fix transmit queue management

[Eric Dumazet]

On Mon, 2013-12-02 at 11:16 -0500, David Miller wrote:

> Fragment lists are not to be found in the transmit function of the
> driver.  Any such SKBs will be linearized or similar first.  We do
> want to support them in some capacity in the future, but right now
> we don't.

Right. Such skb are segmented before reaching the driver, unless driver
asserts NETIF_F_FRAGLIST.

So far, only virtual drivers and 2 or 3 others advertise
NETIF_F_FRAGLIST 'support'.

I suspect that the '2 or 3 others' are plain bugs, I'll check this.

[PATCH] net: do not pretend FRAGLIST support

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

Few network drivers really supports frag_list : virtual drivers.

Some drivers wrongly advertise NETIF_F_FRAGLIST feature.

If skb with a frag_list is given to them, packet on the wire will be
corrupt.

Remove this flag, as core networking stack will make sure to
provide packets that can be sent without corruption.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
Cc: Anirudha Sarangi <anirudh@xilinx.com>
---
 drivers/net/ethernet/ibm/ehea/ehea_main.c         |    2 +-
 drivers/net/ethernet/tehuti/tehuti.c              |    1 -
 drivers/net/ethernet/xilinx/ll_temac_main.c       |    2 +-
 drivers/net/ethernet/xilinx/xilinx_axienet_main.c |    2 +-
 4 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ehea/ehea_main.c b/drivers/net/ethernet/ibm/ehea/ehea_main.c
index 2d1c6bdd3618..7628e0fd8455 100644
--- a/drivers/net/ethernet/ibm/ehea/ehea_main.c
+++ b/drivers/net/ethernet/ibm/ehea/ehea_main.c
@@ -3033,7 +3033,7 @@ static struct ehea_port *ehea_setup_single_port(struct ehea_adapter *adapter,
 
 	dev->hw_features = NETIF_F_SG | NETIF_F_TSO |
 		      NETIF_F_IP_CSUM | NETIF_F_HW_VLAN_CTAG_TX;
-	dev->features = NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_TSO |
+	dev->features = NETIF_F_SG | NETIF_F_TSO |
 		      NETIF_F_HIGHDMA | NETIF_F_IP_CSUM |
 		      NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX |
 		      NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_RXCSUM;
diff --git a/drivers/net/ethernet/tehuti/tehuti.c b/drivers/net/ethernet/tehuti/tehuti.c
index dd0dd6279b4e..4f1d2549130e 100644
--- a/drivers/net/ethernet/tehuti/tehuti.c
+++ b/drivers/net/ethernet/tehuti/tehuti.c
@@ -2019,7 +2019,6 @@ bdx_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		ndev->features = NETIF_F_IP_CSUM | NETIF_F_SG | NETIF_F_TSO
 		    | NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX |
 		    NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_RXCSUM
-		    /*| NETIF_F_FRAGLIST */
 		    ;
 		ndev->hw_features = NETIF_F_IP_CSUM | NETIF_F_SG |
 			NETIF_F_TSO | NETIF_F_HW_VLAN_CTAG_TX;
diff --git a/drivers/net/ethernet/xilinx/ll_temac_main.c b/drivers/net/ethernet/xilinx/ll_temac_main.c
index 1f2364126323..2166e879a096 100644
--- a/drivers/net/ethernet/xilinx/ll_temac_main.c
+++ b/drivers/net/ethernet/xilinx/ll_temac_main.c
@@ -1017,7 +1017,7 @@ static int temac_of_probe(struct platform_device *op)
 	platform_set_drvdata(op, ndev);
 	SET_NETDEV_DEV(ndev, &op->dev);
 	ndev->flags &= ~IFF_MULTICAST;  /* clear multicast */
-	ndev->features = NETIF_F_SG | NETIF_F_FRAGLIST;
+	ndev->features = NETIF_F_SG;
 	ndev->netdev_ops = &temac_netdev_ops;
 	ndev->ethtool_ops = &temac_ethtool_ops;
 #if 0
diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index b2ff038d6d20..f9293da19e26 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -1486,7 +1486,7 @@ static int axienet_of_probe(struct platform_device *op)
 
 	SET_NETDEV_DEV(ndev, &op->dev);
 	ndev->flags &= ~IFF_MULTICAST;  /* clear multicast */
-	ndev->features = NETIF_F_SG | NETIF_F_FRAGLIST;
+	ndev->features = NETIF_F_SG;
 	ndev->netdev_ops = &axienet_netdev_ops;
 	ndev->ethtool_ops = &axienet_ethtool_ops;
 

Re: [PATCH 1/4] pch_gbe: Fix transmit queue management

[Eric Dumazet]

On Mon, 2013-12-02 at 08:33 -0800, Eric Dumazet wrote:

> So far, only virtual drivers and 2 or 3 others advertise
> NETIF_F_FRAGLIST 'support'.
> 
> I suspect that the '2 or 3 others' are plain bugs, I'll check this.
> 

Yep, these all are plain bugs, I submitted a fix.

Re: [PATCH] net: do not pretend FRAGLIST support

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 02 Dec 2013 08:51:13 -0800

> From: Eric Dumazet <edumazet@google.com>
> 
> Few network drivers really supports frag_list : virtual drivers.
> 
> Some drivers wrongly advertise NETIF_F_FRAGLIST feature.
> 
> If skb with a frag_list is given to them, packet on the wire will be
> corrupt.
> 
> Remove this flag, as core networking stack will make sure to
> provide packets that can be sent without corruption.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Good catch, applied and queued up for -stable.