Message-ID: <20240924062100.2545714-1-msp@baylibre.com>
Date: Tue, 24 Sep 2024 08:16:22 +0200
From: Markus Schneider-Pargmann <msp@...libre.com>
To: Matthias Schiffer <matthias.schiffer@...tq-group.com>,
	Marc Kleine-Budde <mkl@...gutronix.de>
Cc: Chandrasekar Ramakrishnan <rcsekar@...sung.com>,
	Vincent Mailhol <mailhol.vincent@...adoo.fr>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>,
	Paolo Abeni <pabeni@...hat.com>,
	Martin Hundebøll <martin@...nix.com>,
	"Felipe Balbi (Intel)" <balbi@...nel.org>,
	Raymond Tan <raymond.tan@...el.com>,
	Jarkko Nikula <jarkko.nikula@...ux.intel.com>,
	linux-can@...r.kernel.org,
	netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	linux@...tq-group.com,
	Markus Schneider-Pargmann <msp@...libre.com>
Subject: [PATCH DO_NOT_APPLY] net: can: m_can: Support tcan level with edge interrupts

The tcan chip has a low level interrupt line that needs to be used.
There are some SoCs and components that do only support edge interrupts
on GPIOs. In the exact example someone wired the tcan chip to a am62
GPIO.

This patch creates a workaround for these situations, enabling the use
of tcan with a falling edge interrupt. Note that this is not the
preferred way to wire a tcan chip to the SoC.

I am detecting the situation by reading the IRQ type. If it is a level
interrupt everything stays the same. Otherwise these were my
considerations and solutions:

With falling edge interrupts we have following issues:
- While handling a IRQF_ONESHOT interrupt the interrupt may be masked as
  long as the interrupt is handled. So if a new interrupt hits during
  the handling of the interrupt may be lost as it is masked. With level
  interrupts that is not a problem because the interrupt line is still
  active/low after the handler is unmasked so it will jump back into
  handling interrupts afterwards. With edge interrupts we will just
  loose the interrupt at this point as we do not see the edge while the
  interrupt is masked. Solution here is to remove the IRQF_ONESHOT flag
  in case edge interrupts are used.
- Reading and clearing the interrupt register is not atomic. So the
  interrupts we clear from the interrupt register may not result in a
  completely cleared interrupt register and leave some unhandled
  interrupt. Again this is fine for level based interrupts as they will
  be causing a new call of the interrupt handler. With edge interrupts
  we will be missing this interrupt. So we need to make sure that the
  clearing of the interrupt register actually cleared it and the
  interrupt line could have gone back to inactive/high. To do that the
  interrupt register is read/cleared/handled repeatedly until it is 0.

Updating the interrupts for coalescing is only done once at the end with
all interrupts that were handled and not for every loop. We don't want
to change interrupts multiple times here.

Signed-off-by: Markus Schneider-Pargmann <msp@...libre.com>
---

This is the draft that I had for edge interrupts. For am62 I will create
a followup patch that covers minor things like IRQF_ONESHOT removal etc.

