Re: [PATCH 5/9] crypto: qce - convert qce_dma_request() to use devres

[neil.armstrong@linaro.org]

On 03/12/2024 10:19, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Make qce_dma_request() into a managed interface. With this we can
> simplify the error path in probe() and drop another operations from
> remove().
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>   drivers/crypto/qce/core.c | 16 +++-------------
>   drivers/crypto/qce/dma.c  | 22 +++++++++++++---------
>   drivers/crypto/qce/dma.h  |  3 +--
>   3 files changed, 17 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
> index cdcddf8f9f02b..e2cda24960f63 100644
> --- a/drivers/crypto/qce/core.c
> +++ b/drivers/crypto/qce/core.c
> @@ -232,13 +232,13 @@ static int qce_crypto_probe(struct platform_device *pdev)
>   	if (ret)
>   		return ret;
>   
> -	ret = qce_dma_request(qce->dev, &qce->dma);
> +	ret = devm_qce_dma_request(qce->dev, &qce->dma);
>   	if (ret)
>   		return ret;
>   
>   	ret = qce_check_version(qce);
>   	if (ret)
> -		goto err_dma;
> +		return ret;
>   
>   	spin_lock_init(&qce->lock);
>   	tasklet_init(&qce->done_tasklet, qce_tasklet_req_done,
> @@ -248,16 +248,7 @@ static int qce_crypto_probe(struct platform_device *pdev)
>   	qce->async_req_enqueue = qce_async_request_enqueue;
>   	qce->async_req_done = qce_async_request_done;
>   
> -	ret = qce_register_algs(qce);
> -	if (ret)
> -		goto err_dma;
> -
> -	return 0;
> -
> -err_dma:
> -	qce_dma_release(&qce->dma);
> -
> -	return ret;
> +	return qce_register_algs(qce);
>   }
>   
>   static void qce_crypto_remove(struct platform_device *pdev)
> @@ -266,7 +257,6 @@ static void qce_crypto_remove(struct platform_device *pdev)
>   
>   	tasklet_kill(&qce->done_tasklet);
>   	qce_unregister_algs(qce);
> -	qce_dma_release(&qce->dma);
>   }
>   
>   static const struct of_device_id qce_crypto_of_match[] = {
> diff --git a/drivers/crypto/qce/dma.c b/drivers/crypto/qce/dma.c
> index 46db5bf366b44..1dec7aea852dd 100644
> --- a/drivers/crypto/qce/dma.c
> +++ b/drivers/crypto/qce/dma.c
> @@ -3,12 +3,22 @@
>    * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved.
>    */
>   
> +#include <linux/device.h>
>   #include <linux/dmaengine.h>
>   #include <crypto/scatterwalk.h>
>   
>   #include "dma.h"
>   
> -int qce_dma_request(struct device *dev, struct qce_dma_data *dma)
> +static void qce_dma_release(void *data)
> +{
> +	struct qce_dma_data *dma = data;
> +
> +	dma_release_channel(dma->txchan);
> +	dma_release_channel(dma->rxchan);
> +	kfree(dma->result_buf);
> +}
> +
> +int devm_qce_dma_request(struct device *dev, struct qce_dma_data *dma)
>   {
>   	int ret;
>   
> @@ -31,7 +41,8 @@ int qce_dma_request(struct device *dev, struct qce_dma_data *dma)
>   
>   	dma->ignore_buf = dma->result_buf + QCE_RESULT_BUF_SZ;
>   
> -	return 0;
> +	return devm_add_action_or_reset(dev, qce_dma_release, dma);
> +
>   error_nomem:
>   	dma_release_channel(dma->rxchan);
>   error_rx:
> @@ -39,13 +50,6 @@ int qce_dma_request(struct device *dev, struct qce_dma_data *dma)
>   	return ret;
>   }
>   
> -void qce_dma_release(struct qce_dma_data *dma)
> -{
> -	dma_release_channel(dma->txchan);
> -	dma_release_channel(dma->rxchan);
> -	kfree(dma->result_buf);
> -}
> -
>   struct scatterlist *
>   qce_sgtable_add(struct sg_table *sgt, struct scatterlist *new_sgl,
>   		unsigned int max_len)
> diff --git a/drivers/crypto/qce/dma.h b/drivers/crypto/qce/dma.h
> index 7864021693608..31629185000e1 100644
> --- a/drivers/crypto/qce/dma.h
> +++ b/drivers/crypto/qce/dma.h
> @@ -34,8 +34,7 @@ struct qce_dma_data {
>   	void *ignore_buf;
>   };
>   
> -int qce_dma_request(struct device *dev, struct qce_dma_data *dma);
> -void qce_dma_release(struct qce_dma_data *dma);
> +int devm_qce_dma_request(struct device *dev, struct qce_dma_data *dma);
>   int qce_dma_prep_sgs(struct qce_dma_data *dma, struct scatterlist *sg_in,
>   		     int in_ents, struct scatterlist *sg_out, int out_ents,
>   		     dma_async_tx_callback cb, void *cb_param);
> 

Nice rework :-)

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>