[PATCH 0/6] Misc. SA2UL fixes/cleanups

Hi All,

The following series includes various fixes for multiple minor
cleanup issues on failure paths in the TI K3 SA2UL Crypto driver.
The patches are all based on top of 5.13-rc1.

Patch 1 is the only non SA2UL patch, and allows the OMAP RNG driver
to be built for K3 platforms and probed properly when SA2UL is
selected, and the SA2UL driver is probed (the rng device is a child
of the sa2ul device).

regards
Suman

Suman Anna (6):
  hwrng: omap - Enable driver for TI K3 family
  crypto: sa2ul - Fix leaks on failure paths with sa_dma_init()
  crypto: sa2ul - Fix pm_runtime enable in sa_ul_probe()
  crypto: sa2ul - Use of_device_get_match_data() helper
  crypto: sa2ul - Use devm_platform_ioremap_resource()
  crypto: sa2ul - Remove child devices in remove

 drivers/char/hw_random/Kconfig |  2 +-
 drivers/crypto/sa2ul.c         | 46 ++++++++++++++++++----------------
 2 files changed, 25 insertions(+), 23 deletions(-)

-- 
2.30.1

The TI K3 family of SoCs have a SA2UL IP that contains a
SafeXcel IP-76 RNG block which is supported by the OMAP
RNG driver. Allow this driver to be built for TI K3
family as well.

Signed-off-by: Suman Anna <redacted>
---
 drivers/char/hw_random/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index 1fe006f3f12f..6450074c0ad7 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig

@@ -165,7 +165,7 @@ config HW_RANDOM_IXP4XX
 
 config HW_RANDOM_OMAP
 	tristate "OMAP Random Number Generator support"
-	depends on ARCH_OMAP16XX || ARCH_OMAP2PLUS || ARCH_MVEBU
+	depends on ARCH_OMAP16XX || ARCH_OMAP2PLUS || ARCH_MVEBU || ARCH_K3
 	default HW_RANDOM
 	help
  	  This driver provides kernel-side support for the Random Number

-- 
2.30.1

The sa_dma_init() function doesn't release the requested dma channels
on all failure paths. Any failure in this function also ends up
leaking the dma pool created in sa_init_mem() in the sa_ul_probe()
function. Fix all of these issues.

Fixes: 7694b6ca649f ("crypto: sa2ul - Add crypto driver")
Signed-off-by: Suman Anna <redacted>
---
 drivers/crypto/sa2ul.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/crypto/sa2ul.c b/drivers/crypto/sa2ul.c
index 1c6929fb3a13..3d6f0af2f938 100644
--- a/drivers/crypto/sa2ul.c
+++ b/drivers/crypto/sa2ul.c

@@ -2300,9 +2300,9 @@ static int sa_dma_init(struct sa_crypto_data *dd)
 
 	dd->dma_rx2 = dma_request_chan(dd->dev, "rx2");
 	if (IS_ERR(dd->dma_rx2)) {
-		dma_release_channel(dd->dma_rx1);
-		return dev_err_probe(dd->dev, PTR_ERR(dd->dma_rx2),
-				     "Unable to request rx2 DMA channel\n");
+		ret = dev_err_probe(dd->dev, PTR_ERR(dd->dma_rx2),
+				    "Unable to request rx2 DMA channel\n");
+		goto err_dma_rx2;
 	}
 
 	dd->dma_tx = dma_request_chan(dd->dev, "tx");

@@ -2323,28 +2323,31 @@ static int sa_dma_init(struct sa_crypto_data *dd)
 	if (ret) {
 		dev_err(dd->dev, "can't configure IN dmaengine slave: %d\n",
 			ret);
-		return ret;
+		goto err_dma_config;
 	}
 
 	ret = dmaengine_slave_config(dd->dma_rx2, &cfg);
 	if (ret) {
 		dev_err(dd->dev, "can't configure IN dmaengine slave: %d\n",
 			ret);
-		return ret;
+		goto err_dma_config;
 	}
 
 	ret = dmaengine_slave_config(dd->dma_tx, &cfg);
 	if (ret) {
 		dev_err(dd->dev, "can't configure OUT dmaengine slave: %d\n",
 			ret);
-		return ret;
+		goto err_dma_config;
 	}
 
 	return 0;
 
+err_dma_config:
+	dma_release_channel(dd->dma_tx);
 err_dma_tx:
-	dma_release_channel(dd->dma_rx1);
 	dma_release_channel(dd->dma_rx2);
