[PATCH 00/10] ftgmac: Rework batch 2 - RX path

[PATCH 00/10] ftgmac: Rework batch 2 - RX path

[Benjamin Herrenschmidt]

This is the second batch of updates to the ftgmac100 driver.

This one tackles the RX path of the driver, simplifying
it greatly to match common practice while significantly
increasing the performance.

(The bulk of the performance gains of my series will be
provided by the TX path improvements, notably fragmented
sends, these will be in the next batch).

[PATCH 01/10] ftgmac100: Move ftgmac100_alloc_rx_page() before its users

[Benjamin Herrenschmidt]

Avoids a forward declaration

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 59 +++++++++++++++-----------------
 1 file changed, 28 insertions(+), 31 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 88dab5f..c77a65b 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -86,9 +86,6 @@ struct ftgmac100 {
 	bool need_mac_restart;
 };
 
-static int ftgmac100_alloc_rx_page(struct ftgmac100 *priv,
-				   struct ftgmac100_rxdes *rxdes, gfp_t gfp);
-
 static void ftgmac100_set_rx_ring_base(struct ftgmac100 *priv, dma_addr_t addr)
 {
 	iowrite32(addr, priv->base + FTGMAC100_OFFSET_RXR_BADR);
@@ -399,6 +396,34 @@ static struct page *ftgmac100_rxdes_get_page(struct ftgmac100 *priv,
 	return *ftgmac100_rxdes_page_slot(priv, rxdes);
 }
 
+static int ftgmac100_alloc_rx_page(struct ftgmac100 *priv,
+				   struct ftgmac100_rxdes *rxdes, gfp_t gfp)
+{
+	struct net_device *netdev = priv->netdev;
+	struct page *page;
+	dma_addr_t map;
+
+	page = alloc_page(gfp);
+	if (!page) {
+		if (net_ratelimit())
+			netdev_err(netdev, "failed to allocate rx page\n");
+		return -ENOMEM;
+	}
+
+	map = dma_map_page(priv->dev, page, 0, RX_BUF_SIZE, DMA_FROM_DEVICE);
+	if (unlikely(dma_mapping_error(priv->dev, map))) {
+		if (net_ratelimit())
+			netdev_err(netdev, "failed to map rx page\n");
+		__free_page(page);
+		return -ENOMEM;
+	}
+
+	ftgmac100_rxdes_set_page(priv, rxdes, page);
+	ftgmac100_rxdes_set_dma_addr(rxdes, map);
+	ftgmac100_rxdes_set_dma_own(priv, rxdes);
+	return 0;
+}
+
 static int ftgmac100_next_rx_pointer(int pointer)
 {
 	return (pointer + 1) & (RX_QUEUE_ENTRIES - 1);
@@ -791,34 +816,6 @@ static int ftgmac100_xmit(struct ftgmac100 *priv, struct sk_buff *skb,
 	return NETDEV_TX_OK;
 }
 
-static int ftgmac100_alloc_rx_page(struct ftgmac100 *priv,
-				   struct ftgmac100_rxdes *rxdes, gfp_t gfp)
-{
-	struct net_device *netdev = priv->netdev;
-	struct page *page;
-	dma_addr_t map;
-
-	page = alloc_page(gfp);
-	if (!page) {
-		if (net_ratelimit())
-			netdev_err(netdev, "failed to allocate rx page\n");
-		return -ENOMEM;
-	}
-
-	map = dma_map_page(priv->dev, page, 0, RX_BUF_SIZE, DMA_FROM_DEVICE);
-	if (unlikely(dma_mapping_error(priv->dev, map))) {
-		if (net_ratelimit())
-			netdev_err(netdev, "failed to map rx page\n");
-		__free_page(page);
-		return -ENOMEM;
-	}
-
-	ftgmac100_rxdes_set_page(priv, rxdes, page);
-	ftgmac100_rxdes_set_dma_addr(rxdes, map);
-	ftgmac100_rxdes_set_dma_own(priv, rxdes);
-	return 0;
-}
-
 static void ftgmac100_free_buffers(struct ftgmac100 *priv)
 {
 	int i;
-- 
2.9.3

[PATCH 02/10] ftgmac100: Drop support for fragmented receive

[Benjamin Herrenschmidt]

We don't support jumbo frames, we will never receive a
fragmented packet, the RX buffer is always big enough,
if not then it's a runaway packet that can be dropped.

So take out the loop that handles such things in
ftgmac100_rx_packet() which will help with subsequent
simplifications and improvements to the RX path

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 37 ++++++++++++++++----------------
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index c77a65b..ee73a57 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -532,13 +532,19 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int *processed)
 	struct net_device *netdev = priv->netdev;
 	struct ftgmac100_rxdes *rxdes;
 	struct sk_buff *skb;
-	bool done = false;
+	struct page *page;
+	unsigned int size;
+	dma_addr_t map;
 
 	rxdes = ftgmac100_rx_locate_first_segment(priv);
 	if (!rxdes)
 		return false;
 
-	if (unlikely(ftgmac100_rx_packet_error(priv, rxdes))) {
+	/* We don't support segmented rx frames, so drop these
+	 * along with packets with errors.
+	 */
+	if (unlikely(!ftgmac100_rxdes_last_segment(rxdes) ||
+		     ftgmac100_rx_packet_error(priv, rxdes))) {
 		ftgmac100_rx_drop_packet(priv);
 		return true;
 	}
@@ -565,28 +571,21 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int *processed)
 	    (ftgmac100_rxdes_is_udp(rxdes) && !ftgmac100_rxdes_udpcs_err(rxdes)))
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
-	do {
-		dma_addr_t map = ftgmac100_rxdes_get_dma_addr(rxdes);
-		struct page *page = ftgmac100_rxdes_get_page(priv, rxdes);
-		unsigned int size;
+	map = ftgmac100_rxdes_get_dma_addr(rxdes);
 
-		dma_unmap_page(priv->dev, map, RX_BUF_SIZE, DMA_FROM_DEVICE);
-
-		size = ftgmac100_rxdes_data_length(rxdes);
-		skb_fill_page_desc(skb, skb_shinfo(skb)->nr_frags, page, 0, size);
+	dma_unmap_page(priv->dev, map, RX_BUF_SIZE, DMA_FROM_DEVICE);
 
-		skb->len += size;
-		skb->data_len += size;
-		skb->truesize += PAGE_SIZE;
+	size = ftgmac100_rxdes_data_length(rxdes);
+	skb_fill_page_desc(skb, skb_shinfo(skb)->nr_frags, page, 0, size);
 
-		if (ftgmac100_rxdes_last_segment(rxdes))
-			done = true;
+	skb->len += size;
+	skb->data_len += size;
+	skb->truesize += PAGE_SIZE;
 
-		ftgmac100_alloc_rx_page(priv, rxdes, GFP_ATOMIC);
+	ftgmac100_alloc_rx_page(priv, rxdes, GFP_ATOMIC);
 
-		ftgmac100_rx_pointer_advance(priv);
-		rxdes = ftgmac100_current_rxdes(priv);
-	} while (!done);
+	ftgmac100_rx_pointer_advance(priv);
+	rxdes = ftgmac100_current_rxdes(priv);
 
 	/* Small frames are copied into linear part of skb to free one page */
 	if (skb->len <= 128) {
-- 
2.9.3

[PATCH 03/10] ftgmac100: Use a scratch buffer for failed RX allocations

[Benjamin Herrenschmidt]

We can occasionally fail to allocate new RX buffers at
runtime or when starting the driver. At the moment the
latter just fails to open which is fine but the former
leaves stale DMA pointers in the ring.

Instead, use a scratch page and have all RX ring descriptors
point to it by default unless a proper buffer can be allocated.

It will help later on when re-initializing the whole ring
at runtime on link changes since there is no clean failure
path there unlike open().

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 42 +++++++++++++++++++++++++++++---
 1 file changed, 38 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index ee73a57..a03cc03 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -69,6 +69,10 @@ struct ftgmac100 {
 	u32 txdes0_edotr_mask;
 	spinlock_t tx_lock;
 
+	/* Scratch page to use when rx skb alloc fails */
+	void *rx_scratch;
+	dma_addr_t rx_scratch_dma;
+
 	/* Component structures */
 	struct net_device *netdev;
 	struct device *dev;
@@ -402,12 +406,14 @@ static int ftgmac100_alloc_rx_page(struct ftgmac100 *priv,
 	struct net_device *netdev = priv->netdev;
 	struct page *page;
 	dma_addr_t map;
+	int err;
 
 	page = alloc_page(gfp);
 	if (!page) {
 		if (net_ratelimit())
 			netdev_err(netdev, "failed to allocate rx page\n");
-		return -ENOMEM;
+		err = -ENOMEM;
+		map = priv->rx_scratch_dma;
 	}
 
 	map = dma_map_page(priv->dev, page, 0, RX_BUF_SIZE, DMA_FROM_DEVICE);
@@ -415,7 +421,9 @@ static int ftgmac100_alloc_rx_page(struct ftgmac100 *priv,
 		if (net_ratelimit())
 			netdev_err(netdev, "failed to map rx page\n");
 		__free_page(page);
-		return -ENOMEM;
+		err = -ENOMEM;
+		map = priv->rx_scratch_dma;
+		page = NULL;
 	}
 
 	ftgmac100_rxdes_set_page(priv, rxdes, page);
@@ -549,6 +557,16 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int *processed)
 		return true;
 	}
 
+	/* If the packet had no buffer (failed to allocate earlier)
+	 * then try to allocate one and skip
+	 */
+	page = ftgmac100_rxdes_get_page(priv, rxdes);
+	if (!page) {
+		ftgmac100_alloc_rx_page(priv, rxdes, GFP_ATOMIC);
+		ftgmac100_rx_pointer_advance(priv);
+		return true;
+	}
+
 	/* start processing */
 	skb = netdev_alloc_skb_ip_align(netdev, 128);
 	if (unlikely(!skb)) {
@@ -852,6 +870,11 @@ static void ftgmac100_free_rings(struct ftgmac100 *priv)
 	if (priv->descs)
 		dma_free_coherent(priv->dev, sizeof(struct ftgmac100_descs),
 				  priv->descs, priv->descs_dma_addr);
+
+	/* Free scratch packet buffer */
+	if (priv->rx_scratch)
+		dma_free_coherent(priv->dev, RX_BUF_SIZE,
+				  priv->rx_scratch, priv->rx_scratch_dma);
 }
 
 static int ftgmac100_alloc_rings(struct ftgmac100 *priv)
@@ -863,6 +886,14 @@ static int ftgmac100_alloc_rings(struct ftgmac100 *priv)
 	if (!priv->descs)
 		return -ENOMEM;
 
+	/* Allocate scratch packet buffer */
+	priv->rx_scratch = dma_alloc_coherent(priv->dev,
+					      RX_BUF_SIZE,
+					      &priv->rx_scratch_dma,
+					      GFP_KERNEL);
+	if (!priv->rx_scratch)
+		return -ENOMEM;
+
 	return 0;
 }
 
@@ -871,8 +902,11 @@ static void ftgmac100_init_rings(struct ftgmac100 *priv)
 	int i;
 
 	/* Initialize RX ring */
-	for (i = 0; i < RX_QUEUE_ENTRIES; i++)
-		priv->descs->rxdes[i].rxdes0 = 0;
+	for (i = 0; i < RX_QUEUE_ENTRIES; i++) {
+		struct ftgmac100_rxdes *rxdes = &priv->descs->rxdes[i];
+		ftgmac100_rxdes_set_dma_addr(rxdes, priv->rx_scratch_dma);
+		rxdes->rxdes0 = 0;
+	}
 	ftgmac100_rxdes_set_end_of_ring(priv, &priv->descs->rxdes[i - 1]);
 
 	/* Initialize TX ring */
-- 
2.9.3

[PATCH 04/10] ftgmac100: Cleanup rx checksum handling

[Benjamin Herrenschmidt]

Read the descriptor field only once and check for IP header
checksum errors as well

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 63 +++++++++++++-------------------
 1 file changed, 25 insertions(+), 38 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index a03cc03..60c2860 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -350,31 +350,11 @@ static dma_addr_t ftgmac100_rxdes_get_dma_addr(struct ftgmac100_rxdes *rxdes)
 	return le32_to_cpu(rxdes->rxdes3);
 }
 
