[PATCH 0/7] RZ/G2L SSI Improvements

[PATCH 0/7] RZ/G2L SSI Improvements

[Biju]

From: Biju Das <biju.das.jz@bp.renesas.com>

The first patch in this series fixes audio channel swap issue because of
half duplex to full duplex switching during the streaming that involves
FIFO reset which drops the word leading to audio channel swap issue. The
second patch fixes sample width value in the hw_params_cache::sample_width
Apart from this, support 24 and 32 bit sample format width.

Biju Das (7):
  ASoC: renesas: rz-ssi: Fix channel swap issue in full duplex mode
  ASoC: renesas: rz-ssi: Fix rz_ssi_priv::hw_params_cache::sample_width
  ASoC: renesas: rz-ssi: Use dev variable in probe()
  ASoC: renesas: rz-ssi: Remove trailing comma in the terminator entry
  ASoC: renesas: rz-ssi: Move DMA configuration
  ASoC: renesas: rz-ssi: Add support for 24 bits sample width
  ASoC: renesas: rz-ssi: Add support for 32 bits sample width

 sound/soc/renesas/rz-ssi.c | 193 +++++++++++++++++++++++++------------
 1 file changed, 132 insertions(+), 61 deletions(-)

-- 
2.43.0

[PATCH 1/7] ASoC: renesas: rz-ssi: Fix channel swap issue in full duplex mode

[Biju]

From: Biju Das <biju.das.jz@bp.renesas.com>


The full duplex audio starts with half duplex mode and then switch to
full duplex mode (another FIFO reset) when both playback/capture
streams available leading to random audio left/right channel swap
issue. Fix this channel swap issue by detecting the full duplex
condition by populating struct dup variable in startup() callback
and synchronize starting both the play and capture at the same time
in rz_ssi_start().

Cc: stable@kernel.org
Fixes: 4f8cd05a4305 ("ASoC: sh: rz-ssi: Add full duplex support")
Co-developed-by: Tony Tang <tony.tang.ks@renesas.com>
Signed-off-by: Tony Tang <tony.tang.ks@renesas.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 sound/soc/renesas/rz-ssi.c | 59 +++++++++++++++++++++++++++++++-------
 1 file changed, 49 insertions(+), 10 deletions(-)

diff --git a/sound/soc/renesas/rz-ssi.c b/sound/soc/renesas/rz-ssi.c
index 81b883e8ac92..f21b332b3656 100644
--- a/sound/soc/renesas/rz-ssi.c
+++ b/sound/soc/renesas/rz-ssi.c
@@ -133,6 +133,12 @@ struct rz_ssi_priv {
 	bool bckp_rise;	/* Bit clock polarity (SSICR.BCKP) */
 	bool dma_rt;
 
+	struct {
+		bool tx_active;
+		bool rx_active;
+		bool one_stream_triggered;
+	} dup;
+
 	/* Full duplex communication support */
 	struct {
 		unsigned int rate;
@@ -332,16 +338,17 @@ static int rz_ssi_start(struct rz_ssi_priv *ssi, struct rz_ssi_stream *strm)
 	bool is_full_duplex;
 	u32 ssicr, ssifcr;
 
-	is_full_duplex = rz_ssi_is_stream_running(&ssi->playback) ||
-		rz_ssi_is_stream_running(&ssi->capture);
+	is_full_duplex = ssi->dup.tx_active && ssi->dup.rx_active;
 	ssicr = rz_ssi_reg_readl(ssi, SSICR);
 	ssifcr = rz_ssi_reg_readl(ssi, SSIFCR);
 	if (!is_full_duplex) {
 		ssifcr &= ~0xF;
 	} else {
-		rz_ssi_reg_mask_setl(ssi, SSICR, SSICR_TEN | SSICR_REN, 0);
-		rz_ssi_set_idle(ssi);
-		ssifcr &= ~SSIFCR_FIFO_RST;
+		if (ssi->dup.one_stream_triggered) {
+			rz_ssi_reg_mask_setl(ssi, SSICR, SSICR_TEN | SSICR_REN, 0);
+			rz_ssi_set_idle(ssi);
+			ssifcr &= ~SSIFCR_FIFO_RST;
+		}
 	}
 
 	/* FIFO interrupt thresholds */
@@ -374,12 +381,18 @@ static int rz_ssi_start(struct rz_ssi_priv *ssi, struct rz_ssi_stream *strm)
 			      SSISR_RUIRQ), 0);
 
 	strm->running = 1;
-	if (is_full_duplex)
-		ssicr |= SSICR_TEN | SSICR_REN;
-	else
+	if (is_full_duplex) {
+		if (ssi->dup.one_stream_triggered) {
+			ssicr |= SSICR_TEN | SSICR_REN;
+			rz_ssi_reg_writel(ssi, SSICR, ssicr);
+			ssi->dup.one_stream_triggered = false;
+		} else {
+			ssi->dup.one_stream_triggered = true;
+		}
+	} else {
 		ssicr |= is_play ? SSICR_TEN : SSICR_REN;
-
-	rz_ssi_reg_writel(ssi, SSICR, ssicr);
+		rz_ssi_reg_writel(ssi, SSICR, ssicr);
+	}
 
 	return 0;
 }
@@ -915,6 +928,30 @@ static int rz_ssi_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 	return 0;
 }
 
+static int rz_ssi_startup(struct snd_pcm_substream *substream,
+			  struct snd_soc_dai *dai)
+{
+	struct rz_ssi_priv *ssi = snd_soc_dai_get_drvdata(dai);
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		ssi->dup.tx_active = true;
+	else
+		ssi->dup.rx_active = true;
+
+	return 0;
+}
+
+static void rz_ssi_shutdown(struct snd_pcm_substream *substream,
+			    struct snd_soc_dai *dai)
+{
+	struct rz_ssi_priv *ssi = snd_soc_dai_get_drvdata(dai);
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		ssi->dup.tx_active = false;
+	else
+		ssi->dup.rx_active = false;
+}
+
 static bool rz_ssi_is_valid_hw_params(struct rz_ssi_priv *ssi, unsigned int rate,
 				      unsigned int channels,
 				      unsigned int sample_width,
@@ -985,6 +1022,8 @@ static int rz_ssi_dai_hw_params(struct snd_pcm_substream *substream,
 }
 
 static const struct snd_soc_dai_ops rz_ssi_dai_ops = {
+	.startup	= rz_ssi_startup,
+	.shutdown	= rz_ssi_shutdown,
 	.trigger	= rz_ssi_dai_trigger,
 	.set_fmt	= rz_ssi_dai_set_fmt,
 	.hw_params	= rz_ssi_dai_hw_params,
-- 
2.43.0

[PATCH 2/7] ASoC: renesas: rz-ssi: Fix rz_ssi_priv::hw_params_cache::sample_width

[Biju]

From: Biju Das <biju.das.jz@bp.renesas.com>

The strm->sample_width is not filled during rz_ssi_dai_hw_params(). This
wrong value is used for caching sample_width in struct hw_params_cache.
Fix this issue by replacing 'strm->sample_width'->'params_width(params)'
in rz_ssi_dai_hw_params(). After this drop the variable sample_width
from struct rz_ssi_stream as it is unused.

Cc: stable@kernel.org
Fixes: 4f8cd05a4305 ("ASoC: sh: rz-ssi: Add full duplex support")
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 sound/soc/renesas/rz-ssi.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/sound/soc/renesas/rz-ssi.c b/sound/soc/renesas/rz-ssi.c
index f21b332b3656..039bec32cb5e 100644
--- a/sound/soc/renesas/rz-ssi.c
+++ b/sound/soc/renesas/rz-ssi.c
@@ -13,6 +13,7 @@
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
 #include <linux/reset.h>
+#include <sound/pcm_params.h>
 #include <sound/soc.h>
 
 /* REGISTER OFFSET */
@@ -87,7 +88,6 @@ struct rz_ssi_stream {
 	int dma_buffer_pos;	/* The address for the next DMA descriptor */
 	int completed_dma_buf_pos; /* The address of the last completed DMA descriptor. */
 	int period_counter;	/* for keeping track of periods transferred */
-	int sample_width;
 	int buffer_pos;		/* current frame position in the buffer */
 	int running;		/* 0=stopped, 1=running */
 
@@ -217,10 +217,7 @@ static inline bool rz_ssi_is_stream_running(struct rz_ssi_stream *strm)
 static void rz_ssi_stream_init(struct rz_ssi_stream *strm,
 			       struct snd_pcm_substream *substream)
 {
-	struct snd_pcm_runtime *runtime = substream->runtime;
-
 	rz_ssi_set_substream(strm, substream);
-	strm->sample_width = samples_to_bytes(runtime, 1);
 	strm->dma_buffer_pos = 0;
 	strm->completed_dma_buf_pos = 0;
 	strm->period_counter = 0;
@@ -982,9 +979,9 @@ static int rz_ssi_dai_hw_params(struct snd_pcm_substream *substream,
 				struct snd_soc_dai *dai)
 {
 	struct rz_ssi_priv *ssi = snd_soc_dai_get_drvdata(dai);
-	struct rz_ssi_stream *strm = rz_ssi_stream_get(ssi, substream);
 	unsigned int sample_bits = hw_param_interval(params,
 					SNDRV_PCM_HW_PARAM_SAMPLE_BITS)->min;
+	unsigned int sample_width = params_width(params);
 	unsigned int channels = params_channels(params);
 	unsigned int rate = params_rate(params);
 	int ret;
@@ -1003,16 +1000,14 @@ static int rz_ssi_dai_hw_params(struct snd_pcm_substream *substream,
 
 	if (rz_ssi_is_stream_running(&ssi->playback) ||
 	    rz_ssi_is_stream_running(&ssi->capture)) {
-		if (rz_ssi_is_valid_hw_params(ssi, rate, channels,
-					      strm->sample_width, sample_bits))
+		if (rz_ssi_is_valid_hw_params(ssi, rate, channels, sample_width, sample_bits))
 			return 0;
 
 		dev_err(ssi->dev, "Full duplex needs same HW params\n");
 		return -EINVAL;
 	}
 
-	rz_ssi_cache_hw_params(ssi, rate, channels, strm->sample_width,
-			       sample_bits);
+	rz_ssi_cache_hw_params(ssi, rate, channels, sample_width, sample_bits);
 
 	ret = rz_ssi_swreset(ssi);
 	if (ret)
-- 
2.43.0

[PATCH 3/7] ASoC: renesas: rz-ssi: Use dev variable in probe()

[Biju]

From: Biju Das <biju.das.jz@bp.renesas.com>

Replace '&pdev->dev'->dev in probe(), this makes few error paths shorter.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 sound/soc/renesas/rz-ssi.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/sound/soc/renesas/rz-ssi.c b/sound/soc/renesas/rz-ssi.c
index 039bec32cb5e..c7d5576c8283 100644
--- a/sound/soc/renesas/rz-ssi.c
+++ b/sound/soc/renesas/rz-ssi.c
@@ -1123,19 +1123,16 @@ static int rz_ssi_probe(struct platform_device *pdev)
 
 	audio_clk = devm_clk_get(dev, "audio_clk1");
 	if (IS_ERR(audio_clk))
-		return dev_err_probe(&pdev->dev, PTR_ERR(audio_clk),
-				     "no audio clk1");
+		return dev_err_probe(dev, PTR_ERR(audio_clk), "no audio clk1");
 
 	ssi->audio_clk_1 = clk_get_rate(audio_clk);
 	audio_clk = devm_clk_get(dev, "audio_clk2");
 	if (IS_ERR(audio_clk))
-		return dev_err_probe(&pdev->dev, PTR_ERR(audio_clk),
-				     "no audio clk2");
+		return dev_err_probe(dev, PTR_ERR(audio_clk), "no audio clk2");
 
 	ssi->audio_clk_2 = clk_get_rate(audio_clk);
 	if (!(ssi->audio_clk_1 || ssi->audio_clk_2))
-		return dev_err_probe(&pdev->dev, -EINVAL,
-				     "no audio clk1 or audio clk2");
+		return dev_err_probe(dev, -EINVAL, "no audio clk1 or audio clk2");
 
 	ssi->audio_mck = ssi->audio_clk_1 ? ssi->audio_clk_1 : ssi->audio_clk_2;
 
-- 
2.43.0

[PATCH 4/7] ASoC: renesas: rz-ssi: Remove trailing comma in the terminator entry

[Biju]

From: Biju Das <biju.das.jz@bp.renesas.com>

Remove trailing comma in the terminator entry for OF table. While at it,
add a space between the braces and comment block.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 sound/soc/renesas/rz-ssi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/renesas/rz-ssi.c b/sound/soc/renesas/rz-ssi.c
index c7d5576c8283..5dc510cf66e1 100644
--- a/sound/soc/renesas/rz-ssi.c
+++ b/sound/soc/renesas/rz-ssi.c
@@ -1248,7 +1248,7 @@ static void rz_ssi_remove(struct platform_device *pdev)
 
 static const struct of_device_id rz_ssi_of_match[] = {
 	{ .compatible = "renesas,rz-ssi", },
-	{/* Sentinel */},
+	{ /* Sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, rz_ssi_of_match);
 
-- 
2.43.0

[PATCH 5/7] ASoC: renesas: rz-ssi: Move DMA configuration

[Biju]

From: Biju Das <biju.das.jz@bp.renesas.com>

Move DMA configuration from rz_ssi_dma_request() to rz_ssi_dai_trigger()
for supporting sample widths higher than 16.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 sound/soc/renesas/rz-ssi.c | 29 ++++++++++++-----------------
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/sound/soc/renesas/rz-ssi.c b/sound/soc/renesas/rz-ssi.c
index 5dc510cf66e1..839934e4b76a 100644
--- a/sound/soc/renesas/rz-ssi.c
+++ b/sound/soc/renesas/rz-ssi.c
@@ -778,14 +778,6 @@ static int rz_ssi_dma_request(struct rz_ssi_priv *ssi, struct device *dev)
 	if (!rz_ssi_is_dma_enabled(ssi))
 		goto no_dma;
 
-	if (ssi->playback.dma_ch &&
-	    (rz_ssi_dma_slave_config(ssi, ssi->playback.dma_ch, true) < 0))
-		goto no_dma;
-
-	if (ssi->capture.dma_ch &&
-	    (rz_ssi_dma_slave_config(ssi, ssi->capture.dma_ch, false) < 0))
-		goto no_dma;
-
 	return 0;
 
 no_dma:
@@ -833,24 +825,27 @@ static int rz_ssi_dai_trigger(struct snd_pcm_substream *substream, int cmd,
 		if (cmd == SNDRV_PCM_TRIGGER_START)
 			rz_ssi_stream_init(strm, substream);
 
-		if (ssi->dma_rt) {
-			bool is_playback;
+		if (rz_ssi_is_dma_enabled(ssi)) {
+			bool is_playback = rz_ssi_stream_is_play(substream);
+
+			if (ssi->dma_rt)
+				ret = rz_ssi_dma_slave_config(ssi, ssi->playback.dma_ch,
+							      is_playback);
+			else
+				ret = rz_ssi_dma_slave_config(ssi, strm->dma_ch,
+							      is_playback);
 
-			is_playback = rz_ssi_stream_is_play(substream);
-			ret = rz_ssi_dma_slave_config(ssi, ssi->playback.dma_ch,
-						      is_playback);
 			/* Fallback to pio */
 			if (ret < 0) {
 				ssi->playback.transfer = rz_ssi_pio_send;
 				ssi->capture.transfer = rz_ssi_pio_recv;
 				rz_ssi_release_dma_channels(ssi);
+			} else {
+				/* For DMA, queue up multiple DMA descriptors */
+				num_transfer = 4;
 			}
 		}
 
-		/* For DMA, queue up multiple DMA descriptors */
-		if (rz_ssi_is_dma_enabled(ssi))
-			num_transfer = 4;
-
 		for (i = 0; i < num_transfer; i++) {
 			ret = strm->transfer(ssi, strm);
 			if (ret)
-- 
2.43.0

[PATCH 6/7] ASoC: renesas: rz-ssi: Add support for 24 bits sample width

[Biju]

From: Biju Das <biju.das.jz@bp.renesas.com>

Add support for 24 bits sample format width for RZ/G2L SoCs.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 sound/soc/renesas/rz-ssi.c | 76 +++++++++++++++++++++++++++++---------
 1 file changed, 58 insertions(+), 18 deletions(-)

diff --git a/sound/soc/renesas/rz-ssi.c b/sound/soc/renesas/rz-ssi.c
index 839934e4b76a..5ebaaa440141 100644
--- a/sound/soc/renesas/rz-ssi.c
+++ b/sound/soc/renesas/rz-ssi.c
@@ -38,6 +38,7 @@
 #define SSICR_MST		BIT(14)
 #define SSICR_BCKP		BIT(13)
 #define SSICR_LRCKP		BIT(12)
+#define SSICR_PDTA		BIT(9)
 #define SSICR_CKDV(x)		(((x) & 0xf) << 4)
 #define SSICR_TEN		BIT(1)
 #define SSICR_REN		BIT(0)
@@ -74,7 +75,7 @@
 #define PREALLOC_BUFFER_MAX	(SZ_32K)
 
 #define SSI_RATES		SNDRV_PCM_RATE_8000_48000 /* 8k-48kHz */
-#define SSI_FMTS		SNDRV_PCM_FMTBIT_S16_LE
+#define SSI_FMTS		(SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE)
 #define SSI_CHAN_MIN		2
 #define SSI_CHAN_MAX		2
 #define SSI_FIFO_DEPTH		32
@@ -294,11 +295,25 @@ static int rz_ssi_clk_setup(struct rz_ssi_priv *ssi, unsigned int rate,
 	}
 
 	/*
-	 * DWL: Data Word Length = 16 bits
+	 * DWL: Data Word Length = {16, 24} bits
 	 * SWL: System Word Length = 32 bits
 	 */
 	ssicr |= SSICR_CKDV(clk_ckdv);
-	ssicr |= SSICR_DWL(1) | SSICR_SWL(3);
+	switch (ssi->hw_params_cache.sample_width) {
+	case 16:
+		ssicr |= SSICR_DWL(1);
+		break;
+	case 24:
+		ssicr |= SSICR_PDTA;
+		ssicr |= SSICR_DWL(5);
+		break;
+	default:
+		dev_err(ssi->dev, "Not support %u data width",
+			ssi->hw_params_cache.sample_width);
+		return -EINVAL;
+	}
+
+	ssicr |= SSICR_SWL(3);
 	rz_ssi_reg_writel(ssi, SSICR, ssicr);
 	rz_ssi_reg_writel(ssi, SSIFCR, SSIFCR_AUCKE | SSIFCR_FIFO_RST);
 
@@ -459,7 +474,6 @@ static int rz_ssi_pio_recv(struct rz_ssi_priv *ssi, struct rz_ssi_stream *strm)
 {
 	struct snd_pcm_substream *substream = strm->substream;
 	struct snd_pcm_runtime *runtime;
-	u16 *buf;
 	int fifo_samples;
 	int frames_left;
 	int samples;
@@ -494,12 +508,23 @@ static int rz_ssi_pio_recv(struct rz_ssi_priv *ssi, struct rz_ssi_stream *strm)
 			break;
 
 		/* calculate new buffer index */
-		buf = (u16 *)runtime->dma_area;
-		buf += strm->buffer_pos * runtime->channels;
+		if (ssi->hw_params_cache.sample_width == 16) {
+			u16 *buf;
 
-		/* Note, only supports 16-bit samples */
-		for (i = 0; i < samples; i++)
-			*buf++ = (u16)(rz_ssi_reg_readl(ssi, SSIFRDR) >> 16);
+			buf = (u16 *)runtime->dma_area;
+			buf += strm->buffer_pos * runtime->channels;
+
+			for (i = 0; i < samples; i++)
+				*buf++ = (u16)(rz_ssi_reg_readl(ssi, SSIFRDR) >> 16);
+		} else {
+			u32 *buf;
+
+			buf = (u32 *)runtime->dma_area;
+			buf += strm->buffer_pos * runtime->channels;
+
+			for (i = 0; i < samples; i++)
+				*buf++ = rz_ssi_reg_readl(ssi, SSIFRDR);
+		}
 
 		rz_ssi_reg_mask_setl(ssi, SSIFSR, SSIFSR_RDF, 0);
 		rz_ssi_pointer_update(strm, samples / runtime->channels);
@@ -517,7 +542,6 @@ static int rz_ssi_pio_send(struct rz_ssi_priv *ssi, struct rz_ssi_stream *strm)
 	int frames_left;
 	int i;
 	u32 ssifsr;
-	u16 *buf;
 
 	if (!rz_ssi_stream_is_valid(ssi, strm))
 		return -EINVAL;
@@ -546,12 +570,23 @@ static int rz_ssi_pio_send(struct rz_ssi_priv *ssi, struct rz_ssi_stream *strm)
 		return 0;
 
 	/* calculate new buffer index */
-	buf = (u16 *)(runtime->dma_area);
-	buf += strm->buffer_pos * runtime->channels;
+	if (ssi->hw_params_cache.sample_width == 16) {
+		u16 *buf;
+
+		buf = (u16 *)(runtime->dma_area);
+		buf += strm->buffer_pos * runtime->channels;
+
+		for (i = 0; i < samples; i++)
+			rz_ssi_reg_writel(ssi, SSIFTDR, ((u32)(*buf++) << 16));
+	} else {
+		u32 *buf;
 
-	/* Note, only supports 16-bit samples */
-	for (i = 0; i < samples; i++)
-		rz_ssi_reg_writel(ssi, SSIFTDR, ((u32)(*buf++) << 16));
+		buf = (u32 *)(runtime->dma_area);
+		buf += strm->buffer_pos * runtime->channels;
+
+		for (i = 0; i < samples; i++)
+			rz_ssi_reg_writel(ssi, SSIFTDR, *buf++);
+	}
 
 	rz_ssi_reg_mask_setl(ssi, SSIFSR, SSIFSR_TDE, 0);
 	rz_ssi_pointer_update(strm, samples / runtime->channels);
@@ -662,8 +697,13 @@ static int rz_ssi_dma_slave_config(struct rz_ssi_priv *ssi,
 	cfg.direction = is_play ? DMA_MEM_TO_DEV : DMA_DEV_TO_MEM;
 	cfg.dst_addr = ssi->phys + SSIFTDR;
 	cfg.src_addr = ssi->phys + SSIFRDR;
-	cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
-	cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+	if (ssi->hw_params_cache.sample_width == 24) {
+		cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+		cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+	} else {
+		cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+		cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+	}
 
 	return dmaengine_slave_config(dma_ch, &cfg);
 }
@@ -981,7 +1021,7 @@ static int rz_ssi_dai_hw_params(struct snd_pcm_substream *substream,
 	unsigned int rate = params_rate(params);
 	int ret;
 
-	if (sample_bits != 16) {
+	if (!(sample_bits == 16 || sample_bits == 24)) {
 		dev_err(ssi->dev, "Unsupported sample width: %d\n",
 			sample_bits);
 		return -EINVAL;
-- 
2.43.0

[PATCH 7/7] ASoC: renesas: rz-ssi: Add support for 32 bits sample width

[Biju]

From: Biju Das <biju.das.jz@bp.renesas.com>

Add support for 32 bits sample format width for RZ/G2L SoCs.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 sound/soc/renesas/rz-ssi.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/sound/soc/renesas/rz-ssi.c b/sound/soc/renesas/rz-ssi.c
index 5ebaaa440141..85d368794de4 100644
--- a/sound/soc/renesas/rz-ssi.c
+++ b/sound/soc/renesas/rz-ssi.c
@@ -75,7 +75,8 @@
 #define PREALLOC_BUFFER_MAX	(SZ_32K)
 
 #define SSI_RATES		SNDRV_PCM_RATE_8000_48000 /* 8k-48kHz */
-#define SSI_FMTS		(SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE)
+#define SSI_FMTS		(SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE | \
+				 SNDRV_PCM_FMTBIT_S32_LE)
 #define SSI_CHAN_MIN		2
 #define SSI_CHAN_MAX		2
 #define SSI_FIFO_DEPTH		32
@@ -295,7 +296,7 @@ static int rz_ssi_clk_setup(struct rz_ssi_priv *ssi, unsigned int rate,
 	}
 
 	/*
-	 * DWL: Data Word Length = {16, 24} bits
+	 * DWL: Data Word Length = {16, 24, 32} bits
 	 * SWL: System Word Length = 32 bits
 	 */
 	ssicr |= SSICR_CKDV(clk_ckdv);
@@ -307,6 +308,9 @@ static int rz_ssi_clk_setup(struct rz_ssi_priv *ssi, unsigned int rate,
 		ssicr |= SSICR_PDTA;
 		ssicr |= SSICR_DWL(5);
 		break;
+	case 32:
+		ssicr |= SSICR_DWL(6);
+		break;
 	default:
 		dev_err(ssi->dev, "Not support %u data width",
 			ssi->hw_params_cache.sample_width);
@@ -697,7 +701,8 @@ static int rz_ssi_dma_slave_config(struct rz_ssi_priv *ssi,
 	cfg.direction = is_play ? DMA_MEM_TO_DEV : DMA_DEV_TO_MEM;
 	cfg.dst_addr = ssi->phys + SSIFTDR;
 	cfg.src_addr = ssi->phys + SSIFRDR;
-	if (ssi->hw_params_cache.sample_width == 24) {
+	if (ssi->hw_params_cache.sample_width == 24 ||
+	    ssi->hw_params_cache.sample_width == 32) {
 		cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
 		cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
 	} else {
@@ -1021,7 +1026,7 @@ static int rz_ssi_dai_hw_params(struct snd_pcm_substream *substream,
 	unsigned int rate = params_rate(params);
 	int ret;
 
-	if (!(sample_bits == 16 || sample_bits == 24)) {
+	if (!(sample_bits == 16 || sample_bits == 24 || sample_bits == 32)) {
 		dev_err(ssi->dev, "Unsupported sample width: %d\n",
 			sample_bits);
 		return -EINVAL;
-- 
2.43.0

Re: [PATCH 0/7] RZ/G2L SSI Improvements

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 532 bytes --]

On Wed, Nov 12, 2025 at 03:41:01PM +0000, Biju wrote:
> From: Biju Das <biju.das.jz@bp.renesas.com>
> 
> The first patch in this series fixes audio channel swap issue because of
> half duplex to full duplex switching during the streaming that involves
> FIFO reset which drops the word leading to audio channel swap issue. The
> second patch fixes sample width value in the hw_params_cache::sample_width
> Apart from this, support 24 and 32 bit sample format width.

Copying in Morimoto-san given that it's Renesas stuff.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 0/7] RZ/G2L SSI Improvements

[Kuninori Morimoto]

Hi Biju

> The first patch in this series fixes audio channel swap issue because of
> half duplex to full duplex switching during the streaming that involves
> FIFO reset which drops the word leading to audio channel swap issue. The
> second patch fixes sample width value in the hw_params_cache::sample_width
> Apart from this, support 24 and 32 bit sample format width.

I think splitting patch (for fixes patch-set, for new feature patch-set)
is easy to handle for Mark ?

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH 1/7] ASoC: renesas: rz-ssi: Fix channel swap issue in full duplex mode

[Kuninori Morimoto]

Hi Biju

Thank you for the patch

> The full duplex audio starts with half duplex mode and then switch to
> full duplex mode (another FIFO reset) when both playback/capture
> streams available leading to random audio left/right channel swap
> issue. Fix this channel swap issue by detecting the full duplex
> condition by populating struct dup variable in startup() callback
> and synchronize starting both the play and capture at the same time
> in rz_ssi_start().
> 
> Cc: stable@kernel.org
> Fixes: 4f8cd05a4305 ("ASoC: sh: rz-ssi: Add full duplex support")
> Co-developed-by: Tony Tang <tony.tang.ks@renesas.com>
> Signed-off-by: Tony Tang <tony.tang.ks@renesas.com>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
(snip)
>  	if (!is_full_duplex) {
>  		ssifcr &= ~0xF;
>  	} else {
> -		rz_ssi_reg_mask_setl(ssi, SSICR, SSICR_TEN | SSICR_REN, 0);
> -		rz_ssi_set_idle(ssi);
> -		ssifcr &= ~SSIFCR_FIFO_RST;
> +		if (ssi->dup.one_stream_triggered) {
> +			rz_ssi_reg_mask_setl(ssi, SSICR, SSICR_TEN | SSICR_REN, 0);
> +			rz_ssi_set_idle(ssi);
> +			ssifcr &= ~SSIFCR_FIFO_RST;
> +		}
>  	}

nitpick

 } else if (...) { ?

for remains

Reviewed-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH 3/7] ASoC: renesas: rz-ssi: Use dev variable in probe()

[Kuninori Morimoto]

Hi Biju

> From: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Replace '&pdev->dev'->dev in probe(), this makes few error paths shorter.

nitpick

'&pdev->dev'->'dev' ?

Reviewed-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Best regards
---
Kuninori Morimoto

Re: [PATCH 2/7] ASoC: renesas: rz-ssi: Fix rz_ssi_priv::hw_params_cache::sample_width

[Kuninori Morimoto]

Hi

> From: Biju Das <biju.das.jz@bp.renesas.com>
> 
> The strm->sample_width is not filled during rz_ssi_dai_hw_params(). This
> wrong value is used for caching sample_width in struct hw_params_cache.
> Fix this issue by replacing 'strm->sample_width'->'params_width(params)'
> in rz_ssi_dai_hw_params(). After this drop the variable sample_width
> from struct rz_ssi_stream as it is unused.
> 
> Cc: stable@kernel.org
> Fixes: 4f8cd05a4305 ("ASoC: sh: rz-ssi: Add full duplex support")
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---

Reviewed-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Best regards
---
Kuninori Morimoto

Re: [PATCH 4/7] ASoC: renesas: rz-ssi: Remove trailing comma in the terminator entry

[Kuninori Morimoto]

Hi

> From: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Remove trailing comma in the terminator entry for OF table. While at it,
> add a space between the braces and comment block.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---

Reviewed-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH 6/7] ASoC: renesas: rz-ssi: Add support for 24 bits sample width

[Kuninori Morimoto]

Hi

> From: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Add support for 24 bits sample format width for RZ/G2L SoCs.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
(snip)
>  	ssicr |= SSICR_CKDV(clk_ckdv);
> -	ssicr |= SSICR_DWL(1) | SSICR_SWL(3);
> +	switch (ssi->hw_params_cache.sample_width) {
> +	case 16:
> +		ssicr |= SSICR_DWL(1);
> +		break;
> +	case 24:
> +		ssicr |= SSICR_PDTA;
> +		ssicr |= SSICR_DWL(5);

nitpick
It can be 1 line ?

> @@ -662,8 +697,13 @@ static int rz_ssi_dma_slave_config(struct rz_ssi_priv *ssi,
>  	cfg.direction = is_play ? DMA_MEM_TO_DEV : DMA_DEV_TO_MEM;
>  	cfg.dst_addr = ssi->phys + SSIFTDR;
>  	cfg.src_addr = ssi->phys + SSIFRDR;
> -	cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> -	cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> +	if (ssi->hw_params_cache.sample_width == 24) {
> +		cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> +		cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> +	} else {
> +		cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> +		cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> +	}

You can reduce extra if check when 32 case ([7/7]) if it was

	if (xxx == 16) {
		...
	} else { // for 24, 32
		...
	}

Thank you for your help !!

Best regards
---
Kuninori Morimoto

RE: [PATCH 0/7] RZ/G2L SSI Improvements

[Biju Das]

Hi Morimoto-san,

Thanks for the feedback.

> -----Original Message-----
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Sent: 13 November 2025 05:49
> Subject: Re: [PATCH 0/7] RZ/G2L SSI Improvements
> 
> 
> Hi Biju
> 
> > The first patch in this series fixes audio channel swap issue because
> > of half duplex to full duplex switching during the streaming that
> > involves FIFO reset which drops the word leading to audio channel swap
> > issue. The second patch fixes sample width value in the
> > hw_params_cache::sample_width Apart from this, support 24 and 32 bit sample format width.
> 
> I think splitting patch (for fixes patch-set, for new feature patch-set) is easy to handle for Mark ?

OK, will split the series into two.

Cheers,
Biju

RE: [PATCH 1/7] ASoC: renesas: rz-ssi: Fix channel swap issue in full duplex mode

[Biju Das]

Hi Morimoto-san,

Thanks for the feedback.

> -----Original Message-----
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Sent: 13 November 2025 05:50
> Subject: Re: [PATCH 1/7] ASoC: renesas: rz-ssi: Fix channel swap issue in full duplex mode
> 
> 
> Hi Biju
> 
> Thank you for the patch
> 
> > The full duplex audio starts with half duplex mode and then switch to
> > full duplex mode (another FIFO reset) when both playback/capture
> > streams available leading to random audio left/right channel swap
> > issue. Fix this channel swap issue by detecting the full duplex
> > condition by populating struct dup variable in startup() callback and
> > synchronize starting both the play and capture at the same time in
> > rz_ssi_start().
> >
> > Cc: stable@kernel.org
> > Fixes: 4f8cd05a4305 ("ASoC: sh: rz-ssi: Add full duplex support")
> > Co-developed-by: Tony Tang <tony.tang.ks@renesas.com>
> > Signed-off-by: Tony Tang <tony.tang.ks@renesas.com>
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> (snip)
> >  	if (!is_full_duplex) {
> >  		ssifcr &= ~0xF;
> >  	} else {
> > -		rz_ssi_reg_mask_setl(ssi, SSICR, SSICR_TEN | SSICR_REN, 0);
> > -		rz_ssi_set_idle(ssi);
> > -		ssifcr &= ~SSIFCR_FIFO_RST;
> > +		if (ssi->dup.one_stream_triggered) {
> > +			rz_ssi_reg_mask_setl(ssi, SSICR, SSICR_TEN | SSICR_REN, 0);
> > +			rz_ssi_set_idle(ssi);
> > +			ssifcr &= ~SSIFCR_FIFO_RST;
> > +		}
> >  	}
> 
> nitpick
> 
>  } else if (...) { ?

Agreed. will use else if.

Cheers,
Biju

RE: [PATCH 3/7] ASoC: renesas: rz-ssi: Use dev variable in probe()

[Biju Das]

Hi Morimoto-san,

Thanks for the feedback.

> -----Original Message-----
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Sent: 13 November 2025 05:50
> Subject: Re: [PATCH 3/7] ASoC: renesas: rz-ssi: Use dev variable in probe()
> 
> 
> Hi Biju
> 
> > From: Biju Das <biju.das.jz@bp.renesas.com>
> >
> > Replace '&pdev->dev'->dev in probe(), this makes few error paths shorter.
> 
> nitpick
> 
> '&pdev->dev'->'dev' ?

OK, will fix this.

Cheers,
Biju

RE: [PATCH 6/7] ASoC: renesas: rz-ssi: Add support for 24 bits sample width

[Biju Das]

Hi Morimoto-san,

Thanks for the feedback.

> -----Original Message-----
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Sent: 13 November 2025 05:56
> Subject: Re: [PATCH 6/7] ASoC: renesas: rz-ssi: Add support for 24 bits sample width
> 
> 
> Hi
> 
> > From: Biju Das <biju.das.jz@bp.renesas.com>
> >
> > Add support for 24 bits sample format width for RZ/G2L SoCs.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> (snip)
> >  	ssicr |= SSICR_CKDV(clk_ckdv);
> > -	ssicr |= SSICR_DWL(1) | SSICR_SWL(3);
> > +	switch (ssi->hw_params_cache.sample_width) {
> > +	case 16:
> > +		ssicr |= SSICR_DWL(1);
> > +		break;
> > +	case 24:
> > +		ssicr |= SSICR_PDTA;
> > +		ssicr |= SSICR_DWL(5);
> 
> nitpick
> It can be 1 line ?

Agreed.

> 
> > @@ -662,8 +697,13 @@ static int rz_ssi_dma_slave_config(struct rz_ssi_priv *ssi,
> >  	cfg.direction = is_play ? DMA_MEM_TO_DEV : DMA_DEV_TO_MEM;
> >  	cfg.dst_addr = ssi->phys + SSIFTDR;
> >  	cfg.src_addr = ssi->phys + SSIFRDR;
> > -	cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> > -	cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> > +	if (ssi->hw_params_cache.sample_width == 24) {
> > +		cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> > +		cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> > +	} else {
> > +		cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> > +		cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> > +	}
> 
> You can reduce extra if check when 32 case ([7/7]) if it was
> 
> 	if (xxx == 16) {
> 		...
> 	} else { // for 24, 32
> 		...
> 	}

OK, will fix this in next version.

Cheers,
Biju

Re: [PATCH 3/7] ASoC: renesas: rz-ssi: Use dev variable in probe()

[Geert Uytterhoeven]

Hi Biju,

On Thu, 13 Nov 2025 at 09:02, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > > From: Biju Das <biju.das.jz@bp.renesas.com>
> > >
> > > Replace '&pdev->dev'->dev in probe(), this makes few error paths shorter.
> >
> > nitpick
> >
> > '&pdev->dev'->'dev' ?

Woow, that "->"-pipeline looks confusing ;-)

>
> OK, will fix this.

Please use English where it makes sense:

    Replace <foo> by <bar> ...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH 3/7] ASoC: renesas: rz-ssi: Use dev variable in probe()

[Biju Das]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 13 November 2025 08:48
> Subject: Re: [PATCH 3/7] ASoC: renesas: rz-ssi: Use dev variable in probe()
> 
> Hi Biju,
> 
> On Thu, 13 Nov 2025 at 09:02, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > > > From: Biju Das <biju.das.jz@bp.renesas.com>
> > > >
> > > > Replace '&pdev->dev'->dev in probe(), this makes few error paths shorter.
> > >
> > > nitpick
> > >
> > > '&pdev->dev'->'dev' ?
> 
> Woow, that "->"-pipeline looks confusing ;-)
> 
> >
> > OK, will fix this.
> 
> Please use English where it makes sense:
> 
>     Replace <foo> by <bar> ...

Ok, will use Replace '&pdev->dev' by 'dev'.

Cheers,
Biju