[PATCH 1/5] crypto: atmel-{aes,sha} - Fix incorrect use of dmaengine_terminate_all()

[PATCH 1/5] crypto: atmel-{aes,sha} - Fix incorrect use of dmaengine_terminate_all()

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

device_terminate_all() is used to abort all the pending and
ongoing transfers on the channel, it should be used just in the
error path.

Also, dmaengine_terminate_all() is deprecated and one should use
dmaengine_terminate_async() or dmaengine_terminate_sync(). The method
is not used in atomic context, use dmaengine_terminate_sync().

A secondary aspect of this patch is that it luckily avoids a deadlock
between atmel_aes and at_hdmac.c. While in tasklet with the lock held,
the dma controller invokes the client callback (dmaengine_terminate_all),
which tries to get the same lock. The at_hdmac fix would be to drop the
lock before invoking the client callback, a fix on at_hdmac will follow.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/crypto/atmel-aes.c | 32 ++------------------------------
 drivers/crypto/atmel-sha.c |  1 -
 2 files changed, 2 insertions(+), 31 deletions(-)

diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
index 47b20df3adfc..c3f0e99d24b0 100644
--- a/drivers/crypto/atmel-aes.c
+++ b/drivers/crypto/atmel-aes.c
@@ -857,27 +857,6 @@ static int atmel_aes_dma_transfer_start(struct atmel_aes_dev *dd,
 	return 0;
 }
 
-static void atmel_aes_dma_transfer_stop(struct atmel_aes_dev *dd,
-					enum dma_transfer_direction dir)
-{
-	struct atmel_aes_dma *dma;
-
-	switch (dir) {
-	case DMA_MEM_TO_DEV:
-		dma = &dd->src;
-		break;
-
-	case DMA_DEV_TO_MEM:
-		dma = &dd->dst;
-		break;
-
-	default:
-		return;
-	}
-
-	dmaengine_terminate_all(dma->chan);
-}
-
 static int atmel_aes_dma_start(struct atmel_aes_dev *dd,
 			       struct scatterlist *src,
 			       struct scatterlist *dst,
@@ -936,25 +915,18 @@ static int atmel_aes_dma_start(struct atmel_aes_dev *dd,
 	return -EINPROGRESS;
 
 output_transfer_stop:
-	atmel_aes_dma_transfer_stop(dd, DMA_DEV_TO_MEM);
+	dmaengine_terminate_sync(dd->dst.chan);
 unmap:
 	atmel_aes_unmap(dd);
 exit:
 	return atmel_aes_complete(dd, err);
 }
 
-static void atmel_aes_dma_stop(struct atmel_aes_dev *dd)
-{
-	atmel_aes_dma_transfer_stop(dd, DMA_MEM_TO_DEV);
-	atmel_aes_dma_transfer_stop(dd, DMA_DEV_TO_MEM);
-	atmel_aes_unmap(dd);
-}
-
 static void atmel_aes_dma_callback(void *data)
 {
 	struct atmel_aes_dev *dd = data;
 
-	atmel_aes_dma_stop(dd);
+	atmel_aes_unmap(dd);
 	dd->is_async = true;
 	(void)dd->resume(dd);
 }
diff --git a/drivers/crypto/atmel-sha.c b/drivers/crypto/atmel-sha.c
index ebf500153700..7cf4ec9ed93a 100644
--- a/drivers/crypto/atmel-sha.c
+++ b/drivers/crypto/atmel-sha.c
@@ -1429,7 +1429,6 @@ static void atmel_sha_dma_callback2(void *data)
 	struct scatterlist *sg;
 	int nents;
 
-	dmaengine_terminate_all(dma->chan);
 	dma_unmap_sg(dd->dev, dma->sg, dma->nents, DMA_TO_DEVICE);
 
 	sg = dma->sg;
-- 
2.20.1

[PATCH 2/5] crypto: atmel-{aes,sha,tdes} - Drop duplicate init of dma_slave_config.direction

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

The 'direction' member of the dma_slave_config will be going away
as it duplicates the direction given in the prepare call.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/crypto/atmel-aes.c  | 1 -
 drivers/crypto/atmel-sha.c  | 1 -
 drivers/crypto/atmel-tdes.c | 2 --
 3 files changed, 4 deletions(-)

diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
index c3f0e99d24b0..d5a116693010 100644
--- a/drivers/crypto/atmel-aes.c
+++ b/drivers/crypto/atmel-aes.c
@@ -817,7 +817,6 @@ static int atmel_aes_dma_transfer_start(struct atmel_aes_dev *dd,
 	int err;
 
 	memset(&config, 0, sizeof(config));
-	config.direction = dir;
 	config.src_addr_width = addr_width;
 	config.dst_addr_width = addr_width;
 	config.src_maxburst = maxburst;
diff --git a/drivers/crypto/atmel-sha.c b/drivers/crypto/atmel-sha.c
index 7cf4ec9ed93a..21fcc04f6688 100644
--- a/drivers/crypto/atmel-sha.c
+++ b/drivers/crypto/atmel-sha.c
@@ -2493,7 +2493,6 @@ static int atmel_sha_dma_init(struct atmel_sha_dev *dd,
 		return ret;
 	}
 
-	dd->dma_lch_in.dma_conf.direction = DMA_MEM_TO_DEV;
 	dd->dma_lch_in.dma_conf.dst_addr = dd->phys_base +
 		SHA_REG_DIN(0);
 	dd->dma_lch_in.dma_conf.src_maxburst = 1;
diff --git a/drivers/crypto/atmel-tdes.c b/drivers/crypto/atmel-tdes.c
index 12268b549596..51b332527efd 100644
--- a/drivers/crypto/atmel-tdes.c
+++ b/drivers/crypto/atmel-tdes.c
@@ -753,7 +753,6 @@ static int atmel_tdes_dma_init(struct atmel_tdes_dev *dd,
 		goto err_dma_in;
 	}
 
-	dd->dma_lch_in.dma_conf.direction = DMA_MEM_TO_DEV;
 	dd->dma_lch_in.dma_conf.dst_addr = dd->phys_base +
 		TDES_IDATA1R;
 	dd->dma_lch_in.dma_conf.src_maxburst = 1;
@@ -770,7 +769,6 @@ static int atmel_tdes_dma_init(struct atmel_tdes_dev *dd,
 		goto err_dma_out;
 	}
 
-	dd->dma_lch_out.dma_conf.direction = DMA_DEV_TO_MEM;
 	dd->dma_lch_out.dma_conf.src_addr = dd->phys_base +
 		TDES_ODATA1R;
 	dd->dma_lch_out.dma_conf.src_maxburst = 1;
-- 
2.20.1

[PATCH 3/5] crypto: atmel-{aes,sha,tdes} - Stop passing unused argument in _dma_init()

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

pdata is not used.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/crypto/atmel-aes.c  | 5 ++---
 drivers/crypto/atmel-sha.c  | 5 ++---
 drivers/crypto/atmel-tdes.c | 5 ++---
 3 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
index d5a116693010..986f1ca682aa 100644
--- a/drivers/crypto/atmel-aes.c
+++ b/drivers/crypto/atmel-aes.c
@@ -2270,8 +2270,7 @@ static void atmel_aes_buff_cleanup(struct atmel_aes_dev *dd)
 	free_page((unsigned long)dd->buf);
 }
 
-static int atmel_aes_dma_init(struct atmel_aes_dev *dd,
-			      struct crypto_platform_data *pdata)
+static int atmel_aes_dma_init(struct atmel_aes_dev *dd)
 {
 	int ret;
 
@@ -2598,7 +2597,7 @@ static int atmel_aes_probe(struct platform_device *pdev)
 	if (err)
 		goto err_iclk_unprepare;
 
-	err = atmel_aes_dma_init(aes_dd, pdata);
+	err = atmel_aes_dma_init(aes_dd);
 	if (err)
 		goto err_buff_cleanup;
 
diff --git a/drivers/crypto/atmel-sha.c b/drivers/crypto/atmel-sha.c
index 21fcc04f6688..a0d42bdc311f 100644
--- a/drivers/crypto/atmel-sha.c
+++ b/drivers/crypto/atmel-sha.c
@@ -2481,8 +2481,7 @@ static int atmel_sha_register_algs(struct atmel_sha_dev *dd)
 	return err;
 }
 
-static int atmel_sha_dma_init(struct atmel_sha_dev *dd,
-				struct crypto_platform_data *pdata)
+static int atmel_sha_dma_init(struct atmel_sha_dev *dd)
 {
 	dd->dma_lch_in.chan = dma_request_chan(dd->dev, "tx");
 	if (IS_ERR(dd->dma_lch_in.chan)) {
@@ -2674,7 +2673,7 @@ static int atmel_sha_probe(struct platform_device *pdev)
 			}
 		}
 
-		err = atmel_sha_dma_init(sha_dd, pdata);
+		err = atmel_sha_dma_init(sha_dd);
 		if (err)
 			goto err_iclk_unprepare;
 
diff --git a/drivers/crypto/atmel-tdes.c b/drivers/crypto/atmel-tdes.c
index 51b332527efd..d42b22775ee9 100644
--- a/drivers/crypto/atmel-tdes.c
+++ b/drivers/crypto/atmel-tdes.c
@@ -741,8 +741,7 @@ static int atmel_tdes_crypt(struct skcipher_request *req, unsigned long mode)
 	return atmel_tdes_handle_queue(ctx->dd, req);
 }
 
