Speed up can327_is_valid_rx_char() with a LUT
[elmcan.git] / module / can327.c
index 75d1ec5bf118d39805adb8b70fefab7e544aac91..aba198651476e1fe79f30ecdf4398e773e0d2c8d 100644 (file)
@@ -70,7 +70,7 @@
 #define ELM327_READY_CHAR '>'
 
 /* Bits in elm->cmds_todo */
-enum elm327_to_to_do_bits {
+enum elm327_tx_do {
        ELM327_TX_DO_CAN_DATA = 0,
        ELM327_TX_DO_CANID_11BIT,
        ELM327_TX_DO_CANID_29BIT_LOW,
@@ -90,7 +90,7 @@ struct can327 {
 
        /* TTY buffers */
        u8 rxbuf[ELM327_SIZE_RXBUF];
-       u8 txbuf[ELM327_SIZE_TXBUF] ____cacheline_aligned;
+       u8 txbuf[ELM327_SIZE_TXBUF];
 
        /* Per-channel lock */
        spinlock_t lock;
@@ -195,8 +195,8 @@ static void elm327_send_frame(struct can327 *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 ^ 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)
@@ -326,21 +326,21 @@ static inline void elm327_uart_side_failure(struct can327 *elm)
        elm327_feed_frame_to_netdev(elm, skb);
 }
 
-/* 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.
+/* Compares a byte buffer (non-NUL terminated) to the payload part of a string,
+ * and returns true iff the buffer (content *and* length) is exactly that
+ * string, without the terminating NUL byte.
  *
- * 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.
+ * Example: If reference is "BUS ERROR", then this returns true iff nbytes == 9
+ *          and !memcmp(buf, "BUS ERROR", 9).
+ *
+ * The reason to use strings is so we can easily include them in the C code,
+ * and to avoid hardcoding lengths.
  */
-static inline int check_len_then_cmp(const u8 *mem, size_t mem_len, const char *str)
+static inline bool elm327_rxbuf_cmp(const u8 *buf, size_t nbytes, const char *reference)
 {
-       size_t str_len = strlen(str);
+       size_t ref_len = strlen(reference);
 
-       return (mem_len == str_len) && !memcmp(mem, str, str_len);
+       return (nbytes == ref_len) && !memcmp(buf, reference, ref_len);
 }
 
 static void elm327_parse_error(struct can327 *elm, size_t len)
@@ -358,26 +358,26 @@ static void elm327_parse_error(struct can327 *elm, size_t len)
                return;
 
        /* Filter possible error messages based on length of RX'd line */
-       if (check_len_then_cmp(elm->rxbuf, len, "UNABLE TO CONNECT")) {
+       if (elm327_rxbuf_cmp(elm->rxbuf, len, "UNABLE TO CONNECT")) {
                netdev_err(elm->dev,
                           "ELM327 reported UNABLE TO CONNECT. Please check your setup.\n");
-       } else if (check_len_then_cmp(elm->rxbuf, len, "BUFFER FULL")) {
+       } else if (elm327_rxbuf_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 (check_len_then_cmp(elm->rxbuf, len, "BUS ERROR")) {
+       } else if (elm327_rxbuf_cmp(elm->rxbuf, len, "BUS ERROR")) {
                frame->can_id |= CAN_ERR_BUSERROR;
-       } else if (check_len_then_cmp(elm->rxbuf, len, "CAN ERROR")) {
+       } else if (elm327_rxbuf_cmp(elm->rxbuf, len, "CAN ERROR")) {
                frame->can_id |= CAN_ERR_PROT;
-       } else if (check_len_then_cmp(elm->rxbuf, len, "<RX ERROR")) {
+       } else if (elm327_rxbuf_cmp(elm->rxbuf, len, "<RX ERROR")) {
                frame->can_id |= CAN_ERR_PROT;
-       } else if (check_len_then_cmp(elm->rxbuf, len, "BUS BUSY")) {
+       } else if (elm327_rxbuf_cmp(elm->rxbuf, len, "BUS BUSY")) {
                frame->can_id |= CAN_ERR_PROT;
                frame->data[2] = CAN_ERR_PROT_OVERLOAD;
-       } else if (check_len_then_cmp(elm->rxbuf, len, "FB ERROR")) {
+       } else if (elm327_rxbuf_cmp(elm->rxbuf, len, "FB ERROR")) {
                frame->can_id |= CAN_ERR_PROT;
                frame->data[2] = CAN_ERR_PROT_TX;
        } else if (len == 5 && !memcmp(elm->rxbuf, "ERR", 3)) {
@@ -601,7 +601,7 @@ static void elm327_handle_prompt(struct can327 *elm)
        } else if (test_and_clear_bit(ELM327_TX_DO_SILENT_MONITOR, &elm->cmds_todo)) {
                snprintf(local_txbuf, sizeof(local_txbuf),
                         "ATCSM%i\r",
-                        !(!(elm->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)));
+                        !!(elm->can.ctrlmode & CAN_CTRLMODE_LISTENONLY));
 
        } else if (test_and_clear_bit(ELM327_TX_DO_RESPONSES, &elm->cmds_todo)) {
                snprintf(local_txbuf, sizeof(local_txbuf),
@@ -695,7 +695,6 @@ static void elm327_parse_rxbuf(struct can327 *elm)
                break;
 
        case ELM327_STATE_GETDUMMYCHAR:
-       {
                /* Wait for 'y' or '>' */
                for (i = 0; i < elm->rxfill; i++) {
                        if (elm->rxbuf[i] == ELM327_DUMMY_CHAR) {
@@ -711,9 +710,7 @@ static void elm327_parse_rxbuf(struct can327 *elm)
                }
 
                elm327_drop_bytes(elm, i);
-
                break;
-       }
 
        case ELM327_STATE_GETPROMPT:
                /* Wait for '>' */
@@ -725,20 +722,17 @@ static void elm327_parse_rxbuf(struct can327 *elm)
 
        case ELM327_STATE_RECEIVING:
                /* Find <CR> delimiting feedback lines. */
-               for (len = 0;
-                    (len < elm->rxfill) && (elm->rxbuf[len] != '\r');
-                    len++) {
-                       /* empty loop */
-               }
+               len = 0;
+               while (len < elm->rxfill && elm->rxbuf[len] != '\r')
+                       len++;
 
                if (len == ELM327_SIZE_RXBUF) {
-                       /* Line exceeds buffer. It's probably all garbage.
+                       /* Assume the buffer ran full with garbage.
                         * Did we even connect at the right baud rate?
                         */
                        netdev_err(elm->dev,
                                   "RX buffer overflow. Faulty ELM327 or UART?\n");
                        elm327_uart_side_failure(elm);
-                       break;
                } else if (len == elm->rxfill) {
                        if (elm327_is_ready_char(elm->rxbuf[elm->rxfill - 1])) {
                                /* The ELM327's AT ST response timeout ran out,
@@ -748,24 +742,22 @@ static void elm327_parse_rxbuf(struct can327 *elm)
                                elm->rxfill = 0;
 
                                elm327_handle_prompt(elm);
-                               break;
                        }
 
                        /* No <CR> found - we haven't received a full line yet.
                         * Wait for more data.
                         */
-                       break;
-               }
-
-               /* We have a full line to parse. */
-               elm327_parse_line(elm, len);
+               } else {
+                       /* We have a full line to parse. */
+                       elm327_parse_line(elm, len);
 
-               /* Remove parsed data from RX buffer. */
-               elm327_drop_bytes(elm, len + 1);
+                       /* Remove parsed data from RX buffer. */
+                       elm327_drop_bytes(elm, len + 1);
 
-               /* More data to parse? */
-               if (elm->rxfill)
-                       elm327_parse_rxbuf(elm);
+                       /* More data to parse? */
+                       if (elm->rxfill)
+                               elm327_parse_rxbuf(elm);
+               }
        }
 }
 
@@ -864,25 +856,26 @@ static netdev_tx_t can327_netdev_start_xmit(struct sk_buff *skb,
        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
+       /* This check will be part of can_dropped_invalid_skb()
+        * in future kernels.
         */
-       spin_lock(&elm->lock);
+       if (elm->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
+               goto out;
 
        /* We shouldn't get here after a hardware fault:
         * can_bus_off() calls netif_carrier_off()
         */
-       WARN_ON_ONCE(elm->uart_side_failure);
-
-       if (!elm->tty ||
-           elm->uart_side_failure ||
-           elm->can.ctrlmode & CAN_CTRLMODE_LISTENONLY) {
-               spin_unlock(&elm->lock);
+       if (elm->uart_side_failure) {
+               WARN_ON_ONCE(elm->uart_side_failure);
                goto out;
        }
 
        netif_stop_queue(dev);
 
+       /* BHs are already disabled, so no spin_lock_bh().
+        * See Documentation/networking/netdevices.txt
+        */
+       spin_lock(&elm->lock);
        elm327_send_frame(elm, frame);
        spin_unlock(&elm->lock);
 
@@ -903,20 +896,30 @@ static const struct net_device_ops can327_netdev_ops = {
        .ndo_change_mtu = can_change_mtu,
 };
 
-static bool can327_is_valid_rx_char(char c)
+static bool can327_is_valid_rx_char(u8 c)
 {
-       return (isdigit(c) ||
-               isupper(c) ||
-               c == ELM327_DUMMY_CHAR ||
-               c == ELM327_READY_CHAR ||
-               c == '<' ||
-               c == 'a' ||
-               c == 'b' ||
-               c == 'v' ||
-               c == '.' ||
-               c == '?' ||
-               c == '\r' ||
-               c == ' ');
+       static const bool lut_char_is_valid['z'] = {
+               ['\r'] = true,
+               [' '] = true,
+               ['.'] = true,
+               ['0'] = true, true, true, true, true,
+               ['5'] = true, true, true, true, true,
+               ['<'] = true,
+               [ELM327_READY_CHAR] = true,
+               ['?'] = true,
+               ['A'] = true, true, true, true, true, true, true,
+               ['H'] = true, true, true, true, true, true, true,
+               ['O'] = true, true, true, true, true, true, true,
+               ['V'] = true, true, true, true, true,
+               ['a'] = true,
+               ['b'] = true,
+               ['v'] = true,
+               [ELM327_DUMMY_CHAR] = true,
+       };
+       BUILD_BUG_ON(ELM327_DUMMY_CHAR >= 'z');
+
+       return (c < ARRAY_SIZE(lut_char_is_valid) &&
+               lut_char_is_valid[c]);
 }
 
 /* Handle incoming ELM327 ASCII data.
@@ -933,10 +936,10 @@ static void can327_ldisc_rx(struct tty_struct *tty,
 {
        struct can327 *elm = (struct can327 *)tty->disc_data;
 
-       spin_lock_bh(&elm->lock);
-
        if (elm->uart_side_failure)
-               goto out;
+               return;
+
+       spin_lock_bh(&elm->lock);
 
        while (count-- && elm->rxfill < ELM327_SIZE_RXBUF) {
                if (fp && *fp++) {
@@ -944,7 +947,8 @@ static void can327_ldisc_rx(struct tty_struct *tty,
 
                        elm327_uart_side_failure(elm);
 
-                       goto out;
+                       spin_unlock_bh(&elm->lock);
+                       return;
                }
 
                /* Ignore NUL characters, which the PIC microcontroller may
@@ -952,7 +956,7 @@ static void can327_ldisc_rx(struct tty_struct *tty,
                 * See ELM327 documentation, which refers to a Microchip PIC
                 * bug description.
                 */
-               if (*cp != 0) {
+               if (*cp) {
                        /* Check for stray characters on the UART line.
                         * Likely caused by bad hardware.
                         */
@@ -962,7 +966,8 @@ static void can327_ldisc_rx(struct tty_struct *tty,
                                           *cp);
                                elm327_uart_side_failure(elm);
 
-                               goto out;
+                               spin_unlock_bh(&elm->lock);
+                               return;
                        }
 
                        elm->rxbuf[elm->rxfill++] = *cp;
@@ -976,12 +981,11 @@ static void can327_ldisc_rx(struct tty_struct *tty,
 
                elm327_uart_side_failure(elm);
 
-               goto out;
+               spin_unlock_bh(&elm->lock);
+               return;
        }
 
        elm327_parse_rxbuf(elm);
-
-out:
        spin_unlock_bh(&elm->lock);
 }
 
@@ -1005,6 +1009,7 @@ static void can327_ldisc_tx_worker(struct work_struct *work)
                                   "Failed to write to tty %s.\n",
                                   elm->tty->name);
                        elm327_uart_side_failure(elm);
+
                        spin_unlock_bh(&elm->lock);
                        return;
                }
@@ -1013,12 +1018,10 @@ static void can327_ldisc_tx_worker(struct work_struct *work)
                elm->txhead += written;
        }
 
-       if (!elm->txleft)  {
+       if (!elm->txleft)
                clear_bit(TTY_DO_WRITE_WAKEUP, &elm->tty->flags);
-               spin_unlock_bh(&elm->lock);
-       } else {
-               spin_unlock_bh(&elm->lock);
-       }
+
+       spin_unlock_bh(&elm->lock);
 }
 
 /* Called by the driver when there's room for more data. */
@@ -1090,16 +1093,14 @@ static int can327_ldisc_open(struct tty_struct *tty)
 
        /* Let 'er rip */
        err = register_candev(elm->dev);
-       if (err)
-               goto out_err;
+       if (err) {
+               free_candev(elm->dev);
+               return err;
+       }
 
        netdev_info(elm->dev, "can327 on %s.\n", tty->name);
 
        return 0;
-
-out_err:
-       free_candev(elm->dev);
-       return err;
 }
 
 /* Close down a can327 channel.