Shorten the derivation comment at the start
[elmcan.git] / module / elmcan.c
index f358a46dafe86f3950be8a2ea15d8bfcb529746f..4409642df1cbc1edcae56b3f92182ad1a93c56f4 100644 (file)
@@ -2,11 +2,15 @@
 /* ELM327 based CAN interface driver (tty line discipline)
  *
  * 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.
+ * and my thanks go to the original authors for their inspiration.
+ *
+ * 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
+#define pr_fmt(fmt) "elmcan: " fmt
 
 #include <linux/init.h>
 #include <linux/module.h>
@@ -20,6 +24,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>
 #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
+ * Starting with Linux v5.18-rc1, N_DEVELOPMENT is defined as 29:
+ * https://github.com/torvalds/linux/commit/c2faf737abfb10f88f2d2612d573e9edc3c42c37
  */
 #ifndef N_DEVELOPMENT
 #define N_DEVELOPMENT 29
 #endif
 
+/* Compatibility for Linux < 5.11 */
+#if LINUX_VERSION_CODE < KERNEL_VERSION(5,11,0)
+#define len can_dlc
+#endif
+
 #define ELM327_NAPI_WEIGHT 4
 
-#define ELM327_SIZE_RXBUF 256
+#define ELM327_SIZE_RXBUF 224
 #define ELM327_SIZE_TXBUF 32
 
 #define ELM327_CAN_CONFIG_SEND_SFF           0x8000
@@ -65,7 +70,7 @@ MODULE_AUTHOR("Max Staudt <max-linux@enpas.org>");
 #define ELM327_READY_CHAR '>'
 
 /* Bits in elm->cmds_todo */
-enum ELM327_TX_DO_BITS {
+enum elm327_to_to_do_bits {
        ELM327_TX_DO_CAN_DATA = 0,
        ELM327_TX_DO_CANID_11BIT,
        ELM327_TX_DO_CANID_29BIT_LOW,
@@ -83,129 +88,109 @@ struct elmcan {
 
        struct can_rx_offload offload;
 
-       /* TTY and netdev devices that we're bridging */
-       struct tty_struct       *tty;
-       struct net_device       *dev;
-
-       /* 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;
+       /* TTY buffers */
+       u8 rxbuf[ELM327_SIZE_RXBUF];
+       u8 txbuf[ELM327_SIZE_TXBUF] ____cacheline_aligned;
 
-       /* Stop the channel on hardware failure.
-        * Once this is true, nothing will be sent to the TTY.
-        */
-       bool                    hw_failure;
+       /* TTY buffer accounting */
+       struct work_struct tx_work;     /* Flushes TTY TX buffer */
+       u8 *txhead;                     /* Next TX byte */
+       unsigned txleft;                /* Bytes left to TX */
+       int rxfill;                     /* Bytes already RX'd in buffer */
 
-       /* 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 */
+       /* TTY and netdev devices that we're bridging */
+       struct tty_struct *tty;
+       struct net_device *dev;
 
-       /* TTY RX helpers */
-       unsigned char rxbuf[ELM327_SIZE_RXBUF];
-       int rxfill;
+       /* Per-channel lock */
+       spinlock_t lock;
 
        /* 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;
+       /* Things we have yet to send */
+       char **next_init_cmd;
+       unsigned long cmds_todo;
 
        /* 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;
-       unsigned long cmds_todo;
-};
+       /* Parser state */
+       bool drop_next_line;
 
-/* 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);
+       /* 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 uart_side_failure;
+};
 
-static inline void elm327_hw_failure(struct elmcan *elm);
+static inline void elm327_uart_side_failure(struct elmcan *elm);
 
-/* Assumes elm->lock taken. */
 static void elm327_send(struct elmcan *elm, const void *buf, size_t len)
 {
-       int actual;
+       int written;
 
-       if (elm->hw_failure)
+       lockdep_assert_held(elm->lock);
+
+       if (elm->uart_side_failure)
                return;
 
        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);
-               elm327_hw_failure(elm);
+               elm327_uart_side_failure(elm);
                return;
        }
 
-       elm->txleft = len - actual;
-       elm->txhead = elm->txbuf + actual;
+       elm->txleft = len - written;
+       elm->txhead = elm->txbuf + written;
+
+       if (elm->txleft)
+               set_bit(TTY_DO_WRITE_WAKEUP, &elm->tty->flags);
 }
 
 /* Take the ELM327 out of almost any state and back into command mode.
  * 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)
 {
-       if (elm->state != ELM_GETDUMMYCHAR && elm->state != ELM_GETPROMPT) {
+       lockdep_assert_held(elm->lock);
+
+       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;
        }
 }
 
-/* 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. */
-               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)
@@ -234,16 +219,15 @@ 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.
+ * 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 */
        "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 */