-static int atmel_tdes_dma_init(struct atmel_tdes_dev *dd,
-			struct crypto_platform_data *pdata)
+static int atmel_tdes_dma_init(struct atmel_tdes_dev *dd)
 {
 	int ret;
 
@@ -1268,7 +1267,7 @@ static int atmel_tdes_probe(struct platform_device *pdev)
 			}
 		}
 
-		err = atmel_tdes_dma_init(tdes_dd, pdata);
+		err = atmel_tdes_dma_init(tdes_dd);
 		if (err)
 			goto err_buff_cleanup;
 
-- 
2.20.1

[PATCH 4/5] crypto: atmel-{sha,tdes} - Print warn message even when deferring

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

Even when deferring, we would like to know what caused it.
Update dev_warn to dev_err because if the DMA init fails,
the probe is stopped.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/crypto/atmel-aes.c  | 2 +-
 drivers/crypto/atmel-sha.c  | 7 ++-----
 drivers/crypto/atmel-tdes.c | 3 +--
 3 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
index 986f1ca682aa..855f0ccc9368 100644
--- a/drivers/crypto/atmel-aes.c
+++ b/drivers/crypto/atmel-aes.c
@@ -2292,7 +2292,7 @@ static int atmel_aes_dma_init(struct atmel_aes_dev *dd)
 err_dma_out:
 	dma_release_channel(dd->src.chan);
 err_dma_in:
-	dev_warn(dd->dev, "no DMA channel available\n");
+	dev_err(dd->dev, "no DMA channel available\n");
 	return ret;
 }
 
diff --git a/drivers/crypto/atmel-sha.c b/drivers/crypto/atmel-sha.c
index a0d42bdc311f..e8e4200c1ab3 100644
--- a/drivers/crypto/atmel-sha.c
+++ b/drivers/crypto/atmel-sha.c
@@ -2485,11 +2485,8 @@ static int atmel_sha_dma_init(struct atmel_sha_dev *dd)
 {
 	dd->dma_lch_in.chan = dma_request_chan(dd->dev, "tx");
 	if (IS_ERR(dd->dma_lch_in.chan)) {
-		int ret = PTR_ERR(dd->dma_lch_in.chan);
-
-		if (ret != -EPROBE_DEFER)
-			dev_warn(dd->dev, "no DMA channel available\n");
-		return ret;
+		dev_err(dd->dev, "DMA channel is not available\n");
+		return PTR_ERR(dd->dma_lch_in.chan);
 	}
 
 	dd->dma_lch_in.dma_conf.dst_addr = dd->phys_base +
diff --git a/drivers/crypto/atmel-tdes.c b/drivers/crypto/atmel-tdes.c
index d42b22775ee9..83a6d42c8921 100644
--- a/drivers/crypto/atmel-tdes.c
+++ b/drivers/crypto/atmel-tdes.c
@@ -783,8 +783,7 @@ static int atmel_tdes_dma_init(struct atmel_tdes_dev *dd)
 err_dma_out:
 	dma_release_channel(dd->dma_lch_in.chan);
 err_dma_in:
-	if (ret != -EPROBE_DEFER)
-		dev_warn(dd->dev, "no DMA channel available\n");
+	dev_err(dd->dev, "no DMA channel available\n");
 	return ret;
 }
 
