areusch commented on a change in pull request #31:
URL: https://github.com/apache/tvm-rfcs/pull/31#discussion_r715198467
##########
File path: rfcs/0031-devices-api.md
##########
@@ -41,7 +41,7 @@ In this example, you can see the diversity of RTOS
implementations for drivers a
[guide-level-explanation]: #guide-level-explanation
## User App
-`tvm_device_<device>_t`s are implemented for each RTOS or platform required,
these are included by the user who chooses as appropriate for their
application. Notably, to avoid dynamic allocation, the user must provide the
`tvm_device_<device>_t` struct and initialise it rather than it being created
and setup for them in the API. This is augmented by named functions for each
device, examples in the case of the "woofles" accelerator:
+TVM presumes that the RTOS, platform, or user application defines a struct
type `tvm_device_<device>_t`, these are included by the user who chooses an
implementation as appropriate for their application. Notably, to avoid dynamic
allocation, the user must provide the `tvm_device_<device>_t` struct and
initialise it rather than it being created and setup for them in the API. TVM
then expects an implementation of the named functions for each device, examples
in the case of the "woofles" accelerator:
Review comment:
could you also explicitly highlight where `<device>` comes from from a
user PoV?
##########
File path: rfcs/0031-devices-api.md
##########
@@ -127,7 +127,7 @@ This simple wrapper enables type checking of these
functions and defining a clea
[reference-level-explanation]: #reference-level-explanation
## Entrypoint
-The entrypoint API defined in [Embedded C Runtime
Interface](https://discuss.tvm.apache.org/t/rfc-utvm-embedded-c-runtime-interface/9951)
is augmented with the `devices` structure which contains implemented
`tvm_device_t` `struct`s for each device used by the network:
+The entrypoint API defined in [Embedded C Runtime
Interface](https://discuss.tvm.apache.org/t/rfc-utvm-embedded-c-runtime-interface/9951)
is augmented with the `tvm_mynetwork_devices` structure which contains
implemented `tvm_device_t` `struct`s for each device used by the network:
Review comment:
i forget if we discussed this already, but @manupa-arm 's RFC used a
different casing so let's just quickly address this here: google c++ styleguide
would have us call this `TvmGenMynetworkDevices`. I kind of like the `_`
separator however, since this name is generated.
##########
File path: rfcs/0031-devices-api.md
##########
@@ -306,6 +293,8 @@ Another route to take is to treat RTOSes as entirely
separate from TVM, requirin
# Unresolved questions
[unresolved-questions]: #unresolved-questions
+Is coupling `Target`s with this Device API to the C Runtime acceptable? From
the authors point of view this seems an acceptable trade off given the devices
won't function correctly without the flow control.
Review comment:
i think it's likely that
[TargetDevice](https://github.com/apache/tvm/pull/8892) will play a role here.
##########
File path: rfcs/0031-devices-api.md
##########
@@ -0,0 +1,363 @@
+- Feature Name: C Device API
+- Start Date: 02-08-2021
+- RFC PR: [apache/tvm-rfcs#31](https://github.com/apache/tvm-rfcs/pull/31)
+- GitHub Issue: [apache/tvm#0000](https://github.com/apache/tvm/issues/0000)
+
+
+# Summary
+[summary]: #summary
+This RFC aims to provide an API which can be used by the C runtime to abstract
the variety of driver APIs for different platforms. This is specifically
catering towards RTOS abstractions for embedded device drivers and aims to
implement a subset of the overall Device API with supporting infrastructure to
enable future expansion.
+
+# Motivation
+[motivation]: #motivation
+
+When using an accelerator, such as the [Arm®
Ethos™-U](https://github.com/apache/tvm-rfcs/pull/11), an Embedded
Real-Time Operating System (RTOS) will provide a device abstraction to access
the device resource. When using these abstractions, TVM needs to understand how
to interact with a device for a given platform.
+
+Taking the common example of a UART interface (imagining the accelerator is
communicated to via this interface); in Zephyr, this would look similar to:
+
+```c
+#include <zephyr.h>
+#include <device.h>
+
+struct device *uart_dev = device_get_binding("USART0");
+
+char data[] = "Hello World!\r\n";
+uart_tx(uart_dev, data, sizeof(data), 100);
+```
+
+Whereas in CMSIS, this would look more similar to:
+
+```c
+ARM_DRIVER_USART* uart_dev = &Driver_USART0;
+uart_dev->Initialize(NULL);
+
+char data[] = "Hello World!\r\n";
+uart_dev->Send(data, sizeof(data)/sizeof(data[0]));
+```
+
+In this example, you can see the diversity of RTOS implementations for drivers
and why it's required to provide a flexible abstraction to pass devices for
micro targets.
+
+# Guide-level explanation
+[guide-level-explanation]: #guide-level-explanation
+
+## User App
+`tvm_device_<device>_t`s are implemented for each RTOS or platform required,
these are included by the user who chooses as appropriate for their
application. Notably, to avoid dynamic allocation, the user must provide the
`tvm_device_<device>_t` struct and initialise it rather than it being created
and setup for them in the API. This is augmented by named functions for each
device, examples in the case of the "woofles" accelerator:
+
+```c
Review comment:
i think the reason i'm hoping for a life-cycle diagram is that the
actions Activate/Open could take are quite varied. i agree the call scheme
below is laid out in call order, but there's nothing explicit which states this
so you just have to know that in advance. could you add something more
explicit? i think the life-cycle diagram is the best thing as we can iterate on
it as we add features here.
--
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]
