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

<p>Some minor changes should be applied but this is already in a pretty good shape.</p>
<ul>
<li>
<p>#define DAC_CLOCK_DEFAULT should belongs to boards/*/include/periph_conf.h</p>
</li>
<li>
<p>Would be good to select the reference voltage through periph_conf.h too<br>
I know VDDANA is broken on SAME54 so external reference must be used but VDDANA works on SAML21 for instance.</p>
</li>
<li>
<p>SAML10/SAML11 DAC are connected to 32kHz GCLK ? Maybe it's time to allocate an unused GCLK to provide a faster clock.</p>
</li>
</ul><hr>

<p>In <a href="https://github.com/RIOT-OS/RIOT/pull/13965#discussion_r418540141">cpu/sam0_common/periph/dac.c</a>:</p>
<pre style='color:#555'>> + * @author      Benjamin Valentin <benpicco@beuth-hochschule.de>
+ *
+ * @}
+ */
+
+#include "cpu.h"
+#include "periph/dac.h"
+#include "periph/gpio.h"
+
+#include "byteorder.h"
+
+#ifndef DAC_CLOCK
+#define DAC_CLOCK   DAC_CLOCK_DEFAULT
+#endif
+
+#define DAC_VAL(in) (in >> (16 - DAC_RES_BITS))
</pre>
<p>I don't get this.<br>
Why playing with the bit position here ?<br>
In the current state, DAC_DACCTRL_LEFTADJ is set to 0 and we don't use dithering so we should write data to bits [0..9] or  [0..11]. there is no need for this macro, just assign the value to the register directly.</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/13965#pullrequestreview-404157891">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/ABE7WYF63MVM776C7YAOXPTRPLFWLANCNFSM4MSLLFYQ">unsubscribe</a>.<img src="https://github.com/notifications/beacon/ABE7WYA3MDQUOQQUZD2AR4LRPLFWLA5CNFSM4MSLLFY2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGODALPLQY.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/13965#pullrequestreview-404157891",
"url": "https://github.com/RIOT-OS/RIOT/pull/13965#pullrequestreview-404157891",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>