cesa: small cleanups

PATCH In review Rosen Penev 2 participants 8 messages Oct 10, 2024 – Oct 19, 2024

Subsystems (2)

Crypto API Marvell crypto driver

lore

Share & tools

lore

Rosen Penev

author

2024-10-10 19:45 UTC #0 lore raw

v2 cover cesa: small cleanups

Mostly increased devm usage and added COMPILE_TEST.

v2: fix wrong returns in clk_get conversions

Rosen Penev (5):
crypto: cesa: add COMPILE_TEST
crypto: cesa: use enabled variants for clk_get
crypto: cesa: remove irq_set_affinity_hint
crypto: cesa: move loop to mv_cesa_put_sram
crypto: cesa: use devm_platform_get_and_ioremap_resource

 drivers/crypto/marvell/Kconfig     |  2 +-
 drivers/crypto/marvell/cesa/cesa.c | 79 ++++++++++--------------------
 2 files changed, 28 insertions(+), 53 deletions(-)

Signature · 1 line

2.46.2

Rosen Penev

author

2024-10-10 19:45 UTC #1 lore raw

PATCH v2 1/5 crypto: cesa: add COMPILE_TEST

This can build on x86. Allows more CI coverage.

Signed-off-by Rosen Penev <[email protected]>

---
 drivers/crypto/marvell/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/crypto/marvell/Kconfig +1 -1

index 78217577aa54..4c25a78ab3ed 100644
@@ -7,7 +7,7 @@ config CRYPTO_DEV_MARVELL
 
 config CRYPTO_DEV_MARVELL_CESA
 	tristate "Marvell's Cryptographic Engine driver"
-	depends on PLAT_ORION || ARCH_MVEBU
+	depends on PLAT_ORION || ARCH_MVEBU || COMPILE_TEST
 	select CRYPTO_LIB_AES
 	select CRYPTO_LIB_DES
 	select CRYPTO_SKCIPHER

Signature · 1 line

2.46.2

Rosen Penev

author

2024-10-10 19:45 UTC #2 lore raw

PATCH v2 2/5 crypto: cesa: use enabled variants for clk_get

In reply to Rosen Penev

Avoids manual frees. Also simplifiies the code with _optional

Signed-off-by Rosen Penev <[email protected]>

---
 drivers/crypto/marvell/cesa/cesa.c | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

drivers/crypto/marvell/cesa/cesa.c +10 ~~-18~~

index 5fd31ba715c2..74a1d1ad67d3 100644
@@ -510,25 +510,21 @@ static int mv_cesa_probe(struct platform_device *pdev)
 		 * if the clock does not exist.
 		 */
 		snprintf(res_name, sizeof(res_name), "cesa%u", i);
-		engine->clk = devm_clk_get(dev, res_name);
+		engine->clk = devm_clk_get_optional_enabled(dev, res_name);
 		if (IS_ERR(engine->clk)) {
-			engine->clk = devm_clk_get(dev, NULL);
-			if (IS_ERR(engine->clk))
-				engine->clk = NULL;
+			engine->clk = devm_clk_get_optional_enabled(dev, NULL);
+			if (IS_ERR(engine->clk)) {
+				ret = PTR_ERR(engine->clk);
+				goto err_cleanup;
+			}
 		}
 
 		snprintf(res_name, sizeof(res_name), "cesaz%u", i);
-		engine->zclk = devm_clk_get(dev, res_name);
-		if (IS_ERR(engine->zclk))
-			engine->zclk = NULL;
-
-		ret = clk_prepare_enable(engine->clk);
-		if (ret)
-			goto err_cleanup;
-
-		ret = clk_prepare_enable(engine->zclk);
-		if (ret)
+		engine->zclk = devm_clk_get_optional_enabled(dev, res_name);
+		if (IS_ERR(engine->zclk)) {
+			ret = PTR_ERR(engine->zclk);
 			goto err_cleanup;
+		}
 
 		engine->regs = cesa->regs + CESA_ENGINE_OFF(i);

