<div dir="ltr"><div><div><div><div><div><div><div><div>Hi,<br></div>I have updated the patch and tested getentropy() , it did seem to work (Logs: <a href="https://gist.github.com/madaari/17bdec209eac993538b0192fba014918">here</a>).<br></div>Moreover, i've added BSD-2-clause license please verify it once. Also, i need to shift<br></div>TRNG register structure from AM335x.h to bbb_getentropy.c . since, keeping it in am335x.h<br></div>requires including stdint.h (for uint32_t etc) which i don't think would be right, do let me know if this<br></div>is the right way.<br><br></div>Here's the patch:<br>From f87b39708dd06482957fc7b33e78ab0ee095287b Mon Sep 17 00:00:00 2001<br>From: Udit agarwal <<a href="mailto:dev.madaari@gmail.com">dev.madaari@gmail.com</a>><br>Date: Sun, 11 Mar 2018 23:34:17 +0530<br>Subject: [PATCH] Added getentropy support to BBB BSP<br><br>---<br> bsps/arm/include/libcpu/am335x.h | 22 +++-<br> c/src/lib/libbsp/arm/beagle/Makefile.am | 4 +-<br> .../libbsp/arm/beagle/getentropy/bbb_getentropy.c | 128 +++++++++++++++++++++<br> 3 files changed, 152 insertions(+), 2 deletions(-)<br> create mode 100644 c/src/lib/libbsp/arm/beagle/getentropy/bbb_getentropy.c<br><br>diff --git a/bsps/arm/include/libcpu/am335x.h b/bsps/arm/include/libcpu/am335x.h<br>index 367e97c..bbdc8d0 100644<br>--- a/bsps/arm/include/libcpu/am335x.h<br>+++ b/bsps/arm/include/libcpu/am335x.h<br>@@ -14,6 +14,9 @@<br> * Modified by Ben Gras <<a href="mailto:beng@shrike-systems.com">beng@shrike-systems.com</a>> to add lots<br> * of beagleboard/beaglebone definitions, delete lpc32xx specific<br> * ones, and merge with some other header files.<br>+ *<br>+ * Modified by Udit agarwal <<a href="mailto:dev.madaari@gmail.com">dev.madaari@gmail.com</a>> to add random<br>+ * number generating module definitions.<br> */<br> <br> #if !defined(_AM335X_H_)<br>@@ -701,4 +704,21 @@<br> #define AM335X_CM_PER_OCPWP_L3_CLKSTCTRL_CLKACTIVITY_OCPWP_L4_GCLK (0x00000020u)<br> #define AM335X_I2C_INT_STOP_CONDITION AM335X_I2C_IRQSTATUS_BF<br> <br>-#endif<br>+/* TRNG Register */<br>+<br>+/* RNG base address */<br>+#define RNG_BASE 0x48310000<br>+/* RNG clock control */<br>+#define CM_PER_RNG_CLKCTRL (AM335X_CM_PER_ADDR | (9 << 4))<br>+/* rng module clock status bits */<br>+#define AM335X_CLK_RNG_BIT_MASK (0x30000)<br>+/* Offset from RNG base for output ready flag */<br>+#define RNG_STATUS_RDY (1u << 0) <br>+/* Offset from RNG base for FRO related error */<br>+#define RNG_STATUS_ERR (1u << 1) <br>+/* Offset from RNG base for clock status */<br>+#define RNG_STATUS_CLK (1u << 31) <br>+/* enable module */<br>+#define AM335X_RNG_ENABLE (1 << 10) <br>+<br>+#endif<br>\ No newline at end of file<br>diff --git a/c/src/lib/libbsp/arm/beagle/Makefile.am b/c/src/lib/libbsp/arm/beagle/Makefile.am<br>index 8251660..5d5ade3 100644<br>--- a/c/src/lib/libbsp/arm/beagle/Makefile.am<br>+++ b/c/src/lib/libbsp/arm/beagle/Makefile.am<br>@@ -40,7 +40,6 @@ libbsp_a_LIBADD =<br> <br> # Shared<br> libbsp_a_SOURCES += ../../shared/bootcard.c<br>-libbsp_a_SOURCES += ../../shared/getentropy-cpucounter.c<br> libbsp_a_SOURCES += ../../shared/src/bsp-fdt.c<br> libbsp_a_SOURCES += ../../shared/bspclean.c<br> libbsp_a_SOURCES += ../../shared/bspgetworkarea.c<br>@@ -88,6 +87,9 @@ libbsp_a_SOURCES += gpio/bbb-gpio.c<br> #pwm<br> libbsp_a_SOURCES += pwm/pwm.c<br> <br>+#getentropy<br>+libbsp_a_SOURCES += getentropy/bbb_getentropy.c<br>+<br> #RTC<br> libbsp_a_SOURCES += rtc.c<br> libbsp_a_SOURCES += ../../shared/tod.c<br>diff --git a/c/src/lib/libbsp/arm/beagle/getentropy/bbb_getentropy.c b/c/src/lib/libbsp/arm/beagle/getentropy/bbb_getentropy.c<br>new file mode 100644<br>index 0000000..117c6b8<br>--- /dev/null<br>+++ b/c/src/lib/libbsp/arm/beagle/getentropy/bbb_getentropy.c<br>@@ -0,0 +1,128 @@<br>+/**<br>+* @file<br>+*<br>+* @ingroup arm_beagle<br>+*<br>+* @brief Getentropy implementation on BeagleBone Black BSP<br>+*/<br>+<br>+/*<br>+*Copyright 2018 Udit kumar agarwal <dev.madaari at <a href="http://gmail.com">gmail.com</a>><br>+*<br>+*Redistribution and use in source and binary forms, with or without <br>+*modification, are permitted provided that the following conditions are met:<br>+*<br>+*1. Redistributions of source code must retain the above copyright notice, <br>+* this list of conditions and the following disclaimer.<br>+*<br>+*2. Redistributions in binary form must reproduce the above copyright notice, <br>+* this list of conditions and the following disclaimer in the documentation <br>+* and/or other materials provided with the distribution.<br>+*<br>+*THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" <br>+*AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE <br>+*IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE <br>+*ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE <br>+*LIABLE FOR ANY DIRECT,INDIRECT, INCIDENTAL, SPECIAL,EXEMPLARY, OR <br>+*CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF <br>+*SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA,OR PROFITS; OR BUSINESS <br>+*INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,WHETHER IN CONTRACT,<br>+*STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN <br>+*ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY<br>+*OF SUCH DAMAGE.<br>+*/<br>+<br>+#include <libcpu/am335x.h><br>+#include <unistd.h><br>+#include <string.h><br>+#include <rtems/sysinit.h><br>+#include <stdint.h><br>+<br>+/* max refill 34 * 256 cycles */<br>+#define AM335X_RNG_MAX_REFILL (34 << 16) <br>+/* min refill 33 * 64 cycles */<br>+#define AM335X_RNG_MIN_REFILL (33 << 0) <br>+/* startup 33 * 256 cycles */<br>+#define AM335X_RNG_STARTUP_CYCLES (33 << 16) <br>+<br>+/* TRNG register structure */<br>+struct bbb_trng_register <br>+{<br>+ uint64_t output; /* 00 */ <br>+ uint32_t status; /* 08 */ <br>+ uint32_t irq_en; /* 0c */ <br>+ uint32_t status_clr; /* 10 */ <br>+ uint32_t control; /* 14 */ <br>+ uint32_t config; /* 18 */ <br>+};<br>+typedef struct bbb_trng_register bbb_trng_register;<br>+<br>+/* maximun and minimum refill cycle sets the number of samples to be taken <br>+ from FRO to generate random number */<br>+static void am335x_rng_enable(volatile bbb_trng_register *rng)<br>+{<br>+ rng->control = rng->config = 0;<br>+ rng->config |= AM335X_RNG_MIN_REFILL | AM335X_RNG_MAX_REFILL ;<br>+ rng->control |= AM335X_RNG_STARTUP_CYCLES | AM335X_RNG_ENABLE ;<br>+}<br>+<br>+static void am335x_rng_clock_enable(void)<br>+{<br>+ *(volatile uint8_t *) CM_PER_RNG_CLKCTRL = 2;<br>+ while( <br>+ *(volatile uint32_t *) CM_PER_RNG_CLKCTRL & <br>+ AM335X_CLK_RNG_BIT_MASK <br>+ ) {<br>+ /* wait */<br>+ }<br>+}<br>+<br>+static uint64_t trng_getranddata(volatile bbb_trng_register *rng)<br>+{<br>+ uint64_t output = rng->output;<br>+ return output;<br>+}<br>+<br>+int getentropy(void *ptr, size_t n)<br>+{<br>+ volatile bbb_trng_register *rng = (bbb_trng_register*) RNG_BASE;<br>+ am335x_rng_enable(rng);<br>+ while (n > 0) <br>+ {<br>+ uint64_t random;<br>+ size_t copy;<br>+<br>+ /* wait untill RNG becomes ready with next set of random data */<br>+ while( ( rng->status & RNG_STATUS_RDY ) == 0 )<br>+ {<br>+ /* wait */<br>+ }<br>+<br>+ random = trng_getranddata(rng);<br>+<br>+ /* Checking for error by masking all bits other then error bit in<br>+ status register */<br>+ if( ((rng->status & RNG_STATUS_ERR)>>1) == 1)<br>+ {<br>+ /* clear the status flag after reading to generate new random <br>+ value */<br>+ rng->status_clr = RNG_STATUS_RDY;<br>+ copy = sizeof(random);<br>+ if ( n < copy ) <br>+ {<br>+ copy = n;<br>+ }<br>+ memcpy(ptr, &random, copy);<br>+ n -= copy;<br>+ ptr = (char*)ptr + copy;<br>+ }<br>+ }<br>+<br>+ return 0;<br>+}<br>+<br>+RTEMS_SYSINIT_ITEM(<br>+ am335x_rng_clock_enable,<br>+ RTEMS_SYSINIT_DEVICE_DRIVERS,<br>+ RTEMS_SYSINIT_ORDER_LAST<br>+);<br>\ No newline at end of file<br>-- <br>1.9.1<br><br></div>Best regards,<br></div>Udit agarwal<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 7, 2018 at 11:40 PM, Christian Mauderer <span dir="ltr"><<a href="mailto:list@c-mauderer.de" target="_blank">list@c-mauderer.de</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hello Udit,<br>
<br>
let me start with a hint: It's quite normal that the first patches of a<br>
contributor get a lot of attention and have a lot of revisions. That can<br>
be quite annoying for the one who writes the patch. But please<br>
understand it in the way it's thought: As suggestions for improvement.<br>
<br>
Did you already test that patches? There is a (quite basic) test in the<br>
RTEMS testsuite: libtests/getentropy01. This test should run without<br>
problems. If you have a debugger, you might also want to check whether<br>
your getentropy implementation is reached and not the generic one.<br>
<br>
The patch looks quite good already. But like I said: A lot of revisions.<br>
I added some further remarks in the patch below ;-)<br>
<br>
Best regards<br>
<br>
Christian<br>
<span class=""><br>
<br>
Am 07.03.2018 um 11:26 schrieb Udit agarwal:<br>
> Hi,<br>
</span><span class="">> I have updated the code, please have a look.<br>
> From 74b8f4f5b9dd929b71ed5fb9dd0bc7<wbr>21a6f27a28 Mon Sep 17 00:00:00 2001<br>
</span>> From: Udit agarwal <<a href="mailto:dev.madaari@gmail.com">dev.madaari@gmail.com</a> <mailto:<a href="mailto:dev.madaari@gmail.com">dev.madaari@gmail.com</a>><wbr>><br>
<span class="">> Date: Wed, 7 Mar 2018 15:52:13 +0530<br>
> Subject: [PATCH] Added getentropy support to beagle BSP<br>
><br>
> ---<br>
> bsps/arm/include/libcpu/<wbr>am335x.h | 34 ++++++++-<br>
> c/src/lib/libbsp/arm/beagle/<wbr>Makefile.am | 4 +<br>
> .../libbsp/arm/beagle/<wbr>getentropy/bbb_getentropy.c | 88<br>
> ++++++++++++++++++++++<br>
> 3 files changed, 125 insertions(+), 1 deletion(-)<br>
> create mode 100644 c/src/lib/libbsp/arm/beagle/<wbr>getentropy/bbb_getentropy.c<br>
><br>
> diff --git a/bsps/arm/include/libcpu/<wbr>am335x.h<br>
> b/bsps/arm/include/libcpu/<wbr>am335x.h<br>
> index 367e97c..92af6dc 100644<br>
> --- a/bsps/arm/include/libcpu/<wbr>am335x.h<br>
> +++ b/bsps/arm/include/libcpu/<wbr>am335x.h<br>
> @@ -14,6 +14,9 @@<br>
> * Modified by Ben Gras <<a href="mailto:beng@shrike-systems.com">beng@shrike-systems.com</a><br>
</span>> <mailto:<a href="mailto:beng@shrike-systems.com">beng@shrike-systems.<wbr>com</a>>> to add lots<br>
<span class="">> * of beagleboard/beaglebone definitions, delete lpc32xx specific<br>
> * ones, and merge with some other header files.<br>
> + *<br>
> + * Modified by Udit agarwal <<a href="mailto:dev.madaari@gmail.com">dev.madaari@gmail.com</a><br>
</span>> <mailto:<a href="mailto:dev.madaari@gmail.com">dev.madaari@gmail.com</a>><wbr>> to add random<br>
<div><div class="h5">> + * number generating module definitions along with register structure.<br>
> */<br>
> <br>
> #if !defined(_AM335X_H_)<br>
> @@ -701,4 +704,33 @@<br>
> #define AM335X_CM_PER_OCPWP_L3_<wbr>CLKSTCTRL_CLKACTIVITY_OCPWP_<wbr>L4_GCLK<br>
> (0x00000020u)<br>
> #define AM335X_I2C_INT_STOP_CONDITION AM335X_I2C_IRQSTATUS_BF<br>
> <br>
> -#endif<br>
> +/* TRNG Register */<br>
> +<br>
> +/* RNG base address */<br>
> +#define RNG_BASE 0x48310000<br>
> +/* RNG clock control */<br>
> +#define CM_PER_RNG_CLKCTRL (AM335X_CM_PER_ADDR | (9 << 4))<br>
> +/* rng module clock status bits */<br>
> +#define AM335X_CLK_RNG_BIT_MASK (0x30000)<br>
> +/* Offset from RNG base for output ready flag */<br>
> +#define RNG_STATUS_RDY (1u << 0) <br>
> +/* Offset from RNG base for FRO related error */<br>
> +#define RNG_STATUS_ERR (1u << 1)<br>
> +/* Offset from RNG base for clock status */<br>
> +#define RNG_STATUS_CLK (1u << 31) <br>
> +/* enable module */<br>
> +#define AM335X_RNG_ENABLE (1 << 10)<br>
> +<br>
> +/* TRNG register structure */<br>
> +struct bbb_trng_register<br>
> +{<br>
> + uint64_t output; /*00*/ <br>
> + uint32_t status; /*08*/ <br>
> + uint32_t irq_en; /*0c*/ <br>
> + uint32_t status_clr; /*10*/ <br>
> + uint32_t control; /*14*/ <br>
> + uint32_t config; /*18*/ <br>
> +};<br>
> +typedef struct bbb_trng_register bbb_trng_register;<br>
> +<br>
> +#endif<br>
> \ No newline at end of file<br>
> diff --git a/c/src/lib/libbsp/arm/beagle/<wbr>Makefile.am<br>
> b/c/src/lib/libbsp/arm/beagle/<wbr>Makefile.am<br>
> index 8251660..bf92081 100644<br>
> --- a/c/src/lib/libbsp/arm/beagle/<wbr>Makefile.am<br>
> +++ b/c/src/lib/libbsp/arm/beagle/<wbr>Makefile.am<br>
> @@ -88,9 +88,13 @@ libbsp_a_SOURCES += gpio/bbb-gpio.c<br>
> #pwm<br>
> libbsp_a_SOURCES += pwm/pwm.c<br>
> <br>
> +#getentropy<br>
> +libbsp_a_SOURCES += getentropy/bbb_getentropy.c<br>
> +<br>
<br>
</div></div>Adding that one is good. But please also remove the default<br>
implementation from the Makefile.am:<br>
<br>
libbsp_a_SOURCES += ../../shared/getentropy-<wbr>cpucounter.c<br>
<br>
Otherwise you might get problems during linking.<br>
<span class=""><br>
> #RTC<br>
> libbsp_a_SOURCES += rtc.c<br>
> libbsp_a_SOURCES += ../../shared/tod.c<br>
> +<br>
<br>
</span>You added a empty line in a section where you didn't change anything<br>
else. Normally that's not a good idea because it slightly obfuscates the<br>
history. Try to generate patches that only touch the area that is<br>
related to your change. If you really think that this line should be<br>
added, you should create a second patch with a description like "fix<br>
spacing" or similar. But I wouldn't suggest to do that.<br>
<span class=""><br>
> # Clock<br>
> libbsp_a_SOURCES += clock.c<br>
> libbsp_a_SOURCES += ../../shared/clockdrv_shell.h<br>
> diff --git a/c/src/lib/libbsp/arm/beagle/<wbr>getentropy/bbb_getentropy.c<br>
> b/c/src/lib/libbsp/arm/beagle/<wbr>getentropy/bbb_getentropy.c<br>
> new file mode 100644<br>
> index 0000000..5b7c5d0<br>
> --- /dev/null<br>
> +++ b/c/src/lib/libbsp/arm/beagle/<wbr>getentropy/bbb_getentropy.c<br>
> @@ -0,0 +1,88 @@<br>
> +/**<br>
> +* @file<br>
> +*<br>
> +* @ingroup arm_beagle<br>
> +*<br>
> +* @brief Getentropy implementation on BeagleBone Black BSP<br>
> +*/<br>
> +<br>
> +/*<br>
> +* Copyright (c) 2018 Udit kumar agarwal <dev.madaari at <a href="http://gmail.com" rel="noreferrer" target="_blank">gmail.com</a><br>
</span>> <<a href="http://gmail.com" rel="noreferrer" target="_blank">http://gmail.com</a>>><br>
<div><div class="h5">> +*<br>
> +* The license and distribution terms for this file may be<br>
> +* found in the file LICENSE in this distribution or at<br>
> +* <a href="http://www.rtems.org/license/LICENSE" rel="noreferrer" target="_blank">http://www.rtems.org/license/<wbr>LICENSE</a>.<br>
> +*/<br>
> +<br>
> +#include <libcpu/am335x.h><br>
> +#include <unistd.h><br>
> +#include <string.h><br>
> +#include <rtems/sysinit.h><br>
> +#include <rtems/types.h><br>
> +<br>
> +/* max refill 34 * 256 cycles */<br>
> +#define AM335X_RNG_MAX_REFILL (34 << 16)<br>
> +/* min refill 33 * 64 cycles */<br>
> +#define AM335X_RNG_MIN_REFILL (33 << 0)<br>
> +/* startup 33 * 256 cycles */<br>
> +#define AM335X_RNG_STARTUP_CYCLES (33 << 16)<br>
> +<br>
> +/* maximun and minimum refill cycle sets the number of samples to be taken<br>
> + from FRO to generate random number */<br>
> +static void am335x_rng_enable(bbb_trng_<wbr>register *rng)<br>
> +{<br>
> + rng->control = rng->config = 0;<br>
> + rng->config |= AM335X_RNG_MIN_REFILL | AM335X_RNG_MAX_REFILL ;<br>
> + rng->control |= AM335X_RNG_STARTUP_CYCLES | AM335X_RNG_ENABLE ;<br>
> +}<br>
> +<br>
> +static void am335x_rng_clock_enable(void)<br>
> +{<br>
> + *(volatile uint8_t *) CM_PER_RNG_CLKCTRL = 2;<br>
> + while( *(volatile uint32_t *) CM_PER_RNG_CLKCTRL &<br>
> AM335X_CLK_RNG_BIT_MASK ) {}<br>
<br>
</div></div>Is this line within 80 characters? The RTEMS style uses lines <= 80 chars.<br>
<br>
When I'm already at style: Although it's not strictly enforced in BSPs,<br>
RTEMS uses two spaces for indentation. See<br>
<a href="https://devel.rtems.org/wiki/Developer/Coding/Conventions" rel="noreferrer" target="_blank">https://devel.rtems.org/wiki/<wbr>Developer/Coding/Conventions</a><br>
<br>
Also I don't think we have a style rule for that I would suggest that<br>
you mark it if you have an intentionally empty loop. That makes it clear<br>
that you didn't just forget to write some code. E.g.<br>
<br>
while( some condition ) {<br>
/* wait */<br>
<span class=""> }<br>
<br>
> +}<br>
> +<br>
> +static uint64_t trng_getranddata(bbb_trng_<wbr>register *rng)<br>
> +{<br>
> + uint64_t output = rng->output;<br>
> + return output;<br>
> +}<br>
> +<br>
> +int getentropy(void *ptr, size_t n)<br>
> +{<br>
> + volatile const bbb_trng_register *rng = (bbb_trng_register*) RNG_BASE;<br>
> + am335x_rng_enable(rng);<br>
> + while (n > 0)<br>
> + {<br>
> + uint64_t random;<br>
> + size_t copy;<br>
> +<br>
> + /* wait untill RNG becomes ready with next set of random data */<br>
> + while( ( rng->status & RNG_STATUS_RDY ) == 0 );<br>
<br>
</span>Again: I would suggest to mark it that this loop is intentionally empty.<br>
<span class=""><br>
> +<br>
> + random = trng_getranddata(rng);<br>
> +<br>
> + /* Checking for error by masking all bits other then error bit<br>
> in status register */<br>
<br>
</span><= 80 Chars?<br>
<span class=""><br>
> + if( ((rng->status & RNG_STATUS_ERR)>>1) == 0)<br>
> + {<br>
> + /* clear the status flag after reading to generate new<br>
> random value*/<br>
<br>
</span><= 80 Chars?<br>
<div><div class="h5"><br>
> + rng->status_clr = RNG_STATUS_RDY;<br>
> + copy = sizeof(random);<br>
> + if ( n < copy )<br>
> + {<br>
> + copy = n;<br>
> + }<br>
> + memcpy(ptr, &random, copy);<br>
> + n -= copy;<br>
> + ptr = (char*)ptr + copy;<br>
> + }<br>
> + }<br>
> +<br>
> + return 0;<br>
> +}<br>
> +<br>
> +RTEMS_SYSINIT_ITEM(<br>
> + am335x_rng_clock_enable,<br>
> + RTEMS_SYSINIT_DEVICE_DRIVERS,<br>
> + RTEMS_SYSINIT_ORDER_LAST<br>
> +);<br>
> \ No newline at end of file<br>
> --<br>
> 1.9.1<br>
><br>
> Regards,<br>
> Udit agarwal<br>
><br>
> On Tue, Mar 6, 2018 at 11:05 PM, Gedare Bloom <<a href="mailto:gedare@rtems.org">gedare@rtems.org</a><br>
</div></div><span class="">> <mailto:<a href="mailto:gedare@rtems.org">gedare@rtems.org</a>>> wrote:<br>
><br>
> On Tue, Mar 6, 2018 at 9:43 AM, Udit agarwal <<a href="mailto:dev.madaari@gmail.com">dev.madaari@gmail.com</a><br>
</span><span class="">> <mailto:<a href="mailto:dev.madaari@gmail.com">dev.madaari@gmail.com</a>><wbr>> wrote:<br>
> > Hi,<br>
> > Here's the updated code(I have also attached the patch PFA):<br>
> ><br>
> > From 96e6e1bfd8cffeef5d309eb0a07fe2<wbr>bfd086ef0a Mon Sep 17 00:00:00 2001<br>
> > From: Udit agarwal <<a href="mailto:dev.madaari@gmail.com">dev.madaari@gmail.com</a><br>
</span>> <mailto:<a href="mailto:dev.madaari@gmail.com">dev.madaari@gmail.com</a>><wbr>><br>
<span class="">> > Date: Tue, 6 Mar 2018 20:07:44 +0530<br>
> > Subject: [PATCH] Added getentropy support to BBB BSP<br>
> ><br>
> > ---<br>
</span>> > bsps/arm/beagle/<a href="http://headers.am" rel="noreferrer" target="_blank">headers.am</a> <<a href="http://headers.am" rel="noreferrer" target="_blank">http://headers.am</a>> <br>
<span class="">> | 1 +<br>
> > bsps/arm/beagle/include/bsp/<wbr>bbb_getentropy.h | 57<br>
> +++++++++++++++<br>
> > bsps/arm/include/libcpu/<wbr>am335x.h | 16 +++++<br>
> > c/src/lib/libbsp/arm/beagle/<wbr>Makefile.am | 4 ++<br>
> > .../libbsp/arm/beagle/<wbr>getentropy/bbb_getentropy.c | 80<br>
> > ++++++++++++++++++++++<br>
> > 5 files changed, 158 insertions(+)<br>
> > create mode 100644 bsps/arm/beagle/include/bsp/<wbr>bbb_getentropy.h<br>
> > create mode 100644<br>
> c/src/lib/libbsp/arm/beagle/<wbr>getentropy/bbb_getentropy.c<br>
> ><br>
</span>> > diff --git a/bsps/arm/beagle/<a href="http://headers.am" rel="noreferrer" target="_blank">headers.am</a> <<a href="http://headers.am" rel="noreferrer" target="_blank">http://headers.am</a>><br>
> b/bsps/arm/beagle/<a href="http://headers.am" rel="noreferrer" target="_blank">headers.am</a> <<a href="http://headers.am" rel="noreferrer" target="_blank">http://headers.am</a>><br>
> > index 6692d0b..ac4ff7c 100644<br>
> > --- a/bsps/arm/beagle/<a href="http://headers.am" rel="noreferrer" target="_blank">headers.am</a> <<a href="http://headers.am" rel="noreferrer" target="_blank">http://headers.am</a>><br>
> > +++ b/bsps/arm/beagle/<a href="http://headers.am" rel="noreferrer" target="_blank">headers.am</a> <<a href="http://headers.am" rel="noreferrer" target="_blank">http://headers.am</a>><br>
<div><div class="h5">> > @@ -12,3 +12,4 @@ include_bsp_HEADERS +=<br>
> > ../../../../../../bsps/arm/<wbr>beagle/include/bsp/bbb-pwm.h<br>
> > include_bsp_HEADERS +=<br>
> > ../../../../../../bsps/arm/<wbr>beagle/include/bsp/<wbr>beagleboneblack.h<br>
> > include_bsp_HEADERS +=<br>
> ../../../../../../bsps/arm/<wbr>beagle/include/bsp/i2c.h<br>
> > include_bsp_HEADERS +=<br>
> ../../../../../../bsps/arm/<wbr>beagle/include/bsp/irq.h<br>
> > +include_bsp_HEADERS +=<br>
> > ../../../../../../bsps/arm/<wbr>beagle/include/bsp/bbb_<wbr>getentropy.h<br>
> > diff --git a/bsps/arm/beagle/include/bsp/<wbr>bbb_getentropy.h<br>
> > b/bsps/arm/beagle/include/bsp/<wbr>bbb_getentropy.h<br>
> > new file mode 100644<br>
> > index 0000000..5e46f89<br>
> > --- /dev/null<br>
> > +++ b/bsps/arm/beagle/include/bsp/<wbr>bbb_getentropy.h<br>
> > @@ -0,0 +1,57 @@<br>
> > +/**<br>
> > + * @file<br>
> > + *<br>
> > + * @ingroup arm_beagle<br>
> > + *<br>
> > + * @brief TRNG Definations<br>
> > + */<br>
> > +<br>
> > +/*<br>
> > + * Copyright (c) 2018 Udit kumar agarwal <dev.madaari at<br>
</div></div>> <a href="http://gmail.com" rel="noreferrer" target="_blank">gmail.com</a> <<a href="http://gmail.com" rel="noreferrer" target="_blank">http://gmail.com</a>>><br>
<span class="">> > + *<br>
> > + * The license and distribution terms for this file may be<br>
> > + * found in the file LICENSE in this distribution or at<br>
> > + * <a href="http://www.rtems.org/license/LICENSE" rel="noreferrer" target="_blank">http://www.rtems.org/license/<wbr>LICENSE</a><br>
</span>> <<a href="http://www.rtems.org/license/LICENSE" rel="noreferrer" target="_blank">http://www.rtems.org/license/<wbr>LICENSE</a>>.<br>
<div><div class="h5">> > + */<br>
> > +<br>
> > +<br>
> > +#ifndef _TRNG_<br>
> > +#define _TRNG_<br>
> > +<br>
> ><br>
> +/*---------------------------<wbr>------------------------------<wbr>---------------------<br>
> Please look at similar files in RTEMS for how to add comments.<br>
> We do not use this style /* --------------------<br>
> And we don't put this kind of comment about the file sections.<br>
><br>
> > + * Headers<br>
> > +<br>
> > *-----------------------------<wbr>------------------------------<wbr>-----------------*/<br>
> > +<br>
> > +#include <libcpu/am335x.h><br>
> > +<br>
> > +<br>
> > /*----------------------------<wbr>------------------------------<wbr>--------------------<br>
> > + * Register structure<br>
> > +<br>
> > *-----------------------------<wbr>------------------------------<wbr>-----------------*/<br>
> > +<br>
> > +struct rng {<br>
> > + uint64_t output; /*00*/<br>
> > + uint32_t status; /*08*/<br>
> > + uint32_t irq_en; /*0c*/<br>
> > + uint32_t status_clr; /*10*/<br>
> > + uint32_t control; /*14*/<br>
> > + uint32_t config; /*18*/<br>
> > +};<br>
> for an exported header, the struct namespace needs to be user<br>
> friendly. See similar files at this layer for how to name things.<br>
> Probably, this can be defined in the am335x.h header?<br>
><br>
> > +<br>
> > +typedef volatile struct rng *rng_ref_t;<br>
> I'm not such a big fan of using the typedef to hide things like<br>
> volatile or pointer notation. I don't see a need for this usually.<br>
><br>
> > +<br>
> > +/*---------------------------<wbr>------------------------------<wbr>-------------------*/<br>
> > +/* Exported functions<br>
> > */<br>
> > +/*---------------------------<wbr>------------------------------<wbr>-------------------*/<br>
> > +<br>
> > +/* configure and enable RNG module */<br>
> > +static void am335x_rng_enable(rng_ref_t);<br>
> > +/* enable module clock for random number generator */<br>
> > +static void am335x_rng_clock_enable(void);<br>
> > +/* outputs random data */<br>
> > +static uint64_t trng_getranddata(rng_ref_t);<br>
><br>
> static keyword implies these are not exported.<br>
><br>
> > +/* Copy random data of a specified size to the pointer supplied */<br>
> > +int getentropy(void *ptr, size_t);<br>
> I guess getentropy() is already exported by a different header file<br>
> (unistd.h) in libc? This doesn't belong here.<br>
><br>
> I suspect this header file is not actually necessary.<br>
><br>
> > +<br>
> > +<br>
> > +<br>
> Don't use multiple blank lines in a row. Only one empty line is ever<br>
> needed by RTEMS style.<br>
><br>
> > +#endif /* #ifndef _TRNG_ */<br>
> > \ No newline at end of file<br>
> > diff --git a/bsps/arm/include/libcpu/<wbr>am335x.h<br>
> > b/bsps/arm/include/libcpu/<wbr>am335x.h<br>
> > index 367e97c..8b58f1a 100644<br>
> > --- a/bsps/arm/include/libcpu/<wbr>am335x.h<br>
> > +++ b/bsps/arm/include/libcpu/<wbr>am335x.h<br>
> > @@ -14,6 +14,9 @@<br>
</div></div>> > * Modified by Ben Gras <<a href="mailto:beng@shrike-systems.com">beng@shrike-systems.com</a> <mailto:<a href="mailto:beng@shrike-systems.com">beng@shrike-systems.<wbr>com</a>>> to add lots<br>
<span class="">> > * of beagleboard/beaglebone definitions, delete lpc32xx specific<br>
> > * ones, and merge with some other header files.<br>
> > + *<br>
</span>> > + * Modified by Udit agarwal <<a href="mailto:dev.madaari@gmail.com">dev.madaari@gmail.com</a> <mailto:<a href="mailto:dev.madaari@gmail.com">dev.madaari@gmail.com</a>><wbr>> to add random<br>
<div><div class="h5">> > + * number generating module definations<br>
> typo: definitions<br>
> add a period (full stop).<br>
><br>
> > */<br>
> ><br>
> > #if !defined(_AM335X_H_)<br>
> > @@ -701,4 +704,17 @@<br>
> > #define AM335X_CM_PER_OCPWP_L3_<wbr>CLKSTCTRL_CLKACTIVITY_OCPWP_<wbr>L4_GCLK<br>
> > (0x00000020u)<br>
> > #define AM335X_I2C_INT_STOP_CONDITION AM335X_I2C_IRQSTATUS_BF<br>
> ><br>
> > +/* TRNG Register */<br>
> > +<br>
> > +/* RNG base address */<br>
> > +#define RNG_BASE 0x48310000<br>
> > +/* RNG clock control */<br>
> > +#define CM_PER_RNG_CLKCTRL AM335X_CM_PER_ADDR | 9<<4<br>
> I'd put parentheses around this to avoid any problems with macro<br>
> expansions and order of operations.<br>
><br>
> > +/* Offset from RNG base for output ready flag */<br>
> > +#define RNG_STATUS_RDY (1u << 0)<br>
> > +/* Offset from RNG base for FRO related error*/<br>
> > +#define RNG_STATUS_ERR (1u << 1)<br>
> > +/* Offset from RNG base for clock status */<br>
> > +#define RNG_STATUS_CLK (1u << 31)<br>
> > +<br>
> > #endif<br>
> > diff --git a/c/src/lib/libbsp/arm/beagle/<wbr>Makefile.am<br>
> > b/c/src/lib/libbsp/arm/beagle/<wbr>Makefile.am<br>
> > index 8251660..bf92081 100644<br>
> > --- a/c/src/lib/libbsp/arm/beagle/<wbr>Makefile.am<br>
> > +++ b/c/src/lib/libbsp/arm/beagle/<wbr>Makefile.am<br>
> > @@ -88,9 +88,13 @@ libbsp_a_SOURCES += gpio/bbb-gpio.c<br>
> > #pwm<br>
> > libbsp_a_SOURCES += pwm/pwm.c<br>
> ><br>
> > +#getentropy<br>
> > +libbsp_a_SOURCES += getentropy/bbb_getentropy.c<br>
> > +<br>
> > #RTC<br>
> > libbsp_a_SOURCES += rtc.c<br>
> > libbsp_a_SOURCES += ../../shared/tod.c<br>
> > +<br>
> > # Clock<br>
> > libbsp_a_SOURCES += clock.c<br>
> > libbsp_a_SOURCES += ../../shared/clockdrv_shell.h<br>
> > diff --git a/c/src/lib/libbsp/arm/beagle/<wbr>getentropy/bbb_getentropy.c<br>
> > b/c/src/lib/libbsp/arm/beagle/<wbr>getentropy/bbb_getentropy.c<br>
> > new file mode 100644<br>
> > index 0000000..cab2102<br>
> > --- /dev/null<br>
> > +++ b/c/src/lib/libbsp/arm/beagle/<wbr>getentropy/bbb_getentropy.c<br>
> > @@ -0,0 +1,80 @@<br>
> > +/**<br>
> > +* @file<br>
> > +*<br>
> > +* @ingroup arm_beagle<br>
> > +*<br>
> > +* @brief Getentropy implementation on BeagleBone Black BSP<br>
> > +*/<br>
> > +<br>
> > +/*<br>
> > +* Copyright (c) 2018 Udit kumar agarwal <dev.madaari at <a href="http://gmail.com" rel="noreferrer" target="_blank">gmail.com</a><br>
</div></div>> <<a href="http://gmail.com" rel="noreferrer" target="_blank">http://gmail.com</a>>><br>
<span class="">> > +*<br>
> > +* The license and distribution terms for this file may be<br>
> > +* found in the file LICENSE in this distribution or at<br>
> > +* <a href="http://www.rtems.org/license/LICENSE" rel="noreferrer" target="_blank">http://www.rtems.org/license/<wbr>LICENSE</a><br>
</span><span class="">> <<a href="http://www.rtems.org/license/LICENSE" rel="noreferrer" target="_blank">http://www.rtems.org/license/<wbr>LICENSE</a>>.<br>
> > +*/<br>
> > +<br>
</span><div><div class="h5">> > +#include <unistd.h><br>
> > +#include <stdint.h><br>
> > +#include <string.h><br>
> > +#include <rtems/sysinit.h><br>
> > +<br>
> > +<br>
> > +/* maximun and minimun refill cycle sets the number of samples to<br>
> be taken<br>
> typo: minimum<br>
> > + from FRO to generate random number */<br>
> > +static void am335x_rng_enable(rng_ref_t rng){<br>
> put { on a new line.<br>
><br>
> > + rng->config = 0<br>
> > + | 34 << 16 /* max refill 34 * 256 cycles */<br>
> indent broken lines, but<br>
> > + | 33 << 0; /* min refill 33 * 64 cycles */<br>
> You can fit this on one line, remove the comments and put them above<br>
> the line, e.g.<br>
> rng->config = 34<<16 | 33;<br>
><br>
> I would prefer not to have these hard-coded values though it is<br>
> fragile. what means 34, 16, and 33?<br>
> #define AM335X_RNG_MAX_REFILL (34 << 16)<br>
> #define AM335X_RNG_MIN_REFILL (33)<br>
> #define AM335X_RNG_CONFIG_REFILL (AM335X_RNG_MAX_REFILL |<br>
> AM335X_RNG_MIN_REFILL)<br>
><br>
> rng.config = rng.control = 0;<br>
> rng.config |= AM335X_RNG_CONFIG_REFILL_TIME;<br>
><br>
> > + rng->control = 0<br>
> > + | 33 << 16 /* startup 33 * 256 cycles */<br>
> > + | 1 << 10; /* enable module */<br>
><br>
> rng.control |= AM335X_RNG_CONTROL_STARTUP;<br>
> rng.control |= AM335X_RNG_CONTROL_ENABLE;<br>
><br>
> for example is more readable. look around for other examples.<br>
><br>
> > +}<br>
> > +<br>
> > +static void am335x_rng_clock_enable(void)<br>
> > +{<br>
> > + *(volatile uint8_t *) CM_PER_RNG_CLKCTRL = 2;<br>
> > + while( *(volatile uint32_t *) CM_PER_RNG_CLKCTRL & 0x30000 ) {}<br>
> > +}<br>
> > +<br>
> > +static uint64_t trng_getranddata(rng_ref_t rng){<br>
> > + uint64_t output = rng->output;<br>
> > + return output;<br>
> > +}<br>
> > +<br>
> > +int getentropy(void *ptr, size_t n)<br>
> > +{<br>
> > + rng_ref_t const rng = (rng_ref_t) RNG_BASE;<br>
> > + am335x_rng_enable(rng);<br>
> > + while (n > 0) {<br>
> > + uint64_t random;<br>
> indent blocks of code.<br>
><br>
> > + size_t copy;<br>
> > +<br>
> > + /* wait untill RNG becomes ready with next set of random data */<br>
> > + while( ( rng->status & RNG_STATUS_RDY ) == 0 );<br>
> > +<br>
> > + random = trng_getranddata(rng);<br>
> > +<br>
> > + /* Checking for error by masking all bits other then error bit in<br>
> > status register */<br>
> > + if( ((rng->status & RNG_STATUS_ERR)>>1) == 0){<br>
> > +<br>
> > + /* clear the status flag after reading to generate new random<br>
> > value*/<br>
> > + rng->status_clr = RNG_STATUS_RDY;<br>
> > + copy = sizeof(random);<br>
> > + if ( n < copy ) {<br>
> > + copy = n;<br>
> ditto.<br>
><br>
> > + }<br>
> > + memcpy(ptr, &random, copy);<br>
> > + n -= copy;<br>
> > + ptr += copy;<br>
> pointer arithmetic on void pointer is not legal. You can portably use<br>
> ptr = (char*)ptr + copy;<br>
><br>
> > + }<br>
> > + }<br>
> > +<br>
> > + return 0;<br>
> > +}<br>
> > +<br>
> > +RTEMS_SYSINIT_ITEM(<br>
> > + am335x_rng_clock_enable,<br>
> > + RTEMS_SYSINIT_DEVICE_DRIVERS,<br>
> > + RTEMS_SYSINIT_ORDER_LAST<br>
> > +);<br>
> > \ No newline at end of file<br>
> > --<br>
> > 1.9.1<br>
> ><br>
> > Regards,<br>
> > Udit Agarwal<br>
> ><br>
> > On Tue, Mar 6, 2018 at 12:50 AM, Christian Mauderer<br>
</div></div>> <<a href="mailto:list@c-mauderer.de">list@c-mauderer.de</a> <mailto:<a href="mailto:list@c-mauderer.de">list@c-mauderer.de</a>>><br>
<div><div class="h5">> > wrote:<br>
> >><br>
> >> Am 05.03.2018 um 14:51 schrieb Udit agarwal:<br>
> >> > Hi,<br>
> >> > I tried implementing getentropy on BBB, below is the patch.<br>
> Please have<br>
> >> > a look.<br>
> >> > I followed these(1<br>
> >> > <<a href="https://e2e.ti.com/support/arm/sitara_arm/f/791/t/355064" rel="noreferrer" target="_blank">https://e2e.ti.com/support/<wbr>arm/sitara_arm/f/791/t/355064</a><br>
> <<a href="https://e2e.ti.com/support/arm/sitara_arm/f/791/t/355064" rel="noreferrer" target="_blank">https://e2e.ti.com/support/<wbr>arm/sitara_arm/f/791/t/355064</a>><wbr>> & 2<br>
> >> ><br>
> >> ><br>
> <<a href="http://ftp.netbsd.org/pub/NetBSD/NetBSD-current/src/sys/arch/arm/omap/am335x_trng.c" rel="noreferrer" target="_blank">http://ftp.netbsd.org/pub/<wbr>NetBSD/NetBSD-current/src/sys/<wbr>arch/arm/omap/am335x_trng.c</a><br>
> <<a href="http://ftp.netbsd.org/pub/NetBSD/NetBSD-current/src/sys/arch/arm/omap/am335x_trng.c" rel="noreferrer" target="_blank">http://ftp.netbsd.org/pub/<wbr>NetBSD/NetBSD-current/src/sys/<wbr>arch/arm/omap/am335x_trng.c</a>>>)<br>
> >> > links for code reference. and this<br>
> >> ><br>
> >> ><br>
> <<a href="https://docs.google.com/spreadsheets/d/1VpghMlLtrWQIrcvCsRg3ueRZkX0eGSQkFnzjfsqrd8A/view#gid=0" rel="noreferrer" target="_blank">https://docs.google.com/<wbr>spreadsheets/d/<wbr>1VpghMlLtrWQIrcvCsRg3ueRZkX0eG<wbr>SQkFnzjfsqrd8A/view#gid=0</a><br>
> <<a href="https://docs.google.com/spreadsheets/d/1VpghMlLtrWQIrcvCsRg3ueRZkX0eGSQkFnzjfsqrd8A/view#gid=0" rel="noreferrer" target="_blank">https://docs.google.com/<wbr>spreadsheets/d/<wbr>1VpghMlLtrWQIrcvCsRg3ueRZkX0eG<wbr>SQkFnzjfsqrd8A/view#gid=0</a>>><br>
> >> > for register reference. Moreover, what further configuration in<br>
> RTEMS is<br>
> >> > needed to execute and test this code along with getentropy() sample<br>
> >> > testcode?<br>
> >><br>
> >> Hello Udit,<br>
> >><br>
> >> regarding the patch: Please use 'git format-patch' for generating<br>
> your<br>
> >> patches and send them unchanged to the mailing list (for example with<br>
> >> 'git send-email'. If you follow this workflow, it's easier to<br>
> apply and<br>
> >> test the patches. The preferred workflow for RTEMS is described here:<br>
> >> <a href="https://devel.rtems.org/wiki/Developer/Git/Users#CreatingaPatch" rel="noreferrer" target="_blank">https://devel.rtems.org/wiki/<wbr>Developer/Git/Users#<wbr>CreatingaPatch</a><br>
> <<a href="https://devel.rtems.org/wiki/Developer/Git/Users#CreatingaPatch" rel="noreferrer" target="_blank">https://devel.rtems.org/wiki/<wbr>Developer/Git/Users#<wbr>CreatingaPatch</a>><br>
> >><br>
> >> Regarding documentation: It seems that there is really only few<br>
> >> documentation regarding the crypto part available. The FreeBSD<br>
> driver is<br>
> >> most likely your best source.<br>
> >><br>
> >> I added further comments down in your code.<br>
> >><br>
> >> Best regards<br>
> >><br>
> >> Christian<br>
> >><br>
> >> ><br>
> >> ><br>
> >> > + /*<br>
> >> > + * Copyright (c) 2018 embedded brains GmbH. All rights reserved.<br>
> >> > + *<br>
> >> > + * embedded brains GmbH<br>
> >> > + * Dornierstr. 4<br>
> >> > + * 82178 Puchheim<br>
> >> > + * Germany<br>
> >> > + * <<a href="mailto:rtems@embedded-brains.de">rtems@embedded-brains.de</a><br>
</div></div>> <mailto:<a href="mailto:rtems@embedded-brains.de">rtems@embedded-brains.<wbr>de</a>> <mailto:<a href="mailto:rtems@embedded-brains.de">rtems@embedded-brains.<wbr>de</a><br>
<span class="">> <mailto:<a href="mailto:rtems@embedded-brains.de">rtems@embedded-brains.<wbr>de</a>>>><br>
> >> > + *<br>
> >><br>
> >> Also it's nice if you add an embedded brains header, I don't<br>
> think that<br>
> >> I've seen you when I have been at work today. So please add your own<br>
> >> name to the copyright line instead. With that you can even point<br>
> to that<br>
> >> file and tell everyone that it's from you.<br>
> >><br>
> >> An address isn't really necessary except if you want it for some<br>
> reason<br>
> >> (like PR in case of a company).<br>
> >><br>
> >> > + * The license and distribution terms for this file may be<br>
> >> > + * found in the file LICENSE in this distribution or at<br>
> >> > + * <a href="http://www.rtems.org/license/LICENSE" rel="noreferrer" target="_blank">http://www.rtems.org/license/<wbr>LICENSE</a><br>
</span>> <<a href="http://www.rtems.org/license/LICENSE" rel="noreferrer" target="_blank">http://www.rtems.org/license/<wbr>LICENSE</a>>.<br>
<div><div class="h5">> >> > + */<br>
> >> > +<br>
> >> > + #include <libcpu/am335x.h><br>
> >> > + #include <unistd.h><br>
> >> > + #include <string.h><br>
> >> > + #include <rtems/sysinit.h><br>
> >> > +<br>
> >> > + #define RNG_BASE 0x48310000<br>
> >> > + #define CM_PER_RNG_CLKCTRL 0x44e00090<br>
> >> > + #define RNG_STATUS_RDY (1u << 0) // output ready for reading<br>
> >> > + #define RNG_STATUS_ERR (1u << 1) // FRO shutdown alarm<br>
> >> > + #define RNG_STATUS_CLK (1u << 31) // module functional clock<br>
> >> > active (no irq)<br>
> >><br>
> >> Please add address definitions and the bit masks to the<br>
> >> bsps/arm/include/libcpu/<wbr>am335x.h instead of defining them here<br>
> locally.<br>
> >> Note that for the CM_PER_RNG_CLKCTRL, there is already a<br>
> >> AM335X_CM_PER_ADDR that should be used as a base.<br>
> >><br>
> >> > +<br>
> >> > + typedef volatile struct rng *rng_ref_t;<br>
> >> > +<br>
> >> > + struct rng {<br>
> >> > + /*00*/ uint64_t output; //r-<br>
> >> > + /*08*/ uint32_t status; //r-<br>
> >> > + /*0c*/ uint32_t irq_en; //rw<br>
> >> > + /*10*/ uint32_t status_clr; //-c<br>
> >> > + /*14*/ uint32_t control; //rw<br>
> >> > + /*18*/ uint32_t config; //rw<br>
> >> > + };<br>
> >><br>
> >> Same is true here: It's a structure describing the registers. So it<br>
> >> should be in the header instead.<br>
> >><br>
> >> It seems that you copied that from the forum post. Please always<br>
> check<br>
> >> the license when you copy code. We have to avoid that we get any code<br>
> >> into RTEMS that isn't compatible with RTEMS license.<br>
> >><br>
> >> In this case the code should be obvious enough that - if you just<br>
> >> rewrite it in RTEMS style, there shouldn't be a problem. But just<br>
> >> remember that licensing is always a difficult topic.<br>
> >><br>
> >> > +<br>
> >> > + static void TRNG_Enable(rng_ref_t rng){<br>
> >> > + rng->config = 0<br>
> >> > + | 34 << 16 // max refill 34 * 256 cycles<br>
> >> > + | 33 << 0; // min refill 33 * 64 cycles<br>
> >> > + rng->control = 0<br>
> >> > + | 33 << 16 // startup 33 * 256 cycles<br>
> >> > + | 1 << 10; // enable module<br>
> >> > + }<br>
> >><br>
> >> Please use C-Style comments /* */ in all pure C code (most of RTEMS).<br>
> >><br>
> >> > +<br>
> >> > + static void beagle_trng_clock_enable(void)<br>
> >> > + {<br>
> >> > + // enable module clock<br>
> >> > + *(volatile uint8_t *) CM_PER_RNG_CLKCTRL = 2;<br>
> >> > + while( *(volatile uint32_t *) CM_PER_RNG_CLKCTRL &<br>
> 0x30000 ) {}<br>
> >> > + }<br>
> >> > +<br>
> >> > + static uint64_t TRNG_GetRandData(){<br>
> >> > + uint64_t output = rng->output;<br>
> >><br>
> >> 'rng' isn't defined here.<br>
> >><br>
> >> > + return output;<br>
> >> > + }<br>
> >> > +<br>
> >> > + int getentropy(void *ptr, size_t n)<br>
> >> > + {<br>
> >> > + rng_ref_t const rng = (rng_ref_t) RNG_BASE;<br>
> >> > + TRNG_Enable(rng);<br>
> >> > + while (n > 0) {<br>
> >> > + uint64_t random;<br>
> >> > + size_t copy;<br>
> >> > +<br>
> >> > + while( ( rng->status & RNG_STATUS_RDY ) == 0 ); //wait<br>
> >><br>
> >> You should check specifically for the TRNG_STATUS_READY flag here<br>
> like<br>
> >> done in FreeBSD. I'm not sure what the other flags are but it's quite<br>
> >> possible that there is also some error flag or similar. In that<br>
> case it<br>
> >> would not be a good idea to use the random number.<br>
> >><br>
> >> > +<br>
> >> > + random = TRNG_GetRandData(rng);<br>
> >><br>
> >> Note that you don't have to copy the function names from the atsam<br>
> >> exactly. In the atsam BSP, the library has provided these names.<br>
> >> Otherwise it's quite unusual to start functions with upper case<br>
> in RTEMS.<br>
> >><br>
> >> > +<br>
> >> > + /*<br>
> >> > + * Read TRNG status one more time to avoid race<br>
> condition.<br>
> >> > + * Otherwise we can read (and clear) an old ready<br>
> status but<br>
> >> > get<br>
> >> > + * a new value. The ready status for this value<br>
> wouldn't be<br>
> >> > + * reset.<br>
> >> > + */<br>
> >><br>
> >> That comment seems like it doesn't fit here any longer. You<br>
> acknowledge<br>
> >> the status instead in your next line. Just remove the comment.<br>
> >><br>
> >> > + rng->status_clr = RNG_STATUS_RDY;<br>
> >> > +<br>
> >> > + copy = sizeof(random);<br>
> >> > + if (n < copy ) {<br>
> >> > + copy = n;<br>
> >> > + }<br>
> >> > + memcpy(ptr, &random, copy);<br>
> >> > + n -= copy;<br>
> >> > + ptr += copy;<br>
> >> > + }<br>
> >> > +<br>
> >> > + return 0;<br>
> >> > + }<br>
> >> > +<br>
> >> > + RTEMS_SYSINIT_ITEM(<br>
> >> > + beagle_trng_clock_enable,<br>
> >> > + RTEMS_SYSINIT_DEVICE_DRIVERS,<br>
> >> > + RTEMS_SYSINIT_ORDER_LAST<br>
> >> > + );<br>
> >> ><br>
> >> > Regards,<br>
> >> > Udit agarwal<br>
> >> ><br>
> >> ><br>
> >> > ______________________________<wbr>_________________<br>
> >> > devel mailing list<br>
</div></div>> >> > <a href="mailto:devel@rtems.org">devel@rtems.org</a> <mailto:<a href="mailto:devel@rtems.org">devel@rtems.org</a>><br>
> >> > <a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/<wbr>mailman/listinfo/devel</a><br>
<span class="">> <<a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/<wbr>mailman/listinfo/devel</a>><br>
> >> ><br>
> ><br>
> ><br>
> ><br>
> > ______________________________<wbr>_________________<br>
> > devel mailing list<br>
</span>> > <a href="mailto:devel@rtems.org">devel@rtems.org</a> <mailto:<a href="mailto:devel@rtems.org">devel@rtems.org</a>><br>
> > <a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/<wbr>mailman/listinfo/devel</a><br>
<div class="HOEnZb"><div class="h5">> <<a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/<wbr>mailman/listinfo/devel</a>><br>
><br>
><br>
><br>
><br>
> ______________________________<wbr>_________________<br>
> devel mailing list<br>
> <a href="mailto:devel@rtems.org">devel@rtems.org</a><br>
> <a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/<wbr>mailman/listinfo/devel</a><br>
><br>
</div></div></blockquote></div><br></div>