-static bool ftgmac100_rxdes_is_tcp(struct ftgmac100_rxdes *rxdes)
+static inline bool ftgmac100_rxdes_csum_err(struct ftgmac100_rxdes *rxdes)
 {
-	return (rxdes->rxdes1 & cpu_to_le32(FTGMAC100_RXDES1_PROT_MASK)) ==
-	       cpu_to_le32(FTGMAC100_RXDES1_PROT_TCPIP);
-}
-
-static bool ftgmac100_rxdes_is_udp(struct ftgmac100_rxdes *rxdes)
-{
-	return (rxdes->rxdes1 & cpu_to_le32(FTGMAC100_RXDES1_PROT_MASK)) ==
-	       cpu_to_le32(FTGMAC100_RXDES1_PROT_UDPIP);
-}
-
-static bool ftgmac100_rxdes_tcpcs_err(struct ftgmac100_rxdes *rxdes)
-{
-	return rxdes->rxdes1 & cpu_to_le32(FTGMAC100_RXDES1_TCP_CHKSUM_ERR);
-}
-
-static bool ftgmac100_rxdes_udpcs_err(struct ftgmac100_rxdes *rxdes)
-{
-	return rxdes->rxdes1 & cpu_to_le32(FTGMAC100_RXDES1_UDP_CHKSUM_ERR);
-}
-
-static bool ftgmac100_rxdes_ipcs_err(struct ftgmac100_rxdes *rxdes)
-{
-	return rxdes->rxdes1 & cpu_to_le32(FTGMAC100_RXDES1_IP_CHKSUM_ERR);
+	return !!(rxdes->rxdes1 & cpu_to_le32(FTGMAC100_RXDES1_TCP_CHKSUM_ERR |
+					      FTGMAC100_RXDES1_UDP_CHKSUM_ERR |
+					      FTGMAC100_RXDES1_IP_CHKSUM_ERR));
 }
 
 static inline struct page **ftgmac100_rxdes_page_slot(struct ftgmac100 *priv,
@@ -484,11 +464,6 @@ static bool ftgmac100_rx_packet_error(struct ftgmac100 *priv,
 
 		netdev->stats.rx_crc_errors++;
 		error = true;
-	} else if (unlikely(ftgmac100_rxdes_ipcs_err(rxdes))) {
-		if (net_ratelimit())
-			netdev_info(netdev, "rx IP checksum err\n");
-
-		error = true;
 	}
 
 	if (unlikely(ftgmac100_rxdes_frame_too_long(rxdes))) {
@@ -580,14 +555,23 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int *processed)
 	if (unlikely(ftgmac100_rxdes_multicast(rxdes)))
 		netdev->stats.multicast++;
 
-	/*
-	 * It seems that HW does checksum incorrectly with fragmented packets,
-	 * so we are conservative here - if HW checksum error, let software do
-	 * the checksum again.
+	/* If the HW found checksum errors, bounce it to software.
+	 *
+	 * If we didn't, we need to see if the packet was recognized
+	 * by HW as one of the supported checksummed protocols before
+	 * we accept the HW test results.
 	 */
-	if ((ftgmac100_rxdes_is_tcp(rxdes) && !ftgmac100_rxdes_tcpcs_err(rxdes)) ||
-	    (ftgmac100_rxdes_is_udp(rxdes) && !ftgmac100_rxdes_udpcs_err(rxdes)))
-		skb->ip_summed = CHECKSUM_UNNECESSARY;
+	if (netdev->features & NETIF_F_RXCSUM) {
+		__le32 csum_vlan = rxdes->rxdes1;
+		__le32 err_bits = cpu_to_le32(FTGMAC100_RXDES1_TCP_CHKSUM_ERR |
+					      FTGMAC100_RXDES1_UDP_CHKSUM_ERR |
+					      FTGMAC100_RXDES1_IP_CHKSUM_ERR);
+		if ((csum_vlan & err_bits) ||
+		    !(csum_vlan & cpu_to_le32(FTGMAC100_RXDES1_PROT_MASK)))
+			skb->ip_summed = CHECKSUM_NONE;
+		else
+			skb->ip_summed = CHECKSUM_UNNECESSARY;
+	}
 
 	map = ftgmac100_rxdes_get_dma_addr(rxdes);
 
@@ -619,7 +603,10 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int *processed)
 	netdev->stats.rx_bytes += skb->len;
 
 	/* push packet to protocol stack */
