diff mbox series

[RFC] ARM: omap3: Enable HWMODS for HW Random Number Generator

Message ID 20190828150037.2640-1-aford173@gmail.com (mailing list archive)
State New, archived
Headers show
Series [RFC] ARM: omap3: Enable HWMODS for HW Random Number Generator | expand

Commit Message

Adam Ford Aug. 28, 2019, 3 p.m. UTC
The datasheet for the AM3517 shows the RNG is connected to L4.
It shows the module address for the RNG is 0x480A0000, and it
matches the omap2.dtsi description.  Since the driver can support
omap2 and omap4, it seems reasonable to assume the omap3 would
use the same core for the RNG.

This RFC, mimics much of the omap2 hwmods on the OMAP3. It
also adds the necessary clock for driving the RNG.  Unfortunately,
it appears non-functional.  If anyone has any suggestions on how
to finish the hwmod (or port it to the newer l4 device tree
format), feedback is requested.

Currently the hwmods repond as follows:

[    0.245697] omap_hwmod: rng: _wait_target_ready failed: -22
[    0.245727] omap_hwmod: rng: cannot be enabled for reset (3)
[    6.780792] omap_hwmod: rng: _wait_target_ready failed: -22

Signed-off-by: Adam Ford <aford173@gmail.com>

Comments

Tony Lindgren Sept. 5, 2019, 11:04 p.m. UTC | #1
Hi,

* Adam Ford <aford173@gmail.com> [190828 15:01]:
> The datasheet for the AM3517 shows the RNG is connected to L4.
> It shows the module address for the RNG is 0x480A0000, and it
> matches the omap2.dtsi description.  Since the driver can support
> omap2 and omap4, it seems reasonable to assume the omap3 would
> use the same core for the RNG.
> 
> This RFC, mimics much of the omap2 hwmods on the OMAP3. It
> also adds the necessary clock for driving the RNG.  Unfortunately,
> it appears non-functional.  If anyone has any suggestions on how
> to finish the hwmod (or port it to the newer l4 device tree
> format), feedback is requested.

Yup I'll take the bait :) The patch below seems to do the trick
for me on dm3730 based on translating your patch to probe with
ti-sysc.

Not sure about 34xx, it seems we're missing rng_clk? Care
to give it a try and attempt simlar patches for 34xx and
3517?

At least I'm not needing the "ti,no-reset-on-init" property
that your patch has a comment for. Maybe that's needed on
some other omap3.

Oh and this needs to default to status = "disabled" for
HS devices like n900 as it needs to use the omap3-rom-rng.

Regards,

Tony

8< -----------------------
diff --git a/arch/arm/boot/dts/omap36xx.dtsi b/arch/arm/boot/dts/omap36xx.dtsi
--- a/arch/arm/boot/dts/omap36xx.dtsi
+++ b/arch/arm/boot/dts/omap36xx.dtsi
@@ -140,6 +140,29 @@
 			};
 		};
 
+		rng_target: target-module@480a0000 {
+			compatible = "ti,sysc-omap2", "ti,sysc";
+			reg = <0x480a003c 0x4>,
+			      <0x480a0040 0x4>,
+			      <0x480a0044 0x4>;
+			reg-names = "rev", "sysc", "syss";
+			ti,sysc-mask = <(SYSC_OMAP2_AUTOIDLE)>;
+			ti,sysc-sidle = <SYSC_IDLE_FORCE>,
+					<SYSC_IDLE_NO>;
+			ti,syss-mask = <1>;
+			clocks = <&rng_ick>;
+			clock-names = "ick";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges = <0 0x480a0000 0x2000>;
+
+			rng: rng@0 {
+				compatible = "ti,omap2-rng";
+				reg = <0x0 0x2000>;
+				interrupts = <52>;
+			};
+		};
+
 		/*
 		 * Note that the sysconfig register layout is a subset of the
 		 * "ti,sysc-omap4" type register with just sidle and midle bits
Adam Ford Sept. 9, 2019, 12:13 p.m. UTC | #2
On Thu, Sep 5, 2019 at 6:04 PM Tony Lindgren <tony@atomide.com> wrote:
>
> Hi,
>
> * Adam Ford <aford173@gmail.com> [190828 15:01]:
> > The datasheet for the AM3517 shows the RNG is connected to L4.
> > It shows the module address for the RNG is 0x480A0000, and it
> > matches the omap2.dtsi description.  Since the driver can support
> > omap2 and omap4, it seems reasonable to assume the omap3 would
> > use the same core for the RNG.
> >
> > This RFC, mimics much of the omap2 hwmods on the OMAP3. It
> > also adds the necessary clock for driving the RNG.  Unfortunately,
> > it appears non-functional.  If anyone has any suggestions on how
> > to finish the hwmod (or port it to the newer l4 device tree
> > format), feedback is requested.
>
> Yup I'll take the bait :) The patch below seems to do the trick
> for me on dm3730 based on translating your patch to probe with
> ti-sysc.
>
> Not sure about 34xx, it seems we're missing rng_clk? Care
> to give it a try and attempt simlar patches for 34xx and
> 3517?
>
> At least I'm not needing the "ti,no-reset-on-init" property
> that your patch has a comment for. Maybe that's needed on
> some other omap3.
>
> Oh and this needs to default to status = "disabled" for
> HS devices like n900 as it needs to use the omap3-rom-rng.
>
> Regards,
>
> Tony
>
> 8< -----------------------
> diff --git a/arch/arm/boot/dts/omap36xx.dtsi b/arch/arm/boot/dts/omap36xx.dtsi
> --- a/arch/arm/boot/dts/omap36xx.dtsi
> +++ b/arch/arm/boot/dts/omap36xx.dtsi
> @@ -140,6 +140,29 @@
>                         };
>                 };
>
> +               rng_target: target-module@480a0000 {
> +                       compatible = "ti,sysc-omap2", "ti,sysc";
> +                       reg = <0x480a003c 0x4>,
> +                             <0x480a0040 0x4>,
> +                             <0x480a0044 0x4>;
> +                       reg-names = "rev", "sysc", "syss";
> +                       ti,sysc-mask = <(SYSC_OMAP2_AUTOIDLE)>;
> +                       ti,sysc-sidle = <SYSC_IDLE_FORCE>,
> +                                       <SYSC_IDLE_NO>;
> +                       ti,syss-mask = <1>;
> +                       clocks = <&rng_ick>;
> +                       clock-names = "ick";
> +                       #address-cells = <1>;
> +                       #size-cells = <1>;
> +                       ranges = <0 0x480a0000 0x2000>;
> +
> +                       rng: rng@0 {
> +                               compatible = "ti,omap2-rng";
> +                               reg = <0x0 0x2000>;
> +                               interrupts = <52>;
> +                       };
> +               };
> +

Tony,

Can you tell me what branch you're using?  I am not seeing the note
below, so I am not exactly sure what version to base my testing.

ada,
>                 /*
>                  * Note that the sysconfig register layout is a subset of the
>                  * "ti,sysc-omap4" type register with just sidle and midle bits
Adam Ford Sept. 9, 2019, 1:37 p.m. UTC | #3
On Mon, Sep 9, 2019 at 7:13 AM Adam Ford <aford173@gmail.com> wrote:
>
> On Thu, Sep 5, 2019 at 6:04 PM Tony Lindgren <tony@atomide.com> wrote:
> >
> > Hi,
> >
> > * Adam Ford <aford173@gmail.com> [190828 15:01]:
> > > The datasheet for the AM3517 shows the RNG is connected to L4.
> > > It shows the module address for the RNG is 0x480A0000, and it
> > > matches the omap2.dtsi description.  Since the driver can support
> > > omap2 and omap4, it seems reasonable to assume the omap3 would
> > > use the same core for the RNG.
> > >
> > > This RFC, mimics much of the omap2 hwmods on the OMAP3. It
> > > also adds the necessary clock for driving the RNG.  Unfortunately,
> > > it appears non-functional.  If anyone has any suggestions on how
> > > to finish the hwmod (or port it to the newer l4 device tree
> > > format), feedback is requested.
> >
> > Yup I'll take the bait :) The patch below seems to do the trick
> > for me on dm3730 based on translating your patch to probe with
> > ti-sysc.
> >
> > Not sure about 34xx, it seems we're missing rng_clk? Care
> > to give it a try and attempt simlar patches for 34xx and
> > 3517?
> >
> > At least I'm not needing the "ti,no-reset-on-init" property
> > that your patch has a comment for. Maybe that's needed on
> > some other omap3.
> >
> > Oh and this needs to default to status = "disabled" for
> > HS devices like n900 as it needs to use the omap3-rom-rng.
> >
> > Regards,
> >
> > Tony
> >
> > 8< -----------------------
> > diff --git a/arch/arm/boot/dts/omap36xx.dtsi b/arch/arm/boot/dts/omap36xx.dtsi
> > --- a/arch/arm/boot/dts/omap36xx.dtsi
> > +++ b/arch/arm/boot/dts/omap36xx.dtsi
> > @@ -140,6 +140,29 @@
> >                         };
> >                 };
> >
> > +               rng_target: target-module@480a0000 {
> > +                       compatible = "ti,sysc-omap2", "ti,sysc";
> > +                       reg = <0x480a003c 0x4>,
> > +                             <0x480a0040 0x4>,
> > +                             <0x480a0044 0x4>;
> > +                       reg-names = "rev", "sysc", "syss";
> > +                       ti,sysc-mask = <(SYSC_OMAP2_AUTOIDLE)>;
> > +                       ti,sysc-sidle = <SYSC_IDLE_FORCE>,
> > +                                       <SYSC_IDLE_NO>;
> > +                       ti,syss-mask = <1>;
> > +                       clocks = <&rng_ick>;
> > +                       clock-names = "ick";
> > +                       #address-cells = <1>;
> > +                       #size-cells = <1>;
> > +                       ranges = <0 0x480a0000 0x2000>;
> > +
> > +                       rng: rng@0 {
> > +                               compatible = "ti,omap2-rng";
> > +                               reg = <0x0 0x2000>;
> > +                               interrupts = <52>;
> > +                       };
> > +               };
> > +

I applied this on 5.3 and it is working.  I assume the same is true in for-next.

Do you want to submit a formal patch?  I  can mark it as 'tested-by'
This really helps speed up the startup sequence on boards with sshd
because it delays for nearly 80 seconds waiting for entropy without
the hwrng.

adam
>
> Tony,
>
> Can you tell me what branch you're using?  I am not seeing the note
> below, so I am not exactly sure what version to base my testing.
>
> ada,
> >                 /*
> >                  * Note that the sysconfig register layout is a subset of the
> >                  * "ti,sysc-omap4" type register with just sidle and midle bits
Pali Rohár Sept. 9, 2019, 1:40 p.m. UTC | #4
On Monday 09 September 2019 08:37:09 Adam Ford wrote:
> On Mon, Sep 9, 2019 at 7:13 AM Adam Ford <aford173@gmail.com> wrote:
> >
> > On Thu, Sep 5, 2019 at 6:04 PM Tony Lindgren <tony@atomide.com> wrote:
> > >
> > > Hi,
> > >
> > > * Adam Ford <aford173@gmail.com> [190828 15:01]:
> > > > The datasheet for the AM3517 shows the RNG is connected to L4.
> > > > It shows the module address for the RNG is 0x480A0000, and it
> > > > matches the omap2.dtsi description.  Since the driver can support
> > > > omap2 and omap4, it seems reasonable to assume the omap3 would
> > > > use the same core for the RNG.
> > > >
> > > > This RFC, mimics much of the omap2 hwmods on the OMAP3. It
> > > > also adds the necessary clock for driving the RNG.  Unfortunately,
> > > > it appears non-functional.  If anyone has any suggestions on how
> > > > to finish the hwmod (or port it to the newer l4 device tree
> > > > format), feedback is requested.
> > >
> > > Yup I'll take the bait :) The patch below seems to do the trick
> > > for me on dm3730 based on translating your patch to probe with
> > > ti-sysc.
> > >
> > > Not sure about 34xx, it seems we're missing rng_clk? Care
> > > to give it a try and attempt simlar patches for 34xx and
> > > 3517?
> > >
> > > At least I'm not needing the "ti,no-reset-on-init" property
> > > that your patch has a comment for. Maybe that's needed on
> > > some other omap3.
> > >
> > > Oh and this needs to default to status = "disabled" for
> > > HS devices like n900 as it needs to use the omap3-rom-rng.
> > >
> > > Regards,
> > >
> > > Tony
> > >
> > > 8< -----------------------
> > > diff --git a/arch/arm/boot/dts/omap36xx.dtsi b/arch/arm/boot/dts/omap36xx.dtsi
> > > --- a/arch/arm/boot/dts/omap36xx.dtsi
> > > +++ b/arch/arm/boot/dts/omap36xx.dtsi
> > > @@ -140,6 +140,29 @@
> > >                         };
> > >                 };
> > >
> > > +               rng_target: target-module@480a0000 {
> > > +                       compatible = "ti,sysc-omap2", "ti,sysc";
> > > +                       reg = <0x480a003c 0x4>,
> > > +                             <0x480a0040 0x4>,
> > > +                             <0x480a0044 0x4>;
> > > +                       reg-names = "rev", "sysc", "syss";
> > > +                       ti,sysc-mask = <(SYSC_OMAP2_AUTOIDLE)>;
> > > +                       ti,sysc-sidle = <SYSC_IDLE_FORCE>,
> > > +                                       <SYSC_IDLE_NO>;
> > > +                       ti,syss-mask = <1>;
> > > +                       clocks = <&rng_ick>;
> > > +                       clock-names = "ick";
> > > +                       #address-cells = <1>;
> > > +                       #size-cells = <1>;
> > > +                       ranges = <0 0x480a0000 0x2000>;
> > > +
> > > +                       rng: rng@0 {
> > > +                               compatible = "ti,omap2-rng";
> > > +                               reg = <0x0 0x2000>;
> > > +                               interrupts = <52>;
> > > +                       };
> > > +               };
> > > +
> 
> I applied this on 5.3 and it is working.  I assume the same is true in for-next.
> 
> Do you want to submit a formal patch?  I  can mark it as 'tested-by'
> This really helps speed up the startup sequence on boards with sshd
> because it delays for nearly 80 seconds waiting for entropy without
> the hwrng.

Hi! When applying a patch, could you please disable this rng for n900?

In omap3-n900.dts for rng should be status = "disabled" (as Tony already
wrote), similarly like for aes.

Thanks!

> adam
> >
> > Tony,
> >
> > Can you tell me what branch you're using?  I am not seeing the note
> > below, so I am not exactly sure what version to base my testing.
> >
> > ada,
> > >                 /*
> > >                  * Note that the sysconfig register layout is a subset of the
> > >                  * "ti,sysc-omap4" type register with just sidle and midle bits
Tony Lindgren Sept. 9, 2019, 4:35 p.m. UTC | #5
* Pali Rohár <pali.rohar@gmail.com> [190909 13:41]:
> On Monday 09 September 2019 08:37:09 Adam Ford wrote:
> > I applied this on 5.3 and it is working.  I assume the same is true in for-next.

Hmm I noticed I stopped getting RNG data after several rmmod modprobe
cycles, or several hd /dev/random reads. Anybody else seeing that?

> > Do you want to submit a formal patch?  I  can mark it as 'tested-by'
> > This really helps speed up the startup sequence on boards with sshd
> > because it delays for nearly 80 seconds waiting for entropy without
> > the hwrng.
> 
> Hi! When applying a patch, could you please disable this rng for n900?
> 
> In omap3-n900.dts for rng should be status = "disabled" (as Tony already
> wrote), similarly like for aes.

Yeah I'll post a proper patch after -rc1.

Regards,

Tony
Adam Ford Sept. 9, 2019, 7:19 p.m. UTC | #6
On Mon, Sep 9, 2019 at 11:35 AM Tony Lindgren <tony@atomide.com> wrote:
>
> * Pali Rohár <pali.rohar@gmail.com> [190909 13:41]:
> > On Monday 09 September 2019 08:37:09 Adam Ford wrote:
> > > I applied this on 5.3 and it is working.  I assume the same is true in for-next.
>
> Hmm I noticed I stopped getting RNG data after several rmmod modprobe
> cycles, or several hd /dev/random reads. Anybody else seeing that?

On the Logic PD Torpedo, I was able to read from /dev/hwrng and
/dev/random 10x without issue
I have installed rng-tools and I have sshd running and some other
stuff that might get in the way if I do an rmmod too much, but I
removed and modprobed the omap-rng 3x and never saw an issue reading
either /dev/hwrng or /dev/random.

I have been meaning to test this on the AM3517 and haven't gotten to
it yet, but I assume you've only tested omap3630, is that true?

adam
>
> > > Do you want to submit a formal patch?  I  can mark it as 'tested-by'
> > > This really helps speed up the startup sequence on boards with sshd
> > > because it delays for nearly 80 seconds waiting for entropy without
> > > the hwrng.
> >
> > Hi! When applying a patch, could you please disable this rng for n900?
> >
> > In omap3-n900.dts for rng should be status = "disabled" (as Tony already
> > wrote), similarly like for aes.
>
> Yeah I'll post a proper patch after -rc1.
>
> Regards,
>
> Tony
Adam Ford Sept. 10, 2019, 1:56 p.m. UTC | #7
On Thu, Sep 5, 2019 at 6:04 PM Tony Lindgren <tony@atomide.com> wrote:
>
> Hi,
>
> * Adam Ford <aford173@gmail.com> [190828 15:01]:
> > The datasheet for the AM3517 shows the RNG is connected to L4.
> > It shows the module address for the RNG is 0x480A0000, and it
> > matches the omap2.dtsi description.  Since the driver can support
> > omap2 and omap4, it seems reasonable to assume the omap3 would
> > use the same core for the RNG.
> >
> > This RFC, mimics much of the omap2 hwmods on the OMAP3. It
> > also adds the necessary clock for driving the RNG.  Unfortunately,
> > it appears non-functional.  If anyone has any suggestions on how
> > to finish the hwmod (or port it to the newer l4 device tree
> > format), feedback is requested.
>
> Yup I'll take the bait :) The patch below seems to do the trick
> for me on dm3730 based on translating your patch to probe with
> ti-sysc.
>
> Not sure about 34xx, it seems we're missing rng_clk? Care
> to give it a try and attempt simlar patches for 34xx and
> 3517?
>

I took the block you added to omap36xx and copied it to omap34xx.
Since this is present in the omap2.dtsi, I wonder if it could be used
at the omap3.dtsi level instead of am3517, omap34xx and omap36xx.
What is not clear to me is the clocking architecture needed.  The
omap34xx-omap36xx-clocks.dtsi have the  aes1, rng_ick, sha1, and des1
setup which appears to be present in the am3517 based on that
datasheet, but they are all dependent on the security_l4_ick2 which is
not defined for am35.  I wonder if all these could move to omap3 and
its respective clock file.  Duplicating it in 3x locations doesn't
seem to make sense, but I don't have every permutation of omap3 to
know, and those features are not clearly documented.

I have it working on an omap3530:

[    0.000000] Booting Linux on physical CPU 0x0
[    0.000000] Linux version 5.3.0-rc8-00009-gaa2f12f5625a-dirty
(aford@aford-OptiPlex-7050) (gcc version 8.3.0 (Buildroot
2019.02.4-00056-gb0868303cf)) #11 SMP Mon Sep 9 13:59:31 CDT 2019
[    0.000000] CPU: ARMv7 Processor [411fc083] revision 3 (ARMv7), cr=10c5387d
[    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT
nonaliasing instruction cache
[    0.000000] OF: fdt: Machine model: LogicPD Zoom OMAP35xx SOM-LV
Development Kit
...snip...

[    0.000000] OMAP3430/3530 ES3.1 (l2cache iva sgx neon isp)

... snip...

[    0.000000] random: get_random_bytes called from
start_kernel+0x2e8/0x514 with crng_init=0
[    2.573120] random: fast init done
[    5.172821] random: udevd: uninitialized urandom read (16 bytes read)
[    5.182922] random: udevd: uninitialized urandom read (16 bytes read)
[    5.190460] random: udevd: uninitialized urandom read (16 bytes read)
[    7.739837] omap_rng 480a0000.rng: Random Number Generator ver. 70
[    7.747283] random: crng init done
[    7.750793] random: 1 urandom warning(s) missed due to ratelimiting

And hexdump  is working on both /dev/hwrng and /dev/random
I have not been able to replicate the issue you mentioned about it
dying after a few reads and/or rmmod-modprobe cycles.


> At least I'm not needing the "ti,no-reset-on-init" property
> that your patch has a comment for. Maybe that's needed on
> some other omap3.

The hwmod I used was a copy-paste from omap2, so it might not be
needed in omap3's at all.

>
> Oh and this needs to default to status = "disabled" for
> HS devices like n900 as it needs to use the omap3-rom-rng.

I don't know enough about the HS version of the OMAP3, but what's the
main difference between omap3-rom-rng and this one?

adam
>
> Regards,
>
> Tony
>
> 8< -----------------------
> diff --git a/arch/arm/boot/dts/omap36xx.dtsi b/arch/arm/boot/dts/omap36xx.dtsi
> --- a/arch/arm/boot/dts/omap36xx.dtsi
> +++ b/arch/arm/boot/dts/omap36xx.dtsi
> @@ -140,6 +140,29 @@
>                         };
>                 };
>
> +               rng_target: target-module@480a0000 {
> +                       compatible = "ti,sysc-omap2", "ti,sysc";
> +                       reg = <0x480a003c 0x4>,
> +                             <0x480a0040 0x4>,
> +                             <0x480a0044 0x4>;
> +                       reg-names = "rev", "sysc", "syss";
> +                       ti,sysc-mask = <(SYSC_OMAP2_AUTOIDLE)>;
> +                       ti,sysc-sidle = <SYSC_IDLE_FORCE>,
> +                                       <SYSC_IDLE_NO>;
> +                       ti,syss-mask = <1>;
> +                       clocks = <&rng_ick>;
> +                       clock-names = "ick";
> +                       #address-cells = <1>;
> +                       #size-cells = <1>;
> +                       ranges = <0 0x480a0000 0x2000>;
> +
> +                       rng: rng@0 {
> +                               compatible = "ti,omap2-rng";
> +                               reg = <0x0 0x2000>;
> +                               interrupts = <52>;
> +                       };
> +               };
> +
>                 /*
>                  * Note that the sysconfig register layout is a subset of the
>                  * "ti,sysc-omap4" type register with just sidle and midle bits
Sebastian Reichel Sept. 10, 2019, 2:37 p.m. UTC | #8
Hi,

On Tue, Sep 10, 2019 at 08:56:49AM -0500, Adam Ford wrote:
> On Thu, Sep 5, 2019 at 6:04 PM Tony Lindgren <tony@atomide.com> wrote:
> > Oh and this needs to default to status = "disabled" for
> > HS devices like n900 as it needs to use the omap3-rom-rng.
> 
> I don't know enough about the HS version of the OMAP3, but what's the
> main difference between omap3-rom-rng and this one?

The OMAP HS chips have the bus firewall configured to block direct
access to some cryptography related devices. The kernel will crash
with a bus error, if you try to read/write the registers for
protected devices. The omap3-rom-rng avoids this by communicating
with the security middleware component instead of directly accessing
the RNG hardware.

-- Sebastian
Pali Rohár Sept. 10, 2019, 2:44 p.m. UTC | #9
On Tuesday 10 September 2019 15:37:32 Sebastian Reichel wrote:
> Hi,
> 
> On Tue, Sep 10, 2019 at 08:56:49AM -0500, Adam Ford wrote:
> > On Thu, Sep 5, 2019 at 6:04 PM Tony Lindgren <tony@atomide.com> wrote:
> > > Oh and this needs to default to status = "disabled" for
> > > HS devices like n900 as it needs to use the omap3-rom-rng.
> > 
> > I don't know enough about the HS version of the OMAP3, but what's the
> > main difference between omap3-rom-rng and this one?
> 
> The OMAP HS chips have the bus firewall configured to block direct
> access to some cryptography related devices. The kernel will crash
> with a bus error, if you try to read/write the registers for
> protected devices.

And if you try to read it more times, SOC would be rebooted for security
reasons.

> The omap3-rom-rng avoids this by communicating
> with the security middleware component instead of directly accessing
> the RNG hardware.

And that component is loaded by signed bootloader into "secure" area not
accessible by "non-secure" work (like kernel) and communication is done
via arm smc instruction.
Adam Ford Sept. 10, 2019, 3:48 p.m. UTC | #10
On Mon, Sep 9, 2019 at 11:35 AM Tony Lindgren <tony@atomide.com> wrote:
>
> * Pali Rohár <pali.rohar@gmail.com> [190909 13:41]:
> > On Monday 09 September 2019 08:37:09 Adam Ford wrote:
> > > I applied this on 5.3 and it is working.  I assume the same is true in for-next.
>
> Hmm I noticed I stopped getting RNG data after several rmmod modprobe
> cycles, or several hd /dev/random reads. Anybody else seeing that?
>
> > > Do you want to submit a formal patch?  I  can mark it as 'tested-by'
> > > This really helps speed up the startup sequence on boards with sshd
> > > because it delays for nearly 80 seconds waiting for entropy without
> > > the hwrng.
> >
> > Hi! When applying a patch, could you please disable this rng for n900?
> >
> > In omap3-n900.dts for rng should be status = "disabled" (as Tony already
> > wrote), similarly like for aes.
>
> Yeah I'll post a proper patch after -rc1.

FYI,

By putting your node into omap34xx.dtsi and omap36xx.dtsi along with
the following, I can get the RNG to work on an OMAP3530 and a DM3730.


diff --git a/arch/arm/boot/dts/omap34xx-omap36xx-clocks.dtsi
b/arch/arm/boot/dts/omap34xx-omap36xx-clocks.dtsi
index 5e9d1afcd422..73f351e6d132 100644
--- a/arch/arm/boot/dts/omap34xx-omap36xx-clocks.dtsi
+++ b/arch/arm/boot/dts/omap34xx-omap36xx-clocks.dtsi
@@ -259,7 +259,7 @@
                         <&i2c1_ick>, <&uart2_ick>, <&uart1_ick>, <&gpt11_ick>,
                         <&gpt10_ick>, <&mcbsp5_ick>, <&mcbsp1_ick>,
                         <&omapctrl_ick>, <&aes2_ick>, <&sha12_ick>, <&icr_ick>,
-                        <&des2_ick>, <&mspro_ick>, <&mailboxes_ick>,
+                        <&des2_ick>, <&mspro_ick>, <&mailboxes_ick>,
<&rng_ick>,
                         <&mspro_fck>;
        };
 };

I tried doing the same for am3517, but it doesn't appear to work.  In
fact, the board hangs at boot with no splat, so I assume that some
clock isn't running and causing a hang.  Figure 4-50 in the AM3517 TRM
shows the security_l4_iclk2, so I wonder if the HW mods for AES, SHA,
etc are doing something to enable this clock.  Those HWmods are
disabled on AM3517.  I tried turning on the hwmods for them before
without success, but I'll try it again.

adam
>
> Regards,
>
> Tony
Adam Ford Sept. 10, 2019, 4:21 p.m. UTC | #11
On Tue, Sep 10, 2019 at 10:48 AM Adam Ford <aford173@gmail.com> wrote:
>
> On Mon, Sep 9, 2019 at 11:35 AM Tony Lindgren <tony@atomide.com> wrote:
> >
> > * Pali Rohár <pali.rohar@gmail.com> [190909 13:41]:
> > > On Monday 09 September 2019 08:37:09 Adam Ford wrote:
> > > > I applied this on 5.3 and it is working.  I assume the same is true in for-next.
> >
> > Hmm I noticed I stopped getting RNG data after several rmmod modprobe
> > cycles, or several hd /dev/random reads. Anybody else seeing that?
> >
> > > > Do you want to submit a formal patch?  I  can mark it as 'tested-by'
> > > > This really helps speed up the startup sequence on boards with sshd
> > > > because it delays for nearly 80 seconds waiting for entropy without
> > > > the hwrng.
> > >
> > > Hi! When applying a patch, could you please disable this rng for n900?
> > >
> > > In omap3-n900.dts for rng should be status = "disabled" (as Tony already
> > > wrote), similarly like for aes.
> >
> > Yeah I'll post a proper patch after -rc1.
>
> FYI,
>
> By putting your node into omap34xx.dtsi and omap36xx.dtsi along with
> the following, I can get the RNG to work on an OMAP3530 and a DM3730.
>
>
> diff --git a/arch/arm/boot/dts/omap34xx-omap36xx-clocks.dtsi
> b/arch/arm/boot/dts/omap34xx-omap36xx-clocks.dtsi
> index 5e9d1afcd422..73f351e6d132 100644
> --- a/arch/arm/boot/dts/omap34xx-omap36xx-clocks.dtsi
> +++ b/arch/arm/boot/dts/omap34xx-omap36xx-clocks.dtsi
> @@ -259,7 +259,7 @@
>                          <&i2c1_ick>, <&uart2_ick>, <&uart1_ick>, <&gpt11_ick>,
>                          <&gpt10_ick>, <&mcbsp5_ick>, <&mcbsp1_ick>,
>                          <&omapctrl_ick>, <&aes2_ick>, <&sha12_ick>, <&icr_ick>,
> -                        <&des2_ick>, <&mspro_ick>, <&mailboxes_ick>,
> +                        <&des2_ick>, <&mspro_ick>, <&mailboxes_ick>,
> <&rng_ick>,
>                          <&mspro_fck>;
>         };
>  };
>
> I tried doing the same for am3517, but it doesn't appear to work.  In
> fact, the board hangs at boot with no splat, so I assume that some
> clock isn't running and causing a hang.  Figure 4-50 in the AM3517 TRM
> shows the security_l4_iclk2, so I wonder if the HW mods for AES, SHA,
> etc are doing something to enable this clock.  Those HWmods are
> disabled on AM3517.  I tried turning on the hwmods for them before
> without success, but I'll try it again.

According to a note in omap_hwmod_3xxx_data.c,

/*
 * Apparently the SHA/MD5 and AES accelerator IP blocks are
 * only present on some AM35xx chips, and no one knows which
 * ones.  See
 * http://www.spinics.net/lists/arm-kernel/msg215466.html So
 * if you need these IP blocks on an AM35xx, try uncommenting
 * the following lines.
 */

I decided to uncomment the hwmod entries, and I got the following:

[    0.263222] omap_hwmod: sham: _wait_target_ready failed: -16
[    0.263248] omap_hwmod: sham: cannot be enabled for reset (3)
[    0.265837] omap_hwmod: aes: _wait_target_ready failed: -16
[    0.265851] omap_hwmod: aes: cannot be enabled for reset (3)
[    6.208866] omap_hwmod: sham: _wait_target_ready failed: -16
[    6.287732] omap_hwmod: aes: _wait_target_ready failed: -16

Based on this, I wonder if the sham and aes modules are not present.
If this is the case, it might explain why I cannot use the rng either.

adam
>
> adam
> >
> > Regards,
> >
> > Tony
Pali Rohár Sept. 10, 2019, 4:39 p.m. UTC | #12
On Tuesday 10 September 2019 11:21:34 Adam Ford wrote:
> According to a note in omap_hwmod_3xxx_data.c,
> 
> /*
>  * Apparently the SHA/MD5 and AES accelerator IP blocks are
>  * only present on some AM35xx chips, and no one knows which
>  * ones.  See
>  * http://www.spinics.net/lists/arm-kernel/msg215466.html So
>  * if you need these IP blocks on an AM35xx, try uncommenting
>  * the following lines.
>  */
> 
> I decided to uncomment the hwmod entries, and I got the following:
> 
> [    0.263222] omap_hwmod: sham: _wait_target_ready failed: -16
> [    0.263248] omap_hwmod: sham: cannot be enabled for reset (3)
> [    0.265837] omap_hwmod: aes: _wait_target_ready failed: -16
> [    0.265851] omap_hwmod: aes: cannot be enabled for reset (3)
> [    6.208866] omap_hwmod: sham: _wait_target_ready failed: -16
> [    6.287732] omap_hwmod: aes: _wait_target_ready failed: -16

Hi! Same errors I got in qemu-n900, but not on real N900. So I guess
those errors means that IP blocks are not present.

> Based on this, I wonder if the sham and aes modules are not present.
> If this is the case, it might explain why I cannot use the rng either.

Probably this is the reason, you do not have crypto/rng HW engine.
Adam Ford Oct. 22, 2019, 12:13 p.m. UTC | #13
On Mon, Sep 9, 2019 at 11:35 AM Tony Lindgren <tony@atomide.com> wrote:
>
> * Pali Rohár <pali.rohar@gmail.com> [190909 13:41]:
> > On Monday 09 September 2019 08:37:09 Adam Ford wrote:
> > > I applied this on 5.3 and it is working.  I assume the same is true in for-next.
>
> Hmm I noticed I stopped getting RNG data after several rmmod modprobe
> cycles, or several hd /dev/random reads. Anybody else seeing that?
>
> > > Do you want to submit a formal patch?  I  can mark it as 'tested-by'
> > > This really helps speed up the startup sequence on boards with sshd
> > > because it delays for nearly 80 seconds waiting for entropy without
> > > the hwrng.
> >
> > Hi! When applying a patch, could you please disable this rng for n900?
> >
> > In omap3-n900.dts for rng should be status = "disabled" (as Tony already
> > wrote), similarly like for aes.
>
> Yeah I'll post a proper patch after -rc1.

Tony,

I am just following up on this.  Without the HWRNG there are some
tools and daemons like sshd which wait a long time at startup.  The
patch you sent really helps speed up the startup time in these cases.
At least in my case, it shaves 80 seconds off by eliminating the
delays.

Do you think you'll have time to post a more formal patch?  If not, I
can do it.  I just don't know which mailing list is the more
appropriate.  I was able to verity your patch on a DM3730 and OMAP3530

Thanks,

adam


>
> Regards,
>
> Tony
Tony Lindgren Oct. 22, 2019, 4:06 p.m. UTC | #14
* Adam Ford <aford173@gmail.com> [191022 12:13]:
> On Mon, Sep 9, 2019 at 11:35 AM Tony Lindgren <tony@atomide.com> wrote:
> >
> > * Pali Rohár <pali.rohar@gmail.com> [190909 13:41]:
> > > On Monday 09 September 2019 08:37:09 Adam Ford wrote:
> > > > I applied this on 5.3 and it is working.  I assume the same is true in for-next.
> >
> > Hmm I noticed I stopped getting RNG data after several rmmod modprobe
> > cycles, or several hd /dev/random reads. Anybody else seeing that?
> >
> > > > Do you want to submit a formal patch?  I  can mark it as 'tested-by'
> > > > This really helps speed up the startup sequence on boards with sshd
> > > > because it delays for nearly 80 seconds waiting for entropy without
> > > > the hwrng.
> > >
> > > Hi! When applying a patch, could you please disable this rng for n900?
> > >
> > > In omap3-n900.dts for rng should be status = "disabled" (as Tony already
> > > wrote), similarly like for aes.
> >
> > Yeah I'll post a proper patch after -rc1.
> 
> Tony,
> 
> I am just following up on this.  Without the HWRNG there are some
> tools and daemons like sshd which wait a long time at startup.  The
> patch you sent really helps speed up the startup time in these cases.
> At least in my case, it shaves 80 seconds off by eliminating the
> delays.
> 
> Do you think you'll have time to post a more formal patch?  If not, I
> can do it.  I just don't know which mailing list is the more
> appropriate.  I was able to verity your patch on a DM3730 and OMAP3530

Yeah sorry for the delays, I'll post it when I get a chance to
work on that again. I need to first deal with the other pending
patches for v5.5, and we've had quite a few fixes for v5.4.

Regards,

Tony
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/omap36xx-omap3430es2plus-clocks.dtsi b/arch/arm/boot/dts/omap36xx-omap3430es2plus-clocks.dtsi
index 945537aee3ca..05891dff7fa1 100644
--- a/arch/arm/boot/dts/omap36xx-omap3430es2plus-clocks.dtsi
+++ b/arch/arm/boot/dts/omap36xx-omap3430es2plus-clocks.dtsi
@@ -189,7 +189,7 @@ 
 			 <&mcspi2_ick>, <&mcspi1_ick>, <&i2c3_ick>, <&i2c2_ick>,
 			 <&i2c1_ick>, <&uart2_ick>, <&uart1_ick>, <&gpt11_ick>,
 			 <&gpt10_ick>, <&mcbsp5_ick>, <&mcbsp1_ick>,
-			 <&omapctrl_ick>, <&aes2_ick>, <&sha12_ick>,
+			 <&omapctrl_ick>, <&aes2_ick>, <&sha12_ick>, <&rng_ick>,
 			 <&ssi_ick>;
 	};
 };
diff --git a/arch/arm/mach-omap2/cm-regbits-34xx.h b/arch/arm/mach-omap2/cm-regbits-34xx.h
index 037529a9e969..82330a66e35c 100644
--- a/arch/arm/mach-omap2/cm-regbits-34xx.h
+++ b/arch/arm/mach-omap2/cm-regbits-34xx.h
@@ -17,6 +17,7 @@ 
 #define OMAP3430_CLKACTIVITY_IVA2_MASK			(1 << 0)
 #define OMAP3430_CLKTRCTRL_MPU_MASK			(0x3 << 0)
 #define OMAP3430_ST_AES2_SHIFT				28
