[riot-notifications] [RIOT-OS/RIOT] fe310: Use ecall instruction for thread yield (#15736)

Marian Buschsieweke notifications at github.com
Mon Jan 11 11:52:18 CET 2021


@maribu commented on this pull request.

Some comments inline, mostly style nits

>  {
-    /* Use SW intr to schedule context switch */
-    CLINT_REG(CLINT_MSIP) = 1;
+    /* function arguments are in a0 and a1 as per ABI */
+    __asm__ volatile (
+    "mv a0, %[num] \n"
+    "mv a1, %[ctx] \n"
+    "ECALL\n"
+    "ret"
+    :

```suggestion
    : /* no outputs */
```

>  {
-    /* Use SW intr to schedule context switch */
-    CLINT_REG(CLINT_MSIP) = 1;
+    /* function arguments are in a0 and a1 as per ABI */
+    __asm__ volatile (
+    "mv a0, %[num] \n"
+    "mv a1, %[ctx] \n"
+    "ECALL\n"
+    "ret"
+    :
+    : [num] "r" (num), [ctx] "r" (ctx)
+    :

```suggestion
    : "memory"
```

Without a memory barrier, the compiler might move stores and loads across a call to `thread_yield_higher()`. (At least with `LTO=1`.)

> +            case 8:  /* ECALL from user mode */
+            case 11: /* ECALL from machine mode */

Style nit: `case` labels and the `switch` statement should have the same indent level, only code after the `case` should have an indent.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/15736#pullrequestreview-565240631
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20210111/455ca670/attachment.htm>


More information about the notifications mailing list