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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Calling snd_soc_component_write() inside the ALSA PCM trigger callback causes a 'scheduling while atomic' BUG.
- [High] The fast domain soft reset logic uses an incorrect execution sequence that contradicts the commit message, risking audio glitches.
--

commit 75a2cc70a354fd1f43acb5fb951912b82d12eca6
Author: Christian Hewitt <christianshewitt@gmail.com>
Date:   Sat Jun 27 12:50:30 2026 +0000

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

This commit introduces a soft reset of the S/PDIF fast domain on start and
stop to prevent a buffer underrun "machine gun noise" issue. It adds calls
to reset the domain before enabling the DCU on start, and after disabling
it on stop.

> The vendor driver resets the IEC958 fast domain (AIU_RST_SOFT bit 2) on
> both enable and disable (audio_hw_958_enable), and when reconfiguring
> (audio_hw_958_reset clears AIU_958_DCU_FF_CTRL then resets). Do the same:
> reset before enabling the DCU on start, and after disabling it on stop.

[Severity: High]
Is the sequence of operations implemented in the stop path inverted compared
to this commit message?

The commit message states the reset should happen after disabling the DCU
on stop, but the code executes the reset before disabling the DCU data flow.

> 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 cause a 'scheduling while atomic' bug?

The ALSA PCM trigger callback runs in an atomic context with spinlocks held
and interrupts disabled, but snd_soc_component_write() unconditionally acquires
a sleeping lock:

snd_soc_component_write() {
    ...
    mutex_lock(&component->io_mutex);
    ret = soc_component_write_no_lock(component, reg, val);
    ...
}

Also, does the soft reset on the start path happen too late?

Earlier in fifo_spdif_trigger(), aiu_fifo_trigger() is called to start the
main upstream FIFO before this soft reset is executed. Will starting the FIFO
before the reset result in dropped initial audio samples?

>  		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);
>  		fifo_spdif_dcu_enable(component, false);

[Severity: High]
Will resetting the fast domain while the DCU is still actively fetching data
force the DCU to read from an empty FIFO and trigger an underrun?

As noted in the commit message review above, the soft reset is executed here
before fifo_spdif_dcu_enable(component, false) disables the DCU data flow.

>  		break;
>  	default:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260627125030.808058-1-christianshewitt@gmail.com?part=1

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic