Re: [RFC v2 0/7] pch_can/c_can: fix races and add PCH support to c_can

[Alexander Stein <alexander.stein@systec-electronic.com>]

Hello Wolfgang and others,

On Thursday 29 November 2012 15:39:40, Wolfgang Grandegger wrote:
> here is v2 of my patches for the C_CAN drivers.
> 
> For Michael I have prepared out-of-tree driver sources allowing to
> easily build the drivers also for older 3.x kernel versions. More
> tester are welcome.
> 
> Changes since v1:
> 
> - use init callback after renaming it from initram
> - use different sets of interface registers for rx and tx (like PCH_CAN)
> - use spin_[un]lock_bh to protect the tx objects
> 
> Wolfgang Grandegger (7):
>   pch_can: add spinlocks to protect tx objects
>   c_can: rename callback "initram" to "init" to more general usage
>   c_can: use different sets of interface registers for rx and tx
>   c_can_pci: introduce board specific PCI bar
>   c_can_pci: enable PCI bus master only for MSI
>   c_can_pci: add support for PCH CAN on Intel EG20T PCH
>   c_can: add spinlock to protect tx and  objects
> 
>  drivers/net/can/c_can/c_can.c          |   66 
+++++++++++++++++++++-----------
>  drivers/net/can/c_can/c_can.h          |    3 +-
>  drivers/net/can/c_can/c_can_pci.c      |   56 +++++++++++++++++++++++++--
>  drivers/net/can/c_can/c_can_platform.c |    2 +-
>  drivers/net/can/pch_can.c              |    9 +++++
>  5 files changed, 108 insertions(+), 28 deletions(-)

I backported the c_can patches incl. your patchset to v3.0.31 and tested this 
driver on our own custom atom board using the eg20t pch. CAN in general works, 
but if I run my heavy CAN load testcase I get errors sometimes.
This test works as follows: I send a CAN message to 2 other CAN nodes 
configuring some timings (like burst length or time between each can frame) 
and they send 250000 messages each containing a counter. This way I can detect 
any missing or switched message with a high bus load.
If I use the described software state alone it works, but if I run 'watch 
sensors' in a different ssh session, CAN start to misbehave like missing CAN 
frames or switched order. It seems that I2C usage on the PCH influences the 
CAN part also:(
Even worse, if I use the following patch to check if PCI writes were 
successfully, I notices that some writes (or the consecutive read) don't 
succeed. And I also get lots of I2C timeouts waiting for a xfer complete.

Does anybody have an idea what's going wrong here? Is somebody able to see the 
same problems on their hardware?

Wolfgang: Compared to your v8 c_can drivers for Michael, I'm just missing the 
const bitrate table and don't use pci_register_driver, as I have cherry-picked 
the module_pci_driver patches.

Best regards,
Alexander

diff --git a/drivers/net/can/c_can/c_can_pci.c 
b/drivers/net/can/c_can/c_can_pci.c
index 2516ea9..b124ea5 100644
--- a/drivers/net/can/c_can/c_can_pci.c
+++ b/drivers/net/can/c_can/c_can_pci.c
@@ -80,7 +80,13 @@ static u16 c_can_pci_read_reg_32bit(struct c_can_priv 
*priv,
 static void c_can_pci_write_reg_32bit(struct c_can_priv *priv,
                                      enum reg index, u16 val)
 {
+       u16 reg;
        iowrite32((u32)val, priv->base + 2 * priv->regs[index]);
+       reg = c_can_pci_read_reg_32bit(priv, index);
+       if (reg != val)
+       {
+               netdev_err(priv->dev, "write 0x%x to offset 0x%x failed. got: 
0x%x\n", val, 2 * priv->regs[index], reg);
+       }
 }
 
 static void c_can_pci_reset_pch(const struct c_can_priv *priv, bool enable)