Convert comments to lockdep_assert_held()
[elmcan.git] / module / elmcan.c
index e8f163f81f1c38cc4056344c1203d82a92cce8e7..fd38475b0306b3538b8a14cbbb696b6127efa4a6 100644 (file)
@@ -4,9 +4,14 @@
  * 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
+#define pr_fmt(fmt) "elmcan: " fmt
 
 #include <linux/init.h>
 #include <linux/module.h>
@@ -20,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>
 #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
@@ -89,11 +90,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 +103,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];
@@ -121,22 +122,20 @@ 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;
+       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;
@@ -152,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;
 
@@ -188,24 +188,24 @@ 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)
 {
-       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. */
@@ -239,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 */
@@ -267,7 +264,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;
@@ -292,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;
 
@@ -311,28 +311,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.
- */
+/* 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;
 
-       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->hw_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;
+
+       frame->can_id |= CAN_ERR_BUSOFF;
+       elm327_feed_frame_to_netdev(elm, skb);
 }
 
 /* Compare a buffer to a fixed string */
@@ -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;
@@ -562,31 +565,27 @@ static void elm327_parse_line(struct elmcan *elm, size_t len)
        }
 
        /* 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];
 
+       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;
 
                return;
        }
@@ -648,7 +647,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));
@@ -662,32 +661,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])) {
@@ -702,7 +702,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);
@@ -710,7 +710,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');
@@ -848,11 +848,6 @@ 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;
-       }
-
        /* BHs are already disabled, so no spin_lock_bh().
         * See Documentation/networking/netdevices.txt
         */
@@ -1292,3 +1287,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>");