On 07.05.2010 12:29, Matthias Fuchs wrote:
Hi Oliver,
we have many many SJA1000 based boards and I never heard of a general problem
with back to back writes to the command register. Actually I won't invent
when patching the PEAK driver that way but please don't do so with the other
SJA1000
boards :-)
Also I did not see the situation where back-to-back writes to the command
register
happen.
Hi Matthias,
thanks for your opinion on this topic.
So far i also did not see this kind of problems. But the shown patch fixed the
problem of the write stalls with the BerliOS peak_pci.c driver in the setup
with the PEAK PCI card of my colleague. Maybe it is a timing issue with the
PCI adapter chip on the PEAK PCI card.
Maybe Klaus can give us a better insight, why the special treatment was only
necessary for the COMMAND register ?!?
Regards,
Oliver
On Wednesday 28 April 2010 20:10, Oliver Hartkopp wrote:
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
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core