-	napi_gro_receive(&priv->napi, skb);
+	if (skb->ip_summed == CHECKSUM_NONE)
+		netif_receive_skb(skb);
+	else
+		napi_gro_receive(&priv->napi, skb);
 
 	(*processed)++;
 	return true;
@@ -1573,7 +1560,7 @@ static int ftgmac100_probe(struct platform_device *pdev)
 	 * when NCSI is enabled on the interface. It doesn't work
 	 * in that case.
 	 */
-	netdev->features = NETIF_F_IP_CSUM | NETIF_F_GRO;
+	netdev->features = NETIF_F_RXCSUM | NETIF_F_IP_CSUM | NETIF_F_GRO;
 	if (priv->use_ncsi &&
 	    of_get_property(pdev->dev.of_node, "no-hw-checksum", NULL))
 		netdev->features &= ~NETIF_F_IP_CSUM;
-- 
2.9.3

[PATCH 05/10] ftgmac100: Simplify rx packets error handling

[Benjamin Herrenschmidt]

The fast path has a single unlikely() test for any error bit,
calling into a helper that sets the appropriate statistics.

The various netdev_info aren't particularly interesting. If
we want to differentiate the various length errors later we
can introduce driver specific stats using ethtool.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 61 +++++++++++++-------------------
 1 file changed, 25 insertions(+), 36 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 60c2860..0bfcfdd 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -298,6 +298,18 @@ static void ftgmac100_rxdes_set_dma_own(const struct ftgmac100 *priv,
 	rxdes->rxdes0 &= cpu_to_le32(priv->rxdes0_edorr_mask);
 }
 
+#define RXDES0_ANY_ERROR		( \
+	FTGMAC100_RXDES0_RX_ERR		| \
+	FTGMAC100_RXDES0_CRC_ERR	| \
+	FTGMAC100_RXDES0_FTL		| \
+	FTGMAC100_RXDES0_RUNT		| \
+	FTGMAC100_RXDES0_RX_ODD_NB)
+
+static inline bool ftgmac100_rxdes_any_error(struct ftgmac100_rxdes *rxdes)
+{
+	return rxdes->rxdes0 & cpu_to_le32(RXDES0_ANY_ERROR);
+}
+
 static bool ftgmac100_rxdes_rx_error(struct ftgmac100_rxdes *rxdes)
 {
 	return rxdes->rxdes0 & cpu_to_le32(FTGMAC100_RXDES0_RX_ERR);
@@ -444,49 +456,22 @@ ftgmac100_rx_locate_first_segment(struct ftgmac100 *priv)
 	return NULL;
 }
 
-static bool ftgmac100_rx_packet_error(struct ftgmac100 *priv,
+
+static void ftgmac100_rx_packet_error(struct ftgmac100 *priv,
 				      struct ftgmac100_rxdes *rxdes)
 {
 	struct net_device *netdev = priv->netdev;
-	bool error = false;
-
-	if (unlikely(ftgmac100_rxdes_rx_error(rxdes))) {
-		if (net_ratelimit())
-			netdev_info(netdev, "rx err\n");
 
+	if (ftgmac100_rxdes_rx_error(rxdes))
 		netdev->stats.rx_errors++;
-		error = true;
-	}
-
-	if (unlikely(ftgmac100_rxdes_crc_error(rxdes))) {
-		if (net_ratelimit())
-			netdev_info(netdev, "rx crc err\n");
 
+	if (ftgmac100_rxdes_crc_error(rxdes))
 		netdev->stats.rx_crc_errors++;
-		error = true;
-	}
-
-	if (unlikely(ftgmac100_rxdes_frame_too_long(rxdes))) {
-		if (net_ratelimit())
-			netdev_info(netdev, "rx frame too long\n");
-
-		netdev->stats.rx_length_errors++;
-		error = true;
-	} else if (unlikely(ftgmac100_rxdes_runt(rxdes))) {
-		if (net_ratelimit())
-			netdev_info(netdev, "rx runt\n");
-
-		netdev->stats.rx_length_errors++;
-		error = true;
-	} else if (unlikely(ftgmac100_rxdes_odd_nibble(rxdes))) {
-		if (net_ratelimit())
-			netdev_info(netdev, "rx odd nibble\n");
 
+	if (ftgmac100_rxdes_frame_too_long(rxdes) ||
+	    ftgmac100_rxdes_runt(rxdes) ||
+	    ftgmac100_rxdes_odd_nibble(rxdes))
 		netdev->stats.rx_length_errors++;
-		error = true;
-	}
-
-	return error;
 }
 
 static void ftgmac100_rx_drop_packet(struct ftgmac100 *priv)
@@ -526,8 +511,12 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int *processed)
 	/* We don't support segmented rx frames, so drop these
 	 * along with packets with errors.
 	 */
-	if (unlikely(!ftgmac100_rxdes_last_segment(rxdes) ||
-		     ftgmac100_rx_packet_error(priv, rxdes))) {
+	if (unlikely(!ftgmac100_rxdes_last_segment(rxdes))) {
+		ftgmac100_rx_drop_packet(priv);
+		return true;
+	}
+	if (unlikely(ftgmac100_rxdes_any_error(rxdes))) {
+		ftgmac100_rx_packet_error(priv, rxdes);
 		ftgmac100_rx_drop_packet(priv);
 		return true;
 	}
-- 
2.9.3

[PATCH 06/10] ftgmac100: Simplify rx pointer handling in the rx path

[Benjamin Herrenschmidt]

We don't handle fragmented RX packets, so the "looping"
helpers to locate the first segment of a packet or to
drop a packet aren't actually helping.

Take them out and simplify ftgmac100_rx_packet() further
as a result.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 92 +++++++++-----------------------
 1 file changed, 24 insertions(+), 68 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 0bfcfdd..95d46db 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -429,34 +429,6 @@ static int ftgmac100_next_rx_pointer(int pointer)
 	return (pointer + 1) & (RX_QUEUE_ENTRIES - 1);
 }
 
