[PATCH 0/3] True RNG driver for Samsung Exynos 5250+ SoCs

[PATCH 0/3] True RNG driver for Samsung Exynos 5250+ SoCs

[Łukasz Stelmach]

Hello.

The following patches add support for the true random number generator
found in Samsung Exynos 5250+ SoCs.

Patch #1 adds documentation for devicetree bindings.

Patch #2 introduces the driver and appropriate changes in Makefile and Kconfig.

Patch #3 adds nodes in devicetree files for Exynos SoCs.

Łukasz Stelmach (3):
  dt-bindings: hwrng: Add Samsung Exynos 5250+ True RNG bindings
  hwrng: exynos - add Samsung Exynos True RNG driver
  ARM: dts: exynos: Add nodes for True Random Number Generator

 .../bindings/rng/samsung,exynos5250-trng.txt       |  17 ++
 MAINTAINERS                                        |   7 +
 arch/arm/boot/dts/exynos5.dtsi                     |   5 +
 arch/arm/boot/dts/exynos5250.dtsi                  |   5 +
 arch/arm/boot/dts/exynos5410.dtsi                  |   5 +
 arch/arm/boot/dts/exynos5420.dtsi                  |   5 +
 drivers/char/hw_random/Kconfig                     |  12 +
 drivers/char/hw_random/Makefile                    |   1 +
 drivers/char/hw_random/exynos-trng.c               | 256 +++++++++++++++++++++
 9 files changed, 313 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rng/samsung,exynos5250-trng.txt
 create mode 100644 drivers/char/hw_random/exynos-trng.c

-- 
2.11.0

[PATCH 1/3] dt-bindings: hwrng: Add Samsung Exynos 5250+ True RNG bindings

[Łukasz Stelmach]

Add binding documentation for the True Random Number Generator
found on Samsung Exynos 5250+ SoCs.

Signed-off-by: Łukasz Stelmach <l.stelmach-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 .../devicetree/bindings/rng/samsung,exynos5250-trng.txt | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rng/samsung,exynos5250-trng.txt

diff --git a/Documentation/devicetree/bindings/rng/samsung,exynos5250-trng.txt b/Documentation/devicetree/bindings/rng/samsung,exynos5250-trng.txt
new file mode 100644
index 000000000000..22dfe8cb21c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/rng/samsung,exynos5250-trng.txt
@@ -0,0 +1,17 @@
+Exynos True Random Number Generator
+
+Required properties:
+
+- compatible  : Should be "samsung,exynos5250-trng".
+- reg         : Specifies base physical address and size of the registers map.
+- clocks      : Phandle to clock-controller plus clock-specifier pair.
+- clock-names : "secss" as a clock name.
+
+Example:
+
+	trng@10830600 {
+		compatible = "samsung,exynos5250-trng";
+		reg = <0x10830600 0x100>;
+		clocks = <&clock CLK_SSS>;
+		clock-names = "secss";
+	};
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] dt-bindings: hwrng: Add Samsung Exynos 5250+ True RNG bindings

[Łukasz Stelmach]

Add binding documentation for the True Random Number Generator
found on Samsung Exynos 5250+ SoCs.

Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com>
---
 .../devicetree/bindings/rng/samsung,exynos5250-trng.txt | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rng/samsung,exynos5250-trng.txt

diff --git a/Documentation/devicetree/bindings/rng/samsung,exynos5250-trng.txt b/Documentation/devicetree/bindings/rng/samsung,exynos5250-trng.txt
new file mode 100644
index 000000000000..22dfe8cb21c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/rng/samsung,exynos5250-trng.txt
@@ -0,0 +1,17 @@
+Exynos True Random Number Generator
+
+Required properties:
+
+- compatible  : Should be "samsung,exynos5250-trng".
+- reg         : Specifies base physical address and size of the registers map.
+- clocks      : Phandle to clock-controller plus clock-specifier pair.
+- clock-names : "secss" as a clock name.
+
+Example:
+
+	trng@10830600 {
+		compatible = "samsung,exynos5250-trng";
+		reg = <0x10830600 0x100>;
+		clocks = <&clock CLK_SSS>;
+		clock-names = "secss";
+	};
-- 
2.11.0

[PATCH 2/3] hwrng: exynos - add Samsung Exynos True RNG driver

[Łukasz Stelmach]

Add support for True Random Number Generator found in Samsung Exynos
5250+ SoCs.

Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com>
---
 MAINTAINERS                          |   7 +
 drivers/char/hw_random/Kconfig       |  12 ++
 drivers/char/hw_random/Makefile      |   1 +
 drivers/char/hw_random/exynos-trng.c | 256 +++++++++++++++++++++++++++++++++++
 4 files changed, 276 insertions(+)
 create mode 100644 drivers/char/hw_random/exynos-trng.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 2811a211632c..992074cca612 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11780,6 +11780,13 @@ S:	Maintained
 F:	drivers/crypto/exynos-rng.c
 F:	Documentation/devicetree/bindings/rng/samsung,exynos-rng4.txt
 
+SAMSUNG EXYNOS TRUE RANDOM NUMBER GENERATOR (TRNG) DRIVER
+M:	Łukasz Stelmach <l.stelmach@samsung.com>
+L:	linux-samsung-soc@vger.kernel.org
+S:	Maintained
+F:	drivers/char/hw_random/exynos-trng.c
+F:	Documentation/devicetree/bindings/rng/samsung,exynos5250-trng.txt
+
 SAMSUNG FRAMEBUFFER DRIVER
 M:	Jingoo Han <jingoohan1@gmail.com>
 L:	linux-fbdev@vger.kernel.org
diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index 95a031e9eced..a788ac07081b 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -449,6 +449,18 @@ config HW_RANDOM_S390
 
 	  If unsure, say Y.
 
+config HW_RANDOM_EXYNOS
+       tristate "Samsung Exynos True Random Number Generator support"
+       depends on ARCH_EXYNOS || COMPILE_TEST
+       default HW_RANDOM
+       ---help---
+         This driver provides support for the True Random Number
+         Generator available in Exynos SoCs.
+
+	 To compile this driver as a module, choose M here: the module
+	 will be called exynos-trng.
+
+	 If unsure, say Y.
 endif # HW_RANDOM
 
 config UML_RANDOM
diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
index f3728d008fff..5595df97da7a 100644
--- a/drivers/char/hw_random/Makefile
+++ b/drivers/char/hw_random/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_HW_RANDOM_GEODE) += geode-rng.o
 obj-$(CONFIG_HW_RANDOM_N2RNG) += n2-rng.o
 n2-rng-y := n2-drv.o n2-asm.o
 obj-$(CONFIG_HW_RANDOM_VIA) += via-rng.o
+obj-$(CONFIG_HW_RANDOM_EXYNOS) += exynos-trng.o
 obj-$(CONFIG_HW_RANDOM_IXP4XX) += ixp4xx-rng.o
 obj-$(CONFIG_HW_RANDOM_OMAP) += omap-rng.o
 obj-$(CONFIG_HW_RANDOM_OMAP3_ROM) += omap3-rom-rng.o
diff --git a/drivers/char/hw_random/exynos-trng.c b/drivers/char/hw_random/exynos-trng.c
new file mode 100644
index 000000000000..340e106cae24
--- /dev/null
+++ b/drivers/char/hw_random/exynos-trng.c
@@ -0,0 +1,256 @@
+/*
+ * RNG driver for Exynos TRNGs
+ *
+ * Author: Łukasz Stelmach <l.stelmach@samsung.com>
+ *
+ * Copyright 2017 (c) Samsung Electronics Software, Inc.
+ *
+ * Based on the Exynos PRNG driver drivers/crypto/exynos-rng by
+ * Krzysztof Kozłowski <krzk@kernel.org>
+ *
+ * This program 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;
+ *
+ * This program 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/crypto.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/hw_random.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+
+#define EXYNOS_TRNG_CLKDIV         (0x0)
+#define EXYNOS_TRNG_CTRL           (0x20)
+#define EXYNOS_TRNG_POST_CTRL      (0x30)
+#define EXYNOS_TRNG_ONLINE_CTRL    (0x40)
+#define EXYNOS_TRNG_ONLINE_STAT    (0x44)
+#define EXYNOS_TRNG_ONLINE_MAXCHI2 (0x48)
+#define EXYNOS_TRNG_FIFO_CTRL      (0x50)
+#define EXYNOS_TRNG_FIFO_0         (0x80)
+#define EXYNOS_TRNG_FIFO_1         (0x84)
+#define EXYNOS_TRNG_FIFO_2         (0x88)
+#define EXYNOS_TRNG_FIFO_3         (0x8c)
+#define EXYNOS_TRNG_FIFO_4         (0x90)
+#define EXYNOS_TRNG_FIFO_5         (0x94)
+#define EXYNOS_TRNG_FIFO_6         (0x98)
+#define EXYNOS_TRNG_FIFO_7         (0x9c)
+#define EXYNOS_TRNG_FIFO_LEN       (8)
+#define EXYNOS_TRNG_CLOCK_RATE     (500000)
+
+struct exynos_trng_dev {
+	struct device    *dev;
+	void __iomem     *mem;
+	struct clk       *clk;
+	struct hwrng rng;
+};
+
+struct exynos_trng_dev *exynos_trng_dev;
+
+static inline void exynos_trng_set_reg(struct exynos_trng_dev *trng, u16 reg,
+				       u32 val)
+{
+	/* Check range of reg? */
+	__raw_writel(val, trng->mem + reg);
+}
+
+static inline u32 exynos_trng_get_reg(struct exynos_trng_dev *trng, u16 reg)
+{
+	/* Check range of reg? */
+	return __raw_readl(trng->mem + reg);
+}
+
+static int exynos_trng_do_read(struct hwrng *rng, void *data, size_t max,
+			       bool wait)
+{
+	struct exynos_trng_dev *trng;
+	u32 val;
+
+	max = max > (EXYNOS_TRNG_FIFO_LEN * 4) ?
+		(EXYNOS_TRNG_FIFO_LEN * 4) : max;
+
+	trng = (struct exynos_trng_dev *)rng->priv;
+
+	exynos_trng_set_reg(trng, EXYNOS_TRNG_FIFO_CTRL, max * 8);
+	val = readl_poll_timeout(trng->mem + EXYNOS_TRNG_FIFO_CTRL, val,
+				 val == 0, 200, 1000000);
+	if (val < 0)
+		return val;
+
+	memcpy_fromio(data, trng->mem + EXYNOS_TRNG_FIFO_0, max);
+
+	return max;
+}
+
+static int exynos_trng_init(struct hwrng *rng)
+{
+	struct exynos_trng_dev *trng;
+	unsigned long sss_rate;
+	u32 val;
+
+	trng = (struct exynos_trng_dev *)rng->priv;
+	sss_rate = clk_get_rate(trng->clk);
+
+	/* For most TRNG circuits the clock frequency of under 500 kHz
+	 * is safe.
+	 */
+	val = sss_rate / (EXYNOS_TRNG_CLOCK_RATE * 2);
+	if (val > 0x7fff) {
+		dev_err(trng->dev, "clock divider too large: %d", val);
+		return -ERANGE;
+	}
+	val = val << 1;
+	exynos_trng_set_reg(trng, EXYNOS_TRNG_CLKDIV, val);
+
+	/* Enable the generator. */
+	val = 1 << 31;
+	exynos_trng_set_reg(trng, EXYNOS_TRNG_CTRL, val);
+
+	/* Disable post processing. /dev/hwrng is supposed to deliver
+	 * unprocessed data.
+	 */
+	exynos_trng_set_reg(trng, EXYNOS_TRNG_POST_CTRL, 0);
+
+	return 0;
+}
+
+static int exynos_trng_probe(struct platform_device *pdev)
+{
+	struct exynos_trng_dev *trng;
+	struct resource *res;
+	int ret = -ENOMEM;
+
+	if (exynos_trng_dev)
+		return -EEXIST;
+
+	trng = devm_kzalloc(&pdev->dev, sizeof(*trng), GFP_KERNEL);
+	if (!trng)
+		goto err_ioremap;
+
+	trng->rng.name = devm_kstrdup(&pdev->dev, dev_name(&pdev->dev),
+				      GFP_KERNEL);
+	if (!trng->rng.name)
+		goto err_ioremap;
+
+	trng->rng.init = exynos_trng_init;
+	trng->rng.read = exynos_trng_do_read;
+	trng->rng.priv = (unsigned long) trng;
+
+	platform_set_drvdata(pdev, trng);
+	trng->dev = &pdev->dev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	trng->mem = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(trng->mem)) {
+		dev_err(&pdev->dev, "Couldn't map IO resources.\n");
+		ret = PTR_ERR(trng->mem);
+		goto err_ioremap;
+	}
+
+	pm_runtime_enable(&pdev->dev);
+	ret = pm_runtime_get_sync(&pdev->dev);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "pm_runtime_get_sync() failed: %d.\n", ret);
+		goto err_ioremap;
+	}
+
+	trng->clk = devm_clk_get(&pdev->dev, "secss");
+	if (IS_ERR(trng->clk)) {
+		/* XXX: EPROBE_DEFER ? */
+		dev_err(&pdev->dev, "Couldn't get clock.\n");
+		ret = PTR_ERR(trng->clk);
+		goto err_clock;
+	}
+
+	ret = clk_prepare_enable(trng->clk);
+	if (ret) {
+		dev_err(&pdev->dev, "Unable to enable the clk: %d.\n", ret);
+		goto err_clock;
+	}
+
+	ret = hwrng_register(&trng->rng);
+	if (ret) {
+		dev_err(&pdev->dev, "Couldn't register hwrng device.\n");
+		goto err_register;
+	}
+
+	dev_info(&pdev->dev, "Exynos True Random Number Generator.\n");
+
+	return 0;
+
+err_register:
+	clk_disable_unprepare(trng->clk);
+
+err_clock:
+	trng->mem = NULL;
+	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+
+err_ioremap:
+	dev_err(&pdev->dev, "Initialisation failed.\n");
+	return ret;
+}
+
+static int exynos_trng_remove(struct platform_device *pdev)
+{
+	exynos_trng_dev = NULL;
+
+	return 0;
+}
+
+static int __maybe_unused exynos_trng_suspend(struct device *dev)
+{
+	pm_runtime_put_sync(dev);
+
+	return 0;
+}
+
+static int __maybe_unused exynos_trng_resume(struct device *dev)
+{
+	int ret;
+
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0) {
+		dev_err(dev, "pm_runtime_get_sync() failed: %d.\n", ret);
+		pm_runtime_put_noidle(dev);
+		return ret;
+	}
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(exynos_trng_pm_ops, exynos_trng_suspend,
+			 exynos_trng_resume);
+
+static const struct of_device_id exynos_trng_dt_match[] = {
+	{
+		.compatible = "samsung,exynos5250-trng",
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, exynos_rng_dt_match);
+
+static struct platform_driver exynos_trng_driver = {
+	.driver = {
+		.name = "exynos-trng",
+		.pm = &exynos_trng_pm_ops,
+		.of_match_table = exynos_trng_dt_match,
+	},
+	.probe = exynos_trng_probe,
+	.remove = exynos_trng_remove,
+};
+
+module_platform_driver(exynos_trng_driver);
+MODULE_AUTHOR("Łukasz Stelmach");
+MODULE_DESCRIPTION("H/W TRNG driver for Exynos chips");
+MODULE_LICENSE("GPL");
-- 
2.11.0

[PATCH 3/3] ARM: dts: exynos: Add nodes for True Random Number Generator

[Łukasz Stelmach]

Add nodes for the True Random Number Generator found in Samsung Exynos
5250+ SoCs.

Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com>
---
 arch/arm/boot/dts/exynos5.dtsi    | 5 +++++
 arch/arm/boot/dts/exynos5250.dtsi | 5 +++++
 arch/arm/boot/dts/exynos5410.dtsi | 5 +++++
 arch/arm/boot/dts/exynos5420.dtsi | 5 +++++
 4 files changed, 20 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5.dtsi b/arch/arm/boot/dts/exynos5.dtsi
index 7da4a4285b24..6a298b536e1c 100644
--- a/arch/arm/boot/dts/exynos5.dtsi
+++ b/arch/arm/boot/dts/exynos5.dtsi
@@ -210,5 +210,10 @@
 			reg = <0x10830000 0x300>;
 			interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
 		};
