Re: [PATCH] dmaengine: imx-sdma: Setting DMA_PRIVATE capability during the probe

From: Sascha Hauer <s.hauer@pengutronix.de>
To: Hui Wang <hui.wang@canonical.com>
Cc: dmaengine@vger.kernel.org, vkoul@kernel.org, shawnguo@kernel.org,
	yibin.gong@nxp.com
Subject: Re: [PATCH] dmaengine: imx-sdma: Setting DMA_PRIVATE capability during the probe
Date: Mon, 20 Jun 2022 10:33:10 +0200	[thread overview]
Message-ID: <20220620083310.GV2387@pengutronix.de> (raw)
In-Reply-To: <20220524074933.38413-1-hui.wang@canonical.com>

On Tue, May 24, 2022 at 03:49:33PM +0800, Hui Wang wrote:
> We have an imx6sx EVB, the audio driver fails to get a valid dma chan
> and the audio can't work at all on this board, below is the error log:
>  fsl-ssi-dai 202c000.ssi: Missing dma channel for stream: 0
>  202c000.ssi-nau8822-hifi: ASoC: pcm constructor failed: -22
>  asoc-simple-card sound: ASoC: can't create pcm 202c000.ssi-nau8822-hifi :-22
> 
> Then I checked the usage_count of each dma chan through sysfs, all
> channels are occupied as below:
> ubuntu@ubuntu:cd /sys/devices/platform/soc/2000000.bus/20ec000.sdma/dma
> ubuntu@ubuntu:find . -iname in_use | xargs cat
> 2
> 2
> 2
> ...
> 
> Through debugging, we found the root cause, the
> crypo/async_tx/async_tx.c calls the dmaengine_get() ahead of
> registration of dma_device from imx-sdma.c. In the dmaengine_get(), the
> dmaengine_ref_count will be increased, then in the
> dma_async_device_register(), the client_count of each chan will be
> increased.
> 
> To fix this issue, we could set DMA_PRIVATE to the dma_deivce before
> registration.
> 
> Signed-off-by: Hui Wang <hui.wang@canonical.com>
> ---
>  drivers/dma/imx-sdma.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index 95367a8a81a5..aabe8a8069fb 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -2201,6 +2201,7 @@ static int sdma_probe(struct platform_device *pdev)
>  	for (i = 0; i < sizeof(*sdma->script_addrs) / sizeof(s32); i++)
>  		saddr_arr[i] = -EINVAL;
>  
> +	dma_cap_set(DMA_PRIVATE, sdma->dma_device.cap_mask);

I am not sure about the impacts on the memcpy capability of the SDMA
driver when setting this flag. It looks like this flag influences the
way suitable channels are picked for memcpy, but I don't understand
the code just by looking at it. I see that several other drivers
providing memcpy set this flag as well, so I guess it's ok to set it,
but it would be good to hear a word from Vinod about it.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |