From: John Crispin <john@phrozen.org> Date: 2016-06-10 11:32:13
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
From: John Crispin <john@phrozen.org> Date: 2016-06-10 11:28:34
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 <redacted>
---
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(-)
From: John Crispin <john@phrozen.org> Date: 2016-06-10 11:28:35
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 <redacted>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: John Crispin <john@phrozen.org> Date: 2016-06-10 11:28:37
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(-)
From: John Crispin <john@phrozen.org> Date: 2016-06-10 11:28:40
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(-)
@@ -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((unsignedlong)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
From: John Crispin <john@phrozen.org> Date: 2016-06-10 11:29:13
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 <redacted>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 3 +++
1 file changed, 3 insertions(+)
From: John Crispin <john@phrozen.org> Date: 2016-06-10 11:29:15
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 <redacted>
---
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(-)
From: John Crispin <john@phrozen.org> Date: 2016-06-10 11:29:17
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 <redacted>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
@@ -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
From: John Crispin <john@phrozen.org> Date: 2016-06-10 11:29:55
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(-)
From: John Crispin <john@phrozen.org> Date: 2016-06-10 11:30:23
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(-)
From: John Crispin <john@phrozen.org> Date: 2016-06-10 11:30:53
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(-)
From: John Crispin <john@phrozen.org> Date: 2016-06-10 11:31:39
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 <redacted>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: John Crispin <john@phrozen.org> Date: 2016-06-10 11:32:38
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 <redacted>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 ++
1 file changed, 2 insertions(+)
From: David Miller <davem@davemloft.net> Date: 2016-06-10 17:46:13
From: John Crispin <john@phrozen.org>
Date: Fri, 10 Jun 2016 13:30:15 +0200
On 10/06/2016 13:27, John Crispin wrote:
quoted
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.
From: John Crispin <john@phrozen.org> Date: 2016-06-10 17:50:46
On 10/06/2016 19:46, David Miller wrote:
From: John Crispin <john@phrozen.org>
Date: Fri, 10 Jun 2016 13:30:15 +0200
quoted
On 10/06/2016 13:27, John Crispin wrote:
quoted
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.
From: David Miller <davem@davemloft.net> Date: 2016-06-11 06:30:19
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