Re: [PATCH 04/14] dmaengine: dma350: Check dma_cookie_status() ret code and txstate

[Robin Murphy <robin.murphy@arm.com>]

On 2025-08-23 4:39 pm, Jisheng Zhang wrote:
> If dma_cookie_status() returns DMA_COMPLETE, we can return immediately.
> 
>  From another side, the txstate is an optional parameter used to get a
> struct with auxilary transfer status information. When not provided
> the call to device_tx_status() should return the status of the dma
> cookie. Return the status of dma cookie when the txstate optional
> parameter is not provided.

Again, the current code was definitely intentional - I think this was 
down to the hardware error case, where for reasons I now can't remember 
I still had to nominally complete the aborted descriptor from the IRQ 
handler to avoid causing some worse problem, and hence we don't return 
early without cross-checking dch->status here, because returning 
DMA_COMPLETE when the descriptor hasn't done its job makes dmatest unhappy.

I did spend a *lot* of time exercising all the error cases, and trying 
to get a sensible result across all of the different reporting APIs was 
fiddly to say the least - there's a huge lack of consistency between 
drivers in this regard, and this was just my attempt to be the 
least-worst one :)

Thanks,
Robin.

> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
>   drivers/dma/arm-dma350.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/dma/arm-dma350.c b/drivers/dma/arm-dma350.c
> index 96350d15ed85..17af9bb2a18f 100644
> --- a/drivers/dma/arm-dma350.c
> +++ b/drivers/dma/arm-dma350.c
> @@ -377,6 +377,8 @@ static enum dma_status d350_tx_status(struct dma_chan *chan, dma_cookie_t cookie
>   	u32 residue = 0;
>   
>   	status = dma_cookie_status(chan, cookie, state);
> +	if (status == DMA_COMPLETE || !state)
> +		return status;
>   
>   	spin_lock_irqsave(&dch->vc.lock, flags);
>   	if (cookie == dch->cookie) {