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

27 Dec 2019

      On Fri, 27 Dec 2019 at 18:12, Heinrich Schuchardt xypron.glpk@gmx.de
wrote:
...
On 12/27/19 12:19 PM, Sughosh Ganu wrote:
...
On Fri, 27 Dec 2019 at 13:21, Heinrich Schuchardt <xypron.glpk@gmx.de
mailto:xypron.glpk@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@linaro.org
<mailto:sughosh.ganu@linaro.org>>
 > Reviewed-by: Patrice Chotard <patrice.chotard@st.com
<mailto:patrice.chotard@st.com>>
 > Acked-by: Patrick Delaunay <patrick.delaunay@st.com
<mailto:patrick.delaunay@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/...
,
...
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

Could you, please, provide a link to the document for stm32mp157.
I was required to create a login to wiki.st.com to get access to the
document. Don't think there is a link for the document available.
-sughosh