Hi Dragan,
On 2024-04-15 08:37, Dragan Simic wrote:
> Hello Jonas,
>
> 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.
>
>> + };
>> +
>> 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.
Regards,
Jonas
>
>> };
>>
>> 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
