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.

Matthias

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
> 
> 

-- 
-------------------------------------------------------------------------
Dipl.-Ing. Matthias Fuchs
Head of System Design

esd electronic system design gmbh
Vahrenwalder Str. 207 - 30165 Hannover - GERMANY
Phone: +49-511-37298-0 - Fax: +49-511-37298-68
Please visit our homepage http://www.esd.eu
Quality Products - Made in Germany
-------------------------------------------------------------------------
Geschäftsführer: Klaus Detering, Dr. Werner Schulze
Amtsgericht Hannover HRB 51373 - VAT-ID DE 115672832
-------------------------------------------------------------------------
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core

Reply via email to