On 2024-04-15 10:03, Jonas Karlman wrote:
On 2024-04-15 08:37, Dragan Simic wrote:
Please see a couple of comments below.

On 2024-04-13 20:13, Jonas Karlman wrote:
The RK35xx SoCs contain a crypto engine block that can generate random
numbers.

Enable rng node in soc u-boot.dtsi and enable Kconfig options to take
advantage of the random generator.

Signed-off-by: Jonas Karlman <jo...@kwiboo.se>
---
v2: No change
---
 arch/arm/dts/rk3566-anbernic-rgxx3-u-boot.dtsi | 6 ------
 arch/arm/dts/rk356x-u-boot.dtsi                | 5 +++++
 arch/arm/dts/rk3588s-u-boot.dtsi               | 1 -
 arch/arm/mach-rockchip/Kconfig                 | 4 ++++
 configs/anbernic-rgxx3-rk3566_defconfig        | 2 --
 5 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/arm/dts/rk3566-anbernic-rgxx3-u-boot.dtsi
b/arch/arm/dts/rk3566-anbernic-rgxx3-u-boot.dtsi
index 791f16b206f2..793ed4ae8ae0 100644
--- a/arch/arm/dts/rk3566-anbernic-rgxx3-u-boot.dtsi
+++ b/arch/arm/dts/rk3566-anbernic-rgxx3-u-boot.dtsi
@@ -6,12 +6,6 @@
        chosen {
                u-boot,spl-boot-order = "same-as-spl", &sdmmc1, &sdmmc0;
        };
-
-       rng: rng@fe388000 {
-               compatible = "rockchip,cryptov2-rng";
-               reg = <0x0 0xfe388000 0x0 0x2000>;
-               status = "okay";
-       };
 };

 &dsi_dphy0 {
diff --git a/arch/arm/dts/rk356x-u-boot.dtsi
b/arch/arm/dts/rk356x-u-boot.dtsi
index d347080577d9..05367216e118 100644
--- a/arch/arm/dts/rk356x-u-boot.dtsi
+++ b/arch/arm/dts/rk356x-u-boot.dtsi
@@ -21,6 +21,11 @@
                bootph-all;
        };

+       rng: rng@fe388000 {
+               compatible = "rockchip,cryptov2-rng";
+               reg = <0x0 0xfe388000 0x0 0x2000>;

Shouldn't

  +             status = "okay";

also be specified here?

It is not needed as "okay" is the implied status when status prop is
undefined, and my understanding is that DT maintainers prefer not to
include status = "okay" where the initial node is defined.

More correctly if status prop is defined it must explicitly have "okay"
anything else result in a disabled node, removing the status prop
entirely has the same effect as status = "okay" in a board file.

I see, thanks for the clarification.  In that case, there seem to be
a few redundant instances of 'status = "okay";' in various U-Boot dtsi
files, which should probably be deleted.

+       };
+
        otp: nvmem@fe38c000 {
                compatible = "rockchip,rk3568-otp";
                reg = <0x0 0xfe38c000 0x0 0x4000>;
diff --git a/arch/arm/dts/rk3588s-u-boot.dtsi
b/arch/arm/dts/rk3588s-u-boot.dtsi
index ac67c777adea..233eb79d9ba2 100644
--- a/arch/arm/dts/rk3588s-u-boot.dtsi
+++ b/arch/arm/dts/rk3588s-u-boot.dtsi
@@ -91,7 +91,6 @@
        rng: rng@fe378000 {
                compatible = "rockchip,trngv1";
                reg = <0x0 0xfe378000 0x0 0x200>;
-               status = "disabled";

Shouldn't it be enabled instead?

Same as above, not defining any status prop result in node being enabled.

Board u-boot.dtsi-files could disable the node with status = "disabled"
if they absolutely do not want to have the rng node in U-Boot.

Thanks once again for the clarification.  Please include this tag:

Reviewed-by: Dragan Simic <dsi...@manjaro.org>


        };

        usbdp_phy0: phy@fed80000 {
diff --git a/arch/arm/mach-rockchip/Kconfig
b/arch/arm/mach-rockchip/Kconfig
index 649c22618f36..247b9a3146c2 100644
--- a/arch/arm/mach-rockchip/Kconfig
+++ b/arch/arm/mach-rockchip/Kconfig
@@ -305,9 +305,11 @@ config ROCKCHIP_RK3568
        select BOARD_LATE_INIT
        select DM_REGULATOR_FIXED
        select DM_RESET
+       imply DM_RNG
        imply MISC_INIT_R
        imply MMC_HS200_SUPPORT if MMC_SDHCI_ROCKCHIP
        imply OF_LIBFDT_OVERLAY
+       imply RNG_ROCKCHIP
        imply ROCKCHIP_COMMON_BOARD
        imply ROCKCHIP_OTP
        imply SPL_ATF_NO_PLATFORM_PARAM if SPL_ATF
@@ -334,9 +336,11 @@ config ROCKCHIP_RK3588
        select DM_RESET
        imply BOOTSTD_FULL
        imply CLK_SCMI
+       imply DM_RNG
        imply MISC_INIT_R
        imply MMC_HS200_SUPPORT if MMC_SDHCI_ROCKCHIP
        imply OF_LIBFDT_OVERLAY
+       imply RNG_ROCKCHIP
        imply ROCKCHIP_COMMON_BOARD
        imply ROCKCHIP_OTP
        imply SCMI_FIRMWARE
diff --git a/configs/anbernic-rgxx3-rk3566_defconfig
b/configs/anbernic-rgxx3-rk3566_defconfig
index 24b050c59b53..110237e798f9 100644
--- a/configs/anbernic-rgxx3-rk3566_defconfig
+++ b/configs/anbernic-rgxx3-rk3566_defconfig
@@ -68,8 +68,6 @@ CONFIG_REGULATOR_RK8XX=y
 CONFIG_PWM_ROCKCHIP=y
 CONFIG_SPL_RAM=y
 # CONFIG_RAM_ROCKCHIP_DEBUG is not set
-CONFIG_DM_RNG=y
-CONFIG_RNG_ROCKCHIP=y
 # CONFIG_RNG_SMCCC_TRNG is not set
 CONFIG_BAUDRATE=1500000
 CONFIG_DEBUG_UART_SHIFT=2

Reply via email to