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

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

[Steffen Trumtrar]

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

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 Documentation/devicetree/bindings/rng/mxc_rngc.txt | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rng/mxc_rngc.txt

diff --git a/Documentation/devicetree/bindings/rng/mxc_rngc.txt b/Documentation/devicetree/bindings/rng/mxc_rngc.txt
new file mode 100644
index 000000000000..e147a6dde40a
--- /dev/null
+++ b/Documentation/devicetree/bindings/rng/mxc_rngc.txt
@@ -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>;
+};
-- 
2.7.0

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

[Steffen Trumtrar]

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

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 arch/arm/boot/dts/imx25.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi
index cde329e9b9e3..ec44ed125057 100644
--- a/arch/arm/boot/dts/imx25.dtsi
+++ b/arch/arm/boot/dts/imx25.dtsi
@@ -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>;
-- 
2.7.0

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

[Shawn Guo]

On Mon, Feb 29, 2016 at 04:52:35PM +0100, Steffen Trumtrar wrote:
> Add a devicetree entry for the Random Number Generator Version C (RNGC).
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---
>  arch/arm/boot/dts/imx25.dtsi | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi
> index cde329e9b9e3..ec44ed125057 100644
> --- a/arch/arm/boot/dts/imx25.dtsi
> +++ b/arch/arm/boot/dts/imx25.dtsi
> @@ -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

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

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

[Steffen Trumtrar]

The driver is ported from Freescales Linux git and can be
found in the

	vendor/freescale/imx_2.6.35_maintain

branch.

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 <s.trumtrar@pengutronix.de>
---
 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

diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index dbf22719462f..9d6b5c42255b 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -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
diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
index 5ad397635128..008463bcf662 100644
--- a/drivers/char/hw_random/Makefile
+++ b/drivers/char/hw_random/Makefile
@@ -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
diff --git a/drivers/char/hw_random/mxc-rngc.c b/drivers/char/hw_random/mxc-rngc.c
new file mode 100644
index 000000000000..e31d306dcacd
--- /dev/null
+++ b/drivers/char/hw_random/mxc-rngc.c
@@ -0,0 +1,400 @@
+/*
+ * RNG driver for Freescale RNGC
+ *
+ * Copyright (C) 2008-2012 Freescale Semiconductor, Inc.
+ */
+
+/*
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+/*
+ * Hardware driver for the Intel/AMD/VIA Random Number Generators (RNG)
+ * (c) Copyright 2003 Red Hat Inc <jgarzik@redhat.com>
+ *
+ * derived from
+ *
+ * Hardware driver for the AMD 768 Random Number Generator (RNG)
+ * (c) Copyright 2001 Red Hat Inc <alan@redhat.com>
+ *
+ * derived from
+ *
+ * Hardware driver for Intel i810 Random Number Generator (RNG)
+ * Copyright 2000,2001 Jeff Garzik <jgarzik@pobox.com>
+ * Copyright 2000,2001 Philipp Rumpf <prumpf@mandrakesoft.com>
+ *
+ * This file is licensed under  the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/interrupt.h>
+#include <linux/hw_random.h>
+#include <linux/completion.h>
+#include <linux/io.h>
+
+#define RNGC_VERSION_MAJOR3 3
+
+#define RNGC_VERSION_ID				0x0000
+#define RNGC_COMMAND				0x0004
+#define RNGC_CONTROL				0x0008
+#define RNGC_STATUS				0x000C
+#define RNGC_ERROR				0x0010
+#define RNGC_FIFO				0x0014
+#define RNGC_VERIF_CTRL				0x0020
+#define RNGC_OSC_CTRL_COUNT			0x0028
+#define RNGC_OSC_COUNT				0x002C
+#define RNGC_OSC_COUNT_STATUS			0x0030
+
+#define RNGC_VERID_ZEROS_MASK			0x0f000000
+#define RNGC_VERID_RNG_TYPE_MASK		0xf0000000
+#define RNGC_VERID_RNG_TYPE_SHIFT		28
+#define RNGC_VERID_CHIP_VERSION_MASK		0x00ff0000
+#define RNGC_VERID_CHIP_VERSION_SHIFT		16
+#define RNGC_VERID_VERSION_MAJOR_MASK		0x0000ff00
+#define RNGC_VERID_VERSION_MAJOR_SHIFT		8
+#define RNGC_VERID_VERSION_MINOR_MASK		0x000000ff
+#define RNGC_VERID_VERSION_MINOR_SHIFT		0
+
+#define RNGC_CMD_ZEROS_MASK			0xffffff8c
+#define RNGC_CMD_SW_RST				0x00000040
+#define RNGC_CMD_CLR_ERR			0x00000020
+#define RNGC_CMD_CLR_INT			0x00000010
+#define RNGC_CMD_SEED				0x00000002
+#define RNGC_CMD_SELF_TEST			0x00000001
+
+#define RNGC_CTRL_ZEROS_MASK			0xfffffc8c
+#define RNGC_CTRL_CTL_ACC			0x00000200
+#define RNGC_CTRL_VERIF_MODE			0x00000100
+#define RNGC_CTRL_MASK_ERROR			0x00000040
+
+#define RNGC_CTRL_MASK_DONE			0x00000020
+#define RNGC_CTRL_AUTO_SEED			0x00000010
+#define RNGC_CTRL_FIFO_UFLOW_MASK		0x00000003
+#define RNGC_CTRL_FIFO_UFLOW_SHIFT		0
+
+#define RNGC_CTRL_FIFO_UFLOW_ZEROS_ERROR	0
+#define RNGC_CTRL_FIFO_UFLOW_ZEROS_ERROR2	1
+#define RNGC_CTRL_FIFO_UFLOW_BUS_XFR		2
+#define RNGC_CTRL_FIFO_UFLOW_ZEROS_INTR		3
+
+#define RNGC_STATUS_ST_PF_MASK			0x00c00000
+#define RNGC_STATUS_ST_PF_SHIFT			22
+#define RNGC_STATUS_ST_PF_TRNG			0x00800000
+#define RNGC_STATUS_ST_PF_PRNG			0x00400000
+#define RNGC_STATUS_ERROR			0x00010000
+#define RNGC_STATUS_FIFO_SIZE_MASK		0x0000f000
+#define RNGC_STATUS_FIFO_SIZE_SHIFT		12
+#define RNGC_STATUS_FIFO_LEVEL_MASK		0x00000f00
+#define RNGC_STATUS_FIFO_LEVEL_SHIFT		8
+#define RNGC_STATUS_NEXT_SEED_DONE		0x00000040
+#define RNGC_STATUS_SEED_DONE			0x00000020
+#define RNGC_STATUS_ST_DONE			0x00000010
+#define RNGC_STATUS_RESEED			0x00000008
+#define RNGC_STATUS_SLEEP			0x00000004
+#define RNGC_STATUS_BUSY			0x00000002
+#define RNGC_STATUS_SEC_STATE			0x00000001
+
+#define RNGC_ERROR_STATUS_ZEROS_MASK		0xffffffc0
+#define RNGC_ERROR_STATUS_BAD_KEY		0x00000040
+#define RNGC_ERROR_STATUS_RAND_ERR		0x00000020
+#define RNGC_ERROR_STATUS_FIFO_ERR		0x00000010
+#define RNGC_ERROR_STATUS_STAT_ERR		0x00000008
+#define RNGC_ERROR_STATUS_ST_ERR		0x00000004
+#define RNGC_ERROR_STATUS_OSC_ERR		0x00000002
+#define RNGC_ERROR_STATUS_LFSR_ERR		0x00000001
+
+#define RNG_ADDR_RANGE				0x34
+
+struct mxc_rngc {
+	struct device		*dev;
+	struct clk		*clk;
+	void __iomem		*base;
+	unsigned int		irq;
+	struct hwrng		rng;
+	struct completion	rng_self_testing;
+	struct completion	rng_seed_done;
+};
+
+static int mxc_rngc_read(struct hwrng *rng, void *data, size_t max, bool wait)
+{
+	struct mxc_rngc *rngc = container_of(rng, struct mxc_rngc, rng);
+	unsigned int status;
+	unsigned int level;
+	int retval = 0;
+
+	while (max > sizeof(u32)) {
+		status = __raw_readl(rngc->base + RNGC_STATUS);
+		/* how many random numbers are in FIFO? [0-16] */
+		level = (status & RNGC_STATUS_FIFO_LEVEL_MASK) >>
+			RNGC_STATUS_FIFO_LEVEL_SHIFT;
+
+		/* is there some error while reading this random number? */
+		if (status & RNGC_STATUS_ERROR)
+			break;
+
+		if (level) {
+			/* retrieve a random number from FIFO */
+			*(u32 *)data = __raw_readl(rngc->base + RNGC_FIFO);
+
+			retval += sizeof(u32);
+			data += sizeof(u32);
+			max -= sizeof(u32);
+		}
+	}
+
+	return retval ? retval : -EIO;
+}
+
+static irqreturn_t rngc_irq(int irq, void *priv)
+{
+	struct mxc_rngc *rngc = (struct mxc_rngc *)priv;
+	int handled = IRQ_NONE;
+
+	/* is the seed creation done? */
+	if (__raw_readl(rngc->base + RNGC_STATUS) & RNGC_STATUS_SEED_DONE) {
+		complete(&rngc->rng_seed_done);
+		__raw_writel(RNGC_CMD_CLR_INT | RNGC_CMD_CLR_ERR,
+			     rngc->base + RNGC_COMMAND);
+		handled = IRQ_HANDLED;
+	}
+
+	/* is the self test done? */
+	if (__raw_readl(rngc->base + RNGC_STATUS) & RNGC_STATUS_ST_DONE) {
+		complete(&rngc->rng_self_testing);
+		__raw_writel(RNGC_CMD_CLR_INT | RNGC_CMD_CLR_ERR,
+			     rngc->base + RNGC_COMMAND);
+		handled = IRQ_HANDLED;
+	}
+
+	/* is there any error? */
+	if (__raw_readl(rngc->base + RNGC_STATUS) & RNGC_STATUS_ERROR) {
+		/* clear interrupt */
+		__raw_writel(RNGC_CMD_CLR_INT | RNGC_CMD_CLR_ERR,
+			     rngc->base + RNGC_COMMAND);
+		handled = IRQ_HANDLED;
+	}
+
+	return handled;
+}
+
+static int mxc_rngc_init(struct hwrng *rng)
+{
+	struct mxc_rngc *rngc = container_of(rng, struct mxc_rngc, rng);
+	u32 cmd;
+	u32 ctrl;
+	u32 osc;
+	int err;
+
+	err = __raw_readl(rngc->base + RNGC_STATUS) & RNGC_STATUS_ERROR;
+	if (err) {
+		/* is this a bad keys error ? */
+		if (__raw_readl(rngc->base + RNGC_ERROR) &
+		    RNGC_ERROR_STATUS_BAD_KEY) {
+			dev_err(rngc->dev, "Can't start, Bad Keys.\n");
+			return -EIO;
+		}
+	}
+
+	/* mask all interrupts, will be unmasked soon */
+	ctrl = __raw_readl(rngc->base + RNGC_CONTROL);
+	__raw_writel(ctrl | RNGC_CTRL_MASK_DONE | RNGC_CTRL_MASK_ERROR,
+		     rngc->base + RNGC_CONTROL);
+
+	/* verify if oscillator is working */
+	osc = __raw_readl(rngc->base + RNGC_ERROR);
+	if (osc & RNGC_ERROR_STATUS_OSC_ERR) {
+		dev_err(rngc->dev, "RNGC Oscillator is dead!\n");
+		return -EIO;
+	}
+
+	err = devm_request_irq(rngc->dev, rngc->irq, rngc_irq, 0, rng->name,
+			       (void *)rngc);
+	if (err) {
+		dev_err(rngc->dev, "Can't get interrupt working.\n");
+		return -EIO;
+	}
+
+	/* do self test, repeat until get success */
+	do {
+		/* clear error */
+		cmd = __raw_readl(rngc->base + RNGC_COMMAND);
+		__raw_writel(cmd | RNGC_CMD_CLR_ERR, rngc->base + RNGC_COMMAND);
+
+		/* unmask all interrupt */
+		ctrl = __raw_readl(rngc->base + RNGC_CONTROL);
+		__raw_writel(ctrl & ~(RNGC_CTRL_MASK_DONE |
+				      RNGC_CTRL_MASK_ERROR),
+			     rngc->base + RNGC_CONTROL);
+
+		/* run self test */
+		cmd = __raw_readl(rngc->base + RNGC_COMMAND);
+		__raw_writel(cmd | RNGC_CMD_SELF_TEST,
+			     rngc->base + RNGC_COMMAND);
+
+		wait_for_completion(&rngc->rng_self_testing);
+
+	} while (__raw_readl(rngc->base + RNGC_ERROR) &
+		 RNGC_ERROR_STATUS_ST_ERR);
+
+	/* clear interrupt. Is it really necessary here? */
+	__raw_writel(RNGC_CMD_CLR_INT | RNGC_CMD_CLR_ERR,
+		     rngc->base + RNGC_COMMAND);
+
+	/* create seed, repeat while there is some statistical error */
+	do {
+		/* clear error */
+		cmd = __raw_readl(rngc->base + RNGC_COMMAND);
+		__raw_writel(cmd | RNGC_CMD_CLR_ERR, rngc->base + RNGC_COMMAND);
+
+		/* seed creation */
+		cmd = __raw_readl(rngc->base + RNGC_COMMAND);
+		__raw_writel(cmd | RNGC_CMD_SEED, rngc->base + RNGC_COMMAND);
+
+		wait_for_completion(&rngc->rng_seed_done);
+
+	} while (__raw_readl(rngc->base + RNGC_ERROR) &
+		 RNGC_ERROR_STATUS_STAT_ERR);
+
+	err = __raw_readl(rngc->base + RNGC_ERROR) &
+		(RNGC_ERROR_STATUS_STAT_ERR |
+		 RNGC_ERROR_STATUS_RAND_ERR |
+		 RNGC_ERROR_STATUS_FIFO_ERR |
+		 RNGC_ERROR_STATUS_ST_ERR |
+		 RNGC_ERROR_STATUS_OSC_ERR |
+		 RNGC_ERROR_STATUS_LFSR_ERR);
+
+	if (err) {
+		dev_err(rngc->dev, "FSL RNGC appears inoperable.\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int mxc_rngc_probe(struct platform_device *pdev)
+{
+	struct mxc_rngc *rngc;
+	struct resource *res;
+	int ret;
+
+	rngc = devm_kzalloc(&pdev->dev, sizeof(*rngc), GFP_KERNEL);
+	if (!rngc)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	rngc->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(rngc->base))
+		return PTR_ERR(rngc->base);
+
+	rngc->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(rngc->clk)) {
+		dev_err(&pdev->dev, "Can not get rng_clk\n");
+		return PTR_ERR(rngc->clk);
+	}
+
+	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;
+	}
+
+
+	init_completion(&rngc->rng_self_testing);
+	init_completion(&rngc->rng_seed_done);
+
+	rngc->rng.name = pdev->name;
+	rngc->rng.init = mxc_rngc_init;
+	rngc->rng.read = mxc_rngc_read;
+
+	rngc->dev = &pdev->dev;
+	platform_set_drvdata(pdev, rngc);
+
+	ret = hwrng_register(&rngc->rng);
+	if (ret) {
+		dev_err(&pdev->dev, "FSL RNGC registering failed (%d)\n", ret);
+		clk_disable_unprepare(rngc->clk);
+
+		return ret;
+	}
+
+	dev_info(&pdev->dev, "Freescale RNGC Registered.\n");
+
+	return 0;
+}
+
+static int mxc_rngc_remove(struct platform_device *pdev)
+{
+	struct mxc_rngc *rngc = platform_get_drvdata(pdev);
+
+	hwrng_unregister(&rngc->rng);
+
+	clk_disable_unprepare(rngc->clk);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int mxc_rngc_suspend(struct device *dev)
+{
+	struct mxc_rngc *rngc = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(rngc->clk);
+
+	return 0;
+}
+
+static int mxc_rngc_resume(struct device *dev)
+{
+	struct mxc_rngc *rngc = dev_get_drvdata(dev);
+
+	clk_prepare_enable(rngc->clk);
+
+	return 0;
+}
+
+static const struct dev_pm_ops mxc_rngc_pm_ops = {
+	.suspend	= mxc_rngc_suspend,
+	.resume		= mxc_rngc_resume,
+};
+#endif
+
+static const struct of_device_id mxc_rngc_dt_ids[] = {
+	{ .compatible = "fsl,imx25-rng", .data = NULL, },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mxc_rngc_dt_ids);
+
+static struct platform_driver mxc_rngc_driver = {
+	.probe = mxc_rngc_probe,
+	.remove = mxc_rngc_remove,
+	.driver = {
+		.name = "mxc_rngc",
+#ifdef CONFIG_PM
+		.pm = &mxc_rngc_pm_ops,
+#endif
+		.of_match_table = mxc_rngc_dt_ids,
+	},
+};
+
+module_platform_driver(mxc_rngc_driver);
+MODULE_AUTHOR("Freescale Semiconductor, Inc.");
+MODULE_DESCRIPTION("H/W RNGC driver for i.MX");
+MODULE_LICENSE("GPL");
-- 
2.7.0

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

[Fabio Estevam]

On Mon, Feb 29, 2016 at 12:52 PM, Steffen Trumtrar
<s.trumtrar@pengutronix.de> wrote:

> +       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;
       }

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