+
+		trng: trng@10830600 {
+		      compatible = "samsung,exynos5250-trng";
+		      reg = <0x10830600 0x100>;
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 8032d7e567fc..332fe04b772b 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -1081,4 +1081,9 @@
 	clock-names = "secss";
 };
 
+&trng {
+	clocks = <&clock CLK_SSS>;
+	clock-names = "secss";
+};
+
 #include "exynos5250-pinctrl.dtsi"
diff --git a/arch/arm/boot/dts/exynos5410.dtsi b/arch/arm/boot/dts/exynos5410.dtsi
index 7eab4bc07cec..f3991206a562 100644
--- a/arch/arm/boot/dts/exynos5410.dtsi
+++ b/arch/arm/boot/dts/exynos5410.dtsi
@@ -379,6 +379,11 @@
 		  3 0 0x07000000 0x20000>;
 };
 
+&trng {
+	clocks = <&clock CLK_SSS>;
+	clock-names = "secss";
+};
+
 &usbdrd3_0 {
 	clocks = <&clock CLK_USBD300>;
 	clock-names = "usbdrd30";
diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index 02d2f898efa6..2f4e5542f354 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -1454,6 +1454,11 @@
 	clock-names = "secss";
 };
 
+&trng {
+	clocks = <&clock CLK_SSS>;
+	clock-names = "secss";
+};
+
 &usbdrd3_0 {
 	clocks = <&clock CLK_USBD300>;
 	clock-names = "usbdrd30";
-- 
2.11.0

Re: [PATCH 2/3] hwrng: exynos - add Samsung Exynos True RNG driver

[Andrew F. Davis]

On 11/23/2017 09:09 AM, Łukasz Stelmach wrote:
> Add support for True Random Number Generator found in Samsung Exynos
> 5250+ SoCs.
> 
> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com>
> ---
>  MAINTAINERS                          |   7 +
>  drivers/char/hw_random/Kconfig       |  12 ++
>  drivers/char/hw_random/Makefile      |   1 +
>  drivers/char/hw_random/exynos-trng.c | 256 +++++++++++++++++++++++++++++++++++
>  4 files changed, 276 insertions(+)
>  create mode 100644 drivers/char/hw_random/exynos-trng.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2811a211632c..992074cca612 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11780,6 +11780,13 @@ S:	Maintained
>  F:	drivers/crypto/exynos-rng.c
>  F:	Documentation/devicetree/bindings/rng/samsung,exynos-rng4.txt
>  
> +SAMSUNG EXYNOS TRUE RANDOM NUMBER GENERATOR (TRNG) DRIVER
> +M:	Łukasz Stelmach <l.stelmach@samsung.com>
> +L:	linux-samsung-soc@vger.kernel.org
> +S:	Maintained
> +F:	drivers/char/hw_random/exynos-trng.c
> +F:	Documentation/devicetree/bindings/rng/samsung,exynos5250-trng.txt
> +
>  SAMSUNG FRAMEBUFFER DRIVER
>  M:	Jingoo Han <jingoohan1@gmail.com>
>  L:	linux-fbdev@vger.kernel.org
> diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
> index 95a031e9eced..a788ac07081b 100644
> --- a/drivers/char/hw_random/Kconfig
> +++ b/drivers/char/hw_random/Kconfig
> @@ -449,6 +449,18 @@ config HW_RANDOM_S390
>  
>  	  If unsure, say Y.
>  
> +config HW_RANDOM_EXYNOS
> +       tristate "Samsung Exynos True Random Number Generator support"
> +       depends on ARCH_EXYNOS || COMPILE_TEST
> +       default HW_RANDOM
> +       ---help---
> +         This driver provides support for the True Random Number
> +         Generator available in Exynos SoCs.
> +

Some whitespace strangness here.

> +	 To compile this driver as a module, choose M here: the module
> +	 will be called exynos-trng.
> +
> +	 If unsure, say Y.
>  endif # HW_RANDOM
>  
>  config UML_RANDOM
> diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
> index f3728d008fff..5595df97da7a 100644
> --- a/drivers/char/hw_random/Makefile
> +++ b/drivers/char/hw_random/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_HW_RANDOM_GEODE) += geode-rng.o
>  obj-$(CONFIG_HW_RANDOM_N2RNG) += n2-rng.o
>  n2-rng-y := n2-drv.o n2-asm.o
>  obj-$(CONFIG_HW_RANDOM_VIA) += via-rng.o
> +obj-$(CONFIG_HW_RANDOM_EXYNOS) += exynos-trng.o
>  obj-$(CONFIG_HW_RANDOM_IXP4XX) += ixp4xx-rng.o
>  obj-$(CONFIG_HW_RANDOM_OMAP) += omap-rng.o
>  obj-$(CONFIG_HW_RANDOM_OMAP3_ROM) += omap3-rom-rng.o
> diff --git a/drivers/char/hw_random/exynos-trng.c b/drivers/char/hw_random/exynos-trng.c
> new file mode 100644
> index 000000000000..340e106cae24
> --- /dev/null
> +++ b/drivers/char/hw_random/exynos-trng.c
> @@ -0,0 +1,256 @@
> +/*
> + * RNG driver for Exynos TRNGs
> + *
> + * Author: Łukasz Stelmach <l.stelmach@samsung.com>
> + *
> + * Copyright 2017 (c) Samsung Electronics Software, Inc.
> + *
> + * Based on the Exynos PRNG driver drivers/crypto/exynos-rng by
> + * Krzysztof Kozłowski <krzk@kernel.org>
> + *
> + * This program 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;
> + *
> + * This program 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/crypto.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/hw_random.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +
> +#define EXYNOS_TRNG_CLKDIV         (0x0)
> +#define EXYNOS_TRNG_CTRL           (0x20)
> +#define EXYNOS_TRNG_POST_CTRL      (0x30)
> +#define EXYNOS_TRNG_ONLINE_CTRL    (0x40)
> +#define EXYNOS_TRNG_ONLINE_STAT    (0x44)
> +#define EXYNOS_TRNG_ONLINE_MAXCHI2 (0x48)
> +#define EXYNOS_TRNG_FIFO_CTRL      (0x50)
> +#define EXYNOS_TRNG_FIFO_0         (0x80)
> +#define EXYNOS_TRNG_FIFO_1         (0x84)
> +#define EXYNOS_TRNG_FIFO_2         (0x88)
> +#define EXYNOS_TRNG_FIFO_3         (0x8c)
> +#define EXYNOS_TRNG_FIFO_4         (0x90)
> +#define EXYNOS_TRNG_FIFO_5         (0x94)
> +#define EXYNOS_TRNG_FIFO_6         (0x98)
> +#define EXYNOS_TRNG_FIFO_7         (0x9c)
> +#define EXYNOS_TRNG_FIFO_LEN       (8)
> +#define EXYNOS_TRNG_CLOCK_RATE     (500000)
> +
> +struct exynos_trng_dev {
> +	struct device    *dev;
> +	void __iomem     *mem;
> +	struct clk       *clk;
> +	struct hwrng rng;
> +};
> +
> +struct exynos_trng_dev *exynos_trng_dev;


This only seems to be used as a flag to block this driver from probing
twice, but it doesn't even do that as it is not ever set to anything.
Even if it was, why block a system from having two?

> +
> +static inline void exynos_trng_set_reg(struct exynos_trng_dev *trng, u16 reg,
> +				       u32 val)
> +{
> +	/* Check range of reg? */
> +	__raw_writel(val, trng->mem + reg);

If you are not checking, then just drop this one line function, it adds
nothing.

> +}
> +
> +static inline u32 exynos_trng_get_reg(struct exynos_trng_dev *trng, u16 reg)
> +{
> +	/* Check range of reg? */
> +	return __raw_readl(trng->mem + reg);

Same

> +}
> +
> +static int exynos_trng_do_read(struct hwrng *rng, void *data, size_t max,
> +			       bool wait)
> +{
> +	struct exynos_trng_dev *trng;
> +	u32 val;
> +
> +	max = max > (EXYNOS_TRNG_FIFO_LEN * 4) ?
> +		(EXYNOS_TRNG_FIFO_LEN * 4) : max;

min()

> +
> +	trng = (struct exynos_trng_dev *)rng->priv;
> +
> +	exynos_trng_set_reg(trng, EXYNOS_TRNG_FIFO_CTRL, max * 8);
> +	val = readl_poll_timeout(trng->mem + EXYNOS_TRNG_FIFO_CTRL, val,
> +				 val == 0, 200, 1000000);
> +	if (val < 0)
> +		return val;
> +
> +	memcpy_fromio(data, trng->mem + EXYNOS_TRNG_FIFO_0, max);
> +
> +	return max;
> +}
> +
> +static int exynos_trng_init(struct hwrng *rng)
> +{
> +	struct exynos_trng_dev *trng;
> +	unsigned long sss_rate;
> +	u32 val;
> +
> +	trng = (struct exynos_trng_dev *)rng->priv;

Move up to initializer.

> +	sss_rate = clk_get_rate(trng->clk);
> +
> +	/* For most TRNG circuits the clock frequency of under 500 kHz
> +	 * is safe.
> +	 */
> +	val = sss_rate / (EXYNOS_TRNG_CLOCK_RATE * 2);
> +	if (val > 0x7fff) {
> +		dev_err(trng->dev, "clock divider too large: %d", val);
> +		return -ERANGE;
> +	}
> +	val = val << 1;
> +	exynos_trng_set_reg(trng, EXYNOS_TRNG_CLKDIV, val);
> +
> +	/* Enable the generator. */
> +	val = 1 << 31;

#define TRNG_CTRL_ENABLE BIT(31)

> +	exynos_trng_set_reg(trng, EXYNOS_TRNG_CTRL, val);
> +
> +	/* Disable post processing. /dev/hwrng is supposed to deliver
> +	 * unprocessed data.
> +	 */
> +	exynos_trng_set_reg(trng, EXYNOS_TRNG_POST_CTRL, 0);
> +
> +	return 0;
> +}
> +
> +static int exynos_trng_probe(struct platform_device *pdev)
> +{
> +	struct exynos_trng_dev *trng;
> +	struct resource *res;
> +	int ret = -ENOMEM;
> +
> +	if (exynos_trng_dev)
> +		return -EEXIST;
> +
> +	trng = devm_kzalloc(&pdev->dev, sizeof(*trng), GFP_KERNEL);
> +	if (!trng)
> +		goto err_ioremap;
> +
> +	trng->rng.name = devm_kstrdup(&pdev->dev, dev_name(&pdev->dev),
> +				      GFP_KERNEL);
> +	if (!trng->rng.name)
> +		goto err_ioremap;
> +
> +	trng->rng.init = exynos_trng_init;
> +	trng->rng.read = exynos_trng_do_read;
> +	trng->rng.priv = (unsigned long) trng;
> +
> +	platform_set_drvdata(pdev, trng);
> +	trng->dev = &pdev->dev;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	trng->mem = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(trng->mem)) {
> +		dev_err(&pdev->dev, "Couldn't map IO resources.\n");
> +		ret = PTR_ERR(trng->mem);
> +		goto err_ioremap;
> +	}
> +
> +	pm_runtime_enable(&pdev->dev);
> +	ret = pm_runtime_get_sync(&pdev->dev);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "pm_runtime_get_sync() failed: %d.\n", ret);

"Could not get runtime PM",

Be descriptive an consistent, the other dev_errs don't print ret, it
will get printed as part of the probe fail message.