+err_dma_rx2:
+	dma_release_channel(dd->dma_rx1);
 
 	return ret;
 }

@@ -2414,7 +2417,7 @@ static int sa_ul_probe(struct platform_device *pdev)
 	sa_init_mem(dev_data);
 	ret = sa_dma_init(dev_data);
 	if (ret)
-		goto disable_pm_runtime;
+		goto destroy_dma_pool;
 
 	match = of_match_node(of_match, dev->of_node);
 	if (!match) {

@@ -2454,9 +2457,9 @@ static int sa_ul_probe(struct platform_device *pdev)
 	dma_release_channel(dev_data->dma_rx1);
 	dma_release_channel(dev_data->dma_tx);
 
+destroy_dma_pool:
 	dma_pool_destroy(dev_data->sc_pool);
 
-disable_pm_runtime:
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 

-- 
2.30.1

The pm_runtime APIs added first in commit 7694b6ca649f ("crypto: sa2ul -
Add crypto driver") are not unwound properly and was fixed up partially
in commit 13343badae09 ("crypto: sa2ul - Fix PM reference leak in
sa_ul_probe()"). This fixed up the pm_runtime usage count but not the
state. Fix this properly.

Fixes: 13343badae09 ("crypto: sa2ul - Fix PM reference leak in sa_ul_probe()")
Signed-off-by: Suman Anna <redacted>
---
 drivers/crypto/sa2ul.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/crypto/sa2ul.c b/drivers/crypto/sa2ul.c
index 3d6f0af2f938..a215daedf78a 100644
--- a/drivers/crypto/sa2ul.c
+++ b/drivers/crypto/sa2ul.c

@@ -2411,6 +2411,7 @@ static int sa_ul_probe(struct platform_device *pdev)
 	if (ret < 0) {
 		dev_err(&pdev->dev, "%s: failed to get sync: %d\n", __func__,
 			ret);
+		pm_runtime_disable(dev);
 		return ret;
 	}
 

-- 
2.30.1

Simplify the probe function by using the of_device_get_match_data()
helper instead of open coding. The logic is also moved up to fix the
missing pm_runtime cleanup in case of a match failure.

Fixes: 0bc42311cdff ("crypto: sa2ul - Add support for AM64")
Signed-off-by: Suman Anna <redacted>
---
 drivers/crypto/sa2ul.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/crypto/sa2ul.c b/drivers/crypto/sa2ul.c
index a215daedf78a..9f077ec9dbb7 100644
--- a/drivers/crypto/sa2ul.c
+++ b/drivers/crypto/sa2ul.c

@@ -2388,7 +2388,6 @@ MODULE_DEVICE_TABLE(of, of_match);
 
 static int sa_ul_probe(struct platform_device *pdev)
 {
-	const struct of_device_id *match;
 	struct device *dev = &pdev->dev;
 	struct device_node *node = dev->of_node;
 	struct resource *res;

@@ -2400,6 +2399,10 @@ static int sa_ul_probe(struct platform_device *pdev)
 	if (!dev_data)
 		return -ENOMEM;
 
+	dev_data->match_data = of_device_get_match_data(dev);
+	if (!dev_data->match_data)
+		return -ENODEV;
+
 	sa_k3_dev = dev;
 	dev_data->dev = dev;
 	dev_data->pdev = pdev;

@@ -2420,13 +2423,6 @@ static int sa_ul_probe(struct platform_device *pdev)
 	if (ret)
 		goto destroy_dma_pool;
 
-	match = of_match_node(of_match, dev->of_node);
-	if (!match) {
-		dev_err(dev, "No compatible match found\n");
-		return -ENODEV;
-	}
-	dev_data->match_data = match->data;
-
 	spin_lock_init(&dev_data->scid_lock);
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	saul_base = devm_ioremap_resource(dev, res);

-- 
2.30.1

Simplify the platform_get_resource() and devm_ioremap_resource()
calls with devm_platform_ioremap_resource(). Also add error checking
and move up this block to simplify the cleanup in sa_ul_probe().

Signed-off-by: Suman Anna <redacted>
---
 drivers/crypto/sa2ul.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/sa2ul.c b/drivers/crypto/sa2ul.c
index 9f077ec9dbb7..216702fef945 100644
--- a/drivers/crypto/sa2ul.c
+++ b/drivers/crypto/sa2ul.c

@@ -2390,7 +2390,6 @@ static int sa_ul_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct device_node *node = dev->of_node;
-	struct resource *res;
 	static void __iomem *saul_base;
 	struct sa_crypto_data *dev_data;
 	int ret;

@@ -2403,9 +2402,14 @@ static int sa_ul_probe(struct platform_device *pdev)
 	if (!dev_data->match_data)
 		return -ENODEV;
 
+	saul_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(saul_base))
+		return PTR_ERR(saul_base);
+
 	sa_k3_dev = dev;
 	dev_data->dev = dev;
 	dev_data->pdev = pdev;
+	dev_data->base = saul_base;
 	platform_set_drvdata(pdev, dev_data);
 	dev_set_drvdata(sa_k3_dev, dev_data);
 

@@ -2424,10 +2428,6 @@ static int sa_ul_probe(struct platform_device *pdev)
 		goto destroy_dma_pool;
 
 	spin_lock_init(&dev_data->scid_lock);
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	saul_base = devm_ioremap_resource(dev, res);
-
-	dev_data->base = saul_base;
 
 	if (!dev_data->match_data->skip_engine_control) {
 		u32 val = SA_EEC_ENCSS_EN | SA_EEC_AUTHSS_EN | SA_EEC_CTXCACH_EN |

-- 
2.30.1

The sa_ul_probe creates child devices using of_platform_populate(),
but these are not cleaned up in driver remove. Clean these up
by removing the child devices using of_platform_depopulate().

Signed-off-by: Suman Anna <redacted>
---
 drivers/crypto/sa2ul.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/crypto/sa2ul.c b/drivers/crypto/sa2ul.c
index 216702fef945..51bb69bc573c 100644
--- a/drivers/crypto/sa2ul.c
+++ b/drivers/crypto/sa2ul.c

@@ -2467,6 +2467,8 @@ static int sa_ul_remove(struct platform_device *pdev)
 {
 	struct sa_crypto_data *dev_data = platform_get_drvdata(pdev);
 
+	of_platform_depopulate(&pdev->dev);
+
 	sa_unregister_algos(&pdev->dev);
 
 	dma_release_channel(dev_data->dma_rx2);

-- 
2.30.1

On 14/05/2021 19:12, Suman Anna wrote:

Hi All,

The following series includes various fixes for multiple minor
cleanup issues on failure paths in the TI K3 SA2UL Crypto driver.
The patches are all based on top of 5.13-rc1.

Patch 1 is the only non SA2UL patch, and allows the OMAP RNG driver
to be built for K3 platforms and probed properly when SA2UL is
selected, and the SA2UL driver is probed (the rng device is a child
of the sa2ul device).

regards
Suman

Suman Anna (6):
   hwrng: omap - Enable driver for TI K3 family
   crypto: sa2ul - Fix leaks on failure paths with sa_dma_init()
   crypto: sa2ul - Fix pm_runtime enable in sa_ul_probe()
   crypto: sa2ul - Use of_device_get_match_data() helper
   crypto: sa2ul - Use devm_platform_ioremap_resource()
   crypto: sa2ul - Remove child devices in remove

For the whole series:

Reviewed-by: Tero Kristo <kristo@kernel.org>

  drivers/char/hw_random/Kconfig |  2 +-
  drivers/crypto/sa2ul.c         | 46 ++++++++++++++++++----------------
  2 files changed, 25 insertions(+), 23 deletions(-)

On Fri, May 14, 2021 at 11:12:40AM -0500, Suman Anna wrote:

Hi All,

The following series includes various fixes for multiple minor
cleanup issues on failure paths in the TI K3 SA2UL Crypto driver.
The patches are all based on top of 5.13-rc1.

Patch 1 is the only non SA2UL patch, and allows the OMAP RNG driver
to be built for K3 platforms and probed properly when SA2UL is
selected, and the SA2UL driver is probed (the rng device is a child
of the sa2ul device).

regards
Suman

Suman Anna (6):
  hwrng: omap - Enable driver for TI K3 family
  crypto: sa2ul - Fix leaks on failure paths with sa_dma_init()
  crypto: sa2ul - Fix pm_runtime enable in sa_ul_probe()
  crypto: sa2ul - Use of_device_get_match_data() helper
  crypto: sa2ul - Use devm_platform_ioremap_resource()
  crypto: sa2ul - Remove child devices in remove

 drivers/char/hw_random/Kconfig |  2 +-
 drivers/crypto/sa2ul.c         | 46 ++++++++++++++++++----------------
 2 files changed, 25 insertions(+), 23 deletions(-)

All applied.  Thanks.
-- 
Email: Herbert Xu [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt