From: "Heiko Stübner" <heiko@sntech.de>
To: Dragan Simic <dsimic@manjaro.org>,
Daniel Golle <daniel@makrotopia.org>,
Diederik de Haas <didi.debian@cknow.org>
Cc: "Chen-Yu Tsai" <wens@kernel.org>,
linux-rockchip@lists.infradead.org,
linux-arm-kernel@lists.infradead.org,
"Rob Herring" <robh@kernel.org>,
"Conor Dooley" <conor+dt@kernel.org>,
linux-kernel@vger.kernel.org,
"Herbert Xu" <herbert@gondor.apana.org.au>,
"Martin Kaiser" <martin@kaiser.cx>,
"Sascha Hauer" <s.hauer@pengutronix.de>,
"Sebastian Reichel" <sebastian.reichel@collabora.com>,
"Ard Biesheuvel" <ardb@kernel.org>,
"Uwe Kleine-König" <ukleinek@debian.org>,
devicetree@vger.kernel.org, linux-crypto@vger.kernel.org,
"Philipp Zabel" <p.zabel@pengutronix.de>,
"Olivia Mackall" <olivia@selenic.com>,
"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
"Aurelien Jarno" <aurelien@aurel32.net>
Subject: Re: [PATCH v7 0/3] hwrng: add hwrng support for Rockchip RK3568
Date: Tue, 30 Jul 2024 12:36:43 +0200 [thread overview]
Message-ID: <17577153.5WZRyvrzyv@diego> (raw)
In-Reply-To: <6690040.iosknibmi9@bagend>
Am Dienstag, 30. Juli 2024, 11:03:06 CEST schrieb Diederik de Haas:
> On Tuesday, 30 July 2024 01:18:37 CEST Daniel Golle wrote:
> > On Wed, Jul 24, 2024 at 08:07:51AM +0200, Dragan Simic wrote:
> > > Thanks a lot for the testing. Though, such wildly different test results
> > > can, regrettably, lead to only one conclusion: the HWRNG found in RK3566
> > > is unusable. :/
>
> FTR: I agree with Dragan, unfortunately.
>
> > The results on RK3568 look much better and the series right now also
> > only enabled the RNG on RK3568 systems. However, we have only seen few
> > boards with RK3568 up to now, and I only got a couple of NanoPi R5C
> > here to test, all with good hwrng results.
> >
> > Do you think it would be agreeable to only enable the HWRNG for RK3568
> > as suggested in this series? Or are we expecting quality to also vary
> > as much as it (sadly) does for RK3566?
>
> Unless we get *evidence* to the contrary, we should assume that the HWRNG on
> RK3568 is fine as the currently available test results are fine.
> So I think enabling it only for RK3568 is the right thing to do.
>
> So a 'revert' to v7 variant seems appropriate, but with the following changes:
> - Add `status = "disabled";` property to the definition in rk356x.dtsi
> - Add a new commit where you enable it only for rk3568 and document in the
> commit message why it's not enabled on rk3566 with a possible link to the v7
> thread for clarification on why that is
I was going to protest about the "disable" until reading the 2nd part :-D .
And yeah that makes a lot of sense, "add" it to rk356x.dtsi, as the IP is
part of both variants, but only enable it in rk3568.dtsi because of the
seemingly faulty implementation on the rk3566.
> You could probably also integrate that into 1 commit, but make sure that the
> commit summary and description match the implementation.
> IMO that wasn't 'technically' the case in v8 as the rng node was added to
> rk356x, but it was only enabled on rk3568.
>
> My 0.02
next prev parent reply other threads:[~2024-07-30 10:38 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-14 15:15 [PATCH v7 0/3] hwrng: add hwrng support for Rockchip RK3568 Daniel Golle
2024-07-14 15:15 ` [PATCH v7 1/3] dt-bindings: rng: Add Rockchip RK3568 TRNG Daniel Golle
2024-07-14 15:16 ` [PATCH v7 2/3] hwrng: add hwrng driver for Rockchip RK3568 SoC Daniel Golle
2024-07-15 19:47 ` Martin Kaiser
2024-07-21 0:26 ` Jason A. Donenfeld
2024-07-14 15:18 ` [PATCH v7 3/3] arm64: dts: rockchip: add DT entry for RNG to RK356x Daniel Golle
2024-07-14 18:09 ` [PATCH v7 0/3] hwrng: add hwrng support for Rockchip RK3568 Chen-Yu Tsai
2024-07-16 12:34 ` Diederik de Haas
2024-07-16 13:27 ` Daniel Golle
2024-07-16 13:59 ` Diederik de Haas
2024-07-16 14:13 ` Diederik de Haas
2024-07-16 15:18 ` Chen-Yu Tsai
2024-07-16 16:53 ` Diederik de Haas
2024-07-16 17:19 ` Diederik de Haas
2024-07-17 2:24 ` Daniel Golle
2024-07-17 2:58 ` Chen-Yu Tsai
2024-07-17 3:34 ` Dragan Simic
2024-07-17 5:06 ` Anand Moon
2024-07-17 5:18 ` Dragan Simic
2024-07-17 8:22 ` Diederik de Haas
2024-07-17 8:31 ` Dragan Simic
2024-07-17 8:38 ` Chen-Yu Tsai
2024-07-17 8:49 ` Diederik de Haas
2024-07-17 10:44 ` Daniel Golle
2024-07-17 3:14 ` Dragan Simic
2024-07-22 17:57 ` Chen-Yu Tsai
2024-07-22 19:03 ` Diederik de Haas
2024-07-24 6:07 ` Dragan Simic
2024-07-29 23:18 ` Daniel Golle
2024-07-30 9:03 ` Diederik de Haas
2024-07-30 10:36 ` Heiko Stübner [this message]
2024-07-30 12:08 ` Chen-Yu Tsai
2024-08-01 16:48 ` Dragan Simic
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=17577153.5WZRyvrzyv@diego \
--to=heiko@sntech.de \
--cc=ardb@kernel.org \
--cc=aurelien@aurel32.net \
--cc=conor+dt@kernel.org \
--cc=daniel@makrotopia.org \
--cc=devicetree@vger.kernel.org \
--cc=didi.debian@cknow.org \
--cc=dsimic@manjaro.org \
--cc=herbert@gondor.apana.org.au \
--cc=krzk+dt@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=martin@kaiser.cx \
--cc=olivia@selenic.com \
--cc=p.zabel@pengutronix.de \
--cc=robh@kernel.org \
--cc=s.hauer@pengutronix.de \
--cc=sebastian.reichel@collabora.com \
--cc=ukleinek@debian.org \
--cc=wens@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).