> +		goto err_ioremap;
> +	}
> +
> +	trng->clk = devm_clk_get(&pdev->dev, "secss");
> +	if (IS_ERR(trng->clk)) {
> +		/* XXX: EPROBE_DEFER ? */

Are you asking? If it's posible the clock is not available then yes, you
should not print the below message and exit gracefully.

> +		dev_err(&pdev->dev, "Couldn't get clock.\n");
> +		ret = PTR_ERR(trng->clk);
> +		goto err_clock;
> +	}
> +
> +	ret = clk_prepare_enable(trng->clk);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Unable to enable the clk: %d.\n", ret);
> +		goto err_clock;
> +	}
> +
> +	ret = hwrng_register(&trng->rng);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Couldn't register hwrng device.\n");
> +		goto err_register;
> +	}
> +
> +	dev_info(&pdev->dev, "Exynos True Random Number Generator.\n");
> +
> +	return 0;
> +
> +err_register:
> +	clk_disable_unprepare(trng->clk);
> +
> +err_clock:
> +	trng->mem = NULL;
> +	pm_runtime_put_sync(&pdev->dev);
> +	pm_runtime_disable(&pdev->dev);
> +
> +err_ioremap:
> +	dev_err(&pdev->dev, "Initialisation failed.\n");
> +	return ret;
> +}
> +
> +static int exynos_trng_remove(struct platform_device *pdev)
> +{

pm_runtime_*, or just use devm_ versions and drop this function.

> +	exynos_trng_dev = NULL;
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused exynos_trng_suspend(struct device *dev)
> +{
> +	pm_runtime_put_sync(dev);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused exynos_trng_resume(struct device *dev)
> +{
> +	int ret;
> +
> +	ret = pm_runtime_get_sync(dev);
> +	if (ret < 0) {
> +		dev_err(dev, "pm_runtime_get_sync() failed: %d.\n", ret);
> +		pm_runtime_put_noidle(dev);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(exynos_trng_pm_ops, exynos_trng_suspend,
> +			 exynos_trng_resume);
> +
> +static const struct of_device_id exynos_trng_dt_match[] = {
> +	{
> +		.compatible = "samsung,exynos5250-trng",
> +	},
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, exynos_rng_dt_match);
> +
> +static struct platform_driver exynos_trng_driver = {
> +	.driver = {
> +		.name = "exynos-trng",
> +		.pm = &exynos_trng_pm_ops,
> +		.of_match_table = exynos_trng_dt_match,
> +	},
> +	.probe = exynos_trng_probe,
> +	.remove = exynos_trng_remove,
> +};
> +
> +module_platform_driver(exynos_trng_driver);
> +MODULE_AUTHOR("Łukasz Stelmach");
> +MODULE_DESCRIPTION("H/W TRNG driver for Exynos chips");
> +MODULE_LICENSE("GPL");
> 

Re: [PATCH 1/3] dt-bindings: hwrng: Add Samsung Exynos 5250+ True RNG bindings

[Krzysztof Kozlowski]

On Thu, Nov 23, 2017 at 4:09 PM, Łukasz Stelmach <l.stelmach@samsung.com> wrote:
> Add binding documentation for the True Random Number Generator
> found on Samsung Exynos 5250+ SoCs.
>
> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com>
> ---
>  .../devicetree/bindings/rng/samsung,exynos5250-trng.txt | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/rng/samsung,exynos5250-trng.txt

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

Re: [PATCH 2/3] hwrng: exynos - add Samsung Exynos True RNG driver

[Krzysztof Kozlowski]

On Thu, Nov 23, 2017 at 4:09 PM, Łukasz Stelmach <l.stelmach@samsung.com> wrote:
> Add support for True Random Number Generator found in Samsung Exynos
> 5250+ SoCs.
>
> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com>
> ---
>  MAINTAINERS                          |   7 +
>  drivers/char/hw_random/Kconfig       |  12 ++
>  drivers/char/hw_random/Makefile      |   1 +
>  drivers/char/hw_random/exynos-trng.c | 256 +++++++++++++++++++++++++++++++++++
>  4 files changed, 276 insertions(+)
>  create mode 100644 drivers/char/hw_random/exynos-trng.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2811a211632c..992074cca612 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11780,6 +11780,13 @@ S:     Maintained
>  F:     drivers/crypto/exynos-rng.c
>  F:     Documentation/devicetree/bindings/rng/samsung,exynos-rng4.txt
>
> +SAMSUNG EXYNOS TRUE RANDOM NUMBER GENERATOR (TRNG) DRIVER
> +M:     Łukasz Stelmach <l.stelmach@samsung.com>
> +L:     linux-samsung-soc@vger.kernel.org
> +S:     Maintained
> +F:     drivers/char/hw_random/exynos-trng.c
> +F:     Documentation/devicetree/bindings/rng/samsung,exynos5250-trng.txt
> +
>  SAMSUNG FRAMEBUFFER DRIVER
>  M:     Jingoo Han <jingoohan1@gmail.com>
>  L:     linux-fbdev@vger.kernel.org
> diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
> index 95a031e9eced..a788ac07081b 100644
> --- a/drivers/char/hw_random/Kconfig
> +++ b/drivers/char/hw_random/Kconfig
> @@ -449,6 +449,18 @@ config HW_RANDOM_S390
>
>           If unsure, say Y.
>
> +config HW_RANDOM_EXYNOS
> +       tristate "Samsung Exynos True Random Number Generator support"
> +       depends on ARCH_EXYNOS || COMPILE_TEST
> +       default HW_RANDOM
> +       ---help---
> +         This driver provides support for the True Random Number
> +         Generator available in Exynos SoCs.
> +
> +        To compile this driver as a module, choose M here: the module
> +        will be called exynos-trng.
> +
> +        If unsure, say Y.
>  endif # HW_RANDOM

Thanks for working on TRNG.

1. I was rather thinking about extending existing exynos-rng.c [1] so
it would be using TRNG as seed for PRNG as this gives you much more
random data. Instead you developed totally separate driver which has
its own benefits - one can choose which interface he wants. Although
it is a little bit duplication.

2. I understand that in your current version of TRNG driver there is
no conflict with PRNG and they can work both at the same time?

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/crypto/exynos-rng.c

>
>  config UML_RANDOM
> diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
> index f3728d008fff..5595df97da7a 100644
> --- a/drivers/char/hw_random/Makefile
> +++ b/drivers/char/hw_random/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_HW_RANDOM_GEODE) += geode-rng.o
>  obj-$(CONFIG_HW_RANDOM_N2RNG) += n2-rng.o
>  n2-rng-y := n2-drv.o n2-asm.o
>  obj-$(CONFIG_HW_RANDOM_VIA) += via-rng.o
> +obj-$(CONFIG_HW_RANDOM_EXYNOS) += exynos-trng.o
>  obj-$(CONFIG_HW_RANDOM_IXP4XX) += ixp4xx-rng.o
>  obj-$(CONFIG_HW_RANDOM_OMAP) += omap-rng.o
>  obj-$(CONFIG_HW_RANDOM_OMAP3_ROM) += omap3-rom-rng.o
> diff --git a/drivers/char/hw_random/exynos-trng.c b/drivers/char/hw_random/exynos-trng.c
> new file mode 100644
> index 000000000000..340e106cae24
> --- /dev/null
> +++ b/drivers/char/hw_random/exynos-trng.c
> @@ -0,0 +1,256 @@
> +/*
> + * RNG driver for Exynos TRNGs
> + *
> + * Author: Łukasz Stelmach <l.stelmach@samsung.com>
> + *
> + * Copyright 2017 (c) Samsung Electronics Software, Inc.
> + *
> + * Based on the Exynos PRNG driver drivers/crypto/exynos-rng by
> + * Krzysztof Kozłowski <krzk@kernel.org>
> + *
> + * This program 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;
> + *
> + * This program 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/crypto.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/hw_random.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +
> +#define EXYNOS_TRNG_CLKDIV         (0x0)
> +#define EXYNOS_TRNG_CTRL           (0x20)
> +#define EXYNOS_TRNG_POST_CTRL      (0x30)
> +#define EXYNOS_TRNG_ONLINE_CTRL    (0x40)
> +#define EXYNOS_TRNG_ONLINE_STAT    (0x44)
> +#define EXYNOS_TRNG_ONLINE_MAXCHI2 (0x48)
> +#define EXYNOS_TRNG_FIFO_CTRL      (0x50)
> +#define EXYNOS_TRNG_FIFO_0         (0x80)
> +#define EXYNOS_TRNG_FIFO_1         (0x84)
> +#define EXYNOS_TRNG_FIFO_2         (0x88)
> +#define EXYNOS_TRNG_FIFO_3         (0x8c)
> +#define EXYNOS_TRNG_FIFO_4         (0x90)
> +#define EXYNOS_TRNG_FIFO_5         (0x94)
> +#define EXYNOS_TRNG_FIFO_6         (0x98)
> +#define EXYNOS_TRNG_FIFO_7         (0x9c)
> +#define EXYNOS_TRNG_FIFO_LEN       (8)
> +#define EXYNOS_TRNG_CLOCK_RATE     (500000)
> +
> +struct exynos_trng_dev {
> +       struct device    *dev;
> +       void __iomem     *mem;
> +       struct clk       *clk;
> +       struct hwrng rng;
> +};
> +
> +struct exynos_trng_dev *exynos_trng_dev;

This definitely should not be globally linked. Also I think this is
not even needed to be file-scope.

> +
> +static inline void exynos_trng_set_reg(struct exynos_trng_dev *trng, u16 reg,
> +                                      u32 val)
> +{
> +       /* Check range of reg? */

Is this is a FIXME/TODO note?

> +       __raw_writel(val, trng->mem + reg);
> +}
> +
> +static inline u32 exynos_trng_get_reg(struct exynos_trng_dev *trng, u16 reg)
> +{
> +       /* Check range of reg? */

Ditto

> +       return __raw_readl(trng->mem + reg);
> +}
> +
> +static int exynos_trng_do_read(struct hwrng *rng, void *data, size_t max,
> +                              bool wait)
> +{
> +       struct exynos_trng_dev *trng;
> +       u32 val;
> +
> +       max = max > (EXYNOS_TRNG_FIFO_LEN * 4) ?
> +               (EXYNOS_TRNG_FIFO_LEN * 4) : max;

Maybe use macro min()? This will also nicely point that variable name
"max" is misleading. Max what? Isn't it the size of buffer to read?


> +
> +       trng = (struct exynos_trng_dev *)rng->priv;
> +
> +       exynos_trng_set_reg(trng, EXYNOS_TRNG_FIFO_CTRL, max * 8);
> +       val = readl_poll_timeout(trng->mem + EXYNOS_TRNG_FIFO_CTRL, val,
> +                                val == 0, 200, 1000000);
> +       if (val < 0)
> +               return val;
> +
> +       memcpy_fromio(data, trng->mem + EXYNOS_TRNG_FIFO_0, max);
> +
> +       return max;
> +}
> +
> +static int exynos_trng_init(struct hwrng *rng)
> +{
> +       struct exynos_trng_dev *trng;
> +       unsigned long sss_rate;
> +       u32 val;
> +
> +       trng = (struct exynos_trng_dev *)rng->priv;
> +       sss_rate = clk_get_rate(trng->clk);
> +
> +       /* For most TRNG circuits the clock frequency of under 500 kHz

/* Before actual comment

> +        * is safe.
> +        */
> +       val = sss_rate / (EXYNOS_TRNG_CLOCK_RATE * 2);
> +       if (val > 0x7fff) {
> +               dev_err(trng->dev, "clock divider too large: %d", val);
> +               return -ERANGE;
> +       }
> +       val = val << 1;
> +       exynos_trng_set_reg(trng, EXYNOS_TRNG_CLKDIV, val);
> +
> +       /* Enable the generator. */
> +       val = 1 << 31;
> +       exynos_trng_set_reg(trng, EXYNOS_TRNG_CTRL, val);
> +
> +       /* Disable post processing. /dev/hwrng is supposed to deliver
> +        * unprocessed data.
> +        */

Ditto

> +       exynos_trng_set_reg(trng, EXYNOS_TRNG_POST_CTRL, 0);
> +
> +       return 0;
> +}
> +
> +static int exynos_trng_probe(struct platform_device *pdev)
> +{
> +       struct exynos_trng_dev *trng;
> +       struct resource *res;
> +       int ret = -ENOMEM;
> +
> +       if (exynos_trng_dev)
> +               return -EEXIST;
> +
> +       trng = devm_kzalloc(&pdev->dev, sizeof(*trng), GFP_KERNEL);
> +       if (!trng)
> +               goto err_ioremap;

No, this should just return with -ENOMEM. Do not print errors on
memory alloc failures.

> +
> +       trng->rng.name = devm_kstrdup(&pdev->dev, dev_name(&pdev->dev),
> +                                     GFP_KERNEL);
> +       if (!trng->rng.name)
> +               goto err_ioremap;

Ditto

> +
> +       trng->rng.init = exynos_trng_init;
> +       trng->rng.read = exynos_trng_do_read;
> +       trng->rng.priv = (unsigned long) trng;
> +
> +       platform_set_drvdata(pdev, trng);
> +       trng->dev = &pdev->dev;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       trng->mem = devm_ioremap_resource(&pdev->dev, res);
> +       if (IS_ERR(trng->mem)) {
> +               dev_err(&pdev->dev, "Couldn't map IO resources.\n");
> +               ret = PTR_ERR(trng->mem);
> +               goto err_ioremap;
> +       }
> +
> +       pm_runtime_enable(&pdev->dev);
> +       ret = pm_runtime_get_sync(&pdev->dev);
> +       if (ret < 0) {
> +               dev_err(&pdev->dev, "pm_runtime_get_sync() failed: %d.\n", ret);
> +               goto err_ioremap;

Missing pm_runtime_disable?

> +       }
> +
> +       trng->clk = devm_clk_get(&pdev->dev, "secss");
> +       if (IS_ERR(trng->clk)) {
> +               /* XXX: EPROBE_DEFER ? */

TODO note?
If you have questions how to deal with something, ask them. Do not
force reader to rediscover problems you are aware of.

> +               dev_err(&pdev->dev, "Couldn't get clock.\n");
> +               ret = PTR_ERR(trng->clk);
> +               goto err_clock;
> +       }
> +
> +       ret = clk_prepare_enable(trng->clk);
> +       if (ret) {
> +               dev_err(&pdev->dev, "Unable to enable the clk: %d.\n", ret);
> +               goto err_clock;
> +       }
> +
> +       ret = hwrng_register(&trng->rng);
> +       if (ret) {
> +               dev_err(&pdev->dev, "Couldn't register hwrng device.\n");
> +               goto err_register;
> +       }
> +
> +       dev_info(&pdev->dev, "Exynos True Random Number Generator.\n");
> +
> +       return 0;
> +
> +err_register:
> +       clk_disable_unprepare(trng->clk);
> +
> +err_clock:
> +       trng->mem = NULL;

Why this has to be null-ified?

> +       pm_runtime_put_sync(&pdev->dev);
> +       pm_runtime_disable(&pdev->dev);
> +
> +err_ioremap:
> +       dev_err(&pdev->dev, "Initialisation failed.\n");

Double error msg, you already print one before.

Best regards,
Krzysztof

Re: [PATCH 3/3] ARM: dts: exynos: Add nodes for True Random Number Generator

[Krzysztof Kozlowski]

On Thu, Nov 23, 2017 at 4:09 PM, Łukasz Stelmach <l.stelmach@samsung.com> wrote:
> Add nodes for the True Random Number Generator found in Samsung Exynos
> 5250+ SoCs.
>
> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com>
> ---
>  arch/arm/boot/dts/exynos5.dtsi    | 5 +++++
>  arch/arm/boot/dts/exynos5250.dtsi | 5 +++++
>  arch/arm/boot/dts/exynos5410.dtsi | 5 +++++
>  arch/arm/boot/dts/exynos5420.dtsi | 5 +++++
>  4 files changed, 20 insertions(+)
>

Look okay, I'll take it once bindings and driver get accepted as wel.

Best regards,
Krzysztof

Re: [PATCH 2/3] hwrng: exynos - add Samsung Exynos True RNG driver

[Łukasz Stelmach]

[-- Attachment #1: Type: text/plain, Size: 3683 bytes --]

It was <2017-11-23 czw 17:31>, when Krzysztof Kozlowski wrote:
> On Thu, Nov 23, 2017 at 4:09 PM, Łukasz Stelmach <l.stelmach@samsung.com> wrote:
>> Add support for True Random Number Generator found in Samsung Exynos
>> 5250+ SoCs.
>>
>> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com>
>> ---
>>  MAINTAINERS                          |   7 +
>>  drivers/char/hw_random/Kconfig       |  12 ++
>>  drivers/char/hw_random/Makefile      |   1 +
>>  drivers/char/hw_random/exynos-trng.c | 256 +++++++++++++++++++++++++++++++++++
>>  4 files changed, 276 insertions(+)
>>  create mode 100644 drivers/char/hw_random/exynos-trng.c
>>

[...]

>>  endif # HW_RANDOM
>
> Thanks for working on TRNG.
>
> 1. I was rather thinking about extending existing exynos-rng.c [1] so
> it would be using TRNG as seed for PRNG as this gives you much more
> random data. Instead you developed totally separate driver which has
> its own benefits - one can choose which interface he wants. Although
> it is a little bit duplication.

As far as I can tell, these are two different devices. However, PRNG
shares hardware with the hash engine. Indeed there is a hardware to
connect TRNG and PRNG, but, IMHO, it might be hard to model that
dependency in kernel. To me it seems easier to read TRNG (or
/dev/random) and and write the result to PRNG manually (in software).

> 2. I understand that in your current version of TRNG driver there is
> no conflict with PRNG and they can work both at the same time?

I guess so. I expect no problems as long as TRNG_SEED_START is not set
in the HASH_SEED_CONF register.

> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/crypto/exynos-rng.c
>

>> +
>> +       trng->rng.init = exynos_trng_init;
>> +       trng->rng.read = exynos_trng_do_read;
>> +       trng->rng.priv = (unsigned long) trng;
>> +
>> +       platform_set_drvdata(pdev, trng);
>> +       trng->dev = &pdev->dev;
>> +
>> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +       trng->mem = devm_ioremap_resource(&pdev->dev, res);
>> +       if (IS_ERR(trng->mem)) {
>> +               dev_err(&pdev->dev, "Couldn't map IO resources.\n");
>> +               ret = PTR_ERR(trng->mem);
>> +               goto err_ioremap;
>> +       }
>> +
>> +       pm_runtime_enable(&pdev->dev);
>> +       ret = pm_runtime_get_sync(&pdev->dev);
>> +       if (ret < 0) {
>> +               dev_err(&pdev->dev, "pm_runtime_get_sync() failed: %d.\n", ret);
>> +               goto err_ioremap;
>
> Missing pm_runtime_disable?

Added a separate label down below.

>> +               dev_err(&pdev->dev, "Couldn't get clock.\n");
>> +               ret = PTR_ERR(trng->clk);
>> +               goto err_clock;
>> +       }
>> +
>> +       ret = clk_prepare_enable(trng->clk);
>> +       if (ret) {
>> +               dev_err(&pdev->dev, "Unable to enable the clk: %d.\n", ret);
>> +               goto err_clock;
>> +       }
>> +
>> +       ret = hwrng_register(&trng->rng);
>> +       if (ret) {
>> +               dev_err(&pdev->dev, "Couldn't register hwrng device.\n");
>> +               goto err_register;
>> +       }
>> +
>> +       dev_info(&pdev->dev, "Exynos True Random Number Generator.\n");
>> +
>> +       return 0;
>> +
>> +err_register:
>> +       clk_disable_unprepare(trng->clk);
>> +
>> +err_clock:
>> +       trng->mem = NULL;
>
> Why this has to be null-ified?

I found this pattern in omap-rng.c.

Removed.

I'll wait a little bit more before posting v2.

-- 
Łukasz Stelmach
Samsung R&D Institute Poland
Samsung Electronics

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

Re: [PATCH 2/3] hwrng: exynos - add Samsung Exynos True RNG driver

[Krzysztof Kozlowski]

On Thu, Nov 23, 2017 at 7:46 PM, Łukasz Stelmach <l.stelmach@samsung.com> wrote:
> It was <2017-11-23 czw 17:31>, when Krzysztof Kozlowski wrote:
>> On Thu, Nov 23, 2017 at 4:09 PM, Łukasz Stelmach <l.stelmach@samsung.com> wrote:
>>> Add support for True Random Number Generator found in Samsung Exynos
>>> 5250+ SoCs.
>>>
>>> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com>
>>> ---
>>>  MAINTAINERS                          |   7 +
>>>  drivers/char/hw_random/Kconfig       |  12 ++
>>>  drivers/char/hw_random/Makefile      |   1 +
>>>  drivers/char/hw_random/exynos-trng.c | 256 +++++++++++++++++++++++++++++++++++
>>>  4 files changed, 276 insertions(+)
>>>  create mode 100644 drivers/char/hw_random/exynos-trng.c
>>>
>

+Cc Stephan Müller, who reviewed intensively exynos-rng.c.

> [...]
>
>>>  endif # HW_RANDOM
>>
>> Thanks for working on TRNG.
>>
>> 1. I was rather thinking about extending existing exynos-rng.c [1] so
>> it would be using TRNG as seed for PRNG as this gives you much more
>> random data. Instead you developed totally separate driver which has
>> its own benefits - one can choose which interface he wants. Although
>> it is a little bit duplication.
>
> As far as I can tell, these are two different devices. However, PRNG
> shares hardware with the hash engine. Indeed there is a hardware to
> connect TRNG and PRNG, but, IMHO, it might be hard to model that
> dependency in kernel.

It should be as simple as setting few more registers in SSS module
(actually maybe just enabling TRNG_SEED_START in PRNG). You do not
have to model it in a kernel like connecting some hw_rng entity to
cryptoai's rng_alg. See the jitterentropy-kcapi.c. I understand that
in that case existing exynos-rng.c could expose two different RNG
devices - one PRNG based on user's seed and second TRNG (actually
TRNG+PRNG).

It does not seem difficult to model but the question is whether that
makes sense.


> To me it seems easier to read TRNG (or
> /dev/random) and and write the result to PRNG manually (in software).

Indeed this gives more flexibility to the user (choice of engine) but
first, it is slower, and second it reduces the quality of random
numbers (PRNG reseeds itself... but in this model cannot reseed from
TRNG).

Best regards,
Krzysztof

Re: [PATCH 2/3] hwrng: exynos - add Samsung Exynos True RNG driver

[Stephan Müller]

Am Freitag, 24. November 2017, 13:09:06 CET schrieb Krzysztof Kozlowski:

Hi Krzysztof,
> >> 
> >> 1. I was rather thinking about extending existing exynos-rng.c [1] so
> >> it would be using TRNG as seed for PRNG as this gives you much more
> >> random data. Instead you developed totally separate driver which has
> >> its own benefits - one can choose which interface he wants. Although
> >> it is a little bit duplication.
> > 
> > As far as I can tell, these are two different devices. However, PRNG
> > shares hardware with the hash engine. Indeed there is a hardware to
> > connect TRNG and PRNG, but, IMHO, it might be hard to model that
> > dependency in kernel.
> 
> It should be as simple as setting few more registers in SSS module
> (actually maybe just enabling TRNG_SEED_START in PRNG). You do not
> have to model it in a kernel like connecting some hw_rng entity to
> cryptoai's rng_alg. See the jitterentropy-kcapi.c. I understand that
> in that case existing exynos-rng.c could expose two different RNG
> devices - one PRNG based on user's seed and second TRNG (actually
> TRNG+PRNG).
> 
> It does not seem difficult to model but the question is whether that
> makes sense.

The usage strategy for the PRNGs registered at the kernel crypto API is as 
follows:

1. crypto_rng_alloc

2. crypto_rng_reset

3. crypto_rng_generate

If in step 2 you provide NULL as input, the kernel takes get_random_bytes as 
seed source. Step 2 is the mandatory.

The Linux-RNG can be fed internally from the hw_random framework by the 
function hwrng_fillfn. This function is only used if the current_quality or 
default_quality values in the hw_random framework is set.

For the TRNG, it seems to be not set per default, but could be set as either a 
boot argument or at runtime via /sys.

If that variable is set and the TRNG is registered, it feeds random data into 
the Linux-RNG which in turn is used per default to seed a PRNG. In this case, 
no detour via user space is needed to push data from TRNG to the PRNG. Using 
that mechanism allows you to benefit from additional entropy the Linux-RNG 
collects elsewhere.
> 
> > To me it seems easier to read TRNG (or
> > /dev/random) and and write the result to PRNG manually (in software).
> 
> Indeed this gives more flexibility to the user (choice of engine) but
> first, it is slower, and second it reduces the quality of random
> numbers (PRNG reseeds itself... but in this model cannot reseed from
> TRNG).

Given the reasons above, I would think that keeping the PRMG and TRNG separate 
as offered by the current patch seems reasonable. If configured correctly, the 
TRNG can seed the PRNG at any time (including boot time) without the need of 
user space.
> 
> Best regards,
> Krzysztof


Ciao
Stephan

Re: [PATCH 2/3] hwrng: exynos - add Samsung Exynos True RNG driver

[Krzysztof Kozlowski]

On Fri, Nov 24, 2017 at 2:05 PM, Stephan Müller <smueller-T9tCv8IpfcWELgA04lAiVw@public.gmane.org> wrote:
> Am Freitag, 24. November 2017, 13:09:06 CET schrieb Krzysztof Kozlowski:
>
> Hi Krzysztof,
>> >>
>> >> 1. I was rather thinking about extending existing exynos-rng.c [1] so
>> >> it would be using TRNG as seed for PRNG as this gives you much more
>> >> random data. Instead you developed totally separate driver which has
>> >> its own benefits - one can choose which interface he wants. Although
>> >> it is a little bit duplication.
>> >
>> > As far as I can tell, these are two different devices. However, PRNG
>> > shares hardware with the hash engine. Indeed there is a hardware to
>> > connect TRNG and PRNG, but, IMHO, it might be hard to model that
>> > dependency in kernel.
>>
>> It should be as simple as setting few more registers in SSS module
>> (actually maybe just enabling TRNG_SEED_START in PRNG). You do not
>> have to model it in a kernel like connecting some hw_rng entity to
>> cryptoai's rng_alg. See the jitterentropy-kcapi.c. I understand that
>> in that case existing exynos-rng.c could expose two different RNG
>> devices - one PRNG based on user's seed and second TRNG (actually
>> TRNG+PRNG).
>>
>> It does not seem difficult to model but the question is whether that
>> makes sense.
>
> The usage strategy for the PRNGs registered at the kernel crypto API is as
> follows:
>
> 1. crypto_rng_alloc
>
> 2. crypto_rng_reset
>
> 3. crypto_rng_generate
>
> If in step 2 you provide NULL as input, the kernel takes get_random_bytes as
> seed source. Step 2 is the mandatory.
>
> The Linux-RNG can be fed internally from the hw_random framework by the
> function hwrng_fillfn. This function is only used if the current_quality or
> default_quality values in the hw_random framework is set.
>
> For the TRNG, it seems to be not set per default, but could be set as either a
> boot argument or at runtime via /sys.
>
> If that variable is set and the TRNG is registered, it feeds random data into
> the Linux-RNG which in turn is used per default to seed a PRNG. In this case,
> no detour via user space is needed to push data from TRNG to the PRNG. Using
> that mechanism allows you to benefit from additional entropy the Linux-RNG
> collects elsewhere.
>>
>> > To me it seems easier to read TRNG (or
>> > /dev/random) and and write the result to PRNG manually (in software).
>>
>> Indeed this gives more flexibility to the user (choice of engine) but
>> first, it is slower, and second it reduces the quality of random
>> numbers (PRNG reseeds itself... but in this model cannot reseed from
>> TRNG).
>
> Given the reasons above, I would think that keeping the PRMG and TRNG separate
> as offered by the current patch seems reasonable. If configured correctly, the
> TRNG can seed the PRNG at any time (including boot time) without the need of
> user space.

Hi Stephan,

Thanks for explaining the details. This convinces me so I do not have
any objections against current approach of this second RNG driver for
Exynos.

Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] hwrng: exynos - add Samsung Exynos True RNG driver

[Krzysztof Kozlowski]

On Fri, Nov 24, 2017 at 2:05 PM, Stephan Müller <smueller@chronox.de> wrote:
> Am Freitag, 24. November 2017, 13:09:06 CET schrieb Krzysztof Kozlowski:
>
> Hi Krzysztof,
>> >>
>> >> 1. I was rather thinking about extending existing exynos-rng.c [1] so
>> >> it would be using TRNG as seed for PRNG as this gives you much more
>> >> random data. Instead you developed totally separate driver which has
>> >> its own benefits - one can choose which interface he wants. Although
>> >> it is a little bit duplication.
>> >
>> > As far as I can tell, these are two different devices. However, PRNG
>> > shares hardware with the hash engine. Indeed there is a hardware to
>> > connect TRNG and PRNG, but, IMHO, it might be hard to model that
>> > dependency in kernel.
>>
>> It should be as simple as setting few more registers in SSS module
>> (actually maybe just enabling TRNG_SEED_START in PRNG). You do not
>> have to model it in a kernel like connecting some hw_rng entity to
>> cryptoai's rng_alg. See the jitterentropy-kcapi.c. I understand that
>> in that case existing exynos-rng.c could expose two different RNG
>> devices - one PRNG based on user's seed and second TRNG (actually
>> TRNG+PRNG).
>>
>> It does not seem difficult to model but the question is whether that
>> makes sense.
>
> The usage strategy for the PRNGs registered at the kernel crypto API is as
> follows:
>
> 1. crypto_rng_alloc
>
> 2. crypto_rng_reset
>
> 3. crypto_rng_generate
>
> If in step 2 you provide NULL as input, the kernel takes get_random_bytes as
> seed source. Step 2 is the mandatory.
>
> The Linux-RNG can be fed internally from the hw_random framework by the
> function hwrng_fillfn. This function is only used if the current_quality or
> default_quality values in the hw_random framework is set.
>
> For the TRNG, it seems to be not set per default, but could be set as either a
> boot argument or at runtime via /sys.
>
> If that variable is set and the TRNG is registered, it feeds random data into
> the Linux-RNG which in turn is used per default to seed a PRNG. In this case,
> no detour via user space is needed to push data from TRNG to the PRNG. Using
> that mechanism allows you to benefit from additional entropy the Linux-RNG
> collects elsewhere.
>>
>> > To me it seems easier to read TRNG (or
>> > /dev/random) and and write the result to PRNG manually (in software).
>>
>> Indeed this gives more flexibility to the user (choice of engine) but
>> first, it is slower, and second it reduces the quality of random
>> numbers (PRNG reseeds itself... but in this model cannot reseed from
>> TRNG).
>
> Given the reasons above, I would think that keeping the PRMG and TRNG separate
> as offered by the current patch seems reasonable. If configured correctly, the
> TRNG can seed the PRNG at any time (including boot time) without the need of
> user space.

Hi Stephan,

Thanks for explaining the details. This convinces me so I do not have
any objections against current approach of this second RNG driver for
Exynos.

Best regards,
Krzysztof

Re: [PATCH 2/3] hwrng: exynos - add Samsung Exynos True RNG driver

[PrasannaKumar Muralidharan]

Hi Lukasz,

Some minor comments below.

On 23 November 2017 at 20:39, Łukasz Stelmach <l.stelmach@samsung.com> wrote:
> Add support for True Random Number Generator found in Samsung Exynos
> 5250+ SoCs.
>
> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com>
> ---
>  MAINTAINERS                          |   7 +
>  drivers/char/hw_random/Kconfig       |  12 ++
>  drivers/char/hw_random/Makefile      |   1 +
>  drivers/char/hw_random/exynos-trng.c | 256 +++++++++++++++++++++++++++++++++++
>  4 files changed, 276 insertions(+)
>  create mode 100644 drivers/char/hw_random/exynos-trng.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2811a211632c..992074cca612 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11780,6 +11780,13 @@ S:     Maintained
>  F:     drivers/crypto/exynos-rng.c
>  F:     Documentation/devicetree/bindings/rng/samsung,exynos-rng4.txt
>
> +SAMSUNG EXYNOS TRUE RANDOM NUMBER GENERATOR (TRNG) DRIVER
> +M:     Łukasz Stelmach <l.stelmach@samsung.com>
> +L:     linux-samsung-soc@vger.kernel.org
> +S:     Maintained
> +F:     drivers/char/hw_random/exynos-trng.c
> +F:     Documentation/devicetree/bindings/rng/samsung,exynos5250-trng.txt
> +
>  SAMSUNG FRAMEBUFFER DRIVER
>  M:     Jingoo Han <jingoohan1@gmail.com>
>  L:     linux-fbdev@vger.kernel.org
> diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
> index 95a031e9eced..a788ac07081b 100644
> --- a/drivers/char/hw_random/Kconfig
> +++ b/drivers/char/hw_random/Kconfig
> @@ -449,6 +449,18 @@ config HW_RANDOM_S390
>
>           If unsure, say Y.
>
> +config HW_RANDOM_EXYNOS
> +       tristate "Samsung Exynos True Random Number Generator support"
> +       depends on ARCH_EXYNOS || COMPILE_TEST
> +       default HW_RANDOM
> +       ---help---
> +         This driver provides support for the True Random Number
> +         Generator available in Exynos SoCs.
> +
> +        To compile this driver as a module, choose M here: the module
> +        will be called exynos-trng.
> +
> +        If unsure, say Y.
>  endif # HW_RANDOM
>
>  config UML_RANDOM
> diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
> index f3728d008fff..5595df97da7a 100644
> --- a/drivers/char/hw_random/Makefile
> +++ b/drivers/char/hw_random/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_HW_RANDOM_GEODE) += geode-rng.o
>  obj-$(CONFIG_HW_RANDOM_N2RNG) += n2-rng.o
>  n2-rng-y := n2-drv.o n2-asm.o
>  obj-$(CONFIG_HW_RANDOM_VIA) += via-rng.o
> +obj-$(CONFIG_HW_RANDOM_EXYNOS) += exynos-trng.o
>  obj-$(CONFIG_HW_RANDOM_IXP4XX) += ixp4xx-rng.o
>  obj-$(CONFIG_HW_RANDOM_OMAP) += omap-rng.o
>  obj-$(CONFIG_HW_RANDOM_OMAP3_ROM) += omap3-rom-rng.o
> diff --git a/drivers/char/hw_random/exynos-trng.c b/drivers/char/hw_random/exynos-trng.c
> new file mode 100644
> index 000000000000..340e106cae24
> --- /dev/null
> +++ b/drivers/char/hw_random/exynos-trng.c
> @@ -0,0 +1,256 @@
> +/*
> + * RNG driver for Exynos TRNGs
> + *
> + * Author: Łukasz Stelmach <l.stelmach@samsung.com>
> + *
> + * Copyright 2017 (c) Samsung Electronics Software, Inc.
> + *
> + * Based on the Exynos PRNG driver drivers/crypto/exynos-rng by
> + * Krzysztof Kozłowski <krzk@kernel.org>
> + *
> + * This program 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;
> + *
> + * This program 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/crypto.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/hw_random.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +
> +#define EXYNOS_TRNG_CLKDIV         (0x0)
> +#define EXYNOS_TRNG_CTRL           (0x20)
> +#define EXYNOS_TRNG_POST_CTRL      (0x30)
> +#define EXYNOS_TRNG_ONLINE_CTRL    (0x40)
> +#define EXYNOS_TRNG_ONLINE_STAT    (0x44)
> +#define EXYNOS_TRNG_ONLINE_MAXCHI2 (0x48)
> +#define EXYNOS_TRNG_FIFO_CTRL      (0x50)
> +#define EXYNOS_TRNG_FIFO_0         (0x80)
> +#define EXYNOS_TRNG_FIFO_1         (0x84)
> +#define EXYNOS_TRNG_FIFO_2         (0x88)
> +#define EXYNOS_TRNG_FIFO_3         (0x8c)
> +#define EXYNOS_TRNG_FIFO_4         (0x90)
> +#define EXYNOS_TRNG_FIFO_5         (0x94)
> +#define EXYNOS_TRNG_FIFO_6         (0x98)
> +#define EXYNOS_TRNG_FIFO_7         (0x9c)
> +#define EXYNOS_TRNG_FIFO_LEN       (8)
> +#define EXYNOS_TRNG_CLOCK_RATE     (500000)
> +
> +struct exynos_trng_dev {
> +       struct device    *dev;
> +       void __iomem     *mem;
> +       struct clk       *clk;
> +       struct hwrng rng;
> +};
> +
> +struct exynos_trng_dev *exynos_trng_dev;
> +
> +static inline void exynos_trng_set_reg(struct exynos_trng_dev *trng, u16 reg,
> +                                      u32 val)
> +{
> +       /* Check range of reg? */
> +       __raw_writel(val, trng->mem + reg);

Any specific reason to use __raw_writel? Why not just writel?

> +}
> +
> +static inline u32 exynos_trng_get_reg(struct exynos_trng_dev *trng, u16 reg)
> +{
> +       /* Check range of reg? */
> +       return __raw_readl(trng->mem + reg);

Same as above.

> +}
> +
> +static int exynos_trng_do_read(struct hwrng *rng, void *data, size_t max,
> +                              bool wait)
> +{
> +       struct exynos_trng_dev *trng;
> +       u32 val;
> +
> +       max = max > (EXYNOS_TRNG_FIFO_LEN * 4) ?
> +               (EXYNOS_TRNG_FIFO_LEN * 4) : max;

max is always > 32. This condition is not required.

> +
> +       trng = (struct exynos_trng_dev *)rng->priv;
> +
> +       exynos_trng_set_reg(trng, EXYNOS_TRNG_FIFO_CTRL, max * 8);
> +       val = readl_poll_timeout(trng->mem + EXYNOS_TRNG_FIFO_CTRL, val,
> +                                val == 0, 200, 1000000);
> +       if (val < 0)
> +               return val;
> +
> +       memcpy_fromio(data, trng->mem + EXYNOS_TRNG_FIFO_0, max);
> +
> +       return max;
> +}
> +
> +static int exynos_trng_init(struct hwrng *rng)
> +{
> +       struct exynos_trng_dev *trng;
> +       unsigned long sss_rate;
> +       u32 val;
> +
> +       trng = (struct exynos_trng_dev *)rng->priv;
> +       sss_rate = clk_get_rate(trng->clk);
> +
> +       /* For most TRNG circuits the clock frequency of under 500 kHz
> +        * is safe.
> +        */
> +       val = sss_rate / (EXYNOS_TRNG_CLOCK_RATE * 2);
> +       if (val > 0x7fff) {
> +               dev_err(trng->dev, "clock divider too large: %d", val);
> +               return -ERANGE;
> +       }
> +       val = val << 1;
> +       exynos_trng_set_reg(trng, EXYNOS_TRNG_CLKDIV, val);
> +
> +       /* Enable the generator. */
> +       val = 1 << 31;
> +       exynos_trng_set_reg(trng, EXYNOS_TRNG_CTRL, val);
> +
> +       /* Disable post processing. /dev/hwrng is supposed to deliver
> +        * unprocessed data.
> +        */
> +       exynos_trng_set_reg(trng, EXYNOS_TRNG_POST_CTRL, 0);
> +
> +       return 0;
> +}
> +
> +static int exynos_trng_probe(struct platform_device *pdev)
> +{
> +       struct exynos_trng_dev *trng;
> +       struct resource *res;
> +       int ret = -ENOMEM;
> +
> +       if (exynos_trng_dev)
> +               return -EEXIST;
> +
> +       trng = devm_kzalloc(&pdev->dev, sizeof(*trng), GFP_KERNEL);
> +       if (!trng)
> +               goto err_ioremap;
> +
> +       trng->rng.name = devm_kstrdup(&pdev->dev, dev_name(&pdev->dev),
> +                                     GFP_KERNEL);
> +       if (!trng->rng.name)
> +               goto err_ioremap;
> +
> +       trng->rng.init = exynos_trng_init;
> +       trng->rng.read = exynos_trng_do_read;
> +       trng->rng.priv = (unsigned long) trng;
> +
> +       platform_set_drvdata(pdev, trng);
> +       trng->dev = &pdev->dev;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       trng->mem = devm_ioremap_resource(&pdev->dev, res);
> +       if (IS_ERR(trng->mem)) {
> +               dev_err(&pdev->dev, "Couldn't map IO resources.\n");
> +               ret = PTR_ERR(trng->mem);
> +               goto err_ioremap;
> +       }
> +
> +       pm_runtime_enable(&pdev->dev);
> +       ret = pm_runtime_get_sync(&pdev->dev);
> +       if (ret < 0) {
> +               dev_err(&pdev->dev, "pm_runtime_get_sync() failed: %d.\n", ret);
> +               goto err_ioremap;
> +       }
> +
> +       trng->clk = devm_clk_get(&pdev->dev, "secss");
> +       if (IS_ERR(trng->clk)) {
> +               /* XXX: EPROBE_DEFER ? */
> +               dev_err(&pdev->dev, "Couldn't get clock.\n");
> +               ret = PTR_ERR(trng->clk);
> +               goto err_clock;
> +       }
> +
> +       ret = clk_prepare_enable(trng->clk);
> +       if (ret) {
> +               dev_err(&pdev->dev, "Unable to enable the clk: %d.\n", ret);
> +               goto err_clock;
> +       }

Why not move clk_prepare_enable to init call? While at it please add a
cleanup callback as well.

> +
> +       ret = hwrng_register(&trng->rng);
> +       if (ret) {
> +               dev_err(&pdev->dev, "Couldn't register hwrng device.\n");
> +               goto err_register;
> +       }
> +
> +       dev_info(&pdev->dev, "Exynos True Random Number Generator.\n");
> +
> +       return 0;
> +
> +err_register:
> +       clk_disable_unprepare(trng->clk);
> +
> +err_clock:
> +       trng->mem = NULL;
> +       pm_runtime_put_sync(&pdev->dev);
> +       pm_runtime_disable(&pdev->dev);
> +
> +err_ioremap:
> +       dev_err(&pdev->dev, "Initialisation failed.\n");
> +       return ret;
> +}
> +
> +static int exynos_trng_remove(struct platform_device *pdev)
> +{
> +       exynos_trng_dev = NULL;
> +
> +       return 0;
> +}

remove is not required as it does not do anything significant.

> +
> +static int __maybe_unused exynos_trng_suspend(struct device *dev)
> +{
> +       pm_runtime_put_sync(dev);
> +
> +       return 0;
> +}
> +
> +static int __maybe_unused exynos_trng_resume(struct device *dev)
> +{
> +       int ret;
> +
> +       ret = pm_runtime_get_sync(dev);
> +       if (ret < 0) {
> +               dev_err(dev, "pm_runtime_get_sync() failed: %d.\n", ret);
> +               pm_runtime_put_noidle(dev);
> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(exynos_trng_pm_ops, exynos_trng_suspend,
> +                        exynos_trng_resume);
> +
> +static const struct of_device_id exynos_trng_dt_match[] = {
> +       {
> +               .compatible = "samsung,exynos5250-trng",
> +       },
> +       { },
> +};
> +MODULE_DEVICE_TABLE(of, exynos_rng_dt_match);
> +
> +static struct platform_driver exynos_trng_driver = {
> +       .driver = {
> +               .name = "exynos-trng",
> +               .pm = &exynos_trng_pm_ops,
> +               .of_match_table = exynos_trng_dt_match,
> +       },
> +       .probe = exynos_trng_probe,
> +       .remove = exynos_trng_remove,
> +};
> +
> +module_platform_driver(exynos_trng_driver);
> +MODULE_AUTHOR("Łukasz Stelmach");
> +MODULE_DESCRIPTION("H/W TRNG driver for Exynos chips");
> +MODULE_LICENSE("GPL");
> --
> 2.11.0
>

Regards,
PrasannaKumar

Re: [PATCH 2/3] hwrng: exynos - add Samsung Exynos True RNG driver

[PrasannaKumar Muralidharan]

Hi Lukasz,

Some minor comments below.

On 23 November 2017 at 20:39, Łukasz Stelmach <l.stelmach@samsung.com> wrote:
> Add support for True Random Number Generator found in Samsung Exynos
> 5250+ SoCs.
>
> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com>
> ---
>  MAINTAINERS                          |   7 +
>  drivers/char/hw_random/Kconfig       |  12 ++
>  drivers/char/hw_random/Makefile      |   1 +
>  drivers/char/hw_random/exynos-trng.c | 256 +++++++++++++++++++++++++++++++++++
>  4 files changed, 276 insertions(+)
>  create mode 100644 drivers/char/hw_random/exynos-trng.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2811a211632c..992074cca612 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11780,6 +11780,13 @@ S:     Maintained
>  F:     drivers/crypto/exynos-rng.c
>  F:     Documentation/devicetree/bindings/rng/samsung,exynos-rng4.txt
>
> +SAMSUNG EXYNOS TRUE RANDOM NUMBER GENERATOR (TRNG) DRIVER
> +M:     Łukasz Stelmach <l.stelmach@samsung.com>
> +L:     linux-samsung-soc@vger.kernel.org
> +S:     Maintained
> +F:     drivers/char/hw_random/exynos-trng.c
> +F:     Documentation/devicetree/bindings/rng/samsung,exynos5250-trng.txt
> +
>  SAMSUNG FRAMEBUFFER DRIVER
>  M:     Jingoo Han <jingoohan1@gmail.com>
>  L:     linux-fbdev@vger.kernel.org
> diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
> index 95a031e9eced..a788ac07081b 100644
> --- a/drivers/char/hw_random/Kconfig
> +++ b/drivers/char/hw_random/Kconfig
> @@ -449,6 +449,18 @@ config HW_RANDOM_S390
>
>           If unsure, say Y.
>
> +config HW_RANDOM_EXYNOS
> +       tristate "Samsung Exynos True Random Number Generator support"
> +       depends on ARCH_EXYNOS || COMPILE_TEST
> +       default HW_RANDOM
> +       ---help---
> +         This driver provides support for the True Random Number
> +         Generator available in Exynos SoCs.
> +
> +        To compile this driver as a module, choose M here: the module
> +        will be called exynos-trng.
> +
> +        If unsure, say Y.
>  endif # HW_RANDOM
>
>  config UML_RANDOM
> diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
> index f3728d008fff..5595df97da7a 100644
> --- a/drivers/char/hw_random/Makefile
> +++ b/drivers/char/hw_random/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_HW_RANDOM_GEODE) += geode-rng.o
>  obj-$(CONFIG_HW_RANDOM_N2RNG) += n2-rng.o
>  n2-rng-y := n2-drv.o n2-asm.o
>  obj-$(CONFIG_HW_RANDOM_VIA) += via-rng.o
> +obj-$(CONFIG_HW_RANDOM_EXYNOS) += exynos-trng.o
>  obj-$(CONFIG_HW_RANDOM_IXP4XX) += ixp4xx-rng.o
>  obj-$(CONFIG_HW_RANDOM_OMAP) += omap-rng.o
>  obj-$(CONFIG_HW_RANDOM_OMAP3_ROM) += omap3-rom-rng.o
> diff --git a/drivers/char/hw_random/exynos-trng.c b/drivers/char/hw_random/exynos-trng.c
> new file mode 100644
> index 000000000000..340e106cae24
> --- /dev/null
> +++ b/drivers/char/hw_random/exynos-trng.c
> @@ -0,0 +1,256 @@
> +/*
> + * RNG driver for Exynos TRNGs
> + *
> + * Author: Łukasz Stelmach <l.stelmach@samsung.com>
> + *
> + * Copyright 2017 (c) Samsung Electronics Software, Inc.
> + *
> + * Based on the Exynos PRNG driver drivers/crypto/exynos-rng by
> + * Krzysztof Kozłowski <krzk@kernel.org>
> + *
> + * This program 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;
> + *
> + * This program 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/crypto.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/hw_random.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +
> +#define EXYNOS_TRNG_CLKDIV         (0x0)
> +#define EXYNOS_TRNG_CTRL           (0x20)
> +#define EXYNOS_TRNG_POST_CTRL      (0x30)
> +#define EXYNOS_TRNG_ONLINE_CTRL    (0x40)
> +#define EXYNOS_TRNG_ONLINE_STAT    (0x44)
> +#define EXYNOS_TRNG_ONLINE_MAXCHI2 (0x48)
> +#define EXYNOS_TRNG_FIFO_CTRL      (0x50)
> +#define EXYNOS_TRNG_FIFO_0         (0x80)
> +#define EXYNOS_TRNG_FIFO_1         (0x84)
> +#define EXYNOS_TRNG_FIFO_2         (0x88)
> +#define EXYNOS_TRNG_FIFO_3         (0x8c)
> +#define EXYNOS_TRNG_FIFO_4         (0x90)
> +#define EXYNOS_TRNG_FIFO_5         (0x94)
> +#define EXYNOS_TRNG_FIFO_6         (0x98)
> +#define EXYNOS_TRNG_FIFO_7         (0x9c)
> +#define EXYNOS_TRNG_FIFO_LEN       (8)
> +#define EXYNOS_TRNG_CLOCK_RATE     (500000)
> +
> +struct exynos_trng_dev {
> +       struct device    *dev;
> +       void __iomem     *mem;
> +       struct clk       *clk;
> +       struct hwrng rng;
> +};
> +
> +struct exynos_trng_dev *exynos_trng_dev;
> +
> +static inline void exynos_trng_set_reg(struct exynos_trng_dev *trng, u16 reg,
> +                                      u32 val)
> +{
> +       /* Check range of reg? */
> +       __raw_writel(val, trng->mem + reg);

Any specific reason to use __raw_writel? Why not just writel?

> +}
> +
> +static inline u32 exynos_trng_get_reg(struct exynos_trng_dev *trng, u16 reg)
> +{
> +       /* Check range of reg? */
> +       return __raw_readl(trng->mem + reg);

Same as above.

> +}
> +
> +static int exynos_trng_do_read(struct hwrng *rng, void *data, size_t max,
> +                              bool wait)
> +{
> +       struct exynos_trng_dev *trng;
> +       u32 val;
> +
> +       max = max > (EXYNOS_TRNG_FIFO_LEN * 4) ?
> +               (EXYNOS_TRNG_FIFO_LEN * 4) : max;

max is always > 32. This condition is not required.

> +
> +       trng = (struct exynos_trng_dev *)rng->priv;
> +
> +       exynos_trng_set_reg(trng, EXYNOS_TRNG_FIFO_CTRL, max * 8);
> +       val = readl_poll_timeout(trng->mem + EXYNOS_TRNG_FIFO_CTRL, val,
> +                                val == 0, 200, 1000000);
> +       if (val < 0)
> +               return val;
> +
> +       memcpy_fromio(data, trng->mem + EXYNOS_TRNG_FIFO_0, max);
> +
> +       return max;
> +}
> +
> +static int exynos_trng_init(struct hwrng *rng)
> +{
> +       struct exynos_trng_dev *trng;
> +       unsigned long sss_rate;
> +       u32 val;
> +
> +       trng = (struct exynos_trng_dev *)rng->priv;
> +       sss_rate = clk_get_rate(trng->clk);
> +
> +       /* For most TRNG circuits the clock frequency of under 500 kHz
> +        * is safe.
> +        */
> +       val = sss_rate / (EXYNOS_TRNG_CLOCK_RATE * 2);
> +       if (val > 0x7fff) {
> +               dev_err(trng->dev, "clock divider too large: %d", val);
> +               return -ERANGE;
> +       }
> +       val = val << 1;
> +       exynos_trng_set_reg(trng, EXYNOS_TRNG_CLKDIV, val);
> +
> +       /* Enable the generator. */
> +       val = 1 << 31;
> +       exynos_trng_set_reg(trng, EXYNOS_TRNG_CTRL, val);
> +
> +       /* Disable post processing. /dev/hwrng is supposed to deliver
> +        * unprocessed data.
> +        */
> +       exynos_trng_set_reg(trng, EXYNOS_TRNG_POST_CTRL, 0);
> +
> +       return 0;
> +}
> +
> +static int exynos_trng_probe(struct platform_device *pdev)
> +{
> +       struct exynos_trng_dev *trng;
> +       struct resource *res;
> +       int ret = -ENOMEM;
> +
> +       if (exynos_trng_dev)
> +               return -EEXIST;
> +
> +       trng = devm_kzalloc(&pdev->dev, sizeof(*trng), GFP_KERNEL);
> +       if (!trng)
> +               goto err_ioremap;
> +
> +       trng->rng.name = devm_kstrdup(&pdev->dev, dev_name(&pdev->dev),
> +                                     GFP_KERNEL);
> +       if (!trng->rng.name)
> +               goto err_ioremap;
> +
> +       trng->rng.init = exynos_trng_init;
> +       trng->rng.read = exynos_trng_do_read;
> +       trng->rng.priv = (unsigned long) trng;
> +
> +       platform_set_drvdata(pdev, trng);
> +       trng->dev = &pdev->dev;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       trng->mem = devm_ioremap_resource(&pdev->dev, res);
> +       if (IS_ERR(trng->mem)) {
> +               dev_err(&pdev->dev, "Couldn't map IO resources.\n");
> +               ret = PTR_ERR(trng->mem);
> +               goto err_ioremap;
> +       }
> +
> +       pm_runtime_enable(&pdev->dev);
> +       ret = pm_runtime_get_sync(&pdev->dev);
> +       if (ret < 0) {
> +               dev_err(&pdev->dev, "pm_runtime_get_sync() failed: %d.\n", ret);
> +               goto err_ioremap;
> +       }
> +
> +       trng->clk = devm_clk_get(&pdev->dev, "secss");
> +       if (IS_ERR(trng->clk)) {
> +               /* XXX: EPROBE_DEFER ? */
> +               dev_err(&pdev->dev, "Couldn't get clock.\n");
> +               ret = PTR_ERR(trng->clk);
> +               goto err_clock;
> +       }
> +
> +       ret = clk_prepare_enable(trng->clk);
> +       if (ret) {
> +               dev_err(&pdev->dev, "Unable to enable the clk: %d.\n", ret);
> +               goto err_clock;
> +       }

Why not move clk_prepare_enable to init call? While at it please add a
cleanup callback as well.

> +
> +       ret = hwrng_register(&trng->rng);
> +       if (ret) {
> +               dev_err(&pdev->dev, "Couldn't register hwrng device.\n");
> +               goto err_register;
> +       }
> +
> +       dev_info(&pdev->dev, "Exynos True Random Number Generator.\n");
> +
> +       return 0;
> +
> +err_register:
> +       clk_disable_unprepare(trng->clk);
> +
> +err_clock:
> +       trng->mem = NULL;
> +       pm_runtime_put_sync(&pdev->dev);
> +       pm_runtime_disable(&pdev->dev);
> +
> +err_ioremap:
> +       dev_err(&pdev->dev, "Initialisation failed.\n");
> +       return ret;
> +}
> +
> +static int exynos_trng_remove(struct platform_device *pdev)
> +{
> +       exynos_trng_dev = NULL;
> +
> +       return 0;
> +}

remove is not required as it does not do anything significant.

