<p><b>@cladmi</b> requested changes on this pull request.</p>

<p>I agree with the functional part but still have some remarks with documentation/naming. Please see inline.</p><hr>

<p>In <a href="https://github.com/RIOT-OS/RIOT/pull/11679#discussion_r292940556">tests/xtimer_mutex_lock_timeout/main.c</a>:</p>
<pre style='color:#555'>> @@ -0,0 +1,119 @@
+/*
+ * Copyright (C) 2019 Freie Universitaet Berlin,
</pre>
⬇️ Suggested change
<pre style="color: #555">- * Copyright (C) 2019 Freie Universitaet Berlin,
+ * Copyright (C) 2019 Freie Universität Berlin,
</pre>


<hr>

<p>In <a href="https://github.com/RIOT-OS/RIOT/pull/11679#discussion_r292940649">tests/xtimer_mutex_lock_timeout/tests/01-run.py</a>:</p>
<pre style='color:#555'>> @@ -0,0 +1,33 @@
+#!/usr/bin/env python3
+
+#  Copyright (C) 2019 Freie Universitaet Berlin,
</pre>
⬇️ Suggested change
<pre style="color: #555">-#  Copyright (C) 2019 Freie Universitaet Berlin,
+#  Copyright (C) 2019 Freie Universität Berlin,
</pre>


<hr>

<p>In <a href="https://github.com/RIOT-OS/RIOT/pull/11679#discussion_r292942385">tests/xtimer_mutex_lock_timeout/main.c</a>:</p>
<pre style='color:#555'>> +    /* timeout at one millisecond (1000 us) to make sure it does not spin. */
+    if (xtimer_mutex_lock_timeout(&test_mutex, 1000) == 0) {
+        puts("OK");
+    }
+    else {
+        puts("error: mutex timed out");
+    }
+
+    return 0;
+}
+
+/**
+ * @brief   shell command to test xtimer_mutex_lock_timeout
+ *
+ * the mutex is locked before the function call and
+ * the timeout is greater than XTIMER_BACKOFF (no spinning)
</pre>
<p>Please update in the direction, "the timeout will occur while waiting on the mutex" it would explain more the case without the need to mention spinning.</p>

<hr>

<p>In <a href="https://github.com/RIOT-OS/RIOT/pull/11679#discussion_r292943300">tests/xtimer_mutex_lock_timeout/main.c</a>:</p>
<pre style='color:#555'>> +/**
+ * @brief   List of command for this application.
+ */
+static const shell_command_t shell_commands[] = {
+    { "mutex_timeout_n_spin_unlocked", "unlocked mutex and without spinning",
+      cmd_test_xtimer_mutex_lock_timeout_greater_backoff, },
+    { "mutex_timeout_n_spin_locked", "locked mutex and without spinning",
+      cmd_test_xtimer_mutex_lock_timeout_greater_backoff_locked, },
+    { NULL, NULL, NULL }
+};
+
+/**
+ * @brief   shell command to test xtimer_mutex_lock_timeout
+ *
+ * the mutex is not locked before the function call and
+ * the timeout is greater than XTIMER_BACKOFF (no spinning)
</pre>
<p>Please update in the direction, "the timeout callback will be removed before triggering" it would explain more the case.</p>

<hr>

<p>In <a href="https://github.com/RIOT-OS/RIOT/pull/11679#discussion_r292946518">tests/xtimer_mutex_lock_timeout/main.c</a>:</p>
<pre style='color:#555'>> + * @brief       testing xtimer_mutex_lock_timeout function
+ *
+ *
+ * @author      Julian Holzwarth <julian.holzwarth@fu-berlin.de>
+ *
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include "shell.h"
+#include "xtimer.h"
+
+/**
+ * Foward declarations
+ */
+static int cmd_test_xtimer_mutex_lock_timeout_greater_backoff(int argc,
</pre>
<p>Maybe update the function names to not mention 'backoff' but more what is tested, so testing with a free mutex and long timeout. Would match the documentation updates.</p>
<p>Same for the other function.</p>

<hr>

<p>In <a href="https://github.com/RIOT-OS/RIOT/pull/11679#discussion_r292946653">sys/xtimer/xtimer.c</a>:</p>
<pre style='color:#555'>>  typedef struct {
     mutex_t *mutex;
     thread_t *thread;
-    int timeout;
+    volatile int timeout;
</pre>
<p>Agreed with the change, the value is modified in another context.</p>

<hr>

<p>In <a href="https://github.com/RIOT-OS/RIOT/pull/11679#discussion_r292947786">tests/xtimer_mutex_lock_timeout/main.c</a>:</p>
<pre style='color:#555'>> + *
+ * @param[in] argc  Number of arguments
+ * @param[in] argv  Array of arguments
+ *
+ * @return 0 on success
+ */
+static int cmd_test_xtimer_mutex_lock_timeout_greater_backoff(int argc,
+                                                              char **argv)
+{
+    (void)argc;
+    (void)argv;
+    puts("starting test: xtimer mutex lock timeout");
+    mutex_t test_mutex = MUTEX_INIT;
+
+    /* timeout at one millisecond (1000 us) to make sure it does not spin. */
+    if (xtimer_mutex_lock_timeout(&test_mutex, 1000) == 0) {
</pre>
<p>Please put the <code>1000</code> as a macro with the comment before so it can be re-used in both functions.</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/RIOT-OS/RIOT/pull/11679?email_source=notifications&email_token=ABE7WYATHYJYAJCWJFZFCQTP2ECPJA5CNFSM4HXIH2VKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB3KEYDI#pullrequestreview-248794125">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/ABE7WYFNP2GCDXHXXWHS7DDP2ECPJANCNFSM4HXIH2VA">mute the thread</a>.<img src="https://github.com/notifications/beacon/ABE7WYBZPMYEMBWVKODYQBLP2ECPJA5CNFSM4HXIH2VKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB3KEYDI.gif" height="1" width="1" alt="" /></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/RIOT-OS/RIOT/pull/11679?email_source=notifications\u0026email_token=ABE7WYATHYJYAJCWJFZFCQTP2ECPJA5CNFSM4HXIH2VKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB3KEYDI#pullrequestreview-248794125",
"url": "https://github.com/RIOT-OS/RIOT/pull/11679?email_source=notifications\u0026email_token=ABE7WYATHYJYAJCWJFZFCQTP2ECPJA5CNFSM4HXIH2VKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB3KEYDI#pullrequestreview-248794125",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>