* Re: [PATCH 2/3] hwrng: exynos - add Samsung Exynos True RNG driver
2017-11-23 15:09 ` [PATCH 2/3] hwrng: exynos - add Samsung Exynos True RNG driver Łukasz Stelmach
@ 2017-11-23 15:45 ` Andrew F. Davis
2017-11-23 16:31 ` Krzysztof Kozlowski
2017-11-24 15:25 ` PrasannaKumar Muralidharan
2 siblings, 0 replies; 16+ messages in thread
From: Andrew F. Davis @ 2017-11-23 15:45 UTC (permalink / raw)
To: Łukasz Stelmach, Rob Herring, Matt Mackall, Herbert Xu,
Krzysztof Kozlowski, devicetree, linux-crypto, linux-samsung-soc,
linux-kernel
Cc: Marek Szyprowski, Bartlomiej Zolnierkiewicz
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");
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/3] hwrng: exynos - add Samsung Exynos True RNG driver
2017-11-23 15:09 ` [PATCH 2/3] hwrng: exynos - add Samsung Exynos True RNG driver Łukasz Stelmach
2017-11-23 15:45 ` Andrew F. Davis
@ 2017-11-23 16:31 ` Krzysztof Kozlowski
2017-11-23 18:46 ` Łukasz Stelmach
2017-11-24 15:25 ` PrasannaKumar Muralidharan
2 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2017-11-23 16:31 UTC (permalink / raw)
To: Łukasz Stelmach
Cc: Rob Herring, Matt Mackall, Herbert Xu, devicetree, linux-crypto,
linux-samsung-soc, linux-kernel, Marek Szyprowski,
Bartlomiej Zolnierkiewicz
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
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/3] hwrng: exynos - add Samsung Exynos True RNG driver
2017-11-23 16:31 ` Krzysztof Kozlowski
@ 2017-11-23 18:46 ` Łukasz Stelmach
2017-11-24 12:09 ` Krzysztof Kozlowski
0 siblings, 1 reply; 16+ messages in thread
From: Łukasz Stelmach @ 2017-11-23 18:46 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rob Herring, Matt Mackall, Herbert Xu, devicetree, linux-crypto,
linux-samsung-soc, linux-kernel, Marek Szyprowski,
Bartlomiej Zolnierkiewicz
[-- 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 --]
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/3] hwrng: exynos - add Samsung Exynos True RNG driver
2017-11-23 18:46 ` Łukasz Stelmach
@ 2017-11-24 12:09 ` Krzysztof Kozlowski
2017-11-24 13:05 ` Stephan Müller
0 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2017-11-24 12:09 UTC (permalink / raw)
To: Łukasz Stelmach
Cc: Rob Herring, Matt Mackall, Herbert Xu, devicetree, linux-crypto,
linux-samsung-soc, linux-kernel, Marek Szyprowski,
Bartlomiej Zolnierkiewicz, Stephan Müller
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] hwrng: exynos - add Samsung Exynos True RNG driver
2017-11-24 12:09 ` Krzysztof Kozlowski
@ 2017-11-24 13:05 ` Stephan Müller
2017-11-24 14:13 ` Krzysztof Kozlowski
0 siblings, 1 reply; 16+ messages in thread
From: Stephan Müller @ 2017-11-24 13:05 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Łukasz Stelmach, Rob Herring, Matt Mackall, Herbert Xu,
devicetree, linux-crypto, linux-samsung-soc, linux-kernel,
Marek Szyprowski, Bartlomiej Zolnierkiewicz
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] hwrng: exynos - add Samsung Exynos True RNG driver
2017-11-24 13:05 ` Stephan Müller
@ 2017-11-24 14:13 ` Krzysztof Kozlowski
0 siblings, 0 replies; 16+ messages in thread
From: Krzysztof Kozlowski @ 2017-11-24 14:13 UTC (permalink / raw)
To: Stephan Müller
Cc: Łukasz Stelmach, Rob Herring, Matt Mackall, Herbert Xu,
devicetree, linux-crypto, linux-samsung-soc, linux-kernel,
Marek Szyprowski, Bartlomiej Zolnierkiewicz
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] hwrng: exynos - add Samsung Exynos True RNG driver
2017-11-23 15:09 ` [PATCH 2/3] hwrng: exynos - add Samsung Exynos True RNG driver Łukasz Stelmach
2017-11-23 15:45 ` Andrew F. Davis
2017-11-23 16:31 ` Krzysztof Kozlowski
@ 2017-11-24 15:25 ` PrasannaKumar Muralidharan
2017-11-24 15:54 ` PrasannaKumar Muralidharan
2017-11-27 7:23 ` Łukasz Stelmach
2 siblings, 2 replies; 16+ messages in thread
From: PrasannaKumar Muralidharan @ 2017-11-24 15:25 UTC (permalink / raw)
To: Łukasz Stelmach
Cc: Rob Herring, Matt Mackall, Herbert Xu, Krzysztof Kozlowski,
devicetree, open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
linux-samsung-soc, open list, Marek Szyprowski,
Bartlomiej Zolnierkiewicz
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
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/3] hwrng: exynos - add Samsung Exynos True RNG driver
2017-11-24 15:25 ` PrasannaKumar Muralidharan
@ 2017-11-24 15:54 ` PrasannaKumar Muralidharan
2017-11-27 7:23 ` Łukasz Stelmach
1 sibling, 0 replies; 16+ messages in thread
From: PrasannaKumar Muralidharan @ 2017-11-24 15:54 UTC (permalink / raw)
To: Łukasz Stelmach
Cc: Rob Herring, Matt Mackall, Herbert Xu, Krzysztof Kozlowski,
devicetree, open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
linux-samsung-soc, open list, Marek Szyprowski,
Bartlomiej Zolnierkiewicz
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
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 2/3] hwrng: exynos - add Samsung Exynos True RNG driver
2017-11-24 15:25 ` PrasannaKumar Muralidharan
2017-11-24 15:54 ` PrasannaKumar Muralidharan
@ 2017-11-27 7:23 ` Łukasz Stelmach
1 sibling, 0 replies; 16+ messages in thread
From: Łukasz Stelmach @ 2017-11-27 7:23 UTC (permalink / raw)
To: PrasannaKumar Muralidharan
Cc: Rob Herring, Matt Mackall, Herbert Xu, Krzysztof Kozlowski,
devicetree, linux-crypto, linux-samsung-soc, linux-kernel,
Marek Szyprowski, Bartlomiej Zolnierkiewicz
[-- 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 --]
^ permalink raw reply [flat|nested] 16+ messages in thread