From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EA5323D9686 for ; Tue, 26 May 2026 09:46:49 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779788812; cv=none; b=VR/UyGZuRdY4jZv4erYo28PVxq3RfXBfOg7mlNJjwBQPQNcIN23EV0JD3dwm7N8hpBuu3FDoNCT5UZpEQqp6uApCNcEyO80BHRUgh4/A3GWRh99dW4JG2ZEwt7FJFsvgvlZW2ft9bMUB/p3QPijaAoPhRti+Wiyv3r45qTeOc6g= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779788812; c=relaxed/simple; bh=Z5n1EokKj2tXthuqQvGZ5Cd7p7iKhLgR2g/CuaqYyS8=; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date: Message-Id; b=DpSyQ2dLvzNyhMScybVHg+5xi9PKbG8KIVZECxv/Gai/UE1hM/q4baN8Q7I38NiAI08TbhWZoDRXeQMjyySGbjMoVm7VZTysGI3BODsHsTsZX5ZUN6UYehN9xt3uWg81xUyPgL1+8Ho1+czrFMW6P5Rv90gPEJYUOHLKwCf6kzU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=nFS/NXZv; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="nFS/NXZv" Received: by smtp.kernel.org (Postfix) with ESMTPSA id DAE291F00A3C; Tue, 26 May 2026 09:46:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1779788809; bh=hbxPkevKMRcERFGS3Qcq0aqZ7jT3TmetwosgPvBnU0s=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date; b=nFS/NXZvEMznrlZRBYwrBzYFe9/eK1YKFbdc8Nl9+kU6L/PzB3mhidL0Ou0JWiHMr gMZymRMRwPslIQPL03x5nXJPQn6ptLLjZ1Dl8d2oFCW9kMT5rF2GL2a1sv/hcD6zCW j/TnSB/8KHy25u3IvUNMAKTjbdurlV8CVbhbIsfWjVONLpOkMGapd2i2gZgx6L0TMF kHjknXeYnYsgB9r9fVn1mmVOFyg0J9gqlle/P4Thq/OAYeTTQ6/2W0Gfjq7XB1QZ7U 3SjKkzBXyw1aooJITV4JtIj5I6l8xTckQ3YKUlgwc4LGZYP+CMwYEkuXOCK2m2JFc9 vM0mBUo0sE6cw== From: sashiko-bot@kernel.org Subject: Re: [PATCH v6 16/18] ASoC: renesas: rz-ssi: Add pause support Reply-To: sashiko-reviews@lists.linux.dev To: "Claudiu Beznea" Cc: wsa+renesas@sang-engineering.com, vkoul@kernel.org, dmaengine@vger.kernel.org, Frank.Li@kernel.org In-Reply-To: <20260526084710.3491480-17-claudiu.beznea@kernel.org> References: <20260526084710.3491480-17-claudiu.beznea@kernel.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Tue, 26 May 2026 09:46:48 +0000 Message-Id: <20260526094648.DAE291F00A3C@smtp.kernel.org> Precedence: bulk X-Mailing-List: dmaengine@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: 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 rec= alculates pointers to period boundaries, violating ALSA's lossless pause se= mantics 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 explosi= on on PAUSE_RELEASE. -- commit 7d75d5203513e23a4244ea8798db8bc41af3f70c Author: Claudiu Beznea 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_substrea= m *substream, int cmd, > break; > =20 > 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 =3D 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 =3D 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"). --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260526084710.3491= 480-1-claudiu.beznea@kernel.org?part=3D16