[PATCH 0/7] RK3588 Hardware Random Number Generator Driver

[PATCH 0/7] RK3588 Hardware Random Number Generator Driver

[Nicolas Frattaroli]

This series adds support for the Rockchip RK3588's standalone hardware
random number generator to the existing mainline rockchip-rng driver.

The RK3588 has several hardware random number generators, one in each
the secure-world and non-secure-world crypto accelerator, and one
standalone one in each the secure-world and non-secure-world, so 4
hwrngs in total. This series adds support for the standalone hwrng,
which is an entirely new IP on this SoC and distinct from the one in the
Crypto IP.

The decision to integrate this into the existing rockchip-rng driver was
made based on a few factors:

1. The driver is fairly small.
2. While not much code is shared, some code is, specifically relating to
   power management, the hwrng interface and the probe function.
3. I don't want users to figure out why "CONFIG_HW_RANDOM_ROCKCHIP"
   doesn't enable the RK3588 one, and I really don't see a reason to
   build without both of them considering the other RK3588 TRNG (for
   which there is not yet a driver iirc) *does* share code with the
   existing rockchip-rng driver.

Here are the rngtest5 results from this new driver on my board:

  user@debian-rockchip-rock5b-rk3588:~$ cat /sys/class/misc/hw_random/rng_current 
  rockchip-rng
  user@debian-rockchip-rock5b-rk3588:~$ sudo cat /dev/hwrng | rngtest -c 10000 
  [...]
  rngtest: bits received from input: 200000032
  rngtest: FIPS 140-2 successes: 9990
  rngtest: FIPS 140-2 failures: 10
  rngtest: FIPS 140-2(2001-10-10) Monobit: 1
  rngtest: FIPS 140-2(2001-10-10) Poker: 0
  rngtest: FIPS 140-2(2001-10-10) Runs: 5
  rngtest: FIPS 140-2(2001-10-10) Long run: 4
  rngtest: FIPS 140-2(2001-10-10) Continuous run: 0
  rngtest: input channel speed: (min=212.255; avg=29089.272; max=19531250.000)Kibits/s
  rngtest: FIPS tests speed: (min=64.005; avg=102.494; max=153.818)Mibits/s
  rngtest: Program run time: 11292340 microseconds

As you can see, the quality of the entropy is quite good, and the
throughput is an acceptable 29 Mibit/s.

The series starts out with two patches for the bindings. The bindings
are separate from the rockchip,rk3568-rng bindings, as the required
properties differ. The SCMI reset ID numbers are needed because mainline
uses a different reset numbering scheme, but TF-A uses the downstream
numbering scheme as far as I know. The TRNG must be reset through SCMI.

Next up are two cleanup patches for the existing driver. Even if a
decision is made to split the drivers for whatever reason, these two
patches should be used in the rk3568-rng driver as they get rid of small
peculiarities in the code without meaningfully changing how the driver
works.

Next up is the main driver patch that adds support for the new TRNG. As
the driver was developed by reading the downstream vendor code for this
particular device and reworking it, I've included the downstream vendor
developer who wrote the driver as a Co-developed-by tag with their
existing downstream sign-off.

The penultimate patch adds the node to the rk3588-base.dtsi, and enables
it in the one device I've been testing it on (RADXA Rock 5B). Other
devices may wish to enable it as well after verifying that the TRNG
produces randomness of the expected quality on their particular flavour
of the RK3588.* family.

Last but not least, I'm adding myself to the mailmap, because I now work
for Collabora.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
Nicolas Frattaroli (7):
      dt-bindings: reset: Add SCMI reset IDs for RK3588
      dt-bindings: rng: add binding for Rockchip RK3588 RNG
      hwrng: rockchip: store dev pointer in driver struct
      hwrng: rockchip: eliminate some unnecessary dereferences
      hwrng: rockchip: add support for rk3588's standalone TRNG
      arm64: dts: rockchip: Add rng node to RK3588
      mailmap: add entry for Nicolas Frattaroli

 .mailmap                                           |   1 +
 .../bindings/rng/rockchip,rk3588-rng.yaml          |  61 +++++
 MAINTAINERS                                        |   2 +
 arch/arm64/boot/dts/rockchip/rk3588-base.dtsi      |   9 +
 arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts    |   4 +
 drivers/char/hw_random/Kconfig                     |   3 +-
 drivers/char/hw_random/rockchip-rng.c              | 264 ++++++++++++++++++---
 include/dt-bindings/reset/rockchip,rk3588-cru.h    |  41 +++-
 8 files changed, 349 insertions(+), 36 deletions(-)
---
base-commit: 72deda0abee6e705ae71a93f69f55e33be5bca5c
change-id: 20250130-rk3588-trng-submission-055255c96689

Best regards,
-- 
Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

[PATCH 1/7] dt-bindings: reset: Add SCMI reset IDs for RK3588

[Nicolas Frattaroli]

When TF-A is used to assert/deassert the resets through SCMI, the
IDs communicated to it are different than the ones mainline Linux uses.

Import the list of SCMI reset IDs from mainline TF-A so that devicetrees
can use these IDs more easily.

Co-developed-by: XiaoDong Huang <derrick.huang@rock-chips.com>
Signed-off-by: XiaoDong Huang <derrick.huang@rock-chips.com>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 include/dt-bindings/reset/rockchip,rk3588-cru.h | 41 ++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/include/dt-bindings/reset/rockchip,rk3588-cru.h b/include/dt-bindings/reset/rockchip,rk3588-cru.h
index e2fe4bd5f7f01569c804ae4ea87c7cf0433d2ae7..878beae6dc3baaa9a2eb46f7a81454d360968754 100644
--- a/include/dt-bindings/reset/rockchip,rk3588-cru.h
+++ b/include/dt-bindings/reset/rockchip,rk3588-cru.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
 /*
- * Copyright (c) 2021 Rockchip Electronics Co. Ltd.
+ * Copyright (c) 2021, 2024 Rockchip Electronics Co. Ltd.
  * Copyright (c) 2022 Collabora Ltd.
  *
  * Author: Elaine Zhang <zhangqing@rock-chips.com>
@@ -753,4 +753,43 @@
 
 #define SRST_A_HDMIRX_BIU		660
 
+/* SCMI Secure Resets */
+
+/* Name=SECURE_SOFTRST_CON00,Offset=0xA00 */
+#define SCMI_SRST_A_SECURE_NS_BIU	10
+#define SCMI_SRST_H_SECURE_NS_BIU	11
+#define SCMI_SRST_A_SECURE_S_BIU	12
+#define SCMI_SRST_H_SECURE_S_BIU	13
+#define SCMI_SRST_P_SECURE_S_BIU	14
+#define SCMI_SRST_CRYPTO_CORE		15
+/* Name=SECURE_SOFTRST_CON01,Offset=0xA04 */
+#define SCMI_SRST_CRYPTO_PKA		16
+#define SCMI_SRST_CRYPTO_RNG		17
+#define SCMI_SRST_A_CRYPTO		18
+#define SCMI_SRST_H_CRYPTO		19
+#define SCMI_SRST_KEYLADDER_CORE	25
+#define SCMI_SRST_KEYLADDER_RNG		26
+#define SCMI_SRST_A_KEYLADDER		27
+#define SCMI_SRST_H_KEYLADDER		28
+#define SCMI_SRST_P_OTPC_S		29
+#define SCMI_SRST_OTPC_S		30
+#define SCMI_SRST_WDT_S			31
+/* Name=SECURE_SOFTRST_CON02,Offset=0xA08 */
+#define SCMI_SRST_T_WDT_S		32
+#define SCMI_SRST_H_BOOTROM		33
+#define SCMI_SRST_A_DCF			34
+#define SCMI_SRST_P_DCF			35
+#define SCMI_SRST_H_BOOTROM_NS		37
+#define SCMI_SRST_P_KEYLADDER		46
+#define SCMI_SRST_H_TRNG_S		47
+/* Name=SECURE_SOFTRST_CON03,Offset=0xA0C */
+#define SCMI_SRST_H_TRNG_NS		48
+#define SCMI_SRST_D_SDMMC_BUFFER	49
+#define SCMI_SRST_H_SDMMC		50
+#define SCMI_SRST_H_SDMMC_BUFFER	51
+#define SCMI_SRST_SDMMC			52
+#define SCMI_SRST_P_TRNG_CHK		53
+#define SCMI_SRST_TRNG_S		54
+
+
 #endif

-- 
2.48.1

[PATCH 2/7] dt-bindings: rng: add binding for Rockchip RK3588 RNG

[Nicolas Frattaroli]

The Rockchip RK3588 SoC has two hardware RNGs accessible to the
non-secure world: an RNG in the Crypto IP, and a standalone RNG that is
new to this SoC.

Add a binding for this new standalone RNG.

The RNG is capable of firing an interrupt when entropy is ready, but
all known driver implementations choose to poll instead for performance
reasons. Hence, make the interrupt optional, as it may disappear in
future hardware revisions entirely and certainly isn't needed for the
hardware to function.

The reset is optional as well, as the RNG functions without an explicit
reset. Rockchip's downstream driver does not use the reset at all,
indicating that their engineers have deemed it unnecessary.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 .../bindings/rng/rockchip,rk3588-rng.yaml          | 61 ++++++++++++++++++++++
 MAINTAINERS                                        |  2 +
 2 files changed, 63 insertions(+)

diff --git a/Documentation/devicetree/bindings/rng/rockchip,rk3588-rng.yaml b/Documentation/devicetree/bindings/rng/rockchip,rk3588-rng.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..dff843fa4bf9d5704bbcd106398328588d80b02d
--- /dev/null
+++ b/Documentation/devicetree/bindings/rng/rockchip,rk3588-rng.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rng/rockchip,rk3588-rng.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip RK3588 TRNG
+
+description: True Random Number Generator on Rockchip RK3588 SoC
+
+maintainers:
+  - Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
+
+properties:
+  compatible:
+    enum:
+      - rockchip,rk3588-rng
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: TRNG AHB clock
+
+  # Optional, not used by some driver implementations
+  interrupts:
+    maxItems: 1
+
+  # Optional, hardware works without explicit reset
+  resets:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/rockchip,rk3588-cru.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/reset/rockchip,rk3588-cru.h>
+    bus {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      rng@fe378000 {
+        compatible = "rockchip,rk3588-rng";
+        reg = <0x0 0xfe378000 0x0 0x200>;
+        interrupts = <GIC_SPI 400 IRQ_TYPE_LEVEL_HIGH 0>;
+        clocks = <&scmi_clk SCMI_HCLK_SECURE_NS>;
+        resets = <&scmi_reset SCMI_SRST_H_TRNG_NS>;
+        status = "disabled";
+      };
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index bc8ce7af3303f747e0ef028e5a7b29b0bbba99f4..7daf9bfeb0cb4e9e594b809012c7aa243b0558ae 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20420,8 +20420,10 @@ F:	include/uapi/linux/rkisp1-config.h
 ROCKCHIP RK3568 RANDOM NUMBER GENERATOR SUPPORT
 M:	Daniel Golle <daniel@makrotopia.org>
 M:	Aurelien Jarno <aurelien@aurel32.net>
+M:	Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
 S:	Maintained
 F:	Documentation/devicetree/bindings/rng/rockchip,rk3568-rng.yaml
+F:	Documentation/devicetree/bindings/rng/rockchip,rk3588-rng.yaml
 F:	drivers/char/hw_random/rockchip-rng.c
 
 ROCKCHIP RASTER 2D GRAPHIC ACCELERATION UNIT DRIVER

-- 
2.48.1

[PATCH 3/7] hwrng: rockchip: store dev pointer in driver struct

[Nicolas Frattaroli]

The rockchip rng driver does a dance to store the dev pointer in the
hwrng's unsigned long "priv" member. However, since the struct hwrng
member of rk_rng is not a pointer, we can use container_of to get the
struct rk_rng instance from just the struct hwrng*, which means we don't
have to subvert what little there is in C of a type system and can
instead store a pointer to the device struct in the rk_rng itself.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/char/hw_random/rockchip-rng.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/char/hw_random/rockchip-rng.c b/drivers/char/hw_random/rockchip-rng.c
index 289b385bbf05e3c828b6e26feb2453db62f6b2a2..b1b510087e5862b3b2ed97cefbf10620bcf9b020 100644
--- a/drivers/char/hw_random/rockchip-rng.c
+++ b/drivers/char/hw_random/rockchip-rng.c
@@ -54,6 +54,7 @@ struct rk_rng {
 	void __iomem *base;
 	int clk_num;
 	struct clk_bulk_data *clk_bulks;
+	struct device *dev;
 };
 
 /* The mask in the upper 16 bits determines the bits that are updated */
@@ -70,8 +71,7 @@ static int rk_rng_init(struct hwrng *rng)
 	/* 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);
+		dev_err(rk_rng->dev, "Failed to enable clocks: %d\n", ret);
 		return ret;
 	}
 
@@ -105,7 +105,7 @@ static int rk_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
 	u32 reg;
 	int ret = 0;
 
-	ret = pm_runtime_resume_and_get((struct device *) rk_rng->rng.priv);
+	ret = pm_runtime_resume_and_get(rk_rng->dev);
 	if (ret < 0)
 		return ret;
 
@@ -122,8 +122,8 @@ static int rk_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
 	/* 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);
+	pm_runtime_mark_last_busy(rk_rng->dev);
+	pm_runtime_put_sync_autosuspend(rk_rng->dev);
 
 	return (ret < 0) ? ret : to_read;
 }
@@ -164,7 +164,7 @@ static int rk_rng_probe(struct platform_device *pdev)
 		rk_rng->rng.cleanup = rk_rng_cleanup;
 	}
 	rk_rng->rng.read = rk_rng_read;
-	rk_rng->rng.priv = (unsigned long) dev;
+	rk_rng->dev = dev;
 	rk_rng->rng.quality = 900;
 
 	pm_runtime_set_autosuspend_delay(dev, RK_RNG_AUTOSUSPEND_DELAY);

-- 
2.48.1

[PATCH 4/7] hwrng: rockchip: eliminate some unnecessary dereferences

[Nicolas Frattaroli]

Despite assigning a temporary variable the value of &pdev->dev early on
in the probe function, the probe function then continues to use this
construct when it could just use the local dev variable instead.

Simplify this by using the local dev variable directly.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/char/hw_random/rockchip-rng.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/char/hw_random/rockchip-rng.c b/drivers/char/hw_random/rockchip-rng.c
index b1b510087e5862b3b2ed97cefbf10620bcf9b020..082daea27e937e147195070454f9511a71c8c67e 100644
--- a/drivers/char/hw_random/rockchip-rng.c
+++ b/drivers/char/hw_random/rockchip-rng.c
@@ -148,7 +148,7 @@ static int rk_rng_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, rk_rng->clk_num,
 				     "Failed to get clks property\n");
 
-	rst = devm_reset_control_array_get_exclusive(&pdev->dev);
+	rst = devm_reset_control_array_get_exclusive(dev);
 	if (IS_ERR(rst))
 		return dev_err_probe(dev, PTR_ERR(rst), "Failed to get reset property\n");
 
@@ -171,11 +171,11 @@ static int rk_rng_probe(struct platform_device *pdev)
 	pm_runtime_use_autosuspend(dev);
 	ret = devm_pm_runtime_enable(dev);
 	if (ret)
-		return dev_err_probe(&pdev->dev, ret, "Runtime pm activation failed.\n");
+		return dev_err_probe(dev, ret, "Runtime pm activation failed.\n");
 
 	ret = devm_hwrng_register(dev, &rk_rng->rng);
 	if (ret)
-		return dev_err_probe(&pdev->dev, ret, "Failed to register Rockchip hwrng\n");
+		return dev_err_probe(dev, ret, "Failed to register Rockchip hwrng\n");
 
 	return 0;
 }

-- 
2.48.1

[PATCH 5/7] hwrng: rockchip: add support for rk3588's standalone TRNG

[Nicolas Frattaroli]

The RK3588 SoC includes several TRNGs, one part of the Crypto IP block,
and the other one (referred to as "trngv1") as a standalone new IP.

Add support for this new standalone TRNG to the driver by both
generalising it to support multiple different rockchip RNGs and then
implementing the required functionality for the new hardware.

This work was partly based on the downstream vendor driver by Rockchip's
Lin Jinhan, which is why they are listed as a Co-author.

While the hardware does support notifying the CPU with an IRQ when the
random data is ready, I've discovered while implementing the code to use
this interrupt that this results in significantly slower throughput of
the TRNG even when under heavy CPU load. I assume this is because with
only 32 bytes of data per invocation, the overhead of reinitialising a
completion, enabling the interrupt, sleeping and then triggering the
completion in the IRQ handler is way more expensive than busylooping.

Speaking of busylooping, the poll interval for reading the ISTAT is an
atomic read with a delay of 0. In my testing, I've found that this gives
us the largest throughput, and it appears the random data is ready
pretty much the moment we begin polling, as increasing the poll delay
leads to a drop in throughput significant enough to not just be due to
the poll interval missing the ideal timing by a microsecond or two.

According to downstream, the IP should take 1024 clock cycles to
generate 56 bits of random data, which at 150MHz should work out to
6.8us. I did not test whether the data really does take 256/56*6.8us
to arrive, though changing the readl to a __raw_readl makes no
difference in throughput, and this data does pass the rngtest FIPS
checks, so I'm not entirely sure what's going on but I presume it's got
something to do with the AHB bus speed and the memory barriers that
mainline's readl/writel functions insert.

The only other current SoC that uses this new IP is the Rockchip RV1106,
but that SoC does not have mainline support as of the time of writing,
so we make no effort to declare it as supported for now.

Co-developed-by: Lin Jinhan <troy.lin@rock-chips.com>
Signed-off-by: Lin Jinhan <troy.lin@rock-chips.com>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/char/hw_random/Kconfig        |   3 +-
 drivers/char/hw_random/rockchip-rng.c | 248 ++++++++++++++++++++++++++++++----
 2 files changed, 224 insertions(+), 27 deletions(-)

diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index 17854f052386efa0cf3e4c83d60dd9d6d64755ea..6abf832abeba372d2152bd4905447f4b8fbfb6a0 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -606,7 +606,8 @@ config HW_RANDOM_ROCKCHIP
 	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.
+	  Generator hardware found on some Rockchip SoCs like RK3566, RK3568
+	  or RK3588.
 
 	  To compile this driver as a module, choose M here: the
 	  module will be called rockchip-rng.
diff --git a/drivers/char/hw_random/rockchip-rng.c b/drivers/char/hw_random/rockchip-rng.c
index 082daea27e937e147195070454f9511a71c8c67e..f0c8a09a67406726a43866001ed09515eeeba03e 100644
--- a/drivers/char/hw_random/rockchip-rng.c
+++ b/drivers/char/hw_random/rockchip-rng.c
@@ -1,12 +1,14 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * rockchip-rng.c True Random Number Generator driver for Rockchip RK3568 SoC
+ * rockchip-rng.c True Random Number Generator driver for Rockchip SoCs
  *
  * Copyright (c) 2018, Fuzhou Rockchip Electronics Co., Ltd.
  * Copyright (c) 2022, Aurelien Jarno
+ * Copyright (c) 2025, Collabora Ltd.
  * Authors:
  *  Lin Jinhan <troy.lin@rock-chips.com>
  *  Aurelien Jarno <aurelien@aurel32.net>
+ *  Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
  */
 #include <linux/clk.h>
 #include <linux/hw_random.h>
@@ -32,6 +34,9 @@
  */
 #define RK_RNG_SAMPLE_CNT		1000
 
+/* after how many bytes of output TRNGv1 implementations should be reseeded */
+#define RK_TRNG_V1_AUTO_RESEED_CNT	16000
+
 /* TRNG registers from RK3568 TRM-Part2, section 5.4.1 */
 #define TRNG_RST_CTL			0x0004
 #define TRNG_RNG_CTL			0x0400
@@ -49,25 +54,85 @@
 #define TRNG_RNG_SAMPLE_CNT		0x0404
 #define TRNG_RNG_DOUT			0x0410
 
+/*
+ * TRNG V1 register definitions
+ * The TRNG V1 IP is a stand-alone TRNG implementation (not part of a crypto IP)
+ * and can be found in the Rockchip RK3588 SoC
+ */
+#define TRNG_V1_CTRL				0x0000
+#define TRNG_V1_CTRL_NOP			0x00
+#define TRNG_V1_CTRL_RAND			0x01
+#define TRNG_V1_CTRL_SEED			0x02
+
+#define TRNG_V1_STAT				0x0004
+#define TRNG_V1_STAT_SEEDED			BIT(9)
+#define TRNG_V1_STAT_GENERATING			BIT(30)
+#define TRNG_V1_STAT_RESEEDING			BIT(31)
+
+#define TRNG_V1_MODE				0x0008
+#define TRNG_V1_MODE_128_BIT			(0x00 << 3)
+#define TRNG_V1_MODE_256_BIT			(0x01 << 3)
+
+/* Interrupt Enable register; unused because polling is faster */
+#define TRNG_V1_IE				0x0010
+#define TRNG_V1_IE_GLBL_EN			BIT(31)
+#define TRNG_V1_IE_SEED_DONE_EN			BIT(1)
+#define TRNG_V1_IE_RAND_RDY_EN			BIT(0)
+
+#define TRNG_V1_ISTAT				0x0014
+#define TRNG_V1_ISTAT_RAND_RDY			BIT(0)
+
+/* RAND0 ~ RAND7 */
+#define TRNG_V1_RAND0				0x0020
+#define TRNG_V1_RAND7				0x003C
+
+/* Auto Reseed Register */
+#define TRNG_V1_AUTO_RQSTS			0x0060
+
+#define TRNG_V1_VERSION				0x00F0
+#define TRNG_v1_VERSION_CODE			0x46bc
+/* end of TRNG_V1 register definitions */
+
+/* Before removing this assert, give rk3588_rng_read an upper bound of 32 */
+static_assert(RK_RNG_MAX_BYTE <= (TRNG_V1_RAND7 + 4 - TRNG_V1_RAND0),
+	      "You raised RK_RNG_MAX_BYTE and broke rk3588-rng, congrats.");
+
 struct rk_rng {
 	struct hwrng rng;
 	void __iomem *base;
 	int clk_num;
 	struct clk_bulk_data *clk_bulks;
+	struct rk_rng_soc_data	*soc_data;
 	struct device *dev;
 };
 
+struct rk_rng_soc_data {
+	int (*rk_rng_init)(struct hwrng *rng);
+	int (*rk_rng_read)(struct hwrng *rng, void *buf, size_t max, bool wait);
+	void (*rk_rng_cleanup)(struct hwrng *rng);
+	unsigned short quality;
+	bool reset_optional;
+};
+
 /* The mask in the upper 16 bits determines the bits that are updated */
 static void rk_rng_write_ctl(struct rk_rng *rng, u32 val, u32 mask)
 {
 	writel((mask << 16) | val, rng->base + TRNG_RNG_CTL);
 }
 
-static int rk_rng_init(struct hwrng *rng)
+static inline void rk_rng_writel(struct rk_rng *rng, u32 val, u32 offset)
 {
-	struct rk_rng *rk_rng = container_of(rng, struct rk_rng, rng);
-	int ret;
+	writel(val, rng->base + offset);
+}
+
+static inline u32 rk_rng_readl(struct rk_rng *rng, u32 offset)
+{
+	return readl(rng->base + offset);
+}
 
+static int rk_rng_enable_clks(struct rk_rng *rk_rng)
+{
+	int ret;
 	/* start clocks */
 	ret = clk_bulk_prepare_enable(rk_rng->clk_num, rk_rng->clk_bulks);
 	if (ret < 0) {
@@ -75,6 +140,18 @@ static int rk_rng_init(struct hwrng *rng)
 		return ret;
 	}
 
+	return 0;
+}
+
+static int rk3568_rng_init(struct hwrng *rng)
+{
+	struct rk_rng *rk_rng = container_of(rng, struct rk_rng, rng);
+	int ret;
+
+	ret = rk_rng_enable_clks(rk_rng);
+	if (ret < 0)
+		return ret;
+
 	/* set the sample period */
 	writel(RK_RNG_SAMPLE_CNT, rk_rng->base + TRNG_RNG_SAMPLE_CNT);
 
@@ -87,7 +164,7 @@ static int rk_rng_init(struct hwrng *rng)
 	return 0;
 }
 
-static void rk_rng_cleanup(struct hwrng *rng)
+static void rk3568_rng_cleanup(struct hwrng *rng)
 {
 	struct rk_rng *rk_rng = container_of(rng, struct rk_rng, rng);
 
@@ -98,7 +175,7 @@ static void rk_rng_cleanup(struct hwrng *rng)
 	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)
+static int rk3568_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);
@@ -128,9 +205,126 @@ static int rk_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
 	return (ret < 0) ? ret : to_read;
 }
 
