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