tmedicci commented on code in PR #19176:
URL: https://github.com/apache/nuttx/pull/19176#discussion_r3443989129


##########
boards/risc-v/esp32c6/common/src/esp_board_twai.c:
##########
@@ -77,24 +77,30 @@ int board_twai_setup(int port)
     }
 
 #ifdef CONFIG_ESPRESSIF_TWAI0
-  /* Register the TWAI driver at "/dev/can0" */
-
-  ret = can_register("/dev/can0", twai);
-  if (ret < 0)
+  if (port == 0)
     {
-      canerr("ERROR: TWAI0 register failed: %d\n", ret);
-      return ret;
+      /* Register the TWAI driver at "/dev/can0" */
+
+      ret = can_register("/dev/can0", twai);
+      if (ret < 0)
+        {
+          canerr("ERROR: TWAI0 register failed: %d\n", ret);
+          return ret;
+        }
     }
 #endif /* CONFIG_ESPRESSIF_TWAI0 */
 
 #ifdef CONFIG_ESPRESSIF_TWAI1
-  /* Register the TWAI driver at "/dev/can1" */
-
-  ret = can_register("/dev/can1", twai);
-  if (ret < 0)
+  if (port == 1)
     {
-      canerr("ERROR: TWAI1 register failed: %d\n", ret);
-      return ret;
+      /* Register the TWAI driver at "/dev/can1" */
+
+      ret = can_register("/dev/can1", twai);
+      if (ret < 0)
+        {
+          canerr("ERROR: TWAI1 register failed: %d\n", ret);
+          return ret;
+        }

Review Comment:
   Instead of duplicating the `can_register` function, can't `port` be used to 
derive the device's path and the same function used?
   
   Think of a case with more than two ports; it wouldn't make sense to 
replicate the function n times.



##########
boards/risc-v/esp32p4/common/src/esp_board_twai.c:
##########
@@ -77,27 +77,47 @@ int board_twai_setup(int port)
     }
 
 #ifdef CONFIG_ESPRESSIF_TWAI0
-  /* Register the TWAI driver at "/dev/can0" */
-
-  ret = can_register("/dev/can0", twai);
-  if (ret < 0)
+  if (port == 0)
     {
-      canerr("ERROR: TWAI0 register failed: %d\n", ret);
-      return ret;
+      /* Register the TWAI driver at "/dev/can0" */
+
+      ret = can_register("/dev/can0", twai);
+      if (ret < 0)
+        {
+          canerr("ERROR: TWAI0 register failed: %d\n", ret);
+          return ret;
+        }
     }
 #endif /* CONFIG_ESPRESSIF_TWAI0 */
 
 #ifdef CONFIG_ESPRESSIF_TWAI1
-  /* Register the TWAI driver at "/dev/can1" */
-
-  ret = can_register("/dev/can1", twai);
-  if (ret < 0)
+  if (port == 1)
     {
-      canerr("ERROR: TWAI1 register failed: %d\n", ret);
-      return ret;
+      /* Register the TWAI driver at "/dev/can1" */
+
+      ret = can_register("/dev/can1", twai);
+      if (ret < 0)
+        {
+          canerr("ERROR: TWAI1 register failed: %d\n", ret);
+          return ret;
+        }
     }
 #endif /* CONFIG_ESPRESSIF_TWAI1 */
 
+#ifdef CONFIG_ESPRESSIF_TWAI2
+  if (port == 2)
+    {
+      /* Register the TWAI driver at "/dev/can2" */
+
+      ret = can_register("/dev/can2", twai);
+      if (ret < 0)
+        {
+          canerr("ERROR: TWAI2 register failed: %d\n", ret);
+          return ret;
+        }
+    }

Review Comment:
   Same (about multiple calls to `can_register`).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to