crypto: inside-secure: improve clock management

PATCH New Gregory CLEMENT 2 participants 6 messages Mar 13, 2018 – Mar 23, 2018

Subsystems (4)

Stable branch Device tree bindings Crypto API Inside secure crypto driver

lore

Share & tools

lore

Gregory CLEMENT

author

2018-03-13 16:48 UTC #0 lore raw

cover crypto: inside-secure: improve clock management

Hi,

This short series fixes the way the clocks are used for the SafeXcel EIP-197 controller embedded in the Marvell Armada 7K/8K SoCs. On these SoCs a second one is needed in order to clock the registers. It was not noticed until now because we relied on the bootloader and also because the clock driver was wrong.

Thanks to this fix, it would be possible to fix the clock driver without introducing a regression.

While I was working on the clocks I found a bug in the clock management which was fixed with the first patch and should be applied to v4.16.

The second patch is a small improvement which is aim for v4.17 as well as the last patch.

Thanks,

Gregory

Gregory CLEMENT (3):
crypto: inside-secure - fix clock management
crypto: inside-secure - improve clock initialization
crypto: inside-secure - fix clock resource by adding a register clock

 .../bindings/crypto/inside-secure-safexcel.txt     |  6 ++-
 drivers/crypto/inside-secure/safexcel.c            | 47 +++++++++++++++-------
 drivers/crypto/inside-secure/safexcel.h            |  1 +
 3 files changed, 38 insertions(+), 16 deletions(-)

Signature · 1 line

2.16.1

Gregory CLEMENT

author

2018-03-13 16:48 UTC #1 lore raw

PATCH 1/3 crypto: inside-secure - fix clock management

In this driver the clock is got but never put when the driver is removed or if there is an error in the probe.

Using the managed version of clk_get() allows to let the kernel take care of it.

Fixes 1b44c5a60c13 ("crypto: inside-secure - add SafeXcel EIP197 crypto

engine driver")

cc stable at vger.kernel.org

Signed-off-by Gregory CLEMENT <[email protected]>

---
 drivers/crypto/inside-secure/safexcel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/crypto/inside-secure/safexcel.c +1 -1

index 384b4ceb37f0..09adeaa0da6b 100644
@@ -881,7 +881,7 @@ static int safexcel_probe(struct platform_device *pdev)
 		return PTR_ERR(priv->base);
 	}
 
-	priv->clk = of_clk_get(dev->of_node, 0);
+	priv->clk = devm_clk_get(&pdev->dev, NULL);
 	if (!IS_ERR(priv->clk)) {
 		ret = clk_prepare_enable(priv->clk);
 		if (ret) {

Signature · 1 line

2.16.1

Gregory CLEMENT

author

2018-03-13 16:48 UTC #2 lore raw

PATCH 2/3 crypto: inside-secure - improve clock initialization

In reply to Gregory CLEMENT

The clock is optional, but if it is present we should managed it. If there is an error while trying getting it, we should exit and report this error.

So instead of returning an error only in the -EPROBE case, turn it in an other way and ignore the clock only if it is not present (-ENOENT case).

Signed-off-by Gregory CLEMENT <[email protected]>

---
 drivers/crypto/inside-secure/safexcel.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

drivers/crypto/inside-secure/safexcel.c +6 -5

index 09adeaa0da6b..cbcb5d9f17bd 100644
@@ -882,16 +882,17 @@ static int safexcel_probe(struct platform_device *pdev)
 	}
 
 	priv->clk = devm_clk_get(&pdev->dev, NULL);
-	if (!IS_ERR(priv->clk)) {
+	ret = PTR_ERR_OR_ZERO(priv->clk);
+	/* The clock isn't mandatory */
+	if  (ret != -ENOENT) {
+		if (ret)
+			return ret;
+
 		ret = clk_prepare_enable(priv->clk);
 		if (ret) {
 			dev_err(dev, "unable to enable clk (%d)\n", ret);
 			return ret;
 		}
-	} else {
-		/* The clock isn't mandatory */
-		if (PTR_ERR(priv->clk) == -EPROBE_DEFER)
-			return -EPROBE_DEFER;
 	}
 
 	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));

Signature · 1 line

2.16.1

Gregory CLEMENT

author

2018-03-13 16:48 UTC #3 lore raw

