[PATCH 0/4] hwrng: omap - fixes and improvements

[PATCH 0/4] hwrng: omap - fixes and improvements

[Thomas Petazzoni]

Hello,

This small patch series brings a few fixes and improvements to the
omap_rng driver. The first fix is particularly important, as it fixes
using the driver built as a module on SoCs that require a clock for
the IP to work properly.

Thanks,

Thomas

Thomas Petazzoni (4):
  hwrng: omap - write registers after enabling the clock
  hwrng: omap - use devm_clk_get() instead of of_clk_get()
  hwrng: omap - Do not access INTMASK_REG on EIP76
  hwrng: omap - move clock related code to omap_rng_probe()

 drivers/char/hw_random/omap-rng.c | 36 +++++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 11 deletions(-)

-- 
2.7.4

[PATCH 1/4] hwrng: omap - write registers after enabling the clock

[Thomas Petazzoni]

Commit 383212425c926 ("hwrng: omap - Add device variant for SafeXcel
IP-76 found in Armada 8K") added support for the SafeXcel IP-76 variant
of the IP. This modification included getting a reference and enabling a
clock. Unfortunately, this was done *after* writing to the
RNG_INTMASK_REG register. This generally works fine when the driver is
built-in because the clock might have been left enabled by the
bootloader, but fails short when the driver is built as a module: it
causes a system hang because a register is being accessed while the
clock is not enabled.

This commit fixes that by making the register access *after* enabling
the clock.

This issue was found by the kernelci.org testing effort.

Fixes: 383212425c926 ("hwrng: omap - Add device variant for SafeXcel IP-76 found in Armada 8K")
Cc: <stable@vger.kernel.org>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/char/hw_random/omap-rng.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/char/hw_random/omap-rng.c b/drivers/char/hw_random/omap-rng.c
index 3ad86fd..efa3747 100644
--- a/drivers/char/hw_random/omap-rng.c
+++ b/drivers/char/hw_random/omap-rng.c
@@ -397,7 +397,6 @@ static int of_get_omap_rng_device_details(struct omap_rng_dev *priv,
 				irq, err);
 			return err;
 		}
-		omap_rng_write(priv, RNG_INTMASK_REG, RNG_SHUTDOWN_OFLO_MASK);
 
 		priv->clk = of_clk_get(pdev->dev.of_node, 0);
 		if (IS_ERR(priv->clk) && PTR_ERR(priv->clk) == -EPROBE_DEFER)
@@ -408,6 +407,8 @@ static int of_get_omap_rng_device_details(struct omap_rng_dev *priv,
 				dev_err(&pdev->dev, "unable to enable the clk, "
 						    "err = %d\n", err);
 		}
+
+		omap_rng_write(priv, RNG_INTMASK_REG, RNG_SHUTDOWN_OFLO_MASK);
 	}
 	return 0;
 }
-- 
2.7.4

[PATCH 2/4] hwrng: omap - use devm_clk_get() instead of of_clk_get()

[Thomas Petazzoni]

The omap-rng driver currently uses of_clk_get() to get a reference to
the clock, but never releases that reference. This commit fixes that by
using devm_clk_get() instead.

Fixes: 383212425c926 ("hwrng: omap - Add device variant for SafeXcel IP-76 found in Armada 8K")
Cc: <stable@vger.kernel.org>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/char/hw_random/omap-rng.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/hw_random/omap-rng.c b/drivers/char/hw_random/omap-rng.c
index efa3747..d286628 100644
--- a/drivers/char/hw_random/omap-rng.c
+++ b/drivers/char/hw_random/omap-rng.c
@@ -398,7 +398,7 @@ static int of_get_omap_rng_device_details(struct omap_rng_dev *priv,
 			return err;
 		}
 
-		priv->clk = of_clk_get(pdev->dev.of_node, 0);
+		priv->clk = devm_clk_get(&pdev->dev, NULL);
 		if (IS_ERR(priv->clk) && PTR_ERR(priv->clk) == -EPROBE_DEFER)
 			return -EPROBE_DEFER;
 		if (!IS_ERR(priv->clk)) {
-- 
2.7.4

[PATCH 3/4] hwrng: omap - Do not access INTMASK_REG on EIP76

[Thomas Petazzoni]

The INTMASK_REG register does not exist on EIP76. Due to this, the call:

   omap_rng_write(priv, RNG_INTMASK_REG, RNG_SHUTDOWN_OFLO_MASK);

ends up, through the reg_map_eip76[] array, in accessing the register at
offset 0, which is the RNG_OUTPUT_0_REG. This by itself doesn't cause
any problem, but clearly doesn't enable the interrupt as it was
expected.

On EIP76, the register that allows to enable the interrupt is
RNG_CONTROL_REG. And just like RNG_INTMASK_REG, it's bit 1 of this
register that allows to enable the shutdown_oflo interrupt.

Fixes: 383212425c926 ("hwrng: omap - Add device variant for SafeXcel IP-76 found in Armada 8K")
Cc: <stable@vger.kernel.org>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/char/hw_random/omap-rng.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/char/hw_random/omap-rng.c b/drivers/char/hw_random/omap-rng.c
index d286628..b1ad125 100644
--- a/drivers/char/hw_random/omap-rng.c
+++ b/drivers/char/hw_random/omap-rng.c
@@ -408,7 +408,18 @@ static int of_get_omap_rng_device_details(struct omap_rng_dev *priv,
 						    "err = %d\n", err);
 		}
 
