This patchset introduces a driver for the STM32 hardware random number generator.
Daniel Thompson (3): dt-bindings: Document the STM32 HW RNG bindings hwrng: stm32 - add support for STM32 HW RNG ARM: dts: stm32f429: Adopt STM32 RNG driver
.../devicetree/bindings/hwrng/stm32-rng.txt | 21 +++ arch/arm/boot/dts/stm32f429.dtsi | 7 + drivers/char/hw_random/Kconfig | 12 ++ drivers/char/hw_random/Makefile | 1 + drivers/char/hw_random/stm32-rng.c | 192 +++++++++++++++++++++ 5 files changed, 233 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwrng/stm32-rng.txt create mode 100644 drivers/char/hw_random/stm32-rng.c
-- 2.4.3
This adds documenttaion of device tree binds for the STM32 hardware random number generator.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org --- .../devicetree/bindings/hwrng/stm32-rng.txt | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwrng/stm32-rng.txt
diff --git a/Documentation/devicetree/bindings/hwrng/stm32-rng.txt b/Documentation/devicetree/bindings/hwrng/stm32-rng.txt new file mode 100644 index 000000000000..47f04176f93b --- /dev/null +++ b/Documentation/devicetree/bindings/hwrng/stm32-rng.txt @@ -0,0 +1,21 @@ +STMicroelectronics STM32 HW RNG +=============================== + +The STM32 hardware random number generator is a simple fixed purpose IP and +is fully separated from other crypto functions. + +Required properties: + +- compatible : Should be "st,stm32-rng" +- reg : Should be register base and length as documented in the datasheet +- interrupts : The designated IRQ line for the RNG +- clocks : The clock needed to enable the RNG + +Example: + + rng: rng@50060800 { + compatible = "st,stm32-rng"; + reg = <0x50060800 0x400>; + interrupts = <80>; + clocks = <&rcc 0 38>; + };
Hi Daniel,
On 10/03/2015 10:35 PM, Daniel Thompson wrote:
This adds documenttaion of device tree binds for the STM32 hardware
s/documenttaion/documentation/
random number generator.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org
.../devicetree/bindings/hwrng/stm32-rng.txt | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwrng/stm32-rng.txt
Except the typo, the patch looks good to me. When fixed, you can add my: Acked-by: Maxime Coquelin mcoquelin.stm32@gmail.com
Thanks! Maxime
On Sat, Oct 3, 2015 at 3:35 PM, Daniel Thompson daniel.thompson@linaro.org wrote:
This adds documenttaion of device tree binds for the STM32 hardware random number generator.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org
.../devicetree/bindings/hwrng/stm32-rng.txt | 21 +++++++++++++++++++++
Please use bindings/rng/... as I'm consolidating bindings there.
Otherwise,
Acked-by: Rob Herring robh@kernel.org
1 file changed, 21 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwrng/stm32-rng.txt
diff --git a/Documentation/devicetree/bindings/hwrng/stm32-rng.txt b/Documentation/devicetree/bindings/hwrng/stm32-rng.txt new file mode 100644 index 000000000000..47f04176f93b --- /dev/null +++ b/Documentation/devicetree/bindings/hwrng/stm32-rng.txt @@ -0,0 +1,21 @@ +STMicroelectronics STM32 HW RNG +===============================
+The STM32 hardware random number generator is a simple fixed purpose IP and +is fully separated from other crypto functions.
+Required properties:
+- compatible : Should be "st,stm32-rng" +- reg : Should be register base and length as documented in the datasheet +- interrupts : The designated IRQ line for the RNG +- clocks : The clock needed to enable the RNG
+Example:
rng: rng@50060800 {
compatible = "st,stm32-rng";
reg = <0x50060800 0x400>;
interrupts = <80>;
clocks = <&rcc 0 38>;
};
-- 2.4.3
Add support for STMicroelectronics STM32 random number generator.
The config value defaults to N, reflecting the fact that STM32 is a very low resource microcontroller platform and unlikely to be targeted by any "grown up" defconfigs.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org --- drivers/char/hw_random/Kconfig | 12 +++ drivers/char/hw_random/Makefile | 1 + drivers/char/hw_random/stm32-rng.c | 192 +++++++++++++++++++++++++++++++++++++ 3 files changed, 205 insertions(+) create mode 100644 drivers/char/hw_random/stm32-rng.c
diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig index f48cf11c655e..7930cc9b719c 100644 --- a/drivers/char/hw_random/Kconfig +++ b/drivers/char/hw_random/Kconfig @@ -359,6 +359,18 @@ config HW_RANDOM_XGENE
If unsure, say Y.
+config HW_RANDOM_STM32 + tristate "STMicroelectronics STM32 random number generator" + depends on HW_RANDOM && (ARCH_STM32 || COMPILE_TEST) + help + This driver provides kernel-side support for the Random Number + Generator hardware found on STM32 microcontrollers. + + To compile this driver as a module, choose M here: the + module will be called stm32-rng. + + If unsure, say N. + endif # HW_RANDOM
config UML_RANDOM diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile index 055bb01510ad..8b49c0f7abb1 100644 --- a/drivers/char/hw_random/Makefile +++ b/drivers/char/hw_random/Makefile @@ -31,3 +31,4 @@ obj-$(CONFIG_HW_RANDOM_BCM2835) += bcm2835-rng.o obj-$(CONFIG_HW_RANDOM_IPROC_RNG200) += iproc-rng200.o obj-$(CONFIG_HW_RANDOM_MSM) += msm-rng.o obj-$(CONFIG_HW_RANDOM_XGENE) += xgene-rng.o +obj-$(CONFIG_HW_RANDOM_STM32) += stm32-rng.o diff --git a/drivers/char/hw_random/stm32-rng.c b/drivers/char/hw_random/stm32-rng.c new file mode 100644 index 000000000000..37dfa5fca105 --- /dev/null +++ b/drivers/char/hw_random/stm32-rng.c @@ -0,0 +1,192 @@ +/* + * Copyright (c) 2015, Daniel Thompson + * + * This file is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This file is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/hw_random.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of_address.h> +#include <linux/of_platform.h> +#include <linux/slab.h> + +#define RNG_CR 0x00 +#define RNG_CR_RNGEN BIT(2) + +#define RNG_SR 0x04 +#define RNG_SR_SEIS BIT(6) +#define RNG_SR_CEIS BIT(5) +#define RNG_SR_DRDY BIT(0) + +#define RNG_DR 0x08 + +/* + * It takes 40 cycles @ 48MHz to generate each random number (e.g. <1us). + * At the time of writing STM32 parts max out at ~200MHz meaning a timeout + * of 500 leaves us a very comfortable margin for error. The loop to which + * the timeout applies takes at least 4 instructions per cycle so the + * timeout is enough to take us up to multi-GHz parts! + */ +#define RNG_TIMEOUT 500 + +struct stm32_rng_private { + struct hwrng rng; + void __iomem *base; + struct clk *clk; +}; + +static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool wait) +{ + struct stm32_rng_private *priv = + container_of(rng, struct stm32_rng_private, rng); + u32 cr, sr; + int retval = 0; + + /* enable random number generation */ + clk_enable(priv->clk); + cr = readl(priv->base + RNG_CR); + writel(cr | RNG_CR_RNGEN, priv->base + RNG_CR); + + while (max > sizeof(u32)) { + sr = readl(priv->base + RNG_SR); + if (!sr && wait) { + unsigned int timeout = RNG_TIMEOUT; + + do { + cpu_relax(); + sr = readl(priv->base + RNG_SR); + } while (!sr && --timeout); + } + + /* Has h/ware error dection been triggered? */ + if (WARN_ON(sr & (RNG_SR_SEIS | RNG_SR_CEIS))) + break; + + /* No data ready... */ + if (!sr) + break; + + *(u32 *)data = readl(priv->base + RNG_DR); + + retval += sizeof(u32); + data += sizeof(u32); + max -= sizeof(u32); + } + + /* disable the generator */ + writel(cr, priv->base + RNG_CR); + clk_disable(priv->clk); + + return retval || !wait ? retval : -EIO; +} + +static int stm32_rng_init(struct hwrng *rng) +{ + struct stm32_rng_private *priv = + container_of(rng, struct stm32_rng_private, rng); + int err; + u32 sr; + + err = clk_prepare(priv->clk); + if (err) + return err; + + /* clear error indicators */ + sr = readl(priv->base + RNG_SR); + sr &= ~(RNG_SR_SEIS | RNG_SR_CEIS); + writel(sr, priv->base + RNG_SR); + + return 0; +} + +static void stm32_rng_cleanup(struct hwrng *rng) +{ + struct stm32_rng_private *priv = + container_of(rng, struct stm32_rng_private, rng); + + clk_unprepare(priv->clk); +} + +static int stm32_rng_remove(struct platform_device *ofdev) +{ + struct device *dev = &ofdev->dev; + struct stm32_rng_private *priv = dev_get_drvdata(dev); + + hwrng_unregister(&priv->rng); + + return 0; +} + +static int stm32_rng_probe(struct platform_device *ofdev) +{ + struct device *dev = &ofdev->dev; + struct device_node *np = ofdev->dev.of_node; + struct stm32_rng_private *priv; + struct resource res; + int err; + + priv = devm_kzalloc(dev, sizeof(struct stm32_rng_private), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + err = of_address_to_resource(np, 0, &res); + if (err) + return err; + + priv->base = devm_ioremap_resource(dev, &res); + if (IS_ERR(priv->base)) + return PTR_ERR(priv->base); + + priv->clk = devm_clk_get(&ofdev->dev, NULL); + if (IS_ERR(priv->clk)) + return PTR_ERR(priv->clk); + + dev_set_drvdata(dev, priv); + + priv->rng.name = dev_driver_string(dev), + priv->rng.init = stm32_rng_init, + priv->rng.cleanup = stm32_rng_cleanup, + priv->rng.read = stm32_rng_read, + priv->rng.priv = (unsigned long) dev; + + err = hwrng_register(&priv->rng); + if (err) + return err; + + return 0; +} + +static const struct of_device_id stm32_rng_match[] = { + { + .compatible = "st,stm32-rng", + }, + {}, +}; +MODULE_DEVICE_TABLE(of, stm32_rng_match); + +static struct platform_driver stm32_rng_driver = { + .driver = { + .name = "stm32_rng", + .of_match_table = stm32_rng_match, + }, + .probe = stm32_rng_probe, + .remove = stm32_rng_remove, +}; + +module_platform_driver(stm32_rng_driver); + +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("Daniel Thompson daniel.thompson@linaro.org"); +MODULE_DESCRIPTION("STMicroelectronics STM32 RNG device driver");
Hi Daniel,
[auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]
coccinelle warnings: (new ones prefixed by >>)
drivers/char/hw_random/stm32-rng.c:164:1-4: WARNING: end returns can be simpified
Please review and possibly fold the followup patch.
--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
drivers/char/hw_random/stm32-rng.c:164:1-4: WARNING: end returns can be simpified
Simplify a trivial if-return sequence. Possibly combine with a preceding function call.
Generated by: scripts/coccinelle/misc/simple_return.cocci
CC: Daniel Thompson daniel.thompson@linaro.org Signed-off-by: Fengguang Wu fengguang.wu@intel.com ---
stm32-rng.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
--- a/drivers/char/hw_random/stm32-rng.c +++ b/drivers/char/hw_random/stm32-rng.c @@ -161,11 +161,7 @@ static int stm32_rng_probe(struct platfo priv->rng.read = stm32_rng_read, priv->rng.priv = (unsigned long) dev;
- err = hwrng_register(&priv->rng); - if (err) - return err; - - return 0; + return hwrng_register(&priv->rng); }
static const struct of_device_id stm32_rng_match[] = {
Hi Daniel,
On 10/03/2015 10:35 PM, Daniel Thompson wrote:
Add support for STMicroelectronics STM32 random number generator.
The config value defaults to N, reflecting the fact that STM32 is a very low resource microcontroller platform and unlikely to be targeted by any "grown up" defconfigs.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org
drivers/char/hw_random/Kconfig | 12 +++ drivers/char/hw_random/Makefile | 1 + drivers/char/hw_random/stm32-rng.c | 192 +++++++++++++++++++++++++++++++++++++ 3 files changed, 205 insertions(+) create mode 100644 drivers/char/hw_random/stm32-rng.c
<snip>
diff --git a/drivers/char/hw_random/stm32-rng.c b/drivers/char/hw_random/stm32-rng.c new file mode 100644 index 000000000000..37dfa5fca105 --- /dev/null +++ b/drivers/char/hw_random/stm32-rng.c @@ -0,0 +1,192 @@
<snip>
+#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/hw_random.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of_address.h> +#include <linux/of_platform.h> +#include <linux/slab.h>
+#define RNG_CR 0x00 +#define RNG_CR_RNGEN BIT(2)
+#define RNG_SR 0x04 +#define RNG_SR_SEIS BIT(6) +#define RNG_SR_CEIS BIT(5) +#define RNG_SR_DRDY BIT(0)
+#define RNG_DR 0x08
+/*
- It takes 40 cycles @ 48MHz to generate each random number (e.g. <1us).
- At the time of writing STM32 parts max out at ~200MHz meaning a timeout
- of 500 leaves us a very comfortable margin for error. The loop to which
- the timeout applies takes at least 4 instructions per cycle so the
- timeout is enough to take us up to multi-GHz parts!
- */
+#define RNG_TIMEOUT 500
+struct stm32_rng_private {
- struct hwrng rng;
- void __iomem *base;
- struct clk *clk;
+};
+static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool wait) +{
- struct stm32_rng_private *priv =
container_of(rng, struct stm32_rng_private, rng);
- u32 cr, sr;
- int retval = 0;
- /* enable random number generation */
- clk_enable(priv->clk);
- cr = readl(priv->base + RNG_CR);
- writel(cr | RNG_CR_RNGEN, priv->base + RNG_CR);
- while (max > sizeof(u32)) {
sr = readl(priv->base + RNG_SR);
if (!sr && wait) {
unsigned int timeout = RNG_TIMEOUT;
do {
cpu_relax();
sr = readl(priv->base + RNG_SR);
} while (!sr && --timeout);
}
/* Has h/ware error dection been triggered? */
if (WARN_ON(sr & (RNG_SR_SEIS | RNG_SR_CEIS)))
Maybe you should instead use WARN_ONCE? Because from what I understand in the datasheet, CEIS bit indicates and error with clock configuration. If that happens, it is likely the same error will occur each time this function will be called.
break;
/* No data ready... */
if (!sr)
break;
Maybe you could perform this check before the error detection, as if !sr, the HW error condition will be always false.
*(u32 *)data = readl(priv->base + RNG_DR);
retval += sizeof(u32);
data += sizeof(u32);
max -= sizeof(u32);
- }
- /* disable the generator */
- writel(cr, priv->base + RNG_CR);
- clk_disable(priv->clk);
- return retval || !wait ? retval : -EIO;
+}
Couldn't we use "_relaxed" versions of readl/writel? I might save some not needed barriers.
+static int stm32_rng_probe(struct platform_device *ofdev) +{
- struct device *dev = &ofdev->dev;
- struct device_node *np = ofdev->dev.of_node;
- struct stm32_rng_private *priv;
- struct resource res;
- int err;
- priv = devm_kzalloc(dev, sizeof(struct stm32_rng_private), GFP_KERNEL);
- if (!priv)
return -ENOMEM;
- err = of_address_to_resource(np, 0, &res);
- if (err)
return err;
- priv->base = devm_ioremap_resource(dev, &res);
- if (IS_ERR(priv->base))
return PTR_ERR(priv->base);
- priv->clk = devm_clk_get(&ofdev->dev, NULL);
- if (IS_ERR(priv->clk))
return PTR_ERR(priv->clk);
- dev_set_drvdata(dev, priv);
- priv->rng.name = dev_driver_string(dev),
- priv->rng.init = stm32_rng_init,
- priv->rng.cleanup = stm32_rng_cleanup,
- priv->rng.read = stm32_rng_read,
- priv->rng.priv = (unsigned long) dev;
- err = hwrng_register(&priv->rng);
- if (err)
return err;
- return 0;
As detected with Coccinelle by Fengguang build system, you could simplify: return hwrng_register(&priv->rng);
Regards, Maxime
On 4 October 2015 at 09:52, Maxime Coquelin mcoquelin.stm32@gmail.com wrote:
Hi Daniel,
On 10/03/2015 10:35 PM, Daniel Thompson wrote:
Add support for STMicroelectronics STM32 random number generator.
The config value defaults to N, reflecting the fact that STM32 is a very low resource microcontroller platform and unlikely to be targeted by any "grown up" defconfigs.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org
drivers/char/hw_random/Kconfig | 12 +++ drivers/char/hw_random/Makefile | 1 + drivers/char/hw_random/stm32-rng.c | 192 +++++++++++++++++++++++++++++++++++++ 3 files changed, 205 insertions(+) create mode 100644 drivers/char/hw_random/stm32-rng.c
<snip> > > diff --git a/drivers/char/hw_random/stm32-rng.c > b/drivers/char/hw_random/stm32-rng.c > new file mode 100644 > index 000000000000..37dfa5fca105 > --- /dev/null > +++ b/drivers/char/hw_random/stm32-rng.c > @@ -0,0 +1,192 @@
<snip>
+#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/hw_random.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of_address.h> +#include <linux/of_platform.h> +#include <linux/slab.h>
+#define RNG_CR 0x00 +#define RNG_CR_RNGEN BIT(2)
+#define RNG_SR 0x04 +#define RNG_SR_SEIS BIT(6) +#define RNG_SR_CEIS BIT(5) +#define RNG_SR_DRDY BIT(0)
+#define RNG_DR 0x08
+/*
- It takes 40 cycles @ 48MHz to generate each random number (e.g. <1us).
- At the time of writing STM32 parts max out at ~200MHz meaning a
timeout
- of 500 leaves us a very comfortable margin for error. The loop to
which
- the timeout applies takes at least 4 instructions per cycle so the
- timeout is enough to take us up to multi-GHz parts!
- */
+#define RNG_TIMEOUT 500
+struct stm32_rng_private {
struct hwrng rng;
void __iomem *base;
struct clk *clk;
+};
+static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool wait) +{
struct stm32_rng_private *priv =
container_of(rng, struct stm32_rng_private, rng);
u32 cr, sr;
int retval = 0;
/* enable random number generation */
clk_enable(priv->clk);
cr = readl(priv->base + RNG_CR);
writel(cr | RNG_CR_RNGEN, priv->base + RNG_CR);
while (max > sizeof(u32)) {
sr = readl(priv->base + RNG_SR);
if (!sr && wait) {
unsigned int timeout = RNG_TIMEOUT;
do {
cpu_relax();
sr = readl(priv->base + RNG_SR);
} while (!sr && --timeout);
}
/* Has h/ware error dection been triggered? */
if (WARN_ON(sr & (RNG_SR_SEIS | RNG_SR_CEIS)))
Maybe you should instead use WARN_ONCE? Because from what I understand in the datasheet, CEIS bit indicates and error with clock configuration. If that happens, it is likely the same error will occur each time this function will be called.
Ok.
break;
/* No data ready... */
if (!sr)
break;
Maybe you could perform this check before the error detection, as if !sr, the HW error condition will be always false.
I plan to combine both checks since timing out is still a serious error even if the h/ware didn't detect it.
*(u32 *)data = readl(priv->base + RNG_DR);
retval += sizeof(u32);
data += sizeof(u32);
max -= sizeof(u32);
}
/* disable the generator */
writel(cr, priv->base + RNG_CR);
clk_disable(priv->clk);
return retval || !wait ? retval : -EIO;
+}
Couldn't we use "_relaxed" versions of readl/writel? I might save some not needed barriers.
Will fix. I had a "historic" reason to avoid _relaxed but this is no longer relevant.
+static int stm32_rng_probe(struct platform_device *ofdev) +{
struct device *dev = &ofdev->dev;
struct device_node *np = ofdev->dev.of_node;
struct stm32_rng_private *priv;
struct resource res;
int err;
priv = devm_kzalloc(dev, sizeof(struct stm32_rng_private),
GFP_KERNEL);
if (!priv)
return -ENOMEM;
err = of_address_to_resource(np, 0, &res);
if (err)
return err;
priv->base = devm_ioremap_resource(dev, &res);
if (IS_ERR(priv->base))
return PTR_ERR(priv->base);
priv->clk = devm_clk_get(&ofdev->dev, NULL);
if (IS_ERR(priv->clk))
return PTR_ERR(priv->clk);
dev_set_drvdata(dev, priv);
priv->rng.name = dev_driver_string(dev),
priv->rng.init = stm32_rng_init,
priv->rng.cleanup = stm32_rng_cleanup,
priv->rng.read = stm32_rng_read,
priv->rng.priv = (unsigned long) dev;
err = hwrng_register(&priv->rng);
if (err)
return err;
return 0;
As detected with Coccinelle by Fengguang build system, you could simplify: return hwrng_register(&priv->rng);
Will do.
Thanks
Daniel.
On Sat, Oct 3, 2015 at 10:35 PM, Daniel Thompson daniel.thompson@linaro.org wrote:
Add support for STMicroelectronics STM32 random number generator.
The config value defaults to N, reflecting the fact that STM32 is a very low resource microcontroller platform and unlikely to be targeted by any "grown up" defconfigs.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org
Incidentally both you and Lee Jones submitted similar RNG drivers from the same company (ST) recent weeks:
Lee's ST thing: http://marc.info/?l=linux-crypto-vger&m=144249760524971&w=2
Daniel's ST thing: http://marc.info/?l=linux-crypto-vger&m=144390463810134&w=2
And we also have from old ST: drivers/char/hw_random/nomadik-rng.c
1. Are these similar IPs that could be unified in a driver, just different offsets in memory?
2. Could you at least cross-review each others drivers because both tight loops to read bytes seem to contain similar semantics.
3. I took out the datasheet for Nomadik STn8820 and it seems that the hardware is very similar to what this driver is trying to drive. CR, SR and DR are in the same place, can you check if you also even have the PrimeCell magic in the words at offset 0xfe0 thru 0xffc?
In any case it seems likely that this driver should supercede and replace drivers/char/hw_random/nomadik-rng.c still using the PrimeCell bus, and if it doesn't have the PrimeCell IDs in hardware, this can be specified in the device tree using arm,primecell-periphid = <0x000805e1>; in the node if need be. The in-tree driver is dangerously simple and assume too much IMO.
Now the rest:
/* enable random number generation */
clk_enable(priv->clk);
cr = readl(priv->base + RNG_CR);
writel(cr | RNG_CR_RNGEN, priv->base + RNG_CR);
The Nomadik datasheet says this works the inverse way: setting bit 2 to 1 *disables* the RNG. Can you double-check this?
The Nomadik version also has a TSTCLK bit 3, that should always be set and loop-back checked in SR bit 1 to see that the block is properly clocked. (Read data will hang on an AHB stall if it's unclocked) Is this missing from STM32?
- sr = readl(priv->base + RNG_SR);
I strongly recommend that you use readl_relaxed() for this and all other reads on the hotpath, the Nomadik uses __raw_readl() but that is just for the same reasons that readl_relaxed() should be used: we just wanna read, not clean buffers etc.
- /* Has h/ware error dection been triggered? */
- if (WARN_ON(sr & (RNG_SR_SEIS | RNG_SR_CEIS)))
break;
- /* No data ready... */
- if (!sr)
break;
This assumes that only bits 6,5 and 0 are ever used in this hardware. Are you sure? On Nomadik DRDY bit 0 is the same, bit 1 is the clock test bit mentioned above, bit 2 is FAULT set if an invalid bit sequence is detected and should definately be checked if the HW supports it. Please mask explicitly for DRDY at least.
The bit layout gives at hand that this is indeed a derived IP block, I wonder what bits 3 & 4 may be used for in some or all revisions?
Then this construct:
+static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool wait) +{
(...)
- /* enable random number generation */
- clk_enable(priv->clk);
- cr = readl(priv->base + RNG_CR);
- writel(cr | RNG_CR_RNGEN, priv->base + RNG_CR);
(...)
- /* disable the generator */
- writel(cr, priv->base + RNG_CR);
- clk_disable(priv->clk);
This is in the hotpath where megabytes of data may be read out by consecutive reads. I think it's wise to move the clock enable/disable and also the write to cr to enable/disable the block to runtime PM hooks, so that you can e.g. set some hysteresis around the disablement with pm_runtime_set_autosuspend_delay(&dev->dev, 50); pm_runtime_use_autosuspend(&dev->dev);pm_runtime_put_autosuspend(). For a primecell check the usage in drivers/mmc/host/mmci.c for a complex usecase or e.g. drivers/hwtracing/coresight/coresight-tpiu.c for a simpler usecase.
For the performance hints I guess you can even test whether what I'm saying makes sense or not by reading some megabytes of random and timing it.
Yours, Linus Walleij
On 4 October 2015 at 11:32, Linus Walleij linus.walleij@linaro.org wrote:
On Sat, Oct 3, 2015 at 10:35 PM, Daniel Thompson daniel.thompson@linaro.org wrote:
Add support for STMicroelectronics STM32 random number generator.
The config value defaults to N, reflecting the fact that STM32 is a very low resource microcontroller platform and unlikely to be targeted by any "grown up" defconfigs.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org
Incidentally both you and Lee Jones submitted similar RNG drivers from the same company (ST) recent weeks:
Lee's ST thing: http://marc.info/?l=linux-crypto-vger&m=144249760524971&w=2
Daniel's ST thing: http://marc.info/?l=linux-crypto-vger&m=144390463810134&w=2
And we also have from old ST: drivers/char/hw_random/nomadik-rng.c
- Are these similar IPs that could be unified in a driver, just different
offsets in memory?
I don't think so. I can't see any commonality of register structure or data transfer approach (neither PIO nor DMA).
There have been exceptions but, in general, STM32 microcontrollers seem to have very little in common with ST SoC/ASIC devices except for the manufacturers id.
- Could you at least cross-review each others drivers because both
tight loops to read bytes seem to contain similar semantics.
I'll take a look although, as above, I suspect any similarities in structure will be based on the problem domain rather than shared heritage in the hardware design.
- I took out the datasheet for Nomadik STn8820 and it seems that
the hardware is very similar to what this driver is trying to drive. CR, SR and DR are in the same place, can you check if you also even have the PrimeCell magic in the words at offset 0xfe0 thru 0xffc?
The register window for the STM32 RNG is only 0x400 (and I'll fix the DT for v2 ;-) ) so the STM32 version isn't primecell-like.
In any case it seems likely that this driver should supercede and replace drivers/char/hw_random/nomadik-rng.c still using the PrimeCell bus, and if it doesn't have the PrimeCell IDs in hardware, this can be specified in the device tree using arm,primecell-periphid = <0x000805e1>; in the node if need be. The in-tree driver is dangerously simple and assume too much IMO.
Not sure about this, but I'll take a closer look. There is a certain family resemblance in the register set but there are significant differences in data transfer between the Nomadik and STM32 hardware.
Now the rest:
/* enable random number generation */
clk_enable(priv->clk);
cr = readl(priv->base + RNG_CR);
writel(cr | RNG_CR_RNGEN, priv->base + RNG_CR);
The Nomadik datasheet says this works the inverse way: setting bit 2 to 1 *disables* the RNG. Can you double-check this?
Nomadik driver doesn't seem to have to set anything to enable the RNG, so assuming the CR is resets to zero on nomadik I guess the bit meaning is inverted on the two platforms.
The Nomadik version also has a TSTCLK bit 3, that should always be set and loop-back checked in SR bit 1 to see that the block is properly clocked. (Read data will hang on an AHB stall if it's unclocked) Is this missing from STM32?
Bit 3 is reserved on STM32. I don't think the STM32 has logic to hang the bus if the cell doesn't have data ready (it's not mentioned in the datasheet but I've not verified what happens if we read when things are not ready).
- sr = readl(priv->base + RNG_SR);
I strongly recommend that you use readl_relaxed() for this and all other reads on the hotpath, the Nomadik uses __raw_readl() but that is just for the same reasons that readl_relaxed() should be used: we just wanna read, not clean buffers etc.
I'll fix this. Choosing writel/readl was deliberate (STM32 is a niche platform so I *really* wanted COMPILE_TEST to work) but this reasons is no longer relevant.
- /* Has h/ware error dection been triggered? */
- if (WARN_ON(sr & (RNG_SR_SEIS | RNG_SR_CEIS)))
break;
- /* No data ready... */
- if (!sr)
break;
This assumes that only bits 6,5 and 0 are ever used in this hardware. Are you sure? On Nomadik DRDY bit 0 is the same, bit 1 is the clock test bit mentioned above, bit 2 is FAULT set if an invalid bit sequence is detected and should definately be checked if the HW supports it. Please mask explicitly for DRDY at least.
There are also bits 1 and 2 but these are non-sticky versions of bits 5 and 6 so it should work... but I agree its not very conservative.
I plan to combine both branches into a simple "if (sr != RNG_SR_DRDY)" since any other value indicates an error condition that we should warn about anyway.
The bit layout gives at hand that this is indeed a derived IP block, I wonder what bits 3 & 4 may be used for in some or all revisions?
Then this construct:
+static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool wait) +{
(...)
- /* enable random number generation */
- clk_enable(priv->clk);
- cr = readl(priv->base + RNG_CR);
- writel(cr | RNG_CR_RNGEN, priv->base + RNG_CR);
(...)
- /* disable the generator */
- writel(cr, priv->base + RNG_CR);
- clk_disable(priv->clk);
This is in the hotpath where megabytes of data may be read out by consecutive reads. I think it's wise to move the clock enable/disable and also the write to cr to enable/disable the block to runtime PM hooks, so that you can e.g. set some hysteresis around the disablement with pm_runtime_set_autosuspend_delay(&dev->dev, 50); pm_runtime_use_autosuspend(&dev->dev);pm_runtime_put_autosuspend(). For a primecell check the usage in drivers/mmc/host/mmci.c for a complex usecase or e.g. drivers/hwtracing/coresight/coresight-tpiu.c for a simpler usecase.
For the performance hints I guess you can even test whether what I'm saying makes sense or not by reading some megabytes of random and timing it.
I'll have to benchmark this. clk_enable/disable have pretty simple implementations on STM32 so there might not be much in it.
Insisting on minimal power (which I think it important in microcontroller platforms) does hurt bandwidth more than you might expect because the framework does not pass big userspace reads to the underlying driver. This means the costs of clock control are spread over 32 bytes rather than over the whole of a big read.
Daniel.
On 5 October 2015 at 10:22, Daniel Thompson daniel.thompson@linaro.org wrote:
On 4 October 2015 at 11:32, Linus Walleij linus.walleij@linaro.org wrote:
On Sat, Oct 3, 2015 at 10:35 PM, Daniel Thompson 3. I took out the datasheet for Nomadik STn8820 and it seems that the hardware is very similar to what this driver is trying to drive. CR, SR and DR are in the same place, can you check if you also even have the PrimeCell magic in the words at offset 0xfe0 thru 0xffc?
The register window for the STM32 RNG is only 0x400 (and I'll fix the DT for v2 ;-) ) so the STM32 version isn't primecell-like.
In any case it seems likely that this driver should supercede and replace drivers/char/hw_random/nomadik-rng.c still using the PrimeCell bus, and if it doesn't have the PrimeCell IDs in hardware, this can be specified in the device tree using arm,primecell-periphid = <0x000805e1>; in the node if need be. The in-tree driver is dangerously simple and assume too much IMO.
Not sure about this, but I'll take a closer look. There is a certain family resemblance in the register set but there are significant differences in data transfer between the Nomadik and STM32 hardware.
Having looked at this I don't really think we would gain very much from combining these drivers. The cells have different (albeit slightly similar) register layouts, different ways to transfer data (16- versus 32-bit) and different ways to report test/report incorrect clocking. In the case of the RNG the differences therefore span the entire of its feature set.
I think combining them just results in two drivers that happen to share a file and I dislike having to add "fake" information to the devicetree to make it hang together.
Is that convincing for you?
Daniel.
On 5 October 2015 at 10:22, Daniel Thompson daniel.thompson@linaro.org wrote:
On 4 October 2015 at 11:32, Linus Walleij linus.walleij@linaro.org wrote:
On Sat, Oct 3, 2015 at 10:35 PM, Daniel Thompson daniel.thompson@linaro.org wrote: Then this construct:
+static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool wait) +{
(...)
- /* enable random number generation */
- clk_enable(priv->clk);
- cr = readl(priv->base + RNG_CR);
- writel(cr | RNG_CR_RNGEN, priv->base + RNG_CR);
(...)
- /* disable the generator */
- writel(cr, priv->base + RNG_CR);
- clk_disable(priv->clk);
This is in the hotpath where megabytes of data may be read out by consecutive reads. I think it's wise to move the clock enable/disable and also the write to cr to enable/disable the block to runtime PM hooks, so that you can e.g. set some hysteresis around the disablement with pm_runtime_set_autosuspend_delay(&dev->dev, 50); pm_runtime_use_autosuspend(&dev->dev);pm_runtime_put_autosuspend(). For a primecell check the usage in drivers/mmc/host/mmci.c for a complex usecase or e.g. drivers/hwtracing/coresight/coresight-tpiu.c for a simpler usecase.
For the performance hints I guess you can even test whether what I'm saying makes sense or not by reading some megabytes of random and timing it.
I'll have to benchmark this. clk_enable/disable have pretty simple implementations on STM32 so there might not be much in it.
Well... I was extremely wrong about that!
Switching the driver over to autosuspend yielded a very significant performance boost: ~1.1MiB/s to ~1.5MiB/s .
Very pleased with that!
Daniel.
New bindings and driver have been created for STM32 series parts. This patch integrates this changes.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org --- arch/arm/boot/dts/stm32f429.dtsi | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi index cb0613090243..90081fc22c6c 100644 --- a/arch/arm/boot/dts/stm32f429.dtsi +++ b/arch/arm/boot/dts/stm32f429.dtsi @@ -175,6 +175,13 @@ reg = <0x40023800 0x400>; clocks = <&clk_hse>; }; + + rng: rng@50060800 { + compatible = "st,stm32-rng"; + reg = <0x50060800 0x400>; + interrupts = <80>; + clocks = <&rcc 0 38>; + }; }; };
Hi Daniel,
On 10/03/2015 10:35 PM, Daniel Thompson wrote:
New bindings and driver have been created for STM32 series parts. This patch integrates this changes.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org
arch/arm/boot/dts/stm32f429.dtsi | 7 +++++++ 1 file changed, 7 insertions(+)
Acked-by: Maxime Coquelin mcoquelin.stm32@gmail.com
I will queue it to STM32 tree as soon as the DT bindings are accepted.
Thanks, Maxime
This patchset introduces a driver for the STM32 hardware random number generator.
v2:
* Moved binding docs from .../hwrng/ to .../rng/ and renamed to match convention in new directory (Rob Herring). * Adopted runtime PM and auto-suspend instead of managing the clocks from the read function (Linus Walleij). Increased bandwidth by ~30%. * Simplified error detection in main read loop (Linus Walleij, Maxime Coquelin). * Only WARN_ONCE() when hardware failure mechanisms trigger (Maxime Coquelin). * Simplify end of probe function after cocci warning (Fengguang Wu). * Switch to devm_hwrng_register.
Daniel Thompson (3): dt-bindings: Document the STM32 HW RNG bindings hwrng: stm32 - add support for STM32 HW RNG ARM: dts: stm32f429: Adopt STM32 RNG driver
.../devicetree/bindings/rng/st,stm32-rng.txt | 21 +++ arch/arm/boot/dts/stm32f429.dtsi | 7 + drivers/char/hw_random/Kconfig | 12 ++ drivers/char/hw_random/Makefile | 1 + drivers/char/hw_random/stm32-rng.c | 202 +++++++++++++++++++++ 5 files changed, 243 insertions(+) create mode 100644 Documentation/devicetree/bindings/rng/st,stm32-rng.txt create mode 100644 drivers/char/hw_random/stm32-rng.c
-- 2.4.3
This adds documentation of device tree bindings for the STM32 hardware random number generator.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org Acked-by: Maxime Coquelin mcoquelin.stm32@gmail.com Acked-by: Rob Herring robh@kernel.org --- .../devicetree/bindings/rng/st,stm32-rng.txt | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 Documentation/devicetree/bindings/rng/st,stm32-rng.txt
diff --git a/Documentation/devicetree/bindings/rng/st,stm32-rng.txt b/Documentation/devicetree/bindings/rng/st,stm32-rng.txt new file mode 100644 index 000000000000..47f04176f93b --- /dev/null +++ b/Documentation/devicetree/bindings/rng/st,stm32-rng.txt @@ -0,0 +1,21 @@ +STMicroelectronics STM32 HW RNG +=============================== + +The STM32 hardware random number generator is a simple fixed purpose IP and +is fully separated from other crypto functions. + +Required properties: + +- compatible : Should be "st,stm32-rng" +- reg : Should be register base and length as documented in the datasheet +- interrupts : The designated IRQ line for the RNG +- clocks : The clock needed to enable the RNG + +Example: + + rng: rng@50060800 { + compatible = "st,stm32-rng"; + reg = <0x50060800 0x400>; + interrupts = <80>; + clocks = <&rcc 0 38>; + };
Add support for STMicroelectronics STM32 random number generator.
The config value defaults to N, reflecting the fact that STM32 is a very low resource microcontroller platform and unlikely to be targeted by any "grown up" defconfigs.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org --- drivers/char/hw_random/Kconfig | 12 +++ drivers/char/hw_random/Makefile | 1 + drivers/char/hw_random/stm32-rng.c | 202 +++++++++++++++++++++++++++++++++++++ 3 files changed, 215 insertions(+) create mode 100644 drivers/char/hw_random/stm32-rng.c
diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig index f48cf11c655e..7930cc9b719c 100644 --- a/drivers/char/hw_random/Kconfig +++ b/drivers/char/hw_random/Kconfig @@ -359,6 +359,18 @@ config HW_RANDOM_XGENE
If unsure, say Y.
+config HW_RANDOM_STM32 + tristate "STMicroelectronics STM32 random number generator" + depends on HW_RANDOM && (ARCH_STM32 || COMPILE_TEST) + help + This driver provides kernel-side support for the Random Number + Generator hardware found on STM32 microcontrollers. + + To compile this driver as a module, choose M here: the + module will be called stm32-rng. + + If unsure, say N. + endif # HW_RANDOM
config UML_RANDOM diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile index 055bb01510ad..8b49c0f7abb1 100644 --- a/drivers/char/hw_random/Makefile +++ b/drivers/char/hw_random/Makefile @@ -31,3 +31,4 @@ obj-$(CONFIG_HW_RANDOM_BCM2835) += bcm2835-rng.o obj-$(CONFIG_HW_RANDOM_IPROC_RNG200) += iproc-rng200.o obj-$(CONFIG_HW_RANDOM_MSM) += msm-rng.o obj-$(CONFIG_HW_RANDOM_XGENE) += xgene-rng.o +obj-$(CONFIG_HW_RANDOM_STM32) += stm32-rng.o diff --git a/drivers/char/hw_random/stm32-rng.c b/drivers/char/hw_random/stm32-rng.c new file mode 100644 index 000000000000..7fa3656a5fc5 --- /dev/null +++ b/drivers/char/hw_random/stm32-rng.c @@ -0,0 +1,202 @@ +/* + * Copyright (c) 2015, Daniel Thompson + * + * This file is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This file is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/hw_random.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of_address.h> +#include <linux/of_platform.h> +#include <linux/pm_runtime.h> +#include <linux/slab.h> + +#define RNG_CR 0x00 +#define RNG_CR_RNGEN BIT(2) + +#define RNG_SR 0x04 +#define RNG_SR_SEIS BIT(6) +#define RNG_SR_CEIS BIT(5) +#define RNG_SR_DRDY BIT(0) + +#define RNG_DR 0x08 + +/* + * It takes 40 cycles @ 48MHz to generate each random number (e.g. <1us). + * At the time of writing STM32 parts max out at ~200MHz meaning a timeout + * of 500 leaves us a very comfortable margin for error. The loop to which + * the timeout applies takes at least 4 instructions per iteration so the + * timeout is enough to take us up to multi-GHz parts! + */ +#define RNG_TIMEOUT 500 + +struct stm32_rng_private { + struct hwrng rng; + void __iomem *base; + struct clk *clk; +}; + +static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool wait) +{ + struct stm32_rng_private *priv = + container_of(rng, struct stm32_rng_private, rng); + u32 sr; + int retval = 0; + + pm_runtime_get_sync((struct device *) priv->rng.priv); + + while (max > sizeof(u32)) { + sr = readl_relaxed(priv->base + RNG_SR); + if (!sr && wait) { + unsigned int timeout = RNG_TIMEOUT; + + do { + cpu_relax(); + sr = readl_relaxed(priv->base + RNG_SR); + } while (!sr && --timeout); + } + + /* If error detected or data not ready... */ + if (sr != RNG_SR_DRDY) + break; + + *(u32 *)data = readl_relaxed(priv->base + RNG_DR); + + retval += sizeof(u32); + data += sizeof(u32); + max -= sizeof(u32); + } + + if (WARN_ONCE(sr & (RNG_SR_SEIS | RNG_SR_CEIS), + "bad RNG status - %x\n", sr)) + writel_relaxed(0, priv->base + RNG_SR); + + pm_runtime_mark_last_busy((struct device *) priv->rng.priv); + pm_runtime_put_sync_autosuspend((struct device *) priv->rng.priv); + + return retval || !wait ? retval : -EIO; +} + +static int stm32_rng_init(struct hwrng *rng) +{ + struct stm32_rng_private *priv = + container_of(rng, struct stm32_rng_private, rng); + int err; + + err = clk_prepare_enable(priv->clk); + if (err) + return err; + + writel_relaxed(RNG_CR_RNGEN, priv->base + RNG_CR); + + /* clear error indicators */ + writel_relaxed(0, priv->base + RNG_SR); + + return 0; +} + +static void stm32_rng_cleanup(struct hwrng *rng) +{ + struct stm32_rng_private *priv = + container_of(rng, struct stm32_rng_private, rng); + + writel_relaxed(0, priv->base + RNG_CR); + clk_disable_unprepare(priv->clk); +} + +static int stm32_rng_probe(struct platform_device *ofdev) +{ + struct device *dev = &ofdev->dev; + struct device_node *np = ofdev->dev.of_node; + struct stm32_rng_private *priv; + struct resource res; + int err; + + priv = devm_kzalloc(dev, sizeof(struct stm32_rng_private), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + err = of_address_to_resource(np, 0, &res); + if (err) + return err; + + priv->base = devm_ioremap_resource(dev, &res); + if (IS_ERR(priv->base)) + return PTR_ERR(priv->base); + + priv->clk = devm_clk_get(&ofdev->dev, NULL); + if (IS_ERR(priv->clk)) + return PTR_ERR(priv->clk); + + dev_set_drvdata(dev, priv); + + priv->rng.name = dev_driver_string(dev), +#ifndef CONFIG_PM + priv->rng.init = stm32_rng_init, + priv->rng.cleanup = stm32_rng_cleanup, +#endif + priv->rng.read = stm32_rng_read, + priv->rng.priv = (unsigned long) dev; + + pm_runtime_set_autosuspend_delay(dev, 100); + pm_runtime_use_autosuspend(dev); + pm_runtime_enable(dev); + + return devm_hwrng_register(dev, &priv->rng); +} + +#ifdef CONFIG_PM +static int stm32_rng_runtime_suspend(struct device *dev) +{ + struct stm32_rng_private *priv = dev_get_drvdata(pdev); + + stm32_rng_cleanup(&priv->rng); + + return 0; +} + +static int stm32_rng_runtime_resume(struct device *dev) +{ + struct stm32_rng_private *priv = dev_get_drvdata(pdev); + + return stm32_rng_init(&priv->rng); +} +#endif + +static UNIVERSAL_DEV_PM_OPS(stm32_rng_pm_ops, stm32_rng_runtime_suspend, + stm32_rng_runtime_resume, NULL); + +static const struct of_device_id stm32_rng_match[] = { + { + .compatible = "st,stm32-rng", + }, + {}, +}; +MODULE_DEVICE_TABLE(of, stm32_rng_match); + +static struct platform_driver stm32_rng_driver = { + .driver = { + .name = "stm32-rng", + .pm = &stm32_rng_pm_ops, + .of_match_table = stm32_rng_match, + }, + .probe = stm32_rng_probe, +}; + +module_platform_driver(stm32_rng_driver); + +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("Daniel Thompson daniel.thompson@linaro.org"); +MODULE_DESCRIPTION("STMicroelectronics STM32 RNG device driver");
On Mon, Oct 12, 2015 at 10:21 AM, Daniel Thompson daniel.thompson@linaro.org wrote:
Add support for STMicroelectronics STM32 random number generator.
The config value defaults to N, reflecting the fact that STM32 is a very low resource microcontroller platform and unlikely to be targeted by any "grown up" defconfigs.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org
This is a fine driver, love the performance boost you reported and stand corrected on the likeness of the other Nomadik driver. Reviewed-by: Linus Walleij linus.walleij@linaro.org
+static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
Now this read() function is so nice that I feel obliged to go in and tidy up the Nomadik driver to be as nice.
Yours, Linus Walleij
New bindings and driver have been created for STM32 series parts. This patch integrates this changes.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org Acked-by: Maxime Coquelin mcoquelin.stm32@gmail.com --- arch/arm/boot/dts/stm32f429.dtsi | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi index cb0613090243..90081fc22c6c 100644 --- a/arch/arm/boot/dts/stm32f429.dtsi +++ b/arch/arm/boot/dts/stm32f429.dtsi @@ -175,6 +175,13 @@ reg = <0x40023800 0x400>; clocks = <&clk_hse>; }; + + rng: rng@50060800 { + compatible = "st,stm32-rng"; + reg = <0x50060800 0x400>; + interrupts = <80>; + clocks = <&rcc 0 38>; + }; }; };
On Mon, Oct 12, 2015 at 09:21:27AM +0100, Daniel Thompson wrote:
This patchset introduces a driver for the STM32 hardware random number generator.
v2:
- Moved binding docs from .../hwrng/ to .../rng/ and renamed to match convention in new directory (Rob Herring).
- Adopted runtime PM and auto-suspend instead of managing the clocks from the read function (Linus Walleij). Increased bandwidth by ~30%.
- Simplified error detection in main read loop (Linus Walleij, Maxime Coquelin).
- Only WARN_ONCE() when hardware failure mechanisms trigger (Maxime Coquelin).
- Simplify end of probe function after cocci warning (Fengguang Wu).
- Switch to devm_hwrng_register.
All applied. Thanks!
Herbert,
2015-10-14 16:28 GMT+02:00 Herbert Xu herbert@gondor.apana.org.au:
On Mon, Oct 12, 2015 at 09:21:27AM +0100, Daniel Thompson wrote:
This patchset introduces a driver for the STM32 hardware random number generator.
v2:
- Moved binding docs from .../hwrng/ to .../rng/ and renamed to match convention in new directory (Rob Herring).
- Adopted runtime PM and auto-suspend instead of managing the clocks from the read function (Linus Walleij). Increased bandwidth by ~30%.
- Simplified error detection in main read loop (Linus Walleij, Maxime Coquelin).
- Only WARN_ONCE() when hardware failure mechanisms trigger (Maxime Coquelin).
- Simplify end of probe function after cocci warning (Fengguang Wu).
- Switch to devm_hwrng_register.
All applied. Thanks!
I see you again have applied the DT patch like for STi three weeks ago. The DT patch should go through the arm_soc tree to avoid Linus having to handle conflicts resolution.
Can you please remove these two patches from your tree? ba25d8b ARM: STi: STiH407: Enable the 2 HW Random Number Generators for STiH4{07, 10} b47c9fa ARM: dts: stm32f429: Adopt STM32 RNG driver
Thanks, Maxime
This patchset introduces a driver for the STM32 hardware random number generator.
Herbert: I have assumed the v2 version is *so* badly broken (sorry again for that) that you would probably choose to remove v2 completely and replace it with v3. In a moment I will also post a patch to fix the build without ripping out v2. Choose whichever suits you best.
v3:
* Fixed build with CONFIG_PM (Fengguang Wu). Runtime tested as normal and also build tested on stm32 (which has CONFIG_PM set) and on x86/COMPILE_TEST with and without CONFIG_PM.
v2:
* Moved binding docs from .../hwrng/ to .../rng/ and renamed to match convention in new directory (Rob Herring). * Adopted runtime PM and auto-suspend instead of managing the clocks from the read function (Linus Walleij). Increased bandwidth by ~30%. * Simplified error detection in main read loop (Linus Walleij, Maxime Coquelin). * Only WARN_ONCE() when hardware failure mechanisms trigger (Maxime Coquelin). * Simplify end of probe function after cocci warning (Fengguang Wu). * Switch to devm_hwrng_register.
Daniel Thompson (3): dt-bindings: Document the STM32 HW RNG bindings hwrng: stm32 - add support for STM32 HW RNG ARM: dts: stm32f429: Adopt STM32 RNG driver
.../devicetree/bindings/rng/st,stm32-rng.txt | 21 +++ arch/arm/boot/dts/stm32f429.dtsi | 7 + drivers/char/hw_random/Kconfig | 12 ++ drivers/char/hw_random/Makefile | 1 + drivers/char/hw_random/stm32-rng.c | 202 +++++++++++++++++++++ 5 files changed, 243 insertions(+) create mode 100644 Documentation/devicetree/bindings/rng/st,stm32-rng.txt create mode 100644 drivers/char/hw_random/stm32-rng.c
-- 2.4.3
This adds documentation of device tree bindings for the STM32 hardware random number generator.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org Acked-by: Maxime Coquelin mcoquelin.stm32@gmail.com Acked-by: Rob Herring robh@kernel.org --- .../devicetree/bindings/rng/st,stm32-rng.txt | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 Documentation/devicetree/bindings/rng/st,stm32-rng.txt
diff --git a/Documentation/devicetree/bindings/rng/st,stm32-rng.txt b/Documentation/devicetree/bindings/rng/st,stm32-rng.txt new file mode 100644 index 000000000000..47f04176f93b --- /dev/null +++ b/Documentation/devicetree/bindings/rng/st,stm32-rng.txt @@ -0,0 +1,21 @@ +STMicroelectronics STM32 HW RNG +=============================== + +The STM32 hardware random number generator is a simple fixed purpose IP and +is fully separated from other crypto functions. + +Required properties: + +- compatible : Should be "st,stm32-rng" +- reg : Should be register base and length as documented in the datasheet +- interrupts : The designated IRQ line for the RNG +- clocks : The clock needed to enable the RNG + +Example: + + rng: rng@50060800 { + compatible = "st,stm32-rng"; + reg = <0x50060800 0x400>; + interrupts = <80>; + clocks = <&rcc 0 38>; + };
Add support for STMicroelectronics STM32 random number generator.
The config value defaults to N, reflecting the fact that STM32 is a very low resource microcontroller platform and unlikely to be targeted by any "grown up" defconfigs.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org Reviewed-by: Linus Walleij linus.walleij@linaro.org --- drivers/char/hw_random/Kconfig | 12 +++ drivers/char/hw_random/Makefile | 1 + drivers/char/hw_random/stm32-rng.c | 202 +++++++++++++++++++++++++++++++++++++ 3 files changed, 215 insertions(+) create mode 100644 drivers/char/hw_random/stm32-rng.c
diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig index f48cf11c655e..7930cc9b719c 100644 --- a/drivers/char/hw_random/Kconfig +++ b/drivers/char/hw_random/Kconfig @@ -359,6 +359,18 @@ config HW_RANDOM_XGENE
If unsure, say Y.
+config HW_RANDOM_STM32 + tristate "STMicroelectronics STM32 random number generator" + depends on HW_RANDOM && (ARCH_STM32 || COMPILE_TEST) + help + This driver provides kernel-side support for the Random Number + Generator hardware found on STM32 microcontrollers. + + To compile this driver as a module, choose M here: the + module will be called stm32-rng. + + If unsure, say N. + endif # HW_RANDOM
config UML_RANDOM diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile index 055bb01510ad..8b49c0f7abb1 100644 --- a/drivers/char/hw_random/Makefile +++ b/drivers/char/hw_random/Makefile @@ -31,3 +31,4 @@ obj-$(CONFIG_HW_RANDOM_BCM2835) += bcm2835-rng.o obj-$(CONFIG_HW_RANDOM_IPROC_RNG200) += iproc-rng200.o obj-$(CONFIG_HW_RANDOM_MSM) += msm-rng.o obj-$(CONFIG_HW_RANDOM_XGENE) += xgene-rng.o +obj-$(CONFIG_HW_RANDOM_STM32) += stm32-rng.o diff --git a/drivers/char/hw_random/stm32-rng.c b/drivers/char/hw_random/stm32-rng.c new file mode 100644 index 000000000000..92a810648bd0 --- /dev/null +++ b/drivers/char/hw_random/stm32-rng.c @@ -0,0 +1,202 @@ +/* + * Copyright (c) 2015, Daniel Thompson + * + * This file is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This file is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/hw_random.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of_address.h> +#include <linux/of_platform.h> +#include <linux/pm_runtime.h> +#include <linux/slab.h> + +#define RNG_CR 0x00 +#define RNG_CR_RNGEN BIT(2) + +#define RNG_SR 0x04 +#define RNG_SR_SEIS BIT(6) +#define RNG_SR_CEIS BIT(5) +#define RNG_SR_DRDY BIT(0) + +#define RNG_DR 0x08 + +/* + * It takes 40 cycles @ 48MHz to generate each random number (e.g. <1us). + * At the time of writing STM32 parts max out at ~200MHz meaning a timeout + * of 500 leaves us a very comfortable margin for error. The loop to which + * the timeout applies takes at least 4 instructions per iteration so the + * timeout is enough to take us up to multi-GHz parts! + */ +#define RNG_TIMEOUT 500 + +struct stm32_rng_private { + struct hwrng rng; + void __iomem *base; + struct clk *clk; +}; + +static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool wait) +{ + struct stm32_rng_private *priv = + container_of(rng, struct stm32_rng_private, rng); + u32 sr; + int retval = 0; + + pm_runtime_get_sync((struct device *) priv->rng.priv); + + while (max > sizeof(u32)) { + sr = readl_relaxed(priv->base + RNG_SR); + if (!sr && wait) { + unsigned int timeout = RNG_TIMEOUT; + + do { + cpu_relax(); + sr = readl_relaxed(priv->base + RNG_SR); + } while (!sr && --timeout); + } + + /* If error detected or data not ready... */ + if (sr != RNG_SR_DRDY) + break; + + *(u32 *)data = readl_relaxed(priv->base + RNG_DR); + + retval += sizeof(u32); + data += sizeof(u32); + max -= sizeof(u32); + } + + if (WARN_ONCE(sr & (RNG_SR_SEIS | RNG_SR_CEIS), + "bad RNG status - %x\n", sr)) + writel_relaxed(0, priv->base + RNG_SR); + + pm_runtime_mark_last_busy((struct device *) priv->rng.priv); + pm_runtime_put_sync_autosuspend((struct device *) priv->rng.priv); + + return retval || !wait ? retval : -EIO; +} + +static int stm32_rng_init(struct hwrng *rng) +{ + struct stm32_rng_private *priv = + container_of(rng, struct stm32_rng_private, rng); + int err; + + err = clk_prepare_enable(priv->clk); + if (err) + return err; + + writel_relaxed(RNG_CR_RNGEN, priv->base + RNG_CR); + + /* clear error indicators */ + writel_relaxed(0, priv->base + RNG_SR); + + return 0; +} + +static void stm32_rng_cleanup(struct hwrng *rng) +{ + struct stm32_rng_private *priv = + container_of(rng, struct stm32_rng_private, rng); + + writel_relaxed(0, priv->base + RNG_CR); + clk_disable_unprepare(priv->clk); +} + +static int stm32_rng_probe(struct platform_device *ofdev) +{ + struct device *dev = &ofdev->dev; + struct device_node *np = ofdev->dev.of_node; + struct stm32_rng_private *priv; + struct resource res; + int err; + + priv = devm_kzalloc(dev, sizeof(struct stm32_rng_private), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + err = of_address_to_resource(np, 0, &res); + if (err) + return err; + + priv->base = devm_ioremap_resource(dev, &res); + if (IS_ERR(priv->base)) + return PTR_ERR(priv->base); + + priv->clk = devm_clk_get(&ofdev->dev, NULL); + if (IS_ERR(priv->clk)) + return PTR_ERR(priv->clk); + + dev_set_drvdata(dev, priv); + + priv->rng.name = dev_driver_string(dev), +#ifndef CONFIG_PM + priv->rng.init = stm32_rng_init, + priv->rng.cleanup = stm32_rng_cleanup, +#endif + priv->rng.read = stm32_rng_read, + priv->rng.priv = (unsigned long) dev; + + pm_runtime_set_autosuspend_delay(dev, 100); + pm_runtime_use_autosuspend(dev); + pm_runtime_enable(dev); + + return devm_hwrng_register(dev, &priv->rng); +} + +#ifdef CONFIG_PM +static int stm32_rng_runtime_suspend(struct device *dev) +{ + struct stm32_rng_private *priv = dev_get_drvdata(dev); + + stm32_rng_cleanup(&priv->rng); + + return 0; +} + +static int stm32_rng_runtime_resume(struct device *dev) +{ + struct stm32_rng_private *priv = dev_get_drvdata(dev); + + return stm32_rng_init(&priv->rng); +} +#endif + +static UNIVERSAL_DEV_PM_OPS(stm32_rng_pm_ops, stm32_rng_runtime_suspend, + stm32_rng_runtime_resume, NULL); + +static const struct of_device_id stm32_rng_match[] = { + { + .compatible = "st,stm32-rng", + }, + {}, +}; +MODULE_DEVICE_TABLE(of, stm32_rng_match); + +static struct platform_driver stm32_rng_driver = { + .driver = { + .name = "stm32-rng", + .pm = &stm32_rng_pm_ops, + .of_match_table = stm32_rng_match, + }, + .probe = stm32_rng_probe, +}; + +module_platform_driver(stm32_rng_driver); + +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("Daniel Thompson daniel.thompson@linaro.org"); +MODULE_DESCRIPTION("STMicroelectronics STM32 RNG device driver");
New bindings and driver have been created for STM32 series parts. This patch integrates this changes.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org Acked-by: Maxime Coquelin mcoquelin.stm32@gmail.com --- arch/arm/boot/dts/stm32f429.dtsi | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi index cb0613090243..90081fc22c6c 100644 --- a/arch/arm/boot/dts/stm32f429.dtsi +++ b/arch/arm/boot/dts/stm32f429.dtsi @@ -175,6 +175,13 @@ reg = <0x40023800 0x400>; clocks = <&clk_hse>; }; + + rng: rng@50060800 { + compatible = "st,stm32-rng"; + reg = <0x50060800 0x400>; + interrupts = <80>; + clocks = <&rcc 0 38>; + }; }; };
Commit c6a97c42e399 ("hwrng: stm32 - add support for STM32 HW RNG") was inadequately tested (actually it was tested quite hard so incompetent would be a better description that inadequate) and does not compile on platforms with CONFIG_PM set.
Fix this.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org ---
Notes: Herbert: This is the diff between v2 and v3 of my STM32 HW RNG patch. This is partly reassurance of what has changed and also "just in case" you prefer to simply fix the problem.
drivers/char/hw_random/stm32-rng.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/char/hw_random/stm32-rng.c b/drivers/char/hw_random/stm32-rng.c index 7fa3656a5fc5..92a810648bd0 100644 --- a/drivers/char/hw_random/stm32-rng.c +++ b/drivers/char/hw_random/stm32-rng.c @@ -160,7 +160,7 @@ static int stm32_rng_probe(struct platform_device *ofdev) #ifdef CONFIG_PM static int stm32_rng_runtime_suspend(struct device *dev) { - struct stm32_rng_private *priv = dev_get_drvdata(pdev); + struct stm32_rng_private *priv = dev_get_drvdata(dev);
stm32_rng_cleanup(&priv->rng);
@@ -169,7 +169,7 @@ static int stm32_rng_runtime_suspend(struct device *dev)
static int stm32_rng_runtime_resume(struct device *dev) { - struct stm32_rng_private *priv = dev_get_drvdata(pdev); + struct stm32_rng_private *priv = dev_get_drvdata(dev);
return stm32_rng_init(&priv->rng); } -- 2.4.3
On Wed, Oct 14, 2015 at 05:04:55PM +0100, Daniel Thompson wrote:
Commit c6a97c42e399 ("hwrng: stm32 - add support for STM32 HW RNG") was inadequately tested (actually it was tested quite hard so incompetent would be a better description that inadequate) and does not compile on platforms with CONFIG_PM set.
Fix this.
Signed-off-by: Daniel Thompson daniel.thompson@linaro.org
Patch applied. Thanks!
linaro-kernel@lists.linaro.org