All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/9] Add support for SafeXcel IP-76 to OMAP RNG
@ 2016-09-06 15:38 Romain Perier
  2016-09-06 15:38 ` [PATCH 1/9] dt-bindings: Add vendor prefix for INSIDE Secure Romain Perier
                   ` (8 more replies)
  0 siblings, 9 replies; 16+ messages in thread
From: Romain Perier @ 2016-09-06 15:38 UTC (permalink / raw)
  To: dsaxena, mpm, Herbert Xu
  Cc: Gregory Clement, Thomas Petazzoni, Romain Perier, Nadav Haklai,
	Omri Itach, Shadi Ammouri, Yahuda Yitschak, Hanna Hawa,
	Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas, linux-crypto

The driver omap-rng has a lot of similarity with the IP block SafeXcel
IP-76. A lot of registers are the same and the way that the driver works
is very closed the description of the TRNG EIP76 in its datasheet.

This series refactorize the driver, add support for generating bigger
output random data and add a device variant for SafeXcel IP-76, found
in Armada 8K.

Romain Perier (9):
  dt-bindings: Add vendor prefix for INSIDE Secure
  dt-bindings: omap-rng: Document SafeXcel IP-76 device variant
  hwrng: omap - Switch to non-obsolete read API implementation
  hwrng: omap - Use the managed device resource API for registration
  hwrng: omap - Remove global definition of hwrng
  hwrng: omap - Add support for 128-bit output of data
  hwrng: omap - Don't prefix the probe message with OMAP
  hwrng: omap - Add device variant for SafeXcel IP-76 found in Armada 8K
  arm64: dts: marvell: add TRNG description for Armada 8K CP

 Documentation/devicetree/bindings/rng/omap_rng.txt |  14 +-
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 .../boot/dts/marvell/armada-cp110-master.dtsi      |   8 ++
 .../arm64/boot/dts/marvell/armada-cp110-slave.dtsi |   8 ++
 drivers/char/hw_random/Kconfig                     |   2 +-
 drivers/char/hw_random/omap-rng.c                  | 159 +++++++++++++++------
 6 files changed, 145 insertions(+), 47 deletions(-)

-- 
2.9.3

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/9] dt-bindings: Add vendor prefix for INSIDE Secure
  2016-09-06 15:38 [PATCH 0/9] Add support for SafeXcel IP-76 to OMAP RNG Romain Perier
@ 2016-09-06 15:38 ` Romain Perier
  2016-09-06 15:38 ` [PATCH 2/9] dt-bindings: omap-rng: Document SafeXcel IP-76 device variant Romain Perier
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Romain Perier @ 2016-09-06 15:38 UTC (permalink / raw)
  To: dsaxena, mpm, Herbert Xu
  Cc: Gregory Clement, Thomas Petazzoni, Romain Perier, Nadav Haklai,
	Omri Itach, Shadi Ammouri, Yahuda Yitschak, Hanna Hawa,
	Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas, linux-crypto

This commits adds a vendor for the company INSIDE Secure.
See https://www.insidesecure.com, for more details.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 1992aa9..6a5e872 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -132,6 +132,7 @@ infineon Infineon Technologies
 inforce	Inforce Computing
 ingenic	Ingenic Semiconductor
 innolux	Innolux Corporation
