Fix for bug #73

Deniz-Eren · Deniz-Eren · commit ee6692544ade · 2024-07-10T12:10:58.000+10:00
- Implemented command-line option '-R' to allow for Bus error state recovery; particularly for Error Passive State issues.
- Removed legacy debug asserts in netif_wake_queue() and netif_stop_queue(); these were only active for Debug builds.
- Fixed a bug in command-line help option '-r'; it was still '-b' from prior changes.
diff --git a/README.md b/README.md
@@ -148,6 +148,20 @@ Options:
     -r delay   - Bus-off recovery delay timer duration (milliseconds).
                  If set to 0ms, then the bus-off recovery is disabled!
                  Default: 50ms
+    -R count   - Error state recovery error count value (trigger limit).
+                 Associated SJA1000 states:
+                    < 96   - CAN state error active
+                    < 128  - CAN state error warning
+                    < 256  - CAN state error passive (main reason for feature)
+                    >= 256 - CAN state error bus-off
+                 If enabled, CAN state stopped and sleeping will trigger the
+                 recovery also for any value.
+                 To prevent Error Passive State issues experienced with some
+                 hardware, recommended value to use is 128. This will reboot the
+                 chip if the chip gets overwhelmed with errors; if this
+                 behaviour is desired.
+                 If set to 0, then the error state recovery is disabled.
+                 Default: 0
     -x         - Start the driver with extended MIDs enabled.
                  Device suboptions take precedence over this option.
     -?/h       - Print help menu and exit.
diff --git a/config/PROGRAM_VERSION b/config/PROGRAM_VERSION
@@ -1 +1 @@
-1.3.4
+1.3.5
diff --git a/src/config.c b/src/config.c
@@ -50,6 +50,8 @@
  */
 int optr = 0;
 int optr_restart_ms = DEFAULT_RESTART_MS;
+int optR = 0;
+int optR_error_count = DEFAULT_ERROR_COUNT;
 int optv = 0;
 int optl = 0;
 int opti = 0;
diff --git a/src/include/config.h b/src/include/config.h
@@ -37,6 +37,7 @@
 #define DEFAULT_NUM_RX_CHANNELS 1
 #define DEFAULT_NUM_TX_CHANNELS 1
 #define DEFAULT_RESTART_MS      50 // Default bus-off restart delay