[Uwe Kleine-König]

On Mon, Feb 29, 2016 at 06:16:50PM -0300, Fabio Estevam wrote:
> On Mon, Feb 29, 2016 at 12:52 PM, Steffen Trumtrar
> <s.trumtrar@pengutronix.de> wrote:
> 
> > +       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) {

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

>                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

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

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

[Fabio Estevam]

On Mon, Feb 29, 2016 at 6:38 PM, Uwe Kleine-K?nig
<u.kleine-koenig@pengutronix.de> wrote:
> On Mon, Feb 29, 2016 at 06:16:50PM -0300, Fabio Estevam wrote:
>> On Mon, Feb 29, 2016 at 12:52 PM, Steffen Trumtrar
>> <s.trumtrar@pengutronix.de> wrote:
>>
>> > +       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) {
>
> rngc->irq is unsigned, so this is never true.
>
>>                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;
         }

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

[Uwe Kleine-König]

Hello Fabio,

On Mon, Feb 29, 2016 at 08:54:19PM -0300, Fabio Estevam wrote:
> On Mon, Feb 29, 2016 at 6:38 PM, Uwe Kleine-K?nig
> <u.kleine-koenig@pengutronix.de> wrote:
> > On Mon, Feb 29, 2016 at 06:16:50PM -0300, Fabio Estevam wrote:
> >> On Mon, Feb 29, 2016 at 12:52 PM, Steffen Trumtrar
> >> <s.trumtrar@pengutronix.de> wrote:
> >>
> >> > +       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) {
> >
> > rngc->irq is unsigned, so this is never true.
> >
> >>                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.

> 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

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

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

[Steffen Trumtrar]

Hi!

On Tue, Mar 01, 2016 at 08:49:37AM +0100, Uwe Kleine-K?nig wrote:
> Hello Fabio,
> 
> On Mon, Feb 29, 2016 at 08:54:19PM -0300, Fabio Estevam wrote:
> > On Mon, Feb 29, 2016 at 6:38 PM, Uwe Kleine-K?nig
> > <u.kleine-koenig@pengutronix.de> wrote:
> > > On Mon, Feb 29, 2016 at 06:16:50PM -0300, Fabio Estevam wrote:
> > >> On Mon, Feb 29, 2016 at 12:52 PM, Steffen Trumtrar
> > >> <s.trumtrar@pengutronix.de> wrote:
> > >>
> > >> > +       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) {
> > >
> > > rngc->irq is unsigned, so this is never true.
> > >
> > >>                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.
> 

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.

> > 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.

> 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

-- 
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 |

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

[Rob Herring]

On Mon, Feb 29, 2016 at 04:52:34PM +0100, Steffen Trumtrar wrote:
> Add binding documentation for the Freescale RNGC found on
> some i.MX2/3/5 SoCs.
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---
>  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 <robh@kernel.org>