hwrng: add hwrng support for Rockchip RK3568

From: Aurelien Jarno <[email protected]>

Add the RNG bindings for the RK3568 SoC from Rockchip

Signed-off-by Aurelien Jarno <[email protected]>

Signed-off-by Daniel Golle <[email protected]>

---
 .../bindings/rng/rockchip,rk3568-rng.yaml     | 60 +++++++++++++++++++
 MAINTAINERS                                   |  6 ++
 2 files changed, 66 insertions(+)
 create mode 100644

Documentation/devicetree/bindings/rng/rockchip,rk3568-rng.yaml

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/rng/rockchip,rk3568-rng.yaml new file mode 100644
index 000000000000..d45f03683fbe

Documentation/devicetree/bindings/rng/rockchip,rk3568-rng.yaml line 9

Quoted 3 lines

Documentation/devicetree/bindings/rng/rockchip,rk3568-rng.yaml new file +3 -0

+title: Rockchip TRNG
+
+description: True Random Number Generator on Rokchip RK3568 SoC

I *think* that the TRNG for rk3588 is different, so shouldn't the title be:

Rockchip TRNG for rk356x SoCs

From: Aurelien Jarno <[email protected]>

Add the RNG bindings for the RK3568 SoC from Rockchip

Signed-off-by Aurelien Jarno <[email protected]>

Signed-off-by Daniel Golle <[email protected]>

(quoted diff) +0 -0

b/drivers/char/hw_random/rockchip-rng.c new file mode 100644 index 000000000000..6070abb73847

drivers/char/hw_random/rockchip-rng.c new file +10 -0

@@ -0,0 +1,229 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * rockchip-rng.c True Random Number Generator driver for Rockchip SoCs
+ *
+ * Copyright (c) 2018, Fuzhou Rockchip Electronics Co., Ltd.
+ * Copyright (c) 2022, Aurelien Jarno
+ * Authors:
+ *  Lin Jinhan <[email protected]>
+ *  Aurelien Jarno <[email protected]>
+ */

From: Aurelien Jarno <[email protected]>

drivers/char/hw_random/rockchip-rng.c new file +12 -0

+
+static int rk_rng_init(struct hwrng *rng)
+{
+	struct rk_rng *rk_rng = container_of(rng, struct rk_rng, rng);
+	int ret;
+
+	/* start clocks */
+	ret = clk_bulk_prepare_enable(rk_rng->clk_num, rk_rng->clk_bulks);
+	if (ret < 0) {
+		dev_err((struct device *) rk_rng->rng.priv,
+			"Failed to enable clks %d\n", ret);
+		return ret;

drivers/char/hw_random/rockchip-rng.c new file +58 -0

+
+	/* Start collecting random data */
+	writel_relaxed(TRNG_RNG_CTL_START, rk_rng->base + TRNG_RNG_CTL);
+
+	ret = readl_poll_timeout(rk_rng->base + TRNG_RNG_CTL, reg,
+				 !(reg & TRNG_RNG_CTL_START),
+				 RK_RNG_POLL_PERIOD_US,
+				 RK_RNG_POLL_TIMEOUT_US);
+	if (ret < 0)
+		goto out;
+
+	/* Read random data stored in the registers */
+	memcpy_fromio(buf, rk_rng->base + TRNG_RNG_DOUT, to_read);
+out:
+	pm_runtime_mark_last_busy((struct device *) rk_rng->rng.priv);
+	pm_runtime_put_sync_autosuspend((struct device *) rk_rng->rng.priv);
+
+	return to_read;
+}
+
+static int rk_rng_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rk_rng *rk_rng;
+	int ret;
+
+	rk_rng = devm_kzalloc(dev, sizeof(*rk_rng), GFP_KERNEL);
+	if (!rk_rng)
+		return -ENOMEM;
+
+	rk_rng->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(rk_rng->base))
+		return PTR_ERR(rk_rng->base);
+
+	rk_rng->clk_num = devm_clk_bulk_get_all(dev, &rk_rng->clk_bulks);
+	if (rk_rng->clk_num < 0)
+		return dev_err_probe(dev, rk_rng->clk_num,
+				     "Failed to get clks property\n");
+
+	rk_rng->rst = devm_reset_control_array_get(&pdev->dev, false, false);
+	if (IS_ERR(rk_rng->rst))
+		return dev_err_probe(dev, PTR_ERR(rk_rng->rst),
+				     "Failed to get reset property\n");
+
+	reset_control_assert(rk_rng->rst);
+	udelay(2);
+	reset_control_deassert(rk_rng->rst);
+
+	platform_set_drvdata(pdev, rk_rng);
+
+	rk_rng->rng.name = dev_driver_string(dev);
+#ifndef CONFIG_PM
+	rk_rng->rng.init = rk_rng_init;
+	rk_rng->rng.cleanup = rk_rng_cleanup;
+#endif
+	rk_rng->rng.read = rk_rng_read;
+	rk_rng->rng.priv = (unsigned long) dev;
+	rk_rng->rng.quality = 900;

