linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2 00/11] net: mediatek: various small fixes
@ 2016-06-10 11:27 John Crispin
  2016-06-10 11:27 ` [PATCH V2 01/11] net: mediatek: add missing return code check John Crispin
                   ` (12 more replies)
  0 siblings, 13 replies; 16+ messages in thread
From: John Crispin @ 2016-06-10 11:27 UTC (permalink / raw)
  To: David S. Miller
  Cc: Felix Fietkau, Sean Wang, netdev, linux-mediatek, linux-kernel,
	John Crispin

This series contains various small fixes that we stumbled across while
doing thorough testing and code level reviewing of the driver. The only
patch that sticks out is the first one, which addresses a DQL related
issue. The rest are just minor fixes.

Changes in V2:
* drop the DQL patch from the list until a better solution is found

John Crispin (11):
  net: mediatek: add missing return code check
  net: mediatek: fix missing free of scratch memory
  net: mediatek: invalid buffer lookup in mtk_tx_map()
  net: mediatek: dropped rx packets are not being counted properly
  net: mediatek: add next data pointer coherency protection
  net: mediatek: disable all interrupts during probe
  net: mediatek: fix threshold value
  net: mediatek: increase watchdog_timeo
  net: mediatek: fix off by one in the TX ring allocation
  net: mediatek: only wake the queue if it is stopped
  net: mediatek: remove superfluous queue wake up call

 drivers/net/ethernet/mediatek/mtk_eth_soc.c |   61 ++++++++++++++++++---------
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |    3 ++
 2 files changed, 45 insertions(+), 19 deletions(-)

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH V2 01/11] net: mediatek: add missing return code check
  2016-06-10 11:27 [PATCH V2 00/11] net: mediatek: various small fixes John Crispin
@ 2016-06-10 11:27 ` John Crispin
  2016-06-10 11:27 ` [PATCH V2 02/11] net: mediatek: fix missing free of scratch memory John Crispin
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: John Crispin @ 2016-06-10 11:27 UTC (permalink / raw)
  To: David S. Miller
  Cc: Felix Fietkau, Sean Wang, netdev, linux-mediatek, linux-kernel,
	John Crispin

The code fails to check if the scratch memory was properly allocated. Add
this check and return with an error if the allocation failed.

Signed-off-by: John Crispin <john@phrozen.org>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 45f8dbf..6780886 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -467,6 +467,9 @@ static int mtk_init_fq_dma(struct mtk_eth *eth)
 
 	eth->scratch_head = kcalloc(cnt, MTK_QDMA_PAGE_SIZE,
 				    GFP_KERNEL);
