Skip to contentSkip to conversation

net: sh_eth: fix the rxdesc pointer when rx descriptor empty happens

PATCH Applied Shimoda, Yoshihiro 3 participants 6 messages May 29, 2012 – Jun 23, 2012

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 net: sh_eth: fix the rxdesc pointer when rx descriptor empty happens

When Receive Descriptor Empty happens, rxdesc pointer of the driver and actual next descriptor of the controller may be mismatch. This patch fixes it.

---
 drivers/net/ethernet/renesas/sh_eth.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)
drivers/net/ethernet/renesas/sh_eth.c +5 -3
index be3c221..667169b 100644
@@ -1101,8 +1101,12 @@ static int sh_eth_rx(struct net_device *ndev)
 
 	/* Restart Rx engine if stopped. */
 	/* If we don't need to check status, don't. -KDU */
-	if (!(sh_eth_read(ndev, EDRRR) & EDRRR_R))
+	if (!(sh_eth_read(ndev, EDRRR) & EDRRR_R)) {
+		/* fix the values for the next receiving */
+		mdp->cur_rx = mdp->dirty_rx = (sh_eth_read(ndev, RDFAR) -
+					       sh_eth_read(ndev, RDLAR)) >> 4;
 		sh_eth_write(ndev, EDRRR_R, EDRRR);
+	}
 
 	return 0;
 }
@@ -1199,8 +1203,6 @@ static void sh_eth_error(struct net_device *ndev, int intr_status)
 		/* Receive Descriptor Empty int */
 		ndev->stats.rx_over_errors++;
 
-		if (sh_eth_read(ndev, EDRRR) ^ EDRRR_R)
-			sh_eth_write(ndev, EDRRR_R, EDRRR);
 		if (netif_msg_rx_err(mdp))
 			dev_err(&ndev->dev, "Receive Descriptor Empty\n");
 	}
Signature · 1 line
1.7.1
#2 lore raw
In reply to Shimoda, Yoshihiro

Hello Shimoda-san

Quoted 3 lines · Yoshihiro

When Receive Descriptor Empty happens, rxdesc pointer of the driver and actual next descriptor of the controller may be mismatch. This patch fixes it.

Unfortunately, this patch breaks networking on ecovec (sh7724). Booting with dhcp and NFS-root progresses very slowly with lots of "nfs: server not responding / Ok" messages and never completes. Reverting it in current Linus' tree fixes the problem.

Thanks
Guennadi

Quoted 40 lines
---
 drivers/net/ethernet/renesas/sh_eth.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)
drivers/net/ethernet/renesas/sh_eth.c +5 -3
index be3c221..667169b 100644
@@ -1101,8 +1101,12 @@ static int sh_eth_rx(struct net_device *ndev)
 
 	/* Restart Rx engine if stopped. */
 	/* If we don't need to check status, don't. -KDU */
-	if (!(sh_eth_read(ndev, EDRRR) & EDRRR_R))
+	if (!(sh_eth_read(ndev, EDRRR) & EDRRR_R)) {
+		/* fix the values for the next receiving */
+		mdp->cur_rx = mdp->dirty_rx = (sh_eth_read(ndev, RDFAR) -
+					       sh_eth_read(ndev, RDLAR)) >> 4;
 		sh_eth_write(ndev, EDRRR_R, EDRRR);
+	}
 
 	return 0;
 }
@@ -1199,8 +1203,6 @@ static void sh_eth_error(struct net_device *ndev, int intr_status)
 		/* Receive Descriptor Empty int */
 		ndev->stats.rx_over_errors++;
 
-		if (sh_eth_read(ndev, EDRRR) ^ EDRRR_R)
-			sh_eth_write(ndev, EDRRR_R, EDRRR);
 		if (netif_msg_rx_err(mdp))
 			dev_err(&ndev->dev, "Receive Descriptor Empty\n");
 	}
Signature · 5 lines
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

#3 lore raw

Hello Guennadi-san,

Quoted 4 lines · Guennadi Liakhovetski

Hello Shimoda-san

On Tue, 29 May 2012, Shimoda, Yoshihiro wrote:

Quoted 8 lines
Quoted 3 lines

When Receive Descriptor Empty happens, rxdesc pointer of the driver and actual next descriptor of the controller may be mismatch. This patch fixes it.

Unfortunately, this patch breaks networking on ecovec (sh7724). Booting with dhcp and NFS-root progresses very slowly with lots of "nfs: server not responding / Ok" messages and never completes. Reverting it in current Linus' tree fixes the problem.

Thank you very much for the report.
The SH7724 doesn't set the RMCR register. So, the EDRRR will be clear after the controller receives a freme every time.
So, the "fix the rxdesc pointer" had to check a condition.

I wrote a patch for the issue as the following.
If possible, would you try the patch?

Best regards,
Yoshihiro Shimoda
---
Subject: [PATCH] net: sh_eth: fix the condition to fix the cur_tx/dirty_rx

The following commit couldn't work if the RMCR is not set to 1.

"net: sh_eth: fix the rxdesc pointer when rx descriptor empty happens" commit id 79fba9f51755c704c0a7d7b7f0df10874dc0a744

If RMCR is not set, the controller will clear the EDRRR after it received a frame. In this case, the driver doesn't need to fix the value of cur_rx/dirty_rx. The driver only needs it when the controll detects receive descriptors are empty.

---
 drivers/net/ethernet/renesas/sh_eth.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)
drivers/net/ethernet/renesas/sh_eth.c +6 -5
index 667169b..79bf09b 100644
@@ -1011,7 +1011,7 @@ static int sh_eth_txfree(struct net_device *ndev)
 }
 
 /* Packet receive function */
-static int sh_eth_rx(struct net_device *ndev)
+static int sh_eth_rx(struct net_device *ndev, u32 intr_status)
 {
 	struct sh_eth_private *mdp = netdev_priv(ndev);
 	struct sh_eth_rxdesc *rxdesc;
@@ -1102,9 +1102,11 @@ static int sh_eth_rx(struct net_device *ndev)
 	/* Restart Rx engine if stopped. */
 	/* If we don't need to check status, don't. -KDU */
 	if (!(sh_eth_read(ndev, EDRRR) & EDRRR_R)) {
-		/* fix the values for the next receiving */
-		mdp->cur_rx = mdp->dirty_rx = (sh_eth_read(ndev, RDFAR) -
-					       sh_eth_read(ndev, RDLAR)) >> 4;
+		/* fix the values for the next receiving if RDE is set */
+		if (intr_status & EESR_RDE)
+			mdp->cur_rx = mdp->dirty_rx +				(sh_eth_read(ndev, RDFAR) -
+				 sh_eth_read(ndev, RDLAR)) >> 4;
 		sh_eth_write(ndev, EDRRR_R, EDRRR);
 	}
 
@@ -1273,7 +1275,7 @@ static irqreturn_t sh_eth_interrupt(int irq, void *netdev)
 			EESR_RTSF | /* short frame recv */
 			EESR_PRE  | /* PHY-LSI recv error */
 			EESR_CERF)){ /* recv frame CRC error */
-		sh_eth_rx(ndev);
+		sh_eth_rx(ndev, intr_status);
 	}
 
 	/* Tx Check */
Signature · 1 line
1.7.1
#4 lore raw

Hi Shimoda-san

Quoted 16 lines · Yoshihiro

Hello Guennadi-san,

Quoted 4 lines · Guennadi Liakhovetski

Hello Shimoda-san

On Tue, 29 May 2012, Shimoda, Yoshihiro wrote:

Quoted 8 lines
Quoted 3 lines

When Receive Descriptor Empty happens, rxdesc pointer of the driver and actual next descriptor of the controller may be mismatch. This patch fixes it.

Unfortunately, this patch breaks networking on ecovec (sh7724). Booting with dhcp and NFS-root progresses very slowly with lots of "nfs: server not responding / Ok" messages and never completes. Reverting it in current Linus' tree fixes the problem.

Quoted 7 lines