+static int rk3588_rng_init(struct hwrng *rng)
+{
+	struct rk_rng *rk_rng = container_of(rng, struct rk_rng, rng);
+	u32 version, status, mask, istat;
+	int ret;
+
+	ret = rk_rng_enable_clks(rk_rng);
+	if (ret < 0)
+		return ret;
+
+	version = rk_rng_readl(rk_rng, TRNG_V1_VERSION);
+	if (version != TRNG_v1_VERSION_CODE) {
+		dev_err(rk_rng->dev,
+			"wrong trng version, expected = %08x, actual = %08x\n",
+			TRNG_V1_VERSION, version);
+		ret = -EFAULT;
+		goto err_disable_clk;
+	}
+
+	mask = TRNG_V1_STAT_SEEDED | TRNG_V1_STAT_GENERATING |
+	       TRNG_V1_STAT_RESEEDING;
+	if (readl_poll_timeout(rk_rng->base + TRNG_V1_STAT, status,
+			       (status & mask) == TRNG_V1_STAT_SEEDED,
+			       RK_RNG_POLL_PERIOD_US, RK_RNG_POLL_TIMEOUT_US) < 0) {
+		dev_err(rk_rng->dev, "timed out waiting for hwrng to reseed\n");
+		ret = -ETIMEDOUT;
+		goto err_disable_clk;
+	}
+
+	/*
+	 * clear ISTAT flag, downstream advises to do this to avoid
+	 * auto-reseeding "on power on"
+	 */
+	istat = rk_rng_readl(rk_rng, TRNG_V1_ISTAT);
+	rk_rng_writel(rk_rng, istat, TRNG_V1_ISTAT);
+
+	/* auto reseed after RK_TRNG_V1_AUTO_RESEED_CNT bytes */
+	rk_rng_writel(rk_rng, RK_TRNG_V1_AUTO_RESEED_CNT / 16, TRNG_V1_AUTO_RQSTS);
+
+	return 0;
+err_disable_clk:
+	clk_bulk_disable_unprepare(rk_rng->clk_num, rk_rng->clk_bulks);
+	return ret;
+}
+
+static void rk3588_rng_cleanup(struct hwrng *rng)
+{
+	struct rk_rng *rk_rng = container_of(rng, struct rk_rng, rng);
+
+	clk_bulk_disable_unprepare(rk_rng->clk_num, rk_rng->clk_bulks);
+}
+
+static int rk3588_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);
+	int ret = 0;
+	u32 reg;
+
+	ret = pm_runtime_resume_and_get(rk_rng->dev);
+	if (ret < 0)
+		return ret;
+
+	/* Clear ISTAT, even without interrupts enabled, this will be updated */
+	reg = rk_rng_readl(rk_rng, TRNG_V1_ISTAT);
+	rk_rng_writel(rk_rng, reg, TRNG_V1_ISTAT);
+
+	/* generate 256 bits of random data */
+	rk_rng_writel(rk_rng, TRNG_V1_MODE_256_BIT, TRNG_V1_MODE);
+	rk_rng_writel(rk_rng, TRNG_V1_CTRL_RAND, TRNG_V1_CTRL);
+
+	ret = readl_poll_timeout_atomic(rk_rng->base + TRNG_V1_ISTAT, reg,
+					(reg & TRNG_V1_ISTAT_RAND_RDY), 0,
+					RK_RNG_POLL_TIMEOUT_US);
+	if (ret < 0)
+		goto out;
+
+	/* Read random data that's in registers TRNG_V1_RAND0 through RAND7 */
+	memcpy_fromio(buf, rk_rng->base + TRNG_V1_RAND0, to_read);
+
+out:
+	/* Clear ISTAT */
+	rk_rng_writel(rk_rng, reg, TRNG_V1_ISTAT);
+	/* close the TRNG */
+	rk_rng_writel(rk_rng, TRNG_V1_CTRL_NOP, TRNG_V1_CTRL);
+
+	pm_runtime_mark_last_busy(rk_rng->dev);
+	pm_runtime_put_sync_autosuspend(rk_rng->dev);
+
+	return (ret < 0) ? ret : to_read;
+}
+
+static const struct rk_rng_soc_data rk3568_soc_data = {
+	.rk_rng_init = rk3568_rng_init,
+	.rk_rng_read = rk3568_rng_read,
+	.rk_rng_cleanup = rk3568_rng_cleanup,
+	.quality = 900,
+	.reset_optional = false,
+};
+
+static const struct rk_rng_soc_data rk3588_soc_data = {
+	.rk_rng_init = rk3588_rng_init,
+	.rk_rng_read = rk3588_rng_read,
+	.rk_rng_cleanup = rk3588_rng_cleanup,
+	.quality = 999,		/* as determined by actual testing */
+	.reset_optional = true,
+};
+
+static const struct of_device_id rk_rng_dt_match[] = {
+	{ .compatible = "rockchip,rk3568-rng", .data = (void *)&rk3568_soc_data },
+	{ .compatible = "rockchip,rk3588-rng", .data = (void *)&rk3588_soc_data },
+	{ /* sentinel */ },
+};
+
+MODULE_DEVICE_TABLE(of, rk_rng_dt_match);
+
 static int rk_rng_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	const struct of_device_id *match;
 	struct reset_control *rst;
 	struct rk_rng *rk_rng;
 	int ret;
@@ -139,6 +333,8 @@ static int rk_rng_probe(struct platform_device *pdev)
 	if (!rk_rng)
 		return -ENOMEM;
 
+	match = of_match_node(rk_rng_dt_match, dev->of_node);
+	rk_rng->soc_data = (struct rk_rng_soc_data *)match->data;
 	rk_rng->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(rk_rng->base))
 		return PTR_ERR(rk_rng->base);
@@ -148,24 +344,30 @@ static int rk_rng_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, rk_rng->clk_num,
 				     "Failed to get clks property\n");
 
-	rst = devm_reset_control_array_get_exclusive(dev);
-	if (IS_ERR(rst))
-		return dev_err_probe(dev, PTR_ERR(rst), "Failed to get reset property\n");
+	if (rk_rng->soc_data->reset_optional)
+		rst = devm_reset_control_array_get_optional_exclusive(dev);
+	else
+		rst = devm_reset_control_array_get_exclusive(dev);
+
+	if (rst) {
+		if (IS_ERR(rst))
+			return dev_err_probe(dev, PTR_ERR(rst), "Failed to get reset property\n");
 
-	reset_control_assert(rst);
-	udelay(2);
-	reset_control_deassert(rst);
+		reset_control_assert(rst);
+		udelay(2);
+		reset_control_deassert(rst);
+	}
 
 	platform_set_drvdata(pdev, rk_rng);
 
 	rk_rng->rng.name = dev_driver_string(dev);
 	if (!IS_ENABLED(CONFIG_PM)) {
-		rk_rng->rng.init = rk_rng_init;
-		rk_rng->rng.cleanup = rk_rng_cleanup;
+		rk_rng->rng.init = rk_rng->soc_data->rk_rng_init;
+		rk_rng->rng.cleanup = rk_rng->soc_data->rk_rng_cleanup;
 	}
