Documentation: devicetree: add Freescale RNGC binding

PATCH Superseded Steffen Trumtrar 5 participants 10 messages Feb 29, 2016 – Mar 7, 2016

Subsystems (4)

Device tree bindings Arm i.MX and Layerscape Crypto API Hardware random number generator core

lore

Share & tools

lore

Steffen Trumtrar

author

2016-02-29 15:52 UTC #0 lore raw

PATCH 1/3 Documentation: devicetree: add Freescale RNGC binding

Add binding documentation for the Freescale RNGC found on some i.MX2/3/5 SoCs.

Signed-off-by Steffen Trumtrar <[email protected]>

---
 Documentation/devicetree/bindings/rng/mxc_rngc.txt | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rng/mxc_rngc.txt

Documentation/devicetree/bindings/rng/mxc_rngc.txt new file +16 -0

new file mode 100644
index 000000000000..e147a6dde40a
@@ -0,0 +1,16 @@
+Freescale RNGC (Random Number Generator Version C)
+
+Required properties:
+- compatible : Should be "fsl,imx25-rng"
+- reg : Offset and length of the register set of this block
+- interrupts : the interrupt number for the RNG block
+- clocks: should contain the RNG clk source
+
+Example:
+
+rng at 53fb0000 {
+	compatible = "fsl,imx25-rng";
+	reg = <0x53fb0000 0x4000>;
+	interrupts = <22>;
+	clocks = <&clks 109>;
+};

Signature · 1 line

2.7.0

Steffen Trumtrar

author

2016-02-29 15:52 UTC #1 lore raw

PATCH 2/3 ARM: i.MX25: add RNGC node to dtsi

Add a devicetree entry for the Random Number Generator Version C (RNGC).

Signed-off-by Steffen Trumtrar <[email protected]>

---
 arch/arm/boot/dts/imx25.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

arch/arm/boot/dts/imx25.dtsi +9 -0

index cde329e9b9e3..ec44ed125057 100644
@@ -397,6 +397,15 @@
 				interrupts = <41>;
 			};
 
+			rng: rng at 53fb0000 {
+				compatible = "fsl,imx25-rng";
+				reg = <0x53fb0000 0x4000>;
+				clocks = <&clks 109>;
+				clock-names = "ipg";
+				interrupts = <22>;
+				interrupt-names = "rng";
+			};
+
 			esdhc1: esdhc at 53fb4000 {
 				compatible = "fsl,imx25-esdhc";
 				reg = <0x53fb4000 0x4000>;

Signature · 1 line

2.7.0

Shawn Guo

2016-03-01 13:37 UTC #2 lore raw

PATCH 2/3 ARM: i.MX25: add RNGC node to dtsi

Quoted 12 lines · Steffen Trumtrar

Add a devicetree entry for the Random Number Generator Version C (RNGC).
Signed-off-by Steffen Trumtrar <[email protected]>
---
 arch/arm/boot/dts/imx25.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)
arch/arm/boot/dts/imx25.dtsi +0 -0
index cde329e9b9e3..ec44ed125057 100644

arch/arm/boot/dts/imx25.dtsi line 406

Quoted 10 lines

arch/arm/boot/dts/imx25.dtsi +7 -0

@@ -397,6 +397,15 @@
 				interrupts = <41>;
 			};
 
+			rng: rng at 53fb0000 {
+				compatible = "fsl,imx25-rng";
+				reg = <0x53fb0000 0x4000>;
+				clocks = <&clks 109>;
+				clock-names = "ipg";
+				interrupts = <22>;
+				interrupt-names = "rng";

I do not see interrupt-names in the binding doc (patch #1). IMO, it should be dropped.

Shawn

Quoted 9 lines

arch/arm/boot/dts/imx25.dtsi +2 -0

+			};
+
 			esdhc1: esdhc at 53fb4000 {
 				compatible = "fsl,imx25-esdhc";
 				reg = <0x53fb4000 0x4000>;

Signature · 1 line

2.7.0

According to that code, the RNGC is found on Freescales i.MX3/5 SoCs. The i.MX2x actually has an RNGB, which has no driver implementation in Freescales kernel. However as it turns out, the driver for the RNGC works fine on the (at least) i.MX25. So, they seem to be somewhat compatible.

Signed-off-by Steffen Trumtrar <[email protected]>

---
 drivers/char/hw_random/Kconfig    |  13 ++
 drivers/char/hw_random/Makefile   |   1 +
 drivers/char/hw_random/mxc-rngc.c | 400 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 414 insertions(+)
 create mode 100644 drivers/char/hw_random/mxc-rngc.c

drivers/char/hw_random/Kconfig +13 -0

index dbf22719462f..9d6b5c42255b 100644
@@ -255,6 +255,19 @@ config HW_RANDOM_MXC_RNGA
 
 	  If unsure, say Y.
 
+config HW_RANDOM_MXC_RNGC
+	tristate "Freescale i.MX RNGC Random Number Generator"
+	depends on ARCH_MXC
+	default HW_RANDOM
+	---help---
+	  This driver provides kernel-side support for the Random Number
+	  Generator hardware found on some Freescale i.MX processors.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called mxc-rngc.
+
+	  If unsure, say Y.
+
 config HW_RANDOM_NOMADIK
 	tristate "ST-Ericsson Nomadik Random Number Generator support"
 	depends on ARCH_NOMADIK

drivers/char/hw_random/Makefile +1 -0

index 5ad397635128..008463bcf662 100644
@@ -20,6 +20,7 @@ obj-$(CONFIG_HW_RANDOM_PASEMI) += pasemi-rng.o
 obj-$(CONFIG_HW_RANDOM_VIRTIO) += virtio-rng.o
 obj-$(CONFIG_HW_RANDOM_TX4939) += tx4939-rng.o
 obj-$(CONFIG_HW_RANDOM_MXC_RNGA) += mxc-rnga.o
+obj-$(CONFIG_HW_RANDOM_MXC_RNGC) += mxc-rngc.o
 obj-$(CONFIG_HW_RANDOM_OCTEON) += octeon-rng.o
 obj-$(CONFIG_HW_RANDOM_NOMADIK) += nomadik-rng.o
 obj-$(CONFIG_HW_RANDOM_PPC4XX) += ppc4xx-rng.o

drivers/char/hw_random/mxc-rngc.c new file +400 -0

400 lines not on this page · raw message

Signature · 1 line

2.7.0

Fabio Estevam

reviewer

2016-02-29 21:16 UTC #4 lore raw

PATCH 3/3 hwrng: mxc-fsl - add support for Freescale RNGC

drivers/char/hw_random/mxc-rngc.c line 316

Quoted 10 lines · Steffen Trumtrar

drivers/char/hw_random/mxc-rngc.c new file +10 -0

+       ret = clk_prepare_enable(rngc->clk);
+       if (ret)
+               return ret;
+
+       rngc->irq = platform_get_irq(pdev, 0);
+       if (!rngc->irq) {
+               dev_err(&pdev->dev, "FSL RNGC couldn't get irq\n");
+               clk_disable_unprepare(rngc->clk);
+
+               return ret;

You are returning the wrong error code here:

Better do like this:

       rngc->irq = platform_get_irq(pdev, 0);
       if (rngc->irq < 0) {
               dev_err(&pdev->dev, "FSL RNGC couldn't get irq\n");
               clk_disable_unprepare(rngc->clk);
               return rngc->irq;
       }

Uwe Kleine-König

2016-02-29 21:38 UTC #5 lore raw

PATCH 3/3 hwrng: mxc-fsl - add support for Freescale RNGC

Quoted 16 lines · Fabio Estevam

drivers/char/hw_random/mxc-rngc.c line 316

Quoted 10 lines · Steffen Trumtrar

drivers/char/hw_random/mxc-rngc.c new file +10 -0

+       ret = clk_prepare_enable(rngc->clk);
+       if (ret)
+               return ret;
+
+       rngc->irq = platform_get_irq(pdev, 0);
+       if (!rngc->irq) {
+               dev_err(&pdev->dev, "FSL RNGC couldn't get irq\n");
+               clk_disable_unprepare(rngc->clk);
+
+               return ret;

You are returning the wrong error code here:

Quoted 4 lines

Better do like this:

       rngc->irq = platform_get_irq(pdev, 0);
       if (rngc->irq < 0) {

rngc->irq is unsigned, so this is never true.

Quoted 4 lines

               dev_err(&pdev->dev, "FSL RNGC couldn't get irq\n");
               clk_disable_unprepare(rngc->clk);
               return rngc->irq;
       }

So here comes my better approach:

	ret = platform_get_irq(pdev, 0);
	if (ret <= 0) {
		if (ret == 0)
			ret = -EINVAL;
		dev_err(&pdev->dev, "FSL RNGC couldn't get irq\n");
		clk_disable_unprepare(rngc->clk);

		return ret;
	}

	rngc->irq = ret;

Best regards
Uwe

Signature · 2 lines

Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Fabio Estevam

reviewer

2016-02-29 23:54 UTC #6 lore raw

PATCH 3/3 hwrng: mxc-fsl - add support for Freescale RNGC

Quoted 22 lines · Uwe Kleine-K?nig

Quoted 16 lines · Fabio Estevam

drivers/char/hw_random/mxc-rngc.c line 316

Quoted 10 lines · Steffen Trumtrar

drivers/char/hw_random/mxc-rngc.c new file +10 -0

+       ret = clk_prepare_enable(rngc->clk);
+       if (ret)
+               return ret;
+
+       rngc->irq = platform_get_irq(pdev, 0);
+       if (!rngc->irq) {
+               dev_err(&pdev->dev, "FSL RNGC couldn't get irq\n");
+               clk_disable_unprepare(rngc->clk);
+
+               return ret;

You are returning the wrong error code here:

Quoted 4 lines

Better do like this:

       rngc->irq = platform_get_irq(pdev, 0);
       if (rngc->irq < 0) {

Quoted 8 lines

rngc->irq is unsigned, so this is never true.
Quoted 4 lines
               dev_err(&pdev->dev, "FSL RNGC couldn't get irq\n");
               clk_disable_unprepare(rngc->clk);
               return rngc->irq;
       }
So here comes my better approach:

As irq is only used inside probe it can be removed from struct mxc_rngc.

Or maybe like this:

         ret = platform_get_irq(pdev, 0);
         if (ret < 0) {
                 dev_err(&pdev->dev, "FSL RNGC couldn't get irq\n");
                 clk_disable_unprepare(rngc->clk);
                 return ret;
         }

Uwe Kleine-König

2016-03-01 07:49 UTC #7 lore raw

PATCH 3/3 hwrng: mxc-fsl - add support for Freescale RNGC

Hello Fabio,

Quoted 26 lines · Fabio Estevam

Quoted 24 lines · Uwe Kleine-K?nig
Quoted 16 lines · Fabio Estevam
drivers/char/hw_random/mxc-rngc.c line 316
Quoted 10 lines · Steffen Trumtrar
drivers/char/hw_random/mxc-rngc.c new file +10 -0
+       ret = clk_prepare_enable(rngc->clk);
+       if (ret)
+               return ret;
+
+       rngc->irq = platform_get_irq(pdev, 0);
+       if (!rngc->irq) {
+               dev_err(&pdev->dev, "FSL RNGC couldn't get irq\n");
+               clk_disable_unprepare(rngc->clk);
+
+               return ret;
You are returning the wrong error code here:
Quoted 4 lines
Better do like this:
       rngc->irq = platform_get_irq(pdev, 0);
       if (rngc->irq < 0) {
rngc->irq is unsigned, so this is never true.

Quoted 8 lines

Quoted 6 lines
Quoted 4 lines
               dev_err(&pdev->dev, "FSL RNGC couldn't get irq\n");
               clk_disable_unprepare(rngc->clk);
               return rngc->irq;
       }
So here comes my better approach:
As irq is only used inside probe it can be removed from struct mxc_rngc.

Good idea.

Quoted 8 lines

Or maybe like this:

         ret = platform_get_irq(pdev, 0);
         if (ret < 0) {
                 dev_err(&pdev->dev, "FSL RNGC couldn't get irq\n");
                 clk_disable_unprepare(rngc->clk);
                 return ret;
         }

Some people think platform_get_irq returning 0 should be handled as error.

Best regards
Uwe

Signature · 2 lines

Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Steffen Trumtrar

author

2016-03-07 13:03 UTC #8 lore raw

PATCH 3/3 hwrng: mxc-fsl - add support for Freescale RNGC

Hi!

Quoted 34 lines · Uwe Kleine-K?nig

Hello Fabio,
Quoted 31 lines · Fabio Estevam
Quoted 29 lines · Uwe Kleine-K?nig
Quoted 16 lines · Fabio Estevam
drivers/char/hw_random/mxc-rngc.c line 316
Quoted 10 lines · Steffen Trumtrar
drivers/char/hw_random/mxc-rngc.c new file +10 -0
+       ret = clk_prepare_enable(rngc->clk);
+       if (ret)
+               return ret;
+
+       rngc->irq = platform_get_irq(pdev, 0);
+       if (!rngc->irq) {
+               dev_err(&pdev->dev, "FSL RNGC couldn't get irq\n");
+               clk_disable_unprepare(rngc->clk);
+
+               return ret;
You are returning the wrong error code here:
Quoted 4 lines
Better do like this:
       rngc->irq = platform_get_irq(pdev, 0);
       if (rngc->irq < 0) {
rngc->irq is unsigned, so this is never true.
Quoted 4 lines
               dev_err(&pdev->dev, "FSL RNGC couldn't get irq\n");
               clk_disable_unprepare(rngc->clk);
               return rngc->irq;
       }

Quoted 6 lines

Quoted 3 lines
Quoted 1 line
So here comes my better approach:
As irq is only used inside probe it can be removed from struct mxc_rngc.
Good idea.

Actually it is currently used in the mxc_rngc_init function, but I think I can just move the call there into the probe function and get rid of the irq in the struct.

Quoted 9 lines

Quoted 8 lines

Or maybe like this:

         ret = platform_get_irq(pdev, 0);
         if (ret < 0) {
                 dev_err(&pdev->dev, "FSL RNGC couldn't get irq\n");
                 clk_disable_unprepare(rngc->clk);
                 return ret;
         }

This looks better, yes. I will change that, of course.

Quoted 2 lines

Some people think platform_get_irq returning 0 should be handled as error.

And who is right? drivers/of/unittest.c checks for irq < 0 for example.

Thanks,
Steffen

Signature · 4 lines

Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Rob Herring

maintainer

2016-03-03 23:56 UTC #9 lore raw

PATCH 1/3 Documentation: devicetree: add Freescale RNGC binding

In reply to Steffen Trumtrar

Quoted 8 lines · Steffen Trumtrar

Add binding documentation for the Freescale RNGC found on some i.MX2/3/5 SoCs.
Signed-off-by Steffen Trumtrar <[email protected]>
---
 Documentation/devicetree/bindings/rng/mxc_rngc.txt | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rng/mxc_rngc.txt

Acked-by Rob Herring <[email protected]>