@@ -571,8 +567,6 @@ static int mv_cesa_probe(struct platform_device *pdev)
 
 err_cleanup:
 	for (i = 0; i < caps->nengines; i++) {
-		clk_disable_unprepare(cesa->engines[i].zclk);
-		clk_disable_unprepare(cesa->engines[i].clk);
 		mv_cesa_put_sram(pdev, i);
 		if (cesa->engines[i].irq > 0)
 			irq_set_affinity_hint(cesa->engines[i].irq, NULL);

@@ -589,8 +583,6 @@ static void mv_cesa_remove(struct platform_device *pdev)
 	mv_cesa_remove_algs(cesa);
 
 	for (i = 0; i < cesa->caps->nengines; i++) {
-		clk_disable_unprepare(cesa->engines[i].zclk);
-		clk_disable_unprepare(cesa->engines[i].clk);
 		mv_cesa_put_sram(pdev, i);
 		irq_set_affinity_hint(cesa->engines[i].irq, NULL);
 	}

Signature · 1 line

2.46.2

Rosen Penev

author

2024-10-10 19:45 UTC #3 lore raw

PATCH v2 3/5 crypto: cesa: remove irq_set_affinity_hint

In reply to Rosen Penev

This is unnecessary as the irqs are devm managed.

Signed-off-by Rosen Penev <[email protected]>

---
 drivers/crypto/marvell/cesa/cesa.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

drivers/crypto/marvell/cesa/cesa.c +2 -7

index 74a1d1ad67d3..21e13f4a1f48 100644
@@ -566,11 +566,8 @@ static int mv_cesa_probe(struct platform_device *pdev)
 	return 0;
 
 err_cleanup:
-	for (i = 0; i < caps->nengines; i++) {
+	for (i = 0; i < caps->nengines; i++)
 		mv_cesa_put_sram(pdev, i);
-		if (cesa->engines[i].irq > 0)
-			irq_set_affinity_hint(cesa->engines[i].irq, NULL);
-	}
 
 	return ret;
 }

@@ -582,10 +579,8 @@ static void mv_cesa_remove(struct platform_device *pdev)
 
 	mv_cesa_remove_algs(cesa);
 
-	for (i = 0; i < cesa->caps->nengines; i++) {
+	for (i = 0; i < cesa->caps->nengines; i++)
 		mv_cesa_put_sram(pdev, i);
-		irq_set_affinity_hint(cesa->engines[i].irq, NULL);
-	}
 }
 
 static const struct platform_device_id mv_cesa_plat_id_table[] = {

Signature · 1 line

2.46.2

Rosen Penev

author

2024-10-10 19:45 UTC #4 lore raw

PATCH v2 4/5 crypto: cesa: move loop to mv_cesa_put_sram

In reply to Rosen Penev

Simpler to move it there. More readable remove.

Signed-off-by Rosen Penev <[email protected]>

---
 drivers/crypto/marvell/cesa/cesa.c | 31 +++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

drivers/crypto/marvell/cesa/cesa.c +16 ~~-15~~

index 21e13f4a1f48..a65f328b7094 100644
@@ -416,17 +416,23 @@ static int mv_cesa_get_sram(struct platform_device *pdev, int idx)
 	return 0;
 }
 
-static void mv_cesa_put_sram(struct platform_device *pdev, int idx)
+static void mv_cesa_put_sram(struct platform_device *pdev)
 {
 	struct mv_cesa_dev *cesa = platform_get_drvdata(pdev);
-	struct mv_cesa_engine *engine = &cesa->engines[idx];
+	const struct mv_cesa_caps *caps = &orion_caps;
+	int i;
 
-	if (engine->pool)
-		gen_pool_free(engine->pool, (unsigned long)engine->sram_pool,
-			      cesa->sram_size);
-	else
-		dma_unmap_resource(cesa->dev, engine->sram_dma,
-				   cesa->sram_size, DMA_BIDIRECTIONAL, 0);
+	for (i = 0; i < caps->nengines; i++) {
+		struct mv_cesa_engine *engine = &cesa->engines[i];
+		if (engine->pool)
+			gen_pool_free(engine->pool,
+				      (unsigned long)engine->sram_pool,
+				      cesa->sram_size);
+		else
+			dma_unmap_resource(cesa->dev, engine->sram_dma,
+					   cesa->sram_size, DMA_BIDIRECTIONAL,
+					   0);
+	}
 }
 
 static int mv_cesa_probe(struct platform_device *pdev)

