[PATCH v8 3/3] arm64: dts: rockchip: add DT entry for RNG to RK356x

Daniel Golle posted 3 patches 1 year, 4 months ago
There is a newer version of this series
[PATCH v8 3/3] arm64: dts: rockchip: add DT entry for RNG to RK356x
Posted by Daniel Golle 1 year, 4 months ago
From: Aurelien Jarno <aurelien@aurel32.net>

Enable the just added Rockchip RNG driver for RK356x SoCs.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 arch/arm64/boot/dts/rockchip/rk3568.dtsi |  7 +++++++
 arch/arm64/boot/dts/rockchip/rk356x.dtsi | 10 ++++++++++
 2 files changed, 17 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
index f1be76a54ceb..b9c6b2dc87fa 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
@@ -257,6 +257,13 @@ power-domain@RK3568_PD_PIPE {
 	};
 };
 
+&rng {
+	rockchip,sample-count = <1000>;
+	quality = <900>;
+
+	status = "okay";
+};
+
 &usb_host0_xhci {
 	phys = <&usb2phy0_otg>, <&combphy0 PHY_TYPE_USB3>;
 	phy-names = "usb2-phy", "usb3-phy";
diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
index 4690be841a1c..d160a23fd495 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
@@ -1113,6 +1113,16 @@ sdhci: mmc@fe310000 {
 		status = "disabled";
 	};
 
+	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";
+		status = "disabled";
+	};
+
 	i2s0_8ch: i2s@fe400000 {
 		compatible = "rockchip,rk3568-i2s-tdm";
 		reg = <0x0 0xfe400000 0x0 0x1000>;
-- 
2.45.2
Re: [PATCH v8 3/3] arm64: dts: rockchip: add DT entry for RNG to RK356x
Posted by Jason A. Donenfeld 1 year, 4 months ago
On Sun, Jul 21, 2024 at 01:48:38AM +0100, Daniel Golle wrote:
> From: Aurelien Jarno <aurelien@aurel32.net>
> 
> Enable the just added Rockchip RNG driver for RK356x SoCs.
> 
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
>  arch/arm64/boot/dts/rockchip/rk3568.dtsi |  7 +++++++
>  arch/arm64/boot/dts/rockchip/rk356x.dtsi | 10 ++++++++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> index f1be76a54ceb..b9c6b2dc87fa 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> @@ -257,6 +257,13 @@ power-domain@RK3568_PD_PIPE {
>  	};
>  };
>  
> +&rng {
> +	rockchip,sample-count = <1000>;
> +	quality = <900>;

As I already wrote you for v7, quality is out of 1024, not 1000, so this
won't hit 90% as you intend.

But also, I think putting this in the DT is a mistake. Other drivers
don't generally do this, and if the hardware is actually the same piece
to piece (it is...), then there's not per-manufactured unit tweaking
needed. So keep this in the actual driver C like other drivers.

Jason
Re: [PATCH v8 3/3] arm64: dts: rockchip: add DT entry for RNG to RK356x
Posted by Daniel Golle 1 year, 4 months ago
On Sun, Jul 21, 2024 at 02:07:23PM +0200, Jason A. Donenfeld wrote:
> On Sun, Jul 21, 2024 at 01:48:38AM +0100, Daniel Golle wrote:
> > From: Aurelien Jarno <aurelien@aurel32.net>
> > 
> > Enable the just added Rockchip RNG driver for RK356x SoCs.
> > 
> > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > ---
> >  arch/arm64/boot/dts/rockchip/rk3568.dtsi |  7 +++++++
> >  arch/arm64/boot/dts/rockchip/rk356x.dtsi | 10 ++++++++++
> >  2 files changed, 17 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> > index f1be76a54ceb..b9c6b2dc87fa 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> > @@ -257,6 +257,13 @@ power-domain@RK3568_PD_PIPE {
> >  	};
> >  };
> >  
> > +&rng {
> > +	rockchip,sample-count = <1000>;
> > +	quality = <900>;
> 
> As I already wrote you for v7, quality is out of 1024, not 1000, so this
> won't hit 90% as you intend.

It's not actually 90%. Around 125 out of 1000 test runs are failing on
the R5C boards I got here, so that makes it 87.5% which is pretty close
to the 87.9% of the 900/1024 figure there, hence I kept it 900 despite
your comment.

> 
> But also, I think putting this in the DT is a mistake. Other drivers
> don't generally do this, and if the hardware is actually the same piece
> to piece (it is...), then there's not per-manufactured unit tweaking
> needed. So keep this in the actual driver C like other drivers.

So quality should be assigned using the DT compatible, right?
And if needed we should have several of them, one for each SoC (if
testing now turns out to show that the results are specific for the SoC
rather than for the board).
Re: [PATCH v8 3/3] arm64: dts: rockchip: add DT entry for RNG to RK356x
Posted by Jason A. Donenfeld 1 year, 4 months ago
On Sun, Jul 21, 2024 at 3:49 PM Daniel Golle <daniel@makrotopia.org> wrote:
>
> On Sun, Jul 21, 2024 at 02:07:23PM +0200, Jason A. Donenfeld wrote:
> > On Sun, Jul 21, 2024 at 01:48:38AM +0100, Daniel Golle wrote:
> > > From: Aurelien Jarno <aurelien@aurel32.net>
> > >
> > > Enable the just added Rockchip RNG driver for RK356x SoCs.
> > >
> > > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> > > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > > ---
> > >  arch/arm64/boot/dts/rockchip/rk3568.dtsi |  7 +++++++
> > >  arch/arm64/boot/dts/rockchip/rk356x.dtsi | 10 ++++++++++
> > >  2 files changed, 17 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> > > index f1be76a54ceb..b9c6b2dc87fa 100644
> > > --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> > > +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> > > @@ -257,6 +257,13 @@ power-domain@RK3568_PD_PIPE {
> > >     };
> > >  };
> > >
> > > +&rng {
> > > +   rockchip,sample-count = <1000>;
> > > +   quality = <900>;
> >
> > As I already wrote you for v7, quality is out of 1024, not 1000, so this
> > won't hit 90% as you intend.
>
> It's not actually 90%. Around 125 out of 1000 test runs are failing on
> the R5C boards I got here, so that makes it 87.5% which is pretty close
> to the 87.9% of the 900/1024 figure there, hence I kept it 900 despite
> your comment.

Just do 922?

>
> >
> > But also, I think putting this in the DT is a mistake. Other drivers
> > don't generally do this, and if the hardware is actually the same piece
> > to piece (it is...), then there's not per-manufactured unit tweaking
> > needed. So keep this in the actual driver C like other drivers.
>
> So quality should be assigned using the DT compatible, right?
> And if needed we should have several of them, one for each SoC (if
> testing now turns out to show that the results are specific for the SoC
> rather than for the board).

No, do it in the C. If you don't have evidence of such crazy
complexity and diversity, don't overengineer for it. Nothing else does
this in the DT.
Re: [PATCH v8 3/3] arm64: dts: rockchip: add DT entry for RNG to RK356x
Posted by Dragan Simic 1 year, 4 months ago
Hello all,

On 2024-07-21 14:07, Jason A. Donenfeld wrote:
> On Sun, Jul 21, 2024 at 01:48:38AM +0100, Daniel Golle wrote:
>> From: Aurelien Jarno <aurelien@aurel32.net>
>> 
>> Enable the just added Rockchip RNG driver for RK356x SoCs.
>> 
>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
>> ---
>>  arch/arm64/boot/dts/rockchip/rk3568.dtsi |  7 +++++++
>>  arch/arm64/boot/dts/rockchip/rk356x.dtsi | 10 ++++++++++
>>  2 files changed, 17 insertions(+)
>> 
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi 
>> b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
>> index f1be76a54ceb..b9c6b2dc87fa 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3568.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
>> @@ -257,6 +257,13 @@ power-domain@RK3568_PD_PIPE {
>>  	};
>>  };
>> 
>> +&rng {
>> +	rockchip,sample-count = <1000>;
>> +	quality = <900>;
> 
> As I already wrote you for v7, quality is out of 1024, not 1000, so 
> this
> won't hit 90% as you intend.
> 
> But also, I think putting this in the DT is a mistake. Other drivers
> don't generally do this, and if the hardware is actually the same piece
> to piece (it is...), then there's not per-manufactured unit tweaking
> needed. So keep this in the actual driver C like other drivers.

Actually, if we find out that some samples of RK3568 have HWRNG that
performs poorly, we'll be able to regrettably conclude that this driver
cannot be used at all.  As we remember, RK3566 has been already proven
to have inconsistent HRWNG that may perform poorly, which basically
disqualifies the RK3566 from using this driver.

Thus, I agree that the per-SoC-variant parameters should be moved
to the driver code in the final version.  However, this is still
a development version that has the parameters in the DT specifically
to allow easier testing of the different parameter values.