PATCH 3/3 crypto: inside-secure - fix clock resource by adding a register clock

In reply to Gregory CLEMENT

On Armada 7K/8K we need to explicitly enable the register clock. This clock is optional because not all the SoCs using this IP need it but at least for Armada 7K/8K it is actually mandatory.

The binding documentation is updated accordingly.

Signed-off-by Gregory CLEMENT <[email protected]>

---
 .../bindings/crypto/inside-secure-safexcel.txt     |  6 +++-
 drivers/crypto/inside-secure/safexcel.c            | 34 ++++++++++++++++------
 drivers/crypto/inside-secure/safexcel.h            |  1 +
 3 files changed, 31 insertions(+), 10 deletions(-)

Documentation/devicetree/bindings/crypto/inside-secure-safexcel.txt +5 -1

index 30c3ce6b502e..5dba55cdfa63 100644
@@ -8,7 +8,11 @@ Required properties:
 - interrupt-names: Should be "ring0", "ring1", "ring2", "ring3", "eip", "mem".
 
 Optional properties:
-- clocks: Reference to the crypto engine clock.
+- clocks: Reference to the crypto engine clocks, the second clock is
+          needed for the Armada 7K/8K SoCs.
+- clock-names: mandatory if there is a second clock, in this case the
+               name must be "core" for the first clock and "reg" for
+               the second one.
 
 Example:

drivers/crypto/inside-secure/safexcel.c +25 -9

index cbcb5d9f17bd..2f68b4ed5500 100644
@@ -895,16 +895,30 @@ static int safexcel_probe(struct platform_device *pdev)
 		}
 	}
 
+	priv->reg_clk = devm_clk_get(&pdev->dev, "reg");
+	ret = PTR_ERR_OR_ZERO(priv->reg_clk);
+	/* The clock isn't mandatory */
+	if  (ret != -ENOENT) {
+		if (ret)
+			goto err_core_clk;
+
+		ret = clk_prepare_enable(priv->reg_clk);
+		if (ret) {
+			dev_err(dev, "unable to enable reg clk (%d)\n", ret);
+			goto err_core_clk;
+		}
+	}
+
 	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
 	if (ret)
-		goto err_clk;
+		goto err_reg_clk;
 
 	priv->context_pool = dmam_pool_create("safexcel-context", dev,
 					      sizeof(struct safexcel_context_record),
 					      1, 0);
 	if (!priv->context_pool) {
 		ret = -ENOMEM;
-		goto err_clk;
+		goto err_reg_clk;
 	}
 
 	safexcel_configure(priv);

@@ -919,12 +933,12 @@ static int safexcel_probe(struct platform_device *pdev)
 						     &priv->ring[i].cdr,
 						     &priv->ring[i].rdr);
 		if (ret)
-			goto err_clk;
+			goto err_reg_clk;
 
 		ring_irq = devm_kzalloc(dev, sizeof(*ring_irq), GFP_KERNEL);
 		if (!ring_irq) {
 			ret = -ENOMEM;
-			goto err_clk;
+			goto err_reg_clk;
 		}
 
 		ring_irq->priv = priv;

@@ -936,7 +950,7 @@ static int safexcel_probe(struct platform_device *pdev)
 						ring_irq);
 		if (irq < 0) {
 			ret = irq;
-			goto err_clk;
+			goto err_reg_clk;
 		}
 
 		priv->ring[i].work_data.priv = priv;

@@ -947,7 +961,7 @@ static int safexcel_probe(struct platform_device *pdev)
 		priv->ring[i].workqueue = create_singlethread_workqueue(wq_name);
 		if (!priv->ring[i].workqueue) {
 			ret = -ENOMEM;
-			goto err_clk;
+			goto err_reg_clk;
 		}
 
 		priv->ring[i].requests = 0;

@@ -968,18 +982,20 @@ static int safexcel_probe(struct platform_device *pdev)
 	ret = safexcel_hw_init(priv);
 	if (ret) {
 		dev_err(dev, "EIP h/w init failed (%d)\n", ret);
-		goto err_clk;
+		goto err_reg_clk;
 	}
 
 	ret = safexcel_register_algorithms(priv);
 	if (ret) {
 		dev_err(dev, "Failed to register algorithms (%d)\n", ret);
-		goto err_clk;
+		goto err_reg_clk;
 	}
 
 	return 0;
 
