Re: [PATCH v3] ASoC: meson: aiu: fifo-spdif: soft reset the S/PDIF datapath on start/stop

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Calling `snd_soc_component_write()` in the ALSA PCM `trigger` callback attempts to acquire a sleeping mutex in an atomic context.
--

commit 1376cbf94327e951ccd270ef5b1029d5db3c5d0f
Author: Christian Hewitt <[email protected]>

ASoC: meson: aiu: fifo-spdif: soft reset the S/PDIF datapath on start/stop

This commit soft-resets the spdif datapath on start and stop to fix "machine 
gun noise" buffer underruns. It mirrors the downstream vendor driver's 
behavior by toggling the reset bits.

> diff --git a/sound/soc/meson/aiu-fifo-spdif.c b/sound/soc/meson/aiu-fifo-spdif.c
> index e0e00ec026dcc..a9861c5d66374 100644
> --- a/sound/soc/meson/aiu-fifo-spdif.c
> +++ b/sound/soc/meson/aiu-fifo-spdif.c

[ ... ]

> @@ -68,11 +69,15 @@ static int fifo_spdif_trigger(struct snd_pcm_substream *substream, int cmd,
>  	case SNDRV_PCM_TRIGGER_START:
>  	case SNDRV_PCM_TRIGGER_RESUME:
>  	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> +		snd_soc_component_write(component, AIU_RST_SOFT,
> +					AIU_RST_SOFT_958_FAST);

[Severity: High]
Does this code attempt to sleep in an atomic context?

The audio core executes the trigger callback atomically with
snd_pcm_stream_lock_irq() (unless the non-atomic flag is set by the machine
driver, which is not the case for gx-card.c).

However, snd_soc_component_write() unconditionally attempts to acquire a
sleeping mutex:

sound/soc/soc-component.c:snd_soc_component_write() {
    ...
    mutex_lock(&component->io_mutex);
    ...
}

Will this lead to a scheduling while atomic splat when starting the stream?

>  		fifo_spdif_dcu_enable(component, true);
>  		break;
>  	case SNDRV_PCM_TRIGGER_SUSPEND:
>  	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
>  	case SNDRV_PCM_TRIGGER_STOP:
> +		snd_soc_component_write(component, AIU_RST_SOFT,
> +					AIU_RST_SOFT_958_FAST);

[Severity: High]
Can this write also trigger a sleeping in atomic context regression when
stopping the device?

>  		fifo_spdif_dcu_enable(component, false);
>  		break;

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

_______________________________________________
linux-amlogic mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-amlogic