-static void ftgmac100_rx_pointer_advance(struct ftgmac100 *priv)
-{
-	priv->rx_pointer = ftgmac100_next_rx_pointer(priv->rx_pointer);
-}
-
-static struct ftgmac100_rxdes *ftgmac100_current_rxdes(struct ftgmac100 *priv)
-{
-	return &priv->descs->rxdes[priv->rx_pointer];
-}
-
-static struct ftgmac100_rxdes *
-ftgmac100_rx_locate_first_segment(struct ftgmac100 *priv)
-{
-	struct ftgmac100_rxdes *rxdes = ftgmac100_current_rxdes(priv);
-
-	while (ftgmac100_rxdes_packet_ready(rxdes)) {
-		if (ftgmac100_rxdes_first_segment(rxdes))
-			return rxdes;
-
-		ftgmac100_rxdes_set_dma_own(priv, rxdes);
-		ftgmac100_rx_pointer_advance(priv);
-		rxdes = ftgmac100_current_rxdes(priv);
-	}
-
-	return NULL;
-}
-
-
 static void ftgmac100_rx_packet_error(struct ftgmac100 *priv,
 				      struct ftgmac100_rxdes *rxdes)
 {
@@ -474,51 +446,32 @@ static void ftgmac100_rx_packet_error(struct ftgmac100 *priv,
 		netdev->stats.rx_length_errors++;
 }
 
-static void ftgmac100_rx_drop_packet(struct ftgmac100 *priv)
-{
-	struct net_device *netdev = priv->netdev;
-	struct ftgmac100_rxdes *rxdes = ftgmac100_current_rxdes(priv);
-	bool done = false;
-
-	if (net_ratelimit())
-		netdev_dbg(netdev, "drop packet %p\n", rxdes);
-
-	do {
-		if (ftgmac100_rxdes_last_segment(rxdes))
-			done = true;
-
-		ftgmac100_rxdes_set_dma_own(priv, rxdes);
-		ftgmac100_rx_pointer_advance(priv);
-		rxdes = ftgmac100_current_rxdes(priv);
-	} while (!done && ftgmac100_rxdes_packet_ready(rxdes));
-
-	netdev->stats.rx_dropped++;
-}
-
 static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int *processed)
 {
 	struct net_device *netdev = priv->netdev;
 	struct ftgmac100_rxdes *rxdes;
 	struct sk_buff *skb;
 	struct page *page;
-	unsigned int size;
+	unsigned int pointer, size;
 	dma_addr_t map;
 
-	rxdes = ftgmac100_rx_locate_first_segment(priv);
-	if (!rxdes)
+	/* Grab next RX descriptor */
+	pointer = priv->rx_pointer;
+	rxdes = &priv->descs->rxdes[pointer];
+
+	/* Do we have a packet ? */
+	if (!ftgmac100_rxdes_packet_ready(rxdes))
 		return false;
 
-	/* We don't support segmented rx frames, so drop these
-	 * along with packets with errors.
-	 */
-	if (unlikely(!ftgmac100_rxdes_last_segment(rxdes))) {
-		ftgmac100_rx_drop_packet(priv);
-		return true;
-	}
+	/* We don't cope with fragmented RX packets */
+	if (unlikely(!ftgmac100_rxdes_first_segment(rxdes) ||
+		     !ftgmac100_rxdes_last_segment(rxdes)))
+		goto drop;
+
+	/* Any error (other than csum offload) flagged ? */
 	if (unlikely(ftgmac100_rxdes_any_error(rxdes))) {
 		ftgmac100_rx_packet_error(priv, rxdes);
-		ftgmac100_rx_drop_packet(priv);
-		return true;
+		goto drop;
 	}
 
 	/* If the packet had no buffer (failed to allocate earlier)
@@ -527,8 +480,7 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int *processed)
 	page = ftgmac100_rxdes_get_page(priv, rxdes);
 	if (!page) {
 		ftgmac100_alloc_rx_page(priv, rxdes, GFP_ATOMIC);
-		ftgmac100_rx_pointer_advance(priv);
-		return true;
+		goto drop;
 	}
 
 	/* start processing */
@@ -536,9 +488,7 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int *processed)
 	if (unlikely(!skb)) {
 		if (net_ratelimit())
 			netdev_err(netdev, "rx skb alloc failed\n");
-
-		ftgmac100_rx_drop_packet(priv);
-		return true;
+		goto drop;
 	}
 
 	if (unlikely(ftgmac100_rxdes_multicast(rxdes)))
@@ -575,8 +525,7 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int *processed)
 
 	ftgmac100_alloc_rx_page(priv, rxdes, GFP_ATOMIC);
 
-	ftgmac100_rx_pointer_advance(priv);
-	rxdes = ftgmac100_current_rxdes(priv);
+	priv->rx_pointer = ftgmac100_next_rx_pointer(pointer);
 
 	/* Small frames are copied into linear part of skb to free one page */
 	if (skb->len <= 128) {
@@ -599,6 +548,13 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int *processed)
 
 	(*processed)++;
 	return true;
+
+ drop:
+	/* Clean rxdes0 (which resets own bit) */
+	rxdes->rxdes0 &= cpu_to_le32(priv->rxdes0_edorr_mask);
+	priv->rx_pointer = ftgmac100_next_rx_pointer(pointer);
+	netdev->stats.rx_dropped++;
+	return true;
 }
 
 static void ftgmac100_txdes_reset(const struct ftgmac100 *priv,
-- 
2.9.3

[PATCH 07/10] ftgmac100: Directly receive into sk_buffs

[Benjamin Herrenschmidt]

The current driver receive path allocates pages and stashes
them into SKB fragments. This is not particularly useful as
we don't support jumbo frames (which wouldn't be great with
the small FIFOs on all the known implementations) anyway.

It also makes us flush the caches and allocate more memory
for RX than necessary.

So set our RX buf to our max packet size instead (which we
bump to 1536 bytes to account for packets with vlan tags
etc...) like most other ethernet drivers.

Then allocate skbs when populating the receive ring and DMA
directly into them.

This simplifies the RX path further.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 142 ++++++++++++-------------------
 1 file changed, 56 insertions(+), 86 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 95d46db..86bda0a 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -41,8 +41,8 @@
 #define RX_QUEUE_ENTRIES	256	/* must be power of 2 */
 #define TX_QUEUE_ENTRIES	512	/* must be power of 2 */
 
