stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Marc Kleine-Budde <mkl@pengutronix.de>
To: netdev@vger.kernel.org, linux-can <linux-can@vger.kernel.org>
Cc: davem@davemloft.net, kernel@pengutronix.de,
	jhofstee@victronenergy.com,
	"Martin Hundebøll" <martin@geanix.com>,
	"Kurt Van Dijck" <dev.kurt@vandijck-laurijssen.be>,
	"Wolfgang Grandegger" <wg@grandegger.com>,
	"Joe Burmeister" <joe.burmeister@devtank.co.uk>,
	linux-stable <stable@vger.kernel.org>,
	"Marc Kleine-Budde" <mkl@pengutronix.de>
Subject: [PATCH 08/29] can: c_can: c_can_poll(): only read status register after status IRQ
Date: Thu, 10 Oct 2019 14:17:29 +0200	[thread overview]
Message-ID: <20191010121750.27237-9-mkl@pengutronix.de> (raw)
In-Reply-To: <20191010121750.27237-1-mkl@pengutronix.de>

From: Kurt Van Dijck <dev.kurt@vandijck-laurijssen.be>

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>
---
 drivers/net/can/c_can/c_can.c | 25 ++++++++++++++++++++-----
 drivers/net/can/c_can/c_can.h |  1 +
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index 606b7d8ffe13..9b61bfbea6cd 100644
--- 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 8acdc7fa4792..d5567a7c1c6d 100644
--- 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);
-- 
2.23.0


  parent reply	other threads:[~2019-10-10 12:18 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20191010121750.27237-1-mkl@pengutronix.de>
2019-10-10 12:17 ` [PATCH 01/29] can: dev: add missing of_node_put() after calling of_get_child_by_name() Marc Kleine-Budde
2019-10-10 12:17 ` [PATCH 02/29] can: gs_usb: gs_can_open(): prevent memory leak Marc Kleine-Budde
2019-10-10 12:17 ` [PATCH 03/29] can: mcba_usb: fix use-after-free on disconnect Marc Kleine-Budde
2019-10-10 12:17 ` [PATCH 04/29] can: usb_8dev: " Marc Kleine-Budde
2019-10-10 12:17 ` [PATCH 05/29] can: flexcan: disable completely the ECC mechanism Marc Kleine-Budde
2019-10-10 12:17 ` [PATCH 06/29] can: peak_usb: fix a potential out-of-sync while decoding packets Marc Kleine-Budde
2019-10-10 12:17 ` Marc Kleine-Budde [this message]
2019-10-10 12:17 ` [PATCH 12/29] can: rx-offload: can_rx_offload_queue_sorted(): fix error handling, avoid skb mem leak Marc Kleine-Budde

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191010121750.27237-9-mkl@pengutronix.de \
    --to=mkl@pengutronix.de \
    --cc=davem@davemloft.net \
    --cc=dev.kurt@vandijck-laurijssen.be \
    --cc=jhofstee@victronenergy.com \
    --cc=joe.burmeister@devtank.co.uk \
    --cc=kernel@pengutronix.de \
    --cc=linux-can@vger.kernel.org \
    --cc=martin@geanix.com \
    --cc=netdev@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=wg@grandegger.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).