On Fri, 27 Dec 2019 at 18:25, Heinrich Schuchardt <xypron.g...@gmx.de> 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 <xypron.g...@gmx.de > >> <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 <sughosh.g...@linaro.org > >> <mailto:sughosh.g...@linaro.org>> > >> > Reviewed-by: Patrice Chotard <patrice.chot...@st.com > >> <mailto:patrice.chot...@st.com>> > >> > Acked-by: Patrick Delaunay <patrick.delau...@st.com > >> <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 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. > > RM0436 Reference manual STM32MP157 advanced Arm®-based 32-bit MPUs > > > https://www.st.com/content/ccc/resource/technical/document/reference_manual/group0/51/ba/9e/5e/78/5b/4b/dd/DM00327659/files/DM00327659.pdf/jcr:content/translations/en.DM00327659.pdf > > chapter 37.3.7 - "Error management" requires the following error handling: > > 1. Clear the SEIS bit by writing it to “0”. > 2. Read out 12 words from the RNG_DR register, and discard each of them > in order to clean the pipeline. > 3. Confirm that SEIS is still cleared. Random number generation is back > to normal > > So the same error handling can be used for both SoCs. > Which is what the current logic is doing. In case, the SEIS bit is still set after reading the 12 words, the driver returns an error, else it continues. -sughosh