Re: [PATCH 1/1] spi: use dmaengine_desc_set_callback()

[Thierry Reding]

On Mon, Nov 02, 2026 at 12:39:58PM -0500, Frank.Li@xxxxxxxxxxx wrote:
[...]
> diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
> index 087735ed9c80c..b115daf2c76f3 100644
> --- a/drivers/spi/spi-davinci.c
> +++ b/drivers/spi/spi-davinci.c
> @@ -703,10 +703,8 @@ static int davinci_spi_bufs(struct spi_device *spi, struct spi_transfer *t)
>  		if (!txdesc)
>  			goto err_desc;
>  
> -		rxdesc->callback = davinci_spi_dma_rx_callback;
> -		rxdesc->callback_param = (void *)dspi;
> -		txdesc->callback = davinci_spi_dma_tx_callback;
> -		txdesc->callback_param = (void *)dspi;
> +		dmaengine_desc_set_callback(rxdesc, davinci_spi_dma_rx_callback, NULL, (void *)dspi);
> +		dmaengine_desc_set_callback(txdesc, davinci_spi_dma_tx_callback, NULL, (void *)dspi);

You could probably drop the cast while at it.

> diff --git a/drivers/spi/spi-fsl-lpspi.c b/drivers/spi/spi-fsl-lpspi.c
> index b25f94b2ef738..7798197d5b350 100644
> --- a/drivers/spi/spi-fsl-lpspi.c
> +++ b/drivers/spi/spi-fsl-lpspi.c
> @@ -634,8 +634,7 @@ static int fsl_lpspi_dma_transfer(struct spi_controller *controller,
>  	if (!desc_rx)
>  		return -EINVAL;
>  
> -	desc_rx->callback = fsl_lpspi_dma_rx_callback;
> -	desc_rx->callback_param = (void *)fsl_lpspi;
> +	dmaengine_desc_set_callback(desc_rx, fsl_lpspi_dma_rx_callback, NULL, (void *)fsl_lpspi);

Same here.

>  	dmaengine_submit(desc_rx);
>  	reinit_completion(&fsl_lpspi->dma_rx_completion);
>  	dma_async_issue_pending(controller->dma_rx);
> @@ -648,8 +647,7 @@ static int fsl_lpspi_dma_transfer(struct spi_controller *controller,
>  		return -EINVAL;
>  	}
>  
> -	desc_tx->callback = fsl_lpspi_dma_tx_callback;
> -	desc_tx->callback_param = (void *)fsl_lpspi;
> +	dmaengine_desc_set_callback(desc_tx, fsl_lpspi_dma_tx_callback, NULL, (void *)fsl_lpspi);

And here.

> diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
> index cadf0b93f4403..f8a3ac11f49d0 100644
> --- a/drivers/spi/spi-imx.c
> +++ b/drivers/spi/spi-imx.c
> @@ -1738,8 +1738,7 @@ static int spi_imx_dma_submit(struct spi_imx_data *spi_imx,
>  		return -EINVAL;
>  	}
>  
> -	desc_rx->callback = spi_imx_dma_rx_callback;
> -	desc_rx->callback_param = (void *)spi_imx;
> +	dmaengine_desc_set_callback(desc_rx, spi_imx_dma_rx_callback, NULL, (void *)spi_imx);

Here, too.

>  	cookie = dmaengine_submit(desc_rx);
>  	if (dma_submit_error(cookie)) {
>  		dev_err(spi_imx->dev, "submitting DMA RX failed\n");
> @@ -1756,8 +1755,7 @@ static int spi_imx_dma_submit(struct spi_imx_data *spi_imx,
>  	if (!desc_tx)
>  		goto dmaengine_terminate_rx;
>  
> -	desc_tx->callback = spi_imx_dma_tx_callback;
> -	desc_tx->callback_param = (void *)spi_imx;
> +	dmaengine_desc_set_callback(desc_tx, spi_imx_dma_tx_callback, NULL, (void *)spi_imx);

And here.

> index eb3485149e75a..fdda26eb311fb 100644
> --- a/drivers/spi/spi-topcliff-pch.c
> +++ b/drivers/spi/spi-topcliff-pch.c
> @@ -1021,8 +1021,7 @@ static void pch_spi_handle_dma(struct pch_spi_data *data, int *bpw)
>  		return;
>  	}
>  	dma_sync_sg_for_device(&data->host->dev, sg, num, DMA_FROM_DEVICE);
> -	desc_rx->callback = pch_dma_rx_complete;
> -	desc_rx->callback_param = data;
> +	dmaengine_desc_set_callback(desc_rx, pch_dma_rx_complete, NULL, data);
>  	dma->nent = num;
>  	dma->desc_rx = desc_rx;
>  
> @@ -1083,8 +1082,7 @@ static void pch_spi_handle_dma(struct pch_spi_data *data, int *bpw)
>  		return;
>  	}
>  	dma_sync_sg_for_device(&data->host->dev, sg, num, DMA_TO_DEVICE);
> -	desc_tx->callback = NULL;
> -	desc_tx->callback_param = data;
> +	dmaengine_desc_set_callback(desc_tx, NULL, NULL, data);

This is a preexisting issue, so nothing to do with your patch, but I'm
not sure I understand the purpose of setting callback_param to != NULL
when callback == NULL. If neither callback nor callback_result are non-
NULL, then callback_params seems to be completely unused.

Anyway, none of the above are blockers, so:

Reviewed-by: Thierry Reding <treding@xxxxxxxxxx>

Attachment: signature.asc
Description: PGP signature