Quoted 20 lines · Daniel Golle

From: Aurelien Jarno <[email protected]>

drivers/char/hw_random/rockchip-rng.c new file +17 -0

+
+static int rk_rng_init(struct hwrng *rng)
+{
+     struct rk_rng *rk_rng = container_of(rng, struct rk_rng, rng);
+     int ret;
+
+     /* start clocks */
+     ret = clk_bulk_prepare_enable(rk_rng->clk_num, rk_rng->clk_bulks);
+     if (ret < 0) {
+             dev_err((struct device *) rk_rng->rng.priv,
+                     "Failed to enable clks %d\n", ret);
+             return ret;
+     }
+
+     /* set the sample period */
+     writel(RK_RNG_SAMPLE_CNT, rk_rng->base + TRNG_RNG_SAMPLE_CNT);
+

drivers/char/hw_random/rockchip-rng.c line 78

Quoted 5 lines

drivers/char/hw_random/rockchip-rng.c new file +5 -0

+     /* set osc ring speed and enable it */
+     writel_relaxed(TRNG_RNG_CTL_LEN_256_BIT |
+                    TRNG_RNG_CTL_OSC_RING_SPEED_0 |
+                    TRNG_RNG_CTL_ENABLE,
+                    rk_rng->base + TRNG_RNG_CTL);

I doubt relaxed write is here intentional. Enabling should be last instruction, so this should be ordered write.

drivers/char/hw_random/rockchip-rng.c line 88

Quoted 10 lines

drivers/char/hw_random/rockchip-rng.c new file +10 -0

+
+     return 0;
+}
+
+static void rk_rng_cleanup(struct hwrng *rng)
+{
+     struct rk_rng *rk_rng = container_of(rng, struct rk_rng, rng);
+
+     /* stop TRNG */
+     writel_relaxed(0, rk_rng->base + TRNG_RNG_CTL);

This should not be relaxed. This might lead to very tricky to debug issues.

drivers/char/hw_random/rockchip-rng.c line 101

Quoted 13 lines

drivers/char/hw_random/rockchip-rng.c new file +13 -0

+
+     /* stop clocks */
+     clk_bulk_disable_unprepare(rk_rng->clk_num, rk_rng->clk_bulks);
+}
+
+static int rk_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
+{
+     struct rk_rng *rk_rng = container_of(rng, struct rk_rng, rng);
+     size_t to_read = min_t(size_t, max, RK_RNG_MAX_BYTE);
+     u32 reg;
+     int ret = 0;
+
+     ret = pm_runtime_get_sync((struct device *) rk_rng->rng.priv);

Why this cannot be just simpler pm_runtime_resume_and_get()?

Review

Quoted 2 lines

(quoted diff) +2 -0

+     if (ret < 0)
+             goto out;

This does not look like correct error path. Device was not busy here.

Quoted 58 lines

drivers/char/hw_random/rockchip-rng.c new file +58 -0

+
+     /* Start collecting random data */
+     writel_relaxed(TRNG_RNG_CTL_START, rk_rng->base + TRNG_RNG_CTL);
+
+     ret = readl_poll_timeout(rk_rng->base + TRNG_RNG_CTL, reg,
+                              !(reg & TRNG_RNG_CTL_START),
+                              RK_RNG_POLL_PERIOD_US,
+                              RK_RNG_POLL_TIMEOUT_US);
+     if (ret < 0)
+             goto out;
+
+     /* Read random data stored in the registers */
+     memcpy_fromio(buf, rk_rng->base + TRNG_RNG_DOUT, to_read);
+out:
+     pm_runtime_mark_last_busy((struct device *) rk_rng->rng.priv);
+     pm_runtime_put_sync_autosuspend((struct device *) rk_rng->rng.priv);
+
+     return to_read;
+}
+
+static int rk_rng_probe(struct platform_device *pdev)
+{
+     struct device *dev = &pdev->dev;
+     struct rk_rng *rk_rng;
+     int ret;
+
+     rk_rng = devm_kzalloc(dev, sizeof(*rk_rng), GFP_KERNEL);
+     if (!rk_rng)
+             return -ENOMEM;
+
+     rk_rng->base = devm_platform_ioremap_resource(pdev, 0);
+     if (IS_ERR(rk_rng->base))
+             return PTR_ERR(rk_rng->base);
+
+     rk_rng->clk_num = devm_clk_bulk_get_all(dev, &rk_rng->clk_bulks);
+     if (rk_rng->clk_num < 0)
+             return dev_err_probe(dev, rk_rng->clk_num,
+                                  "Failed to get clks property\n");
+
+     rk_rng->rst = devm_reset_control_array_get(&pdev->dev, false, false);
+     if (IS_ERR(rk_rng->rst))
+             return dev_err_probe(dev, PTR_ERR(rk_rng->rst),
+                                  "Failed to get reset property\n");
+
+     reset_control_assert(rk_rng->rst);
+     udelay(2);
+     reset_control_deassert(rk_rng->rst);
+
+     platform_set_drvdata(pdev, rk_rng);
+
+     rk_rng->rng.name = dev_driver_string(dev);
+#ifndef CONFIG_PM
+     rk_rng->rng.init = rk_rng_init;
+     rk_rng->rng.cleanup = rk_rng_cleanup;
+#endif
+     rk_rng->rng.read = rk_rng_read;
+     rk_rng->rng.priv = (unsigned long) dev;
+     rk_rng->rng.quality = 900;

drivers/char/hw_random/rockchip-rng.c line 171

Quoted 10 lines

drivers/char/hw_random/rockchip-rng.c new file +10 -0

+
+     pm_runtime_set_autosuspend_delay(dev, RK_RNG_AUTOSUSPEND_DELAY);
+     pm_runtime_use_autosuspend(dev);
+     pm_runtime_enable(dev);
+
+     ret = devm_hwrng_register(dev, &rk_rng->rng);
+     if (ret)
+             return dev_err_probe(&pdev->dev, ret, "Failed to register Rockchip hwrng\n");
+
+     dev_info(&pdev->dev, "Registered Rockchip hwrng\n");

Drop, driver should be silent on success.

Best regards,
Krzysztof

Quoted 1 line · Daniel Golle

From: Aurelien Jarno <[email protected]>

Quoted 4 lines

drivers/char/hw_random/rockchip-rng.c new file +4 -0

+	pm_runtime_set_autosuspend_delay(dev, RK_RNG_AUTOSUSPEND_DELAY);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_enable(dev);
+

Review

Quoted 6 lines

drivers/char/hw_random/rockchip-rng.c new file +3 -0

+	ret = devm_hwrng_register(dev, &rk_rng->rng);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "Failed to register Rockchip 

hwrng\n");

(quoted diff) +2 -0

+
+	dev_info(&pdev->dev, "Registered Rockchip hwrng\n");

Drop, driver should be silent on success.

Quoted 2 lines · Krzysztof Kozlowski

Quoted 1 line · Daniel Golle

From: Aurelien Jarno <[email protected]>

[snip]

Quoted 4 lines

Quoted 4 lines

+    pm_runtime_set_autosuspend_delay(dev, RK_RNG_AUTOSUSPEND_DELAY); +    pm_runtime_use_autosuspend(dev);
+    pm_runtime_enable(dev);
+

Quoted 8 lines

Quoted 6 lines

+    ret = devm_hwrng_register(dev, &rk_rng->rng);
+    if (ret)
+        return dev_err_probe(&pdev->dev, ret, "Failed to register Rockchip hwrng\n");
+
+    dev_info(&pdev->dev, "Registered Rockchip hwrng\n");

Drop, driver should be silent on success.

I respectfully disagree.  Many drivers print a single line upon successful probing, which I find very useful.  In this particular case, it's even more useful, because some people may be concerned about the use of hardware TRNGs, so we should actually make sure to announce it.

Quoted 20 lines · Dragan Simic

Quoted 2 lines · Krzysztof Kozlowski

Quoted 1 line · Daniel Golle

From: Aurelien Jarno <[email protected]>

[snip]

Quoted 5 lines

Quoted 5 lines

+    pm_runtime_set_autosuspend_delay(dev,
RK_RNG_AUTOSUSPEND_DELAY);
+    pm_runtime_use_autosuspend(dev);
+    pm_runtime_enable(dev);
+

Quoted 8 lines

Quoted 6 lines

+    ret = devm_hwrng_register(dev, &rk_rng->rng);
+    if (ret)
+        return dev_err_probe(&pdev->dev, ret, "Failed to register Rockchip hwrng\n");
+
+    dev_info(&pdev->dev, "Registered Rockchip hwrng\n");

Drop, driver should be silent on success.

Quoted 5 lines

I respectfully disagree.  Many drivers print a single line upon successful probing, which I find very useful.  In this particular case, it's even more useful, because some people may be concerned about the use of hardware TRNGs, so we should actually make sure to announce it.

I agree to Krzysztof here. From the POV of a driver author, your own driver is very important and while you write it, it really interests *you* if the driver is successfully probed. However from a system perspective these are annoying: There are easily >50 devices[1] on a system, if all of these print a message in probe, you have little chance to see the relevant messages. Even if every driver author thinks their work is a special snow flake that is worth announcing, in practice users only care about your driver if there is a problem. Additionally each message takes time and so delays the boot process. Additionally each message takes place in the printk ring buffer and so edges out earlier messages that might be more important.

So +1 for dropping the dev_info() or at least using dev_debug() for it.

Best regards
Uwe

[1] On my laptop if have:

	$ find /sys/devices -name driver | wc -l
	87

On a Raspberrypi it yields 66.

Hello Uwe,

Quoted 27 lines · Uwe Kleine-König

Quoted 20 lines · Dragan Simic

Quoted 2 lines · Krzysztof Kozlowski

Quoted 1 line · Daniel Golle

From: Aurelien Jarno <[email protected]>

[snip]

Quoted 5 lines

Quoted 5 lines

+ pm_runtime_set_autosuspend_delay(dev,
RK_RNG_AUTOSUSPEND_DELAY);

drivers/char/hw_random/rockchip-rng.c new file +3 -0

+    pm_runtime_use_autosuspend(dev);
+    pm_runtime_enable(dev);
+

Quoted 8 lines

Review

Quoted 6 lines

drivers/char/hw_random/rockchip-rng.c new file +3 -0

+    ret = devm_hwrng_register(dev, &rk_rng->rng);
+    if (ret)
+        return dev_err_probe(&pdev->dev, ret, "Failed to register 

Rockchip hwrng\n");

(quoted diff) +2 -0

+
+    dev_info(&pdev->dev, "Registered Rockchip hwrng\n");

Drop, driver should be silent on success.

Quoted 5 lines

I respectfully disagree. Many drivers print a single line upon successful probing, which I find very useful. In this particular case, it's even more useful, because some people may be concerned about the use of hardware TRNGs, so we should actually make sure to announce it.

Quoted 13 lines

I agree to Krzysztof here. From the POV of a driver author, your own driver is very important and while you write it, it really interests *you* if the driver is successfully probed. However from a system perspective these are annoying: There are easily >50 devices[1] on a system, if all of these print a message in probe, you have little chance
to see the relevant messages. Even if every driver author thinks their work is a special snow flake that is worth announcing, in practice users
only care about your driver if there is a problem. Additionally each message takes time and so delays the boot process. Additionally each message takes place in the printk ring buffer and so edges out earlier messages that might be more important.

Well, I don't find those messages annoying, for the drivers I've had nothing to do with. Also, in my experience, 99.9% of users don't care about the kernel messages at all, be it everything hunky-dory, or be it something really wrong somewhere.

Quoted 1 line

So +1 for dropping the dev_info() or at least using dev_debug() for it.

Am Samstag, 22. Juni 2024, 12:29:33 CEST schrieb Dragan Simic:

Quoted 47 lines · Uwe Kleine-König

Quoted 27 lines · Dragan Simic

Quoted 20 lines · Krzysztof Kozlowski

Quoted 2 lines · Daniel Golle

Quoted 1 line

From: Aurelien Jarno <[email protected]>

[snip]

Quoted 5 lines

Quoted 5 lines

+ pm_runtime_set_autosuspend_delay(dev,
RK_RNG_AUTOSUSPEND_DELAY);

drivers/char/hw_random/rockchip-rng.c new file +3 -0

+    pm_runtime_use_autosuspend(dev);
+    pm_runtime_enable(dev);
+

Quoted 8 lines

Review

Quoted 6 lines

drivers/char/hw_random/rockchip-rng.c new file +3 -0

+    ret = devm_hwrng_register(dev, &rk_rng->rng);
+    if (ret)
+        return dev_err_probe(&pdev->dev, ret, "Failed to register

Rockchip hwrng\n");

(quoted diff) +2 -0

+
+    dev_info(&pdev->dev, "Registered Rockchip hwrng\n");

Drop, driver should be silent on success.

Quoted 5 lines

I respectfully disagree. Many drivers print a single line upon successful probing, which I find very useful. In this particular case, it's even more useful, because some people may be concerned about the use of hardware TRNGs, so we should actually make sure to announce it.

Quoted 13 lines

I agree to Krzysztof here. From the POV of a driver author, your own driver is very important and while you write it, it really interests *you* if the driver is successfully probed. However from a system perspective these are annoying: There are easily >50 devices[1] on a system, if all of these print a message in probe, you have little chance
to see the relevant messages. Even if every driver author thinks their work is a special snow flake that is worth announcing, in practice users
only care about your driver if there is a problem. Additionally each message takes time and so delays the boot process. Additionally each message takes place in the printk ring buffer and so edges out earlier messages that might be more important.

Well, I don't find those messages annoying, for the drivers I've had nothing to do with. Also, in my experience, 99.9% of users don't care about the kernel messages at all, be it everything hunky-dory, or be it something really wrong somewhere.

Quoted 1 line

Quoted 1 line

So +1 for dropping the dev_info() or at least using dev_debug() for it.

Just for 2ct ... I'm also in the don't print too much camp ;-) . When parsing kernel logs to see where things fail, messages just telling me about sucesses make things more difficult.

So really this message should be dropped or at least as Uwe suggests made a dev_dbg.

Quoted 9 lines · Heiko Stübner

Am Samstag, 22. Juni 2024, 12:29:33 CEST schrieb Dragan Simic:

Quoted 8 lines · Uwe Kleine-König

Quoted 7 lines · Dragan Simic

Quoted 6 lines · Krzysztof Kozlowski

Quoted 4 lines · Daniel Golle

Quoted 1 line

+ dev_info(&pdev->dev, "Registered Rockchip hwrng\n");

Drop, driver should be silent on success.

Quoted 3 lines

[...]
So really this message should be dropped or at least as Uwe suggests made a dev_dbg.

As a note, "dmesg --level=err,warn", for example, is rather useful when it comes to filtering the kernel messages to see only those that are signs of a trouble.

Quoted 10 lines · Dragan Simic

Quoted 9 lines · Heiko Stübner

Am Samstag, 22. Juni 2024, 12:29:33 CEST schrieb Dragan Simic:

Quoted 8 lines · Uwe Kleine-König

Quoted 7 lines · Dragan Simic

Quoted 6 lines · Krzysztof Kozlowski

Quoted 4 lines · Daniel Golle

Quoted 1 line

+ dev_info(&pdev->dev, "Registered Rockchip hwrng\n");

Drop, driver should be silent on success.

Quoted 7 lines

Quoted 3 lines

[...]
So really this message should be dropped or at least as Uwe suggests made a dev_dbg.

As a note, "dmesg --level=err,warn", for example, is rather useful when it comes to filtering the kernel messages to see only those that are signs of a trouble.

IMHO it's a bit sad, that there is such a long thread about something so trivial, but I want to make a few points:

- not all dmesg implementations support this:

root@machine:~ dmesg --level=err,warn
dmesg: unrecognized option '--level=err,warn' BusyBox v1.36.1 () multi-call binary.

Usage: dmesg [-cr] [-n LEVEL] [-s SIZE]

Print or control the kernel ring buffer

-c Clear ring buffer after printing
-n LEVEL Set console logging level
-s SIZE Buffer size
-r Print raw message buffer

- Your argument that the output of this dev_info can easily be ignored is a very weak reason to keep it.

- I personally consider it hard sometimes to accept feedback if I
think
it's wrong and there is a good reason to do it the way I want it. But there are now three people opposing your position, who brought forward (IMHO) good reasons and even a constructive alternative was presented. Please stay open minded while weighting the options. The questions to ask here include:

- How many people care for this message? During every boot? Is it maybe enough for these people to check in /sys if the device probed successfully? Or maybe even the absence of an error
message
is enough?
- How many people get this message and don't care about the presented information? How many people are even annoyed by it?
- Is the delay and memory usage introduced by this message
justified
then, even if it's small?

Hello Krzysztof,

Quoted 2 lines · Krzysztof Kozlowski

Quoted 1 line · Daniel Golle

From: Aurelien Jarno <[email protected]>

[snip]

Quoted 4 lines

Quoted 4 lines

drivers/char/hw_random/rockchip-rng.c new file +4 -0

+	pm_runtime_set_autosuspend_delay(dev, RK_RNG_AUTOSUSPEND_DELAY);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_enable(dev);
+

Quoted 8 lines

Review

Quoted 6 lines

drivers/char/hw_random/rockchip-rng.c new file +3 -0

+	ret = devm_hwrng_register(dev, &rk_rng->rng);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "Failed to register Rockchip 

hwrng\n");

(quoted diff) +2 -0

+
+	dev_info(&pdev->dev, "Registered Rockchip hwrng\n");

Drop, driver should be silent on success.

I respectfully disagree. Many drivers print a single line upon successful probing, which I find very useful. In this particular

case, it's even more useful, because some people may be concerned about the use of hardware TRNGs, so we should actually make sure to announce it.

Quoted 20 lines · Dragan Simic

Quoted 2 lines · Krzysztof Kozlowski

Quoted 1 line · Daniel Golle

From: Aurelien Jarno <[email protected]>

[snip]

Quoted 13 lines

Review

Quoted 11 lines

drivers/char/hw_random/rockchip-rng.c new file +7 -0

+	pm_runtime_set_autosuspend_delay(dev, RK_RNG_AUTOSUSPEND_DELAY);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_enable(dev);
+
+	ret = devm_hwrng_register(dev, &rk_rng->rng);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "Failed to register 

Rockchip
hwrng\n");

(quoted diff) +2 -0

+
+	dev_info(&pdev->dev, "Registered Rockchip hwrng\n");

Drop, driver should be silent on success.

Quoted 2 lines

I respectfully disagree. Many drivers print a single line upon successful probing, which I find very useful. In this particular

No, it's duplicating existing interfaces and polluting log unnecessarily without any useful information.

Quoted 3 lines

case, it's even more useful, because some people may be concerned about the use of hardware TRNGs, so we should actually make sure to announce it.

From: Aurelien Jarno <[email protected]>

Rockchip SoCs used to have a random number generator as part of their crypto device, and support for it has to be added to the corresponding driver. However newer Rockchip SoCs like the RK356x have an independent True Random Number Generator device. This patch adds a driver for it, greatly inspired from the downstream driver.

The TRNG device does not seem to have a signal conditionner and the FIPS 140-2 test returns a lot of failures. They can be reduced by increasing RK_RNG_SAMPLE_CNT, in a tradeoff between quality and speed. This value has been adjusted to get ~90% of successes and the quality value has been set accordingly.

Signed-off-by Aurelien Jarno <[email protected]>

[[email protected]: code style fixes]

Signed-off-by Daniel Golle <[email protected]>

---
 MAINTAINERS                           |   1 +
 drivers/char/hw_random/Kconfig        |  14 ++
 drivers/char/hw_random/Makefile       |   1 +
 drivers/char/hw_random/rockchip-rng.c | 229 ++++++++++++++++++++++++++
 4 files changed, 245 insertions(+)
 create mode 100644 drivers/char/hw_random/rockchip-rng.c

MAINTAINERS +1 -0

index 77d449c89bf2..299b8c1a5fb5 100644
@@ -19493,6 +19493,7 @@ M:      Daniel Golle <[email protected]>
 M:     Aurelien Jarno <[email protected]>
 S:     Maintained
 F:     Documentation/devicetree/bindings/rng/rockchip,rk3568-rng.yaml
+F:     drivers/char/hw_random/rockchip-rng.c
 
 ROCKCHIP RASTER 2D GRAPHIC ACCELERATION UNIT DRIVER
 M:     Jacob Chen <[email protected]>

drivers/char/hw_random/Kconfig +14 -0

index 442c40efb200..2b62cd08f91a 100644
@@ -573,6 +573,20 @@ config HW_RANDOM_JH7110
          To compile this driver as a module, choose M here.
          The module will be called jh7110-trng.
 
+config HW_RANDOM_ROCKCHIP
+       tristate "Rockchip True Random Number Generator"
+       depends on HW_RANDOM && (ARCH_ROCKCHIP || COMPILE_TEST)
+       depends on HAS_IOMEM
+       default HW_RANDOM
+       help
+         This driver provides kernel-side support for the True Random Number
+         Generator hardware found on some Rockchip SoC like RK3566 or RK3568.
+
+         To compile this driver as a module, choose M here: the
+         module will be called rockchip-rng.
+
+         If unsure, say Y.
+
 endif # HW_RANDOM
 
 config UML_RANDOM

drivers/char/hw_random/Makefile +1 -0

index 32549a1186dc..01f012eab440 100644
@@ -48,4 +48,5 @@ obj-$(CONFIG_HW_RANDOM_XIPHERA) += xiphera-trng.o
 obj-$(CONFIG_HW_RANDOM_ARM_SMCCC_TRNG) += arm_smccc_trng.o
 obj-$(CONFIG_HW_RANDOM_CN10K) += cn10k-rng.o
 obj-$(CONFIG_HW_RANDOM_POLARFIRE_SOC) += mpfs-rng.o
+obj-$(CONFIG_HW_RANDOM_ROCKCHIP) += rockchip-rng.o
 obj-$(CONFIG_HW_RANDOM_JH7110) += jh7110-trng.o

drivers/char/hw_random/rockchip-rng.c new file +7 -0

new file mode 100644
index 000000000000..6070abb73847
@@ -0,0 +1,229 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * rockchip-rng.c True Random Number Generator driver for Rockchip SoCs
+ *
+ * Copyright (c) 2018, Fuzhou Rockchip Electronics Co., Ltd.
+ * Copyright (c) 2022, Aurelien Jarno
+ * Authors:

drivers/char/hw_random/rockchip-rng.c new file +138 -0

+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+#include <linux/slab.h>
+
+#define RK_RNG_AUTOSUSPEND_DELAY       100
+#define RK_RNG_MAX_BYTE                        32
+#define RK_RNG_POLL_PERIOD_US          100
+#define RK_RNG_POLL_TIMEOUT_US         10000
+
+/*
+ * TRNG collects osc ring output bit every RK_RNG_SAMPLE_CNT time. The value is
+ * a tradeoff between speed and quality and has been adjusted to get a quality
+ * of ~900 (~90% of FIPS 140-2 successes).
+ */
+#define RK_RNG_SAMPLE_CNT              1000
+
+/* TRNG registers from RK3568 TRM-Part2, section 5.4.1 */
+#define TRNG_RST_CTL                   0x0004
+#define TRNG_RNG_CTL                   0x0400
+#define TRNG_RNG_CTL_LEN_64_BIT                (0x00 << 4)
+#define TRNG_RNG_CTL_LEN_128_BIT       (0x01 << 4)
+#define TRNG_RNG_CTL_LEN_192_BIT       (0x02 << 4)
+#define TRNG_RNG_CTL_LEN_256_BIT       (0x03 << 4)
+#define TRNG_RNG_CTL_OSC_RING_SPEED_0  (0x00 << 2)
+#define TRNG_RNG_CTL_OSC_RING_SPEED_1  (0x01 << 2)
+#define TRNG_RNG_CTL_OSC_RING_SPEED_2  (0x02 << 2)
+#define TRNG_RNG_CTL_OSC_RING_SPEED_3  (0x03 << 2)
+#define TRNG_RNG_CTL_ENABLE            BIT(1)
+#define TRNG_RNG_CTL_START             BIT(0)
+#define TRNG_RNG_SAMPLE_CNT            0x0404
+#define TRNG_RNG_DOUT                  0x0410
+
+struct rk_rng {
+       struct hwrng rng;
+       void __iomem *base;
+       struct reset_control *rst;
+       int clk_num;
+       struct clk_bulk_data *clk_bulks;
+};
+
+static int rk_rng_init(struct hwrng *rng)
+{
+       struct rk_rng *rk_rng = container_of(rng, struct rk_rng, rng);
+       int ret;
+
+       /* start clocks */
+       ret = clk_bulk_prepare_enable(rk_rng->clk_num, rk_rng->clk_bulks);
+       if (ret < 0) {
+               dev_err((struct device *) rk_rng->rng.priv,
+                       "Failed to enable clks %d\n", ret);
+               return ret;
+       }
+
+       /* set the sample period */
+       writel(RK_RNG_SAMPLE_CNT, rk_rng->base + TRNG_RNG_SAMPLE_CNT);
+
+       /* set osc ring speed and enable it */
+       writel_relaxed(TRNG_RNG_CTL_LEN_256_BIT |
+                      TRNG_RNG_CTL_OSC_RING_SPEED_0 |
+                      TRNG_RNG_CTL_ENABLE,
+                      rk_rng->base + TRNG_RNG_CTL);
+
+       return 0;
+}
+
+static void rk_rng_cleanup(struct hwrng *rng)
+{
+       struct rk_rng *rk_rng = container_of(rng, struct rk_rng, rng);
+
+       /* stop TRNG */
+       writel_relaxed(0, rk_rng->base + TRNG_RNG_CTL);
+
+       /* stop clocks */
+       clk_bulk_disable_unprepare(rk_rng->clk_num, rk_rng->clk_bulks);
+}
+
+static int rk_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
+{
+       struct rk_rng *rk_rng = container_of(rng, struct rk_rng, rng);
+       size_t to_read = min_t(size_t, max, RK_RNG_MAX_BYTE);
+       u32 reg;
+       int ret = 0;
+
+       ret = pm_runtime_get_sync((struct device *) rk_rng->rng.priv);
+       if (ret < 0)
+               goto out;
+
+       /* Start collecting random data */
+       writel_relaxed(TRNG_RNG_CTL_START, rk_rng->base + TRNG_RNG_CTL);
+
+       ret = readl_poll_timeout(rk_rng->base + TRNG_RNG_CTL, reg,
+                                !(reg & TRNG_RNG_CTL_START),
+                                RK_RNG_POLL_PERIOD_US,
+                                RK_RNG_POLL_TIMEOUT_US);
+       if (ret < 0)
+               goto out;
+
+       /* Read random data stored in the registers */
+       memcpy_fromio(buf, rk_rng->base + TRNG_RNG_DOUT, to_read);
+out:
+       pm_runtime_mark_last_busy((struct device *) rk_rng->rng.priv);
+       pm_runtime_put_sync_autosuspend((struct device *) rk_rng->rng.priv);
+
+       return to_read;
+}
+
+static int rk_rng_probe(struct platform_device *pdev)
+{
+       struct device *dev = &pdev->dev;
+       struct rk_rng *rk_rng;
+       int ret;
+
+       rk_rng = devm_kzalloc(dev, sizeof(*rk_rng), GFP_KERNEL);
+       if (!rk_rng)
+               return -ENOMEM;
+
+       rk_rng->base = devm_platform_ioremap_resource(pdev, 0);
+       if (IS_ERR(rk_rng->base))
+               return PTR_ERR(rk_rng->base);
+
+       rk_rng->clk_num = devm_clk_bulk_get_all(dev, &rk_rng->clk_bulks);
+       if (rk_rng->clk_num < 0)
+               return dev_err_probe(dev, rk_rng->clk_num,
+                                    "Failed to get clks property\n");
+
+       rk_rng->rst = devm_reset_control_array_get(&pdev->dev, false, false);
+       if (IS_ERR(rk_rng->rst))
+               return dev_err_probe(dev, PTR_ERR(rk_rng->rst),
+                                    "Failed to get reset property\n");
+
+       reset_control_assert(rk_rng->rst);
+       udelay(2);
+       reset_control_deassert(rk_rng->rst);
+
+       platform_set_drvdata(pdev, rk_rng);
+
+       rk_rng->rng.name = dev_driver_string(dev);
+#ifndef CONFIG_PM

drivers/char/hw_random/rockchip-rng.c new file +53 -1

+{
+       pm_runtime_disable(&pdev->dev);
+
+       return 0;
+}
+
+#ifdef CONFIG_PM
+static int rk_rng_runtime_suspend(struct device *dev)
+{
+       struct rk_rng *rk_rng = dev_get_drvdata(dev);
+
+       rk_rng_cleanup(&rk_rng->rng);
+
+       return 0;
+}
+
+static int rk_rng_runtime_resume(struct device *dev)
+{
+       struct rk_rng *rk_rng = dev_get_drvdata(dev);
+
+       return rk_rng_init(&rk_rng->rng);
+}
+#endif
+
+static const struct dev_pm_ops rk_rng_pm_ops = {
+       SET_RUNTIME_PM_OPS(rk_rng_runtime_suspend,
+                               rk_rng_runtime_resume, NULL)
+       SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+                               pm_runtime_force_resume)
+};
+
+static const struct of_device_id rk_rng_dt_match[] = {
+       { .compatible = "rockchip,rk3568-rng", },
+       { /* sentinel */ },
+};
+
+MODULE_DEVICE_TABLE(of, rk_rng_dt_match);
+
+static struct platform_driver rk_rng_driver = {
+       .driver = {
+               .name   = "rockchip-rng",
+               .pm     = &rk_rng_pm_ops,
+               .of_match_table = rk_rng_dt_match,
+       },
+       .probe  = rk_rng_probe,
+       .remove = rk_rng_remove,
+};
+
+module_platform_driver(rk_rng_driver);
+
+MODULE_DESCRIPTION("Rockchip True Random Number Generator driver");
+MODULE_AUTHOR("Lin Jinhan <[email protected]>, Aurelien Jarno <[email protected]>");
+MODULE_LICENSE("GPL");
--

2.45.2

From: Aurelien Jarno <[email protected]>

Rockchip SoCs used to have a random number generator as part of their crypto device, and support for it has to be added to the corresponding driver. However newer Rockchip SoCs like the RK356x have an independent True Random Number Generator device. This patch adds a driver for it, greatly inspired from the downstream driver.

The TRNG device does not seem to have a signal conditionner and the FIPS 140-2 test returns a lot of failures. They can be reduced by increasing RK_RNG_SAMPLE_CNT, in a tradeoff between quality and speed. This value has been adjusted to get ~90% of successes and the quality value has been set accordingly.

Signed-off-by Aurelien Jarno <[email protected]>

[[email protected]: code style fixes]

Signed-off-by Daniel Golle <[email protected]>

---
 MAINTAINERS                           |   1 +
 drivers/char/hw_random/Kconfig        |  14 ++
 drivers/char/hw_random/Makefile       |   1 +
 drivers/char/hw_random/rockchip-rng.c | 229 ++++++++++++++++++++++++++
 4 files changed, 245 insertions(+)
 create mode 100644 drivers/char/hw_random/rockchip-rng.c

drivers/char/hw_random/rockchip-rng.c new file +0 -0

new file mode 100644
index 000000000000..6070abb73847
@@ -0,0 +1,229 @@

drivers/char/hw_random/rockchip-rng.c new file +10 -0

+static int rk_rng_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rk_rng *rk_rng;
+	int ret;
+
+	rk_rng = devm_kzalloc(dev, sizeof(*rk_rng), GFP_KERNEL);
+	if (!rk_rng)
+		return -ENOMEM;
+

(quoted diff) +0 -0

b/arch/arm64/boot/dts/rockchip/rk356x.dtsi index d8543b5557ee..57c8103500ea 100644

arch/arm64/boot/dts/rockchip/rk356x.dtsi +3 -0

@@ -1855,6 +1855,15 @@ usb2phy1_otg: otg-port {
                };
        };
 
+       rng: rng@fe388000 {
+               compatible = "rockchip,rk3568-rng";
+               reg = <0x0 0xfe388000 0x0 0x4000>;

Quoted 9 lines

(quoted diff) +0 -0

b/arch/arm64/boot/dts/rockchip/rk356x.dtsi index d8543b5557ee..57c8103500ea 100644

arch/arm64/boot/dts/rockchip/rk356x.dtsi +0 -0

@@ -1855,6 +1855,15 @@ usb2phy1_otg: otg-port {
                };
        };
 

arch/arm64/boot/dts/rockchip/rk356x.dtsi usb2phy1_otg: otg-port {

Quoted 13 lines

arch/arm64/boot/dts/rockchip/rk356x.dtsi +9 -1

+       rng: rng@fe388000 {
+               compatible = "rockchip,rk3568-rng";
+               reg = <0x0 0xfe388000 0x0 0x4000>;
+               clocks = <&cru CLK_TRNG_NS>, <&cru HCLK_TRNG_NS>;
+               clock-names = "core", "ahb";
+               resets = <&cru SRST_TRNG_NS>;
+               reset-names = "reset";
+       };
+
        pinctrl: pinctrl {
                compatible = "rockchip,rk3568-pinctrl";
                rockchip,grf = <&grf>;
--

I had placed the node between ``sdhci: mmc@fe310000`` and ``i2s0_8ch: i2s@fe400000`` which I think is the proper order.

Rockchip SoCs used to have a random number generator as part of their crypto device, and support for it has to be added to the corresponding driver.

However newer Rockchip SoCs like the RK3568 have an independent True Random Number Generator device. This patchset adds a driver for it and enable it in the device tree.

v2 of this patchset has been submitted by Aurelien Jarno in November 2022. A follow-up submission addressing the comments received for v2 never happened.