+inside-secure	INSIDE Secure
 intel	Intel Corporation
 intercontrol	Inter Control Group
 invensense	InvenSense Inc.
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/9] dt-bindings: omap-rng: Document SafeXcel IP-76 device variant
  2016-09-06 15:38 [PATCH 0/9] Add support for SafeXcel IP-76 to OMAP RNG Romain Perier
  2016-09-06 15:38 ` [PATCH 1/9] dt-bindings: Add vendor prefix for INSIDE Secure Romain Perier
@ 2016-09-06 15:38 ` Romain Perier
  2016-09-06 15:38 ` [PATCH 3/9] hwrng: omap - Switch to non-obsolete read API implementation Romain Perier
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Romain Perier @ 2016-09-06 15:38 UTC (permalink / raw)
  To: dsaxena, mpm, Herbert Xu
  Cc: Gregory Clement, Thomas Petazzoni, Romain Perier, Nadav Haklai,
	Omri Itach, Shadi Ammouri, Yahuda Yitschak, Hanna Hawa,
	Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas, linux-crypto

This commits add missing fields in the documentation that are used
by the new device variant. It also includes DT example to show how
the variant should be used.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 Documentation/devicetree/bindings/rng/omap_rng.txt | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/rng/omap_rng.txt b/Documentation/devicetree/bindings/rng/omap_rng.txt
index 6a62acd..4714772 100644
--- a/Documentation/devicetree/bindings/rng/omap_rng.txt
+++ b/Documentation/devicetree/bindings/rng/omap_rng.txt
@@ -1,4 +1,4 @@
-OMAP SoC HWRNG Module
+OMAP SoC and Inside-Secure HWRNG Module
 
 Required properties:
 
@@ -6,11 +6,13 @@ Required properties:
   RNG versions:
   - "ti,omap2-rng" for OMAP2.
   - "ti,omap4-rng" for OMAP4, OMAP5 and AM33XX.
+  - "inside-secure,safexcel-eip76" for SoCs with EIP76 IP block
   Note that these two versions are incompatible.
 - ti,hwmods: Name of the hwmod associated with the RNG module
 - reg : Offset and length of the register set for the module
 - interrupts : the interrupt number for the RNG module.
-		Only used for "ti,omap4-rng".
+		Used for "ti,omap4-rng" and "inside-secure,safexcel-eip76"
+- clocks: the trng clock source
 
 Example:
 /* AM335x */
@@ -20,3 +22,11 @@ rng: rng@48310000 {
 	reg = <0x48310000 0x2000>;
 	interrupts = <111>;
 };
+
+/* SafeXcel IP-76 */
+trng: rng@f2760000 {
+	compatible = "inside-secure,safexcel-eip76";
+	reg = <0xf2760000 0x7d>;
+	interrupts = <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>;
+	clocks = <&cpm_syscon0 1 25>;
+};
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/9] hwrng: omap - Switch to non-obsolete read API implementation
  2016-09-06 15:38 [PATCH 0/9] Add support for SafeXcel IP-76 to OMAP RNG Romain Perier
  2016-09-06 15:38 ` [PATCH 1/9] dt-bindings: Add vendor prefix for INSIDE Secure Romain Perier
  2016-09-06 15:38 ` [PATCH 2/9] dt-bindings: omap-rng: Document SafeXcel IP-76 device variant Romain Perier
@ 2016-09-06 15:38 ` Romain Perier
  2016-09-06 15:38 ` [PATCH 4/9] hwrng: omap - Use the managed device resource API for registration Romain Perier
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Romain Perier @ 2016-09-06 15:38 UTC (permalink / raw)
  To: dsaxena, mpm, Herbert Xu
  Cc: Gregory Clement, Thomas Petazzoni, Romain Perier, Nadav Haklai,
	Omri Itach, Shadi Ammouri, Yahuda Yitschak, Hanna Hawa,
	Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas, linux-crypto

The ".data_present" and ".data_read" operations are marked as
OBSOLETE in the hwrng API. We have to use the ".read" operation instead.
It makes the driver simpler and removes the need to do a busy loop to
wait until enough data is generated by the IP. We simplify this step by
only checking the status of the engine, if there is data, we copy the
data to the output buffer and the amout of copied data is returned to the
caller, otherwise zero is returned. The hwrng core will re-call the read
operation as many times as required until enough data has been copied.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 drivers/char/hw_random/omap-rng.c | 39 ++++++++++++---------------------------
 1 file changed, 12 insertions(+), 27 deletions(-)

diff --git a/drivers/char/hw_random/omap-rng.c b/drivers/char/hw_random/omap-rng.c
index 01d4be2..d47b24d 100644
--- a/drivers/char/hw_random/omap-rng.c
+++ b/drivers/char/hw_random/omap-rng.c
@@ -140,41 +140,27 @@ static inline void omap_rng_write(struct omap_rng_dev *priv, u16 reg,
 	__raw_writel(val, priv->base + priv->pdata->regs[reg]);
 }
 
-static int omap_rng_data_present(struct hwrng *rng, int wait)
+
+static int omap_rng_do_read(struct hwrng *rng, void *data, size_t max,
+			    bool wait)
 {
 	struct omap_rng_dev *priv;
-	int data, i;
 
 	priv = (struct omap_rng_dev *)rng->priv;
 
-	for (i = 0; i < 20; i++) {
-		data = priv->pdata->data_present(priv);
-		if (data || !wait)
-			break;
-		/* RNG produces data fast enough (2+ MBit/sec, even
-		 * during "rngtest" loads, that these delays don't
-		 * seem to trigger.  We *could* use the RNG IRQ, but
-		 * that'd be higher overhead ... so why bother?
-		 */
-		udelay(10);
-	}
-	return data;
-}
-
-static int omap_rng_data_read(struct hwrng *rng, u32 *data)
-{
-	struct omap_rng_dev *priv;
-	u32 data_size, i;
+	if (max < priv->pdata->data_size)
+		return 0;
 
-	priv = (struct omap_rng_dev *)rng->priv;
-	data_size = priv->pdata->data_size;
+	if (!priv->pdata->data_present(priv))
+		return 0;
 
-	for (i = 0; i < data_size / sizeof(u32); i++)
-		data[i] = omap_rng_read(priv, RNG_OUTPUT_L_REG + i);
+	memcpy_fromio(data, priv->base + priv->pdata->regs[RNG_OUTPUT_L_REG],
+		      priv->pdata->data_size);
 
 	if (priv->pdata->regs[RNG_INTACK_REG])
 		omap_rng_write(priv, RNG_INTACK_REG, RNG_REG_INTACK_RDY_MASK);
-	return data_size;
+
+	return priv->pdata->data_size;
 }
 
 static int omap_rng_init(struct hwrng *rng)
@@ -195,8 +181,7 @@ static void omap_rng_cleanup(struct hwrng *rng)
 
 static struct hwrng omap_rng_ops = {
 	.name		= "omap",
-	.data_present	= omap_rng_data_present,
-	.data_read	= omap_rng_data_read,
+	.read 		= omap_rng_do_read,
 	.init		= omap_rng_init,
 	.cleanup	= omap_rng_cleanup,
 };
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/9] hwrng: omap - Use the managed device resource API for registration
  2016-09-06 15:38 [PATCH 0/9] Add support for SafeXcel IP-76 to OMAP RNG Romain Perier
                   ` (2 preceding siblings ...)
  2016-09-06 15:38 ` [PATCH 3/9] hwrng: omap - Switch to non-obsolete read API implementation Romain Perier
@ 2016-09-06 15:38 ` Romain Perier
  2016-09-06 16:31   ` PrasannaKumar Muralidharan
  2016-09-06 15:38 ` [PATCH 5/9] hwrng: omap - Remove global definition of hwrng Romain Perier
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Romain Perier @ 2016-09-06 15:38 UTC (permalink / raw)
  To: dsaxena, mpm, Herbert Xu
  Cc: Gregory Clement, Thomas Petazzoni, Romain Perier, Nadav Haklai,
	Omri Itach, Shadi Ammouri, Yahuda Yitschak, Hanna Hawa,
	Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas, linux-crypto

Use devm_hwrng_register instead of hwrng_register. It avoids the need
to handle unregistration explicitly from the remove function.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 drivers/char/hw_random/omap-rng.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/char/hw_random/omap-rng.c b/drivers/char/hw_random/omap-rng.c
index d47b24d..171c3e8 100644
--- a/drivers/char/hw_random/omap-rng.c
+++ b/drivers/char/hw_random/omap-rng.c
@@ -381,7 +381,7 @@ static int omap_rng_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_ioremap;
 
-	ret = hwrng_register(&omap_rng_ops);
+	ret = devm_hwrng_register(dev, &omap_rng_ops);
 	if (ret)
 		goto err_register;
 
@@ -402,8 +402,6 @@ static int omap_rng_remove(struct platform_device *pdev)
 {
 	struct omap_rng_dev *priv = platform_get_drvdata(pdev);
 
-	hwrng_unregister(&omap_rng_ops);
-
 	priv->pdata->cleanup(priv);
 
 	pm_runtime_put_sync(&pdev->dev);
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/9] hwrng: omap - Remove global definition of hwrng
  2016-09-06 15:38 [PATCH 0/9] Add support for SafeXcel IP-76 to OMAP RNG Romain Perier
                   ` (3 preceding siblings ...)
  2016-09-06 15:38 ` [PATCH 4/9] hwrng: omap - Use the managed device resource API for registration Romain Perier
