can: c_can: c_can_poll(): only read status register after status IRQ

commit 3cb3eaac52c0f145d895f4b6c22834d5f02b8569 upstream.

When the status register is read without the status IRQ pending, the
chip may not raise the interrupt line for an upcoming status interrupt
and the driver may miss a status interrupt.

It is critical that the BUSOFF status interrupt is forwarded to the
higher layers, since no more interrupts will follow without
intervention.

Thanks to Wolfgang and Joe for bringing up the first idea.

Signed-off-by: Kurt Van Dijck <dev.kurt@vandijck-laurijssen.be>
Cc: Wolfgang Grandegger <wg@grandegger.com>
Cc: Joe Burmeister <joe.burmeister@devtank.co.uk>
Fixes: fa39b54ccf28 ("can: c_can: Get rid of pointless interrupts")
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index e3dccd3200d5d834f13ad036c290c51e3091052e..7d35f6737499cb5cdf48cea3e4cff66d1ca4f1a8 100644 (file)
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -97,6 +97,9 @@
 #define BTR_TSEG2_SHIFT                12
 #define BTR_TSEG2_MASK         (0x7 << BTR_TSEG2_SHIFT)
 
+/* interrupt register */
+#define INT_STS_PENDING                0x8000
+
 /* brp extension register */
 #define BRP_EXT_BRPE_MASK      0x0f
 #define BRP_EXT_BRPE_SHIFT     0
@@ -1029,10 +1032,16 @@ static int c_can_poll(struct napi_struct *napi, int quota)
        u16 curr, last = priv->last_status;
        int work_done = 0;
 
-       priv->last_status = curr = priv->read_reg(priv, C_CAN_STS_REG);
-       /* Ack status on C_CAN. D_CAN is self clearing */
-       if (priv->type != BOSCH_D_CAN)
-               priv->write_reg(priv, C_CAN_STS_REG, LEC_UNUSED);
+       /* Only read the status register if a status interrupt was pending */
+       if (atomic_xchg(&priv->sie_pending, 0)) {
+               priv->last_status = curr = priv->read_reg(priv, C_CAN_STS_REG);
+               /* Ack status on C_CAN. D_CAN is self clearing */
+               if (priv->type != BOSCH_D_CAN)
+                       priv->write_reg(priv, C_CAN_STS_REG, LEC_UNUSED);
+       } else {
+               /* no change detected ... */
+               curr = last;
+       }
 
        /* handle state changes */
        if ((curr & STATUS_EWARN) && (!(last & STATUS_EWARN))) {
@@ -1083,10 +1092,16 @@ static irqreturn_t c_can_isr(int irq, void *dev_id)
 {
        struct net_device *dev = (struct net_device *)dev_id;
        struct c_can_priv *priv = netdev_priv(dev);
+       int reg_int;
 
-       if (!priv->read_reg(priv, C_CAN_INT_REG))
+       reg_int = priv->read_reg(priv, C_CAN_INT_REG);
+       if (!reg_int)
                return IRQ_NONE;
 
+       /* save for later use */
+       if (reg_int & INT_STS_PENDING)
+               atomic_set(&priv->sie_pending, 1);
+
        /* disable all interrupts and schedule the NAPI */
        c_can_irq_control(priv, false);
        napi_schedule(&priv->napi);

diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
index 8acdc7fa4792f24438cb0cd84d73b44f0c46f7d8..d5567a7c1c6d4652a04f8135b381387e3a2e5304 100644 (file)
--- a/drivers/net/can/c_can/c_can.h
+++ b/drivers/net/can/c_can/c_can.h
@@ -198,6 +198,7 @@ struct c_can_priv {
        struct net_device *dev;
        struct device *device;
        atomic_t tx_active;
+       atomic_t sie_pending;
        unsigned long tx_dir;
        int last_status;
        u16 (*read_reg) (const struct c_can_priv *priv, enum reg index);