The main aim of this set is to allow users to access ST's hardware
random number generator. It's a simple device, which only requires
a simple driver.
We're also taking the liberty to update some out of date HWRNG
documentation and making the sysfs interface a little easier to
use by ignoring any '\n' which may have been inadvertently passed.
Lee Jones (6):
Documentation: hw_random: Fix device node name /dev/hw_random =>
/dev/hwrng
hwrng: core: Simplify RNG switching from sysfs
hwrng: st: Provide DT bindings for ST's Random Number Generator
hwrng: st: Add support for ST's HW Random Number Generator
ARM: STi: STiH407: Enable the 2 HW Random Number Generators for
STiH4{07,10}
MAINTAINERS: Add ST's Random Number Generator to the ST entry
Documentation/devicetree/bindings/rng/st,rng.txt | 15 +++
Documentation/hw_random.txt | 8 +-
MAINTAINERS | 1 +
arch/arm/boot/dts/stih407-family.dtsi | 14 +++
drivers/char/hw_random/Kconfig | 10 ++
drivers/char/hw_random/Makefile | 1 +
drivers/char/hw_random/core.c | 7 +-
drivers/char/hw_random/st-rng.c | 142 +++++++++++++++++++++++
8 files changed, 193 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/rng/st,rng.txt
create mode 100644 drivers/char/hw_random/st-rng.c
--
1.9.1
Signed-off-by: Lee Jones <[email protected]>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index fd60784..b084d69 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1502,6 +1502,7 @@ W: http://www.stlinux.com
S: Maintained
F: arch/arm/mach-sti/
F: arch/arm/boot/dts/sti*
+F: drivers/char/hw_random/st-rng.c
F: drivers/clocksource/arm_global_timer.c
F: drivers/i2c/busses/i2c-st.c
F: drivers/media/rc/st_rc.c
--
1.9.1
In April 2099, commit d405640 ("Driver Core: misc: add node name support
for misc devices.") inadvertently changed the device node name from
/dev/hw_random to /dev/hwrng. Since 6 years has passed since the change
it seems unpractical to change it back now, as this node name is probably
considered ABI by now. So instead, we'll just change the documentation
to match the current situation.
NB: It looks like rng-tools have already been updated.
Signed-off-by: Lee Jones <[email protected]>
---
Documentation/hw_random.txt | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/hw_random.txt b/Documentation/hw_random.txt
index 026e237..fce1634 100644
--- a/Documentation/hw_random.txt
+++ b/Documentation/hw_random.txt
@@ -3,7 +3,7 @@ Introduction:
The hw_random framework is software that makes use of a
special hardware feature on your CPU or motherboard,
a Random Number Generator (RNG). The software has two parts:
- a core providing the /dev/hw_random character device and its
+ a core providing the /dev/hwrng character device and its
sysfs support, plus a hardware-specific driver that plugs
into that core.
@@ -14,7 +14,7 @@ Introduction:
http://sourceforge.net/projects/gkernel/
- Those tools use /dev/hw_random to fill the kernel entropy pool,
+ Those tools use /dev/hwrng to fill the kernel entropy pool,
which is used internally and exported by the /dev/urandom and
/dev/random special files.
@@ -32,13 +32,13 @@ Theory of operation:
The rng-tools package uses such tests in "rngd", and lets you
run them by hand with a "rngtest" utility.
- /dev/hw_random is char device major 10, minor 183.
+ /dev/hwrng is char device major 10, minor 183.
CLASS DEVICE. There is a /sys/class/misc/hw_random node with
two unique attributes, "rng_available" and "rng_current". The
"rng_available" attribute lists the hardware-specific drivers
available, while "rng_current" lists the one which is currently
- connected to /dev/hw_random. If your system has more than one
+ connected to /dev/hwrng. If your system has more than one
RNG available, you may change the one used by writing a name from
the list in "rng_available" into "rng_current".
--
1.9.1
Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/boot/dts/stih407-family.dtsi | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/arch/arm/boot/dts/stih407-family.dtsi b/arch/arm/boot/dts/stih407-family.dtsi
index 838b812..9452b42 100644
--- a/arch/arm/boot/dts/stih407-family.dtsi
+++ b/arch/arm/boot/dts/stih407-family.dtsi
@@ -565,5 +565,19 @@
<&phy_port2 PHY_TYPE_USB3>;
};
};
+
+ rng10: rng@08a89000 {
+ compatible = "st,rng";
+ reg = <0x08a89000 0x1000>;
+ clocks = <&clk_sysin>;
+ status = "okay";
+ };
+
+ rng11: rng@08a8a000 {
+ compatible = "st,rng";
+ reg = <0x08a8a000 0x1000>;
+ clocks = <&clk_sysin>;
+ status = "okay";
+ };
};
};
--
1.9.1
Signed-off-by: Pankaj Dev <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/char/hw_random/Kconfig | 10 +++
drivers/char/hw_random/Makefile | 1 +
drivers/char/hw_random/st-rng.c | 142 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 153 insertions(+)
create mode 100644 drivers/char/hw_random/st-rng.c
diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index f48cf11..bb6cb77 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -346,6 +346,16 @@ config HW_RANDOM_MSM
If unsure, say Y.
+config HW_RANDOM_ST
+ tristate "ST Microelectronics HW Random Number Generator support"
+ depends on HW_RANDOM && ARCH_STI
+ ---help---
+ This driver provides kernel-side support for the Random Number
+ Generator hardware found on STi series of SoCs.
+
+ To compile this driver as a module, choose M here: the
+ module will be called st-rng.
+
config HW_RANDOM_XGENE
tristate "APM X-Gene True Random Number Generator (TRNG) support"
depends on HW_RANDOM && ARCH_XGENE
diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
index 055bb01..8bcfb45 100644
--- a/drivers/char/hw_random/Makefile
+++ b/drivers/char/hw_random/Makefile
@@ -30,4 +30,5 @@ obj-$(CONFIG_HW_RANDOM_TPM) += tpm-rng.o
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_ST) += st-rng.o
obj-$(CONFIG_HW_RANDOM_XGENE) += xgene-rng.o
diff --git a/drivers/char/hw_random/st-rng.c b/drivers/char/hw_random/st-rng.c
new file mode 100644
index 0000000..717a831
--- /dev/null
+++ b/drivers/char/hw_random/st-rng.c
@@ -0,0 +1,142 @@
+/*
+ * ST Random Number Generator Driver ST's Platforms
+ *
+ * Author: Pankaj Dev: <[email protected]>
+ * Lee Jones <[email protected]>
+ *
+ * Copyright (C) 2015 STMicroelectronics (R&D) Limited
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/hw_random.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+/* Registers */
+#define ST_RNG_STATUS_REG 0x20
+#define ST_RNG_DATA_REG 0x24
+
+/* Registers fields */
+#define ST_RNG_STATUS_BAD_SEQUENCE BIT(0)
+#define ST_RNG_STATUS_BAD_ALTERNANCE BIT(1)
+#define ST_RNG_STATUS_FIFO_FULL BIT(5)
+
+#define ST_RNG_FIFO_SIZE 8
+#define ST_RNG_SAMPLE_SIZE 2 /* 2 Byte (16bit) samples */
+
+/* Samples are available every 0.667us, which we round to 1us */
+#define ST_RNG_FILL_FIFO_TIMEOUT (1 * (ST_RNG_FIFO_SIZE / ST_RNG_SAMPLE_SIZE))
+
+struct st_rng_data {
+ void __iomem *base;
+ struct clk *clk;
+ struct hwrng ops;
+};
+
+static int st_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
+{
+ struct st_rng_data *ddata = (struct st_rng_data *)rng->priv;
+ u32 status;
+ int i;
+
+ if (max < sizeof(u16))
+ return -EINVAL;
+
+ /* Wait until FIFO is full - max 4uS*/
+ for (i = 0; i < ST_RNG_FILL_FIFO_TIMEOUT; i++) {
+ status = readl_relaxed(ddata->base + ST_RNG_STATUS_REG);
+ if (status & ST_RNG_STATUS_FIFO_FULL)
+ break;
+ udelay(1);
+ }
+
+ if (i == ST_RNG_FILL_FIFO_TIMEOUT)
+ return 0;
+
+ for (i = 0; i < ST_RNG_FIFO_SIZE && i < max; i += 2)
+ *(u16 *)(data + i) =
+ readl_relaxed(ddata->base + ST_RNG_DATA_REG);
+
+ return i - 2; /* No of bytes read */
+}
+
+static int st_rng_probe(struct platform_device *pdev)
+{
+ struct st_rng_data *ddata;
+ struct resource *res;
+ struct clk *clk;
+ void __iomem *base;
+ int ret;
+
+ ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL);
+ if (!ddata)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ clk = devm_clk_get(&pdev->dev, NULL);
+ if (!clk)
+ return -EINVAL;
+
+ clk_prepare_enable(clk);
+
+ ddata->ops.priv = (unsigned long)ddata;
+ ddata->ops.read = st_rng_read;
+ ddata->ops.name = pdev->name;
+ ddata->base = base;
+ ddata->clk = clk;
+
+ dev_set_drvdata(&pdev->dev, ddata);
+
+ ret = hwrng_register(&ddata->ops);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to register HW RNG\n");
+ return ret;
+ }
+
+ dev_info(&pdev->dev, "Successfully registered HW RNG\n");
+
+ return 0;
+}
+
+static int st_rng_remove(struct platform_device *pdev)
+{
+ struct st_rng_data *ddata = dev_get_drvdata(&pdev->dev);
+
+ hwrng_unregister(&ddata->ops);
+
+ clk_disable_unprepare(ddata->clk);
+
+ return 0;
+}
+
+static const struct of_device_id st_rng_match[] = {
+ { .compatible = "st,rng" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, st_rng_match);
+
+static struct platform_driver st_rng_driver = {
+ .driver = {
+ .name = "st-hwrandom",
+ .of_match_table = of_match_ptr(st_rng_match),
+ },
+ .probe = st_rng_probe,
+ .remove = st_rng_remove
+};
+
+module_platform_driver(st_rng_driver);
+
+MODULE_AUTHOR("Pankaj Dev <[email protected]>");
+MODULE_LICENSE("GPL v2");
--
1.9.1
Signed-off-by: Lee Jones <[email protected]>
---
Documentation/devicetree/bindings/rng/st,rng.txt | 15 +++++++++++++++
1 file changed, 15 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rng/st,rng.txt
diff --git a/Documentation/devicetree/bindings/rng/st,rng.txt b/Documentation/devicetree/bindings/rng/st,rng.txt
new file mode 100644
index 0000000..dbc64e6
--- /dev/null
+++ b/Documentation/devicetree/bindings/rng/st,rng.txt
@@ -0,0 +1,15 @@
+STMicroelectronics HW Random Number Generator
+----------------------------------------------
+
+Required parameters:
+compatible : Should be "st,rng"
+reg : Base address and size of IP's register map.
+clocks : Phandle to device's clock (See: ../clocks/clock-bindings.txt)
+
+Example:
+
+rng@0xfee80000 {
+ compatible = "st,rng";
+ reg = <0xfee80000 0x1000>;
+ clocks = <&clk_sysin>;
+}
--
1.9.1
If we attempt to use sysfs to change the current RNG in the usual
way i.e. issuing something like:
`echo 8a8a000.rng > /sys/devices/virtual/misc/hw_random/rng_current`
... it will fail because the code doesn't currently take the '\n'
into consideration. Well, now it does.
Signed-off-by: Lee Jones <[email protected]>
---
drivers/char/hw_random/core.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index da8faf7..14dc984 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -316,6 +316,7 @@ static ssize_t hwrng_attr_current_store(struct device *dev,
const char *buf, size_t len)
{
int err;
+ int snip = 0;
struct hwrng *rng;
err = mutex_lock_interruptible(&rng_mutex);
@@ -323,7 +324,11 @@ static ssize_t hwrng_attr_current_store(struct device *dev,
return -ERESTARTSYS;
err = -ENODEV;
list_for_each_entry(rng, &rng_list, list) {
- if (strcmp(rng->name, buf) == 0) {
+
+ if (buf[len-1] == '\n')
+ snip = 1; /* Snip one character */
+
+ if (strncmp(rng->name, buf, len - snip) == 0) {
err = 0;
if (rng != current_rng)
err = set_current_rng(rng);
--
1.9.1
Hi Lee,
On 11 September 2015 at 21:08, Lee Jones <[email protected]> wrote:
> Signed-off-by: Pankaj Dev <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> drivers/char/hw_random/Kconfig | 10 +++
> drivers/char/hw_random/Makefile | 1 +
> drivers/char/hw_random/st-rng.c | 142 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 153 insertions(+)
> create mode 100644 drivers/char/hw_random/st-rng.c
>
> diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
> index f48cf11..bb6cb77 100644
> --- a/drivers/char/hw_random/Kconfig
> +++ b/drivers/char/hw_random/Kconfig
> @@ -346,6 +346,16 @@ config HW_RANDOM_MSM
>
> If unsure, say Y.
>
> +config HW_RANDOM_ST
> + tristate "ST Microelectronics HW Random Number Generator support"
> + depends on HW_RANDOM && ARCH_STI
> + ---help---
> + This driver provides kernel-side support for the Random Number
> + Generator hardware found on STi series of SoCs.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called st-rng.
> +
> config HW_RANDOM_XGENE
> tristate "APM X-Gene True Random Number Generator (TRNG) support"
> depends on HW_RANDOM && ARCH_XGENE
> diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
> index 055bb01..8bcfb45 100644
> --- a/drivers/char/hw_random/Makefile
> +++ b/drivers/char/hw_random/Makefile
> @@ -30,4 +30,5 @@ obj-$(CONFIG_HW_RANDOM_TPM) += tpm-rng.o
> 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_ST) += st-rng.o
> obj-$(CONFIG_HW_RANDOM_XGENE) += xgene-rng.o
> diff --git a/drivers/char/hw_random/st-rng.c b/drivers/char/hw_random/st-rng.c
> new file mode 100644
> index 0000000..717a831
> --- /dev/null
> +++ b/drivers/char/hw_random/st-rng.c
> @@ -0,0 +1,142 @@
> +/*
> + * ST Random Number Generator Driver ST's Platforms
> + *
> + * Author: Pankaj Dev: <[email protected]>
> + * Lee Jones <[email protected]>
> + *
> + * Copyright (C) 2015 STMicroelectronics (R&D) Limited
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/hw_random.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +/* Registers */
> +#define ST_RNG_STATUS_REG 0x20
> +#define ST_RNG_DATA_REG 0x24
> +
> +/* Registers fields */
> +#define ST_RNG_STATUS_BAD_SEQUENCE BIT(0)
> +#define ST_RNG_STATUS_BAD_ALTERNANCE BIT(1)
> +#define ST_RNG_STATUS_FIFO_FULL BIT(5)
> +
> +#define ST_RNG_FIFO_SIZE 8
> +#define ST_RNG_SAMPLE_SIZE 2 /* 2 Byte (16bit) samples */
> +
> +/* Samples are available every 0.667us, which we round to 1us */
> +#define ST_RNG_FILL_FIFO_TIMEOUT (1 * (ST_RNG_FIFO_SIZE / ST_RNG_SAMPLE_SIZE))
> +
> +struct st_rng_data {
> + void __iomem *base;
> + struct clk *clk;
> + struct hwrng ops;
> +};
> +
> +static int st_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
> +{
> + struct st_rng_data *ddata = (struct st_rng_data *)rng->priv;
> + u32 status;
> + int i;
> +
> + if (max < sizeof(u16))
> + return -EINVAL;
> +
> + /* Wait until FIFO is full - max 4uS*/
> + for (i = 0; i < ST_RNG_FILL_FIFO_TIMEOUT; i++) {
> + status = readl_relaxed(ddata->base + ST_RNG_STATUS_REG);
> + if (status & ST_RNG_STATUS_FIFO_FULL)
> + break;
> + udelay(1);
> + }
> +
> + if (i == ST_RNG_FILL_FIFO_TIMEOUT)
> + return 0;
> +
> + for (i = 0; i < ST_RNG_FIFO_SIZE && i < max; i += 2)
> + *(u16 *)(data + i) =
> + readl_relaxed(ddata->base + ST_RNG_DATA_REG);
Will this code always be called with max % 2 == 0? ...
I.e. will
st_rng_read(rng, data[5], 5, waitflg)
overflow an array?
> +
> + return i - 2; /* No of bytes read */
Really? Doesn't this return 0 on st_rng_read(rng, data[2], 2, waitflg);
> +}
> +
> +static int st_rng_probe(struct platform_device *pdev)
> +{
> + struct st_rng_data *ddata;
> + struct resource *res;
> + struct clk *clk;
> + void __iomem *base;
> + int ret;
> +
> + ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL);
> + if (!ddata)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(base))
Are we leaking ddata here ? I.e. before its attached with set_drvdata?
> + return PTR_ERR(base);
> +
> + clk = devm_clk_get(&pdev->dev, NULL);
> + if (!clk)
And here ...
> + return -EINVAL;
> +
> + clk_prepare_enable(clk);
> +
> + ddata->ops.priv = (unsigned long)ddata;
> + ddata->ops.read = st_rng_read;
> + ddata->ops.name = pdev->name;
> + ddata->base = base;
> + ddata->clk = clk;
> +
> + dev_set_drvdata(&pdev->dev, ddata);
> +
> + ret = hwrng_register(&ddata->ops);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to register HW RNG\n");
> + return ret;
> + }
> +
> + dev_info(&pdev->dev, "Successfully registered HW RNG\n");
> +
> + return 0;
> +}
> +
> +static int st_rng_remove(struct platform_device *pdev)
> +{
> + struct st_rng_data *ddata = dev_get_drvdata(&pdev->dev);
> +
> + hwrng_unregister(&ddata->ops);
> +
> + clk_disable_unprepare(ddata->clk);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id st_rng_match[] = {
> + { .compatible = "st,rng" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, st_rng_match);
> +
> +static struct platform_driver st_rng_driver = {
> + .driver = {
> + .name = "st-hwrandom",
> + .of_match_table = of_match_ptr(st_rng_match),
> + },
> + .probe = st_rng_probe,
> + .remove = st_rng_remove
> +};
> +
> +module_platform_driver(st_rng_driver);
> +
> +MODULE_AUTHOR("Pankaj Dev <[email protected]>");
> +MODULE_LICENSE("GPL v2");
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Regards
Kieran
Now available for:
$GREATCOMPANY add -A linuxdevs/kieranbingham/ && \
$GREATCOMPANY commit -m "HR: Add fantastic new employee"
On Fri, Sep 11, 2015 at 5:08 PM, Lee Jones <[email protected]> wrote:
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + clk = devm_clk_get(&pdev->dev, NULL);
> + if (!clk)
> + return -EINVAL;
This should be:
if (IS_ERR(clk))
return PTR_ERR(clk);
> +
> + clk_prepare_enable(clk);
This may fail, so better check its return value and propagate it on error.
> +
> + ddata->ops.priv = (unsigned long)ddata;
> + ddata->ops.read = st_rng_read;
> + ddata->ops.name = pdev->name;
> + ddata->base = base;
> + ddata->clk = clk;
> +
> + dev_set_drvdata(&pdev->dev, ddata);
> +
> + ret = hwrng_register(&ddata->ops);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to register HW RNG\n");
> + return ret;
> + }
> +
> + dev_info(&pdev->dev, "Successfully registered HW RNG\n");
No need to put this info.
>>>>> "Lee" == Lee Jones <[email protected]> writes:
> If we attempt to use sysfs to change the current RNG in the usual
> way i.e. issuing something like:
> `echo 8a8a000.rng > /sys/devices/virtual/misc/hw_random/rng_current`
> ... it will fail because the code doesn't currently take the '\n'
> into consideration. Well, now it does.
> Signed-off-by: Lee Jones <[email protected]>
> ---
> drivers/char/hw_random/core.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index da8faf7..14dc984 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
> @@ -316,6 +316,7 @@ static ssize_t hwrng_attr_current_store(struct device *dev,
> const char *buf, size_t len)
> {
> int err;
> + int snip = 0;
> struct hwrng *rng;
> err = mutex_lock_interruptible(&rng_mutex);
> @@ -323,7 +324,11 @@ static ssize_t hwrng_attr_current_store(struct device *dev,
> return -ERESTARTSYS;
> err = -ENODEV;
> list_for_each_entry(rng, &rng_list, list) {
> - if (strcmp(rng->name, buf) == 0) {
> +
> + if (buf[len-1] == '\n')
> + snip = 1; /* Snip one character */
> +
How about using sysfs_streq() instead, which is what is done elsewhere?
--
Bye, Peter Korsgaard
On Sun, 13 Sep 2015, Peter Korsgaard wrote:
> >>>>> "Lee" == Lee Jones <[email protected]> writes:
>
> > If we attempt to use sysfs to change the current RNG in the usual
> > way i.e. issuing something like:
>
> > `echo 8a8a000.rng > /sys/devices/virtual/misc/hw_random/rng_current`
>
> > ... it will fail because the code doesn't currently take the '\n'
> > into consideration. Well, now it does.
>
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> > drivers/char/hw_random/core.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
>
> > diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> > index da8faf7..14dc984 100644
> > --- a/drivers/char/hw_random/core.c
> > +++ b/drivers/char/hw_random/core.c
> > @@ -316,6 +316,7 @@ static ssize_t hwrng_attr_current_store(struct device *dev,
> > const char *buf, size_t len)
> > {
> > int err;
> > + int snip = 0;
> > struct hwrng *rng;
>
> > err = mutex_lock_interruptible(&rng_mutex);
> > @@ -323,7 +324,11 @@ static ssize_t hwrng_attr_current_store(struct device *dev,
> > return -ERESTARTSYS;
> > err = -ENODEV;
> > list_for_each_entry(rng, &rng_list, list) {
> > - if (strcmp(rng->name, buf) == 0) {
> > +
> > + if (buf[len-1] == '\n')
> > + snip = 1; /* Snip one character */
> > +
>
> How about using sysfs_streq() instead, which is what is done elsewhere?
Perfect. This is exactly what I'm trying to do, thanks.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Sat, 12 Sep 2015, Fabio Estevam wrote:
> On Fri, Sep 11, 2015 at 5:08 PM, Lee Jones <[email protected]> wrote:
>
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + base = devm_ioremap_resource(&pdev->dev, res);
> > + if (IS_ERR(base))
> > + return PTR_ERR(base);
> > +
> > + clk = devm_clk_get(&pdev->dev, NULL);
> > + if (!clk)
> > + return -EINVAL;
>
> This should be:
>
> if (IS_ERR(clk))
> return PTR_ERR(clk);
You're right. Will fix.
> > +
> > + clk_prepare_enable(clk);
>
> This may fail, so better check its return value and propagate it on error.
Looks like the jury is out on this one.
$ git grep clk_prepare_enable | grep '= clk\|if (' | wc -l
659
$ git grep clk_prepare_enable | grep -v '= clk\|if (' | wc -l
569
... but it's not a problem to fix up. Will do.
> > + ddata->ops.priv = (unsigned long)ddata;
> > + ddata->ops.read = st_rng_read;
> > + ddata->ops.name = pdev->name;
> > + ddata->base = base;
> > + ddata->clk = clk;
> > +
> > + dev_set_drvdata(&pdev->dev, ddata);
> > +
> > + ret = hwrng_register(&ddata->ops);
> > + if (ret) {
> > + dev_err(&pdev->dev, "Failed to register HW RNG\n");
> > + return ret;
> > + }
> > +
> > + dev_info(&pdev->dev, "Successfully registered HW RNG\n");
>
> No need to put this info.
Ah, you caught me!
I know these types of prints are usually deemed not useful; however,
unless there is a failure both this driver and the core driver are
silent, so the user doesn't know that 1 or more of these devices are
available. I personally like to see an entry in the bootlog for this
kind of functionality.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Sat, 12 Sep 2015, Kieran Bingham wrote:
> On 11 September 2015 at 21:08, Lee Jones <[email protected]> wrote:
> > Signed-off-by: Pankaj Dev <[email protected]>
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> > drivers/char/hw_random/Kconfig | 10 +++
> > drivers/char/hw_random/Makefile | 1 +
> > drivers/char/hw_random/st-rng.c | 142 ++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 153 insertions(+)
> > create mode 100644 drivers/char/hw_random/st-rng.c
> >
> > diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
> > index f48cf11..bb6cb77 100644
> > --- a/drivers/char/hw_random/Kconfig
> > +++ b/drivers/char/hw_random/Kconfig
> > @@ -346,6 +346,16 @@ config HW_RANDOM_MSM
> >
> > If unsure, say Y.
> >
> > +config HW_RANDOM_ST
> > + tristate "ST Microelectronics HW Random Number Generator support"
> > + depends on HW_RANDOM && ARCH_STI
> > + ---help---
> > + This driver provides kernel-side support for the Random Number
> > + Generator hardware found on STi series of SoCs.
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called st-rng.
> > +
> > config HW_RANDOM_XGENE
> > tristate "APM X-Gene True Random Number Generator (TRNG) support"
> > depends on HW_RANDOM && ARCH_XGENE
> > diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
> > index 055bb01..8bcfb45 100644
> > --- a/drivers/char/hw_random/Makefile
> > +++ b/drivers/char/hw_random/Makefile
> > @@ -30,4 +30,5 @@ obj-$(CONFIG_HW_RANDOM_TPM) += tpm-rng.o
> > 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_ST) += st-rng.o
> > obj-$(CONFIG_HW_RANDOM_XGENE) += xgene-rng.o
> > diff --git a/drivers/char/hw_random/st-rng.c b/drivers/char/hw_random/st-rng.c
> > new file mode 100644
> > index 0000000..717a831
> > --- /dev/null
> > +++ b/drivers/char/hw_random/st-rng.c
> > @@ -0,0 +1,142 @@
> > +/*
> > + * ST Random Number Generator Driver ST's Platforms
> > + *
> > + * Author: Pankaj Dev: <[email protected]>
> > + * Lee Jones <[email protected]>
> > + *
> > + * Copyright (C) 2015 STMicroelectronics (R&D) Limited
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/hw_random.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/slab.h>
> > +
> > +/* Registers */
> > +#define ST_RNG_STATUS_REG 0x20
> > +#define ST_RNG_DATA_REG 0x24
> > +
> > +/* Registers fields */
> > +#define ST_RNG_STATUS_BAD_SEQUENCE BIT(0)
> > +#define ST_RNG_STATUS_BAD_ALTERNANCE BIT(1)
> > +#define ST_RNG_STATUS_FIFO_FULL BIT(5)
> > +
> > +#define ST_RNG_FIFO_SIZE 8
> > +#define ST_RNG_SAMPLE_SIZE 2 /* 2 Byte (16bit) samples */
> > +
> > +/* Samples are available every 0.667us, which we round to 1us */
> > +#define ST_RNG_FILL_FIFO_TIMEOUT (1 * (ST_RNG_FIFO_SIZE / ST_RNG_SAMPLE_SIZE))
> > +
> > +struct st_rng_data {
> > + void __iomem *base;
> > + struct clk *clk;
> > + struct hwrng ops;
> > +};
> > +
> > +static int st_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
> > +{
> > + struct st_rng_data *ddata = (struct st_rng_data *)rng->priv;
> > + u32 status;
> > + int i;
> > +
> > + if (max < sizeof(u16))
> > + return -EINVAL;
> > +
> > + /* Wait until FIFO is full - max 4uS*/
> > + for (i = 0; i < ST_RNG_FILL_FIFO_TIMEOUT; i++) {
> > + status = readl_relaxed(ddata->base + ST_RNG_STATUS_REG);
> > + if (status & ST_RNG_STATUS_FIFO_FULL)
> > + break;
> > + udelay(1);
> > + }
> > +
> > + if (i == ST_RNG_FILL_FIFO_TIMEOUT)
> > + return 0;
> > +
> > + for (i = 0; i < ST_RNG_FIFO_SIZE && i < max; i += 2)
> > + *(u16 *)(data + i) =
> > + readl_relaxed(ddata->base + ST_RNG_DATA_REG);
>
> Will this code always be called with max % 2 == 0? ...
> I.e. will
> st_rng_read(rng, data[5], 5, waitflg)
> overflow an array?
.read() can be called with either 16, 32 or the size of the cache,
which is unlikely to be anything !(N^2) (it's 64 in our case).
> > +
> > + return i - 2; /* No of bytes read */
>
> Really? Doesn't this return 0 on st_rng_read(rng, data[2], 2, waitflg);
Hmm... I assumed the OA's math was good here.
Good spot. I will run some tests to confirm.
> > +}
> > +
> > +static int st_rng_probe(struct platform_device *pdev)
> > +{
> > + struct st_rng_data *ddata;
> > + struct resource *res;
> > + struct clk *clk;
> > + void __iomem *base;
> > + int ret;
> > +
> > + ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL);
> > + if (!ddata)
> > + return -ENOMEM;
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + base = devm_ioremap_resource(&pdev->dev, res);
> > + if (IS_ERR(base))
>
> Are we leaking ddata here ? I.e. before its attached with set_drvdata?
It won't leak. That's the point of devm_*.
Please see: Documentation/driver-model/devres.txt
> > + return PTR_ERR(base);
> > +
> > + clk = devm_clk_get(&pdev->dev, NULL);
> > + if (!clk)
>
> And here ...
Nope.
> > + return -EINVAL;
> > +
> > + clk_prepare_enable(clk);
> > +
> > + ddata->ops.priv = (unsigned long)ddata;
> > + ddata->ops.read = st_rng_read;
> > + ddata->ops.name = pdev->name;
> > + ddata->base = base;
> > + ddata->clk = clk;
> > +
> > + dev_set_drvdata(&pdev->dev, ddata);
> > +
> > + ret = hwrng_register(&ddata->ops);
> > + if (ret) {
> > + dev_err(&pdev->dev, "Failed to register HW RNG\n");
> > + return ret;
> > + }
> > +
> > + dev_info(&pdev->dev, "Successfully registered HW RNG\n");
> > +
> > + return 0;
> > +}
> > +
> > +static int st_rng_remove(struct platform_device *pdev)
> > +{
> > + struct st_rng_data *ddata = dev_get_drvdata(&pdev->dev);
> > +
> > + hwrng_unregister(&ddata->ops);
> > +
> > + clk_disable_unprepare(ddata->clk);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id st_rng_match[] = {
> > + { .compatible = "st,rng" },
> > + {},
> > +};
> > +MODULE_DEVICE_TABLE(of, st_rng_match);
> > +
> > +static struct platform_driver st_rng_driver = {
> > + .driver = {
> > + .name = "st-hwrandom",
> > + .of_match_table = of_match_ptr(st_rng_match),
> > + },
> > + .probe = st_rng_probe,
> > + .remove = st_rng_remove
> > +};
> > +
> > +module_platform_driver(st_rng_driver);
> > +
> > +MODULE_AUTHOR("Pankaj Dev <[email protected]>");
> > +MODULE_LICENSE("GPL v2");
> >
>
>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Lee,
On 11 September 2015 at 21:08, Lee Jones <[email protected]> wrote:
> In April 2099, commit d405640
That's impressive ... :D
> ("Driver Core: misc: add node name support
> for misc devices.") inadvertently changed the device node name from
> /dev/hw_random to /dev/hwrng. Since 6 years has passed since the change
> it seems unpractical to change it back now, as this node name is probably
> considered ABI by now. So instead, we'll just change the documentation
> to match the current situation.
Ugh ... how messy ... but yes - I agree it may be too late to change
back as rngd already defaults to /dev/hwrng
Perhaps you could just use your time machine to continue back in time
to before it was used :-) In fact can I borrow it after you're done
please :)
Other than 's/2099/2009/g' - The patch is OK. All entries to
/dev/hw_random in hw_random.txt converted,
but outside of Documentation, drivers/char/hw_random/Kconfig also
references /dev/hw_random ...
Therefore perhaps that could be updated in this patch too?
--
Kieran
Now available for:
$GREATCOMPANY add -A linuxdevs/kieranbingham/ && \
$GREATCOMPANY commit -m "HR: Add fantastic new employee"
> NB: It looks like rng-tools have already been updated.
>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> Documentation/hw_random.txt | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/hw_random.txt b/Documentation/hw_random.txt
> index 026e237..fce1634 100644
> --- a/Documentation/hw_random.txt
> +++ b/Documentation/hw_random.txt
> @@ -3,7 +3,7 @@ Introduction:
> The hw_random framework is software that makes use of a
> special hardware feature on your CPU or motherboard,
> a Random Number Generator (RNG). The software has two parts:
> - a core providing the /dev/hw_random character device and its
> + a core providing the /dev/hwrng character device and its
> sysfs support, plus a hardware-specific driver that plugs
> into that core.
>
> @@ -14,7 +14,7 @@ Introduction:
>
> http://sourceforge.net/projects/gkernel/
>
> - Those tools use /dev/hw_random to fill the kernel entropy pool,
> + Those tools use /dev/hwrng to fill the kernel entropy pool,
> which is used internally and exported by the /dev/urandom and
> /dev/random special files.
>
> @@ -32,13 +32,13 @@ Theory of operation:
> The rng-tools package uses such tests in "rngd", and lets you
> run them by hand with a "rngtest" utility.
>
> - /dev/hw_random is char device major 10, minor 183.
> + /dev/hwrng is char device major 10, minor 183.
>
> CLASS DEVICE. There is a /sys/class/misc/hw_random node with
> two unique attributes, "rng_available" and "rng_current". The
> "rng_available" attribute lists the hardware-specific drivers
> available, while "rng_current" lists the one which is currently
> - connected to /dev/hw_random. If your system has more than one
> + connected to /dev/hwrng. If your system has more than one
> RNG available, you may change the one used by writing a name from
> the list in "rng_available" into "rng_current".
>
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html