Hi Jürgen,
Juergen Beisert wrote:
On Donnerstag, 10. Juli 2008, Grant Likely wrote:
On Thu, Jul 10, 2008 at 02:39:09PM +0200, Wolfram Sang wrote:
Hello,
today, I was debugging a kernel crash on a board with a MPC5200B using
2.6.26-rc9. I found the following code in drivers/net/fec_mpc52xx.c:
<snip>
I assume the proper thing to do is to set a flag in the ISR and handle
the soft reset later in some other context. Having never dealt with the
network core and its drivers so far, I am not sure which place would be
the right one to perform the soft reset. To not make things worse, I
hope people with more insight to network stuff can deliver a suitable
solution to this problem.
Thanks for the bug report. I'll take a look.
Some update:
Enabling XLB pipelining let occure this error less often. Kernel disables this
feature by default yet.
The comment talks about "cfr errate 292." that is valid for MPC5200A, but
_it_seems_ no longer for MPC5200B. Has anybody experience if we can enabling
pipelining on MPC5200B CPUs without triggering this bug?
No, I haven't...
We currently are playing with this setting:
Index: arch/powerpc/platforms/52xx/mpc52xx_common.c
===================================================================
--- arch/powerpc/platforms/52xx/mpc52xx_common.c.orig
+++ arch/powerpc/platforms/52xx/mpc52xx_common.c
@@ -99,11 +99,11 @@
out_be32(&xlb->master_pri_enable, 0xff);
out_be32(&xlb->master_priority, 0x11111111);
- /* Disable XLB pipelining
- * (cfr errate 292. We could do this only just before ATA PIO
- * transaction and re-enable it afterwards ...)
+ /*
+ * Enable pipelining, fixes FEC problems. The previous workaround seems
+ * not needed, as we have an MPC5200B (not A).
*/
- out_be32(&xlb->config, in_be32(&xlb->config) | MPC52xx_XLB_CFG_PLDIS);
+ out_be32(&xlb->config, in_be32(&xlb->config) & ~MPC52xx_XLB_CFG_PLDIS);
iounmap(xlb);
}
...but I prepared a patch to do the reset in the process context. Would be
nice if you could give the patch below a try.
Wolfgang.
===================================================================
From: Wolfgang Grandegger <[EMAIL PROTECTED]>
Subject: [PATCH] powerpc: fec_mpc52xx: Don't call mpc52xx_fec_reset() in ISR
Calling mpc52xx_fec_reset() in the ISR is a bug. This patch puts a task
for resetting the FEC into the kernel-global workqueue to be processed
lateron savely in process context.
Signed-off-by: Wolfgang Grandegger <[EMAIL PROTECTED]>
---
drivers/net/fec_mpc52xx.c | 31 +++++++++++++++++++++++++++----
1 file changed, 27 insertions(+), 4 deletions(-)
Index: linux-2.6.26/drivers/net/fec_mpc52xx.c
===================================================================
--- linux-2.6.26.orig/drivers/net/fec_mpc52xx.c
+++ linux-2.6.26/drivers/net/fec_mpc52xx.c
@@ -24,6 +24,7 @@
#include <linux/crc32.h>
#include <linux/hardirq.h>
#include <linux/delay.h>
+#include <linux/workqueue.h>
#include <linux/of_device.h>
#include <linux/of_platform.h>
@@ -57,6 +58,8 @@ struct mpc52xx_fec_priv {
struct bcom_task *tx_dmatsk;
spinlock_t lock;
int msg_enable;
+ struct work_struct reset_task;
+ struct net_device *ndev;
/* MDIO link details */
int phy_addr;
@@ -83,6 +86,19 @@ static int debug = -1; /* the above defa
module_param(debug, int, 0);
MODULE_PARM_DESC(debug, "debugging messages level");
+static void mpc52xx_fec_reset_task(struct work_struct *work)
+{
+ struct mpc52xx_fec_priv *priv =
+ container_of(work, struct mpc52xx_fec_priv, reset_task);
+ struct net_device *dev = priv->ndev;
+
+ dev_warn(&dev->dev, "deferred FEC reset due to errors\n");
+
+ mpc52xx_fec_reset(dev);
+
+ netif_wake_queue(dev);
+}
+
static void mpc52xx_fec_tx_timeout(struct net_device *dev)
{
dev_warn(&dev->dev, "transmit timed out\n");
@@ -355,6 +371,8 @@ static int mpc52xx_fec_close(struct net_
{
struct mpc52xx_fec_priv *priv = netdev_priv(dev);
+ flush_scheduled_work();
+
netif_stop_queue(dev);
mpc52xx_fec_stop(dev);
@@ -522,9 +540,9 @@ static irqreturn_t mpc52xx_fec_interrupt
if (net_ratelimit() && (ievent & FEC_IEVENT_XFIFO_ERROR))
dev_warn(&dev->dev, "FEC_IEVENT_XFIFO_ERROR\n");
- mpc52xx_fec_reset(dev);
-
- netif_wake_queue(dev);
+ netif_stop_queue(dev);
+ netif_carrier_off(dev);
+ schedule_work(&priv->reset_task);
return IRQ_HANDLED;
}
@@ -934,7 +952,9 @@ mpc52xx_fec_probe(struct of_device *op,
priv->t_irq = priv->r_irq = ndev->irq = NO_IRQ; /* IRQ are free for now
*/
- spin_lock_init(&priv->lock);
+ priv->ndev = ndev;
+ spin_lock_init(&priv->lock);
+ INIT_WORK(&priv->reset_task, mpc52xx_fec_reset_task);
/* ioremap the zones */
priv->fec = ioremap(mem.start, sizeof(struct mpc52xx_fec));
@@ -1068,6 +1088,9 @@ mpc52xx_fec_remove(struct of_device *op)
ndev = dev_get_drvdata(&op->dev);
priv = netdev_priv(ndev);
+ if (netif_running(ndev))
+ mpc52xx_fec_close(ndev);
+
unregister_netdev(ndev);
irq_dispose_mapping(ndev->irq);
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