@ 2016-09-06 15:38 ` Romain Perier
  2016-09-06 15:38 ` [PATCH 6/9] hwrng: omap - Add support for 128-bit output of data Romain Perier
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Romain Perier @ 2016-09-06 15:38 UTC (permalink / raw)
  To: dsaxena, mpm, Herbert Xu
  Cc: Gregory Clement, Thomas Petazzoni, Romain Perier, Nadav Haklai,
	Omri Itach, Shadi Ammouri, Yahuda Yitschak, Hanna Hawa,
	Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas, linux-crypto

The omap-rng driver currently assumes that there will only ever be a
single instance of an RNG device. For this reason, there is a statically
allocated struct hwrng, with a fixed name. However, registering two
struct hwrng with the same isn't accepted by the RNG framework, so we
need to switch to a dynamically allocated struct hwrng, each using a
different name. Then, we define the name of this hwrng to "dev_name(dev)",
so the name of the data structure is unique per device.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 drivers/char/hw_random/omap-rng.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/char/hw_random/omap-rng.c b/drivers/char/hw_random/omap-rng.c
index 171c3e8..f9a99b2 100644
--- a/drivers/char/hw_random/omap-rng.c
+++ b/drivers/char/hw_random/omap-rng.c
@@ -127,6 +127,7 @@ struct omap_rng_dev {
 	void __iomem			*base;
 	struct device			*dev;
 	const struct omap_rng_pdata	*pdata;
+	struct hwrng rng;
 };
 
 static inline u32 omap_rng_read(struct omap_rng_dev *priv, u16 reg)
@@ -179,12 +180,6 @@ static void omap_rng_cleanup(struct hwrng *rng)
 	priv->pdata->cleanup(priv);
 }
 
-static struct hwrng omap_rng_ops = {
-	.name		= "omap",
-	.read 		= omap_rng_do_read,
-	.init		= omap_rng_init,
-	.cleanup	= omap_rng_cleanup,
-};
 
 static inline u32 omap2_rng_data_present(struct omap_rng_dev *priv)
 {
@@ -357,7 +352,11 @@ static int omap_rng_probe(struct platform_device *pdev)
 	if (!priv)
 		return -ENOMEM;
 
-	omap_rng_ops.priv = (unsigned long)priv;
+	priv->rng.read = omap_rng_do_read;
+	priv->rng.init = omap_rng_init;
+	priv->rng.cleanup = omap_rng_cleanup;
+
+	priv->rng.priv = (unsigned long)priv;
 	platform_set_drvdata(pdev, priv);
 	priv->dev = dev;
 
@@ -368,6 +367,12 @@ static int omap_rng_probe(struct platform_device *pdev)
 		goto err_ioremap;
 	}
 
+	priv->rng.name = devm_kstrdup(dev, dev_name(dev), GFP_KERNEL);
+	if (!priv->rng.name) {
+		ret = -ENOMEM;
+		goto err_register;
+	}
+
 	pm_runtime_enable(&pdev->dev);
 	ret = pm_runtime_get_sync(&pdev->dev);
 	if (ret) {
@@ -381,7 +386,7 @@ static int omap_rng_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_ioremap;
 
-	ret = devm_hwrng_register(dev, &omap_rng_ops);
+	ret = devm_hwrng_register(dev, &priv->rng);
 	if (ret)
 		goto err_register;
 
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 6/9] hwrng: omap - Add support for 128-bit output of data
  2016-09-06 15:38 [PATCH 0/9] Add support for SafeXcel IP-76 to OMAP RNG Romain Perier
                   ` (4 preceding siblings ...)
  2016-09-06 15:38 ` [PATCH 5/9] hwrng: omap - Remove global definition of hwrng Romain Perier
@ 2016-09-06 15:38 ` Romain Perier
  2016-09-06 15:38 ` [PATCH 7/9] hwrng: omap - Don't prefix the probe message with OMAP Romain Perier
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Romain Perier @ 2016-09-06 15:38 UTC (permalink / raw)
  To: dsaxena, mpm, Herbert Xu
  Cc: Gregory Clement, Thomas Petazzoni, Romain Perier, Nadav Haklai,
	Omri Itach, Shadi Ammouri, Yahuda Yitschak, Hanna Hawa,
	Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas, linux-crypto

