[PATCH v5 3/8] stm32mp1: rng: Add a driver for random number generator(rng) device

Heinrich Schuchardt xypron.glpk at gmx.de
Fri Dec 27 13:42:11 CET 2019


On 12/27/19 12:19 PM, Sughosh Ganu wrote:
>
> On Fri, 27 Dec 2019 at 13:21, Heinrich Schuchardt <xypron.glpk at gmx.de
> <mailto:xypron.glpk at gmx.de>> wrote:
>
>     On 12/26/19 6:25 PM, Sughosh Ganu wrote:
>      > Add a driver for the rng device found on stm32mp1 platforms. The
>      > driver provides a routine for reading the random number seed from the
>      > hardware device.
>      >
>      > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org
>     <mailto:sughosh.ganu at linaro.org>>
>      > Reviewed-by: Patrice Chotard <patrice.chotard at st.com
>     <mailto:patrice.chotard at st.com>>
>      > Acked-by: Patrick Delaunay <patrick.delaunay at st.com
>     <mailto:patrick.delaunay at st.com>>
>      > ---
>      > Changes since V4:
>      > * Return number of bytes read on a successful read, and a -ve
>     value on
>      >    error.
>      >
>      >   drivers/rng/Kconfig        |   7 ++
>      >   drivers/rng/Makefile       |   1 +
>      >   drivers/rng/stm32mp1_rng.c | 165
>     +++++++++++++++++++++++++++++++++++++++++++++
>      >   3 files changed, 173 insertions(+)
>      >   create mode 100644 drivers/rng/stm32mp1_rng.c
>      >
>      > diff --git a/drivers/rng/Kconfig b/drivers/rng/Kconfig
>      > index dd44cc0..c9c4751 100644
>      > --- a/drivers/rng/Kconfig
>      > +++ b/drivers/rng/Kconfig
>      > @@ -5,3 +5,10 @@ config DM_RNG
>      >         Enable driver model for random number generator(rng) devices.
>      >         This interface is used to initialise the rng device and to
>      >         read the random seed from the device.
>      > +
>      > +config RNG_STM32MP1
>      > +     bool "Enable random number generator for STM32MP1"
>      > +     depends on ARCH_STM32MP && DM_RNG
>      > +     default n
>      > +     help
>      > +       Enable STM32MP1 rng driver.
>      > diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile
>      > index 311705b..699beb3 100644
>      > --- a/drivers/rng/Makefile
>      > +++ b/drivers/rng/Makefile
>      > @@ -4,3 +4,4 @@
>      >   #
>      >
>      >   obj-$(CONFIG_DM_RNG) += rng-uclass.o
>      > +obj-$(CONFIG_RNG_STM32MP1) += stm32mp1_rng.o
>      > diff --git a/drivers/rng/stm32mp1_rng.c b/drivers/rng/stm32mp1_rng.c
>      > new file mode 100644
>      > index 0000000..56ad848
>      > --- /dev/null
>      > +++ b/drivers/rng/stm32mp1_rng.c
>      > @@ -0,0 +1,165 @@
>      > +// SPDX-License-Identifier: GPL-2.0-or-later
>      > +/*
>      > + * Copyright (c) 2019, Linaro Limited
>      > + */
>      > +
>      > +#include <common.h>
>      > +#include <clk.h>
>      > +#include <dm.h>
>      > +#include <reset.h>
>      > +#include <rng.h>
>      > +
>      > +#include <asm/io.h>
>      > +#include <linux/iopoll.h>
>      > +#include <linux/kernel.h>
>      > +
>      > +#define RNG_CR 0x00
>      > +#define RNG_CR_RNGEN BIT(2)
>      > +#define RNG_CR_CED BIT(5)
>      > +
>      > +#define RNG_SR 0x04
>      > +#define RNG_SR_SEIS BIT(6)
>      > +#define RNG_SR_CEIS BIT(5)
>      > +#define RNG_SR_SECS BIT(2)
>      > +#define RNG_SR_DRDY BIT(0)
>      > +
>      > +#define RNG_DR 0x08
>      > +
>      > +struct stm32_rng_platdata {
>      > +     fdt_addr_t base;
>      > +     struct clk clk;
>      > +     struct reset_ctl rst;
>      > +};
>      > +
>      > +static int stm32_rng_read(struct udevice *dev, void *data,
>     size_t len)
>      > +{
>      > +     int retval = 0, i, nbytes;
>      > +     u32 sr, count, reg;
>      > +     size_t increment;
>      > +     struct stm32_rng_platdata *pdata = dev_get_platdata(dev);
>      > +
>      > +     /*
>      > +      * Only INT_MAX number of bytes can be returned. If more
>      > +      * bytes need to be read, the caller must do it in a loop.
>      > +      */
>      > +     if (len > INT_MAX)
>      > +             len = INT_MAX;
>      > +
>      > +     nbytes = len;
>      > +     while (len > 0) {
>      > +             retval = readl_poll_timeout(pdata->base + RNG_SR, sr,
>      > +                                         sr & RNG_SR_DRDY, 10000);
>      > +             if (retval)
>      > +                     return retval;
>      > +
>      > +             if (sr & (RNG_SR_SEIS | RNG_SR_SECS)) {
>      > +                     /* As per SoC TRM */
>      > +                     clrbits_le32(pdata->base + RNG_SR,
>     RNG_SR_SEIS);
>      > +                     for (i = 0; i < 12; i++)
>      > +                             readl(pdata->base + RNG_DR);
>      > +                     if (readl(pdata->base + RNG_SR) &
>     RNG_SR_SEIS) {
>      > +                             printf("RNG Noise");
>      > +                             return -EIO;
>
>     The SEIS bit indicates a seed error. According to the RM0090 Reference
>     manual for the STM32F429/439 you should clear the SEIS bit and set the
>     RNGEN bit to restart the RNG.
>
>     https://www.st.com/content/ccc/resource/technical/document/reference_manual/3d/6d/5a/66/b4/99/40/d4/DM00031020.pdf/files/DM00031020.pdf/jcr:content/translations/en.DM00031020.pdf,
>     page 768.
>
>     So why do you return an error code here? What do you expect the caller
>     to do?
>
>
> I am referring to the stm32mp157 trm, Noise source error detection, pg 1939.

Could you, please, provide a link to the document for stm32mp157.

compatible = "st,stm32-rng" is provided in U-Boot by:

* stm32f429.dtsi included by
   stm32429i-eval.dts, arch/arm/dts/stm32f429-disco.dts, and
   stm32f469-disco.dts

* as well as by stm32mp157c.dtsi included by
   stm32mp157a-avenger96.dts, stm32mp157a-dk1.dts, and
   stm32mp157c-ed1.dts.

So we have to consider both SoCs here.

Best regards

Heinrich

>
>
>     Should we check the CEIS flag which indicates a clock error?
>
>
> The stm32mp1 clk driver in tf-a sets the rng peripheral source to
> lsi_ck, which is 32KHz. This is configured after tests done by ST
> engineers to provide better entropy. However, with this clock source,
> the CEIS bit is set, since Frng is less than (Fhclk/32). This is not an
> issue though, since the trm clearly states that the clock error has no
> impact on the generated random numbers, and that the application can
> still read the RNG_DR register.
>
>
>      > +                     }
>      > +                     /* start again */
>      > +                     continue;
>      > +             }
>      > +
>
>     It took me some time to understand what this loop does. Please, add a
>     comment indicating that we copy up to 16 random bytes from the RNG.
>
>
> Ok. Will do.
>
> -sughosh



More information about the U-Boot mailing list