-#define MAX_PKT_SIZE		1518
-#define RX_BUF_SIZE		PAGE_SIZE	/* must be smaller than 0x3fff */
+#define MAX_PKT_SIZE		1536
+#define RX_BUF_SIZE		MAX_PKT_SIZE	/* must be smaller than 0x3fff */
 
 struct ftgmac100_descs {
 	struct ftgmac100_rxdes rxdes[RX_QUEUE_ENTRIES];
@@ -58,7 +58,7 @@ struct ftgmac100 {
 	dma_addr_t descs_dma_addr;
 
 	/* Rx ring */
-	struct page *rx_pages[RX_QUEUE_ENTRIES];
+	struct sk_buff *rx_skbs[RX_QUEUE_ENTRIES];
 	unsigned int rx_pointer;
 	u32 rxdes0_edorr_mask;
 
@@ -291,13 +291,6 @@ static bool ftgmac100_rxdes_packet_ready(struct ftgmac100_rxdes *rxdes)
 	return rxdes->rxdes0 & cpu_to_le32(FTGMAC100_RXDES0_RXPKT_RDY);
 }
 
-static void ftgmac100_rxdes_set_dma_own(const struct ftgmac100 *priv,
-					struct ftgmac100_rxdes *rxdes)
-{
-	/* clear status bits */
-	rxdes->rxdes0 &= cpu_to_le32(priv->rxdes0_edorr_mask);
-}
-
 #define RXDES0_ANY_ERROR		( \
 	FTGMAC100_RXDES0_RX_ERR		| \
 	FTGMAC100_RXDES0_CRC_ERR	| \
@@ -369,58 +362,45 @@ static inline bool ftgmac100_rxdes_csum_err(struct ftgmac100_rxdes *rxdes)
 					      FTGMAC100_RXDES1_IP_CHKSUM_ERR));
 }
 
-static inline struct page **ftgmac100_rxdes_page_slot(struct ftgmac100 *priv,
-						      struct ftgmac100_rxdes *rxdes)
-{
-	return &priv->rx_pages[rxdes - priv->descs->rxdes];
-}
-
-/*
- * rxdes2 is not used by hardware. We use it to keep track of page.
- * Since hardware does not touch it, we can skip cpu_to_le32()/le32_to_cpu().
- */
-static void ftgmac100_rxdes_set_page(struct ftgmac100 *priv,
-				     struct ftgmac100_rxdes *rxdes,
-				     struct page *page)
-{
-	*ftgmac100_rxdes_page_slot(priv, rxdes) = page;
-}
-
-static struct page *ftgmac100_rxdes_get_page(struct ftgmac100 *priv,
-					     struct ftgmac100_rxdes *rxdes)
-{
-	return *ftgmac100_rxdes_page_slot(priv, rxdes);
-}
-
-static int ftgmac100_alloc_rx_page(struct ftgmac100 *priv,
-				   struct ftgmac100_rxdes *rxdes, gfp_t gfp)
+static int ftgmac100_alloc_rx_buf(struct ftgmac100 *priv, unsigned int entry,
+				  struct ftgmac100_rxdes *rxdes, gfp_t gfp)
 {
 	struct net_device *netdev = priv->netdev;
-	struct page *page;
+	struct sk_buff *skb;
 	dma_addr_t map;
 	int err;
 
-	page = alloc_page(gfp);
-	if (!page) {
+	skb = netdev_alloc_skb_ip_align(netdev, RX_BUF_SIZE);
+	if (unlikely(!skb)) {
 		if (net_ratelimit())
-			netdev_err(netdev, "failed to allocate rx page\n");
+			netdev_warn(netdev, "failed to allocate rx skb\n");
 		err = -ENOMEM;
 		map = priv->rx_scratch_dma;
+	} else {
+		map = dma_map_single(priv->dev, skb->data, RX_BUF_SIZE,
+				     DMA_FROM_DEVICE);
+		if (unlikely(dma_mapping_error(priv->dev, map))) {
+			if (net_ratelimit())
+				netdev_err(netdev, "failed to map rx page\n");
+			dev_kfree_skb_any(skb);
+			map = priv->rx_scratch_dma;
+			skb = NULL;
+			err = -ENOMEM;
+		}
 	}
 
-	map = dma_map_page(priv->dev, page, 0, RX_BUF_SIZE, DMA_FROM_DEVICE);
-	if (unlikely(dma_mapping_error(priv->dev, map))) {
-		if (net_ratelimit())
-			netdev_err(netdev, "failed to map rx page\n");
-		__free_page(page);
-		err = -ENOMEM;
-		map = priv->rx_scratch_dma;
-		page = NULL;
-	}
+	/* Store skb */
+	priv->rx_skbs[entry] = skb;
 
-	ftgmac100_rxdes_set_page(priv, rxdes, page);
+	/* Store DMA address into RX desc */
 	ftgmac100_rxdes_set_dma_addr(rxdes, map);
-	ftgmac100_rxdes_set_dma_own(priv, rxdes);
+
+	/* Ensure the above is ordered vs clearing the OWN bit */
+	dma_wmb();
+
+	/* Clean rxdes0 (which resets own bit) */
+	rxdes->rxdes0 &= cpu_to_le32(priv->rxdes0_edorr_mask);
+
 	return 0;
 }
 
@@ -451,7 +431,6 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int *processed)
 	struct net_device *netdev = priv->netdev;
 	struct ftgmac100_rxdes *rxdes;
 	struct sk_buff *skb;
-	struct page *page;
 	unsigned int pointer, size;
 	dma_addr_t map;
 
@@ -474,20 +453,12 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int *processed)
 		goto drop;
 	}
 
-	/* If the packet had no buffer (failed to allocate earlier)
+	/* If the packet had no skb (failed to allocate earlier)
 	 * then try to allocate one and skip
 	 */
-	page = ftgmac100_rxdes_get_page(priv, rxdes);
-	if (!page) {
-		ftgmac100_alloc_rx_page(priv, rxdes, GFP_ATOMIC);
-		goto drop;
-	}
-
-	/* start processing */
-	skb = netdev_alloc_skb_ip_align(netdev, 128);
-	if (unlikely(!skb)) {
-		if (net_ratelimit())
-			netdev_err(netdev, "rx skb alloc failed\n");
+	skb = priv->rx_skbs[pointer];
+	if (!unlikely(skb)) {
+		ftgmac100_alloc_rx_buf(priv, pointer, rxdes, GFP_ATOMIC);
 		goto drop;
 	}
 
@@ -512,33 +483,31 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int *processed)
 			skb->ip_summed = CHECKSUM_UNNECESSARY;
 	}
 
-	map = ftgmac100_rxdes_get_dma_addr(rxdes);
-
-	dma_unmap_page(priv->dev, map, RX_BUF_SIZE, DMA_FROM_DEVICE);
-
+	/* Grab received size annd transfer to skb */
 	size = ftgmac100_rxdes_data_length(rxdes);
-	skb_fill_page_desc(skb, skb_shinfo(skb)->nr_frags, page, 0, size);
+	skb_put(skb, size);
 
-	skb->len += size;
-	skb->data_len += size;
-	skb->truesize += PAGE_SIZE;
+	/* Tear down DMA mapping, do necessary cache management */
+	map = ftgmac100_rxdes_get_dma_addr(rxdes);
+#if defined(CONFIG_ARM) && !defined(CONFIG_ARM_DMA_USE_IOMMU)
+	/* When we don't have an iommu, we can save cycles by not
+	 * invalidating the cache for the part of the packet that
+	 * wasn't received.
+	 */
+	dma_unmap_single(priv->dev, map, size, DMA_FROM_DEVICE);
+#else
+	dma_unmap_single(priv->dev, map, RX_BUF_SIZE, DMA_FROM_DEVICE);
+#endif
 
-	ftgmac100_alloc_rx_page(priv, rxdes, GFP_ATOMIC);
 