+	if (unlikely(!eth->scratch_head))
+		return -ENOMEM;
+
 	dma_addr = dma_map_single(eth->dev,
 				  eth->scratch_head, cnt * MTK_QDMA_PAGE_SIZE,
 				  DMA_FROM_DEVICE);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V2 02/11] net: mediatek: fix missing free of scratch memory
  2016-06-10 11:27 [PATCH V2 00/11] net: mediatek: various small fixes John Crispin
  2016-06-10 11:27 ` [PATCH V2 01/11] net: mediatek: add missing return code check John Crispin
@ 2016-06-10 11:27 ` John Crispin
  2016-06-10 11:28 ` [PATCH V2 03/11] net: mediatek: invalid buffer lookup in mtk_tx_map() John Crispin
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: John Crispin @ 2016-06-10 11:27 UTC (permalink / raw)
  To: David S. Miller
  Cc: Felix Fietkau, Sean Wang, netdev, linux-mediatek, linux-kernel,
	John Crispin

Scratch memory gets allocated in mtk_init_fq_dma() but the corresponding
code to free it is missing inside mtk_dma_free() causing a memory leak.
With this patch applied, we can run ifconfig up/down several thousand
times without any problems.

Signed-off-by: John Crispin <john@phrozen.org>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |   18 +++++++++++++-----
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |    2 ++
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 6780886..8d79bbf 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -453,14 +453,14 @@ static inline void mtk_rx_get_desc(struct mtk_rx_dma *rxd,
 /* the qdma core needs scratch memory to be setup */
 static int mtk_init_fq_dma(struct mtk_eth *eth)
 {
-	dma_addr_t phy_ring_head, phy_ring_tail;
+	dma_addr_t phy_ring_tail;
 	int cnt = MTK_DMA_SIZE;
 	dma_addr_t dma_addr;
 	int i;
 
 	eth->scratch_ring = dma_alloc_coherent(eth->dev,
 					       cnt * sizeof(struct mtk_tx_dma),
-					       &phy_ring_head,
+					       &eth->phy_scratch_ring,
 					       GFP_ATOMIC | __GFP_ZERO);
 	if (unlikely(!eth->scratch_ring))
 		return -ENOMEM;
@@ -477,19 +477,19 @@ static int mtk_init_fq_dma(struct mtk_eth *eth)
 		return -ENOMEM;
 
 	memset(eth->scratch_ring, 0x0, sizeof(struct mtk_tx_dma) * cnt);
-	phy_ring_tail = phy_ring_head +
+	phy_ring_tail = eth->phy_scratch_ring +
 			(sizeof(struct mtk_tx_dma) * (cnt - 1));
 
 	for (i = 0; i < cnt; i++) {
 		eth->scratch_ring[i].txd1 =
 					(dma_addr + (i * MTK_QDMA_PAGE_SIZE));
 		if (i < cnt - 1)
-			eth->scratch_ring[i].txd2 = (phy_ring_head +
+			eth->scratch_ring[i].txd2 = (eth->phy_scratch_ring +
 				((i + 1) * sizeof(struct mtk_tx_dma)));
 		eth->scratch_ring[i].txd3 = TX_DMA_SDL(MTK_QDMA_PAGE_SIZE);
 	}
 
-	mtk_w32(eth, phy_ring_head, MTK_QDMA_FQ_HEAD);
+	mtk_w32(eth, eth->phy_scratch_ring, MTK_QDMA_FQ_HEAD);
 	mtk_w32(eth, phy_ring_tail, MTK_QDMA_FQ_TAIL);
 	mtk_w32(eth, (cnt << 16) | cnt, MTK_QDMA_FQ_CNT);
 	mtk_w32(eth, MTK_QDMA_PAGE_SIZE << 16, MTK_QDMA_FQ_BLEN);
@@ -1189,6 +1189,14 @@ static void mtk_dma_free(struct mtk_eth *eth)
 	for (i = 0; i < MTK_MAC_COUNT; i++)
 		if (eth->netdev[i])
 			netdev_reset_queue(eth->netdev[i]);
+	if (eth->scratch_ring) {
+		dma_free_coherent(eth->dev,
+				  MTK_DMA_SIZE * sizeof(struct mtk_tx_dma),
+				  eth->scratch_ring,
+				  eth->phy_scratch_ring);
+		eth->scratch_ring = NULL;
+		eth->phy_scratch_ring = 0;
+	}
 	mtk_tx_clean(eth);
 	mtk_rx_clean(eth);
 	kfree(eth->scratch_head);
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index eed626d..57f7e8a 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -357,6 +357,7 @@ struct mtk_rx_ring {
  * @rx_ring:		Pointer to the memore holding info about the RX ring
  * @rx_napi:		The NAPI struct
  * @scratch_ring:	Newer SoCs need memory for a second HW managed TX ring
+ * @phy_scratch_ring:	physical address of scratch_ring
  * @scratch_head:	The scratch memory that scratch_ring points to.
  * @clk_ethif:		The ethif clock
  * @clk_esw:		The switch clock
@@ -384,6 +385,7 @@ struct mtk_eth {
 	struct mtk_rx_ring		rx_ring;
 	struct napi_struct		rx_napi;
 	struct mtk_tx_dma		*scratch_ring;
+	dma_addr_t			phy_scratch_ring;
 	void				*scratch_head;
 	struct clk			*clk_ethif;
 	struct clk			*clk_esw;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V2 03/11] net: mediatek: invalid buffer lookup in mtk_tx_map()
  2016-06-10 11:27 [PATCH V2 00/11] net: mediatek: various small fixes John Crispin
  2016-06-10 11:27 ` [PATCH V2 01/11] net: mediatek: add missing return code check John Crispin
  2016-06-10 11:27 ` [PATCH V2 02/11] net: mediatek: fix missing free of scratch memory John Crispin
@ 2016-06-10 11:28 ` John Crispin
  2016-06-10 11:28 ` [PATCH V2 04/11] net: mediatek: dropped rx packets are not being counted properly John Crispin
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: John Crispin @ 2016-06-10 11:28 UTC (permalink / raw)
  To: David S. Miller
  Cc: Felix Fietkau, Sean Wang, netdev, linux-mediatek, linux-kernel,
	John Crispin

The lookup of the tx_buffer in the error path inside mtk_tx_map() uses the
wrong descriptor pointer. This looks like a copy & paste error. Change the
code to use the correct pointer.

Signed-off-by: John Crispin <john@phrozen.org>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 8d79bbf..4f22ee9 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -655,7 +655,7 @@ static int mtk_tx_map(struct sk_buff *skb, struct net_device *dev,
 
 err_dma:
 	do {
-		tx_buf = mtk_desc_to_tx_buf(ring, txd);
+		tx_buf = mtk_desc_to_tx_buf(ring, itxd);
 
 		/* unmap dma */
 		mtk_tx_unmap(&dev->dev, tx_buf);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V2 04/11] net: mediatek: dropped rx packets are not being counted properly
  2016-06-10 11:27 [PATCH V2 00/11] net: mediatek: various small fixes John Crispin
                   ` (2 preceding siblings ...)
  2016-06-10 11:28 ` [PATCH V2 03/11] net: mediatek: invalid buffer lookup in mtk_tx_map() John Crispin
@ 2016-06-10 11:28 ` John Crispin
  2016-06-10 11:28 ` [PATCH V2 05/11] net: mediatek: add next data pointer coherency protection John Crispin
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: John Crispin @ 2016-06-10 11:28 UTC (permalink / raw)
  To: David S. Miller
  Cc: Felix Fietkau, Sean Wang, netdev, linux-mediatek, linux-kernel,
	John Crispin

There are two places inside mtk_poll_rx where rx_dropped is not being
incremented properly. Fix this by adding the missing code to increment
the counter.

Signed-off-by: John Crispin <john@phrozen.org>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 4f22ee9..0610262 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -810,6 +810,7 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
 					  DMA_FROM_DEVICE);
 		if (unlikely(dma_mapping_error(&netdev->dev, dma_addr))) {
 			skb_free_frag(new_data);
+			netdev->stats.rx_dropped++;
 			goto release_desc;
 		}
 
@@ -817,6 +818,7 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
 		skb = build_skb(data, ring->frag_size);
 		if (unlikely(!skb)) {
 			put_page(virt_to_head_page(new_data));
+			netdev->stats.rx_dropped++;
 			goto release_desc;
 		}
 		skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V2 05/11] net: mediatek: add next data pointer coherency protection
  2016-06-10 11:27 [PATCH V2 00/11] net: mediatek: various small fixes John Crispin
                   ` (3 preceding siblings ...)
  2016-06-10 11:28 ` [PATCH V2 04/11] net: mediatek: dropped rx packets are not being counted properly John Crispin
@ 2016-06-10 11:28 ` John Crispin
  2016-06-10 11:28 ` [PATCH V2 06/11] net: mediatek: disable all interrupts during probe John Crispin
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: John Crispin @ 2016-06-10 11:28 UTC (permalink / raw)
  To: David S. Miller
  Cc: Felix Fietkau, Sean Wang, netdev, linux-mediatek, linux-kernel,
	John Crispin

The QDMA engine can fail to update the register pointing to the next TX
descriptor if this bit does not get set in the QDMA configuration register.
Not setting this bit can result in invalid values inside the TX rings
registers which will causes TX stalls.

Signed-off-by: John Crispin <john@phrozen.org>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |    2 +-
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |    1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 0610262..bfac376 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1261,7 +1261,7 @@ static int mtk_start_dma(struct mtk_eth *eth)
 	mtk_w32(eth,
 		MTK_TX_WB_DDONE | MTK_RX_DMA_EN | MTK_TX_DMA_EN |
 		MTK_RX_2B_OFFSET | MTK_DMA_SIZE_16DWORDS |
-		MTK_RX_BT_32DWORDS,
+		MTK_RX_BT_32DWORDS | MTK_NDP_CO_PRO,
 		MTK_QDMA_GLO_CFG);
 
 	return 0;
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index 57f7e8a..a5eb7c6 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -91,6 +91,7 @@
 #define MTK_QDMA_GLO_CFG	0x1A04
 #define MTK_RX_2B_OFFSET	BIT(31)
 #define MTK_RX_BT_32DWORDS	(3 << 11)
+#define MTK_NDP_CO_PRO		BIT(10)
 #define MTK_TX_WB_DDONE		BIT(6)
 #define MTK_DMA_SIZE_16DWORDS	(2 << 4)
 #define MTK_RX_DMA_BUSY		BIT(3)
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V2 06/11] net: mediatek: disable all interrupts during probe
  2016-06-10 11:27 [PATCH V2 00/11] net: mediatek: various small fixes John Crispin
                   ` (4 preceding siblings ...)
  2016-06-10 11:28 ` [PATCH V2 05/11] net: mediatek: add next data pointer coherency protection John Crispin
@ 2016-06-10 11:28 ` John Crispin
  2016-06-10 11:28 ` [PATCH V2 07/11] net: mediatek: fix threshold value John Crispin
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: John Crispin @ 2016-06-10 11:28 UTC (permalink / raw)
  To: David S. Miller
  Cc: Felix Fietkau, Sean Wang, netdev, linux-mediatek, linux-kernel,
	John Crispin

The current code only disables those IRQs that we will later use. To
ensure that we have a predefined state, we really want to disable all IRQs.
Change the code to disable all IRQs to achieve this.

Signed-off-by: John Crispin <john@phrozen.org>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index bfac376..93af4e3 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1375,7 +1375,7 @@ static int __init mtk_hw_init(struct mtk_eth *eth)
 
 	/* disable delay and normal interrupt */
 	mtk_w32(eth, 0, MTK_QDMA_DELAY_INT);
-	mtk_irq_disable(eth, MTK_TX_DONE_INT | MTK_RX_DONE_INT);
+	mtk_irq_disable(eth, ~0);
 	mtk_w32(eth, RST_GL_PSE, MTK_RST_GL);
 	mtk_w32(eth, 0, MTK_RST_GL);
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V2 07/11] net: mediatek: fix threshold value
  2016-06-10 11:27 [PATCH V2 00/11] net: mediatek: various small fixes John Crispin
                   ` (5 preceding siblings ...)
  2016-06-10 11:28 ` [PATCH V2 06/11] net: mediatek: disable all interrupts during probe John Crispin
@ 2016-06-10 11:28 ` John Crispin
  2016-06-10 11:28 ` [PATCH V2 08/11] net: mediatek: increase watchdog_timeo John Crispin
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: John Crispin @ 2016-06-10 11:28 UTC (permalink / raw)
  To: David S. Miller
  Cc: Felix Fietkau, Sean Wang, netdev, linux-mediatek, linux-kernel,
	John Crispin

The logic to calculate the threshold value for stopping the TX queue is
bad. Currently it will always use 1/2 of the rings size, which is way too
much. Set the threshold to MAX_SKB_FRAGS. This makes sure that the queue
is stopped when there is not enough room to accept an additional segment. 

Signed-off-by: John Crispin <john@phrozen.org>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 93af4e3..8b289e1 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1012,8 +1012,7 @@ static int mtk_tx_alloc(struct mtk_eth *eth)
 	atomic_set(&ring->free_count, MTK_DMA_SIZE - 2);
 	ring->next_free = &ring->dma[0];
 	ring->last_free = &ring->dma[MTK_DMA_SIZE - 2];
-	ring->thresh = max((unsigned long)MTK_DMA_SIZE >> 2,
-			      MAX_SKB_FRAGS);
+	ring->thresh = MAX_SKB_FRAGS;
 
 	/* make sure that all changes to the dma ring are flushed before we
 	 * continue
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V2 08/11] net: mediatek: increase watchdog_timeo
  2016-06-10 11:27 [PATCH V2 00/11] net: mediatek: various small fixes John Crispin
                   ` (6 preceding siblings ...)
  2016-06-10 11:28 ` [PATCH V2 07/11] net: mediatek: fix threshold value John Crispin
@ 2016-06-10 11:28 ` John Crispin
  2016-06-10 11:28 ` [PATCH V2 09/11] net: mediatek: fix off by one in the TX ring allocation John Crispin
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: John Crispin @ 2016-06-10 11:28 UTC (permalink / raw)
  To: David S. Miller
  Cc: Felix Fietkau, Sean Wang, netdev, linux-mediatek, linux-kernel,
	John Crispin

During stress testing, after reducing the threshold value, we have seen
TX timeouts that were caused by the watchdog_timeo value being too low.
Increase the value to 5 * HZ which is a value commonly used by many other
drivers.

Signed-off-by: John Crispin <john@phrozen.org>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 8b289e1..24714ab 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1688,7 +1688,7 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
 	mac->hw_stats->reg_offset = id * MTK_STAT_OFFSET;
 
 	SET_NETDEV_DEV(eth->netdev[id], eth->dev);
-	eth->netdev[id]->watchdog_timeo = HZ;
+	eth->netdev[id]->watchdog_timeo = 5 * HZ;
 	eth->netdev[id]->netdev_ops = &mtk_netdev_ops;
 	eth->netdev[id]->base_addr = (unsigned long)eth->base;
 	eth->netdev[id]->vlan_features = MTK_HW_FEATURES &
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V2 09/11] net: mediatek: fix off by one in the TX ring allocation
  2016-06-10 11:27 [PATCH V2 00/11] net: mediatek: various small fixes John Crispin
                   ` (7 preceding siblings ...)
  2016-06-10 11:28 ` [PATCH V2 08/11] net: mediatek: increase watchdog_timeo John Crispin
@ 2016-06-10 11:28 ` John Crispin
  2016-06-10 11:28 ` [PATCH V2 10/11] net: mediatek: only wake the queue if it is stopped John Crispin
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: John Crispin @ 2016-06-10 11:28 UTC (permalink / raw)
  To: David S. Miller
  Cc: Felix Fietkau, Sean Wang, netdev, linux-mediatek, linux-kernel,
	John Crispin

The TX ring setup has an off by one error causing it to not utilise all
descriptors. This has the side effect that we need to reset the next
pointer at runtime to make it work. Fix the off by one and remove the
code fixing the ring at runtime.

Signed-off-by: John Crispin <john@phrozen.org>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 24714ab..6daf48b 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -903,7 +903,6 @@ static int mtk_poll_tx(struct mtk_eth *eth, int budget, bool *tx_again)
 		}
 		mtk_tx_unmap(eth->dev, tx_buf);
 
-		ring->last_free->txd2 = next_cpu;
 		ring->last_free = desc;
 		atomic_inc(&ring->free_count);
 
@@ -1011,7 +1010,7 @@ static int mtk_tx_alloc(struct mtk_eth *eth)
 
 	atomic_set(&ring->free_count, MTK_DMA_SIZE - 2);
 	ring->next_free = &ring->dma[0];
-	ring->last_free = &ring->dma[MTK_DMA_SIZE - 2];
+	ring->last_free = &ring->dma[MTK_DMA_SIZE - 1];
 	ring->thresh = MAX_SKB_FRAGS;
 
 	/* make sure that all changes to the dma ring are flushed before we
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V2 10/11] net: mediatek: only wake the queue if it is stopped
  2016-06-10 11:27 [PATCH V2 00/11] net: mediatek: various small fixes John Crispin
                   ` (8 preceding siblings ...)
  2016-06-10 11:28 ` [PATCH V2 09/11] net: mediatek: fix off by one in the TX ring allocation John Crispin
@ 2016-06-10 11:28 ` John Crispin
  2016-06-10 11:28 ` [PATCH V2 11/11] net: mediatek: remove superfluous queue wake up call John Crispin
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: John Crispin @ 2016-06-10 11:28 UTC (permalink / raw)
  To: David S. Miller
  Cc: Felix Fietkau, Sean Wang, netdev, linux-mediatek, linux-kernel,
	John Crispin

The current code unconditionally wakes up the queue at the end of each
tx_poll action. Change the code to only wake up the queues if any of
them have actually been stopped before.

Signed-off-by: John Crispin <john@phrozen.org>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |   17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 6daf48b..40d3cfd 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -685,6 +685,20 @@ static inline int mtk_cal_txd_req(struct sk_buff *skb)
 	return nfrags;
 }
 
+static int mtk_queue_stopped(struct mtk_eth *eth)
+{
+	int i;
+
+	for (i = 0; i < MTK_MAC_COUNT; i++) {
+		if (!eth->netdev[i])
+			continue;
+		if (netif_queue_stopped(eth->netdev[i]))
+			return 1;
+	}
+
+	return 0;
+}
+
 static void mtk_wake_queue(struct mtk_eth *eth)
 {
 	int i;
@@ -929,7 +943,8 @@ static int mtk_poll_tx(struct mtk_eth *eth, int budget, bool *tx_again)
 	if (!total)
 		return 0;
 
-	if (atomic_read(&ring->free_count) > ring->thresh)
+	if (mtk_queue_stopped(eth) &&
+	    (atomic_read(&ring->free_count) > ring->thresh))
 		mtk_wake_queue(eth);
 
 	return total;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V2 11/11] net: mediatek: remove superfluous queue wake up call
  2016-06-10 11:27 [PATCH V2 00/11] net: mediatek: various small fixes John Crispin
                   ` (9 preceding siblings ...)
  2016-06-10 11:28 ` [PATCH V2 10/11] net: mediatek: only wake the queue if it is stopped John Crispin
@ 2016-06-10 11:28 ` John Crispin
  2016-06-10 11:30 ` [PATCH V2 00/11] net: mediatek: various small fixes John Crispin
  2016-06-11  6:30 ` David Miller
  12 siblings, 0 replies; 16+ messages in thread