+#define OMAP34XX_ST_RNG_SHIFT                           2
 #define OMAP3430_ST_SHA12_SHIFT				27
 #define AM35XX_ST_UART4_SHIFT				23
 #define OMAP3430_ST_HDQ_SHIFT				22
diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index f52438bdfc14..bae4487383b6 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -1627,6 +1627,42 @@  static struct omap_hwmod omap3xxx_gpmc_hwmod = {
 	.flags		= HWMOD_NO_IDLEST | DEBUG_OMAP_GPMC_HWMOD_FLAGS,
 };
 
+/* RNG */
+
+static struct omap_hwmod_class_sysconfig omap3_rng_sysc = {
+	.rev_offs	= 0x3c,
+	.sysc_offs	= 0x40,
+	.syss_offs	= 0x44,
+	.sysc_flags	= (SYSC_HAS_SOFTRESET | SYSC_HAS_AUTOIDLE |
+			   SYSS_HAS_RESET_STATUS),
+	.sysc_fields	= &omap_hwmod_sysc_type1,
+};
+
+static struct omap_hwmod_class omap3_rng_hwmod_class = {
+	.name		= "rng",
+	.sysc		= &omap3_rng_sysc,
+};
+
+struct omap_hwmod omap3xxx_rng_hwmod = {
+	.name		= "rng",
+	.main_clk	= "rng_ick",
+	.prcm		= {
+		.omap2 = {
+			.module_offs = CORE_MOD,
+			.idlest_reg_id = 4,
+			.idlest_idle_bit = OMAP34XX_ST_RNG_SHIFT,
+		},
+	},
+	/*
+	 * XXX The first read from the SYSSTATUS register of the RNG
+	 * after the SYSCONFIG SOFTRESET bit is set triggers an
+	 * imprecise external abort.  It's unclear why this happens.
+	 * Until this is analyzed, skip the IP block reset.
+	 */
+	.flags		= HWMOD_INIT_NO_RESET,
+	.class		= &omap3_rng_hwmod_class,
+};
+
 /*
  * interfaces
  */