@@ -262,7 +246,9 @@ static char *elm327_init_script[] = {
 
 static void elm327_init(struct elmcan *elm)
 {
-       elm->state = ELM_NOTINIT;
+       lockdep_assert_held(elm->lock);
+
+       elm->state = ELM327_STATE_NOTINIT;
        elm->can_frame_to_send.can_id = 0x7df; /* ELM327 HW default */
        elm->rxfill = 0;
        elm->drop_next_line = 0;
@@ -287,10 +273,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;
 
@@ -306,34 +293,29 @@ 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.
- */
-static inline void elm327_hw_failure(struct elmcan *elm)
+/* Called when we're out of ideas and just want it all to end. */
+static inline void elm327_uart_side_failure(struct elmcan *elm)
 {
        struct can_frame *frame;
        struct sk_buff *skb;
 
-       skb = alloc_can_err_skb(elm->dev, &frame);
-       if (!skb)
-               return;
+       lockdep_assert_held(elm->lock);
 
-       frame->data[5] = 'R';
-       frame->data[6] = 'I';
-       frame->data[7] = 'P';
+       elm->uart_side_failure = true;
 
-       elm327_feed_frame_to_netdev(elm, skb);
+       elm->can.can_stats.bus_off++;
+       netif_stop_queue(elm->dev);
+       elm->can.state = CAN_STATE_BUS_OFF;
+       can_bus_off(elm->dev);
 
        netdev_err(elm->dev, "ELM327 misbehaved. Blocking further communication.\n");
 
-       elm->hw_failure = true;
-       can_bus_off(elm->dev);
-}
+       skb = alloc_can_err_skb(elm->dev, &frame);
+       if (!skb)
+               return;
 
-/* Compare a buffer to a fixed string */
-static inline int _memstrcmp(const u8 *mem, const char *str)
-{
-       return memcmp(mem, str, strlen(str));
+       frame->can_id |= CAN_ERR_BUSOFF;
+       elm327_feed_frame_to_netdev(elm, skb);
 }
 
 /* Compare buffer to string length, then compare buffer to fixed string.
@@ -341,20 +323,25 @@ static inline int _memstrcmp(const u8 *mem, const char *str)
  *  - 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);
 }
 
-/* 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:
@@ -363,29 +350,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]);
@@ -411,8 +398,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)
 {
@@ -422,6 +407,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;
@@ -462,7 +449,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.
@@ -483,7 +469,7 @@ static int elm327_parse_frame(struct elmcan *elm, size_t len)
         */
 
        /* Read CAN data length */
-       frame->can_dlc = (hex_to_bin(elm->rxbuf[datastart - 2]) << 0);
+       frame->len = (hex_to_bin(elm->rxbuf[datastart - 2]) << 0);
 
        /* Read CAN ID */
        if (frame->can_id & CAN_EFF_FLAG) {
@@ -503,7 +489,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;
        }
 
@@ -511,13 +497,13 @@ static int elm327_parse_frame(struct elmcan *elm, size_t len)
         * Note: RTR frames have a DLC, but no actual payload.
         */
        if (!(frame->can_id & CAN_RTR_FLAG) &&
-           (hexlen < frame->can_dlc * 3 + datastart)) {
+           (hexlen < frame->len * 3 + datastart)) {
                /* Incomplete frame.
                 * Probably the ELM327's RS232 TX buffer was full.
                 * Emit an error frame and exit.
                 */
                frame->can_id = CAN_ERR_FLAG | CAN_ERR_CRTL;
-               frame->can_dlc = CAN_ERR_DLC;
+               frame->len = CAN_ERR_DLC;
                frame->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
                elm327_feed_frame_to_netdev(elm, skb);
 
@@ -530,7 +516,7 @@ static int elm327_parse_frame(struct elmcan *elm, size_t len)
        }
 
        /* Parse the data nibbles. */
-       for (i = 0; i < frame->can_dlc; i++) {
+       for (i = 0; i < frame->len; i++) {
                frame->data[i] = (hex_to_bin(elm->rxbuf[datastart + 3*i]) << 4)
                               | (hex_to_bin(elm->rxbuf[datastart + 3*i + 1]));
        }
@@ -541,9 +527,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;
@@ -552,43 +539,50 @@ 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;
        }
 
        /* Regular parsing */
-       switch (elm->state) {
-       case ELM_RECEIVING:
-               if (elm327_parse_frame(elm, len)) {
-                       /* Parse an error line. */
-                       elm327_parse_error(elm, len);
+       if (elm->state == ELM327_STATE_RECEIVING
+           && elm327_parse_frame(elm, len)) {
+               /* Parse an error line. */
+               elm327_parse_error(elm, len);
 
-                       /* Start afresh. */
-                       elm327_kick_into_cmd_mode(elm);
-               }
-               break;
-       default:
-               break;
+               /* Start afresh. */
+               elm327_kick_into_cmd_mode(elm);
        }
 }
 
-/* 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];
+       /* 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);
 
        if (!elm->cmds_todo) {
                /* Enter CAN monitor mode */
                elm327_send(elm, "ATMA\r", 5);
-               elm->state = ELM_RECEIVING;
+               elm->state = ELM327_STATE_RECEIVING;
+
+               /* We will be in the default state once this command is
+                * send, so enable the TX packet queue.
+                */
+               netif_wake_queue(elm->dev);
 
                return;
        }
 
        /* 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)) {
@@ -597,31 +591,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)) {
@@ -629,21 +630,29 @@ 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->can_dlc; i++) {
-                               sprintf(&local_txbuf[2 * i], "%02X",
+                       for (i = 0; i < frame->len; i++) {
+                               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;
-               elm->state = ELM_RECEIVING;
+               elm->state = ELM327_STATE_RECEIVING;
+
+               /* We will be in the default state once this command is
+                * send, so enable the TX packet queue.
+                */
+               netif_wake_queue(elm->dev);
        }
 
        elm327_send(elm, local_txbuf, strlen(local_txbuf));
