Update comment about N_DEVELOPMENT
[elmcan.git] / module / elmcan.c
index 1ac12913d78b5f1466bbb8ff18e3ef827c87ec84..76e138905fec0677374dfd7e373c0ab39abed290 100644 (file)
@@ -44,8 +44,8 @@
 #include <linux/can/rx-offload.h>
 
 /* 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
@@ -58,7 +58,7 @@
 
 #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
@@ -89,6 +89,16 @@ struct elmcan {
 
        struct can_rx_offload offload;
 
+       /* TTY buffers */
+       u8 rxbuf[ELM327_SIZE_RXBUF];
+       u8 txbuf[ELM327_SIZE_TXBUF] ____cacheline_aligned;
+
+       /* 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 and netdev devices that we're bridging */
        struct tty_struct *tty;
        struct net_device *dev;
@@ -96,23 +106,6 @@ struct elmcan {
        /* Per-channel lock */
        spinlock_t 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;
-
-       /* TTY TX helpers */
-       struct work_struct tx_work;     /* Flushes TTY TX buffer   */
-       u8 *txbuf;                      /* Pointer to our TX buffer */
-       u8 *txhead;                     /* Pointer to next TX byte */
-       unsigned txleft;                /* Bytes left to TX */
-
-       /* TTY RX helpers */
-       u8 rxbuf[ELM327_SIZE_RXBUF];
-       int rxfill;
-
        /* State machine */
        enum {
                ELM327_STATE_NOTINIT = 0,
@@ -121,7 +114,9 @@ struct elmcan {
                ELM327_STATE_RECEIVING,
        } state;
 
-       bool 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
@@ -130,9 +125,15 @@ struct elmcan {
        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;
+
+       /* 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_uart_side_failure(struct elmcan *elm);
@@ -160,9 +161,7 @@ static void elm327_send(struct elmcan *elm, const void *buf, size_t len)
        elm->txleft = len - written;
        elm->txhead = elm->txbuf + written;
 
-       if (!elm->txleft)
-               netif_wake_queue(elm->dev);
-       else
+       if (elm->txleft)
                set_bit(TTY_DO_WRITE_WAKEUP, &elm->tty->flags);
 }
 
@@ -572,6 +571,11 @@ static void elm327_handle_prompt(struct elmcan *elm)
                elm327_send(elm, "ATMA\r", 5);
                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;
        }
 
@@ -645,6 +649,11 @@ static void elm327_handle_prompt(struct elmcan *elm)
 
                elm->drop_next_line = 1;
                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));
@@ -771,17 +780,19 @@ static int elmcan_netdev_open(struct net_device *dev)
        int err;
 
        spin_lock_bh(&elm->lock);
-       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;
-       }
 
        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) {
@@ -816,24 +827,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;
@@ -1006,7 +1009,6 @@ static void elmcan_ldisc_tx_worker(struct work_struct *work)
        if (!elm->txleft)  {
                clear_bit(TTY_DO_WRITE_WAKEUP, &elm->tty->flags);
                spin_unlock_bh(&elm->lock);
-               netif_wake_queue(elm->dev);
        } else {
                spin_unlock_bh(&elm->lock);
        }
@@ -1058,15 +1060,8 @@ 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);
        INIT_WORK(&elm->tx_work, elmcan_ldisc_tx_worker);
 
@@ -1096,7 +1091,6 @@ static int elmcan_ldisc_open(struct tty_struct *tty)
        return 0;
 
 out_err:
-       kfree(elm->txbuf);
        free_candev(elm->dev);
        return err;
 }
@@ -1112,13 +1106,12 @@ static void elmcan_ldisc_close(struct tty_struct *tty)
 {
        struct elmcan *elm = (struct elmcan *)tty->disc_data;
 
-       /* unregister_netdev() calls .ndo_stop() so we don't have to. */
+       /* 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.
+        */
        unregister_candev(elm->dev);
 
-       /* 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 */
        spin_lock_bh(&elm->lock);
        tty->disc_data = NULL;
@@ -1127,7 +1120,6 @@ 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);
 }