getentropy() implementation on BBB

Udit agarwal dev.madaari at gmail.com
Wed Mar 7 10:26:02 UTC 2018


Hi,
I have updated the code, please have a look.
>From 74b8f4f5b9dd929b71ed5fb9dd0bc721a6f27a28 Mon Sep 17 00:00:00 2001
From: Udit agarwal <dev.madaari at gmail.com>
Date: Wed, 7 Mar 2018 15:52:13 +0530
Subject: [PATCH] Added getentropy support to beagle BSP

---
 bsps/arm/include/libcpu/am335x.h                   | 34 ++++++++-
 c/src/lib/libbsp/arm/beagle/Makefile.am            |  4 +
 .../libbsp/arm/beagle/getentropy/bbb_getentropy.c  | 88
++++++++++++++++++++++
 3 files changed, 125 insertions(+), 1 deletion(-)
 create mode 100644 c/src/lib/libbsp/arm/beagle/getentropy/bbb_getentropy.c

diff --git a/bsps/arm/include/libcpu/am335x.h
b/bsps/arm/include/libcpu/am335x.h
index 367e97c..92af6dc 100644
--- a/bsps/arm/include/libcpu/am335x.h
+++ b/bsps/arm/include/libcpu/am335x.h
@@ -14,6 +14,9 @@
  * Modified by Ben Gras <beng at shrike-systems.com> to add lots
  * of beagleboard/beaglebone definitions, delete lpc32xx specific
  * ones, and merge with some other header files.
+ *
+ * Modified by Udit agarwal <dev.madaari at gmail.com> to add random
+ * number generating module definitions along with register structure.
  */

 #if !defined(_AM335X_H_)
@@ -701,4 +704,33 @@
 #define AM335X_CM_PER_OCPWP_L3_CLKSTCTRL_CLKACTIVITY_OCPWP_L4_GCLK
(0x00000020u)
 #define AM335X_I2C_INT_STOP_CONDITION AM335X_I2C_IRQSTATUS_BF

-#endif
+/* TRNG Register */
+
+/* RNG base address */
+#define RNG_BASE 0x48310000
+/* RNG clock control */
+#define CM_PER_RNG_CLKCTRL (AM335X_CM_PER_ADDR | (9 << 4))
+/* rng module clock status bits */
+#define AM335X_CLK_RNG_BIT_MASK (0x30000)
+/* Offset from RNG base for output ready flag */
+#define RNG_STATUS_RDY (1u <<  0)
+/* Offset from RNG base for FRO related error */
+#define RNG_STATUS_ERR (1u <<  1)
+/* Offset from RNG base for clock status */
+#define RNG_STATUS_CLK (1u << 31)
+/* enable module */
+#define AM335X_RNG_ENABLE (1 << 10)
+
+/* TRNG register structure */
+struct bbb_trng_register
+{
+    uint64_t output; /*00*/
+    uint32_t status; /*08*/
+    uint32_t irq_en; /*0c*/
+    uint32_t status_clr; /*10*/
+    uint32_t control; /*14*/
+    uint32_t config; /*18*/
+};
+typedef struct bbb_trng_register bbb_trng_register;
+
+#endif
\ No newline at end of file
diff --git a/c/src/lib/libbsp/arm/beagle/Makefile.am
b/c/src/lib/libbsp/arm/beagle/Makefile.am
index 8251660..bf92081 100644
--- a/c/src/lib/libbsp/arm/beagle/Makefile.am
+++ b/c/src/lib/libbsp/arm/beagle/Makefile.am
@@ -88,9 +88,13 @@ libbsp_a_SOURCES += gpio/bbb-gpio.c
 #pwm
 libbsp_a_SOURCES += pwm/pwm.c

+#getentropy
+libbsp_a_SOURCES += getentropy/bbb_getentropy.c
+
 #RTC
 libbsp_a_SOURCES += rtc.c
 libbsp_a_SOURCES += ../../shared/tod.c
+
 # Clock
 libbsp_a_SOURCES += clock.c
 libbsp_a_SOURCES += ../../shared/clockdrv_shell.h