@@ -2508,6 +2544,13 @@  static struct omap_hwmod omap3xxx_sham_hwmod = {
 	.class		= &omap3xxx_sham_class,
 };
 
+/* l4_core -> rng */
+struct omap_hwmod_ocp_if omap3xxx_l4_core__rng = {
+	.master		= &omap3xxx_l4_core_hwmod,
+	.slave		= &omap3xxx_rng_hwmod,
+	.clk		= "rng_ick",
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
 
 static struct omap_hwmod_ocp_if omap3xxx_l4_core__sham = {
 	.master		= &omap3xxx_l4_core_hwmod,
@@ -2769,6 +2812,7 @@  static struct omap_hwmod_ocp_if *omap36xx_hwmod_ocp_ifs[] __initdata = {
 	&omap3xxx_l4_core__mmu_isp,
 	&omap3xxx_l3_main__mmu_iva,
 	&omap3xxx_l4_core__ssi,
+	&omap3xxx_l4_core__rng,
 	NULL,
 };
 
@@ -2788,6 +2832,7 @@  static struct omap_hwmod_ocp_if *am35xx_hwmod_ocp_ifs[] __initdata = {
 	&am35xx_l4_core__mdio,
 	&am35xx_emac__l3,
 	&am35xx_l4_core__emac,
+	&omap3xxx_l4_core__rng,
 	NULL,
 };