Re: [PATCH v4 4/5] ASoC: fsl_asrc: expose individual DAIs per conversion path

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] ASoC component is unregistered via devres after hardware clocks are disabled in remove(), leading to system hangs if an active stream is terminated during driver unbind.
--

--- Patch [4]: [PATCH v4 4/5] ASoC: fsl_asrc: expose individual DAIs per conversion path ---
commit c1765b1c3c3716b7c2b1c94ee1d7914bfa585bff
Author: Shengjiu Wang <[email protected]>

ASoC: fsl_asrc: expose individual DAIs per conversion path

This commit replaces the single fsl_asrc_dai instance with an array of
three DAI drivers to allow multiple independent conversion paths to be
used simultaneously. It updates the probe function to register all three
DAIs.

> diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
> index 2755ee57c6a17..a5d6c4c1e9292 100644
> --- a/sound/soc/fsl/fsl_asrc.c
> +++ b/sound/soc/fsl/fsl_asrc.c

[ ... ]

> @@ -1393,7 +1442,7 @@ static int fsl_asrc_probe(struct platform_device *pdev)
>  		goto err_pm_get_sync;
>  
>  	ret = devm_snd_soc_register_component(&pdev->dev, &fsl_asrc_component,
> -					      &fsl_asrc_dai, 1);
> +					      fsl_asrc_dai, ARRAY_SIZE(fsl_asrc_dai));

[Severity: High]
This is a pre-existing issue, but does using devres to register the ASoC
component cause a race condition during driver unbind?

Since the component is registered with devm_snd_soc_register_component(),
it won't be unregistered until the devres release phase, which happens
after fsl_asrc_remove() finishes.

If an ALSA stream is active during unbind, fsl_asrc_remove() gates the
ASRC IPG and MEM clocks:

fsl_asrc_remove() {
	...
	pm_runtime_disable(&pdev->dev);
	if (!pm_runtime_status_suspended(&pdev->dev))
		fsl_asrc_runtime_suspend(&pdev->dev);
}

During the window between fsl_asrc_remove() completing and the devres
release phase unregistering the component, the PCM streams and DMA
channels are still active.

Can the DMA controller attempt to access the clock-gated ASRC FIFOs
during this window, resulting in a synchronous bus fault or asynchronous
external abort?

Would it be safer to use snd_soc_register_component() here instead, and
manually call snd_soc_unregister_component() early in fsl_asrc_remove()
before gating the hardware clocks?

>  	if (ret) {
>  		dev_err(&pdev->dev, "failed to register ASoC DAI\n");
>  		goto err_pm_get_sync;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4