Re: [PATCH v6 16/18] ASoC: renesas: rz-ssi: Add pause support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The pause/resume implementation terminates the DMA channel and recalculates pointers to period boundaries, violating ALSA's lossless pause semantics and causing permanent audio data loss.
- [High] In full-duplex mode, PAUSE_PUSH fails to pause the hardware or DMA, leading to hardware starvation, auto-restart loops, and DMA queue explosion on PAUSE_RELEASE.
--

commit 7d75d5203513e23a4244ea8798db8bc41af3f70c
Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

ASoC: renesas: rz-ssi: Add pause support

This commit adds ALSA pause support (PAUSE_PUSH and PAUSE_RELEASE) to the
rz-ssi driver as a preparatory step for switching to PCM dmaengine APIs.

> diff --git a/sound/soc/renesas/rz-ssi.c b/sound/soc/renesas/rz-ssi.c
> index 71e434cfe07b9..d4e1dded3a9c1 100644
> --- a/sound/soc/renesas/rz-ssi.c
> +++ b/sound/soc/renesas/rz-ssi.c
[ ... ]
> @@ -888,6 +889,7 @@ static int rz_ssi_dai_trigger(struct snd_pcm_substream *substream, int cmd,
>  		break;
>  
>  	case SNDRV_PCM_TRIGGER_SUSPEND:
> +	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
>  		rz_ssi_stop(ssi, strm);
>  		break;

[Severity: High]
When PAUSE_PUSH is issued in half-duplex mode, rz_ssi_stop() calls
dmaengine_terminate_async(), which discards all in-flight DMA progress.

On PAUSE_RELEASE, rz_ssi_trigger_resume() recalculates the DMA pointer:

static int rz_ssi_trigger_resume(struct rz_ssi_priv *ssi,
                                 struct rz_ssi_stream *strm)
{
        ...
        strm->dma_buffer_pos = strm->completed_dma_buf_pos +
                               runtime->period_size;
        ...
}

Does this effectively snap to the next period boundary and permanently skip
the partially transferred period, violating ALSA's lossless pause semantics?

Additionally, calling dmaengine_terminate_async() without
dmaengine_synchronize() before submitting new descriptors can expose race
conditions. I notice this is fixed later in the series in commit
7b32dcd4a1c5c ("ASoC: renesas: rz-ssi: Use generic PCM dmaengine APIs").

[Severity: High]
During full-duplex operation, if PAUSE_PUSH is issued to one stream while
the other runs, rz_ssi_stop() returns early without disabling the hardware
or terminating the DMA channel:

static int rz_ssi_stop(struct rz_ssi_priv *ssi, struct rz_ssi_stream *strm)
{
        strm->running = 0;

        if (rz_ssi_is_stream_running(&ssi->playback) ||
            rz_ssi_is_stream_running(&ssi->capture))
                return 0;
        ...
}

Because the stream is marked as stopped but the hardware keeps running,
the DMA callbacks will stop queuing new descriptors. Will this starve the
hardware FIFO and trigger an under/overrun interrupt?

If the interrupt handler rz_ssi_interrupt() detects the error, will it
force the stream back into a running state via rz_ssi_start() because
strm->substream is not cleared during a pause?

When PAUSE_RELEASE is subsequently issued, the driver falls through to
SNDRV_PCM_TRIGGER_START and queues 4 additional descriptors onto the
already-running DMA channel. Can this corrupt the state and risk queue
exhaustion?

I see this is also fixed later in the series in commit 7b32dcd4a1c5c
("ASoC: renesas: rz-ssi: Use generic PCM dmaengine APIs").

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260526084710.3491480-1-claudiu.beznea@kernel.org?part=16