-		omap_rng_write(priv, RNG_INTMASK_REG, RNG_SHUTDOWN_OFLO_MASK);
+		/*
+		 * On OMAP4, enabling the shutdown_oflo interrupt is
+		 * done in the interrupt mask register. There is no
+		 * such register on EIP76, and it's enabled by the
+		 * same bit in the control register
+		 */
+		if (priv->pdata->regs[RNG_INTMASK_REG])
+			omap_rng_write(priv, RNG_INTMASK_REG,
+				       RNG_SHUTDOWN_OFLO_MASK);
+		else
+			omap_rng_write(priv, RNG_CONTROL_REG,
+				       RNG_SHUTDOWN_OFLO_MASK);
 	}
 	return 0;
 }
-- 
2.7.4

[PATCH 4/4] hwrng: omap - move clock related code to omap_rng_probe()

[Thomas Petazzoni]

Currently, the code that takes a reference to the clock and enables it
is located inside of_get_omap_rng_device_details(), called only when
probing through the Device Tree.

However, there is nothing that makes this clock logic dependent on the
Device Tree, so it makes more sense to have it in omap_rng_probe()
directly.

Moreover, we make sure to bail out if we can't enable the clock. Indeed,
while the clock is optional, if a clock is present, we really want to
succeed in enabling it. And we fix the error message to fit on one line,
so that it is grep-friendly.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/char/hw_random/omap-rng.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/char/hw_random/omap-rng.c b/drivers/char/hw_random/omap-rng.c
index b1ad125..74d11ae 100644
--- a/drivers/char/hw_random/omap-rng.c
+++ b/drivers/char/hw_random/omap-rng.c
@@ -398,16 +398,6 @@ static int of_get_omap_rng_device_details(struct omap_rng_dev *priv,
 			return err;
 		}
 
-		priv->clk = devm_clk_get(&pdev->dev, NULL);
-		if (IS_ERR(priv->clk) && PTR_ERR(priv->clk) == -EPROBE_DEFER)
-			return -EPROBE_DEFER;
-		if (!IS_ERR(priv->clk)) {
-			err = clk_prepare_enable(priv->clk);
-			if (err)
-				dev_err(&pdev->dev, "unable to enable the clk, "
-						    "err = %d\n", err);
-		}
-
 		/*
 		 * On OMAP4, enabling the shutdown_oflo interrupt is
 		 * done in the interrupt mask register. There is no
@@ -478,6 +468,18 @@ static int omap_rng_probe(struct platform_device *pdev)
 		goto err_ioremap;
 	}
 
+	priv->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(priv->clk) && PTR_ERR(priv->clk) == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
+	if (!IS_ERR(priv->clk)) {
+		ret = clk_prepare_enable(priv->clk);
+		if (ret) {
+			dev_err(&pdev->dev,
+				"Unable to enable the clk: %d\n", ret);
+			goto err_register;
+		}
+	}
+
 	ret = (dev->of_node) ? of_get_omap_rng_device_details(priv, pdev) :
 				get_omap_rng_device_details(priv);
 	if (ret)
-- 
2.7.4

Re: [PATCH 0/4] hwrng: omap - fixes and improvements

[Jason Cooper]

Hi Thomas,

On Tue, Mar 07, 2017 at 03:14:45PM +0100, Thomas Petazzoni wrote:
> Hello,
> 
> This small patch series brings a few fixes and improvements to the
> omap_rng driver. The first fix is particularly important, as it fixes
> using the driver built as a module on SoCs that require a clock for
> the IP to work properly.
> 
> Thanks,
> 
> Thomas
> 
> Thomas Petazzoni (4):
>   hwrng: omap - write registers after enabling the clock
>   hwrng: omap - use devm_clk_get() instead of of_clk_get()
>   hwrng: omap - Do not access INTMASK_REG on EIP76
>   hwrng: omap - move clock related code to omap_rng_probe()
> 
>  drivers/char/hw_random/omap-rng.c | 36 +++++++++++++++++++++++++-----------
>  1 file changed, 25 insertions(+), 11 deletions(-)

For the whole series,

Acked-by: Jason Cooper <jason@lakedaemon.net>

thx,

Jason.

Re: [PATCH 0/4] hwrng: omap - fixes and improvements

[Romain Perier]

Hello,


Le 07/03/2017 à 15:14, Thomas Petazzoni a écrit :
> Hello,
>
> This small patch series brings a few fixes and improvements to the
> omap_rng driver. The first fix is particularly important, as it fixes
> using the driver built as a module on SoCs that require a clock for
> the IP to work properly.
>
> Thanks,
>
> Thomas
>
> Thomas Petazzoni (4):
>   hwrng: omap - write registers after enabling the clock
>   hwrng: omap - use devm_clk_get() instead of of_clk_get()
>   hwrng: omap - Do not access INTMASK_REG on EIP76
>   hwrng: omap - move clock related code to omap_rng_probe()
>
>  drivers/char/hw_random/omap-rng.c | 36 +++++++++++++++++++++++++-----------
>  1 file changed, 25 insertions(+), 11 deletions(-)

For the whole series,

Reviewed-by: Romain Perier <romain.perier@collabora.com>

Re: [PATCH 0/4] hwrng: omap - fixes and improvements

[Herbert Xu]

On Tue, Mar 07, 2017 at 03:14:45PM +0100, Thomas Petazzoni wrote:
> Hello,
> 
> This small patch series brings a few fixes and improvements to the
> omap_rng driver. The first fix is particularly important, as it fixes
> using the driver built as a module on SoCs that require a clock for
> the IP to work properly.
> 
> Thanks,
> 
> Thomas
> 
> Thomas Petazzoni (4):
>   hwrng: omap - write registers after enabling the clock
>   hwrng: omap - use devm_clk_get() instead of of_clk_get()
>   hwrng: omap - Do not access INTMASK_REG on EIP76
>   hwrng: omap - move clock related code to omap_rng_probe()

Patches 1-3 applied to crypto and patch 4 applied to cryptodev.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt