lkml.org 
[lkml]   [2017]   [Nov]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 2/3] hwrng: exynos - add Samsung Exynos True RNG driver
Date
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
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2017-11-23 19:47    [W:0.060 / U:1.352 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site