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

[Dragan Simic <dsimic@manjaro.org>]

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.