From 2ea8bc70cba76048f02e7fadd6c7ccdbb7f0d316 Mon Sep 17 00:00:00 2001
From: norly <ny-git@enpas.org>
Date: Wed, 16 Mar 2022 21:16:27 +0100
Subject: [PATCH 01/16] Re-add author names

---
 module/elmcan.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/module/elmcan.c b/module/elmcan.c
index 198ef51..cfcbf2f 100644
--- a/module/elmcan.c
+++ b/module/elmcan.c
@@ -4,6 +4,11 @@
  * This driver started as a derivative of linux/drivers/net/can/slcan.c
  * and my thanks go to the original authors for their inspiration, even
  * after almost none of their code is left.
+ *
+ * elmcan.c Author : Max Staudt <max-linux@enpas.org>
+ * slcan.c Author  : Oliver Hartkopp <socketcan@hartkopp.net>
+ * slip.c Authors  : Laurence Culhane <loz@holmes.demon.co.uk>
+ *                   Fred N. van Kempen <waltje@uwalt.nl.mugnet.org>
  */
 
 #define pr_fmt(fmt) "[elmcan] " fmt
-- 
2.30.2


From 0eb2f39be87a38965050fa90933486d0b712e80d Mon Sep 17 00:00:00 2001
From: norly <ny-git@enpas.org>
Date: Wed, 16 Mar 2022 21:26:06 +0100
Subject: [PATCH 02/16] Remove braces from pr_fmt

---
 module/elmcan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/module/elmcan.c b/module/elmcan.c
index cfcbf2f..7b22728 100644
--- a/module/elmcan.c
+++ b/module/elmcan.c
@@ -11,7 +11,7 @@
  *                   Fred N. van Kempen <waltje@uwalt.nl.mugnet.org>
  */
 
-#define pr_fmt(fmt) "[elmcan] " fmt
+#define pr_fmt(fmt) "elmcan: " fmt
 
 #include <linux/init.h>
 #include <linux/module.h>
-- 
2.30.2


From b81986ae35699dce8b54cba956af2dcd91241eb8 Mon Sep 17 00:00:00 2001
From: norly <ny-git@enpas.org>
Date: Wed, 16 Mar 2022 21:26:25 +0100
Subject: [PATCH 03/16] Move MODULE_* to end of file

---
 module/elmcan.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/module/elmcan.c b/module/elmcan.c
index 7b22728..31aa8fc 100644
--- a/module/elmcan.c
+++ b/module/elmcan.c
@@ -42,11 +42,6 @@
 #include <linux/can/led.h>
 #include <linux/can/rx-offload.h>
 
-MODULE_ALIAS_LDISC(N_DEVELOPMENT);
-MODULE_DESCRIPTION("ELM327 based CAN interface");
-MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Max Staudt <max-linux@enpas.org>");
-
 /* Line discipline ID number.
  * N_DEVELOPMENT will likely be defined from Linux 5.18 onwards:
  * https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/commit/?h=tty-next&id=c2faf737abfb10f88f2d2612d573e9edc3c42c37
@@ -1287,3 +1282,8 @@ static void __exit elmcan_exit(void)
 
 module_init(elmcan_init);
 module_exit(elmcan_exit);
+
+MODULE_ALIAS_LDISC(N_DEVELOPMENT);
+MODULE_DESCRIPTION("ELM327 based CAN interface");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Max Staudt <max-linux@enpas.org>");
-- 
2.30.2


From af305c7448da594fed5d6fc26490d4b87774098b Mon Sep 17 00:00:00 2001
From: norly <ny-git@enpas.org>
Date: Wed, 16 Mar 2022 21:32:20 +0100
Subject: [PATCH 04/16] Unify indentation in struct elmcan

---
 module/elmcan.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/module/elmcan.c b/module/elmcan.c
index 31aa8fc..106ae10 100644
--- a/module/elmcan.c
+++ b/module/elmcan.c
@@ -89,11 +89,11 @@ struct elmcan {
 	struct can_rx_offload offload;
 
 	/* TTY and netdev devices that we're bridging */
-	struct tty_struct	*tty;
-	struct net_device	*dev;
+	struct tty_struct *tty;
+	struct net_device *dev;
 
 	/* Per-channel lock */
-	spinlock_t		lock;
+	spinlock_t lock;
 
 	/* Keep track of how many things are using this struct.
 	 * Once it reaches 0, we are in the process of cleaning up,
@@ -102,18 +102,18 @@ struct elmcan {
 	 * decrement to 0, and refcount_dec() spills a WARN_ONCE in
 	 * that case.
 	 */
-	atomic_t		refcount;
+	atomic_t refcount;
 
 	/* Stop the channel on hardware failure.
 	 * Once this is true, nothing will be sent to the TTY.
 	 */
-	bool			hw_failure;
+	bool hw_failure;
 
 	/* TTY TX helpers */
-	struct work_struct	tx_work;	/* Flushes TTY TX buffer   */
-	unsigned char		*txbuf;
-	unsigned char		*txhead;	/* Pointer to next TX byte */
-	int			txleft;		/* Bytes left to TX */
+	struct work_struct tx_work;	/* Flushes TTY TX buffer   */
+	unsigned char *txbuf;
+	unsigned char *txhead;		/* Pointer to next TX byte */
+	int txleft;			/* Bytes left to TX */
 
 	/* TTY RX helpers */
 	unsigned char rxbuf[ELM327_SIZE_RXBUF];
-- 
2.30.2


From 489643c2ad25482d526687138d6ada04bd3dbe2a Mon Sep 17 00:00:00 2001
From: norly <ny-git@enpas.org>
Date: Wed, 16 Mar 2022 22:15:01 +0100
Subject: [PATCH 05/16] Rename ELM_ to ELM327_STATE_

---
 module/elmcan.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/module/elmcan.c b/module/elmcan.c
index 106ae10..ed652de 100644
--- a/module/elmcan.c
+++ b/module/elmcan.c
@@ -121,10 +121,10 @@ struct elmcan {
 
 	/* State machine */
 	enum {
-		ELM_NOTINIT = 0,
-		ELM_GETDUMMYCHAR,
-		ELM_GETPROMPT,
-		ELM_RECEIVING,
+		ELM327_STATE_NOTINIT = 0,
+		ELM327_STATE_GETDUMMYCHAR,
+		ELM327_STATE_GETPROMPT,
+		ELM327_STATE_RECEIVING,
 	} state;
 
 	int drop_next_line;
@@ -193,10 +193,11 @@ static void elm327_send(struct elmcan *elm, const void *buf, size_t len)
  */
 static void elm327_kick_into_cmd_mode(struct elmcan *elm)
 {
-	if (elm->state != ELM_GETDUMMYCHAR && elm->state != ELM_GETPROMPT) {
+	if (elm->state != ELM327_STATE_GETDUMMYCHAR &&
+	    elm->state != ELM327_STATE_GETPROMPT) {
 		elm327_send(elm, ELM327_DUMMY_STRING, 1);
 
-		elm->state = ELM_GETDUMMYCHAR;
+		elm->state = ELM327_STATE_GETDUMMYCHAR;
 	}
 }
 
@@ -267,7 +268,7 @@ static char *elm327_init_script[] = {
 
 static void elm327_init(struct elmcan *elm)
 {
-	elm->state = ELM_NOTINIT;
+	elm->state = ELM327_STATE_NOTINIT;
 	elm->can_frame_to_send.can_id = 0x7df; /* ELM327 HW default */
 	elm->rxfill = 0;
 	elm->drop_next_line = 0;
@@ -563,7 +564,7 @@ static void elm327_parse_line(struct elmcan *elm, size_t len)
 	}
 
 	/* Regular parsing */
-	if (elm->state == ELM_RECEIVING
+	if (elm->state == ELM327_STATE_RECEIVING
 	    && elm327_parse_frame(elm, len)) {
 		/* Parse an error line. */
 		elm327_parse_error(elm, len);
@@ -582,7 +583,7 @@ static void elm327_handle_prompt(struct elmcan *elm)
 	if (!elm->cmds_todo) {
 		/* Enter CAN monitor mode */
 		elm327_send(elm, "ATMA\r", 5);
-		elm->state = ELM_RECEIVING;
+		elm->state = ELM327_STATE_RECEIVING;
 
 		return;
 	}
@@ -644,7 +645,7 @@ static void elm327_handle_prompt(struct elmcan *elm)
 		}
 
 		elm->drop_next_line = 1;
-		elm->state = ELM_RECEIVING;
+		elm->state = ELM327_STATE_RECEIVING;
 	}
 
 	elm327_send(elm, local_txbuf, strlen(local_txbuf));
@@ -672,17 +673,17 @@ static void elm327_parse_rxbuf(struct elmcan *elm)
 	int i;
 
 	switch (elm->state) {
-	case ELM_NOTINIT:
+	case ELM327_STATE_NOTINIT:
 		elm->rxfill = 0;
 		break;
 
-	case ELM_GETDUMMYCHAR:
+	case ELM327_STATE_GETDUMMYCHAR:
 	{
 		/* Wait for 'y' or '>' */
 		for (i = 0; i < elm->rxfill; i++) {
 			if (elm->rxbuf[i] == ELM327_DUMMY_CHAR) {
 				elm327_send(elm, "\r", 1);
-				elm->state = ELM_GETPROMPT;
+				elm->state = ELM327_STATE_GETPROMPT;
 				i++;
 				break;
 			} else if (elm327_is_ready_char(elm->rxbuf[i])) {
@@ -697,7 +698,7 @@ static void elm327_parse_rxbuf(struct elmcan *elm)
 		break;
 	}
 
-	case ELM_GETPROMPT:
+	case ELM327_STATE_GETPROMPT:
 		/* Wait for '>' */
 		if (elm327_is_ready_char(elm->rxbuf[elm->rxfill - 1]))
 			elm327_handle_prompt(elm);
@@ -705,7 +706,7 @@ static void elm327_parse_rxbuf(struct elmcan *elm)
 		elm->rxfill = 0;
 		break;
 
-	case ELM_RECEIVING:
+	case ELM327_STATE_RECEIVING:
 		/* Find <CR> delimiting feedback lines. */
 		for (len = 0;
 		     (len < elm->rxfill) && (elm->rxbuf[len] != '\r');
-- 
2.30.2


From 858a6a0ffa6af2bfa6c567f796f6f0565e777cef Mon Sep 17 00:00:00 2001
From: norly <ny-git@enpas.org>
Date: Wed, 16 Mar 2022 22:29:58 +0100
Subject: [PATCH 06/16] Clean up types and unused vars in struct elmcan

---
 module/elmcan.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/module/elmcan.c b/module/elmcan.c
index ed652de..dd8889b 100644
--- a/module/elmcan.c
+++ b/module/elmcan.c
@@ -127,16 +127,14 @@ struct elmcan {
 		ELM327_STATE_RECEIVING,
 	} state;
 
-	int drop_next_line;
+	bool drop_next_line;
 
 	/* The CAN frame and config the ELM327 is sending/using,
 	 * or will send/use after finishing all cmds_todo
 	 */
 	struct can_frame can_frame_to_send;
-	unsigned short can_config;
-	unsigned long can_bitrate;
-	unsigned char can_bitrate_divisor;
-	int silent_monitoring;
+	u16 can_config;
+	u8 can_bitrate_divisor;
 
 	/* Things we have yet to send */
 	char **next_init_cmd;
-- 
2.30.2


From 6cce55d96e2888876a17b841e21a7133c576d210 Mon Sep 17 00:00:00 2001
From: norly <ny-git@enpas.org>
Date: Wed, 16 Mar 2022 22:34:01 +0100
Subject: [PATCH 07/16] Convert comments to lockdep_assert_held()

---
 module/elmcan.c | 52 +++++++++++++++++++++++++++----------------------
 1 file changed, 29 insertions(+), 23 deletions(-)

diff --git a/module/elmcan.c b/module/elmcan.c
index dd8889b..fd38475 100644
--- a/module/elmcan.c
+++ b/module/elmcan.c
@@ -25,6 +25,7 @@
 #include <linux/if_ether.h>
 #include <linux/kernel.h>
 #include <linux/list.h>
+#include <linux/lockdep.h>
 #include <linux/netdevice.h>
 #include <linux/skbuff.h>
 #include <linux/spinlock.h>
@@ -150,11 +151,12 @@ static DEFINE_SPINLOCK(elmcan_discdata_lock);
 
 static inline void elm327_hw_failure(struct elmcan *elm);
 
-/* Assumes elm->lock taken. */
 static void elm327_send(struct elmcan *elm, const void *buf, size_t len)
 {
 	int actual;
 
+	lockdep_assert_held(elm->lock);
+
 	if (elm->hw_failure)
 		return;
 
@@ -186,11 +188,11 @@ static void elm327_send(struct elmcan *elm, const void *buf, size_t len)
  * We send ELM327_DUMMY_CHAR which will either abort any running
  * operation, or be echoed back to us in case we're already in command
  * mode.
- *
- * Assumes elm->lock taken.
  */
 static void elm327_kick_into_cmd_mode(struct elmcan *elm)
 {
+	lockdep_assert_held(elm->lock);
+
 	if (elm->state != ELM327_STATE_GETDUMMYCHAR &&
 	    elm->state != ELM327_STATE_GETPROMPT) {
 		elm327_send(elm, ELM327_DUMMY_STRING, 1);
@@ -199,12 +201,11 @@ static void elm327_kick_into_cmd_mode(struct elmcan *elm)
 	}
 }
 
-/* Schedule a CAN frame and necessary config changes to be sent to the TTY.
- *
- * Assumes elm->lock taken.
- */
+/* Schedule a CAN frame and necessary config changes to be sent to the TTY. */
 static void elm327_send_frame(struct elmcan *elm, struct can_frame *frame)
 {
+	lockdep_assert_held(elm->lock);
+
 	/* Schedule any necessary changes in ELM327's CAN configuration */
 	if (elm->can_frame_to_send.can_id != frame->can_id) {
 		/* Set the new CAN ID for transmission. */
@@ -238,16 +239,13 @@ static void elm327_send_frame(struct elmcan *elm, struct can_frame *frame)
 	elm327_kick_into_cmd_mode(elm);
 }
 
-/* ELM327 initialization sequence.
- *
- * Assumes elm->lock taken.
- */
+/* ELM327 initialisation sequence. */
 static char *elm327_init_script[] = {
 	"AT WS\r",        /* v1.0: Warm Start */
 	"AT PP FF OFF\r", /* v1.0: All Programmable Parameters Off */
 	"AT M0\r",        /* v1.0: Memory Off */
 	"AT AL\r",        /* v1.0: Allow Long messages */
-	"AT BI\r",        /* v1.0: Bypass Initialization */
+	"AT BI\r",        /* v1.0: Bypass Initialisation */
 	"AT CAF0\r",      /* v1.0: CAN Auto Formatting Off */
 	"AT CFC0\r",      /* v1.0: CAN Flow Control Off */
 	"AT CF 000\r",    /* v1.0: Reset CAN ID Filter */
@@ -266,6 +264,8 @@ static char *elm327_init_script[] = {
 
 static void elm327_init(struct elmcan *elm)
 {
+	lockdep_assert_held(elm->lock);
+
 	elm->state = ELM327_STATE_NOTINIT;
 	elm->can_frame_to_send.can_id = 0x7df; /* ELM327 HW default */
 	elm->rxfill = 0;
@@ -291,10 +291,11 @@ static void elm327_init(struct elmcan *elm)
 	elm327_kick_into_cmd_mode(elm);
 }
 
-/* Assumes elm->lock taken. */
 static void elm327_feed_frame_to_netdev(struct elmcan *elm,
 					struct sk_buff *skb)
 {
+	lockdep_assert_held(elm->lock);
+
 	if (!netif_running(elm->dev))
 		return;
 
@@ -310,14 +311,14 @@ static void elm327_feed_frame_to_netdev(struct elmcan *elm,
 #endif
 }
 
-/* Called when we're out of ideas and just want it all to end.
- * Assumes elm->lock taken.
- */
+/* Called when we're out of ideas and just want it all to end. */
 static inline void elm327_hw_failure(struct elmcan *elm)
 {
 	struct can_frame *frame;
 	struct sk_buff *skb;
 
+	lockdep_assert_held(elm->lock);
+
 	elm->hw_failure = true;
 
 	elm->can.can_stats.bus_off++;
@@ -354,12 +355,13 @@ static inline int _len_memstrcmp(const u8 *mem, size_t mem_len, const char *str)
 	return (mem_len != str_len) || memcmp(mem, str, str_len);
 }
 
-/* Assumes elm->lock taken. */
 static void elm327_parse_error(struct elmcan *elm, size_t len)
 {
 	struct can_frame *frame;
 	struct sk_buff *skb;
 
+	lockdep_assert_held(elm->lock);
+
 	skb = alloc_can_err_skb(elm->dev, &frame);
 	if (!skb)
 		/* It's okay to return here:
@@ -416,8 +418,6 @@ static void elm327_parse_error(struct elmcan *elm, size_t len)
  * Instead of a payload, RTR indicates a remote request.
  *
  * We will use the spaces and line length to guess the format.
- *
- * Assumes elm->lock taken.
  */
 static int elm327_parse_frame(struct elmcan *elm, size_t len)
 {
@@ -427,6 +427,8 @@ static int elm327_parse_frame(struct elmcan *elm, size_t len)
 	int datastart;
 	int i;
 
+	lockdep_assert_held(elm->lock);
+
 	skb = alloc_can_skb(elm->dev, &frame);
 	if (!skb)
 		return -ENOMEM;
@@ -546,9 +548,10 @@ static int elm327_parse_frame(struct elmcan *elm, size_t len)
 	return 0;
 }
 
-/* Assumes elm->lock taken. */
 static void elm327_parse_line(struct elmcan *elm, size_t len)
 {
+	lockdep_assert_held(elm->lock);
+
 	/* Skip empty lines */
 	if (!len)
 		return;
@@ -572,12 +575,13 @@ static void elm327_parse_line(struct elmcan *elm, size_t len)
 	}
 }
 
-/* Assumes elm->lock taken. */
 static void elm327_handle_prompt(struct elmcan *elm)
 {
 	struct can_frame *frame = &elm->can_frame_to_send;
 	char local_txbuf[20];
 
+	lockdep_assert_held(elm->lock);
+
 	if (!elm->cmds_todo) {
 		/* Enter CAN monitor mode */
 		elm327_send(elm, "ATMA\r", 5);
@@ -657,19 +661,21 @@ static bool elm327_is_ready_char(char c)
 	return (c & 0x3f) == ELM327_READY_CHAR;
 }
 
-/* Assumes elm->lock taken. */
 static void elm327_drop_bytes(struct elmcan *elm, size_t i)
 {
+	lockdep_assert_held(elm->lock);
+
 	memmove(&elm->rxbuf[0], &elm->rxbuf[i], ELM327_SIZE_RXBUF - i);
 	elm->rxfill -= i;
 }
 
-/* Assumes elm->lock taken. */
 static void elm327_parse_rxbuf(struct elmcan *elm)
 {
 	size_t len;
 	int i;
 
+	lockdep_assert_held(elm->lock);
+
 	switch (elm->state) {
 	case ELM327_STATE_NOTINIT:
 		elm->rxfill = 0;
-- 
2.30.2


From 521a2bdc04a021a0e58bd093097b751951caf408 Mon Sep 17 00:00:00 2001
From: norly <ny-git@enpas.org>
Date: Thu, 17 Mar 2022 01:32:39 +0100
Subject: [PATCH 08/16] Minor cleanups

---
 module/elmcan.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/module/elmcan.c b/module/elmcan.c
index fd38475..a75ddf1 100644
--- a/module/elmcan.c
+++ b/module/elmcan.c
@@ -209,8 +209,8 @@ static void elm327_send_frame(struct elmcan *elm, struct can_frame *frame)
 	/* Schedule any necessary changes in ELM327's CAN configuration */
 	if (elm->can_frame_to_send.can_id != frame->can_id) {
 		/* Set the new CAN ID for transmission. */
-		if ((frame->can_id & CAN_EFF_FLAG)
-		    ^ (elm->can_frame_to_send.can_id & CAN_EFF_FLAG)) {
+		if ((frame->can_id & CAN_EFF_FLAG) ^
+		    (elm->can_frame_to_send.can_id & CAN_EFF_FLAG)) {
 			elm->can_config = (frame->can_id & CAN_EFF_FLAG
 						? 0
 						: ELM327_CAN_CONFIG_SEND_SFF)
@@ -469,7 +469,6 @@ static int elm327_parse_frame(struct elmcan *elm, size_t len)
 		frame->can_id = CAN_EFF_FLAG;
 		datastart = 14;
 	} else if (elm->rxbuf[3] == ' ' && elm->rxbuf[5] == ' ') {
-		frame->can_id = 0;
 		datastart = 6;
 	} else {
 		/* This is not a well-formatted data line.
-- 
2.30.2


From e3bbac3f9ae12f56602547bdb4057f9c8dc545ee Mon Sep 17 00:00:00 2001
From: norly <ny-git@enpas.org>
Date: Thu, 17 Mar 2022 21:11:11 +0100
Subject: [PATCH 09/16] Drop superflouous elm->can.state = CAN_STATE_STOPPED;

---
 module/elmcan.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/module/elmcan.c b/module/elmcan.c
index a75ddf1..95a8206 100644
--- a/module/elmcan.c
+++ b/module/elmcan.c
@@ -1117,7 +1117,6 @@ static int elmcan_ldisc_open(struct tty_struct *tty)
 	INIT_WORK(&elm->tx_work, elmcan_ldisc_tx_worker);
 
 	/* Configure CAN metadata */
-	elm->can.state = CAN_STATE_STOPPED;
 	elm->can.bitrate_const = elmcan_bitrate_const;
 	elm->can.bitrate_const_cnt = ARRAY_SIZE(elmcan_bitrate_const);
 	elm->can.do_set_bittiming = elmcan_do_set_bittiming;
-- 
2.30.2


From 6b25b028af5f6a6ff0d96841f8679db60e0ffbe7 Mon Sep 17 00:00:00 2001
From: norly <ny-git@enpas.org>
Date: Thu, 17 Mar 2022 21:14:59 +0100
Subject: [PATCH 10/16] Add can_dropped_invalid_skb() to
 elmcan_netdev_start_xmit()

---
 module/elmcan.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/module/elmcan.c b/module/elmcan.c
index 95a8206..9a58655 100644
--- a/module/elmcan.c
+++ b/module/elmcan.c
@@ -847,6 +847,9 @@ static netdev_tx_t elmcan_netdev_start_xmit(struct sk_buff *skb,
 	struct elmcan *elm = netdev_priv(dev);
 	struct can_frame *frame = (struct can_frame *)skb->data;
 
+	if (can_dropped_invalid_skb(dev, skb))
+		return NETDEV_TX_OK;
+
 	/* BHs are already disabled, so no spin_lock_bh().
 	 * See Documentation/networking/netdevices.txt
 	 */
-- 
2.30.2


From 7553f57af5a80c80dcb7957d1347a06dcc580805 Mon Sep 17 00:00:00 2001
From: norly <ny-git@enpas.org>
Date: Thu, 17 Mar 2022 21:23:10 +0100
Subject: [PATCH 11/16] Get rid of dummy mailbox_read() for rx_offload

Use can_rx_offload_add_manual() instead of can_rx_offload_add_fifo(),
starting with Linux 5.10.
---
 module/elmcan.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/module/elmcan.c b/module/elmcan.c
index 9a58655..81d21f4 100644
--- a/module/elmcan.c
+++ b/module/elmcan.c
@@ -755,6 +755,7 @@ static void elm327_parse_rxbuf(struct elmcan *elm)
 	}
 }
 
+#if LINUX_VERSION_CODE < KERNEL_VERSION(5,10,0)
 /* Dummy needed to use can_rx_offload */
 static struct sk_buff *elmcan_mailbox_read(struct can_rx_offload *offload,
 					   unsigned int n, u32 *timestamp,
@@ -764,6 +765,7 @@ static struct sk_buff *elmcan_mailbox_read(struct can_rx_offload *offload,
 
 	return ERR_PTR(-ENOBUFS);
 }
+#endif
 
 static int elmcan_netdev_open(struct net_device *dev)
 {
@@ -792,8 +794,12 @@ static int elmcan_netdev_open(struct net_device *dev)
 	elm327_init(elm);
 	spin_unlock_bh(&elm->lock);
 
+#if LINUX_VERSION_CODE < KERNEL_VERSION(5,10,0)
 	elm->offload.mailbox_read = elmcan_mailbox_read;
 	err = can_rx_offload_add_fifo(dev, &elm->offload, ELM327_NAPI_WEIGHT);
+#else
+	err = can_rx_offload_add_manual(dev, &elm->offload, ELM327_NAPI_WEIGHT);
+#endif
 	if (err) {
 		close_candev(dev);
 		return err;
-- 
2.30.2


From 3e68b297c23584d52a0d51b63516018cb90faf55 Mon Sep 17 00:00:00 2001
From: norly <ny-git@enpas.org>
Date: Wed, 27 Apr 2022 00:22:03 +0200
Subject: [PATCH 12/16] Simplify TTY sending code and locking

The great get_elm() dance was unnecessary, since the TTY layer already
synchronises *all* calls to ldisc_ops with tty_ldisc_ref() and
tty_ldisc_lock().

All that we have to ensure is that the worker does not race against
close(), which is trivial if we clear TTY_DO_WRITE_WAKEUP and then flush
the worker.

Also, the worker no longer checks whether the netdev is up - all it
needs to do is to write out the TTY buffer, and wake up the netdev when
the buffer is empty.
---
 module/elmcan.c | 175 +++++++++++-------------------------------------
 1 file changed, 40 insertions(+), 135 deletions(-)

diff --git a/module/elmcan.c b/module/elmcan.c
index 81d21f4..75f1359 100644
--- a/module/elmcan.c
+++ b/module/elmcan.c
@@ -96,15 +96,6 @@ struct elmcan {
 	/* Per-channel lock */
 	spinlock_t lock;
 
-	/* Keep track of how many things are using this struct.
-	 * Once it reaches 0, we are in the process of cleaning up,
-	 * and new operations will be cancelled immediately.
-	 * Use atomic_t rather than refcount_t because we deliberately
-	 * decrement to 0, and refcount_dec() spills a WARN_ONCE in
-	 * that case.
-	 */
-	atomic_t refcount;
-
 	/* Stop the channel on hardware failure.
 	 * Once this is true, nothing will be sent to the TTY.
 	 */
@@ -112,12 +103,12 @@ struct elmcan {
 
 	/* TTY TX helpers */
 	struct work_struct tx_work;	/* Flushes TTY TX buffer   */
-	unsigned char *txbuf;
-	unsigned char *txhead;		/* Pointer to next TX byte */
-	int txleft;			/* Bytes left to TX */
+	u8 *txbuf;			/* Pointer to our TX buffer */
+	u8 *txhead;			/* Pointer to next TX byte */
+	unsigned txleft;		/* Bytes left to TX */
 
 	/* TTY RX helpers */
-	unsigned char rxbuf[ELM327_SIZE_RXBUF];
+	u8 rxbuf[ELM327_SIZE_RXBUF];
 	int rxfill;
 
 	/* State machine */
@@ -142,18 +133,11 @@ struct elmcan {
 	unsigned long cmds_todo;
 };
 
-/* A lock for all tty->disc_data handled by this ldisc.
- * This is to prevent a case where tty->disc_data is set to NULL,
- * yet someone is still trying to dereference it.
- * Without this, we cannot do a clean shutdown.
- */
-static DEFINE_SPINLOCK(elmcan_discdata_lock);
-
 static inline void elm327_hw_failure(struct elmcan *elm);
 
 static void elm327_send(struct elmcan *elm, const void *buf, size_t len)
 {
-	int actual;
+	int written;
 
 	lockdep_assert_held(elm->lock);
 
@@ -162,17 +146,8 @@ static void elm327_send(struct elmcan *elm, const void *buf, size_t len)
 
 	memcpy(elm->txbuf, buf, len);
 
-	/* Order of next two lines is *very* important.
-	 * When we are sending a little amount of data,
-	 * the transfer may be completed inside the ops->write()
-	 * routine, because it's running with interrupts enabled.
-	 * In this case we *never* got WRITE_WAKEUP event,
-	 * if we did not request it before write operation.
-	 *       14 Oct 1994  Dmitry Gorodchanin.
-	 */
-	set_bit(TTY_DO_WRITE_WAKEUP, &elm->tty->flags);
-	actual = elm->tty->ops->write(elm->tty, elm->txbuf, len);
-	if (actual < 0) {
+	written = elm->tty->ops->write(elm->tty, elm->txbuf, len);
+	if (written < 0) {
 		netdev_err(elm->dev,
 			   "Failed to write to tty %s.\n",
 			   elm->tty->name);
@@ -180,8 +155,13 @@ static void elm327_send(struct elmcan *elm, const void *buf, size_t len)
 		return;
 	}
 
-	elm->txleft = len - actual;
-	elm->txhead = elm->txbuf + actual;
+	elm->txleft = len - written;
+	elm->txhead = elm->txbuf + written;
+
+	if (!elm->txleft)
+		netif_wake_queue(elm->dev);
+	else
+		set_bit(TTY_DO_WRITE_WAKEUP, &elm->tty->flags);
 }
 
 /* Take the ELM327 out of almost any state and back into command mode.
@@ -895,39 +875,6 @@ static const struct net_device_ops elmcan_netdev_ops = {
 	.ndo_change_mtu	= can_change_mtu,
 };
 
-/* Get a reference to our struct, taking into account locks/refcounts.
- * This is to ensure ordering in case we are shutting down, and to ensure
- * there is a refcount at all (otherwise tty->disc_data may be freed and
- * before we increment the refcount).
- * Use this for anything that can race against elmcan_ldisc_close().
- */
-static struct elmcan *get_elm(struct tty_struct *tty)
-{
-	struct elmcan *elm;
-	bool got_ref;
-
-	spin_lock_bh(&elmcan_discdata_lock);
-	elm = (struct elmcan *)tty->disc_data;
-
-	if (!elm) {
-		spin_unlock_bh(&elmcan_discdata_lock);
-		return NULL;
-	}
-
-	got_ref = atomic_inc_not_zero(&elm->refcount);
-	spin_unlock_bh(&elmcan_discdata_lock);
-
-	if (!got_ref)
-		return NULL;
-
-	return elm;
-}
-
-static void put_elm(struct elmcan *elm)
-{
-	atomic_dec(&elm->refcount);
-}
-
 static bool elmcan_is_valid_rx_char(char c)
 {
 	return (isdigit(c) ||
@@ -956,10 +903,7 @@ static void elmcan_ldisc_rx(struct tty_struct *tty,
 			    const unsigned char *cp, const char *fp, int count)
 #endif
 {
-	struct elmcan *elm = get_elm(tty);
-
-	if (!elm)
-		return;
+	struct elmcan *elm = (struct elmcan *)tty->disc_data;
 
 	spin_lock_bh(&elm->lock);
 
@@ -1011,7 +955,6 @@ static void elmcan_ldisc_rx(struct tty_struct *tty,
 
 out:
 	spin_unlock_bh(&elm->lock);
-	put_elm(elm);
 }
 
 /* Write out remaining transmit buffer.
@@ -1019,59 +962,44 @@ out:
  */
 static void elmcan_ldisc_tx_worker(struct work_struct *work)
 {
-	/* No need to use get_elm() here, as we'll always flush workers
-	 * before destroying the elmcan object.
-	 */
 	struct elmcan *elm = container_of(work, struct elmcan, tx_work);
-	ssize_t actual;
+	ssize_t written;
 
-	spin_lock_bh(&elm->lock);
-	if (elm->hw_failure) {
-		spin_unlock_bh(&elm->lock);
+	if (elm->hw_failure)
 		return;
-	}
 
-	if (!elm->tty || !netif_running(elm->dev)) {
-		spin_unlock_bh(&elm->lock);
-		return;
+	spin_lock_bh(&elm->lock);
+
+	if (elm->txleft) {
+		written = elm->tty->ops->write(elm->tty, elm->txhead, elm->txleft);
+		if (written < 0) {
+			netdev_err(elm->dev,
+				   "Failed to write to tty %s.\n",
+				   elm->tty->name);
+			elm327_hw_failure(elm);
+			spin_unlock_bh(&elm->lock);
+			return;
+		} else {
+			elm->txleft -= written;
+			elm->txhead += written;
+		}
 	}
 
-	if (elm->txleft <= 0)  {
-		/* Our TTY write buffer is empty:
-		 * Allow netdev to hand us another packet
-		 */
+	if (!elm->txleft)  {
 		clear_bit(TTY_DO_WRITE_WAKEUP, &elm->tty->flags);
 		spin_unlock_bh(&elm->lock);
 		netif_wake_queue(elm->dev);
-		return;
-	}
-
-	actual = elm->tty->ops->write(elm->tty, elm->txhead, elm->txleft);
-	if (actual < 0) {
-		netdev_err(elm->dev,
-			   "Failed to write to tty %s.\n",
-			   elm->tty->name);
-		elm327_hw_failure(elm);
+	} else {
 		spin_unlock_bh(&elm->lock);
-		return;
 	}
-
-	elm->txleft -= actual;
-	elm->txhead += actual;
-	spin_unlock_bh(&elm->lock);
 }
 
 /* Called by the driver when there's room for more data. */
 static void elmcan_ldisc_tx_wakeup(struct tty_struct *tty)
 {
-	struct elmcan *elm = get_elm(tty);
-
-	if (!elm)
-		return;
+	struct elmcan *elm = (struct elmcan *)tty->disc_data;
 
 	schedule_work(&elm->tx_work);
-
-	put_elm(elm);
 }
 
 /* ELM327 can only handle bitrates that are integer divisors of 500 kHz,
@@ -1122,7 +1050,6 @@ static int elmcan_ldisc_open(struct tty_struct *tty)
 	tty->receive_room = 65536; /* We don't flow control */
 	elm->txleft = 0; /* Clear TTY TX buffer */
 	spin_lock_init(&elm->lock);
-	atomic_set(&elm->refcount, 1);
 	INIT_WORK(&elm->tx_work, elmcan_ldisc_tx_worker);
 
 	/* Configure CAN metadata */
@@ -1165,26 +1092,13 @@ out_err:
  */
 static void elmcan_ldisc_close(struct tty_struct *tty)
 {
-	struct elmcan *elm = get_elm(tty);
-
-	if (!elm)
-		return;
+	struct elmcan *elm = (struct elmcan *)tty->disc_data;
 
 	/* unregister_netdev() calls .ndo_stop() so we don't have to. */
 	unregister_candev(elm->dev);
 
-	/* Decrease the refcount twice, once for our own get_elm(),
-	 * and once to remove the count of 1 that we set in _open().
-	 * Once it reaches 0, we can safely destroy it.
-	 */
-	put_elm(elm);
-	put_elm(elm);
-
-	while (atomic_read(&elm->refcount) > 0)
-		msleep_interruptible(10);
-
-	/* At this point, all ldisc calls to us have become no-ops. */
-
+	/* Ensure that our worker won't be rescheduled */
+	clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
 	flush_work(&elm->tx_work);
 
 	/* Mark channel as dead */
@@ -1217,29 +1131,20 @@ static int elmcan_ldisc_ioctl(struct tty_struct *tty,
 #endif
 			      unsigned int cmd, unsigned long arg)
 {
-	struct elmcan *elm = get_elm(tty);
+	struct elmcan *elm = (struct elmcan *)tty->disc_data;
 	unsigned int tmp;
 
-	if (!elm)
-		return -EINVAL;
-
 	switch (cmd) {
 	case SIOCGIFNAME:
 		tmp = strnlen(elm->dev->name, IFNAMSIZ - 1) + 1;
-		if (copy_to_user((void __user *)arg, elm->dev->name, tmp)) {
-			put_elm(elm);
+		if (copy_to_user((void __user *)arg, elm->dev->name, tmp))
 			return -EFAULT;
-		}
-
-		put_elm(elm);
 		return 0;
 
 	case SIOCSIFHWADDR:
-		put_elm(elm);
 		return -EINVAL;
 
 	default:
-		put_elm(elm);
 #if LINUX_VERSION_CODE < KERNEL_VERSION(5,16,0)
 		return tty_mode_ioctl(tty, file, cmd, arg);
 #else
-- 
2.30.2


From 7733766253b5d880a1a63ee7957449ad9e390c05 Mon Sep 17 00:00:00 2001
From: norly <ny-git@enpas.org>
Date: Wed, 27 Apr 2022 00:33:15 +0200
Subject: [PATCH 13/16] Remove .hangup()

We were just calling .close(), which the TTY layer does for us anyway.
And it does so with the correct locking.
---
 module/elmcan.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/module/elmcan.c b/module/elmcan.c
index 75f1359..7bb2784 100644
--- a/module/elmcan.c
+++ b/module/elmcan.c
@@ -1113,18 +1113,6 @@ static void elmcan_ldisc_close(struct tty_struct *tty)
 	free_candev(elm->dev);
 }
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(5,16,0)
-static int elmcan_ldisc_hangup(struct tty_struct *tty)
-#else
-static void elmcan_ldisc_hangup(struct tty_struct *tty)
-#endif
-{
-	elmcan_ldisc_close(tty);
-#if LINUX_VERSION_CODE < KERNEL_VERSION(5,16,0)
-	return 0;
-#endif
-}
-
 static int elmcan_ldisc_ioctl(struct tty_struct *tty,
 #if LINUX_VERSION_CODE < KERNEL_VERSION(5,17,0)
 			      struct file *file,
@@ -1161,7 +1149,6 @@ static struct tty_ldisc_ops elmcan_ldisc = {
 	.write_wakeup	= elmcan_ldisc_tx_wakeup,
 	.open		= elmcan_ldisc_open,
 	.close		= elmcan_ldisc_close,
-	.hangup		= elmcan_ldisc_hangup,
 	.ioctl		= elmcan_ldisc_ioctl,
 };
 
-- 
2.30.2


From cf7dd7b99f1553ee042ba31ee602f37691d7d8a3 Mon Sep 17 00:00:00 2001
From: norly <ny-git@enpas.org>
Date: Wed, 27 Apr 2022 00:49:30 +0200
Subject: [PATCH 14/16] Clarify memory/string comparisons

We really can't use strncmp() here. Sorry!
---
 module/elmcan.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/module/elmcan.c b/module/elmcan.c
index 7bb2784..35f194e 100644
--- a/module/elmcan.c
+++ b/module/elmcan.c
@@ -316,23 +316,21 @@ static inline void elm327_hw_failure(struct elmcan *elm)
 	elm327_feed_frame_to_netdev(elm, skb);
 }
 
-/* Compare a buffer to a fixed string */
-static inline int _memstrcmp(const u8 *mem, const char *str)
-{
-	return memcmp(mem, str, strlen(str));
-}
-
 /* Compare buffer to string length, then compare buffer to fixed string.
  * This ensures two things:
  *  - It flags cases where the fixed string is only the start of the
  *    buffer, rather than exactly all of it.
  *  - It avoids byte comparisons in case the length doesn't match.
+ *
+ * strncmp() cannot be used here because it accepts the following wrong case:
+ *   strncmp("CAN ER", "CAN ERROR", 6);
+ * This must fail, hence this helper function.
  */
-static inline int _len_memstrcmp(const u8 *mem, size_t mem_len, const char *str)
+static inline int check_len_then_cmp(const u8 *mem, size_t mem_len, const char *str)
 {
 	size_t str_len = strlen(str);
 
-	return (mem_len != str_len) || memcmp(mem, str, str_len);
+	return (mem_len == str_len) && !memcmp(mem, str, str_len);
 }
 
 static void elm327_parse_error(struct elmcan *elm, size_t len)
@@ -350,29 +348,29 @@ static void elm327_parse_error(struct elmcan *elm, size_t len)
 		return;
 
 	/* Filter possible error messages based on length of RX'd line */
-	if (!_len_memstrcmp(elm->rxbuf, len, "UNABLE TO CONNECT")) {
+	if (check_len_then_cmp(elm->rxbuf, len, "UNABLE TO CONNECT")) {
 		netdev_err(elm->dev,
 			   "ELM327 reported UNABLE TO CONNECT. Please check your setup.\n");
-	} else if (!_len_memstrcmp(elm->rxbuf, len, "BUFFER FULL")) {
+	} else if (check_len_then_cmp(elm->rxbuf, len, "BUFFER FULL")) {
 		/* This will only happen if the last data line was complete.
 		 * Otherwise, elm327_parse_frame() will heuristically
 		 * emit this kind of error frame instead.
 		 */
 		frame->can_id |= CAN_ERR_CRTL;
 		frame->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
-	} else if (!_len_memstrcmp(elm->rxbuf, len, "BUS ERROR")) {
+	} else if (check_len_then_cmp(elm->rxbuf, len, "BUS ERROR")) {
 		frame->can_id |= CAN_ERR_BUSERROR;
-	} else if (!_len_memstrcmp(elm->rxbuf, len, "CAN ERROR")) {
+	} else if (check_len_then_cmp(elm->rxbuf, len, "CAN ERROR")) {
 		frame->can_id |= CAN_ERR_PROT;
-	} else if (!_len_memstrcmp(elm->rxbuf, len, "<RX ERROR")) {
+	} else if (check_len_then_cmp(elm->rxbuf, len, "<RX ERROR")) {
 		frame->can_id |= CAN_ERR_PROT;
-	} else if (!_len_memstrcmp(elm->rxbuf, len, "BUS BUSY")) {
+	} else if (check_len_then_cmp(elm->rxbuf, len, "BUS BUSY")) {
 		frame->can_id |= CAN_ERR_PROT;
 		frame->data[2] = CAN_ERR_PROT_OVERLOAD;
-	} else if (!_len_memstrcmp(elm->rxbuf, len, "FB ERROR")) {
+	} else if (check_len_then_cmp(elm->rxbuf, len, "FB ERROR")) {
 		frame->can_id |= CAN_ERR_PROT;
 		frame->data[2] = CAN_ERR_PROT_TX;
-	} else if (len == 5 && !_memstrcmp(elm->rxbuf, "ERR")) {
+	} else if (len == 5 && !memcmp(elm->rxbuf, "ERR", 3)) {
 		/* ERR is followed by two digits, hence line length 5 */
 		netdev_err(elm->dev, "ELM327 reported an ERR%c%c. Please power it off and on again.\n",
 			   elm->rxbuf[3], elm->rxbuf[4]);
@@ -489,7 +487,7 @@ static int elm327_parse_frame(struct elmcan *elm, size_t len)
 
 	/* Check for RTR frame */
 	if (elm->rxfill >= hexlen + 3 &&
-	    !_memstrcmp(&elm->rxbuf[hexlen], "RTR")) {
+	    !memcmp(&elm->rxbuf[hexlen], "RTR", 3)) {
 		frame->can_id |= CAN_RTR_FLAG;
 	}
 
@@ -539,7 +537,7 @@ static void elm327_parse_line(struct elmcan *elm, size_t len)
 	if (elm->drop_next_line) {
 		elm->drop_next_line = 0;
 		return;
-	} else if (!_memstrcmp(elm->rxbuf, "AT")) {
+	} else if (!memcmp(elm->rxbuf, "AT", 2)) {
 		return;
 	}
 
-- 
2.30.2


From eb2866d3b45253bba4a654201ca062e6ae387647 Mon Sep 17 00:00:00 2001
From: norly <ny-git@enpas.org>
Date: Wed, 27 Apr 2022 00:56:11 +0200
Subject: [PATCH 15/16] elm327_handle_prompt(): Explain size of local buffer

Also use snprintf() instead of sprintf()/strcpy(), to clarify the
size restriction.
---
 module/elmcan.c | 44 +++++++++++++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 13 deletions(-)

diff --git a/module/elmcan.c b/module/elmcan.c
index 35f194e..9c19d76 100644
--- a/module/elmcan.c
+++ b/module/elmcan.c
@@ -219,7 +219,9 @@ static void elm327_send_frame(struct elmcan *elm, struct can_frame *frame)
 	elm327_kick_into_cmd_mode(elm);
 }
 
-/* ELM327 initialisation sequence. */
+/* ELM327 initialisation sequence.
+ * The line length is limited by the buffer in elm327_handle_prompt().
+ */
 static char *elm327_init_script[] = {
 	"AT WS\r",        /* v1.0: Warm Start */
 	"AT PP FF OFF\r", /* v1.0: All Programmable Parameters Off */
@@ -555,7 +557,11 @@ static void elm327_parse_line(struct elmcan *elm, size_t len)
 static void elm327_handle_prompt(struct elmcan *elm)
 {
 	struct can_frame *frame = &elm->can_frame_to_send;
-	char local_txbuf[20];
+	/* Size this buffer for the largest ELM327 line we may generate,
+	 * which is currently an 8 byte CAN frame's payload hexdump.
+	 * Items in elm327_init_script must fit here, too!
+	 */
+	char local_txbuf[sizeof("0102030405060708\r")];
 
 	lockdep_assert_held(elm->lock);
 
@@ -569,7 +575,9 @@ static void elm327_handle_prompt(struct elmcan *elm)
 
 	/* Reconfigure ELM327 step by step as indicated by elm->cmds_todo */
 	if (test_bit(ELM327_TX_DO_INIT, &elm->cmds_todo)) {
-		strcpy(local_txbuf, *elm->next_init_cmd);
+		snprintf(local_txbuf, sizeof(local_txbuf),
+			"%s",
+			*elm->next_init_cmd);
 
 		elm->next_init_cmd++;
 		if (!(*elm->next_init_cmd)) {
@@ -578,31 +586,38 @@ static void elm327_handle_prompt(struct elmcan *elm)
 		}
 
 	} else if (test_and_clear_bit(ELM327_TX_DO_SILENT_MONITOR, &elm->cmds_todo)) {
-		sprintf(local_txbuf, "ATCSM%i\r",
+		snprintf(local_txbuf, sizeof(local_txbuf),
+			"ATCSM%i\r",
 			!(!(elm->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)));
 
 	} else if (test_and_clear_bit(ELM327_TX_DO_RESPONSES, &elm->cmds_todo)) {
-		sprintf(local_txbuf, "ATR%i\r",
+		snprintf(local_txbuf, sizeof(local_txbuf),
+			"ATR%i\r",
 			!(elm->can.ctrlmode & CAN_CTRLMODE_LISTENONLY));
 
 	} else if (test_and_clear_bit(ELM327_TX_DO_CAN_CONFIG, &elm->cmds_todo)) {
-		sprintf(local_txbuf, "ATPC\r");
+		snprintf(local_txbuf, sizeof(local_txbuf),
+			"ATPC\r");
 		set_bit(ELM327_TX_DO_CAN_CONFIG_PART2, &elm->cmds_todo);
 
 	} else if (test_and_clear_bit(ELM327_TX_DO_CAN_CONFIG_PART2, &elm->cmds_todo)) {
-		sprintf(local_txbuf, "ATPB%04X\r",
+		snprintf(local_txbuf, sizeof(local_txbuf),
+			"ATPB%04X\r",
 			elm->can_config);
 
 	} else if (test_and_clear_bit(ELM327_TX_DO_CANID_29BIT_HIGH, &elm->cmds_todo)) {
-		sprintf(local_txbuf, "ATCP%02X\r",
+		snprintf(local_txbuf, sizeof(local_txbuf),
+			"ATCP%02X\r",
 			(frame->can_id & CAN_EFF_MASK) >> 24);
 
 	} else if (test_and_clear_bit(ELM327_TX_DO_CANID_29BIT_LOW, &elm->cmds_todo)) {
-		sprintf(local_txbuf, "ATSH%06X\r",
+		snprintf(local_txbuf, sizeof(local_txbuf),
+			"ATSH%06X\r",
 			frame->can_id & CAN_EFF_MASK & ((1 << 24) - 1));
 
 	} else if (test_and_clear_bit(ELM327_TX_DO_CANID_11BIT, &elm->cmds_todo)) {
-		sprintf(local_txbuf, "ATSH%03X\r",
+		snprintf(local_txbuf, sizeof(local_txbuf),
+			"ATSH%03X\r",
 			frame->can_id & CAN_SFF_MASK);
 
 	} else if (test_and_clear_bit(ELM327_TX_DO_CAN_DATA, &elm->cmds_todo)) {
@@ -610,17 +625,20 @@ static void elm327_handle_prompt(struct elmcan *elm)
 			/* Send an RTR frame. Their DLC is fixed.
 			 * Some chips don't send them at all.
 			 */
-			sprintf(local_txbuf, "ATRTR\r");
+			snprintf(local_txbuf, sizeof(local_txbuf),
+				"ATRTR\r");
 		} else {
 			/* Send a regular CAN data frame */
 			int i;
 
 			for (i = 0; i < frame->len; i++) {
-				sprintf(&local_txbuf[2 * i], "%02X",
+				snprintf(&local_txbuf[2 * i], sizeof(local_txbuf),
+					"%02X",
 					frame->data[i]);
 			}
 
-			sprintf(&local_txbuf[2 * i], "\r");
+			snprintf(&local_txbuf[2 * i], sizeof(local_txbuf),
+				"\r");
 		}
 
 		elm->drop_next_line = 1;
-- 
2.30.2


From 553218bcfa7d4dbea97a0c1aa104720aa188d15b Mon Sep 17 00:00:00 2001
From: norly <ny-git@enpas.org>
Date: Wed, 27 Apr 2022 19:26:58 +0200
Subject: [PATCH 16/16] Rename hw_failure to uart_side_failure for easier
 understanding

---
 module/elmcan.c | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/module/elmcan.c b/module/elmcan.c
index 9c19d76..6a7b951 100644
--- a/module/elmcan.c
+++ b/module/elmcan.c
@@ -96,10 +96,12 @@ struct elmcan {
 	/* Per-channel lock */
 	spinlock_t lock;
 
-	/* Stop the channel on hardware failure.
+	/* Stop the channel on UART side hardware failure, e.g. stray
+	 * characters or neverending lines. This may be caused by bad
+	 * UART wiring, a bad ELM327, a bad UART bridge...
 	 * Once this is true, nothing will be sent to the TTY.
 	 */
-	bool hw_failure;
+	bool uart_side_failure;
 
 	/* TTY TX helpers */
 	struct work_struct tx_work;	/* Flushes TTY TX buffer   */
@@ -133,7 +135,7 @@ struct elmcan {
 	unsigned long cmds_todo;
 };
 
-static inline void elm327_hw_failure(struct elmcan *elm);
+static inline void elm327_uart_side_failure(struct elmcan *elm);
 
 static void elm327_send(struct elmcan *elm, const void *buf, size_t len)
 {
@@ -141,7 +143,7 @@ static void elm327_send(struct elmcan *elm, const void *buf, size_t len)
 
 	lockdep_assert_held(elm->lock);
 
-	if (elm->hw_failure)
+	if (elm->uart_side_failure)
 		return;
 
 	memcpy(elm->txbuf, buf, len);
@@ -151,7 +153,7 @@ static void elm327_send(struct elmcan *elm, const void *buf, size_t len)
 		netdev_err(elm->dev,
 			   "Failed to write to tty %s.\n",
 			   elm->tty->name);
-		elm327_hw_failure(elm);
+		elm327_uart_side_failure(elm);
 		return;
 	}
 
@@ -294,14 +296,14 @@ static void elm327_feed_frame_to_netdev(struct elmcan *elm,
 }
 
 /* Called when we're out of ideas and just want it all to end. */
-static inline void elm327_hw_failure(struct elmcan *elm)
+static inline void elm327_uart_side_failure(struct elmcan *elm)
 {
 	struct can_frame *frame;
 	struct sk_buff *skb;
 
 	lockdep_assert_held(elm->lock);
 
-	elm->hw_failure = true;
+	elm->uart_side_failure = true;
 
 	elm->can.can_stats.bus_off++;
 	netif_stop_queue(elm->dev);
@@ -719,7 +721,7 @@ static void elm327_parse_rxbuf(struct elmcan *elm)
 			 */
 			netdev_err(elm->dev,
 				   "RX buffer overflow. Faulty ELM327 or UART?\n");
-			elm327_hw_failure(elm);
+			elm327_uart_side_failure(elm);
 			break;
 		} else if (len == elm->rxfill) {
 			if (elm327_is_ready_char(elm->rxbuf[elm->rxfill - 1])) {
@@ -769,7 +771,7 @@ static int elmcan_netdev_open(struct net_device *dev)
 	int err;
 
 	spin_lock_bh(&elm->lock);
-	if (elm->hw_failure) {
+	if (elm->uart_side_failure) {
 		netdev_err(elm->dev, "Refusing to open interface after a hardware fault has been detected.\n");
 		spin_unlock_bh(&elm->lock);
 		return -EIO;
@@ -860,10 +862,10 @@ static netdev_tx_t elmcan_netdev_start_xmit(struct sk_buff *skb,
 	/* We shouldn't get here after a hardware fault:
 	 * can_bus_off() calls netif_carrier_off()
 	 */
-	WARN_ON_ONCE(elm->hw_failure);
+	WARN_ON_ONCE(elm->uart_side_failure);
 
 	if (!elm->tty ||
-	    elm->hw_failure ||
+	    elm->uart_side_failure ||
 	    elm->can.ctrlmode & CAN_CTRLMODE_LISTENONLY) {
 		spin_unlock(&elm->lock);
 		goto out;
@@ -923,14 +925,14 @@ static void elmcan_ldisc_rx(struct tty_struct *tty,
 
 	spin_lock_bh(&elm->lock);
 
-	if (elm->hw_failure)
+	if (elm->uart_side_failure)
 		goto out;
 
 	while (count-- && elm->rxfill < ELM327_SIZE_RXBUF) {
 		if (fp && *fp++) {
 			netdev_err(elm->dev, "Error in received character stream. Check your wiring.");
 
-			elm327_hw_failure(elm);
+			elm327_uart_side_failure(elm);
 
 			goto out;
 		}
@@ -948,7 +950,7 @@ static void elmcan_ldisc_rx(struct tty_struct *tty,
 				netdev_err(elm->dev,
 					   "Received illegal character %02x.\n",
 					   *cp);
-				elm327_hw_failure(elm);
+				elm327_uart_side_failure(elm);
 
 				goto out;
 			}
@@ -962,7 +964,7 @@ static void elmcan_ldisc_rx(struct tty_struct *tty,
 	if (count >= 0) {
 		netdev_err(elm->dev, "Receive buffer overflowed. Bad chip or wiring?");
 
-		elm327_hw_failure(elm);
+		elm327_uart_side_failure(elm);
 
 		goto out;
 	}
@@ -981,7 +983,7 @@ static void elmcan_ldisc_tx_worker(struct work_struct *work)
 	struct elmcan *elm = container_of(work, struct elmcan, tx_work);
 	ssize_t written;
 
-	if (elm->hw_failure)
+	if (elm->uart_side_failure)
 		return;
 
 	spin_lock_bh(&elm->lock);
@@ -992,7 +994,7 @@ static void elmcan_ldisc_tx_worker(struct work_struct *work)
 			netdev_err(elm->dev,
 				   "Failed to write to tty %s.\n",
 				   elm->tty->name);
-			elm327_hw_failure(elm);
+			elm327_uart_side_failure(elm);
 			spin_unlock_bh(&elm->lock);
 			return;
 		} else {
-- 
2.30.2