gromero commented on a change in pull request #9068:
URL: https://github.com/apache/tvm/pull/9068#discussion_r717705437



##########
File path: apps/microtvm/zephyr/template_project/microtvm_api_server.py
##########
@@ -545,6 +546,10 @@ def _find_openocd_serial_port(cls, options):
 
         return ports[0].device
 
+    @classmethod
+    def _find_jlink_serial_port(cls, options):
+        return cls._find_openocd_serial_port(options)

Review comment:
       @mehrdadh I think that ideally `_find_jlink_serial_port()` should not be 
made merely a wrapper for `_find_openocd_serial_port()`. I understand that the 
code for the later works well for finding a serial for Jlink too, but openocd 
is not really involved on detecting the serial port in the end. It looks in 
Zephyr's CMake configs and goes for a detection using `usb` module. Hence I 
think that `_find_openocd_serial_port()` should be made a generic code (and 
renamed to something like `_find_serial_port()`) and then be used for both 
"openocd" and "jlink" flash runners.
   
   `_find_openocd_serial_port()` even contains debug message which cites 
openocd which is confusing because under the hood for the board in question 
`JLinkExe` will be used to flash the fw, not openocd.
   
   The `ProjectOption`  `openocd_serial` with also contains "openocd" also 
becomes incorrect since that option also applies now to `jlink` flash runner, 
so ideally it should be adapted too I think.




-- 
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