From: John Crispin @ 2016-06-10 11:28 UTC (permalink / raw)
  To: David S. Miller
  Cc: Felix Fietkau, Sean Wang, netdev, linux-mediatek, linux-kernel,
	John Crispin

The code checks if the queue should be stopped because we are below the
threshold of free descriptors only to check if it should be started again.
If we do end up in a state where we are at the threshold limit, it makes
more sense to just stop the queue and wait for the next IRQ to trigger the
TX housekeeping again. There is no rush in enqueuing the next packet, it
needs to wait for all the others in the queue to be dispatched first
anyway.

Signed-off-by: John Crispin <john@phrozen.org>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 40d3cfd..ebfca7d 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -764,12 +764,9 @@ static int mtk_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	if (mtk_tx_map(skb, dev, tx_num, ring, gso) < 0)
 		goto drop;
 
-	if (unlikely(atomic_read(&ring->free_count) <= ring->thresh)) {
+	if (unlikely(atomic_read(&ring->free_count) <= ring->thresh))
 		mtk_stop_queue(eth);
-		if (unlikely(atomic_read(&ring->free_count) >
-			     ring->thresh))
-			mtk_wake_queue(eth);
-	}
+
 	spin_unlock_irqrestore(&eth->page_lock, flags);
 
 	return NETDEV_TX_OK;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH V2 00/11] net: mediatek: various small fixes
  2016-06-10 11:27 [PATCH V2 00/11] net: mediatek: various small fixes John Crispin
                   ` (10 preceding siblings ...)
  2016-06-10 11:28 ` [PATCH V2 11/11] net: mediatek: remove superfluous queue wake up call John Crispin
