mkatanbaf commented on code in PR #13885:
URL: https://github.com/apache/tvm/pull/13885#discussion_r1094911794


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -413,6 +418,9 @@ def _create_prj_conf(
                 "CONFIG_UART_INTERRUPT_DRIVEN=y\n"
                 "\n"
             )
+            if config_led and not self._is_qemu(board, False):

Review Comment:
   I believe similar to QEMU, the fvp should be excluded too 



##########
apps/microtvm/zephyr/template_project/src/host_driven/platform.h:
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef TVM_APPS_MICROTVM_ZEPHYR_HOST_DRIVEN_PLATFORM_H_
+#define TVM_APPS_MICROTVM_ZEPHYR_HOST_DRIVEN_PLATFORM_H_
+
+#include <zephyr/drivers/gpio.h>
+
+#ifdef CONFIG_LED

Review Comment:
   do we need to place the prototypes of the functions defined in platform.c 
here?



##########
apps/microtvm/zephyr/template_project/src/mlperftiny/platform.cc:
##########
@@ -162,3 +172,33 @@ int8_t QuantizeFloatToInt8(float value, float scale, int 
zero_point) {
   }
   return (int8_t)(result);
 }
+
+// UART read.
+char TVMPlatformUartRxRead() {

Review Comment:
   why some functions in this file have the `__attribute__((weak))` and some 
don't? What is the logic behind applying it?



##########
src/runtime/crt/host/platform-template.h:
##########
@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file tvm/runtime/crt/host/platform-template.h
+ * \brief Template for CRT platform configuration.
+ */
+#ifndef TVM_RUNTIME_CRT_HOST_PLATFORM_TEMPLATE_H_
+#define TVM_RUNTIME_CRT_HOST_PLATFORM_TEMPLATE_H_
+
+#include <tvm/runtime/crt/page_allocator.h>
+
+MemoryManagerInterface* memory_manager;
+

Review Comment:
   same comment about the function prototypes



##########
apps/microtvm/zephyr/template_project/src/aot_standalone_demo/platform.h:
##########
@@ -17,12 +17,13 @@
  * under the License.
  */
 
-#ifndef TVM_APPS_MICROTVM_ZEPHYR_AOT_STANDALONE_DEMO_ZEPHYR_UART_H_
-#define TVM_APPS_MICROTVM_ZEPHYR_AOT_STANDALONE_DEMO_ZEPHYR_UART_H_
+#ifndef TVM_APPS_MICROTVM_ZEPHYR_AOT_STANDALONE_DEMO_PLATFORM_H_
+#define TVM_APPS_MICROTVM_ZEPHYR_AOT_STANDALONE_DEMO_PLATFORM_H_
 
 #include <stdint.h>
+#include <tvm/runtime/crt/stack_allocator.h>
 
-// Used to read data from the UART.
+extern tvm_workspace_t app_workspace;

Review Comment:
   do we need to place the prototypes of the functions defined in platform.c 
here?



##########
apps/microtvm/zephyr/template_project/src/host_driven/platform.c:
##########
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "tvm/platform.h"
+
+#include <stdarg.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <zephyr/kernel.h>
+#include <zephyr/sys/reboot.h>
+
+#include "dlpack/dlpack.h"
+#include "tvm/runtime/crt/error_codes.h"
+
+// Heap for use by TVMPlatformMemoryAllocate.
+K_HEAP_DEFINE(tvm_heap, HEAP_SIZE_BYTES);
+
+// Called by TVM when a message needs to be formatted.
+__attribute__((weak)) size_t TVMPlatformFormatMessage(char* out_buf, size_t 
out_buf_size_bytes,
+                                                      const char* fmt, va_list 
args) {
+  return vsnprintk(out_buf, out_buf_size_bytes, fmt, args);
+}
+
+// Called by TVM when an internal invariant is violated, and execution cannot 
continue.
+__attribute__((weak)) void TVMPlatformAbort(tvm_crt_error_t error) {
+  TVMLogf("TVMError: 0x%x", error);

Review Comment:
   Do we need the `TVMLogf` function in here? I don't see it in this file. Is 
it defined in any of the included files?



##########
apps/microtvm/zephyr/template_project/src/aot_standalone_demo/platform.c:
##########
@@ -16,19 +16,64 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-#include "zephyr_uart.h"
 
+#include "tvm/platform.h"
+
+#include <assert.h>
+#include <stdarg.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
 #include <zephyr/drivers/uart.h>
+#include <zephyr/sys/reboot.h>
 #include <zephyr/sys/ring_buffer.h>
 
 #include "crt_config.h"
+#include "dlpack/dlpack.h"
+#include "tvm/runtime/crt/error_codes.h"
+#include "tvmgen_default.h"
 
 static const struct device* g_microtvm_uart;
 #define RING_BUF_SIZE_BYTES (TVM_CRT_MAX_PACKET_SIZE_BYTES + 100)
 
 // Ring buffer used to store data read from the UART on rx interrupt.
 RING_BUF_DECLARE(uart_rx_rbuf, RING_BUF_SIZE_BYTES);
 
+void TVMLogf(const char* msg, ...) {

Review Comment:
   Does it need the `__attribute__((weak))` similar to the other functions here?



##########
apps/microtvm/zephyr/template_project/src/host_driven/main.c:
##########
@@ -110,17 +103,6 @@ size_t TVMPlatformFormatMessage(char* out_buf, size_t 
out_buf_size_bytes, const
   return vsnprintk(out_buf, out_buf_size_bytes, fmt, args);

Review Comment:
   Move this function to platform.c?



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