> +
> +static int __maybe_unused exynos_trng_suspend(struct device *dev)
> +{
> +       pm_runtime_put_sync(dev);
> +
> +       return 0;
> +}
> +
> +static int __maybe_unused exynos_trng_resume(struct device *dev)
> +{
> +       int ret;
> +
> +       ret = pm_runtime_get_sync(dev);
> +       if (ret < 0) {
> +               dev_err(dev, "pm_runtime_get_sync() failed: %d.\n", ret);
> +               pm_runtime_put_noidle(dev);
> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(exynos_trng_pm_ops, exynos_trng_suspend,
> +                        exynos_trng_resume);
> +
> +static const struct of_device_id exynos_trng_dt_match[] = {
> +       {
> +               .compatible = "samsung,exynos5250-trng",
> +       },
> +       { },
> +};
> +MODULE_DEVICE_TABLE(of, exynos_rng_dt_match);
> +
> +static struct platform_driver exynos_trng_driver = {
> +       .driver = {
> +               .name = "exynos-trng",
> +               .pm = &exynos_trng_pm_ops,
> +               .of_match_table = exynos_trng_dt_match,
> +       },
> +       .probe = exynos_trng_probe,
> +       .remove = exynos_trng_remove,
> +};
> +
> +module_platform_driver(exynos_trng_driver);
> +MODULE_AUTHOR("Łukasz Stelmach");
> +MODULE_DESCRIPTION("H/W TRNG driver for Exynos chips");
> +MODULE_LICENSE("GPL");
> --
> 2.11.0
>

Regards,
PrasannaKumar

Re: [PATCH 2/3] hwrng: exynos - add Samsung Exynos True RNG driver

[PrasannaKumar Muralidharan]

On 24 November 2017 at 20:55, PrasannaKumar Muralidharan
<prasannatsmkumar@gmail.com> wrote:
> Hi Lukasz,
>
> Some minor comments below.
>
> On 23 November 2017 at 20:39, Łukasz Stelmach <l.stelmach@samsung.com> wrote:
>> Add support for True Random Number Generator found in Samsung Exynos
>> 5250+ SoCs.
>>
>> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com>
>> ---
>>  MAINTAINERS                          |   7 +
>>  drivers/char/hw_random/Kconfig       |  12 ++
>>  drivers/char/hw_random/Makefile      |   1 +
>>  drivers/char/hw_random/exynos-trng.c | 256 +++++++++++++++++++++++++++++++++++
>>  4 files changed, 276 insertions(+)
>>  create mode 100644 drivers/char/hw_random/exynos-trng.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 2811a211632c..992074cca612 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -11780,6 +11780,13 @@ S:     Maintained
>>  F:     drivers/crypto/exynos-rng.c
>>  F:     Documentation/devicetree/bindings/rng/samsung,exynos-rng4.txt
>>
>> +SAMSUNG EXYNOS TRUE RANDOM NUMBER GENERATOR (TRNG) DRIVER
>> +M:     Łukasz Stelmach <l.stelmach@samsung.com>
>> +L:     linux-samsung-soc@vger.kernel.org
>> +S:     Maintained
>> +F:     drivers/char/hw_random/exynos-trng.c
>> +F:     Documentation/devicetree/bindings/rng/samsung,exynos5250-trng.txt
>> +
>>  SAMSUNG FRAMEBUFFER DRIVER
>>  M:     Jingoo Han <jingoohan1@gmail.com>
>>  L:     linux-fbdev@vger.kernel.org
>> diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
>> index 95a031e9eced..a788ac07081b 100644
>> --- a/drivers/char/hw_random/Kconfig
>> +++ b/drivers/char/hw_random/Kconfig
>> @@ -449,6 +449,18 @@ config HW_RANDOM_S390
>>
>>           If unsure, say Y.
>>
>> +config HW_RANDOM_EXYNOS
>> +       tristate "Samsung Exynos True Random Number Generator support"
>> +       depends on ARCH_EXYNOS || COMPILE_TEST
>> +       default HW_RANDOM
>> +       ---help---
>> +         This driver provides support for the True Random Number
>> +         Generator available in Exynos SoCs.
>> +
>> +        To compile this driver as a module, choose M here: the module
>> +        will be called exynos-trng.
>> +
>> +        If unsure, say Y.
>>  endif # HW_RANDOM
>>
>>  config UML_RANDOM
>> diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
>> index f3728d008fff..5595df97da7a 100644
>> --- a/drivers/char/hw_random/Makefile
>> +++ b/drivers/char/hw_random/Makefile
>> @@ -14,6 +14,7 @@ obj-$(CONFIG_HW_RANDOM_GEODE) += geode-rng.o
>>  obj-$(CONFIG_HW_RANDOM_N2RNG) += n2-rng.o
>>  n2-rng-y := n2-drv.o n2-asm.o
>>  obj-$(CONFIG_HW_RANDOM_VIA) += via-rng.o
>> +obj-$(CONFIG_HW_RANDOM_EXYNOS) += exynos-trng.o
>>  obj-$(CONFIG_HW_RANDOM_IXP4XX) += ixp4xx-rng.o
>>  obj-$(CONFIG_HW_RANDOM_OMAP) += omap-rng.o
>>  obj-$(CONFIG_HW_RANDOM_OMAP3_ROM) += omap3-rom-rng.o
>> diff --git a/drivers/char/hw_random/exynos-trng.c b/drivers/char/hw_random/exynos-trng.c
>> new file mode 100644
>> index 000000000000..340e106cae24
>> --- /dev/null
>> +++ b/drivers/char/hw_random/exynos-trng.c
>> @@ -0,0 +1,256 @@
>> +/*
>> + * RNG driver for Exynos TRNGs
>> + *
>> + * Author: Łukasz Stelmach <l.stelmach@samsung.com>
>> + *
>> + * Copyright 2017 (c) Samsung Electronics Software, Inc.
>> + *
>> + * Based on the Exynos PRNG driver drivers/crypto/exynos-rng by
>> + * Krzysztof Kozłowski <krzk@kernel.org>
>> + *
>> + * This program 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;
>> + *
>> + * This program 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/crypto.h>
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/hw_random.h>
>> +#include <linux/io.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pm_runtime.h>
>> +
>> +#define EXYNOS_TRNG_CLKDIV         (0x0)
>> +#define EXYNOS_TRNG_CTRL           (0x20)
>> +#define EXYNOS_TRNG_POST_CTRL      (0x30)
>> +#define EXYNOS_TRNG_ONLINE_CTRL    (0x40)
>> +#define EXYNOS_TRNG_ONLINE_STAT    (0x44)
>> +#define EXYNOS_TRNG_ONLINE_MAXCHI2 (0x48)
>> +#define EXYNOS_TRNG_FIFO_CTRL      (0x50)
>> +#define EXYNOS_TRNG_FIFO_0         (0x80)
>> +#define EXYNOS_TRNG_FIFO_1         (0x84)
>> +#define EXYNOS_TRNG_FIFO_2         (0x88)
>> +#define EXYNOS_TRNG_FIFO_3         (0x8c)
>> +#define EXYNOS_TRNG_FIFO_4         (0x90)
>> +#define EXYNOS_TRNG_FIFO_5         (0x94)
>> +#define EXYNOS_TRNG_FIFO_6         (0x98)
>> +#define EXYNOS_TRNG_FIFO_7         (0x9c)
>> +#define EXYNOS_TRNG_FIFO_LEN       (8)
>> +#define EXYNOS_TRNG_CLOCK_RATE     (500000)
>> +
>> +struct exynos_trng_dev {
>> +       struct device    *dev;
>> +       void __iomem     *mem;
>> +       struct clk       *clk;
>> +       struct hwrng rng;
>> +};
>> +
>> +struct exynos_trng_dev *exynos_trng_dev;
>> +
>> +static inline void exynos_trng_set_reg(struct exynos_trng_dev *trng, u16 reg,
>> +                                      u32 val)
>> +{
>> +       /* Check range of reg? */
>> +       __raw_writel(val, trng->mem + reg);
>
> Any specific reason to use __raw_writel? Why not just writel?
>
>> +}
>> +
>> +static inline u32 exynos_trng_get_reg(struct exynos_trng_dev *trng, u16 reg)
>> +{
>> +       /* Check range of reg? */
>> +       return __raw_readl(trng->mem + reg);
>
> Same as above.
>
>> +}
>> +
>> +static int exynos_trng_do_read(struct hwrng *rng, void *data, size_t max,
>> +                              bool wait)
>> +{
>> +       struct exynos_trng_dev *trng;
>> +       u32 val;
>> +
>> +       max = max > (EXYNOS_TRNG_FIFO_LEN * 4) ?
>> +               (EXYNOS_TRNG_FIFO_LEN * 4) : max;
>
> max is always > 32. This condition is not required.
>
>> +
>> +       trng = (struct exynos_trng_dev *)rng->priv;
>> +
>> +       exynos_trng_set_reg(trng, EXYNOS_TRNG_FIFO_CTRL, max * 8);
>> +       val = readl_poll_timeout(trng->mem + EXYNOS_TRNG_FIFO_CTRL, val,
>> +                                val == 0, 200, 1000000);
>> +       if (val < 0)
>> +               return val;
>> +
>> +       memcpy_fromio(data, trng->mem + EXYNOS_TRNG_FIFO_0, max);
>> +
>> +       return max;
>> +}
>> +
>> +static int exynos_trng_init(struct hwrng *rng)
>> +{
>> +       struct exynos_trng_dev *trng;
>> +       unsigned long sss_rate;
>> +       u32 val;
>> +
>> +       trng = (struct exynos_trng_dev *)rng->priv;
>> +       sss_rate = clk_get_rate(trng->clk);
>> +
>> +       /* For most TRNG circuits the clock frequency of under 500 kHz
>> +        * is safe.
>> +        */
>> +       val = sss_rate / (EXYNOS_TRNG_CLOCK_RATE * 2);
>> +       if (val > 0x7fff) {
>> +               dev_err(trng->dev, "clock divider too large: %d", val);
>> +               return -ERANGE;
>> +       }
>> +       val = val << 1;
>> +       exynos_trng_set_reg(trng, EXYNOS_TRNG_CLKDIV, val);
>> +
>> +       /* Enable the generator. */
>> +       val = 1 << 31;
>> +       exynos_trng_set_reg(trng, EXYNOS_TRNG_CTRL, val);
>> +
>> +       /* Disable post processing. /dev/hwrng is supposed to deliver
>> +        * unprocessed data.
>> +        */
>> +       exynos_trng_set_reg(trng, EXYNOS_TRNG_POST_CTRL, 0);
>> +
>> +       return 0;
>> +}
>> +
>> +static int exynos_trng_probe(struct platform_device *pdev)
>> +{
>> +       struct exynos_trng_dev *trng;
>> +       struct resource *res;
>> +       int ret = -ENOMEM;
>> +
>> +       if (exynos_trng_dev)
>> +               return -EEXIST;
>> +
>> +       trng = devm_kzalloc(&pdev->dev, sizeof(*trng), GFP_KERNEL);
>> +       if (!trng)
>> +               goto err_ioremap;
>> +
>> +       trng->rng.name = devm_kstrdup(&pdev->dev, dev_name(&pdev->dev),
>> +                                     GFP_KERNEL);
>> +       if (!trng->rng.name)
>> +               goto err_ioremap;
>> +
>> +       trng->rng.init = exynos_trng_init;
>> +       trng->rng.read = exynos_trng_do_read;
>> +       trng->rng.priv = (unsigned long) trng;
>> +
>> +       platform_set_drvdata(pdev, trng);
>> +       trng->dev = &pdev->dev;
>> +
>> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +       trng->mem = devm_ioremap_resource(&pdev->dev, res);
>> +       if (IS_ERR(trng->mem)) {
>> +               dev_err(&pdev->dev, "Couldn't map IO resources.\n");
>> +               ret = PTR_ERR(trng->mem);
>> +               goto err_ioremap;
>> +       }
>> +
>> +       pm_runtime_enable(&pdev->dev);
>> +       ret = pm_runtime_get_sync(&pdev->dev);
>> +       if (ret < 0) {
>> +               dev_err(&pdev->dev, "pm_runtime_get_sync() failed: %d.\n", ret);
>> +               goto err_ioremap;
>> +       }
>> +
>> +       trng->clk = devm_clk_get(&pdev->dev, "secss");
>> +       if (IS_ERR(trng->clk)) {
>> +               /* XXX: EPROBE_DEFER ? */
>> +               dev_err(&pdev->dev, "Couldn't get clock.\n");
>> +               ret = PTR_ERR(trng->clk);
>> +               goto err_clock;
>> +       }
>> +
>> +       ret = clk_prepare_enable(trng->clk);
>> +       if (ret) {
>> +               dev_err(&pdev->dev, "Unable to enable the clk: %d.\n", ret);
>> +               goto err_clock;
>> +       }
>
> Why not move clk_prepare_enable to init call? While at it please add a
> cleanup callback as well.
>
>> +
>> +       ret = hwrng_register(&trng->rng);
>> +       if (ret) {
>> +               dev_err(&pdev->dev, "Couldn't register hwrng device.\n");
>> +               goto err_register;
>> +       }
>> +
>> +       dev_info(&pdev->dev, "Exynos True Random Number Generator.\n");
>> +
>> +       return 0;
>> +
>> +err_register:
>> +       clk_disable_unprepare(trng->clk);
>> +
>> +err_clock:
>> +       trng->mem = NULL;
>> +       pm_runtime_put_sync(&pdev->dev);
>> +       pm_runtime_disable(&pdev->dev);
>> +
>> +err_ioremap:
>> +       dev_err(&pdev->dev, "Initialisation failed.\n");
>> +       return ret;
>> +}
>> +
>> +static int exynos_trng_remove(struct platform_device *pdev)
>> +{
>> +       exynos_trng_dev = NULL;
>> +
>> +       return 0;
>> +}
>
> remove is not required as it does not do anything significant.

Should unregister from hwrng in remove callback.

>
>> +
>> +static int __maybe_unused exynos_trng_suspend(struct device *dev)
>> +{
>> +       pm_runtime_put_sync(dev);
>> +
>> +       return 0;
>> +}
>> +
>> +static int __maybe_unused exynos_trng_resume(struct device *dev)
>> +{
>> +       int ret;
>> +
>> +       ret = pm_runtime_get_sync(dev);
>> +       if (ret < 0) {
>> +               dev_err(dev, "pm_runtime_get_sync() failed: %d.\n", ret);
>> +               pm_runtime_put_noidle(dev);
>> +               return ret;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static SIMPLE_DEV_PM_OPS(exynos_trng_pm_ops, exynos_trng_suspend,
>> +                        exynos_trng_resume);
>> +
>> +static const struct of_device_id exynos_trng_dt_match[] = {
>> +       {
>> +               .compatible = "samsung,exynos5250-trng",
>> +       },
>> +       { },
>> +};
>> +MODULE_DEVICE_TABLE(of, exynos_rng_dt_match);
>> +
>> +static struct platform_driver exynos_trng_driver = {
>> +       .driver = {
>> +               .name = "exynos-trng",
>> +               .pm = &exynos_trng_pm_ops,
>> +               .of_match_table = exynos_trng_dt_match,
>> +       },
>> +       .probe = exynos_trng_probe,
>> +       .remove = exynos_trng_remove,
>> +};
>> +
>> +module_platform_driver(exynos_trng_driver);
>> +MODULE_AUTHOR("Łukasz Stelmach");
>> +MODULE_DESCRIPTION("H/W TRNG driver for Exynos chips");
>> +MODULE_LICENSE("GPL");
>> --
>> 2.11.0
>>
>
> Regards,
> PrasannaKumar

Re: [PATCH 2/3] hwrng: exynos - add Samsung Exynos True RNG driver

[PrasannaKumar Muralidharan]

On 24 November 2017 at 20:55, PrasannaKumar Muralidharan
<prasannatsmkumar@gmail.com> wrote:
> Hi Lukasz,
>
> Some minor comments below.
>
> On 23 November 2017 at 20:39, Łukasz Stelmach <l.stelmach@samsung.com> wrote:
>> Add support for True Random Number Generator found in Samsung Exynos
>> 5250+ SoCs.
>>
>> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com>
>> ---
>>  MAINTAINERS                          |   7 +
>>  drivers/char/hw_random/Kconfig       |  12 ++
>>  drivers/char/hw_random/Makefile      |   1 +
>>  drivers/char/hw_random/exynos-trng.c | 256 +++++++++++++++++++++++++++++++++++
>>  4 files changed, 276 insertions(+)
>>  create mode 100644 drivers/char/hw_random/exynos-trng.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 2811a211632c..992074cca612 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -11780,6 +11780,13 @@ S:     Maintained
>>  F:     drivers/crypto/exynos-rng.c
>>  F:     Documentation/devicetree/bindings/rng/samsung,exynos-rng4.txt
>>
>> +SAMSUNG EXYNOS TRUE RANDOM NUMBER GENERATOR (TRNG) DRIVER
>> +M:     Łukasz Stelmach <l.stelmach@samsung.com>
>> +L:     linux-samsung-soc@vger.kernel.org
>> +S:     Maintained
>> +F:     drivers/char/hw_random/exynos-trng.c
>> +F:     Documentation/devicetree/bindings/rng/samsung,exynos5250-trng.txt
>> +
>>  SAMSUNG FRAMEBUFFER DRIVER
>>  M:     Jingoo Han <jingoohan1@gmail.com>
>>  L:     linux-fbdev@vger.kernel.org
>> diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
>> index 95a031e9eced..a788ac07081b 100644
>> --- a/drivers/char/hw_random/Kconfig
>> +++ b/drivers/char/hw_random/Kconfig
>> @@ -449,6 +449,18 @@ config HW_RANDOM_S390
>>
>>           If unsure, say Y.
>>
>> +config HW_RANDOM_EXYNOS
>> +       tristate "Samsung Exynos True Random Number Generator support"
>> +       depends on ARCH_EXYNOS || COMPILE_TEST
>> +       default HW_RANDOM
>> +       ---help---
>> +         This driver provides support for the True Random Number
>> +         Generator available in Exynos SoCs.
>> +
>> +        To compile this driver as a module, choose M here: the module
>> +        will be called exynos-trng.
>> +
>> +        If unsure, say Y.
>>  endif # HW_RANDOM
>>
>>  config UML_RANDOM
>> diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
>> index f3728d008fff..5595df97da7a 100644
>> --- a/drivers/char/hw_random/Makefile
>> +++ b/drivers/char/hw_random/Makefile
>> @@ -14,6 +14,7 @@ obj-$(CONFIG_HW_RANDOM_GEODE) += geode-rng.o
>>  obj-$(CONFIG_HW_RANDOM_N2RNG) += n2-rng.o
>>  n2-rng-y := n2-drv.o n2-asm.o
>>  obj-$(CONFIG_HW_RANDOM_VIA) += via-rng.o
>> +obj-$(CONFIG_HW_RANDOM_EXYNOS) += exynos-trng.o
>>  obj-$(CONFIG_HW_RANDOM_IXP4XX) += ixp4xx-rng.o
>>  obj-$(CONFIG_HW_RANDOM_OMAP) += omap-rng.o
>>  obj-$(CONFIG_HW_RANDOM_OMAP3_ROM) += omap3-rom-rng.o
>> diff --git a/drivers/char/hw_random/exynos-trng.c b/drivers/char/hw_random/exynos-trng.c
>> new file mode 100644
>> index 000000000000..340e106cae24
>> --- /dev/null
>> +++ b/drivers/char/hw_random/exynos-trng.c
>> @@ -0,0 +1,256 @@
>> +/*
>> + * RNG driver for Exynos TRNGs
>> + *
>> + * Author: Łukasz Stelmach <l.stelmach@samsung.com>
>> + *
>> + * Copyright 2017 (c) Samsung Electronics Software, Inc.
>> + *
>> + * Based on the Exynos PRNG driver drivers/crypto/exynos-rng by
>> + * Krzysztof Kozłowski <krzk@kernel.org>
>> + *
>> + * This program 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;
>> + *
>> + * This program 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/crypto.h>
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/hw_random.h>
>> +#include <linux/io.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pm_runtime.h>
>> +
>> +#define EXYNOS_TRNG_CLKDIV         (0x0)
>> +#define EXYNOS_TRNG_CTRL           (0x20)
>> +#define EXYNOS_TRNG_POST_CTRL      (0x30)
>> +#define EXYNOS_TRNG_ONLINE_CTRL    (0x40)
>> +#define EXYNOS_TRNG_ONLINE_STAT    (0x44)
>> +#define EXYNOS_TRNG_ONLINE_MAXCHI2 (0x48)
>> +#define EXYNOS_TRNG_FIFO_CTRL      (0x50)
>> +#define EXYNOS_TRNG_FIFO_0         (0x80)
>> +#define EXYNOS_TRNG_FIFO_1         (0x84)
>> +#define EXYNOS_TRNG_FIFO_2         (0x88)
>> +#define EXYNOS_TRNG_FIFO_3         (0x8c)
>> +#define EXYNOS_TRNG_FIFO_4         (0x90)
>> +#define EXYNOS_TRNG_FIFO_5         (0x94)
>> +#define EXYNOS_TRNG_FIFO_6         (0x98)
>> +#define EXYNOS_TRNG_FIFO_7         (0x9c)
>> +#define EXYNOS_TRNG_FIFO_LEN       (8)
>> +#define EXYNOS_TRNG_CLOCK_RATE     (500000)
>> +
>> +struct exynos_trng_dev {
>> +       struct device    *dev;
>> +       void __iomem     *mem;
>> +       struct clk       *clk;
>> +       struct hwrng rng;
>> +};
>> +
>> +struct exynos_trng_dev *exynos_trng_dev;
>> +
>> +static inline void exynos_trng_set_reg(struct exynos_trng_dev *trng, u16 reg,
>> +                                      u32 val)
>> +{
>> +       /* Check range of reg? */
>> +       __raw_writel(val, trng->mem + reg);
>
> Any specific reason to use __raw_writel? Why not just writel?
>
>> +}
>> +
>> +static inline u32 exynos_trng_get_reg(struct exynos_trng_dev *trng, u16 reg)
>> +{
>> +       /* Check range of reg? */
>> +       return __raw_readl(trng->mem + reg);
>
> Same as above.
>
>> +}
>> +
>> +static int exynos_trng_do_read(struct hwrng *rng, void *data, size_t max,
>> +                              bool wait)
>> +{
>> +       struct exynos_trng_dev *trng;
>> +       u32 val;
>> +
>> +       max = max > (EXYNOS_TRNG_FIFO_LEN * 4) ?
>> +               (EXYNOS_TRNG_FIFO_LEN * 4) : max;
>
> max is always > 32. This condition is not required.
>
>> +
>> +       trng = (struct exynos_trng_dev *)rng->priv;
>> +
>> +       exynos_trng_set_reg(trng, EXYNOS_TRNG_FIFO_CTRL, max * 8);
>> +       val = readl_poll_timeout(trng->mem + EXYNOS_TRNG_FIFO_CTRL, val,
>> +                                val == 0, 200, 1000000);
>> +       if (val < 0)
>> +               return val;
>> +
>> +       memcpy_fromio(data, trng->mem + EXYNOS_TRNG_FIFO_0, max);
>> +
>> +       return max;
>> +}
>> +
>> +static int exynos_trng_init(struct hwrng *rng)
>> +{
>> +       struct exynos_trng_dev *trng;
>> +       unsigned long sss_rate;
>> +       u32 val;
>> +
>> +       trng = (struct exynos_trng_dev *)rng->priv;
>> +       sss_rate = clk_get_rate(trng->clk);
>> +
>> +       /* For most TRNG circuits the clock frequency of under 500 kHz
>> +        * is safe.
>> +        */
>> +       val = sss_rate / (EXYNOS_TRNG_CLOCK_RATE * 2);
>> +       if (val > 0x7fff) {
>> +               dev_err(trng->dev, "clock divider too large: %d", val);
>> +               return -ERANGE;
>> +       }
>> +       val = val << 1;
>> +       exynos_trng_set_reg(trng, EXYNOS_TRNG_CLKDIV, val);
>> +
>> +       /* Enable the generator. */
>> +       val = 1 << 31;
>> +       exynos_trng_set_reg(trng, EXYNOS_TRNG_CTRL, val);
>> +
>> +       /* Disable post processing. /dev/hwrng is supposed to deliver
>> +        * unprocessed data.
>> +        */
>> +       exynos_trng_set_reg(trng, EXYNOS_TRNG_POST_CTRL, 0);
>> +
>> +       return 0;
>> +}
>> +
>> +static int exynos_trng_probe(struct platform_device *pdev)
>> +{
>> +       struct exynos_trng_dev *trng;
>> +       struct resource *res;
>> +       int ret = -ENOMEM;
>> +
>> +       if (exynos_trng_dev)
>> +               return -EEXIST;
>> +
>> +       trng = devm_kzalloc(&pdev->dev, sizeof(*trng), GFP_KERNEL);
>> +       if (!trng)
>> +               goto err_ioremap;
>> +
>> +       trng->rng.name = devm_kstrdup(&pdev->dev, dev_name(&pdev->dev),
>> +                                     GFP_KERNEL);
>> +       if (!trng->rng.name)
>> +               goto err_ioremap;
>> +
>> +       trng->rng.init = exynos_trng_init;
>> +       trng->rng.read = exynos_trng_do_read;
>> +       trng->rng.priv = (unsigned long) trng;
>> +
>> +       platform_set_drvdata(pdev, trng);
>> +       trng->dev = &pdev->dev;
>> +
>> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +       trng->mem = devm_ioremap_resource(&pdev->dev, res);
>> +       if (IS_ERR(trng->mem)) {
>> +               dev_err(&pdev->dev, "Couldn't map IO resources.\n");
>> +               ret = PTR_ERR(trng->mem);
>> +               goto err_ioremap;
>> +       }
>> +
>> +       pm_runtime_enable(&pdev->dev);
>> +       ret = pm_runtime_get_sync(&pdev->dev);
>> +       if (ret < 0) {
>> +               dev_err(&pdev->dev, "pm_runtime_get_sync() failed: %d.\n", ret);
>> +               goto err_ioremap;
>> +       }
>> +
>> +       trng->clk = devm_clk_get(&pdev->dev, "secss");
>> +       if (IS_ERR(trng->clk)) {
>> +               /* XXX: EPROBE_DEFER ? */
>> +               dev_err(&pdev->dev, "Couldn't get clock.\n");
>> +               ret = PTR_ERR(trng->clk);
>> +               goto err_clock;
>> +       }
>> +
>> +       ret = clk_prepare_enable(trng->clk);
>> +       if (ret) {
>> +               dev_err(&pdev->dev, "Unable to enable the clk: %d.\n", ret);
>> +               goto err_clock;
>> +       }
>
> Why not move clk_prepare_enable to init call? While at it please add a
> cleanup callback as well.
>
>> +
>> +       ret = hwrng_register(&trng->rng);
>> +       if (ret) {
>> +               dev_err(&pdev->dev, "Couldn't register hwrng device.\n");
>> +               goto err_register;
>> +       }
>> +
>> +       dev_info(&pdev->dev, "Exynos True Random Number Generator.\n");
>> +
>> +       return 0;
>> +
>> +err_register:
>> +       clk_disable_unprepare(trng->clk);
>> +
>> +err_clock:
>> +       trng->mem = NULL;
>> +       pm_runtime_put_sync(&pdev->dev);
>> +       pm_runtime_disable(&pdev->dev);
>> +
>> +err_ioremap:
>> +       dev_err(&pdev->dev, "Initialisation failed.\n");
>> +       return ret;
>> +}
>> +
>> +static int exynos_trng_remove(struct platform_device *pdev)
>> +{
>> +       exynos_trng_dev = NULL;
>> +
>> +       return 0;
>> +}
>
> remove is not required as it does not do anything significant.

Should unregister from hwrng in remove callback.

>
>> +
>> +static int __maybe_unused exynos_trng_suspend(struct device *dev)
>> +{
>> +       pm_runtime_put_sync(dev);
>> +
>> +       return 0;
>> +}
>> +
>> +static int __maybe_unused exynos_trng_resume(struct device *dev)
>> +{
>> +       int ret;
>> +
>> +       ret = pm_runtime_get_sync(dev);
>> +       if (ret < 0) {
>> +               dev_err(dev, "pm_runtime_get_sync() failed: %d.\n", ret);
>> +               pm_runtime_put_noidle(dev);
>> +               return ret;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static SIMPLE_DEV_PM_OPS(exynos_trng_pm_ops, exynos_trng_suspend,
>> +                        exynos_trng_resume);
>> +
>> +static const struct of_device_id exynos_trng_dt_match[] = {
>> +       {
>> +               .compatible = "samsung,exynos5250-trng",
>> +       },
>> +       { },
>> +};
>> +MODULE_DEVICE_TABLE(of, exynos_rng_dt_match);
>> +
>> +static struct platform_driver exynos_trng_driver = {
>> +       .driver = {
>> +               .name = "exynos-trng",
>> +               .pm = &exynos_trng_pm_ops,
>> +               .of_match_table = exynos_trng_dt_match,
>> +       },
>> +       .probe = exynos_trng_probe,
>> +       .remove = exynos_trng_remove,
>> +};
>> +
>> +module_platform_driver(exynos_trng_driver);
>> +MODULE_AUTHOR("Łukasz Stelmach");
>> +MODULE_DESCRIPTION("H/W TRNG driver for Exynos chips");
>> +MODULE_LICENSE("GPL");
>> --
>> 2.11.0
>>
>
> Regards,
> PrasannaKumar

Re: [PATCH 1/3] dt-bindings: hwrng: Add Samsung Exynos 5250+ True RNG bindings

[Rob Herring]

On Thu, Nov 23, 2017 at 04:09:12PM +0100, Łukasz Stelmach wrote:
> Add binding documentation for the True Random Number Generator
> found on Samsung Exynos 5250+ SoCs.
> 
> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com>
> ---
>  .../devicetree/bindings/rng/samsung,exynos5250-trng.txt | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/rng/samsung,exynos5250-trng.txt

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 2/3] hwrng: exynos - add Samsung Exynos True RNG driver

[Łukasz Stelmach]

[-- Attachment #1: Type: text/plain, Size: 630 bytes --]

It was <2017-11-24 pią 16:25>, when PrasannaKumar Muralidharan wrote:
>> +}
>> +
>> +static int exynos_trng_do_read(struct hwrng *rng, void *data, size_t max,
>> +                              bool wait)
>> +{
>> +       struct exynos_trng_dev *trng;
>> +       u32 val;
>> +
>> +       max = max > (EXYNOS_TRNG_FIFO_LEN * 4) ?
>> +               (EXYNOS_TRNG_FIFO_LEN * 4) : max;
>
> max is always > 32. This condition is not required.

No, it is not. http://elixir.free-electrons.com/linux/latest/source/drivers/char/hw_random/core.c#L67
-- 
Łukasz Stelmach
Samsung R&D Institute Poland
Samsung Electronics

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]