+	/* Resplenish rx ring */
+	ftgmac100_alloc_rx_buf(priv, pointer, rxdes, GFP_ATOMIC);
 	priv->rx_pointer = ftgmac100_next_rx_pointer(pointer);
 
-	/* Small frames are copied into linear part of skb to free one page */
-	if (skb->len <= 128) {
-		skb->truesize -= PAGE_SIZE;
-		__pskb_pull_tail(skb, skb->len);
-	} else {
-		/* We pull the minimum amount into linear part */
-		__pskb_pull_tail(skb, ETH_HLEN);
-	}
 	skb->protocol = eth_type_trans(skb, netdev);
 
 	netdev->stats.rx_packets++;
-	netdev->stats.rx_bytes += skb->len;
+	netdev->stats.rx_bytes += size;
 
 	/* push packet to protocol stack */
 	if (skb->ip_summed == CHECKSUM_NONE)
@@ -772,14 +741,15 @@ static void ftgmac100_free_buffers(struct ftgmac100 *priv)
 	/* Free all RX buffers */
 	for (i = 0; i < RX_QUEUE_ENTRIES; i++) {
 		struct ftgmac100_rxdes *rxdes = &priv->descs->rxdes[i];
-		struct page *page = ftgmac100_rxdes_get_page(priv, rxdes);
+		struct sk_buff *skb = priv->rx_skbs[i];
 		dma_addr_t map = ftgmac100_rxdes_get_dma_addr(rxdes);
 
-		if (!page)
+		if (!skb)
 			continue;
 
-		dma_unmap_page(priv->dev, map, RX_BUF_SIZE, DMA_FROM_DEVICE);
-		__free_page(page);
+		priv->rx_skbs[i] = NULL;
+		dma_unmap_single(priv->dev, map, RX_BUF_SIZE, DMA_FROM_DEVICE);
+		dev_kfree_skb_any(skb);
 	}
 
 	/* Free all TX buffers */
@@ -854,7 +824,7 @@ static int ftgmac100_alloc_rx_buffers(struct ftgmac100 *priv)
 	for (i = 0; i < RX_QUEUE_ENTRIES; i++) {
 		struct ftgmac100_rxdes *rxdes = &priv->descs->rxdes[i];
 
-		if (ftgmac100_alloc_rx_page(priv, rxdes, GFP_KERNEL))
+		if (ftgmac100_alloc_rx_buf(priv, i, rxdes, GFP_KERNEL))
 			return -ENOMEM;
 	}
 	return 0;
-- 
2.9.3

[PATCH 08/10] ftgmac100: Add missing barrier in ftgmac100_rx_packet()

[Benjamin Herrenschmidt]

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 86bda0a..594af30 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -442,6 +442,9 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int *processed)
 	if (!ftgmac100_rxdes_packet_ready(rxdes))
 		return false;
 
+	/* Order subsequent reads with the test for the ready bit */
+	dma_rmb();
+
 	/* We don't cope with fragmented RX packets */
 	if (unlikely(!ftgmac100_rxdes_first_segment(rxdes) ||
 		     !ftgmac100_rxdes_last_segment(rxdes)))
-- 
2.9.3

[PATCH 09/10] ftgmac100: Remove rx descriptor accessors

[Benjamin Herrenschmidt]

Directly access the fields when needed. The accessors add clutter
not clarity and in some cases cause unnecessary read-modify-write
type access on the slow (uncached) descriptor memory.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 152 +++++++++----------------------
 drivers/net/ethernet/faraday/ftgmac100.h |  16 +++-
 2 files changed, 53 insertions(+), 115 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 594af30..e42ef8f 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -276,92 +276,6 @@ static void ftgmac100_stop_hw(struct ftgmac100 *priv)
 	iowrite32(0, priv->base + FTGMAC100_OFFSET_MACCR);
 }
 
-static bool ftgmac100_rxdes_first_segment(struct ftgmac100_rxdes *rxdes)
-{
-	return rxdes->rxdes0 & cpu_to_le32(FTGMAC100_RXDES0_FRS);
-}
-
-static bool ftgmac100_rxdes_last_segment(struct ftgmac100_rxdes *rxdes)
-{
-	return rxdes->rxdes0 & cpu_to_le32(FTGMAC100_RXDES0_LRS);
-}
-
-static bool ftgmac100_rxdes_packet_ready(struct ftgmac100_rxdes *rxdes)
-{
-	return rxdes->rxdes0 & cpu_to_le32(FTGMAC100_RXDES0_RXPKT_RDY);
-}
-
-#define RXDES0_ANY_ERROR		( \
-	FTGMAC100_RXDES0_RX_ERR		| \
-	FTGMAC100_RXDES0_CRC_ERR	| \
-	FTGMAC100_RXDES0_FTL		| \
-	FTGMAC100_RXDES0_RUNT		| \
-	FTGMAC100_RXDES0_RX_ODD_NB)
-
-static inline bool ftgmac100_rxdes_any_error(struct ftgmac100_rxdes *rxdes)
-{
-	return rxdes->rxdes0 & cpu_to_le32(RXDES0_ANY_ERROR);
-}
-
-static bool ftgmac100_rxdes_rx_error(struct ftgmac100_rxdes *rxdes)
-{
-	return rxdes->rxdes0 & cpu_to_le32(FTGMAC100_RXDES0_RX_ERR);
-}
-
-static bool ftgmac100_rxdes_crc_error(struct ftgmac100_rxdes *rxdes)
-{
-	return rxdes->rxdes0 & cpu_to_le32(FTGMAC100_RXDES0_CRC_ERR);
-}
-
-static bool ftgmac100_rxdes_frame_too_long(struct ftgmac100_rxdes *rxdes)
-{
-	return rxdes->rxdes0 & cpu_to_le32(FTGMAC100_RXDES0_FTL);
-}
-
-static bool ftgmac100_rxdes_runt(struct ftgmac100_rxdes *rxdes)
-{
-	return rxdes->rxdes0 & cpu_to_le32(FTGMAC100_RXDES0_RUNT);
-}
-
-static bool ftgmac100_rxdes_odd_nibble(struct ftgmac100_rxdes *rxdes)
-{
-	return rxdes->rxdes0 & cpu_to_le32(FTGMAC100_RXDES0_RX_ODD_NB);
-}
-
-static unsigned int ftgmac100_rxdes_data_length(struct ftgmac100_rxdes *rxdes)
-{
-	return le32_to_cpu(rxdes->rxdes0) & FTGMAC100_RXDES0_VDBC;
-}
-
-static bool ftgmac100_rxdes_multicast(struct ftgmac100_rxdes *rxdes)
-{
-	return rxdes->rxdes0 & cpu_to_le32(FTGMAC100_RXDES0_MULTICAST);
-}
-
-static void ftgmac100_rxdes_set_end_of_ring(const struct ftgmac100 *priv,
-					    struct ftgmac100_rxdes *rxdes)
-{
-	rxdes->rxdes0 |= cpu_to_le32(priv->rxdes0_edorr_mask);
-}
-
-static void ftgmac100_rxdes_set_dma_addr(struct ftgmac100_rxdes *rxdes,
-					 dma_addr_t addr)
-{
-	rxdes->rxdes3 = cpu_to_le32(addr);
-}
-
-static dma_addr_t ftgmac100_rxdes_get_dma_addr(struct ftgmac100_rxdes *rxdes)
-{
-	return le32_to_cpu(rxdes->rxdes3);
-}
-
-static inline bool ftgmac100_rxdes_csum_err(struct ftgmac100_rxdes *rxdes)
-{
-	return !!(rxdes->rxdes1 & cpu_to_le32(FTGMAC100_RXDES1_TCP_CHKSUM_ERR |
-					      FTGMAC100_RXDES1_UDP_CHKSUM_ERR |
-					      FTGMAC100_RXDES1_IP_CHKSUM_ERR));
-}
-
 static int ftgmac100_alloc_rx_buf(struct ftgmac100 *priv, unsigned int entry,
 				  struct ftgmac100_rxdes *rxdes, gfp_t gfp)
 {
@@ -393,13 +307,16 @@ static int ftgmac100_alloc_rx_buf(struct ftgmac100 *priv, unsigned int entry,
 	priv->rx_skbs[entry] = skb;
 
 	/* Store DMA address into RX desc */
-	ftgmac100_rxdes_set_dma_addr(rxdes, map);
+	rxdes->rxdes3 = cpu_to_le32(map);
 
 	/* Ensure the above is ordered vs clearing the OWN bit */
 	dma_wmb();
 
-	/* Clean rxdes0 (which resets own bit) */
-	rxdes->rxdes0 &= cpu_to_le32(priv->rxdes0_edorr_mask);
+	/* Clean status (which resets own bit) */
+	if (entry == (RX_QUEUE_ENTRIES - 1))
+		rxdes->rxdes0 = cpu_to_le32(priv->rxdes0_edorr_mask);
+	else
+		rxdes->rxdes0 = 0;
 
 	return 0;
 }
@@ -409,20 +326,19 @@ static int ftgmac100_next_rx_pointer(int pointer)
 	return (pointer + 1) & (RX_QUEUE_ENTRIES - 1);
 }
 