@ 2016-06-10 11:30 ` John Crispin
  2016-06-10 17:46   ` David Miller
  2016-06-11  6:30 ` David Miller
  12 siblings, 1 reply; 16+ messages in thread
From: John Crispin @ 2016-06-10 11:30 UTC (permalink / raw)
  To: David S. Miller
  Cc: Sean Wang, netdev, linux-kernel, linux-mediatek, Felix Fietkau



On 10/06/2016 13:27, John Crispin wrote:
> This series contains various small fixes that we stumbled across while
> doing thorough testing and code level reviewing of the driver. The only
> patch that sticks out is the first one, which addresses a DQL related
> issue. The rest are just minor fixes.
> 

Hi David,

i forgot to remove the last sentence here. can you live with that as it
wont end up in the git history or do you want me to send a V3 with this
line removed.

	John


> Changes in V2:
> * drop the DQL patch from the list until a better solution is found
> 
> John Crispin (11):
>   net: mediatek: add missing return code check
>   net: mediatek: fix missing free of scratch memory
>   net: mediatek: invalid buffer lookup in mtk_tx_map()
>   net: mediatek: dropped rx packets are not being counted properly
>   net: mediatek: add next data pointer coherency protection
>   net: mediatek: disable all interrupts during probe
>   net: mediatek: fix threshold value
>   net: mediatek: increase watchdog_timeo
>   net: mediatek: fix off by one in the TX ring allocation
>   net: mediatek: only wake the queue if it is stopped
>   net: mediatek: remove superfluous queue wake up call
> 
>  drivers/net/ethernet/mediatek/mtk_eth_soc.c |   61 ++++++++++++++++++---------
>  drivers/net/ethernet/mediatek/mtk_eth_soc.h |    3 ++
>  2 files changed, 45 insertions(+), 19 deletions(-)
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH V2 00/11] net: mediatek: various small fixes
  2016-06-10 11:30 ` [PATCH V2 00/11] net: mediatek: various small fixes John Crispin