@@ -657,32 +666,33 @@ 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 ELM_NOTINIT:
+       case ELM327_STATE_NOTINIT:
                elm->rxfill = 0;
                break;
 
-       case ELM_GETDUMMYCHAR:
+       case ELM327_STATE_GETDUMMYCHAR:
        {
                /* Wait for 'y' or '>' */
-               int i;
-
                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 +707,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 +715,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');
@@ -719,7 +729,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])) {
@@ -751,6 +761,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,
@@ -760,6 +771,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)
 {
@@ -767,17 +779,19 @@ static int elmcan_netdev_open(struct net_device *dev)
        int err;
 
        spin_lock_bh(&elm->lock);
-       if (elm->hw_failure) {
-               netdev_err(elm->dev, "Refusing to open interface after a hardware fault has been detected.\n");
-               spin_unlock_bh(&elm->lock);
-               return -EIO;
-       }
 
        if (!elm->tty) {
                spin_unlock_bh(&elm->lock);
                return -ENODEV;
        }
 
+       if (elm->uart_side_failure)
+               netdev_warn(elm->dev, "Reopening netdev after a UART side fault has been detected.\n");
+
+       /* Clear TTY buffers */
+       elm->rxfill = 0;
+       elm->txleft = 0;
+
        /* open_candev() checks for elm->can.bittiming.bitrate != 0 */
        err = open_candev(dev);
        if (err) {
@@ -788,8 +802,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;
@@ -808,24 +826,16 @@ static int elmcan_netdev_close(struct net_device *dev)
 {
        struct elmcan *elm = netdev_priv(dev);
 
-       netif_stop_queue(dev);
-
+       /* Interrupt whatever the ELM327 is doing right now */
        spin_lock_bh(&elm->lock);
-       if (elm->tty) {
-               /* Interrupt whatever we're doing right now */
-               elm327_send(elm, ELM327_DUMMY_STRING, 1);
-
-               /* Clear the wakeup bit, as the netdev will be down and thus
-                * the wakeup handler won't clear it
-                */
-               clear_bit(TTY_DO_WRITE_WAKEUP, &elm->tty->flags);
+       elm327_send(elm, ELM327_DUMMY_STRING, 1);
+       spin_unlock_bh(&elm->lock);
 
-               spin_unlock_bh(&elm->lock);
+       netif_stop_queue(dev);
 
-               flush_work(&elm->tx_work);
-       } else {
-               spin_unlock_bh(&elm->lock);
-       }
+       /* Give UART one final chance to flush. */
+       clear_bit(TTY_DO_WRITE_WAKEUP, &elm->tty->flags);
+       flush_work(&elm->tx_work);
 
        can_rx_offload_disable(&elm->offload);
        elm->can.state = CAN_STATE_STOPPED;
@@ -843,10 +853,8 @@ 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 (!netif_running(dev))  {
-               netdev_warn(elm->dev, "xmit: iface is down.\n");
-               goto out;
-       }
+       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
@@ -856,10 +864,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;
@@ -871,7 +879,7 @@ static netdev_tx_t elmcan_netdev_start_xmit(struct sk_buff *skb,
        spin_unlock(&elm->lock);
 
        dev->stats.tx_packets++;
-       dev->stats.tx_bytes += frame->can_dlc;
+       dev->stats.tx_bytes += frame->len;
 
        can_led_event(dev, CAN_LED_EVENT_TX);
 
@@ -887,39 +895,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) ||
@@ -948,21 +923,18 @@ 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);
 
-       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;
                }
@@ -980,7 +952,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;
                        }