+#define DEFAULT_ERROR_COUNT     0  // default error state recovery count
 
 
 /*
@@ -47,6 +48,8 @@
 
 extern int optr;
 extern int optr_restart_ms;
+extern int optR;
+extern int optR_error_count;
 extern int optv;
 extern int optl;
 extern int opti;
diff --git a/src/main.c b/src/main.c
@@ -121,13 +121,18 @@ int main (int argc, char* argv[]) {
     opterr = opt_bak_opterr;
     optopt = opt_bak_optopt;
 
-    while ((opt = getopt(argc, argv, "r:d:e:U:u:b:m:viqstlVCEwcx?h")) != -1) {
+    while ((opt = getopt(argc, argv, "r:R:d:e:U:u:b:m:viqstlVCEwcx?h")) != -1) {
         switch (opt) {
         case 'r':
             optr++;
             optr_restart_ms = atoi(optarg);
             break;
 
+        case 'R':
+            optR++;
+            optR_error_count = atoi(optarg);
+            break;
+
         case 'd':
         {
             device_config_t* new_disable_device_config;
diff --git a/src/netif.c b/src/netif.c
@@ -20,11 +20,7 @@
 
 #include <pci/pci.h>
 
-#include <linux/units.h>
-#include <linux/netdevice.h>
-#include <linux/can.h>
-#include <linux/can/error.h>
-#include <linux/can/skb.h>
+#include <drivers/net/can/sja1000/sja1000.h>
 #include <session.h>
 
 #include "netif.h"
@@ -104,6 +100,9 @@ int netif_rx (struct sk_buff* skb) {
 
     /* handle error message frame */
     if (msg->can_id & CAN_ERR_FLAG) {
+        struct sja1000_priv *priv = netdev_priv(skb->dev);
+        enum can_state state = priv->can.state;
+
         if (msg->can_id & CAN_ERR_TX_TIMEOUT) {
             log_warn("netif_rx: %s: TX timeout (by netdevice driver)\n",
                     skb->dev->name);
@@ -144,6 +143,70 @@ int netif_rx (struct sk_buff* skb) {
                     skb->dev->name, msg->data[6], msg->data[7]);
         }
 
+        if (optR_error_count != 0) {
+            bool restart_trigger = false;
+
+            switch (state) {
+            case CAN_STATE_ERROR_ACTIVE:    /* RX/TX error count < 96 */
+                if (optR_error_count < 96) {
+                    restart_trigger = true;
+
+                    log_trace("netif_rx: can_restart_now (Error Active)\n");
+
+                }
+                break;
+            case CAN_STATE_ERROR_WARNING:   /* RX/TX error count < 128 */
+                if (optR_error_count < 128) {
+                    restart_trigger = true;
+
+                    log_trace("netif_rx: can_restart_now (Error Warning)\n");
+                }
+                break;
+            case CAN_STATE_ERROR_PASSIVE:   /* RX/TX error count < 256 */
+                if (optR_error_count < 256) {
+                    restart_trigger = true;
+
+                    log_trace("netif_rx: can_restart_now (Error Passive)\n");
+                }
+                break;
+            case CAN_STATE_BUS_OFF:         /* RX/TX error count >= 256 */
+                if (optR_error_count >= 256) {
+                    restart_trigger = true;
+
+                    log_trace("netif_rx: can_restart_now (Error Bus-Off)\n");
+                }
+                break;
+            case CAN_STATE_STOPPED:         /* Device is stopped */
+                restart_trigger = true;
+                log_trace("netif_rx: can_restart_now (State Stopped)\n");
+                break;
+            case CAN_STATE_SLEEPING:        /* Device is sleeping */
+                restart_trigger = true;
+                log_trace("netif_rx: can_restart_now (State Sleeping)\n");
+                break;
+            default:
+                break;
+            }
+
+            if (restart_trigger) {
+                struct can_priv *priv = netdev_priv(skb->dev);
+
+                // Save automatic restart
+                int backup_restart_ms = priv->restart_ms;
+
+                priv->restart_ms = 0;            // Force automatic restart off
+                priv->state = CAN_STATE_BUS_OFF; // Force CAN-bus state Bus-off
+
+                int err = can_restart_now(skb->dev);
+                if (err) {
+                    log_err("netif_rx: can_restart_now error (%d)\n", err);
+                }
+
+                // Restore automatic restart
+                priv->restart_ms = backup_restart_ms;
+            }
+        }
+
         kfree_skb(skb);
 
         return NET_RX_SUCCESS;
@@ -285,8 +348,6 @@ void netif_wake_queue (struct net_device* dev) {
 
     device_session_t* ds = dev->device_session;
 
-    assert( ds->tx_thread != pthread_self() || shutdown_program );
-
     queue_wake_up(&ds->tx_queue);
 }
 
@@ -295,8 +356,6 @@ void netif_stop_queue (struct net_device* dev) {
 
     device_session_t* ds = dev->device_session;
 
-    assert( ds->tx_thread == pthread_self() || shutdown_program );
-
     queue_stop(&ds->tx_queue);
 }
 
diff --git a/src/prints.c b/src/prints.c
@@ -211,11 +211,25 @@ void print_help (char* program_name) {
     printf("                 e.g. -e 13fe:00d7,0x11\n");
     printf("                 By default MSI capability is enabled and MSI-X is disabled,\n");
     printf("                 and requires enabling to be activated (EXPERIMENTAL).\n");
-    printf("    \e[1m-b delay\e[m   - Bus-off recovery delay timer length (milliseconds).\n");
+    printf("    \e[1m-r delay\e[m   - Bus-off recovery delay timer length (milliseconds).\n");
     printf("                 If set to 0ms, then the bus-off recovery is disabled!\n");
     printf("                 The netif transmission queue fault recovery is also set to this\n");
     printf("                 delay value.\n");
     printf("                 Default: %dms\n", DEFAULT_RESTART_MS);
+    printf("    \e[1m-R count\e[m   - Error state recovery error count value (trigger limit).\n");
+    printf("                 Associated SJA1000 states:\n");
+    printf("                    < 96   - CAN state error active\n");
+    printf("                    < 128  - CAN state error warning\n");
+    printf("                    < 256  - CAN state error passive (main reason for feature)\n");
+    printf("                    >= 256 - CAN state error bus-off\n");
+    printf("                 If enabled, CAN state stopped and sleeping will trigger the\n");
+    printf("                 recovery also for any value.\n");
+    printf("                 To prevent Error Passive State issues experienced with some\n");
+    printf("                 hardware, recommended value to use is 128. This will reboot the\n");
+    printf("                 chip if the chip gets overwhelmed with errors; if this\n");
+    printf("                 behaviour is desired.\n");
+    printf("                 If set to 0, then the error state recovery is disabled.\n");
+    printf("                 Default: %d\n", DEFAULT_ERROR_COUNT);
     printf("    \e[1m-?/h\e[m       - Print help menu and exit.\n");
     printf("\n");
     printf("\e[1mNOTES\e[m\n");