-- 
2.20.1

[PATCH 5/5] crypto: atmel-{aes,tdes} - Update the IV only when the op succeeds

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

Do not update the IV in case of errors.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/crypto/atmel-aes.c  | 2 +-
 drivers/crypto/atmel-tdes.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
index 855f0ccc9368..245d45f93b61 100644
--- a/drivers/crypto/atmel-aes.c
+++ b/drivers/crypto/atmel-aes.c
@@ -546,7 +546,7 @@ static inline int atmel_aes_complete(struct atmel_aes_dev *dd, int err)
 	clk_disable(dd->iclk);
 	dd->flags &= ~AES_FLAGS_BUSY;
 
-	if (!dd->ctx->is_aead &&
+	if (!err && !dd->ctx->is_aead &&
 	    (rctx->mode & AES_FLAGS_OPMODE_MASK) != AES_FLAGS_ECB) {
 		if ((rctx->mode & AES_FLAGS_OPMODE_MASK) != AES_FLAGS_CTR)
 			atmel_aes_set_iv_as_last_ciphertext_block(dd);
diff --git a/drivers/crypto/atmel-tdes.c b/drivers/crypto/atmel-tdes.c
index 83a6d42c8921..e7cd7b01b931 100644
--- a/drivers/crypto/atmel-tdes.c
+++ b/drivers/crypto/atmel-tdes.c
@@ -597,7 +597,7 @@ static void atmel_tdes_finish_req(struct atmel_tdes_dev *dd, int err)
 
 	dd->flags &= ~TDES_FLAGS_BUSY;
 
-	if ((rctx->mode & TDES_FLAGS_OPMODE_MASK) != TDES_FLAGS_ECB)
+	if (!err && (rctx->mode & TDES_FLAGS_OPMODE_MASK) != TDES_FLAGS_ECB)
 		atmel_tdes_set_iv_as_last_ciphertext_block(dd);
 
 	req->base.complete(&req->base, err);
-- 
2.20.1

Re: [PATCH 1/5] crypto: atmel-{aes,sha} - Fix incorrect use of dmaengine_terminate_all()

[Herbert Xu]

On Fri, Dec 13, 2019 at 09:54:42AM +0000, Tudor.Ambarus@microchip.com wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> device_terminate_all() is used to abort all the pending and
> ongoing transfers on the channel, it should be used just in the
> error path.
> 
> Also, dmaengine_terminate_all() is deprecated and one should use
> dmaengine_terminate_async() or dmaengine_terminate_sync(). The method
> is not used in atomic context, use dmaengine_terminate_sync().
> 
> A secondary aspect of this patch is that it luckily avoids a deadlock
> between atmel_aes and at_hdmac.c. While in tasklet with the lock held,
> the dma controller invokes the client callback (dmaengine_terminate_all),
> which tries to get the same lock. The at_hdmac fix would be to drop the
> lock before invoking the client callback, a fix on at_hdmac will follow.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
>  drivers/crypto/atmel-aes.c | 32 ++------------------------------
>  drivers/crypto/atmel-sha.c |  1 -
>  2 files changed, 2 insertions(+), 31 deletions(-)

All applied.  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