@@ -994,7 +966,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;
        }
@@ -1003,7 +975,6 @@ static void elmcan_ldisc_rx(struct tty_struct *tty,
 
 out:
        spin_unlock_bh(&elm->lock);
-       put_elm(elm);
 }
 
 /* Write out remaining transmit buffer.
@@ -1011,59 +982,43 @@ 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->uart_side_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_uart_side_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,
@@ -1104,21 +1059,12 @@ static int elmcan_ldisc_open(struct tty_struct *tty)
                return -ENFILE;
        elm = netdev_priv(dev);
 
-       elm->txbuf = kmalloc(ELM327_SIZE_TXBUF, GFP_KERNEL);
-       if (!elm->txbuf) {
-               err = -ENOMEM;
-               goto out_err;
-       }
-
        /* Configure TTY interface */
        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 */
-       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;
@@ -1144,7 +1090,6 @@ static int elmcan_ldisc_open(struct tty_struct *tty)
        return 0;
 
 out_err:
-       kfree(elm->txbuf);
        free_candev(elm->dev);
        return err;
 }
@@ -1158,27 +1103,13 @@ out_err:
  */
 static void elmcan_ldisc_close(struct tty_struct *tty)
 {
-       struct elmcan *elm = get_elm(tty);
-
-       if (!elm)
-               return;
-
-       /* unregister_netdev() calls .ndo_stop() so we don't have to. */
-       unregister_candev(elm->dev);
+       struct elmcan *elm = (struct elmcan *)tty->disc_data;
 
-       /* 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.
+       /* unregister_netdev() calls .ndo_stop() so we don't have to.
+        * Our .ndo_stop() also flushes the TTY write wakeup handler,
+        * so we can safely set elm->tty = NULL after this.
         */
-       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. */
-
-       flush_work(&elm->tx_work);
+       unregister_candev(elm->dev);
 
        /* Mark channel as dead */
        spin_lock_bh(&elm->lock);
@@ -1188,51 +1119,29 @@ static void elmcan_ldisc_close(struct tty_struct *tty)
 
        netdev_info(elm->dev, "elmcan off %s.\n", tty->name);
 
-       kfree(elm->txbuf);
        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,
 #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
@@ -1249,7 +1158,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,
 };
 
@@ -1287,3 +1195,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>");