diff --git a/c/src/lib/libbsp/arm/beagle/getentropy/bbb_getentropy.c
b/c/src/lib/libbsp/arm/beagle/getentropy/bbb_getentropy.c
new file mode 100644
index 0000000..5b7c5d0
--- /dev/null
+++ b/c/src/lib/libbsp/arm/beagle/getentropy/bbb_getentropy.c
@@ -0,0 +1,88 @@
+/**
+* @file
+*
+* @ingroup arm_beagle
+*
+* @brief Getentropy implementation on BeagleBone Black BSP
+*/
+
+/*
+* Copyright (c) 2018 Udit kumar agarwal <dev.madaari at gmail.com>
+*
+* The license and distribution terms for this file may be
+* found in the file LICENSE in this distribution or at
+* http://www.rtems.org/license/LICENSE.
+*/
+
+#include <libcpu/am335x.h>
+#include <unistd.h>
+#include <string.h>
+#include <rtems/sysinit.h>
+#include <rtems/types.h>
+
+/* max refill 34 * 256 cycles */
+#define AM335X_RNG_MAX_REFILL (34 << 16)
+/* min refill 33 * 64 cycles */
+#define AM335X_RNG_MIN_REFILL (33 << 0)
+/* startup 33 * 256 cycles */
+#define AM335X_RNG_STARTUP_CYCLES (33 << 16)
+
+/* maximun and minimum refill cycle sets the number of samples to be taken
+   from FRO to generate random number */
+static void am335x_rng_enable(bbb_trng_register *rng)
+{
+    rng->control = rng->config = 0;
+    rng->config |= AM335X_RNG_MIN_REFILL | AM335X_RNG_MAX_REFILL ;
+    rng->control |= AM335X_RNG_STARTUP_CYCLES | AM335X_RNG_ENABLE ;
+}
+
+static void am335x_rng_clock_enable(void)
+{
+    *(volatile uint8_t *) CM_PER_RNG_CLKCTRL = 2;
+    while( *(volatile uint32_t *) CM_PER_RNG_CLKCTRL &
AM335X_CLK_RNG_BIT_MASK ) {}
+}
+
+static uint64_t trng_getranddata(bbb_trng_register *rng)
+{
+    uint64_t output = rng->output;
+    return output;
+}
+
+int getentropy(void *ptr, size_t n)
+{
+    volatile const bbb_trng_register  *rng = (bbb_trng_register*) RNG_BASE;
+    am335x_rng_enable(rng);
+    while (n > 0)
+    {
+        uint64_t random;
+        size_t copy;
+
+        /* wait untill RNG becomes ready with next set of random data */
+        while( ( rng->status & RNG_STATUS_RDY ) == 0 );
+
+        random = trng_getranddata(rng);
+
+        /* Checking for error by masking all bits other then error bit in
status register */
+        if( ((rng->status & RNG_STATUS_ERR)>>1) == 0)
+        {
+            /* clear the status flag after reading to generate new random
value*/
+            rng->status_clr = RNG_STATUS_RDY;
+            copy = sizeof(random);
+            if ( n < copy )
+            {
+                copy = n;
+            }
+            memcpy(ptr, &random, copy);
+            n -= copy;
+            ptr = (char*)ptr + copy;
+        }
+    }
+
+    return 0;
+}
+
+RTEMS_SYSINIT_ITEM(
+    am335x_rng_clock_enable,
+    RTEMS_SYSINIT_DEVICE_DRIVERS,
+    RTEMS_SYSINIT_ORDER_LAST
+);
\ No newline at end of file
-- 
1.9.1

Regards,
Udit agarwal

On Tue, Mar 6, 2018 at 11:05 PM, Gedare Bloom <gedare at rtems.org> wrote:

> On Tue, Mar 6, 2018 at 9:43 AM, Udit agarwal <dev.madaari at gmail.com>
> wrote:
> > Hi,
> > Here's the updated code(I have also attached the patch PFA):
> >
> > From 96e6e1bfd8cffeef5d309eb0a07fe2bfd086ef0a Mon Sep 17 00:00:00 2001
> > From: Udit agarwal <dev.madaari at gmail.com>
> > Date: Tue, 6 Mar 2018 20:07:44 +0530
> > Subject: [PATCH] Added getentropy support to BBB BSP
> >
> > ---
> >  bsps/arm/beagle/headers.am                         |  1 +
> >  bsps/arm/beagle/include/bsp/bbb_getentropy.h       | 57 +++++++++++++++
> >  bsps/arm/include/libcpu/am335x.h                   | 16 +++++
> >  c/src/lib/libbsp/arm/beagle/Makefile.am            |  4 ++
> >  .../libbsp/arm/beagle/getentropy/bbb_getentropy.c  | 80
> > ++++++++++++++++++++++
> >  5 files changed, 158 insertions(+)
> >  create mode 100644 bsps/arm/beagle/include/bsp/bbb_getentropy.h
> >  create mode 100644 c/src/lib/libbsp/arm/beagle/
> getentropy/bbb_getentropy.c
> >
> > diff --git a/bsps/arm/beagle/headers.am b/bsps/arm/beagle/headers.am
> > index 6692d0b..ac4ff7c 100644
> > --- a/bsps/arm/beagle/headers.am
> > +++ b/bsps/arm/beagle/headers.am
> > @@ -12,3 +12,4 @@ include_bsp_HEADERS +=
> > ../../../../../../bsps/arm/beagle/include/bsp/bbb-pwm.h
> >  include_bsp_HEADERS +=
> > ../../../../../../bsps/arm/beagle/include/bsp/beagleboneblack.h
> >  include_bsp_HEADERS += ../../../../../../bsps/arm/
> beagle/include/bsp/i2c.h
> >  include_bsp_HEADERS += ../../../../../../bsps/arm/
> beagle/include/bsp/irq.h
> > +include_bsp_HEADERS +=
> > ../../../../../../bsps/arm/beagle/include/bsp/bbb_getentropy.h
> > diff --git a/bsps/arm/beagle/include/bsp/bbb_getentropy.h
> > b/bsps/arm/beagle/include/bsp/bbb_getentropy.h
> > new file mode 100644
> > index 0000000..5e46f89
> > --- /dev/null
> > +++ b/bsps/arm/beagle/include/bsp/bbb_getentropy.h
> > @@ -0,0 +1,57 @@
> > +/**
> > + * @file
> > + *
> > + * @ingroup arm_beagle
> > + *
> > + * @brief TRNG Definations
> > + */
> > +
> > +/*
> > + * Copyright (c) 2018 Udit kumar agarwal <dev.madaari at gmail.com>
> > + *
> > + * The license and distribution terms for this file may be
> > + * found in the file LICENSE in this distribution or at
> > + * http://www.rtems.org/license/LICENSE.
> > + */
> > +
> > +
> > +#ifndef _TRNG_
> > +#define _TRNG_
> > +
> > +/*---------------------------------------------------------
> ---------------------
> Please look at similar files in RTEMS for how to add comments.
> We do not use this style /* --------------------
> And we don't put this kind of comment about the file sections.
>
> > + *         Headers
> > +
> > *-----------------------------------------------------------
> -----------------*/
> > +
> > +#include <libcpu/am335x.h>
> > +
> > +
> > /*----------------------------------------------------------
> --------------------
> > + *         Register structure
> > +
> > *-----------------------------------------------------------
> -----------------*/
> > +
> > +struct rng {
> > +    uint64_t output; /*00*/
> > +    uint32_t status; /*08*/
> > +    uint32_t irq_en; /*0c*/
> > +    uint32_t status_clr; /*10*/
> > +    uint32_t control; /*14*/
> > +    uint32_t config; /*18*/
> > +};
> for an exported header, the struct namespace needs to be user
> friendly. See similar files at this layer for how to name things.
> Probably, this can be defined in the am335x.h header?
>
> > +
> > +typedef volatile struct rng *rng_ref_t;
> I'm not such a big fan of using the typedef to hide things like
> volatile or pointer notation. I don't see a need for this usually.
>
> > +
> > +/*---------------------------------------------------------
> -------------------*/
> > +/*         Exported functions
> > */
> > +/*---------------------------------------------------------
> -------------------*/
> > +
> > +/* configure and enable RNG module  */
> > +static void am335x_rng_enable(rng_ref_t);
> > +/* enable module clock for random number generator */
> > +static void am335x_rng_clock_enable(void);
> > +/* outputs random data */
> > +static uint64_t trng_getranddata(rng_ref_t);
>
> static keyword implies these are not exported.
>
> > +/* Copy random data of a specified size to the pointer supplied */
> > +int getentropy(void *ptr, size_t);
> I guess getentropy() is already exported by a different header file
> (unistd.h) in libc? This doesn't belong here.
>
> I suspect this header file is not actually necessary.
>
> > +
> > +
> > +
> Don't use multiple blank lines in a row. Only one empty line is ever
> needed by RTEMS style.
>
> > +#endif /* #ifndef _TRNG_ */
> > \ No newline at end of file
> > diff --git a/bsps/arm/include/libcpu/am335x.h
> > b/bsps/arm/include/libcpu/am335x.h
> > index 367e97c..8b58f1a 100644
> > --- a/bsps/arm/include/libcpu/am335x.h
> > +++ b/bsps/arm/include/libcpu/am335x.h
> > @@ -14,6 +14,9 @@
> >   * Modified by Ben Gras <beng at shrike-systems.com> to add lots
> >   * of beagleboard/beaglebone definitions, delete lpc32xx specific
> >   * ones, and merge with some other header files.
> > + *
> > + * Modified by Udit agarwal <dev.madaari at gmail.com> to add random
> > + * number generating module definations
> typo: definitions
> add a period (full stop).
>
> >   */
> >
> >  #if !defined(_AM335X_H_)
> > @@ -701,4 +704,17 @@
> >  #define AM335X_CM_PER_OCPWP_L3_CLKSTCTRL_CLKACTIVITY_OCPWP_L4_GCLK
> > (0x00000020u)
> >  #define AM335X_I2C_INT_STOP_CONDITION AM335X_I2C_IRQSTATUS_BF
> >
> > +/* TRNG Register */
> > +
> > +/* RNG base address */
> > +#define RNG_BASE    0x48310000
> > +/* RNG clock control */
> > +#define CM_PER_RNG_CLKCTRL    AM335X_CM_PER_ADDR | 9<<4
> I'd put parentheses around this to avoid any problems with macro
> expansions and order of operations.
>
> > +/* Offset from RNG base for output ready flag */
> > +#define RNG_STATUS_RDY    (1u <<  0)
> > +/* Offset from RNG base for FRO related error*/
> > +#define RNG_STATUS_ERR    (1u <<  1)
> > +/* Offset from RNG base for clock status */
> > +#define RNG_STATUS_CLK    (1u << 31)
> > +
> >  #endif
> > diff --git a/c/src/lib/libbsp/arm/beagle/Makefile.am
> > b/c/src/lib/libbsp/arm/beagle/Makefile.am
> > index 8251660..bf92081 100644
> > --- a/c/src/lib/libbsp/arm/beagle/Makefile.am
> > +++ b/c/src/lib/libbsp/arm/beagle/Makefile.am
> > @@ -88,9 +88,13 @@ libbsp_a_SOURCES += gpio/bbb-gpio.c
> >  #pwm
> >  libbsp_a_SOURCES += pwm/pwm.c
> >
> > +#getentropy
> > +libbsp_a_SOURCES += getentropy/bbb_getentropy.c
> > +
> >  #RTC
> >  libbsp_a_SOURCES += rtc.c
> >  libbsp_a_SOURCES += ../../shared/tod.c
> > +
> >  # Clock
> >  libbsp_a_SOURCES += clock.c
> >  libbsp_a_SOURCES += ../../shared/clockdrv_shell.h
> > diff --git a/c/src/lib/libbsp/arm/beagle/getentropy/bbb_getentropy.c
> > b/c/src/lib/libbsp/arm/beagle/getentropy/bbb_getentropy.c
> > new file mode 100644
> > index 0000000..cab2102
> > --- /dev/null
> > +++ b/c/src/lib/libbsp/arm/beagle/getentropy/bbb_getentropy.c
> > @@ -0,0 +1,80 @@
> > +/**
> > +* @file
> > +*
> > +* @ingroup arm_beagle
> > +*
> > +* @brief Getentropy implementation on BeagleBone Black BSP
> > +*/
> > +
> > +/*
> > +* Copyright (c) 2018 Udit kumar agarwal <dev.madaari at gmail.com>
> > +*
> > +* The license and distribution terms for this file may be
> > +* found in the file LICENSE in this distribution or at
> > +* http://www.rtems.org/license/LICENSE.
> > +*/
> > +
> > +#include <unistd.h>
> > +#include <stdint.h>
> > +#include <string.h>
> > +#include <rtems/sysinit.h>
> > +
> > +
> > +/* maximun and minimun refill cycle sets the number of samples to be
> taken
> typo: minimum
> > +   from FRO to generate random number */
> > +static void am335x_rng_enable(rng_ref_t rng){
> put { on a new line.
>
> > +    rng->config = 0
> > +    | 34 << 16  /* max refill 34 * 256 cycles */
> indent broken lines, but
> > +    | 33 <<  0; /* min refill 33 * 64 cycles */
> You can fit this on one line, remove the comments and put them above
> the line, e.g.
> rng->config = 34<<16 | 33;
>
> I would prefer not to have these hard-coded values though it is
> fragile. what means 34, 16, and 33?
> #define AM335X_RNG_MAX_REFILL (34 << 16)
> #define AM335X_RNG_MIN_REFILL (33)
> #define AM335X_RNG_CONFIG_REFILL (AM335X_RNG_MAX_REFILL |
> AM335X_RNG_MIN_REFILL)
>
> rng.config = rng.control = 0;
> rng.config |= AM335X_RNG_CONFIG_REFILL_TIME;
>
> > +    rng->control = 0
> > +    | 33 << 16  /* startup 33 * 256 cycles */
> > +    |  1 << 10; /* enable module */
>
> rng.control |= AM335X_RNG_CONTROL_STARTUP;
> rng.control |= AM335X_RNG_CONTROL_ENABLE;
>
> for example is more readable. look around for other examples.
>
> > +}
> > +
> > +static void am335x_rng_clock_enable(void)
> > +{
> > +    *(volatile uint8_t *) CM_PER_RNG_CLKCTRL = 2;
> > +    while( *(volatile uint32_t *) CM_PER_RNG_CLKCTRL & 0x30000 ) {}
> > +}
> > +
> > +static uint64_t trng_getranddata(rng_ref_t rng){
> > +    uint64_t output = rng->output;
> > +    return output;
> > +}
> > +
> > +int getentropy(void *ptr, size_t n)
> > +{
> > +    rng_ref_t const rng = (rng_ref_t) RNG_BASE;
> > +    am335x_rng_enable(rng);
> > +    while (n > 0) {
> > +    uint64_t random;
> indent blocks of code.
>
> > +    size_t copy;
> > +
> > +    /* wait untill RNG becomes ready with next set of random data */
> > +    while( ( rng->status & RNG_STATUS_RDY ) == 0 );
> > +
> > +    random = trng_getranddata(rng);
> > +
> > +    /* Checking for error by masking all bits other then error bit in
> > status register */
> > +    if( ((rng->status & RNG_STATUS_ERR)>>1) == 0){
> > +
> > +        /* clear the status flag after reading to generate new random
> > value*/
> > +        rng->status_clr = RNG_STATUS_RDY;
> > +        copy = sizeof(random);
> > +        if ( n < copy ) {
> > +        copy = n;
> ditto.
>
> > +        }
> > +        memcpy(ptr, &random, copy);
> > +        n -= copy;
> > +        ptr += copy;
> pointer arithmetic on void pointer is not legal. You can portably use
> ptr = (char*)ptr + copy;
>
> > +    }
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +RTEMS_SYSINIT_ITEM(
> > +    am335x_rng_clock_enable,
> > +    RTEMS_SYSINIT_DEVICE_DRIVERS,
> > +    RTEMS_SYSINIT_ORDER_LAST
> > +);
> > \ No newline at end of file
> > --
> > 1.9.1
> >
> > Regards,
> > Udit Agarwal
> >
> > On Tue, Mar 6, 2018 at 12:50 AM, Christian Mauderer <list at c-mauderer.de>
> > wrote:
> >>
> >> Am 05.03.2018 um 14:51 schrieb Udit agarwal:
> >> > Hi,
> >> > I tried implementing getentropy on BBB, below is the patch. Please
> have
> >> > a look.
> >> > I followed these(1
> >> > <https://e2e.ti.com/support/arm/sitara_arm/f/791/t/355064> & 2
> >> >
> >> > <http://ftp.netbsd.org/pub/NetBSD/NetBSD-current/src/sys/
> arch/arm/omap/am335x_trng.c>)
> >> > links for code reference. and this
> >> >
> >> > <https://docs.google.com/spreadsheets/d/
> 1VpghMlLtrWQIrcvCsRg3ueRZkX0eGSQkFnzjfsqrd8A/view#gid=0>
> >> > for register reference. Moreover, what further configuration in RTEMS
> is
> >> > needed to execute and test this code along with getentropy() sample
> >> > testcode?
> >>
> >> Hello Udit,
> >>
> >> regarding the patch: Please use 'git format-patch' for generating your
> >> patches and send them unchanged to the mailing list (for example with
> >> 'git send-email'. If you follow this workflow, it's easier to apply and
> >> test the patches. The preferred workflow for RTEMS is described here:
> >> https://devel.rtems.org/wiki/Developer/Git/Users#CreatingaPatch
> >>
> >> Regarding documentation: It seems that there is really only few
> >> documentation regarding the crypto part available. The FreeBSD driver is
> >> most likely your best source.
> >>
> >> I added further comments down in your code.
> >>
> >> Best regards
> >>
> >> Christian
> >>
> >> >
> >> >
> >> > + /*
> >> > +  * Copyright (c) 2018 embedded brains GmbH.  All rights reserved.
> >> > +  *
> >> > +  *  embedded brains GmbH
> >> > +  *  Dornierstr. 4
> >> > +  *  82178 Puchheim
> >> > +  *  Germany
> >> > +  *  <rtems at embedded-brains.de <mailto:rtems at embedded-brains.de>>
> >> > +  *
> >>
> >> Also it's nice if you add an embedded brains header, I don't think that
> >> I've seen you when I have been at work today. So please add your own
> >> name to the copyright line instead. With that you can even point to that
> >> file and tell everyone that it's from you.
> >>
> >> An address isn't really necessary except if you want it for some reason
> >> (like PR in case of a company).
> >>
> >> > +  * The license and distribution terms for this file may be
> >> > +  * found in the file LICENSE in this distribution or at
> >> > +  * http://www.rtems.org/license/LICENSE.
> >> > +  */
> >> > +
> >> > + #include <libcpu/am335x.h>
> >> > + #include <unistd.h>
> >> > + #include <string.h>
> >> > + #include <rtems/sysinit.h>
> >> > +
> >> > + #define RNG_BASE    0x48310000
> >> > + #define CM_PER_RNG_CLKCTRL    0x44e00090
> >> > + #define RNG_STATUS_RDY    (1u <<  0)  // output ready for reading
> >> > + #define RNG_STATUS_ERR    (1u <<  1)  // FRO shutdown alarm
> >> > + #define RNG_STATUS_CLK    (1u << 31)  // module functional clock
> >> > active (no irq)
> >>
> >> Please add address definitions and the bit masks to the
> >> bsps/arm/include/libcpu/am335x.h instead of defining them here locally.
> >> Note that for the CM_PER_RNG_CLKCTRL, there is already a
> >> AM335X_CM_PER_ADDR that should be used as a base.
> >>
> >> > +
> >> > + typedef volatile struct rng *rng_ref_t;
> >> > +
> >> > + struct rng {
> >> > + /*00*/    uint64_t output;    //r-
> >> > + /*08*/    uint32_t status;    //r-
> >> > + /*0c*/    uint32_t irq_en;    //rw
> >> > + /*10*/    uint32_t status_clr;    //-c
> >> > + /*14*/    uint32_t control;    //rw
> >> > + /*18*/    uint32_t config;    //rw
> >> > + };
> >>
> >> Same is true here: It's a structure describing the registers. So it
> >> should be in the header instead.
> >>
> >> It seems that you copied that from the forum post. Please always check
> >> the license when you copy code. We have to avoid that we get any code
> >> into RTEMS that isn't compatible with RTEMS license.
> >>
> >> In this case the code should be obvious enough that - if you just
> >> rewrite it in RTEMS style, there shouldn't be a problem. But just
> >> remember that licensing is always a difficult topic.
> >>
> >> > +
> >> > + static void TRNG_Enable(rng_ref_t rng){
> >> > +     rng->config = 0
> >> > +     | 34 << 16  // max refill 34 * 256 cycles
> >> > +     | 33 <<  0;  // min refill 33 * 64 cycles
> >> > +     rng->control = 0
> >> > +     | 33 << 16  // startup 33 * 256 cycles
> >> > +     |  1 << 10;  // enable module
> >> > + }
> >>
> >> Please use C-Style comments /* */ in all pure C code (most of RTEMS).
> >>
> >> > +
> >> > + static void beagle_trng_clock_enable(void)
> >> > + {
> >> > +     // enable module clock
> >> > +     *(volatile uint8_t *) CM_PER_RNG_CLKCTRL = 2;
> >> > +     while( *(volatile uint32_t *) CM_PER_RNG_CLKCTRL & 0x30000 ) {}
> >> > + }
> >> > +
> >> > + static uint64_t TRNG_GetRandData(){
> >> > +     uint64_t output = rng->output;
> >>
> >> 'rng' isn't defined here.
> >>
> >> > +     return output;
> >> > + }
> >> > +
> >> > + int getentropy(void *ptr, size_t n)
> >> > + {
> >> > +     rng_ref_t const rng = (rng_ref_t) RNG_BASE;
> >> > +     TRNG_Enable(rng);
> >> > +     while (n > 0) {
> >> > +         uint64_t random;
> >> > +         size_t copy;
> >> > +
> >> > +         while( ( rng->status & RNG_STATUS_RDY ) == 0 ); //wait
> >>
> >> You should check specifically for the TRNG_STATUS_READY flag here like
> >> done in FreeBSD. I'm not sure what the other flags are but it's quite
> >> possible that there is also some error flag or similar. In that case it
> >> would not be a good idea to use the random number.
> >>
> >> > +
> >> > +         random = TRNG_GetRandData(rng);
> >>
> >> Note that you don't have to copy the function names from the atsam
> >> exactly. In the atsam BSP, the library has provided these names.
> >> Otherwise it's quite unusual to start functions with upper case in
> RTEMS.
> >>
> >> > +
> >> > +         /*
> >> > +          * Read TRNG status one more time to avoid race condition.
> >> > +          * Otherwise we can read (and clear) an old ready status but
> >> > get
> >> > +          * a new value. The ready status for this value wouldn't be
> >> > +          * reset.
> >> > +          */
> >>
> >> That comment seems like it doesn't fit here any longer. You acknowledge
> >> the status instead in your next line. Just remove the comment.
> >>
> >> > +         rng->status_clr = RNG_STATUS_RDY;
> >> > +
> >> > +         copy = sizeof(random);
> >> > +         if (n < copy ) {
> >> > +             copy = n;
> >> > +         }
> >> > +         memcpy(ptr, &random, copy);
> >> > +         n -= copy;
> >> > +         ptr += copy;
> >> > +     }
> >> > +
> >> > +     return 0;
> >> > + }
> >> > +
> >> > + RTEMS_SYSINIT_ITEM(
> >> > +   beagle_trng_clock_enable,
> >> > +   RTEMS_SYSINIT_DEVICE_DRIVERS,
> >> > +   RTEMS_SYSINIT_ORDER_LAST
> >> > + );
> >> >
> >> > Regards,
> >> > Udit agarwal
> >> >
> >> >
> >> > _______________________________________________
> >> > devel mailing list
> >> > devel at rtems.org
> >> > http://lists.rtems.org/mailman/listinfo/devel
> >> >
> >
> >
> >
> > _______________________________________________
> > devel mailing list
> > devel at rtems.org
> > http://lists.rtems.org/mailman/listinfo/devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20180307/eb7926f2/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Added-getentropy-support-to-beagle-BSP.patch
Type: text/x-patch
Size: 5292 bytes
Desc: not available
URL: <http://lists.rtems.org/pipermail/devel/attachments/20180307/eb7926f2/attachment-0001.bin>


More information about the devel mailing list