@@ -566,21 +572,16 @@ static int mv_cesa_probe(struct platform_device *pdev)
 	return 0;
 
 err_cleanup:
-	for (i = 0; i < caps->nengines; i++)
-		mv_cesa_put_sram(pdev, i);
-
+	mv_cesa_put_sram(pdev);
 	return ret;
 }
 
 static void mv_cesa_remove(struct platform_device *pdev)
 {
 	struct mv_cesa_dev *cesa = platform_get_drvdata(pdev);
-	int i;
 
 	mv_cesa_remove_algs(cesa);
-
-	for (i = 0; i < cesa->caps->nengines; i++)
-		mv_cesa_put_sram(pdev, i);
+	mv_cesa_put_sram(pdev);
 }
 
 static const struct platform_device_id mv_cesa_plat_id_table[] = {

Signature · 1 line

2.46.2

Herbert Xu

maintainer

2024-10-18 09:12 UTC #5 lore raw

Re: [PATCHv2 4/5] crypto: cesa: move loop to mv_cesa_put_sram

drivers/crypto/marvell/cesa/cesa.c static int mv_cesa_probe(struct platform_device *pdev) · line 575

Quoted 9 lines · Rosen Penev

drivers/crypto/marvell/cesa/cesa.c +1 -3

@@ -566,21 +572,16 @@ static int mv_cesa_probe(struct platform_device *pdev)
 	return 0;
 
 err_cleanup:
-	for (i = 0; i < caps->nengines; i++)
-		mv_cesa_put_sram(pdev, i);
-
+	mv_cesa_put_sram(pdev);

I think it'd be cleaner if you introduced a new function that did the loop and kept mv_cesa_put_sram as is.

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

Rosen Penev

author

2024-10-10 19:45 UTC #6 lore raw

PATCH v2 5/5 crypto: cesa: use devm_platform_get_and_ioremap_resource

In reply to Rosen Penev

Removes separate steps. Requires index instead of name.

Signed-off-by Rosen Penev <[email protected]>

---
 drivers/crypto/marvell/cesa/cesa.c | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

drivers/crypto/marvell/cesa/cesa.c +1 ~~-14~~

index a65f328b7094..ef4b1c2336bb 100644
@@ -375,7 +375,6 @@ static int mv_cesa_get_sram(struct platform_device *pdev, int idx)
 {
 	struct mv_cesa_dev *cesa = platform_get_drvdata(pdev);
 	struct mv_cesa_engine *engine = &cesa->engines[idx];
-	const char *res_name = "sram";
 	struct resource *res;
 
 	engine->pool = of_gen_pool_get(cesa->dev->of_node,

@@ -391,19 +390,7 @@ static int mv_cesa_get_sram(struct platform_device *pdev, int idx)
 		return -ENOMEM;
 	}
 
-	if (cesa->caps->nengines > 1) {
-		if (!idx)
-			res_name = "sram0";
-		else
-			res_name = "sram1";
-	}
-
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
-					   res_name);
-	if (!res || resource_size(res) < cesa->sram_size)
-		return -EINVAL;
-
-	engine->sram = devm_ioremap_resource(cesa->dev, res);
+	engine->sram = devm_platform_get_and_ioremap_resource(pdev, idx, &res);
 	if (IS_ERR(engine->sram))
 		return PTR_ERR(engine->sram);

Signature · 1 line

2.46.2

Herbert Xu

maintainer

2024-10-19 11:58 UTC #7 lore raw

In reply to Rosen Penev

Quoted 11 lines · Rosen Penev

Mostly increased devm usage and added COMPILE_TEST.
v2: fix wrong returns in clk_get conversions
Rosen Penev (5):
crypto: cesa: add COMPILE_TEST
crypto: cesa: use enabled variants for clk_get
crypto: cesa: remove irq_set_affinity_hint
crypto: cesa: move loop to mv_cesa_put_sram
crypto: cesa: use devm_platform_get_and_ioremap_resource

Quoted 6 lines

 drivers/crypto/marvell/Kconfig     |  2 +-
 drivers/crypto/marvell/cesa/cesa.c | 79 ++++++++++--------------------
 2 files changed, 28 insertions(+), 53 deletions(-)

Signature · 1 line

2.46.2

Patches 1-3,5 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