So far, this driver only supports up to 64 bits of output data generated
by an RNG. Some IP blocks, like the SafeXcel IP-76 supports up to 128
bits of output data. This commits renames registers descriptions
OUTPUT_L_REG and OUTPUT_H_REG to OUTPUT_0_REG and OUPUT_1_REG,
respectively. It also adds two new values to the enumeration of existing
registers: OUTPUT_2_REG and OUTPUT_3_REG.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 drivers/char/hw_random/omap-rng.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/char/hw_random/omap-rng.c b/drivers/char/hw_random/omap-rng.c
index f9a99b2..6a3aaad 100644
--- a/drivers/char/hw_random/omap-rng.c
+++ b/drivers/char/hw_random/omap-rng.c
@@ -65,8 +65,10 @@
 #define OMAP4_RNG_OUTPUT_SIZE			0x8
 
 enum {
-	RNG_OUTPUT_L_REG = 0,
-	RNG_OUTPUT_H_REG,
+	RNG_OUTPUT_0_REG = 0,
+	RNG_OUTPUT_1_REG,
+	RNG_OUTPUT_2_REG,
+	RNG_OUTPUT_3_REG,
 	RNG_STATUS_REG,
 	RNG_INTMASK_REG,
 	RNG_INTACK_REG,
@@ -82,7 +84,7 @@ enum {
 };
 
 static const u16 reg_map_omap2[] = {
-	[RNG_OUTPUT_L_REG]	= 0x0,
+	[RNG_OUTPUT_0_REG]	= 0x0,
 	[RNG_STATUS_REG]	= 0x4,
 	[RNG_CONFIG_REG]	= 0x28,
 	[RNG_REV_REG]		= 0x3c,
@@ -90,8 +92,8 @@ static const u16 reg_map_omap2[] = {
 };
 
 static const u16 reg_map_omap4[] = {
-	[RNG_OUTPUT_L_REG]	= 0x0,
-	[RNG_OUTPUT_H_REG]	= 0x4,
+	[RNG_OUTPUT_0_REG]	= 0x0,
+	[RNG_OUTPUT_1_REG]	= 0x4,
 	[RNG_STATUS_REG]	= 0x8,
 	[RNG_INTMASK_REG]	= 0xc,
 	[RNG_INTACK_REG]	= 0x10,
@@ -155,7 +157,7 @@ static int omap_rng_do_read(struct hwrng *rng, void *data, size_t max,
 	if (!priv->pdata->data_present(priv))
 		return 0;
 
-	memcpy_fromio(data, priv->base + priv->pdata->regs[RNG_OUTPUT_L_REG],
+	memcpy_fromio(data, priv->base + priv->pdata->regs[RNG_OUTPUT_0_REG],
 		      priv->pdata->data_size);
 
 	if (priv->pdata->regs[RNG_INTACK_REG])
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 7/9] hwrng: omap - Don't prefix the probe message with OMAP
  2016-09-06 15:38 [PATCH 0/9] Add support for SafeXcel IP-76 to OMAP RNG Romain Perier
                   ` (5 preceding siblings ...)
  2016-09-06 15:38 ` [PATCH 6/9] hwrng: omap - Add support for 128-bit output of data Romain Perier
@ 2016-09-06 15:38 ` Romain Perier
  2016-09-06 15:38 ` [PATCH 8/9] hwrng: omap - Add device variant for SafeXcel IP-76 found in Armada 8K Romain Perier
  2016-09-06 15:38 ` [PATCH 9/9] arm64: dts: marvell: add TRNG description for Armada 8K CP Romain Perier
  8 siblings, 0 replies; 16+ messages in thread
From: Romain Perier @ 2016-09-06 15:38 UTC (permalink / raw)
  To: dsaxena, mpm, Herbert Xu
  Cc: Gregory Clement, Thomas Petazzoni, Romain Perier, Nadav Haklai,
	Omri Itach, Shadi Ammouri, Yahuda Yitschak, Hanna Hawa,
	Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas, linux-crypto

So far, this driver was only used for OMAP SoCs. However, if a device
variant is added for an IP block that has nothing to do with the OMAP
platform, the message "OMAP Random Number Generator Ver" is displayed
anyway. Instead of hardcoding "OMAP" into this message, we decide to
only display "Random Number Generator". As dev_info is already
pre-pending the message with the name of the device, we have enough
informations.

Signed-off-by: Romain Perier <romain.perier@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 6a3aaad..f1dcdb3 100644
--- a/drivers/char/hw_random/omap-rng.c
+++ b/drivers/char/hw_random/omap-rng.c
@@ -392,7 +392,7 @@ static int omap_rng_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_register;
 
-	dev_info(&pdev->dev, "OMAP Random Number Generator ver. %02x\n",
+	dev_info(&pdev->dev, "Random Number Generator ver. %02x\n",
 		 omap_rng_read(priv, RNG_REV_REG));
 
 	return 0;
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 8/9] hwrng: omap - Add device variant for SafeXcel IP-76 found in Armada 8K
  2016-09-06 15:38 [PATCH 0/9] Add support for SafeXcel IP-76 to OMAP RNG Romain Perier
                   ` (6 preceding siblings ...)
  2016-09-06 15:38 ` [PATCH 7/9] hwrng: omap - Don't prefix the probe message with OMAP Romain Perier
@ 2016-09-06 15:38 ` Romain Perier
  2016-09-06 15:38 ` [PATCH 9/9] arm64: dts: marvell: add TRNG description for Armada 8K CP Romain Perier
  8 siblings, 0 replies; 16+ messages in thread
From: Romain Perier @ 2016-09-06 15:38 UTC (permalink / raw)
  To: dsaxena, mpm, Herbert Xu
  Cc: Gregory Clement, Thomas Petazzoni, Romain Perier, Nadav Haklai,
	Omri Itach, Shadi Ammouri, Yahuda Yitschak, Hanna Hawa,
	Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas, linux-crypto

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 6021 bytes --]

This commits adds a device variant for Safexcel,EIP76 found in Marvell
Armada 8k. It defines registers mapping with the good offset and add a
specific initialization function.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 drivers/char/hw_random/Kconfig    |  2 +-
 drivers/char/hw_random/omap-rng.c | 85 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 84 insertions(+), 3 deletions(-)

diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index 56ad5a5..aea3613 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -168,7 +168,7 @@ config HW_RANDOM_IXP4XX
 
 config HW_RANDOM_OMAP
 	tristate "OMAP Random Number Generator support"
-	depends on ARCH_OMAP16XX || ARCH_OMAP2PLUS
+	depends on ARCH_OMAP16XX || ARCH_OMAP2PLUS || ARCH_MVEBU
 	default HW_RANDOM
  	---help---
  	  This driver provides kernel-side support for the Random Number
diff --git a/drivers/char/hw_random/omap-rng.c b/drivers/char/hw_random/omap-rng.c
index f1dcdb3..7c86c0d 100644
--- a/drivers/char/hw_random/omap-rng.c
+++ b/drivers/char/hw_random/omap-rng.c
@@ -28,6 +28,7 @@
 #include <linux/of_device.h>
 #include <linux/of_address.h>
 #include <linux/interrupt.h>
+#include <linux/clk.h>
 
 #include <asm/io.h>
 
@@ -63,6 +64,7 @@
 
 #define OMAP2_RNG_OUTPUT_SIZE			0x4
 #define OMAP4_RNG_OUTPUT_SIZE			0x8
+#define EIP76_RNG_OUTPUT_SIZE			0x10
 
 enum {
 	RNG_OUTPUT_0_REG = 0,
@@ -108,6 +110,23 @@ static const u16 reg_map_omap4[] = {
 	[RNG_SYSCONFIG_REG]	= 0x1FE4,
 };
 
+static const u16 reg_map_eip76[] = {
+	[RNG_OUTPUT_0_REG]	= 0x0,
+	[RNG_OUTPUT_1_REG]	= 0x4,
+	[RNG_OUTPUT_2_REG]	= 0x8,
+	[RNG_OUTPUT_3_REG]	= 0xc,
+	[RNG_STATUS_REG]	= 0x10,
+	[RNG_INTACK_REG]	= 0x10,
+	[RNG_CONTROL_REG]	= 0x14,
+	[RNG_CONFIG_REG]	= 0x18,
+	[RNG_ALARMCNT_REG]	= 0x1c,
+	[RNG_FROENABLE_REG]	= 0x20,
+	[RNG_FRODETUNE_REG]	= 0x24,
+	[RNG_ALARMMASK_REG]	= 0x28,
+	[RNG_ALARMSTOP_REG]	= 0x2c,
+	[RNG_REV_REG]		= 0x7c,
+};
+
 struct omap_rng_dev;
 /**
  * struct omap_rng_pdata - RNG IP block-specific data
@@ -130,6 +149,7 @@ struct omap_rng_dev {
 	struct device			*dev;
 	const struct omap_rng_pdata	*pdata;
 	struct hwrng rng;
+	struct clk 			*clk;
 };
 
 static inline u32 omap_rng_read(struct omap_rng_dev *priv, u16 reg)
@@ -213,6 +233,38 @@ static inline u32 omap4_rng_data_present(struct omap_rng_dev *priv)
 	return omap_rng_read(priv, RNG_STATUS_REG) & RNG_REG_STATUS_RDY;
 }
 
+static int eip76_rng_init(struct omap_rng_dev *priv)
+{
+	u32 val;
+
+	/* Return if RNG is already running. */
+	if (omap_rng_read(priv, RNG_CONTROL_REG) & RNG_CONTROL_ENABLE_TRNG_MASK)
+		return 0;
+
+	/*  Number of 512 bit blocks of raw Noise Source output data that must
+	 *  be processed by either the Conditioning Function or the
+	 *  SP 800-90 DRBG ‘BC_DF’ functionality to yield a ‘full entropy’
+	 *  output value.
+	 */
+	val = 0x5 << RNG_CONFIG_MIN_REFIL_CYCLES_SHIFT;
+
+	/* Number of FRO samples that are XOR-ed together into one bit to be
+	 * shifted into the main shift register
+	 */
+	val |= RNG_CONFIG_MAX_REFIL_CYCLES << RNG_CONFIG_MAX_REFIL_CYCLES_SHIFT;
+	omap_rng_write(priv, RNG_CONFIG_REG, val);
+
+	/* Enable all available FROs */
+	omap_rng_write(priv, RNG_FRODETUNE_REG, 0x0);
+	omap_rng_write(priv, RNG_FROENABLE_REG, RNG_REG_FROENABLE_MASK);
+
+	/* Enable TRNG */
+	val = RNG_CONTROL_ENABLE_TRNG_MASK;
+	omap_rng_write(priv, RNG_CONTROL_REG, val);
+
+	return 0;
+}
+
 static int omap4_rng_init(struct omap_rng_dev *priv)
 {
 	u32 val;
@@ -282,6 +334,14 @@ static struct omap_rng_pdata omap4_rng_pdata = {
 	.cleanup	= omap4_rng_cleanup,
 };
 
+static struct omap_rng_pdata eip76_rng_pdata = {
+	.regs		= (u16 *)reg_map_eip76,
+	.data_size	= EIP76_RNG_OUTPUT_SIZE,
+	.data_present	= omap4_rng_data_present,
+	.init		= eip76_rng_init,
+	.cleanup	= omap4_rng_cleanup,
+};
+
 static const struct of_device_id omap_rng_of_match[] = {
 		{
 			.compatible	= "ti,omap2-rng",
@@ -291,6 +351,10 @@ static const struct of_device_id omap_rng_of_match[] = {
 			.compatible	= "ti,omap4-rng",
 			.data		= &omap4_rng_pdata,
 		},
+		{
+			.compatible	= "inside-secure,safexcel-eip76",
+			.data		= &eip76_rng_pdata,
+		},
 		{},
 };
 MODULE_DEVICE_TABLE(of, omap_rng_of_match);
@@ -309,7 +373,8 @@ static int of_get_omap_rng_device_details(struct omap_rng_dev *priv,
 	}
 	priv->pdata = match->data;
 
-	if (of_device_is_compatible(dev->of_node, "ti,omap4-rng")) {
+	if (of_device_is_compatible(dev->of_node, "ti,omap4-rng") ||
+	    of_device_is_compatible(dev->of_node, "inside-secure,safexcel-eip76")) {
 		irq = platform_get_irq(pdev, 0);
 		if (irq < 0) {
 			dev_err(dev, "%s: error getting IRQ resource - %d\n",
@@ -325,6 +390,16 @@ static int of_get_omap_rng_device_details(struct omap_rng_dev *priv,
 			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)
+			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);
+		}
 	}
 	return 0;
 }
@@ -386,7 +461,7 @@ static int omap_rng_probe(struct platform_device *pdev)
 	ret = (dev->of_node) ? of_get_omap_rng_device_details(priv, pdev) :
 				get_omap_rng_device_details(priv);
 	if (ret)
-		goto err_ioremap;
+		goto err_register;
 
 	ret = devm_hwrng_register(dev, &priv->rng);
 	if (ret)
@@ -400,6 +475,9 @@ static int omap_rng_probe(struct platform_device *pdev)
 err_register:
 	priv->base = NULL;
 	pm_runtime_disable(&pdev->dev);
+
+	if (!IS_ERR(priv->clk))
+		clk_disable_unprepare(priv->clk);
 err_ioremap:
 	dev_err(dev, "initialization failed.\n");
 	return ret;
@@ -414,6 +492,9 @@ static int omap_rng_remove(struct platform_device *pdev)
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
+	if (!IS_ERR(priv->clk))
+		clk_disable_unprepare(priv->clk);
+
 	return 0;
 }
 
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 9/9] arm64: dts: marvell: add TRNG description for Armada 8K CP
  2016-09-06 15:38 [PATCH 0/9] Add support for SafeXcel IP-76 to OMAP RNG Romain Perier
                   ` (7 preceding siblings ...)
  2016-09-06 15:38 ` [PATCH 8/9] hwrng: omap - Add device variant for SafeXcel IP-76 found in Armada 8K Romain Perier
@ 2016-09-06 15:38 ` Romain Perier
  8 siblings, 0 replies; 16+ messages in thread
From: Romain Perier @ 2016-09-06 15:38 UTC (permalink / raw)
  To: dsaxena, mpm, Herbert Xu
  Cc: Gregory Clement, Thomas Petazzoni, Romain Perier, Nadav Haklai,
	Omri Itach, Shadi Ammouri, Yahuda Yitschak, Hanna Hawa,
	Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas, linux-crypto

This commits adds the devicetree description of the SafeXcel IP-76 TRNG
found in the two Armada CP110.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi | 8 ++++++++
 arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi  | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
index da31bbb..aaffa24 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
@@ -164,6 +164,14 @@
 				clocks = <&cpm_syscon0 1 21>;
 				status = "disabled";
 			};
+
+			cpm_trng: trng@760000 {
+				compatible = "marvell,armada-8k-rng", "inside-secure,safexcel-eip76";
+				reg = <0x760000 0x7d>;
+				interrupts = <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&cpm_syscon0 1 25>;
+				status = "okay";
+			};
 		};
 
 		cpm_pcie0: pcie@f2600000 {
diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
index 6ff1201..216de12 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
@@ -164,6 +164,14 @@
 				clocks = <&cps_syscon0 1 21>;
 				status = "disabled";
 			};
+
+			cps_trng: trng@760000 {
+				compatible = "marvell,armada-8k-rng", "inside-secure,safexcel-eip76";
+				reg = <0x760000 0x7d>;
+				interrupts = <GIC_SPI 312 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&cps_syscon0 1 25>;
+				status = "okay";
+			};
 		};
 
 		cps_pcie0: pcie@f4600000 {
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/9] hwrng: omap - Use the managed device resource API for registration
  2016-09-06 15:38 ` [PATCH 4/9] hwrng: omap - Use the managed device resource API for registration Romain Perier
@ 2016-09-06 16:31   ` PrasannaKumar Muralidharan
  2016-09-07 14:23     ` Romain Perier
  0 siblings, 1 reply; 16+ messages in thread
From: PrasannaKumar Muralidharan @ 2016-09-06 16:31 UTC (permalink / raw)
  To: Romain Perier
  Cc: dsaxena, mpm, Herbert Xu, Gregory Clement, Thomas Petazzoni,
	Nadav Haklai, Omri Itach, Shadi Ammouri, Yahuda Yitschak,
	Hanna Hawa, Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas,
	linux-crypto

> Use devm_hwrng_register instead of hwrng_register. It avoids the need
> to handle unregistration explicitly from the remove function.
>
> Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
> ---
>  drivers/char/hw_random/omap-rng.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/char/hw_random/omap-rng.c b/drivers/char/hw_random/omap-rng.c
> index d47b24d..171c3e8 100644
> --- a/drivers/char/hw_random/omap-rng.c
> +++ b/drivers/char/hw_random/omap-rng.c
> @@ -381,7 +381,7 @@ static int omap_rng_probe(struct platform_device *pdev)
>         if (ret)
>                 goto err_ioremap;
>
> -       ret = hwrng_register(&omap_rng_ops);
> +       ret = devm_hwrng_register(dev, &omap_rng_ops);
>         if (ret)
>                 goto err_register;
>
> @@ -402,8 +402,6 @@ static int omap_rng_remove(struct platform_device *pdev)
>  {
>         struct omap_rng_dev *priv = platform_get_drvdata(pdev);
>
> -       hwrng_unregister(&omap_rng_ops);
> -
>         priv->pdata->cleanup(priv);
>
>         pm_runtime_put_sync(&pdev->dev);
> --

If devm_hwrng_register is used hwrng_unregister will be called after
pm_runtime_disable is called. If RNG device is in use calling
omap_rng_remove may not work properly.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/9] hwrng: omap - Use the managed device resource API for registration
  2016-09-06 16:31   ` PrasannaKumar Muralidharan
@ 2016-09-07 14:23     ` Romain Perier
  2016-09-07 14:45       ` PrasannaKumar Muralidharan
  0 siblings, 1 reply; 16+ messages in thread
From: Romain Perier @ 2016-09-07 14:23 UTC (permalink / raw)
  To: PrasannaKumar Muralidharan
  Cc: dsaxena, mpm, Herbert Xu, Gregory Clement, Thomas Petazzoni,
	Nadav Haklai, Omri Itach, Shadi Ammouri, Yahuda Yitschak,
	Hanna Hawa, Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas,
	linux-crypto

Hello,

Le 06/09/2016 18:31, PrasannaKumar Muralidharan a écrit :
>> Use devm_hwrng_register instead of hwrng_register. It avoids the need
>> to handle unregistration explicitly from the remove function.
>>
>> Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
>> ---
>>   drivers/char/hw_random/omap-rng.c | 4 +---
>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/char/hw_random/omap-rng.c b/drivers/char/hw_random/omap-rng.c
>> index d47b24d..171c3e8 100644
>> --- a/drivers/char/hw_random/omap-rng.c
>> +++ b/drivers/char/hw_random/omap-rng.c
>> @@ -381,7 +381,7 @@ static int omap_rng_probe(struct platform_device *pdev)
>>          if (ret)
>>                  goto err_ioremap;
>>
>> -       ret = hwrng_register(&omap_rng_ops);
>> +       ret = devm_hwrng_register(dev, &omap_rng_ops);
>>          if (ret)
>>                  goto err_register;
>>
>> @@ -402,8 +402,6 @@ static int omap_rng_remove(struct platform_device *pdev)
>>   {
>>          struct omap_rng_dev *priv = platform_get_drvdata(pdev);
>>
>> -       hwrng_unregister(&omap_rng_ops);
>> -
>>          priv->pdata->cleanup(priv);
>>
>>          pm_runtime_put_sync(&pdev->dev);
>> --
>
> If devm_hwrng_register is used hwrng_unregister will be called after
> pm_runtime_disable is called. If RNG device is in use calling
> omap_rng_remove may not work properly.
>

The case where the remove function is called is if you unbind the driver 
by hand or you call rmmod while the RNG device is used.
I don't think that the kernel will call platform->remove is the device 
is in use (so /dev/hwrng). I mean the argument that the unregister 
function is called after pm_runtime_disable is correct, but I don't 
think that the remove function might be called while the device is in 
use. There is necessarily a mutual exclusive case between "use the 
device" and "call the remove function of the device". However, I am open 
to suggestions.

Regards,
-- 
Romain Perier, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/9] hwrng: omap - Use the managed device resource API for registration
  2016-09-07 14:23     ` Romain Perier
@ 2016-09-07 14:45       ` PrasannaKumar Muralidharan
  2016-09-07 15:38         ` Romain Perier
  2016-09-08 15:47         ` Romain Perier
  0 siblings, 2 replies; 16+ messages in thread
From: PrasannaKumar Muralidharan @ 2016-09-07 14:45 UTC (permalink / raw)
  To: Romain Perier
  Cc: dsaxena, mpm, Herbert Xu, Gregory Clement, Thomas Petazzoni,
	Nadav Haklai, Omri Itach, Shadi Ammouri, Yahuda Yitschak,
	Hanna Hawa, Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas,
	linux-crypto

On 7 September 2016 at 19:53, Romain Perier
<romain.perier@free-electrons.com> wrote:
> Hello,
>
>
> Le 06/09/2016 18:31, PrasannaKumar Muralidharan a écrit :
>>>
>>> Use devm_hwrng_register instead of hwrng_register. It avoids the need
>>> to handle unregistration explicitly from the remove function.
>>>
>>> Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
>>> ---
>>>   drivers/char/hw_random/omap-rng.c | 4 +---
>>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/char/hw_random/omap-rng.c
>>> b/drivers/char/hw_random/omap-rng.c
>>> index d47b24d..171c3e8 100644
>>> --- a/drivers/char/hw_random/omap-rng.c
>>> +++ b/drivers/char/hw_random/omap-rng.c
>>> @@ -381,7 +381,7 @@ static int omap_rng_probe(struct platform_device
>>> *pdev)
>>>          if (ret)
>>>                  goto err_ioremap;
>>>
>>> -       ret = hwrng_register(&omap_rng_ops);
>>> +       ret = devm_hwrng_register(dev, &omap_rng_ops);
>>>          if (ret)
>>>                  goto err_register;
>>>
>>> @@ -402,8 +402,6 @@ static int omap_rng_remove(struct platform_device
>>> *pdev)
>>>   {
>>>          struct omap_rng_dev *priv = platform_get_drvdata(pdev);
>>>
>>> -       hwrng_unregister(&omap_rng_ops);
>>> -
>>>          priv->pdata->cleanup(priv);
>>>
>>>          pm_runtime_put_sync(&pdev->dev);
>>> --
>>
>>
>> If devm_hwrng_register is used hwrng_unregister will be called after
>> pm_runtime_disable is called. If RNG device is in use calling
>> omap_rng_remove may not work properly.
>>
>
> The case where the remove function is called is if you unbind the driver by
> hand or you call rmmod while the RNG device is used.
> I don't think that the kernel will call platform->remove is the device is in
> use (so /dev/hwrng). I mean the argument that the unregister function is
> called after pm_runtime_disable is correct, but I don't think that the
> remove function might be called while the device is in use. There is
> necessarily a mutual exclusive case between "use the device" and "call the
> remove function of the device". However, I am open to suggestions.

The way you explained is good :D. Good point too. But the device is
created by hw_random core (hwrng_modinit in core.c) so the device can
be in use when omap-rng module is removed. Please feel free to correct
me if I am wrong.

Cheers,
PrasannaKumar

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/9] hwrng: omap - Use the managed device resource API for registration
  2016-09-07 14:45       ` PrasannaKumar Muralidharan
@ 2016-09-07 15:38         ` Romain Perier
  2016-09-08 15:47         ` Romain Perier
  1 sibling, 0 replies; 16+ messages in thread
From: Romain Perier @ 2016-09-07 15:38 UTC (permalink / raw)
  To: PrasannaKumar Muralidharan
  Cc: dsaxena, mpm, Herbert Xu, Gregory Clement, Thomas Petazzoni,
	Nadav Haklai, Omri Itach, Shadi Ammouri, Yahuda Yitschak,
	Hanna Hawa, Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas,
	linux-crypto



Le 07/09/2016 16:45, PrasannaKumar Muralidharan a écrit :
> On 7 September 2016 at 19:53, Romain Perier
> <romain.perier@free-electrons.com> wrote:
>> Hello,
>>
>>
>> Le 06/09/2016 18:31, PrasannaKumar Muralidharan a écrit :
>>>>
>>>> Use devm_hwrng_register instead of hwrng_register. It avoids the need
>>>> to handle unregistration explicitly from the remove function.
>>>>
>>>> Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
>>>> ---
>>>>    drivers/char/hw_random/omap-rng.c | 4 +---
>>>>    1 file changed, 1 insertion(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/char/hw_random/omap-rng.c
>>>> b/drivers/char/hw_random/omap-rng.c
>>>> index d47b24d..171c3e8 100644
>>>> --- a/drivers/char/hw_random/omap-rng.c
>>>> +++ b/drivers/char/hw_random/omap-rng.c
>>>> @@ -381,7 +381,7 @@ static int omap_rng_probe(struct platform_device
>>>> *pdev)
>>>>           if (ret)
>>>>                   goto err_ioremap;
>>>>
>>>> -       ret = hwrng_register(&omap_rng_ops);
>>>> +       ret = devm_hwrng_register(dev, &omap_rng_ops);
>>>>           if (ret)
>>>>                   goto err_register;
>>>>
>>>> @@ -402,8 +402,6 @@ static int omap_rng_remove(struct platform_device
>>>> *pdev)
>>>>    {
>>>>           struct omap_rng_dev *priv = platform_get_drvdata(pdev);
>>>>
>>>> -       hwrng_unregister(&omap_rng_ops);
>>>> -
>>>>           priv->pdata->cleanup(priv);
>>>>
>>>>           pm_runtime_put_sync(&pdev->dev);
>>>> --
>>>
>>>
>>> If devm_hwrng_register is used hwrng_unregister will be called after
>>> pm_runtime_disable is called. If RNG device is in use calling
>>> omap_rng_remove may not work properly.
>>>
>>
>> The case where the remove function is called is if you unbind the driver by
>> hand or you call rmmod while the RNG device is used.
>> I don't think that the kernel will call platform->remove is the device is in
>> use (so /dev/hwrng). I mean the argument that the unregister function is
>> called after pm_runtime_disable is correct, but I don't think that the
>> remove function might be called while the device is in use. There is
>> necessarily a mutual exclusive case between "use the device" and "call the
>> remove function of the device". However, I am open to suggestions.
>
> The way you explained is good :D. Good point too. But the device is
> created by hw_random core (hwrng_modinit in core.c) so the device can
> be in use when omap-rng module is removed. Please feel free to correct
> me if I am wrong.

Mhhh, I think that I understood what you meant. The node /dev/hwrng is 
managed by hw_random core, a read might happen on this node while 
platform->remove is called... However, if hwrng_unregister is the first 
function called from platform->remove, the driver is unbinded from the 
hw_random core atomically (unregister and read cannot happen in the same 
time because there is a mutex), so the problem does not happen.
I propose to remove this patch from the series.

Thanks!
Romain
-- 
Romain Perier, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/9] hwrng: omap - Use the managed device resource API for registration
  2016-09-07 14:45       ` PrasannaKumar Muralidharan
  2016-09-07 15:38         ` Romain Perier
@ 2016-09-08 15:47         ` Romain Perier
  2016-09-08 17:02           ` Herbert Xu
  1 sibling, 1 reply; 16+ messages in thread
From: Romain Perier @ 2016-09-08 15:47 UTC (permalink / raw)
  To: PrasannaKumar Muralidharan
  Cc: dsaxena, mpm, Herbert Xu, Gregory Clement, Thomas Petazzoni,
	Nadav Haklai, Omri Itach, Shadi Ammouri, Yahuda Yitschak,
	Hanna Hawa, Neta Zur Hershkovits, Igal Liberman, Marcin Wojtas,
	linux-crypto



Le 07/09/2016 16:45, PrasannaKumar Muralidharan a écrit :
> On 7 September 2016 at 19:53, Romain Perier
> <romain.perier@free-electrons.com> wrote:
>> Hello,
>>
>>
>> Le 06/09/2016 18:31, PrasannaKumar Muralidharan a écrit :
>>>>
>>>> Use devm_hwrng_register instead of hwrng_register. It avoids the need
>>>> to handle unregistration explicitly from the remove function.
>>>>
>>>> Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
>>>> ---
>>>>    drivers/char/hw_random/omap-rng.c | 4 +---
>>>>    1 file changed, 1 insertion(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/char/hw_random/omap-rng.c
>>>> b/drivers/char/hw_random/omap-rng.c
>>>> index d47b24d..171c3e8 100644
>>>> --- a/drivers/char/hw_random/omap-rng.c
>>>> +++ b/drivers/char/hw_random/omap-rng.c
>>>> @@ -381,7 +381,7 @@ static int omap_rng_probe(struct platform_device
>>>> *pdev)
>>>>           if (ret)
>>>>                   goto err_ioremap;
>>>>
>>>> -       ret = hwrng_register(&omap_rng_ops);
>>>> +       ret = devm_hwrng_register(dev, &omap_rng_ops);
>>>>           if (ret)
>>>>                   goto err_register;
>>>>
>>>> @@ -402,8 +402,6 @@ static int omap_rng_remove(struct platform_device
>>>> *pdev)
>>>>    {
>>>>           struct omap_rng_dev *priv = platform_get_drvdata(pdev);
>>>>
>>>> -       hwrng_unregister(&omap_rng_ops);
>>>> -
>>>>           priv->pdata->cleanup(priv);
>>>>
>>>>           pm_runtime_put_sync(&pdev->dev);
>>>> --
>>>
>>>
>>> If devm_hwrng_register is used hwrng_unregister will be called after
>>> pm_runtime_disable is called. If RNG device is in use calling
>>> omap_rng_remove may not work properly.
>>>
>>
>> The case where the remove function is called is if you unbind the driver by
>> hand or you call rmmod while the RNG device is used.
>> I don't think that the kernel will call platform->remove is the device is in
>> use (so /dev/hwrng). I mean the argument that the unregister function is
>> called after pm_runtime_disable is correct, but I don't think that the
>> remove function might be called while the device is in use. There is
>> necessarily a mutual exclusive case between "use the device" and "call the
>> remove function of the device". However, I am open to suggestions.
>
> The way you explained is good :D. Good point too. But the device is
> created by hw_random core (hwrng_modinit in core.c) so the device can
> be in use when omap-rng module is removed. Please feel free to correct
> me if I am wrong.
>
> Cheers,
> PrasannaKumar
>

Hi,

I was wondering something. hwrng_unregister does not check the kref 
reference counter of the object... so technically if the current 
rng_device is in use, with or without devm... calling platform->remove 
will break the driver anyway because hwrng_unregister will unbind the 
device from /dev/hwrng. What I mean is that I think that we had this 
issue even without devm_hwrng_register.

Herbert, could you confirm ?

Romain
-- 
Romain Perier, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/9] hwrng: omap - Use the managed device resource API for registration
  2016-09-08 15:47         ` Romain Perier
@ 2016-09-08 17:02           ` Herbert Xu
  0 siblings, 0 replies; 16+ messages in thread
From: Herbert Xu @ 2016-09-08 17:02 UTC (permalink / raw)
  To: Romain Perier
  Cc: PrasannaKumar Muralidharan, dsaxena, mpm, Gregory Clement,
	Thomas Petazzoni, Nadav Haklai, Omri Itach, Shadi Ammouri,
	Yahuda Yitschak, Hanna Hawa, Neta Zur Hershkovits, Igal Liberman,
	Marcin Wojtas, linux-crypto

On Thu, Sep 08, 2016 at 05:47:13PM +0200, Romain Perier wrote:
>
> I was wondering something. hwrng_unregister does not check the kref
> reference counter of the object... so technically if the current
> rng_device is in use, with or without devm... calling
> platform->remove will break the driver anyway because
> hwrng_unregister will unbind the device from /dev/hwrng. What I mean
> is that I think that we had this issue even without
> devm_hwrng_register.
> 
> Herbert, could you confirm ?

Right.  remove can happen at any time and the driver needs to
cope with it by returning an error from data_read if the hardware
disappears in the middle of an operation.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2016-09-08 17:02 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-06 15:38 [PATCH 0/9] Add support for SafeXcel IP-76 to OMAP RNG Romain Perier
2016-09-06 15:38 ` [PATCH 1/9] dt-bindings: Add vendor prefix for INSIDE Secure Romain Perier
2016-09-06 15:38 ` [PATCH 2/9] dt-bindings: omap-rng: Document SafeXcel IP-76 device variant Romain Perier
2016-09-06 15:38 ` [PATCH 3/9] hwrng: omap - Switch to non-obsolete read API implementation Romain Perier
2016-09-06 15:38 ` [PATCH 4/9] hwrng: omap - Use the managed device resource API for registration Romain Perier
2016-09-06 16:31   ` PrasannaKumar Muralidharan
2016-09-07 14:23     ` Romain Perier
2016-09-07 14:45       ` PrasannaKumar Muralidharan
2016-09-07 15:38         ` Romain Perier
2016-09-08 15:47         ` Romain Perier
2016-09-08 17:02           ` Herbert Xu
2016-09-06 15:38 ` [PATCH 5/9] hwrng: omap - Remove global definition of hwrng Romain Perier
2016-09-06 15:38 ` [PATCH 6/9] hwrng: omap - Add support for 128-bit output of data Romain Perier
2016-09-06 15:38 ` [PATCH 7/9] hwrng: omap - Don't prefix the probe message with OMAP Romain Perier
2016-09-06 15:38 ` [PATCH 8/9] hwrng: omap - Add device variant for SafeXcel IP-76 found in Armada 8K Romain Perier
2016-09-06 15:38 ` [PATCH 9/9] arm64: dts: marvell: add TRNG description for Armada 8K CP Romain Perier

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.