On 13.05.2010 20:51, Klaus Hitschler wrote:

> 
> Currently I think it is a matter of SMP systems. If the context switches 
> become faster it may be an issue of UP processors, too.
> 
> I have not seen the behaviour if the SJA1000 is accessed via ioports. ioport 
> accesses seem to be slow enough.
> 
> BTW this behaviour is often seen when chips designed for slow embedded 
> hardware are used as peripheral for fast processors. It is a characteristic 
> symptom that something is not simple usable on SMP systems if the driver's 
> code shows small delays between register accesses and the chip's registers 
> are 
> accessed concurrently inside other code paths. 
> 
> I think my kind of workaround may work with both UP and SMP systems. 


That also my impression.

It took a while to create a minimum invasive patch that doesn't touch all the
drivers, but here it is:

Index: sja1000/sja1000.c
===================================================================
--- sja1000/sja1000.c   (Revision 1177)
+++ sja1000/sja1000.c   (Arbeitskopie)
@@ -88,6 +88,29 @@
        .brp_inc = 1,
 };

+
+static void sja1000_write_cmd_reg(struct sja1000_priv *priv, u8 val)
+{
+       /* the command register needs some locking in SMP systems */
+
+#ifdef CONFIG_SMP
+
+       unsigned long flags;
+
+       spin_lock_irqsave(&priv->wr_reg_lock, flags);
+       priv->write_reg(priv, REG_CMR, val);
+       priv->read_reg(priv, REG_SR);
+       spin_unlock_irqrestore(&priv->wr_reg_lock, flags);
+
+#else
+
+       /* write to the command register without locking */
+       priv->write_reg(priv, REG_CMR, val);
+
+#endif
+}
+
+
 static int sja1000_probe_chip(struct net_device *dev)
 {
        struct sja1000_priv *priv = netdev_priv(dev);
@@ -305,7 +328,7 @@

        can_put_echo_skb(skb, dev, 0);

-       priv->write_reg(priv, REG_CMR, CMD_TR);
+       sja1000_write_cmd_reg(priv, CMD_TR);

        return NETDEV_TX_OK;
 }
@@ -358,7 +381,7 @@
        cf->can_id = id;

        /* release receive buffer */
-       priv->write_reg(priv, REG_CMR, CMD_RRB);
+       sja1000_write_cmd_reg(priv, CMD_RRB);

        netif_rx(skb);

@@ -393,7 +416,7 @@
                cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
                stats->rx_over_errors++;
                stats->rx_errors++;
-               priv->write_reg(priv, REG_CMR, CMD_CDO);        /* clear bit */
+               sja1000_write_cmd_reg(priv, CMD_CDO);   /* clear bit */
        }

        if (isrc & IRQ_EI) {
Index: sja1000/sja1000.h
===================================================================
--- sja1000/sja1000.h   (Revision 1177)
+++ sja1000/sja1000.h   (Arbeitskopie)
@@ -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 */


Any Acked-by or Signed-off-by out there?

Regards,
Oliver

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

Reply via email to