Best
Markus

 drivers/net/can/m_can/m_can.c | 114 +++++++++++++++++++++-------------
 drivers/net/can/m_can/m_can.h |   1 +
 2 files changed, 73 insertions(+), 42 deletions(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 663eb4247029..4b969f29ba55 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1208,6 +1208,7 @@ static void m_can_coalescing_update(struct m_can_classdev *cdev, u32 ir)
 static int m_can_interrupt_handler(struct m_can_classdev *cdev)
 {
 	struct net_device *dev = cdev->net;
+	u32 all_interrupts = 0;
 	u32 ir;
 	int ret;
 
@@ -1215,56 +1216,75 @@ static int m_can_interrupt_handler(struct m_can_classdev *cdev)
 		return IRQ_NONE;
 
 	ir = m_can_read(cdev, M_CAN_IR);
-	m_can_coalescing_update(cdev, ir);
-	if (!ir)
+	all_interrupts |= ir;
+	if (!ir) {
+		m_can_coalescing_update(cdev, 0);
 		return IRQ_NONE;
-
-	/* ACK all irqs */
-	m_can_write(cdev, M_CAN_IR, ir);
-
-	if (cdev->ops->clear_interrupts)
-		cdev->ops->clear_interrupts(cdev);
-
-	/* schedule NAPI in case of
-	 * - rx IRQ
-	 * - state change IRQ
-	 * - bus error IRQ and bus error reporting
-	 */
-	if (ir & (IR_RF0N | IR_RF0W | IR_ERR_ALL_30X)) {
-		cdev->irqstatus = ir;
-		if (!cdev->is_peripheral) {
-			m_can_disable_all_interrupts(cdev);
-			napi_schedule(&cdev->napi);
-		} else {
-			ret = m_can_rx_handler(dev, NAPI_POLL_WEIGHT, ir);
-			if (ret < 0)
-				return ret;
-		}
 	}
 
-	if (cdev->version == 30) {
-		if (ir & IR_TC) {
-			/* Transmission Complete Interrupt*/
-			u32 timestamp = 0;
-			unsigned int frame_len;
+	do {
+		/* ACK all irqs */
+		m_can_write(cdev, M_CAN_IR, ir);
 
-			if (cdev->is_peripheral)
-				timestamp = m_can_get_timestamp(cdev);
-			frame_len = m_can_tx_update_stats(cdev, 0, timestamp);
-			m_can_finish_tx(cdev, 1, frame_len);
+		if (cdev->ops->clear_interrupts)
+			cdev->ops->clear_interrupts(cdev);
+
+		/* schedule NAPI in case of
+		 * - rx IRQ
+		 * - state change IRQ
+		 * - bus error IRQ and bus error reporting
+		 */
+		if (ir & (IR_RF0N | IR_RF0W | IR_ERR_ALL_30X)) {
+			cdev->irqstatus = ir;
+			if (!cdev->is_peripheral) {
+				m_can_disable_all_interrupts(cdev);
+				napi_schedule(&cdev->napi);
+			} else {
+				ret = m_can_rx_handler(dev, NAPI_POLL_WEIGHT, ir);
+				if (ret < 0)
+					return ret;
+			}
 		}
-	} else  {
-		if (ir & (IR_TEFN | IR_TEFW)) {
-			/* New TX FIFO Element arrived */
-			ret = m_can_echo_tx_event(dev);
-			if (ret != 0)
-				return ret;
+
+		if (cdev->version == 30) {
+			if (ir & IR_TC) {
+				/* Transmission Complete Interrupt*/
+				u32 timestamp = 0;
+				unsigned int frame_len;
+
+				if (cdev->is_peripheral)
+					timestamp = m_can_get_timestamp(cdev);
+				frame_len = m_can_tx_update_stats(cdev, 0, timestamp);
+				m_can_finish_tx(cdev, 1, frame_len);
+			}
+		} else  {
+			if (ir & (IR_TEFN | IR_TEFW)) {
+				/* New TX FIFO Element arrived */
+				ret = m_can_echo_tx_event(dev);
+				if (ret != 0)
+					return ret;
+			}
 		}
-	}
+		if (!cdev->irq_type_edge)
+			break;
+
+
+		/* For edge interrupts we need to read the IR register again to
+		 * check that everything is cleared. If it is not, we can not
+		 * make sure the interrupt line is inactive again which is
+		 * required at this point to not miss any new interrupts. So in
+		 * case there are interrupts signaled in IR we repeat the
+		 * interrupt handling.
+		 */
+		ir = m_can_read(cdev, M_CAN_IR);
+		all_interrupts |= ir;
+	} while (ir);
 
 	if (cdev->is_peripheral)
 		can_rx_offload_threaded_irq_finish(&cdev->offload);
 
+	m_can_coalescing_update(cdev, all_interrupts);
+
 	return IRQ_HANDLED;
 }
 
@@ -2009,6 +2029,11 @@ static enum hrtimer_restart hrtimer_callback(struct hrtimer *timer)
 	return HRTIMER_RESTART;
 }
 
+static irqreturn_t m_can_hardirq(int irq, void *dev_id)
+{
+	return IRQ_WAKE_THREAD;
+}
+
 static int m_can_open(struct net_device *dev)
 {
 	struct m_can_classdev *cdev = netdev_priv(dev);
@@ -2034,6 +2059,9 @@ static int m_can_open(struct net_device *dev)
 
 	/* register interrupt handler */
 	if (cdev->is_peripheral) {
+		cdev->irq_type_edge = !(irq_get_trigger_type(dev->irq) &
+					IRQ_TYPE_LEVEL_MASK);
+
 		cdev->tx_wq = alloc_ordered_workqueue("mcan_wq",
 						      WQ_FREEZABLE | WQ_MEM_RECLAIM);
 		if (!cdev->tx_wq) {
@@ -2046,9 +2074,11 @@ static int m_can_open(struct net_device *dev)
 			INIT_WORK(&cdev->tx_ops[i].work, m_can_tx_work_queue);
 		}
 
-		err = request_threaded_irq(dev->irq, NULL, m_can_isr,
-					   IRQF_ONESHOT,
+		err = request_threaded_irq(dev->irq, m_can_hardirq, m_can_isr,
+					   (cdev->irq_type_edge ? 0 : IRQF_ONESHOT),
 					   dev->name, dev);
+		if (cdev->irq_type_edge)
+			netdev_info(dev, "Operating a level interrupt chip with an edge interrupt.\n");
 	} else if (dev->irq) {
 		err = request_irq(dev->irq, m_can_isr, IRQF_SHARED, dev->name,
 				  dev);
diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
index 3a9edc292593..17de56056352 100644
--- a/drivers/net/can/m_can/m_can.h
+++ b/drivers/net/can/m_can/m_can.h
@@ -99,6 +99,7 @@ struct m_can_classdev {
 	int pm_clock_support;
 	int pm_wake_source;
 	int is_peripheral;
+	bool irq_type_edge;
 
 	// Cached M_CAN_IE register content
 	u32 active_interrupts;
-- 
2.45.2

Powered by blists - more mailing lists