-static void ftgmac100_rx_packet_error(struct ftgmac100 *priv,
-				      struct ftgmac100_rxdes *rxdes)
+static void ftgmac100_rx_packet_error(struct ftgmac100 *priv, u32 status)
 {
 	struct net_device *netdev = priv->netdev;
 
-	if (ftgmac100_rxdes_rx_error(rxdes))
+	if (status & FTGMAC100_RXDES0_RX_ERR)
 		netdev->stats.rx_errors++;
 
-	if (ftgmac100_rxdes_crc_error(rxdes))
+	if (status & FTGMAC100_RXDES0_CRC_ERR)
 		netdev->stats.rx_crc_errors++;
 
-	if (ftgmac100_rxdes_frame_too_long(rxdes) ||
-	    ftgmac100_rxdes_runt(rxdes) ||
-	    ftgmac100_rxdes_odd_nibble(rxdes))
+	if (status & (FTGMAC100_RXDES0_FTL |
+		      FTGMAC100_RXDES0_RUNT |
+		      FTGMAC100_RXDES0_RX_ODD_NB))
 		netdev->stats.rx_length_errors++;
 }
 
@@ -432,27 +348,31 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int *processed)
 	struct ftgmac100_rxdes *rxdes;
 	struct sk_buff *skb;
 	unsigned int pointer, size;
+	u32 status;
 	dma_addr_t map;
 
 	/* Grab next RX descriptor */
 	pointer = priv->rx_pointer;
 	rxdes = &priv->descs->rxdes[pointer];
 
+	/* Grab descriptor status */
+	status = le32_to_cpu(rxdes->rxdes0);
+
 	/* Do we have a packet ? */
-	if (!ftgmac100_rxdes_packet_ready(rxdes))
+	if (!(status & FTGMAC100_RXDES0_RXPKT_RDY))
 		return false;
 
 	/* Order subsequent reads with the test for the ready bit */
 	dma_rmb();
 
 	/* We don't cope with fragmented RX packets */
-	if (unlikely(!ftgmac100_rxdes_first_segment(rxdes) ||
-		     !ftgmac100_rxdes_last_segment(rxdes)))
+	if (unlikely(!(status & FTGMAC100_RXDES0_FRS) ||
+		     !(status & FTGMAC100_RXDES0_LRS)))
 		goto drop;
 
 	/* Any error (other than csum offload) flagged ? */
-	if (unlikely(ftgmac100_rxdes_any_error(rxdes))) {
-		ftgmac100_rx_packet_error(priv, rxdes);
+	if (unlikely(status & RXDES0_ANY_ERROR)) {
+		ftgmac100_rx_packet_error(priv, status);
 		goto drop;
 	}
 
@@ -465,7 +385,7 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int *processed)
 		goto drop;
 	}
 
-	if (unlikely(ftgmac100_rxdes_multicast(rxdes)))
+	if (unlikely(status & FTGMAC100_RXDES0_MULTICAST))
 		netdev->stats.multicast++;
 
 	/* If the HW found checksum errors, bounce it to software.
@@ -487,11 +407,12 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int *processed)
 	}
 
 	/* Grab received size annd transfer to skb */
-	size = ftgmac100_rxdes_data_length(rxdes);
+	size = status & FTGMAC100_RXDES0_VDBC;
 	skb_put(skb, size);
 
 	/* Tear down DMA mapping, do necessary cache management */
-	map = ftgmac100_rxdes_get_dma_addr(rxdes);
+	map = le32_to_cpu(rxdes->rxdes3);
+
 #if defined(CONFIG_ARM) && !defined(CONFIG_ARM_DMA_USE_IOMMU)
 	/* When we don't have an iommu, we can save cycles by not
 	 * invalidating the cache for the part of the packet that
@@ -523,7 +444,7 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int *processed)
 
  drop:
 	/* Clean rxdes0 (which resets own bit) */
-	rxdes->rxdes0 &= cpu_to_le32(priv->rxdes0_edorr_mask);
+	rxdes->rxdes0 = cpu_to_le32(status & priv->rxdes0_edorr_mask);
 	priv->rx_pointer = ftgmac100_next_rx_pointer(pointer);
 	netdev->stats.rx_dropped++;
 	return true;
