Hello Wolfgang,

today i got the information from a colleague from IAV regarding the peak_pci
driver that the sending traffic stopped reproducible on a SMP system with a
2.6.31 ...

As the PEAK driver did not produce this problem, he suggested to check the
changes between PEAK driver v6.11 and v6.13 having the Changelog:

"By preventing accidental back-to-back writes to the command register of the
SJA1000 chip in conjunction with multicore processors write stalls were
successfully removed"

See: http://www.peak-system.com/fileadmin/media/linux/index.htm

The change in the v6.13 PEAK driver only cares about the command register and
so i cooked the patch below that really fixed the issue in the setup of the
colleague.

In a 'real' patch it would probably make more sense to introduce a

     write_reg_guarded() or write_cmd_reg()

instead of checking for the command register value each time ...

Do you already know about this SJA1000 problem? Any thoughts about the patch?

Regards,
Oliver



Index: drivers/net/can/sja1000/peak_pci.c
===================================================================
--- drivers/net/can/sja1000/peak_pci.c  (Revision 1174)
+++ drivers/net/can/sja1000/peak_pci.c  (Arbeitskopie)
@@ -91,10 +91,20 @@
        return readb(priv->reg_base + (port << 2));
 }

-static void peak_pci_write_reg(const struct sja1000_priv *priv,
+static void peak_pci_write_reg(struct sja1000_priv *priv,
                               int port, u8 val)
 {
-       writeb(val, priv->reg_base + (port << 2));
+       if (port != REG_CMR)
+               writeb(val, priv->reg_base + (port << 2));
+       else {
+               /* The command register needs some locking in SMP */
+               unsigned long flags;
+
+               spin_lock_irqsave(&priv->wr_reg_lock, flags);
+               writeb(val, priv->reg_base + (REG_CMR << 2));
+               readb(priv->reg_base + (REG_SR << 2));
+               spin_unlock_irqrestore(&priv->wr_reg_lock, flags);
+       }
 }

 static void peak_pci_post_irq(const struct sja1000_priv *priv)
Index: drivers/net/can/sja1000/sja1000.h
===================================================================
--- drivers/net/can/sja1000/sja1000.h   (Revision 1174)
+++ drivers/net/can/sja1000/sja1000.h   (Arbeitskopie)
@@ -158,7 +158,7 @@

        /* the lower-layer is responsible for appropriate locking */
        u8 (*read_reg) (const struct sja1000_priv *priv, int reg);
-       void (*write_reg) (const struct sja1000_priv *priv, int reg, u8 val);
+       void (*write_reg) (struct sja1000_priv *priv, int reg, u8 val);
        void (*pre_irq) (const struct sja1000_priv *priv);
        void (*post_irq) (const struct sja1000_priv *priv);

@@ -168,6 +168,7 @@
        void __iomem *reg_base;  /* ioremap'ed address to registers */
        unsigned long irq_flags; /* for request_irq() */

+       spinlock_t wr_reg_lock; /* lock for concurrent register writes */
        u16 flags;              /* custom mode flags */
        u8 ocr;                 /* output control register */
        u8 cdr;                 /* clock divider register */

_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core

Reply via email to