-	rk_rng->rng.read = rk_rng_read;
+	rk_rng->rng.read = rk_rng->soc_data->rk_rng_read;
 	rk_rng->dev = dev;
-	rk_rng->rng.quality = 900;
+	rk_rng->rng.quality = rk_rng->soc_data->quality;
 
 	pm_runtime_set_autosuspend_delay(dev, RK_RNG_AUTOSUSPEND_DELAY);
 	pm_runtime_use_autosuspend(dev);
@@ -184,7 +386,7 @@ static int __maybe_unused rk_rng_runtime_suspend(struct device *dev)
 {
 	struct rk_rng *rk_rng = dev_get_drvdata(dev);
 
-	rk_rng_cleanup(&rk_rng->rng);
+	rk_rng->soc_data->rk_rng_cleanup(&rk_rng->rng);
 
 	return 0;
 }
@@ -193,7 +395,7 @@ static int __maybe_unused rk_rng_runtime_resume(struct device *dev)
 {
 	struct rk_rng *rk_rng = dev_get_drvdata(dev);
 
-	return rk_rng_init(&rk_rng->rng);
+	return rk_rng->soc_data->rk_rng_init(&rk_rng->rng);
 }
 
 static const struct dev_pm_ops rk_rng_pm_ops = {
@@ -203,13 +405,6 @@ static const struct dev_pm_ops rk_rng_pm_ops = {
 				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",
@@ -221,8 +416,9 @@ static struct platform_driver rk_rng_driver = {
 
 module_platform_driver(rk_rng_driver);
 
-MODULE_DESCRIPTION("Rockchip RK3568 True Random Number Generator driver");
+MODULE_DESCRIPTION("Rockchip True Random Number Generator driver");
 MODULE_AUTHOR("Lin Jinhan <troy.lin@rock-chips.com>");
 MODULE_AUTHOR("Aurelien Jarno <aurelien@aurel32.net>");
 MODULE_AUTHOR("Daniel Golle <daniel@makrotopia.org>");
+MODULE_AUTHOR("Nicolas Frattaroli <nicolas.frattaroli@collabora.com>");
 MODULE_LICENSE("GPL");

-- 
2.48.1

[PATCH 6/7] arm64: dts: rockchip: Add rng node to RK3588

[Nicolas Frattaroli]

Add the RK3588's standalone hardware random number generator node to its
device tree, and enable it on the one board I've tested it on and can
verify it produces good output, the Rock 5B.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 arch/arm64/boot/dts/rockchip/rk3588-base.dtsi   | 9 +++++++++
 arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts | 4 ++++
 2 files changed, 13 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
index 8cfa30837ce72581d0b513a8274ab0177eb5ae15..1c72922bcbe1afd7c49beac771f8b7c6e5cc6e05 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
@@ -1921,6 +1921,15 @@ sdhci: mmc@fe2e0000 {
 		status = "disabled";
 	};
 
+	rng: rng@fe378000 {
+		compatible = "rockchip,rk3588-rng";
+		reg = <0x0 0xfe378000 0x0 0x200>;
+		interrupts = <GIC_SPI 400 IRQ_TYPE_LEVEL_HIGH 0>;
+		clocks = <&scmi_clk SCMI_HCLK_SECURE_NS>;
+		resets = <&scmi_reset SCMI_SRST_H_TRNG_NS>;
+		status = "disabled";
+	};
+
 	i2s0_8ch: i2s@fe470000 {
 		compatible = "rockchip,rk3588-i2s-tdm";
 		reg = <0x0 0xfe470000 0x0 0x1000>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
index d597112f1d5b8ee0b6a4fa17086c8671a5102583..00a915cf266202e26b274cab962f7bf6bcf76fc1 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
@@ -430,6 +430,10 @@ &pwm1 {
 	status = "okay";
 };
 
+&rng {
+	status = "okay";
+};
+
 &saradc {
 	vref-supply = <&avcc_1v8_s0>;
 	status = "okay";

-- 
2.48.1

[PATCH 7/7] mailmap: add entry for Nicolas Frattaroli

[Nicolas Frattaroli]

Add my new Collabora E-Mail address as the main contact.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 .mailmap | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.mailmap b/.mailmap
index 17dd8eb2630e6216336047c36037a509beaa0731..07dc4846c451d0f36e27fe67e224791cd48bfef9 100644
--- a/.mailmap
+++ b/.mailmap
@@ -525,6 +525,7 @@ Nicholas Piggin <npiggin@gmail.com> <npiggin@suse.de>
 Nicholas Piggin <npiggin@gmail.com> <nickpiggin@yahoo.com.au>
 Nicholas Piggin <npiggin@gmail.com> <piggin@cyberone.com.au>
 Nicolas Ferre <nicolas.ferre@microchip.com> <nicolas.ferre@atmel.com>
+Nicolas Frattaroli <nicolas.frattaroli@collabora.com> <frattaroli.nicolas@gmail.com>
 Nicolas Pitre <nico@fluxnic.net> <nicolas.pitre@linaro.org>
 Nicolas Pitre <nico@fluxnic.net> <nico@linaro.org>
 Nicolas Saenz Julienne <nsaenz@kernel.org> <nsaenzjulienne@suse.de>

-- 
2.48.1

Re: [PATCH 2/7] dt-bindings: rng: add binding for Rockchip RK3588 RNG

[Conor Dooley]

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

On Thu, Jan 30, 2025 at 05:31:16PM +0100, Nicolas Frattaroli wrote:
> The Rockchip RK3588 SoC has two hardware RNGs accessible to the
> non-secure world: an RNG in the Crypto IP, and a standalone RNG that is
> new to this SoC.
> 
> Add a binding for this new standalone RNG.
> 
> The RNG is capable of firing an interrupt when entropy is ready, but
> all known driver implementations choose to poll instead for performance
> reasons. Hence, make the interrupt optional, as it may disappear in
> future hardware revisions entirely and certainly isn't needed for the
> hardware to function.
> 
> The reset is optional as well, as the RNG functions without an explicit
> reset. Rockchip's downstream driver does not use the reset at all,
> indicating that their engineers have deemed it unnecessary.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  .../bindings/rng/rockchip,rk3588-rng.yaml          | 61 ++++++++++++++++++++++
>  MAINTAINERS                                        |  2 +
>  2 files changed, 63 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/rng/rockchip,rk3588-rng.yaml b/Documentation/devicetree/bindings/rng/rockchip,rk3588-rng.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..dff843fa4bf9d5704bbcd106398328588d80b02d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rng/rockchip,rk3588-rng.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/rng/rockchip,rk3588-rng.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rockchip RK3588 TRNG
> +
> +description: True Random Number Generator on Rockchip RK3588 SoC
> +
> +maintainers:
> +  - Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - rockchip,rk3588-rng
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: TRNG AHB clock
> +
> +  # Optional, not used by some driver implementations
> +  interrupts:
> +    maxItems: 1
> +
> +  # Optional, hardware works without explicit reset

These sorts of comments are not needed, "required" conveys this
information.

> +  resets:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/rockchip,rk3588-cru.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/reset/rockchip,rk3588-cru.h>
> +    bus {
> +      #address-cells = <2>;
> +      #size-cells = <2>;
> +
> +      rng@fe378000 {
> +        compatible = "rockchip,rk3588-rng";
> +        reg = <0x0 0xfe378000 0x0 0x200>;
> +        interrupts = <GIC_SPI 400 IRQ_TYPE_LEVEL_HIGH 0>;
> +        clocks = <&scmi_clk SCMI_HCLK_SECURE_NS>;
> +        resets = <&scmi_reset SCMI_SRST_H_TRNG_NS>;
> +        status = "disabled";
> +      };
> +    };
> +
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index bc8ce7af3303f747e0ef028e5a7b29b0bbba99f4..7daf9bfeb0cb4e9e594b809012c7aa243b0558ae 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20420,8 +20420,10 @@ F:	include/uapi/linux/rkisp1-config.h
>  ROCKCHIP RK3568 RANDOM NUMBER GENERATOR SUPPORT
>  M:	Daniel Golle <daniel@makrotopia.org>
>  M:	Aurelien Jarno <aurelien@aurel32.net>

> +M:	Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
^^
tbh, not really sure this part of the change should be in this patch

>  S:	Maintained
>  F:	Documentation/devicetree/bindings/rng/rockchip,rk3568-rng.yaml
> +F:	Documentation/devicetree/bindings/rng/rockchip,rk3588-rng.yaml
>  F:	drivers/char/hw_random/rockchip-rng.c
>  
>  ROCKCHIP RASTER 2D GRAPHIC ACCELERATION UNIT DRIVER
> 
> -- 
> 2.48.1
> 

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

Re: [PATCH 1/7] dt-bindings: reset: Add SCMI reset IDs for RK3588

[Conor Dooley]

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

On Thu, Jan 30, 2025 at 05:31:15PM +0100, Nicolas Frattaroli wrote:
> When TF-A is used to assert/deassert the resets through SCMI, the
> IDs communicated to it are different than the ones mainline Linux uses.
> 
> Import the list of SCMI reset IDs from mainline TF-A so that devicetrees
> can use these IDs more easily.
> 
> Co-developed-by: XiaoDong Huang <derrick.huang@rock-chips.com>
> Signed-off-by: XiaoDong Huang <derrick.huang@rock-chips.com>
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

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

Re: [PATCH 2/7] dt-bindings: rng: add binding for Rockchip RK3588 RNG

[Krzysztof Kozlowski]

On 30/01/2025 17:31, Nicolas Frattaroli wrote:
> +title: Rockchip RK3588 TRNG
> +
> +description: True Random Number Generator on Rockchip RK3588 SoC
> +
> +maintainers:
> +  - Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - rockchip,rk3588-rng
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: TRNG AHB clock
> +
> +  # Optional, not used by some driver implementations

What driver implementations? Downstream? They do not matter, because
they are full of all sort of crap.

Can this block have interrupt really disconnected? This is the question
you should answer.


> +  interrupts:
> +    maxItems: 1
> +
> +  # Optional, hardware works without explicit reset

Just because bootloader did something? With that reasoning nothing is
ever required because firmware can abstract it. Either you have there a
reset or not. In this particular case your driver is irrelevant.

> +  resets:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +


BTW, there is a binding for Rockchip TRNG, with a bit different clocks
so I have feeling yours is incomplete here.

> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/rockchip,rk3588-cru.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/reset/rockchip,rk3588-cru.h>
> +    bus {
> +      #address-cells = <2>;
> +      #size-cells = <2>;
> +
> +      rng@fe378000 {
> +        compatible = "rockchip,rk3588-rng";
> +        reg = <0x0 0xfe378000 0x0 0x200>;
> +        interrupts = <GIC_SPI 400 IRQ_TYPE_LEVEL_HIGH 0>;
> +        clocks = <&scmi_clk SCMI_HCLK_SECURE_NS>;
> +        resets = <&scmi_reset SCMI_SRST_H_TRNG_NS>;
> +        status = "disabled";

Examples cannot be disabled.

> +      };
> +    };
> +
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index bc8ce7af3303f747e0ef028e5a7b29b0bbba99f4..7daf9bfeb0cb4e9e594b809012c7aa243b0558ae 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20420,8 +20420,10 @@ F:	include/uapi/linux/rkisp1-config.h
>  ROCKCHIP RK3568 RANDOM NUMBER GENERATOR SUPPORT
>  M:	Daniel Golle <daniel@makrotopia.org>
>  M:	Aurelien Jarno <aurelien@aurel32.net>
> +M:	Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

Like Conor said, this is not really relevant and should be a separate patch.



Best regards,
Krzysztof

Re: [PATCH 5/7] hwrng: rockchip: add support for rk3588's standalone TRNG

[Krzysztof Kozlowski]

On 30/01/2025 17:31, Nicolas Frattaroli wrote:
> +MODULE_DEVICE_TABLE(of, rk_rng_dt_match);
> +
>  static int rk_rng_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> +	const struct of_device_id *match;
>  	struct reset_control *rst;
>  	struct rk_rng *rk_rng;
>  	int ret;
> @@ -139,6 +333,8 @@ static int rk_rng_probe(struct platform_device *pdev)
>  	if (!rk_rng)
>  		return -ENOMEM;
>  
> +	match = of_match_node(rk_rng_dt_match, dev->of_node);
> +	rk_rng->soc_data = (struct rk_rng_soc_data *)match->data;

Don't open code getting match data.

>  	rk_rng->base = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(rk_rng->base))
>  		return PTR_ERR(rk_rng->base);
> @@ -148,24 +344,30 @@ static int rk_rng_probe(struct platform_device *pdev)
>  		return dev_err_probe(dev, rk_rng->clk_num,
>  				     "Failed to get clks property\n");
>  
> -	rst = devm_reset_control_array_get_exclusive(dev);
> -	if (IS_ERR(rst))
> -		return dev_err_probe(dev, PTR_ERR(rst), "Failed to get reset property\n");
> +	if (rk_rng->soc_data->reset_optional)
> +		rst = devm_reset_control_array_get_optional_exclusive(dev);
> +	else
> +		rst = devm_reset_control_array_get_exclusive(dev);
> +
> +	if (rst) {
> +		if (IS_ERR(rst))
> +			return dev_err_probe(dev, PTR_ERR(rst), "Failed to get reset property\n");
>  
> -	reset_control_assert(rst);
> -	udelay(2);
> -	reset_control_deassert(rst);
> +		reset_control_assert(rst);
> +		udelay(2);
> +		reset_control_deassert(rst);
> +	}
>  
>  	platform_set_drvdata(pdev, rk_rng);
>  
>  	rk_rng->rng.name = dev_driver_string(dev);
>  	if (!IS_ENABLED(CONFIG_PM)) {
> -		rk_rng->rng.init = rk_rng_init;
> -		rk_rng->rng.cleanup = rk_rng_cleanup;
> +		rk_rng->rng.init = rk_rng->soc_data->rk_rng_init;
> +		rk_rng->rng.cleanup = rk_rng->soc_data->rk_rng_cleanup;
>  	}
> -	rk_rng->rng.read = rk_rng_read;
> +	rk_rng->rng.read = rk_rng->soc_data->rk_rng_read;
>  	rk_rng->dev = dev;
> -	rk_rng->rng.quality = 900;
> +	rk_rng->rng.quality = rk_rng->soc_data->quality;
>  
>  	pm_runtime_set_autosuspend_delay(dev, RK_RNG_AUTOSUSPEND_DELAY);
>  	pm_runtime_use_autosuspend(dev);
> @@ -184,7 +386,7 @@ static int __maybe_unused rk_rng_runtime_suspend(struct device *dev)
>  {
>  	struct rk_rng *rk_rng = dev_get_drvdata(dev);
>  
> -	rk_rng_cleanup(&rk_rng->rng);
> +	rk_rng->soc_data->rk_rng_cleanup(&rk_rng->rng);
>  
>  	return 0;
>  }
> @@ -193,7 +395,7 @@ static int __maybe_unused rk_rng_runtime_resume(struct device *dev)
>  {
>  	struct rk_rng *rk_rng = dev_get_drvdata(dev);
>  
> -	return rk_rng_init(&rk_rng->rng);
> +	return rk_rng->soc_data->rk_rng_init(&rk_rng->rng);
>  }
>  
>  static const struct dev_pm_ops rk_rng_pm_ops = {
> @@ -203,13 +405,6 @@ static const struct dev_pm_ops rk_rng_pm_ops = {
>  				pm_runtime_force_resume)
>  };
>  
> -static const struct of_device_id rk_rng_dt_match[] = {
> -	{ .compatible = "rockchip,rk3568-rng", },
> -	{ /* sentinel */ },
> -};


No, don't move it. Not necessary and not expected to be in other place.

Best regards,
Krzysztof

Re: [PATCH 7/7] mailmap: add entry for Nicolas Frattaroli

[Krzysztof Kozlowski]

On 30/01/2025 17:31, Nicolas Frattaroli wrote:
> Add my new Collabora E-Mail address as the main contact.

Not sure if you understand the mailmap. The point is to map invalid or
unused addresses to an used one. If you leave Collabora, you will need
to change this line which proves the point that it is not a correct mapping.

> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  .mailmap | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/.mailmap b/.mailmap
> index 17dd8eb2630e6216336047c36037a509beaa0731..07dc4846c451d0f36e27fe67e224791cd48bfef9 100644
> --- a/.mailmap
> +++ b/.mailmap
> @@ -525,6 +525,7 @@ Nicholas Piggin <npiggin@gmail.com> <npiggin@suse.de>
>  Nicholas Piggin <npiggin@gmail.com> <nickpiggin@yahoo.com.au>
>  Nicholas Piggin <npiggin@gmail.com> <piggin@cyberone.com.au>
>  Nicolas Ferre <nicolas.ferre@microchip.com> <nicolas.ferre@atmel.com>
> +Nicolas Frattaroli <nicolas.frattaroli@collabora.com> <frattaroli.nicolas@gmail.com>
Best regards,
Krzysztof

Re: [PATCH 2/7] dt-bindings: rng: add binding for Rockchip RK3588 RNG

[Nicolas Frattaroli]

On Friday 31 January 2025 09:30:51 Central European Standard Time Krzysztof 
Kozlowski wrote:
> On 30/01/2025 17:31, Nicolas Frattaroli wrote:
> > +title: Rockchip RK3588 TRNG
> > +
> > +description: True Random Number Generator on Rockchip RK3588 SoC
> > +
> > +maintainers:
> > +  - Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - rockchip,rk3588-rng
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: TRNG AHB clock
> > +
> > +  # Optional, not used by some driver implementations
> 
> What driver implementations? Downstream? They do not matter, because
> they are full of all sort of crap.

Downstream and this very driver in the series. The patch includes a lengthy 
explanation of why all known implementations have foregone using the 
interrupt.

> Can this block have interrupt really disconnected? This is the question
> you should answer.

I do not have the gift of prescience. If you want me to make the interrupt 
required even though it is pointless just because all hardware that implements 
this that I'm currently aware of does have the interrupt then I will add it as 
a required property.
 
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  # Optional, hardware works without explicit reset
> 
> Just because bootloader did something? With that reasoning nothing is
> ever required because firmware can abstract it. Either you have there a
> reset or not. In this particular case your driver is irrelevant.

No, it's not the bootloader that does it, it's the hardware itself. It's 
power-on reset. Hardware can reset itself without software, I don't know where 
the assumption of it being done by the bootloader comes from.

> > +  resets:
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +
> 
> BTW, there is a binding for Rockchip TRNG, with a bit different clocks
> so I have feeling yours is incomplete here.

I know and they're not. As the cover letter states, this is different hardware 
from the other Rockchip TRNG. This hardware only has one clock. It is a 
separate binding with different clocks because it is different hardware.

The patch's message also states this:

> [...] and a standalone RNG that is new to this SoC.
> 
> Add a binding for this new standalone RNG.

It's a standalone RNG that is new to this SoC. It has different registers, and 
different clock requirements.

> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/rockchip,rk3588-cru.h>
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    #include <dt-bindings/reset/rockchip,rk3588-cru.h>
> > +    bus {
> > +      #address-cells = <2>;
> > +      #size-cells = <2>;
> > +
> > +      rng@fe378000 {
> > +        compatible = "rockchip,rk3588-rng";
> > +        reg = <0x0 0xfe378000 0x0 0x200>;
> > +        interrupts = <GIC_SPI 400 IRQ_TYPE_LEVEL_HIGH 0>;
> > +        clocks = <&scmi_clk SCMI_HCLK_SECURE_NS>;
> > +        resets = <&scmi_reset SCMI_SRST_H_TRNG_NS>;
> > +        status = "disabled";
> 
> Examples cannot be disabled.

Okay, I will fix this.

> > +      };
> > +    };
> > +
> > +...
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index
> > bc8ce7af3303f747e0ef028e5a7b29b0bbba99f4..7daf9bfeb0cb4e9e594b809012c7aa2
> > 43b0558ae 100644 --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -20420,8 +20420,10 @@ F:	include/uapi/linux/rkisp1-config.h
> > 
> >  ROCKCHIP RK3568 RANDOM NUMBER GENERATOR SUPPORT
> >  M:	Daniel Golle <daniel@makrotopia.org>
> >  M:	Aurelien Jarno <aurelien@aurel32.net>
> > 
> > +M:	Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> 
> Like Conor said, this is not really relevant and should be a separate patch.

Should just the added M be a separate patch or also adding the file to the 
MAINTAINERS file? Because if the latter is separate then I think checkpatch 
will yell at me, and if the former is separate then the binding's statement 
that it is maintained by me is disconnected from what's in the MAINTAINERS 
file.

> 
> 
> 
> Best regards,
> Krzysztof

Re: [PATCH 6/7] arm64: dts: rockchip: Add rng node to RK3588

[Krzysztof Kozlowski]

On 30/01/2025 17:31, Nicolas Frattaroli wrote:
> Add the RK3588's standalone hardware random number generator node to its
> device tree, and enable it on the one board I've tested it on and can
> verify it produces good output, the Rock 5B.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  arch/arm64/boot/dts/rockchip/rk3588-base.dtsi   | 9 +++++++++
>  arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts | 4 ++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
> index 8cfa30837ce72581d0b513a8274ab0177eb5ae15..1c72922bcbe1afd7c49beac771f8b7c6e5cc6e05 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
> @@ -1921,6 +1921,15 @@ sdhci: mmc@fe2e0000 {
>  		status = "disabled";
>  	};
>  
> +	rng: rng@fe378000 {
> +		compatible = "rockchip,rk3588-rng";
> +		reg = <0x0 0xfe378000 0x0 0x200>;
> +		interrupts = <GIC_SPI 400 IRQ_TYPE_LEVEL_HIGH 0>;
> +		clocks = <&scmi_clk SCMI_HCLK_SECURE_NS>;
> +		resets = <&scmi_reset SCMI_SRST_H_TRNG_NS>;
> +		status = "disabled";

Why? What resources are missing?

> +	};
> +
>  	i2s0_8ch: i2s@fe470000 {
>  		compatible = "rockchip,rk3588-i2s-tdm";
>  		reg = <0x0 0xfe470000 0x0 0x1000>;
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
> index d597112f1d5b8ee0b6a4fa17086c8671a5102583..00a915cf266202e26b274cab962f7bf6bcf76fc1 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
> @@ -430,6 +430,10 @@ &pwm1 {
>  	status = "okay";
>  };
>  
> +&rng {
> +	status = "okay";

I don't see how RNG is board related.


Best regards,
Krzysztof