Thank you very much for the report.
The SH7724 doesn't set the RMCR register. So, the EDRRR will be clear after the controller receives a freme every time.
So, the "fix the rxdesc pointer" had to check a condition.

I wrote a patch for the issue as the following.
If possible, would you try the patch?

Yes, it does seem to fix the problem.

Thanks
Guennadi

Quoted 60 lines

Best regards,
Yoshihiro Shimoda
---
Subject: [PATCH] net: sh_eth: fix the condition to fix the cur_tx/dirty_rx

The following commit couldn't work if the RMCR is not set to 1.

"net: sh_eth: fix the rxdesc pointer when rx descriptor empty happens" commit id 79fba9f51755c704c0a7d7b7f0df10874dc0a744

If RMCR is not set, the controller will clear the EDRRR after it received a frame. In this case, the driver doesn't need to fix the value of cur_rx/dirty_rx. The driver only needs it when the controll detects receive descriptors are empty.

---
 drivers/net/ethernet/renesas/sh_eth.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)
drivers/net/ethernet/renesas/sh_eth.c +6 -5
index 667169b..79bf09b 100644
@@ -1011,7 +1011,7 @@ static int sh_eth_txfree(struct net_device *ndev)
 }
 
 /* Packet receive function */
-static int sh_eth_rx(struct net_device *ndev)
+static int sh_eth_rx(struct net_device *ndev, u32 intr_status)
 {
 	struct sh_eth_private *mdp = netdev_priv(ndev);
 	struct sh_eth_rxdesc *rxdesc;
@@ -1102,9 +1102,11 @@ static int sh_eth_rx(struct net_device *ndev)
 	/* Restart Rx engine if stopped. */
 	/* If we don't need to check status, don't. -KDU */
 	if (!(sh_eth_read(ndev, EDRRR) & EDRRR_R)) {
-		/* fix the values for the next receiving */
-		mdp->cur_rx = mdp->dirty_rx = (sh_eth_read(ndev, RDFAR) -
-					       sh_eth_read(ndev, RDLAR)) >> 4;
+		/* fix the values for the next receiving if RDE is set */
+		if (intr_status & EESR_RDE)
+			mdp->cur_rx = mdp->dirty_rx > +				(sh_eth_read(ndev, RDFAR) -
+				 sh_eth_read(ndev, RDLAR)) >> 4;
 		sh_eth_write(ndev, EDRRR_R, EDRRR);
 	}
 
@@ -1273,7 +1275,7 @@ static irqreturn_t sh_eth_interrupt(int irq, void *netdev)
 			EESR_RTSF | /* short frame recv */
 			EESR_PRE  | /* PHY-LSI recv error */
 			EESR_CERF)){ /* recv frame CRC error */
-		sh_eth_rx(ndev);
+		sh_eth_rx(ndev, intr_status);
 	}
 
 	/* Tx Check */
Signature · 1 line
1.7.1

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

maintainer Applied
#5 lore raw

From: Guennadi Liakhovetski <[email protected]>
Date: Fri, 22 Jun 2012 16:16:38 +0200 (CEST)

Quoted 25 lines

Hi Shimoda-san

Quoted 21 lines · Yoshihiro

Hello Guennadi-san,

Quoted 4 lines · Guennadi Liakhovetski

Hello Shimoda-san

On Tue, 29 May 2012, Shimoda, Yoshihiro wrote:

Quoted 8 lines
Quoted 3 lines

When Receive Descriptor Empty happens, rxdesc pointer of the driver and actual next descriptor of the controller may be mismatch. This patch fixes it.

Unfortunately, this patch breaks networking on ecovec (sh7724). Booting with dhcp and NFS-root progresses very slowly with lots of "nfs: server not responding / Ok" messages and never completes. Reverting it in current Linus' tree fixes the problem.

Thank you very much for the report.
The SH7724 doesn't set the RMCR register. So, the EDRRR will be clear after the controller receives a freme every time.
So, the "fix the rxdesc pointer" had to check a condition.

Quoted 6 lines
Quoted 2 lines

I wrote a patch for the issue as the following.
If possible, would you try the patch?

Yes, it does seem to fix the problem.

Applied, thanks.