Re: [PATCH v2 08/27] ASoC: soc-generic-dmaengine-pcm: use snd_soc_component_unregister()

["Alvin Šipraga" <alvin.sipraga@analog.com>]

Hi Kuninori,

On Thu, Nov 12, 2026 at 12:32:23AM +0000, Kuninori Morimoto wrote:
> Current snd_dmaengine_pcm_unregister() (X) is using
> snd_soc_unregister_component_by_driver() (Y) to unregister component.
> 
> (X)	void snd_dmaengine_pcm_unregister(...)
> 	{
> (Z)		component = snd_soc_lookup_component(...);
> 		...

Sorry - I was a bit too quick to review this. I think there is another
issue.

(Z) expanded is:

	component = snd_soc_lookup_component(dev, SND_DMAENGINE_PCM_DRV_NAME);

which will walk the global component_list until it finds a component
matching the dev and SND_DMAENGINE_PCM_DRV_NAME.

If there's more than one snd_dmaengine_pcm component registered on the
same dev, how do we know that this lookup will pick the correct one?

It looks like the code was written with the assumption that there can
only be one snd_dmaengine_pcm component per dev. But I didn't look at
the history.

Does such a case exist? If not, maybe the easier solution (for now) is
to check if one has already been registered before registering another
one:

int snd_dmaengine_pcm_register(dev, config, flags)
{
	component = snd_soc_lookup_component(dev, SND_DMAENGINE_PCM_DRV_NAME);
	if (component)
		return -EBUSY;

	...
}

Then you don't have to worry about the 2x unregister problem sashiko
pointed out.

In general there are not many users of this lookup feature. Besides
snd_dmaengine_pcm there's just mtk-soundcard-driver.c and
fsl_asrc_dma.c. Both look a bit weird too. So maybe there's a way to get
rid of it altogether...

Kind regards,
Alvin

> (p)		pcm = snd_soc_component_to_priv(component);
> 
> (Y)		snd_soc_unregister_component_by_driver(...);
> (p)		dmaengine_pcm_release_chan(pcm);
> (p)		kfree(pcm);
> 	}
> 
> But it try to unregister all components from dev (a).
> 
> (Y)	void snd_soc_unregister_component(...)
> 	{
> 		...
> 		while (1) {
> (Z)			component = snd_soc_lookup_component_nolocked(...);
> 			...
> (a)			snd_soc_component_unregister(component);
> 		}
> 	}
> 
> snd_dmaengine_pcm_unregister() (X) need to care/free pcm which is connected
> to component (= p). Both (X) and (Y) are using lookup_component (Z).
> So (X) can handle first component/pcm (p), but can't handle 2nd or more
> component/pcm eventhough (X) was called multiple times, because (Y)
> unregister all components. Let's call snd_soc_component_unregister() (a)
> instead of snd_soc_unregister_component_by_driver() (Y) in (X), and
> unregister target component only, not all.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/r/sashiko-outbox-206296@kernel.org
> ---
>  sound/soc/soc-generic-dmaengine-pcm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c
> index 1ecf3feb2bed0..62f86d97668ef 100644
> --- a/sound/soc/soc-generic-dmaengine-pcm.c
> +++ b/sound/soc/soc-generic-dmaengine-pcm.c
> @@ -514,7 +514,7 @@ void snd_dmaengine_pcm_unregister(struct device *dev)
>  
>  	pcm = snd_soc_component_to_priv(component);
>  
> -	snd_soc_unregister_component_by_driver(dev, component->driver);
> +	snd_soc_component_unregister(component);
>  	dmaengine_pcm_release_chan(pcm);
>  	kfree(pcm);
>  }
> -- 
> 2.53.0
>