@ 2016-06-10 17:46   ` David Miller
  2016-06-10 17:50     ` John Crispin
  0 siblings, 1 reply; 16+ messages in thread
From: David Miller @ 2016-06-10 17:46 UTC (permalink / raw)
  To: john; +Cc: keyhaede, netdev, linux-kernel, linux-mediatek, nbd

From: John Crispin <john@phrozen.org>
Date: Fri, 10 Jun 2016 13:30:15 +0200

> 
> 
> On 10/06/2016 13:27, John Crispin wrote:
>> This series contains various small fixes that we stumbled across while
>> doing thorough testing and code level reviewing of the driver. The only
>> patch that sticks out is the first one, which addresses a DQL related
>> issue. The rest are just minor fixes.
>> 
> 
> Hi David,
> 
> i forgot to remove the last sentence here. can you live with that as it
> wont end up in the git history or do you want me to send a V3 with this
> line removed.

What do you mean it won't end up in the GIT history?  I always put this
introductory text into the merge commit for the patch series.

Now, I can remove it for you, which I will do.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH V2 00/11] net: mediatek: various small fixes
  2016-06-10 17:46   ` David Miller
@ 2016-06-10 17:50     ` John Crispin
  0 siblings, 0 replies; 16+ messages in thread
From: John Crispin @ 2016-06-10 17:50 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, nbd, linux-mediatek, keyhaede, linux-kernel



On 10/06/2016 19:46, David Miller wrote:
> From: John Crispin <john@phrozen.org>
> Date: Fri, 10 Jun 2016 13:30:15 +0200
> 
>>
>>
>> On 10/06/2016 13:27, John Crispin wrote:
>>> This series contains various small fixes that we stumbled across while
>>> doing thorough testing and code level reviewing of the driver. The only
>>> patch that sticks out is the first one, which addresses a DQL related
>>> issue. The rest are just minor fixes.
>>>
>>
>> Hi David,
>>
>> i forgot to remove the last sentence here. can you live with that as it
>> wont end up in the git history or do you want me to send a V3 with this
>> line removed.
> 
> What do you mean it won't end up in the GIT history?  I always put this
> introductory text into the merge commit for the patch series.
> 
> Now, I can remove it for you, which I will do.
> 

cool, thanks a lot !

	John

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH V2 00/11] net: mediatek: various small fixes
  2016-06-10 11:27 [PATCH V2 00/11] net: mediatek: various small fixes John Crispin
                   ` (11 preceding siblings ...)
  2016-06-10 11:30 ` [PATCH V2 00/11] net: mediatek: various small fixes John Crispin
@ 2016-06-11  6:30 ` David Miller
  12 siblings, 0 replies; 16+ messages in thread
From: David Miller @ 2016-06-11  6:30 UTC (permalink / raw)
  To: john; +Cc: nbd, keyhaede, netdev, linux-mediatek, linux-kernel

From: John Crispin <john@phrozen.org>
Date: Fri, 10 Jun 2016 13:27:57 +0200

> This series contains various small fixes that we stumbled across while
> doing thorough testing and code level reviewing of the driver. The only
> patch that sticks out is the first one, which addresses a DQL related
> issue. The rest are just minor fixes.
> 
> Changes in V2:
> * drop the DQL patch from the list until a better solution is found

Series applied, thanks.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2016-06-11  6:30 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-10 11:27 [PATCH V2 00/11] net: mediatek: various small fixes John Crispin
2016-06-10 11:27 ` [PATCH V2 01/11] net: mediatek: add missing return code check John Crispin
2016-06-10 11:27 ` [PATCH V2 02/11] net: mediatek: fix missing free of scratch memory John Crispin
2016-06-10 11:28 ` [PATCH V2 03/11] net: mediatek: invalid buffer lookup in mtk_tx_map() John Crispin
2016-06-10 11:28 ` [PATCH V2 04/11] net: mediatek: dropped rx packets are not being counted properly John Crispin
2016-06-10 11:28 ` [PATCH V2 05/11] net: mediatek: add next data pointer coherency protection John Crispin
2016-06-10 11:28 ` [PATCH V2 06/11] net: mediatek: disable all interrupts during probe John Crispin
2016-06-10 11:28 ` [PATCH V2 07/11] net: mediatek: fix threshold value John Crispin
2016-06-10 11:28 ` [PATCH V2 08/11] net: mediatek: increase watchdog_timeo John Crispin
2016-06-10 11:28 ` [PATCH V2 09/11] net: mediatek: fix off by one in the TX ring allocation John Crispin
2016-06-10 11:28 ` [PATCH V2 10/11] net: mediatek: only wake the queue if it is stopped John Crispin
2016-06-10 11:28 ` [PATCH V2 11/11] net: mediatek: remove superfluous queue wake up call John Crispin
2016-06-10 11:30 ` [PATCH V2 00/11] net: mediatek: various small fixes John Crispin
2016-06-10 17:46   ` David Miller
2016-06-10 17:50     ` John Crispin
2016-06-11  6:30 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).