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

2019-12-27 Thread Sughosh Ganu
On Fri, 27 Dec 2019 at 18:25, Heinrich Schuchardt 
wrote:

> On 12/27/19 1:42 PM, Heinrich Schuchardt wrote:
> > On 12/27/19 12:19 PM, Sughosh Ganu wrote:
> >>
> >> On Fri, 27 Dec 2019 at 13:21, Heinrich Schuchardt  >> > 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  >> >
> >>  > Reviewed-by: Patrice Chotard  >> >
> >>  > Acked-by: Patrick Delaunay  >> >
> >>  > ---
> >>  > 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 000..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 
> >>  > +#include 
> >>  > +#include 
> >>  > +#include 
> >>  > +#include 
> >>  > +
> >>  > +#include 
> >>  > +#include 
> >>  > +#include 
> >>  > +
> >>  > +#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,
> >> 1);
> >>  > + 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");
> >>  > + ret

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

2019-12-27 Thread Sughosh Ganu
On Fri, 27 Dec 2019 at 18:12, Heinrich Schuchardt 
wrote:

> On 12/27/19 12:19 PM, Sughosh Ganu wrote:
> >
> > On Fri, 27 Dec 2019 at 13:21, Heinrich Schuchardt  > > 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  > >
> >  > Reviewed-by: Patrice Chotard  > >
> >  > Acked-by: Patrick Delaunay  > >
> >  > ---
> >  > 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 000..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 
> >  > +#include 
> >  > +#include 
> >  > +#include 
> >  > +#include 
> >  > +
> >  > +#include 
> >  > +#include 
> >  > +#include 
> >  > +
> >  > +#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,
> 1);
> >  > + 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 bi

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

2019-12-27 Thread Heinrich Schuchardt

On 12/27/19 1:42 PM, Heinrich Schuchardt wrote:

On 12/27/19 12:19 PM, Sughosh Ganu wrote:


On Fri, 27 Dec 2019 at 13:21, Heinrich Schuchardt mailto:xypron.g...@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 mailto:sughosh.g...@linaro.org>>
 > Reviewed-by: Patrice Chotard mailto:patrice.chot...@st.com>>
 > Acked-by: Patrick Delaunay mailto:patrick.delau...@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 000..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 
 > +#include 
 > +#include 
 > +#include 
 > +#include 
 > +
 > +#include 
 > +#include 
 > +#include 
 > +
 > +#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, 
1);

 > +             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.


RM0436 Reference manual STM32MP157 advanced Arm®-based 32-bit MPU

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

2019-12-27 Thread Heinrich Schuchardt

On 12/27/19 12:19 PM, Sughosh Ganu wrote:


On Fri, 27 Dec 2019 at 13:21, Heinrich Schuchardt mailto:xypron.g...@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 mailto:sughosh.g...@linaro.org>>
 > Reviewed-by: Patrice Chotard mailto:patrice.chot...@st.com>>
 > Acked-by: Patrick Delaunay mailto:patrick.delau...@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 000..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 
 > +#include 
 > +#include 
 > +#include 
 > +#include 
 > +
 > +#include 
 > +#include 
 > +#include 
 > +
 > +#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, 1);
 > +             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-disc

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

2019-12-27 Thread Sughosh Ganu
On Fri, 27 Dec 2019 at 13:21, Heinrich Schuchardt 
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 
> > Reviewed-by: Patrice Chotard 
> > Acked-by: Patrick Delaunay 
> > ---
> > 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 000..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 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#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, 1);
> > + 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.


>
> 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

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

2019-12-26 Thread Heinrich Schuchardt

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 
Reviewed-by: Patrice Chotard 
Acked-by: Patrick Delaunay 
---
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 000..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 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#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, 1);
+   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?

Should we check the CEIS flag which indicates a clock error?


+   }
+   /* 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.

Best regards

Heinrich


+   count = 4;
+   while (len && count) {
+   reg = readl(pdata->base + RNG_DR);
+   memcpy(data, ®, min(len, sizeof(u32)));
+   increment = min(len, sizeof(u32));
+   data += increment;
+   len -= increment;
+   count--;
+   }
+   }
+
+   return nbytes;
+}
+
+static int stm32_rng_init(struct stm32_rng_platdata *pdata)
+{
+   int err;
+
+   err = clk_enable(&pdata->clk);
+   if (err)
+   return err;
+
+   /* Disable CED */
+   writel(RNG_CR_RNGEN | RNG_CR_CED, pdata->base + RNG_CR);
+
+   /* clear error indicators */
+   writel(0, pdata->base + RNG_SR);
+
+   return 0;
+}
+
+static int stm32_rng_cleanup(struct stm32_rng_platdata *pdata)
+{
+
+   writel(0, pdata->base + RNG_CR);
+

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

2019-12-26 Thread Sughosh Ganu
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 
Reviewed-by: Patrice Chotard 
Acked-by: Patrick Delaunay 
---
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 000..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 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#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, 1);
+   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;
+   }
+   /* start again */
+   continue;
+   }
+
+   count = 4;
+   while (len && count) {
+   reg = readl(pdata->base + RNG_DR);
+   memcpy(data, ®, min(len, sizeof(u32)));
+   increment = min(len, sizeof(u32));
+   data += increment;
+   len -= increment;
+   count--;
+   }
+   }
+
+   return nbytes;
+}
+
+static int stm32_rng_init(struct stm32_rng_platdata *pdata)
+{
+   int err;
+
+   err = clk_enable(&pdata->clk);
+   if (err)
+   return err;
+
+   /* Disable CED */
+   writel(RNG_CR_RNGEN | RNG_CR_CED, pdata->base + RNG_CR);
+
+   /* clear error indicators */
+   writel(0, pdata->base + RNG_SR);
+
+   return 0;
+}
+
+static int stm32_rng_cleanup(struct stm32_rng_platdata *pdata)
+{
+
+   writel(0, pdata->base + RNG_CR);
+
+   return clk_disable(&pdata->clk);
+}
+
+static int stm32_rng_probe(struct udevice *dev)
+{
+   struct stm32_rng_platdata *pdata = dev_get_platdata(dev);
+
+   reset_assert(&pdata->rst);
+   udelay(20);
+   reset_deassert(&pdata->rst);
+
+   return stm32_rng_init(pdata);
+}
+
+static int stm32_rng_remove(struct udevice *dev)
+{
+   struct stm32_rng_platdata *pdata = dev_get_platdata(dev);
+
+   return stm32_rng_cleanup(pdata);
+}
+
+static int stm32_rng_ofdata_to_platdata(struct udevice *dev)
+{
+   struct stm32_rng_platdata *pdata = dev_get_platdata(dev);
+   int err;
+
+   pdata->base = dev_read_addr(dev);
+   if (!pdata->base)
+   return -ENOMEM;
+
+