-err_clk:
+err_reg_clk:
+	clk_disable_unprepare(priv->reg_clk);
+err_core_clk:
 	clk_disable_unprepare(priv->clk);
 	return ret;
 }

drivers/crypto/inside-secure/safexcel.h +1 -0

index d8dff65fc311..4efeb0251daf 100644
@@ -525,6 +525,7 @@ struct safexcel_crypto_priv {
 	void __iomem *base;
 	struct device *dev;
 	struct clk *clk;
+	struct clk *reg_clk;
 	struct safexcel_config config;
 
 	enum safexcel_eip_version version;

Signature · 1 line

2.16.1

Herbert Xu

maintainer

2018-03-23 16:01 UTC #4 lore raw

cover crypto: inside-secure: improve clock management

In reply to Gregory CLEMENT

Quoted 8 lines · Gregory CLEMENT

Hi,
This short series fixes the way the clocks are used for the SafeXcel EIP-197 controller embedded in the Marvell Armada 7K/8K SoCs. On these SoCs a second one is needed in order to clock the registers. It was not noticed until now because we relied on the bootloader and also because the clock driver was wrong.

Quoted 6 lines

Thanks to this fix, it would be possible to fix the clock driver without introducing a regression.
While I was working on the clocks I found a bug in the clock management which was fixed with the first patch and should be applied to v4.16.

I haven't applied it to the crypto 4.16 tree because your other patches depend on it and I don't think the issue doesn't seem to be that severe.

In future it would help if you can avoid such dependencies if you wish the patches to go in as soon as possible.

Quoted 12 lines

The second patch is a small improvement which is aim for v4.17 as well as the last patch.
Thanks,
Gregory
Gregory CLEMENT (3):
crypto: inside-secure - fix clock management
crypto: inside-secure - improve clock initialization
crypto: inside-secure - fix clock resource by adding a register clock

Quoted 4 lines

 .../bindings/crypto/inside-secure-safexcel.txt     |  6 ++-
 drivers/crypto/inside-secure/safexcel.c            | 47 +++++++++++++++-------
 drivers/crypto/inside-secure/safexcel.h            |  1 +
 3 files changed, 38 insertions(+), 16 deletions(-)

All applied. Thanks.

Signature · 3 lines

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Gregory CLEMENT

author

2018-03-23 16:30 UTC #5 lore raw

cover crypto: inside-secure: improve clock management

Hi Herbert,

Quoted 16 lines · Herbert Xu

Quoted 8 lines · Gregory CLEMENT
Hi,
This short series fixes the way the clocks are used for the SafeXcel EIP-197 controller embedded in the Marvell Armada 7K/8K SoCs. On these SoCs a second one is needed in order to clock the registers. It was not noticed until now because we relied on the bootloader and also because the clock driver was wrong.
Quoted 6 lines
Thanks to this fix, it would be possible to fix the clock driver without introducing a regression.
While I was working on the clocks I found a bug in the clock management which was fixed with the first patch and should be applied to v4.16.

Quoted 6 lines

I haven't applied it to the crypto 4.16 tree because your other patches depend on it and I don't think the issue doesn't seem to be that severe.
In future it would help if you can avoid such dependencies if you wish the patches to go in as soon as possible.

Yes sure. And I agree with you, the issue is not that severe, so it can wait for 4.17.

Quoted 13 lines

Quoted 13 lines
The second patch is a small improvement which is aim for v4.17 as well as the last patch.
Thanks,
Gregory
Gregory CLEMENT (3):
crypto: inside-secure - fix clock management
crypto: inside-secure - improve clock initialization
crypto: inside-secure - fix clock resource by adding a register clock

Quoted 6 lines

Quoted 4 lines

 .../bindings/crypto/inside-secure-safexcel.txt     |  6 ++-
 drivers/crypto/inside-secure/safexcel.c            | 47 +++++++++++++++-------
 drivers/crypto/inside-secure/safexcel.h            |  1 +
 3 files changed, 38 insertions(+), 16 deletions(-)

All applied. Thanks.

Thanks

Quoted 4 lines

Signature · 3 lines

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Signature · 3 lines

Gregory Clement, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com