Skip to contentSkip to conversation

net: stmmac: Misc fixes

PATCH Superseded Jose Abreu 2 participants 7 messages Jan 8, 2019 – Jan 9, 2019

Subsystems (4)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Jose Abreu

author
#0 lore raw

cover net: stmmac: Misc fixes

Some small fixes for stmmac targeting -net. Detailed info in commit log.

Cc Joao Pinto <[email protected]>
Cc David S. Miller <[email protected]>
Cc Giuseppe Cavallaro <[email protected]>
Cc Alexandre Torgue <[email protected]>

Jose Abreu (5):
net: stmmac: Fix PCI module removal leak
net: stmmac: dwxgmac2: Only clear interrupts that are active
net: stmmac: Check if CBS is supported before configuring
net: stmmac: Fix the logic of checking if RX Watchdog must be enabled
net: stmmac: Prevent RX starvation in stmmac_napi_poll()

 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c |  6 +--
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 51 +++++++++++-----------
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c   | 10 +++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c    |  2 +
 4 files changed, 41 insertions(+), 28 deletions(-)
Signature · 1 line
2.7.4
author
#1 lore raw

PATCH 1/5 net: stmmac: Fix PCI module removal leak

Since commit b7d0f08e9129, the enable / disable of PCI device is not managed which will result in IO regions not being automatically unmapped. As regions continue mapped it is currently not possible to remove and then probe again the PCI module of stmmac.

Fix this by manually unmapping regions on remove callback.

Cc Joao Pinto <[email protected]>
Cc David S. Miller <[email protected]>
Cc Giuseppe Cavallaro <[email protected]>
Cc Alexandre Torgue <[email protected]>
Fixes b7d0f08e9129 ("net: stmmac: Fix WoL for PCI-based setups")
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c +10 -0
index c54a50dbd5ac..9d58c7cb243c 100644
@@ -299,7 +299,17 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
  */
 static void stmmac_pci_remove(struct pci_dev *pdev)
 {
+	int i;
+
 	stmmac_dvr_remove(&pdev->dev);
+
+	for (i = 0; i <= PCI_STD_RESOURCE_LEN; i++) {
+		if (pci_resource_len(pdev, i) == 0)
+			continue;
+		pcim_iounmap_regions(pdev, BIT(i));
+		break;
+	}
+
 	pci_disable_device(pdev);
 }
 
Signature · 1 line
2.7.4
#2 lore raw

Re: [PATCH net 1/5] net: stmmac: Fix PCI module removal leak

I love your patch! Yet something to improve:

Hi Jose,

I love your patch! Yet something to improve:

[auto build test ERROR on net/master]

url: https://github.com/0day-ci/linux/commits/Jose-Abreu/net-stmmac-Misc-fixes/20190109-071238
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:

        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c: In function 'stmmac_pci_remove':

>> drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c:306:19: error: 'PCI_STD_RESOURCE_LEN' undeclared (first use in this function); did you mean 'PCI_STD_RESOURCE_END'?
     for (i = 0; i <= PCI_STD_RESOURCE_LEN; i++) {
                      ^~~~~~~~~~~~~~~~~~~~
                      PCI_STD_RESOURCE_END

drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c:306:19: note: each undeclared identifier is reported only once for each function it appears in

vim +306 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c

292
293 /**
294 * stmmac_pci_remove
295 *
296 * @pdev: platform device pointer
297 * Description: this function calls the main to free the net resources
298 * and releases the PCI resources.
299 */
300 static void stmmac_pci_remove(struct pci_dev *pdev)
301 {
302 int i;
303
304 stmmac_dvr_remove(&pdev->dev);
305

 > 306		for (i = 0; i <= PCI_STD_RESOURCE_LEN; i++) {

307 if (pci_resource_len(pdev, i) == 0)
308 continue;
309 pcim_iounmap_regions(pdev, BIT(i));
310 break;
311 }
312
313 pci_disable_device(pdev);
314 }
315

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

author
#3 lore raw

PATCH 2/5 net: stmmac: dwxgmac2: Only clear interrupts that are active

In reply to Jose Abreu

In DMA interrupt handler we were clearing all interrupts status, even the ones that were not active. Fix this and only clear the active interrupts.

Cc Joao Pinto <[email protected]>
Cc David S. Miller <[email protected]>
Cc Giuseppe Cavallaro <[email protected]>
Cc Alexandre Torgue <[email protected]>
---
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c +3 -3
index 6c5092e7771c..c5e25580a43f 100644
@@ -263,6 +263,7 @@ static int dwxgmac2_dma_interrupt(void __iomem *ioaddr,
 				  struct stmmac_extra_stats *x, u32 chan)
 {
 	u32 intr_status = readl(ioaddr + XGMAC_DMA_CH_STATUS(chan));
+	u32 intr_en = readl(ioaddr + XGMAC_DMA_CH_INT_EN(chan));
 	int ret = 0;
 
 	/* ABNORMAL interrupts */
@@ -282,8 +283,7 @@ static int dwxgmac2_dma_interrupt(void __iomem *ioaddr,
 		x->normal_irq_n++;
 
 		if (likely(intr_status & XGMAC_RI)) {
-			u32 value = readl(ioaddr + XGMAC_DMA_CH_INT_EN(chan));
-			if (likely(value & XGMAC_RIE)) {
+			if (likely(intr_en & XGMAC_RIE)) {
 				x->rx_normal_irq_n++;
 				ret |= handle_rx;
 			}
@@ -295,7 +295,7 @@ static int dwxgmac2_dma_interrupt(void __iomem *ioaddr,
 	}
 
 	/* Clear interrupts */
-	writel(~0x0, ioaddr + XGMAC_DMA_CH_STATUS(chan));
+	writel(intr_en & intr_status, ioaddr + XGMAC_DMA_CH_STATUS(chan));
 
 	return ret;
 }
Signature · 1 line
2.7.4
author
#4 lore raw

PATCH 3/5 net: stmmac: Check if CBS is supported before configuring

In reply to Jose Abreu

Check if CBS is currently supported before trying to configure it in HW.

Cc Joao Pinto <[email protected]>
Cc David S. Miller <[email protected]>
Cc Giuseppe Cavallaro <[email protected]>
Cc Alexandre Torgue <[email protected]>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c +2 -0
index 531294f4978b..58ea18af9813 100644
@@ -301,6 +301,8 @@ static int tc_setup_cbs(struct stmmac_priv *priv,
 	/* Queue 0 is not AVB capable */
 	if (queue <= 0 || queue >= tx_queues_count)
 		return -EINVAL;
+	if (!priv->dma_cap.av)
+		return -EOPNOTSUPP;
 	if (priv->speed != SPEED_100 && priv->speed != SPEED_1000)
 		return -EOPNOTSUPP;
 
Signature · 1 line
2.7.4
author
#5 lore raw

PATCH 4/5 net: stmmac: Fix the logic of checking if RX Watchdog must be enabled

In reply to Jose Abreu

RX Watchdog can be disabled by platform definitions but currently we are initializing the descriptors before checking if Watchdog must be disabled or not.

Fix this by checking earlier if user wants Watchdog disabled or not.

Cc Joao Pinto <[email protected]>
Cc David S. Miller <[email protected]>
Cc Giuseppe Cavallaro <[email protected]>
Cc Alexandre Torgue <[email protected]>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 24 +++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +12 -12
index 0e0a0789c2ed..83ceb1a12e77 100644
@@ -4168,6 +4168,18 @@ static int stmmac_hw_init(struct stmmac_priv *priv)
 			return ret;
 	}
 
+	/* Rx Watchdog is available in the COREs newer than the 3.40.
+	 * In some case, for example on bugged HW this feature
+	 * has to be disable and this can be done by passing the
+	 * riwt_off field from the platform.
+	 */
+	if (((priv->synopsys_id >= DWMAC_CORE_3_50) ||
+	    (priv->plat->has_xgmac)) && (!priv->plat->riwt_off)) {
+		priv->use_riwt = 1;
+		dev_info(priv->device,
+			 "Enable RX Mitigation via HW Watchdog Timer\n");
+	}
+
 	return 0;
 }
 
@@ -4300,18 +4312,6 @@ int stmmac_dvr_probe(struct device *device,
 	if (flow_ctrl)
 		priv->flow_ctrl = FLOW_AUTO;	/* RX/TX pause on */
 
-	/* Rx Watchdog is available in the COREs newer than the 3.40.
-	 * In some case, for example on bugged HW this feature
-	 * has to be disable and this can be done by passing the
-	 * riwt_off field from the platform.
-	 */
-	if (((priv->synopsys_id >= DWMAC_CORE_3_50) ||
-	    (priv->plat->has_xgmac)) && (!priv->plat->riwt_off)) {
-		priv->use_riwt = 1;
-		dev_info(priv->device,
-			 "Enable RX Mitigation via HW Watchdog Timer\n");
-	}
-
 	/* Setup channels NAPI */
 	maxq = max(priv->plat->rx_queues_to_use, priv->plat->tx_queues_to_use);
 
Signature · 1 line
2.7.4
author
#6 lore raw

PATCH 5/5 net: stmmac: Prevent RX starvation in stmmac_napi_poll()

In reply to Jose Abreu

Currently, TX is given a budget which is consumed by stmmac_tx_clean() and stmmac_rx() is given the remaining non-consumed budget.

This is wrong and in case we are sending a large number of packets this can starve RX because remaining budget will be low.

Let's give always the same budget for RX and TX clean.

While at it, check if we missed any interrupts while we were in NAPI callback by looking at DMA interrupt status.

Cc Joao Pinto <[email protected]>
Cc David S. Miller <[email protected]>
Cc Giuseppe Cavallaro <[email protected]>
Cc Alexandre Torgue <[email protected]>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 27 ++++++++++++-----------
 1 file changed, 14 insertions(+), 13 deletions(-)
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +14 -13
index 83ceb1a12e77..3f23e14891df 100644
@@ -3525,27 +3525,28 @@ static int stmmac_napi_poll(struct napi_struct *napi, int budget)
 	struct stmmac_channel *ch =
 		container_of(napi, struct stmmac_channel, napi);
 	struct stmmac_priv *priv = ch->priv_data;
-	int work_done = 0, work_rem = budget;
+	int work_done, rx_done = 0, tx_done = 0;
 	u32 chan = ch->index;
 
 	priv->xstats.napi_poll++;
 
-	if (ch->has_tx) {
-		int done = stmmac_tx_clean(priv, work_rem, chan);
+	if (ch->has_tx)
+		tx_done = stmmac_tx_clean(priv, budget, chan);
+	if (ch->has_rx)
+		rx_done = stmmac_rx(priv, budget, chan);
 
-		work_done += done;
-		work_rem -= done;
-	}
-
-	if (ch->has_rx) {
-		int done = stmmac_rx(priv, work_rem, chan);
+	work_done = max(rx_done, tx_done);
+	work_done = min(work_done, budget);
 
-		work_done += done;
-		work_rem -= done;
-	}
+	if (work_done < budget && napi_complete_done(napi, work_done)) {
+		int stat;
 
-	if (work_done < budget && napi_complete_done(napi, work_done))
 		stmmac_enable_dma_irq(priv, priv->ioaddr, chan);
+		stat = stmmac_dma_interrupt_status(priv, priv->ioaddr,
+						   &priv->xstats, chan);
+		if (stat && napi_reschedule(napi))
+			stmmac_disable_dma_irq(priv, priv->ioaddr, chan);
+	}
 
 	return work_done;
 }
Signature · 1 line
2.7.4