@@ -745,7 +666,7 @@ static void ftgmac100_free_buffers(struct ftgmac100 *priv)
 	for (i = 0; i < RX_QUEUE_ENTRIES; i++) {
 		struct ftgmac100_rxdes *rxdes = &priv->descs->rxdes[i];
 		struct sk_buff *skb = priv->rx_skbs[i];
-		dma_addr_t map = ftgmac100_rxdes_get_dma_addr(rxdes);
+		dma_addr_t map = le32_to_cpu(rxdes->rxdes3);
 
 		if (!skb)
 			continue;
@@ -804,15 +725,17 @@ static int ftgmac100_alloc_rings(struct ftgmac100 *priv)
 
 static void ftgmac100_init_rings(struct ftgmac100 *priv)
 {
+	struct ftgmac100_rxdes *rxdes;
 	int i;
 
 	/* Initialize RX ring */
 	for (i = 0; i < RX_QUEUE_ENTRIES; i++) {
-		struct ftgmac100_rxdes *rxdes = &priv->descs->rxdes[i];
-		ftgmac100_rxdes_set_dma_addr(rxdes, priv->rx_scratch_dma);
+		rxdes = &priv->descs->rxdes[i];
 		rxdes->rxdes0 = 0;
+		rxdes->rxdes3 = cpu_to_le32(priv->rx_scratch_dma);
 	}
-	ftgmac100_rxdes_set_end_of_ring(priv, &priv->descs->rxdes[i - 1]);
+	/* Mark the end of the ring */
+	rxdes->rxdes0 |= cpu_to_le32(priv->rxdes0_edorr_mask);
 
 	/* Initialize TX ring */
 	for (i = 0; i < TX_QUEUE_ENTRIES; i++)
@@ -1028,6 +951,14 @@ static irqreturn_t ftgmac100_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static bool ftgmac100_check_rx(struct ftgmac100 *priv)
+{
+	struct ftgmac100_rxdes *rxdes = &priv->descs->rxdes[priv->rx_pointer];
+
+	/* Do we have a packet ? */
+	return !!(rxdes->rxdes0 & cpu_to_le32(FTGMAC100_RXDES0_RXPKT_RDY));
+}
+
 static int ftgmac100_poll(struct napi_struct *napi, int budget)
 {
 	struct ftgmac100 *priv = container_of(napi, struct ftgmac100, napi);
@@ -1067,8 +998,7 @@ static int ftgmac100_poll(struct napi_struct *napi, int budget)
 		 */
 		iowrite32(FTGMAC100_INT_RXTX,
 			  priv->base + FTGMAC100_OFFSET_ISR);
-		if (ftgmac100_rxdes_packet_ready
-		    (ftgmac100_current_rxdes(priv)) || priv->tx_pending)
+		if (ftgmac100_check_rx(priv) || priv->tx_pending)
 			return budget;
 
 		/* deschedule NAPI */
diff --git a/drivers/net/ethernet/faraday/ftgmac100.h b/drivers/net/ethernet/faraday/ftgmac100.h
index c4d5cc1..9124785 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.h
+++ b/drivers/net/ethernet/faraday/ftgmac100.h
@@ -227,10 +227,10 @@ struct ftgmac100_txdes {
  * Receive descriptor, aligned to 16 bytes
  */
 struct ftgmac100_rxdes {
-	unsigned int	rxdes0;
-	unsigned int	rxdes1;
-	unsigned int	rxdes2;	/* not used by HW */
-	unsigned int	rxdes3;	/* RXBUF_BADR */
+	__le32	rxdes0; /* Control & status bits */
+	__le32	rxdes1;	/* Checksum and vlan status */
+	__le32	rxdes2; /* length/type on AST2500 */
+	__le32	rxdes3;	/* DMA buffer address */
 } __attribute__ ((aligned(16)));
 
 #define FTGMAC100_RXDES0_VDBC		0x3fff
@@ -248,6 +248,14 @@ struct ftgmac100_rxdes {
 #define FTGMAC100_RXDES0_FRS		(1 << 29)
 #define FTGMAC100_RXDES0_RXPKT_RDY	(1 << 31)
 
+/* Errors we care about for dropping packets */
+#define RXDES0_ANY_ERROR		( \
+	FTGMAC100_RXDES0_RX_ERR		| \
+	FTGMAC100_RXDES0_CRC_ERR	| \
+	FTGMAC100_RXDES0_FTL		| \
+	FTGMAC100_RXDES0_RUNT		| \
+	FTGMAC100_RXDES0_RX_ODD_NB)
+
 #define FTGMAC100_RXDES1_VLANTAG_CI	0xffff
 #define FTGMAC100_RXDES1_PROT_MASK	(0x3 << 20)
 #define FTGMAC100_RXDES1_PROT_NONIP	(0x0 << 20)
-- 
2.9.3

[PATCH 10/10] ftgmac100: Work around HW bug in runt frame detection

[Benjamin Herrenschmidt]

The HW incorrectly calculates the frame size without the vlan
tag and compares that against 64. It will thus flag 64-bytes
frames with a vlan tag as 60-bytes frames "runt" packets
which we'll then drop. Thus we end up dropping ARP packets
on vlan's ...

It does that whether vlan tag stripping is enabled or not.

This works around it by ignoring the "runt" error bit of the
frame has been vlan tagged and is at least 60 bytes.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 35 +++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index e42ef8f..9f18e5e 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -348,7 +348,7 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int *processed)
 	struct ftgmac100_rxdes *rxdes;
 	struct sk_buff *skb;
 	unsigned int pointer, size;
-	u32 status;
+	u32 status, csum_vlan;
 	dma_addr_t map;
 
 	/* Grab next RX descriptor */
@@ -370,10 +370,27 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int *processed)
 		     !(status & FTGMAC100_RXDES0_LRS)))
 		goto drop;
 
+	/* Grab received size and csum vlan field in the descriptor */
+	size = status & FTGMAC100_RXDES0_VDBC;
+	csum_vlan = le32_to_cpu(rxdes->rxdes1);
+
 	/* Any error (other than csum offload) flagged ? */
 	if (unlikely(status & RXDES0_ANY_ERROR)) {
-		ftgmac100_rx_packet_error(priv, status);
-		goto drop;
+		/* Correct for incorrect flagging of runt packets
+		 * with vlan tags... Just accept a runt packet that
+		 * has been flagged as vlan and whose size is at
+		 * least 60 bytes.
+		 */
+		if ((status & FTGMAC100_RXDES0_RUNT) &&
+		    (csum_vlan & FTGMAC100_RXDES1_VLANTAG_AVAIL) &&
+		    (size >= 60))
+			status &= ~FTGMAC100_RXDES0_RUNT;
+
+		/* Any error still in there ? */
+		if (status & RXDES0_ANY_ERROR) {
+			ftgmac100_rx_packet_error(priv, status);
+			goto drop;
+		}
 	}
 
 	/* If the packet had no skb (failed to allocate earlier)
@@ -395,19 +412,17 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int *processed)
 	 * we accept the HW test results.
 	 */
 	if (netdev->features & NETIF_F_RXCSUM) {
-		__le32 csum_vlan = rxdes->rxdes1;
-		__le32 err_bits = cpu_to_le32(FTGMAC100_RXDES1_TCP_CHKSUM_ERR |
-					      FTGMAC100_RXDES1_UDP_CHKSUM_ERR |
-					      FTGMAC100_RXDES1_IP_CHKSUM_ERR);
+		u32 err_bits = FTGMAC100_RXDES1_TCP_CHKSUM_ERR |
+			FTGMAC100_RXDES1_UDP_CHKSUM_ERR |
+			FTGMAC100_RXDES1_IP_CHKSUM_ERR;
 		if ((csum_vlan & err_bits) ||
-		    !(csum_vlan & cpu_to_le32(FTGMAC100_RXDES1_PROT_MASK)))
+		    !(csum_vlan & FTGMAC100_RXDES1_PROT_MASK))
 			skb->ip_summed = CHECKSUM_NONE;
 		else
 			skb->ip_summed = CHECKSUM_UNNECESSARY;
 	}
 
-	/* Grab received size annd transfer to skb */
-	size = status & FTGMAC100_RXDES0_VDBC;
+	/* Transfer received size to skb */
 	skb_put(skb, size);
 
 	/* Tear down DMA mapping, do necessary cache management */
-- 
2.9.3

Re: [PATCH 00/10] ftgmac: Rework batch 2 - RX path

[David Miller]

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Thu,  6 Apr 2017 11:02:42 +1000

> This is the second batch of updates to the ftgmac100 driver.
> 
> This one tackles the RX path of the driver, simplifying
> it greatly to match common practice while significantly
> increasing the performance.
> 
> (The bulk of the performance gains of my series will be
> provided by the TX path improvements, notably fragmented
> sends, these will be in the next batch).

Looks good, series applied, thanks.