From 541011dc2d7c4c82523706f726f422a5e23cc86f Mon Sep 17 00:00:00 2001 From: Claudiu Beznea Date: Tue, 10 Dec 2024 19:09:33 +0200 Subject: [PATCH 01/16] ASoC: renesas: rz-ssi: Terminate all the DMA transactions The stop trigger invokes rz_ssi_stop() and rz_ssi_stream_quit(). - The purpose of rz_ssi_stop() is to disable TX/RX, terminate DMA transactions, and set the controller to idle. - The purpose of rz_ssi_stream_quit() is to reset the substream-specific software data by setting strm->running and strm->substream appropriately. The function rz_ssi_is_stream_running() checks if both strm->substream and strm->running are valid and returns true if so. Its implementation is as follows: static inline bool rz_ssi_is_stream_running(struct rz_ssi_stream *strm) { return strm->substream && strm->running; } When the controller is configured in full-duplex mode (with both playback and capture active), the rz_ssi_stop() function does not modify the controller settings when called for the first substream in the full-duplex setup. Instead, it simply sets strm->running = 0 and returns if the companion substream is still running. The following code illustrates this: 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; // ... } The controller settings, along with the DMA termination (for the last stopped substream), are only applied when the last substream in the full-duplex setup is stopped. While applying the controller settings only when the last substream stops is not problematic, terminating the DMA operations for only one substream causes failures when starting and stopping full-duplex operations multiple times in a loop. To address this issue, call dmaengine_terminate_async() for both substreams involved in the full-duplex setup when the last substream in the setup is stopped. Fixes: 4f8cd05a4305 ("ASoC: sh: rz-ssi: Add full duplex support") Cc: stable@vger.kernel.org Reviewed-by: Biju Das Signed-off-by: Claudiu Beznea Reviewed-by: Geert Uytterhoeven Link: https://patch.msgid.link/20241210170953.2936724-5-claudiu.beznea.uj@bp.renesas.com Signed-off-by: Mark Brown --- sound/soc/renesas/rz-ssi.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/sound/soc/renesas/rz-ssi.c b/sound/soc/renesas/rz-ssi.c index 6efd017aaa7f..2d8721156099 100644 --- a/sound/soc/renesas/rz-ssi.c +++ b/sound/soc/renesas/rz-ssi.c @@ -415,8 +415,12 @@ static int rz_ssi_stop(struct rz_ssi_priv *ssi, struct rz_ssi_stream *strm) rz_ssi_reg_mask_setl(ssi, SSICR, SSICR_TEN | SSICR_REN, 0); /* Cancel all remaining DMA transactions */ - if (rz_ssi_is_dma_enabled(ssi)) - dmaengine_terminate_async(strm->dma_ch); + if (rz_ssi_is_dma_enabled(ssi)) { + if (ssi->playback.dma_ch) + dmaengine_terminate_async(ssi->playback.dma_ch); + if (ssi->capture.dma_ch) + dmaengine_terminate_async(ssi->capture.dma_ch); + } rz_ssi_set_idle(ssi); From 55c209cd4318c701e6e88e0b2512a0f12dd02a7d Mon Sep 17 00:00:00 2001 From: Claudiu Beznea Date: Tue, 10 Dec 2024 19:09:34 +0200 Subject: [PATCH 02/16] ASoC: renesas: rz-ssi: Use only the proper amount of dividers There is no need to populate the ckdv[] with invalid dividers as that part will not be indexed anyway. The ssi->audio_mck/bclk_rate should always be >= 0. While at it, change the ckdv type as u8, as the divider 128 was previously using the s8 sign bit. Signed-off-by: Claudiu Beznea Fixes: 03e786bd43410fa9 ("ASoC: sh: Add RZ/G2L SSIF-2 driver") Reviewed-by: Geert Uytterhoeven Link: https://patch.msgid.link/20241210170953.2936724-6-claudiu.beznea.uj@bp.renesas.com Signed-off-by: Mark Brown --- sound/soc/renesas/rz-ssi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sound/soc/renesas/rz-ssi.c b/sound/soc/renesas/rz-ssi.c index 2d8721156099..178c915331e9 100644 --- a/sound/soc/renesas/rz-ssi.c +++ b/sound/soc/renesas/rz-ssi.c @@ -258,8 +258,7 @@ static void rz_ssi_stream_quit(struct rz_ssi_priv *ssi, static int rz_ssi_clk_setup(struct rz_ssi_priv *ssi, unsigned int rate, unsigned int channels) { - static s8 ckdv[16] = { 1, 2, 4, 8, 16, 32, 64, 128, - 6, 12, 24, 48, 96, -1, -1, -1 }; + static u8 ckdv[] = { 1, 2, 4, 8, 16, 32, 64, 128, 6, 12, 24, 48, 96 }; unsigned int channel_bits = 32; /* System Word Length */ unsigned long bclk_rate = rate * channels * channel_bits; unsigned int div; From 100c6b22d6c70adabdf45dcb346d7d853bff6a30 Mon Sep 17 00:00:00 2001 From: Claudiu Beznea Date: Tue, 10 Dec 2024 19:09:35 +0200 Subject: [PATCH 03/16] ASoC: renesas: rz-ssi: Fix typo on SSI_RATES macro comment The SSI_RATES macro covers 8KHz-48KHz audio frequencies. Update macro comment to reflect it. Reviewed-by: Geert Uytterhoeven Signed-off-by: Claudiu Beznea Link: https://patch.msgid.link/20241210170953.2936724-7-claudiu.beznea.uj@bp.renesas.com Signed-off-by: Mark Brown --- 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 178c915331e9..35929160a8a5 100644 --- a/sound/soc/renesas/rz-ssi.c +++ b/sound/soc/renesas/rz-ssi.c @@ -71,7 +71,7 @@ #define PREALLOC_BUFFER (SZ_32K) #define PREALLOC_BUFFER_MAX (SZ_32K) -#define SSI_RATES SNDRV_PCM_RATE_8000_48000 /* 8k-44.1kHz */ +#define SSI_RATES SNDRV_PCM_RATE_8000_48000 /* 8k-48kHz */ #define SSI_FMTS SNDRV_PCM_FMTBIT_S16_LE #define SSI_CHAN_MIN 2 #define SSI_CHAN_MAX 2 From a73710a25808a585a2bf0a8325eb16fd6a2f370c Mon Sep 17 00:00:00 2001 From: Claudiu Beznea Date: Tue, 10 Dec 2024 19:09:36 +0200 Subject: [PATCH 04/16] ASoC: renesas: rz-ssi: Remove pdev member of struct rz_ssi_priv Remove the pdev member of struct rz_ssi_priv as it is not used. Reviewed-by: Geert Uytterhoeven Signed-off-by: Claudiu Beznea Link: https://patch.msgid.link/20241210170953.2936724-8-claudiu.beznea.uj@bp.renesas.com Signed-off-by: Mark Brown --- sound/soc/renesas/rz-ssi.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/sound/soc/renesas/rz-ssi.c b/sound/soc/renesas/rz-ssi.c index 35929160a8a5..b24c323ee05f 100644 --- a/sound/soc/renesas/rz-ssi.c +++ b/sound/soc/renesas/rz-ssi.c @@ -99,7 +99,6 @@ struct rz_ssi_stream { struct rz_ssi_priv { void __iomem *base; - struct platform_device *pdev; struct reset_control *rstc; struct device *dev; struct clk *sfr_clk; @@ -1043,7 +1042,6 @@ static int rz_ssi_probe(struct platform_device *pdev) if (!ssi) return -ENOMEM; - ssi->pdev = pdev; ssi->dev = &pdev->dev; ssi->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res); if (IS_ERR(ssi->base)) From dec61e16e72db196e8dc1daf7f7022fd98e6d921 Mon Sep 17 00:00:00 2001 From: Claudiu Beznea Date: Tue, 10 Dec 2024 19:09:37 +0200 Subject: [PATCH 05/16] ASoC: renesas: rz-ssi: Remove the rz_ssi_get_dai() function Remove the rz_ssi_get_dai() function and use directly the snd_soc_rtd_to_cpu() where needed or the struct device pointer embedded in the struct rz_ssi_priv objects. Reviewed-by: Geert Uytterhoeven Signed-off-by: Claudiu Beznea Link: https://patch.msgid.link/20241210170953.2936724-9-claudiu.beznea.uj@bp.renesas.com Signed-off-by: Mark Brown --- sound/soc/renesas/rz-ssi.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/sound/soc/renesas/rz-ssi.c b/sound/soc/renesas/rz-ssi.c index b24c323ee05f..e2e172d8e9db 100644 --- a/sound/soc/renesas/rz-ssi.c +++ b/sound/soc/renesas/rz-ssi.c @@ -162,14 +162,6 @@ static void rz_ssi_reg_mask_setl(struct rz_ssi_priv *priv, uint reg, writel(val, (priv->base + reg)); } -static inline struct snd_soc_dai * -rz_ssi_get_dai(struct snd_pcm_substream *substream) -{ - struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); - - return snd_soc_rtd_to_cpu(rtd, 0); -} - static inline bool rz_ssi_stream_is_play(struct rz_ssi_priv *ssi, struct snd_pcm_substream *substream) { @@ -243,15 +235,15 @@ static void rz_ssi_stream_init(struct rz_ssi_stream *strm, static void rz_ssi_stream_quit(struct rz_ssi_priv *ssi, struct rz_ssi_stream *strm) { - struct snd_soc_dai *dai = rz_ssi_get_dai(strm->substream); + struct device *dev = ssi->dev; rz_ssi_set_substream(strm, NULL); if (strm->oerr_num > 0) - dev_info(dai->dev, "overrun = %d\n", strm->oerr_num); + dev_info(dev, "overrun = %d\n", strm->oerr_num); if (strm->uerr_num > 0) - dev_info(dai->dev, "underrun = %d\n", strm->uerr_num); + dev_info(dev, "underrun = %d\n", strm->uerr_num); } static int rz_ssi_clk_setup(struct rz_ssi_priv *ssi, unsigned int rate, @@ -988,7 +980,8 @@ static int rz_ssi_pcm_open(struct snd_soc_component *component, static snd_pcm_uframes_t rz_ssi_pcm_pointer(struct snd_soc_component *component, struct snd_pcm_substream *substream) { - struct snd_soc_dai *dai = rz_ssi_get_dai(substream); + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); + struct snd_soc_dai *dai = snd_soc_rtd_to_cpu(rtd, 0); struct rz_ssi_priv *ssi = snd_soc_dai_get_drvdata(dai); struct rz_ssi_stream *strm = rz_ssi_stream_get(ssi, substream); From 109e60866f11c7db8f720f01b0bda3105c47b463 Mon Sep 17 00:00:00 2001 From: Claudiu Beznea Date: Tue, 10 Dec 2024 19:09:38 +0200 Subject: [PATCH 06/16] ASoC: renesas: rz-ssi: Remove the first argument of rz_ssi_stream_is_play() The first argument of the rz_ssi_stream_is_play() is not used. Remove it. Reviewed-by: Geert Uytterhoeven Signed-off-by: Claudiu Beznea Link: https://patch.msgid.link/20241210170953.2936724-10-claudiu.beznea.uj@bp.renesas.com Signed-off-by: Mark Brown --- sound/soc/renesas/rz-ssi.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/sound/soc/renesas/rz-ssi.c b/sound/soc/renesas/rz-ssi.c index e2e172d8e9db..1a98f6b3e6a7 100644 --- a/sound/soc/renesas/rz-ssi.c +++ b/sound/soc/renesas/rz-ssi.c @@ -162,8 +162,7 @@ static void rz_ssi_reg_mask_setl(struct rz_ssi_priv *priv, uint reg, writel(val, (priv->base + reg)); } -static inline bool rz_ssi_stream_is_play(struct rz_ssi_priv *ssi, - struct snd_pcm_substream *substream) +static inline bool rz_ssi_stream_is_play(struct snd_pcm_substream *substream) { return substream->stream == SNDRV_PCM_STREAM_PLAYBACK; } @@ -337,7 +336,7 @@ static void rz_ssi_set_idle(struct rz_ssi_priv *ssi) static int rz_ssi_start(struct rz_ssi_priv *ssi, struct rz_ssi_stream *strm) { - bool is_play = rz_ssi_stream_is_play(ssi, strm->substream); + bool is_play = rz_ssi_stream_is_play(strm->substream); bool is_full_duplex; u32 ssicr, ssifcr; @@ -674,7 +673,7 @@ static int rz_ssi_dma_transfer(struct rz_ssi_priv *ssi, */ return 0; - dir = rz_ssi_stream_is_play(ssi, substream) ? DMA_MEM_TO_DEV : DMA_DEV_TO_MEM; + dir = rz_ssi_stream_is_play(substream) ? DMA_MEM_TO_DEV : DMA_DEV_TO_MEM; /* Always transfer 1 period */ amount = runtime->period_size; @@ -800,7 +799,7 @@ static int rz_ssi_dai_trigger(struct snd_pcm_substream *substream, int cmd, if (ssi->dma_rt) { bool is_playback; - is_playback = rz_ssi_stream_is_play(ssi, substream); + is_playback = rz_ssi_stream_is_play(substream); ret = rz_ssi_dma_slave_config(ssi, ssi->playback.dma_ch, is_playback); /* Fallback to pio */ From 4bf77dfa3308b7cfda29d9c4ead1dc32f1ceefa9 Mon Sep 17 00:00:00 2001 From: Claudiu Beznea Date: Tue, 10 Dec 2024 19:09:39 +0200 Subject: [PATCH 07/16] ASoC: renesas: rz-ssi: Use readl_poll_timeout_atomic() Use readl_poll_timeout_atomic() instead of hardcoding something similar. While at it replace dev_info() with dev_warn_ratelimited() as the rz_ssi_set_idle() can also be called from IRQ context and if the SSI idle is not properly set this is at least a warning for user. Reviewed-by: Geert Uytterhoeven Signed-off-by: Claudiu Beznea Link: https://patch.msgid.link/20241210170953.2936724-11-claudiu.beznea.uj@bp.renesas.com Signed-off-by: Mark Brown --- sound/soc/renesas/rz-ssi.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/sound/soc/renesas/rz-ssi.c b/sound/soc/renesas/rz-ssi.c index 1a98f6b3e6a7..03d409d3070c 100644 --- a/sound/soc/renesas/rz-ssi.c +++ b/sound/soc/renesas/rz-ssi.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -307,7 +308,8 @@ static int rz_ssi_clk_setup(struct rz_ssi_priv *ssi, unsigned int rate, static void rz_ssi_set_idle(struct rz_ssi_priv *ssi) { - int timeout; + u32 tmp; + int ret; /* Disable irqs */ rz_ssi_reg_mask_setl(ssi, SSICR, SSICR_TUIEN | SSICR_TOIEN | @@ -320,15 +322,9 @@ static void rz_ssi_set_idle(struct rz_ssi_priv *ssi) SSISR_RUIRQ), 0); /* Wait for idle */ - timeout = 100; - while (--timeout) { - if (rz_ssi_reg_readl(ssi, SSISR) & SSISR_IIRQ) - break; - udelay(1); - } - - if (!timeout) - dev_info(ssi->dev, "timeout waiting for SSI idle\n"); + ret = readl_poll_timeout_atomic(ssi->base + SSISR, tmp, (tmp & SSISR_IIRQ), 1, 100); + if (ret) + dev_warn_ratelimited(ssi->dev, "timeout waiting for SSI idle\n"); /* Hold FIFOs in reset */ rz_ssi_reg_mask_setl(ssi, SSIFCR, 0, SSIFCR_FIFO_RST); From 403366d2a43eb7c911c6cddf1d7882e429d1212d Mon Sep 17 00:00:00 2001 From: Claudiu Beznea Date: Tue, 10 Dec 2024 19:09:40 +0200 Subject: [PATCH 08/16] ASoC: renesas: rz-ssi: Use temporary variable for struct device Use a temporary variable for the struct device pointers to avoid dereferencing. Reviewed-by: Geert Uytterhoeven Signed-off-by: Claudiu Beznea Link: https://patch.msgid.link/20241210170953.2936724-12-claudiu.beznea.uj@bp.renesas.com Signed-off-by: Mark Brown --- sound/soc/renesas/rz-ssi.c | 62 +++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/sound/soc/renesas/rz-ssi.c b/sound/soc/renesas/rz-ssi.c index 03d409d3070c..74632e2482f8 100644 --- a/sound/soc/renesas/rz-ssi.c +++ b/sound/soc/renesas/rz-ssi.c @@ -1021,36 +1021,37 @@ static const struct snd_soc_component_driver rz_ssi_soc_component = { static int rz_ssi_probe(struct platform_device *pdev) { + struct device *dev = &pdev->dev; struct rz_ssi_priv *ssi; struct clk *audio_clk; struct resource *res; int ret; - ssi = devm_kzalloc(&pdev->dev, sizeof(*ssi), GFP_KERNEL); + ssi = devm_kzalloc(dev, sizeof(*ssi), GFP_KERNEL); if (!ssi) return -ENOMEM; - ssi->dev = &pdev->dev; + ssi->dev = dev; ssi->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res); if (IS_ERR(ssi->base)) return PTR_ERR(ssi->base); ssi->phys = res->start; - ssi->clk = devm_clk_get(&pdev->dev, "ssi"); + ssi->clk = devm_clk_get(dev, "ssi"); if (IS_ERR(ssi->clk)) return PTR_ERR(ssi->clk); - ssi->sfr_clk = devm_clk_get(&pdev->dev, "ssi_sfr"); + ssi->sfr_clk = devm_clk_get(dev, "ssi_sfr"); if (IS_ERR(ssi->sfr_clk)) return PTR_ERR(ssi->sfr_clk); - audio_clk = devm_clk_get(&pdev->dev, "audio_clk1"); + 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"); ssi->audio_clk_1 = clk_get_rate(audio_clk); - audio_clk = devm_clk_get(&pdev->dev, "audio_clk2"); + 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"); @@ -1063,13 +1064,13 @@ static int rz_ssi_probe(struct platform_device *pdev) ssi->audio_mck = ssi->audio_clk_1 ? ssi->audio_clk_1 : ssi->audio_clk_2; /* Detect DMA support */ - ret = rz_ssi_dma_request(ssi, &pdev->dev); + ret = rz_ssi_dma_request(ssi, dev); if (ret < 0) { - dev_warn(&pdev->dev, "DMA not available, using PIO\n"); + dev_warn(dev, "DMA not available, using PIO\n"); ssi->playback.transfer = rz_ssi_pio_send; ssi->capture.transfer = rz_ssi_pio_recv; } else { - dev_info(&pdev->dev, "DMA enabled"); + dev_info(dev, "DMA enabled"); ssi->playback.transfer = rz_ssi_dma_transfer; ssi->capture.transfer = rz_ssi_dma_transfer; } @@ -1078,7 +1079,7 @@ static int rz_ssi_probe(struct platform_device *pdev) ssi->capture.priv = ssi; spin_lock_init(&ssi->lock); - dev_set_drvdata(&pdev->dev, ssi); + dev_set_drvdata(dev, ssi); /* Error Interrupt */ ssi->irq_int = platform_get_irq_byname(pdev, "int_req"); @@ -1087,12 +1088,11 @@ static int rz_ssi_probe(struct platform_device *pdev) return ssi->irq_int; } - ret = devm_request_irq(&pdev->dev, ssi->irq_int, &rz_ssi_interrupt, - 0, dev_name(&pdev->dev), ssi); + ret = devm_request_irq(dev, ssi->irq_int, &rz_ssi_interrupt, + 0, dev_name(dev), ssi); if (ret < 0) { rz_ssi_release_dma_channels(ssi); - return dev_err_probe(&pdev->dev, ret, - "irq request error (int_req)\n"); + return dev_err_probe(dev, ret, "irq request error (int_req)\n"); } if (!rz_ssi_is_dma_enabled(ssi)) { @@ -1104,11 +1104,11 @@ static int rz_ssi_probe(struct platform_device *pdev) if (ssi->irq_rt < 0) return ssi->irq_rt; - ret = devm_request_irq(&pdev->dev, ssi->irq_rt, + ret = devm_request_irq(dev, ssi->irq_rt, &rz_ssi_interrupt, 0, - dev_name(&pdev->dev), ssi); + dev_name(dev), ssi); if (ret < 0) - return dev_err_probe(&pdev->dev, ret, + return dev_err_probe(dev, ret, "irq request error (dma_rt)\n"); } else { if (ssi->irq_tx < 0) @@ -1117,50 +1117,50 @@ static int rz_ssi_probe(struct platform_device *pdev) if (ssi->irq_rx < 0) return ssi->irq_rx; - ret = devm_request_irq(&pdev->dev, ssi->irq_tx, + ret = devm_request_irq(dev, ssi->irq_tx, &rz_ssi_interrupt, 0, - dev_name(&pdev->dev), ssi); + dev_name(dev), ssi); if (ret < 0) - return dev_err_probe(&pdev->dev, ret, + return dev_err_probe(dev, ret, "irq request error (dma_tx)\n"); - ret = devm_request_irq(&pdev->dev, ssi->irq_rx, + ret = devm_request_irq(dev, ssi->irq_rx, &rz_ssi_interrupt, 0, - dev_name(&pdev->dev), ssi); + dev_name(dev), ssi); if (ret < 0) - return dev_err_probe(&pdev->dev, ret, + return dev_err_probe(dev, ret, "irq request error (dma_rx)\n"); } } - ssi->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL); + ssi->rstc = devm_reset_control_get_exclusive(dev, NULL); if (IS_ERR(ssi->rstc)) { ret = PTR_ERR(ssi->rstc); goto err_reset; } reset_control_deassert(ssi->rstc); - pm_runtime_enable(&pdev->dev); - ret = pm_runtime_resume_and_get(&pdev->dev); + pm_runtime_enable(dev); + ret = pm_runtime_resume_and_get(dev); if (ret < 0) { - dev_err(&pdev->dev, "pm_runtime_resume_and_get failed\n"); + dev_err(dev, "pm_runtime_resume_and_get failed\n"); goto err_pm; } - ret = devm_snd_soc_register_component(&pdev->dev, &rz_ssi_soc_component, + ret = devm_snd_soc_register_component(dev, &rz_ssi_soc_component, rz_ssi_soc_dai, ARRAY_SIZE(rz_ssi_soc_dai)); if (ret < 0) { - dev_err(&pdev->dev, "failed to register snd component\n"); + dev_err(dev, "failed to register snd component\n"); goto err_snd_soc; } return 0; err_snd_soc: - pm_runtime_put(ssi->dev); + pm_runtime_put(dev); err_pm: - pm_runtime_disable(ssi->dev); + pm_runtime_disable(dev); reset_control_assert(ssi->rstc); err_reset: rz_ssi_release_dma_channels(ssi); From f0c155c9da7536ab33687b5207eb21e704122a56 Mon Sep 17 00:00:00 2001 From: Claudiu Beznea Date: Tue, 10 Dec 2024 19:09:41 +0200 Subject: [PATCH 09/16] ASoC: renesas: rz-ssi: Use goto label names that specify their actions Use goto label names that specify their action. In this way we can have a better understanding of what is the action associated with the label by just reading the label name. Reviewed-by: Geert Uytterhoeven Signed-off-by: Claudiu Beznea Link: https://patch.msgid.link/20241210170953.2936724-13-claudiu.beznea.uj@bp.renesas.com Signed-off-by: Mark Brown --- sound/soc/renesas/rz-ssi.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/sound/soc/renesas/rz-ssi.c b/sound/soc/renesas/rz-ssi.c index 74632e2482f8..209b5b8827e5 100644 --- a/sound/soc/renesas/rz-ssi.c +++ b/sound/soc/renesas/rz-ssi.c @@ -1084,15 +1084,15 @@ static int rz_ssi_probe(struct platform_device *pdev) /* Error Interrupt */ ssi->irq_int = platform_get_irq_byname(pdev, "int_req"); if (ssi->irq_int < 0) { - rz_ssi_release_dma_channels(ssi); - return ssi->irq_int; + ret = ssi->irq_int; + goto err_release_dma_chs; } ret = devm_request_irq(dev, ssi->irq_int, &rz_ssi_interrupt, 0, dev_name(dev), ssi); if (ret < 0) { - rz_ssi_release_dma_channels(ssi); - return dev_err_probe(dev, ret, "irq request error (int_req)\n"); + dev_err_probe(dev, ret, "irq request error (int_req)\n"); + goto err_release_dma_chs; } if (!rz_ssi_is_dma_enabled(ssi)) { @@ -1136,7 +1136,7 @@ static int rz_ssi_probe(struct platform_device *pdev) ssi->rstc = devm_reset_control_get_exclusive(dev, NULL); if (IS_ERR(ssi->rstc)) { ret = PTR_ERR(ssi->rstc); - goto err_reset; + goto err_release_dma_chs; } reset_control_deassert(ssi->rstc); @@ -1152,17 +1152,17 @@ static int rz_ssi_probe(struct platform_device *pdev) ARRAY_SIZE(rz_ssi_soc_dai)); if (ret < 0) { dev_err(dev, "failed to register snd component\n"); - goto err_snd_soc; + goto err_pm_put; } return 0; -err_snd_soc: +err_pm_put: pm_runtime_put(dev); err_pm: pm_runtime_disable(dev); reset_control_assert(ssi->rstc); -err_reset: +err_release_dma_chs: rz_ssi_release_dma_channels(ssi); return ret; From e8fcf25f562891d5c0734d4f49c44bb6aa72bc15 Mon Sep 17 00:00:00 2001 From: Claudiu Beznea Date: Tue, 10 Dec 2024 19:09:42 +0200 Subject: [PATCH 10/16] ASoC: renesas: rz-ssi: Rely on the ASoC subsystem to runtime resume/suspend the SSI The ASoC subsystem takes care of runtime resume/suspend the audio devices when needed. Just enable the runtime PM on the SSI driver and let the subsystem runtime resume/suspend it. While at it use directly the devm_pm_runtime_enable(). Reviewed-by: Geert Uytterhoeven Signed-off-by: Claudiu Beznea Link: https://patch.msgid.link/20241210170953.2936724-14-claudiu.beznea.uj@bp.renesas.com Signed-off-by: Mark Brown --- sound/soc/renesas/rz-ssi.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/sound/soc/renesas/rz-ssi.c b/sound/soc/renesas/rz-ssi.c index 209b5b8827e5..878158344f88 100644 --- a/sound/soc/renesas/rz-ssi.c +++ b/sound/soc/renesas/rz-ssi.c @@ -1140,11 +1140,10 @@ static int rz_ssi_probe(struct platform_device *pdev) } reset_control_deassert(ssi->rstc); - pm_runtime_enable(dev); - ret = pm_runtime_resume_and_get(dev); + ret = devm_pm_runtime_enable(dev); if (ret < 0) { - dev_err(dev, "pm_runtime_resume_and_get failed\n"); - goto err_pm; + dev_err(dev, "Failed to enable runtime PM!\n"); + goto err_reset; } ret = devm_snd_soc_register_component(dev, &rz_ssi_soc_component, @@ -1152,15 +1151,12 @@ static int rz_ssi_probe(struct platform_device *pdev) ARRAY_SIZE(rz_ssi_soc_dai)); if (ret < 0) { dev_err(dev, "failed to register snd component\n"); - goto err_pm_put; + goto err_reset; } return 0; -err_pm_put: - pm_runtime_put(dev); -err_pm: - pm_runtime_disable(dev); +err_reset: reset_control_assert(ssi->rstc); err_release_dma_chs: rz_ssi_release_dma_channels(ssi); @@ -1174,8 +1170,6 @@ static void rz_ssi_remove(struct platform_device *pdev) rz_ssi_release_dma_channels(ssi); - pm_runtime_put(ssi->dev); - pm_runtime_disable(ssi->dev); reset_control_assert(ssi->rstc); } From cf3a79e4f826fc680fd7bfef7c427e2cc6023bc3 Mon Sep 17 00:00:00 2001 From: Claudiu Beznea Date: Tue, 10 Dec 2024 19:09:43 +0200 Subject: [PATCH 11/16] ASoC: renesas: rz-ssi: Enable runtime PM autosuspend support Enable runtime PM autosuspend support. The chosen autosuspend delay is zero for immediate autosuspend. In case there are users that need a different autosuspend delay, it can be adjusted through sysfs. Reviewed-by: Geert Uytterhoeven Signed-off-by: Claudiu Beznea Link: https://patch.msgid.link/20241210170953.2936724-15-claudiu.beznea.uj@bp.renesas.com Signed-off-by: Mark Brown --- sound/soc/renesas/rz-ssi.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sound/soc/renesas/rz-ssi.c b/sound/soc/renesas/rz-ssi.c index 878158344f88..eebf2d647ef2 100644 --- a/sound/soc/renesas/rz-ssi.c +++ b/sound/soc/renesas/rz-ssi.c @@ -1140,6 +1140,9 @@ static int rz_ssi_probe(struct platform_device *pdev) } reset_control_deassert(ssi->rstc); + /* Default 0 for power saving. Can be overridden via sysfs. */ + pm_runtime_set_autosuspend_delay(dev, 0); + pm_runtime_use_autosuspend(dev); ret = devm_pm_runtime_enable(dev); if (ret < 0) { dev_err(dev, "Failed to enable runtime PM!\n"); From 3888672495fcaee98b90196c0a899b1c2eb57d5b Mon Sep 17 00:00:00 2001 From: Claudiu Beznea Date: Tue, 10 Dec 2024 19:09:44 +0200 Subject: [PATCH 12/16] ASoC: renesas: rz-ssi: Add runtime PM support Add runtime PM support to the ssi driver. This assert/de-assert the reset lines on runtime suspend/resume. Along with it the de-assertion of the reset line from probe function was removed as it is not necessary anymore. Reviewed-by: Geert Uytterhoeven Signed-off-by: Claudiu Beznea Link: https://patch.msgid.link/20241210170953.2936724-16-claudiu.beznea.uj@bp.renesas.com Signed-off-by: Mark Brown --- sound/soc/renesas/rz-ssi.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/sound/soc/renesas/rz-ssi.c b/sound/soc/renesas/rz-ssi.c index eebf2d647ef2..34c2e22b5a67 100644 --- a/sound/soc/renesas/rz-ssi.c +++ b/sound/soc/renesas/rz-ssi.c @@ -1139,14 +1139,13 @@ static int rz_ssi_probe(struct platform_device *pdev) goto err_release_dma_chs; } - reset_control_deassert(ssi->rstc); /* Default 0 for power saving. Can be overridden via sysfs. */ pm_runtime_set_autosuspend_delay(dev, 0); pm_runtime_use_autosuspend(dev); ret = devm_pm_runtime_enable(dev); if (ret < 0) { dev_err(dev, "Failed to enable runtime PM!\n"); - goto err_reset; + goto err_release_dma_chs; } ret = devm_snd_soc_register_component(dev, &rz_ssi_soc_component, @@ -1154,13 +1153,11 @@ static int rz_ssi_probe(struct platform_device *pdev) ARRAY_SIZE(rz_ssi_soc_dai)); if (ret < 0) { dev_err(dev, "failed to register snd component\n"); - goto err_reset; + goto err_release_dma_chs; } return 0; -err_reset: - reset_control_assert(ssi->rstc); err_release_dma_chs: rz_ssi_release_dma_channels(ssi); @@ -1182,10 +1179,29 @@ static const struct of_device_id rz_ssi_of_match[] = { }; MODULE_DEVICE_TABLE(of, rz_ssi_of_match); +static int rz_ssi_runtime_suspend(struct device *dev) +{ + struct rz_ssi_priv *ssi = dev_get_drvdata(dev); + + return reset_control_assert(ssi->rstc); +} + +static int rz_ssi_runtime_resume(struct device *dev) +{ + struct rz_ssi_priv *ssi = dev_get_drvdata(dev); + + return reset_control_deassert(ssi->rstc); +} + +static const struct dev_pm_ops rz_ssi_pm_ops = { + RUNTIME_PM_OPS(rz_ssi_runtime_suspend, rz_ssi_runtime_resume, NULL) +}; + static struct platform_driver rz_ssi_driver = { .driver = { .name = "rz-ssi-pcm-audio", .of_match_table = rz_ssi_of_match, + .pm = pm_ptr(&rz_ssi_pm_ops), }, .probe = rz_ssi_probe, .remove = rz_ssi_remove, From fc2a31affb22394d1d74d3ecc86b5c68da33d52a Mon Sep 17 00:00:00 2001 From: Claudiu Beznea Date: Tue, 10 Dec 2024 19:09:45 +0200 Subject: [PATCH 13/16] ASoC: renesas: rz-ssi: Issue software reset in hw_params API The code initially issued software reset on SNDRV_PCM_TRIGGER_START action only before starting the first stream. This can be easily moved to hw_params() as the action is similar to setting the clocks. Moreover, according to the hardware manual (Table 35.7 Bits Initialized by Software Reset of the SSIFCR.SSIRST Bit) the software reset action acts also on the clock dividers bits. Due to this issue the software reset in hw_params() before configuring the clock dividers. This also simplifies the code in trigger API. Reviewed-by: Geert Uytterhoeven Signed-off-by: Claudiu Beznea Link: https://patch.msgid.link/20241210170953.2936724-17-claudiu.beznea.uj@bp.renesas.com Signed-off-by: Mark Brown --- sound/soc/renesas/rz-ssi.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/sound/soc/renesas/rz-ssi.c b/sound/soc/renesas/rz-ssi.c index 34c2e22b5a67..486822d79458 100644 --- a/sound/soc/renesas/rz-ssi.c +++ b/sound/soc/renesas/rz-ssi.c @@ -388,6 +388,15 @@ static int rz_ssi_start(struct rz_ssi_priv *ssi, struct rz_ssi_stream *strm) return 0; } +static int rz_ssi_swreset(struct rz_ssi_priv *ssi) +{ + u32 tmp; + + rz_ssi_reg_mask_setl(ssi, SSIFCR, 0, SSIFCR_SSIRST); + rz_ssi_reg_mask_setl(ssi, SSIFCR, SSIFCR_SSIRST, 0); + return readl_poll_timeout_atomic(ssi->base + SSIFCR, tmp, !(tmp & SSIFCR_SSIRST), 1, 5); +} + static int rz_ssi_stop(struct rz_ssi_priv *ssi, struct rz_ssi_stream *strm) { strm->running = 0; @@ -782,14 +791,6 @@ static int rz_ssi_dai_trigger(struct snd_pcm_substream *substream, int cmd, switch (cmd) { case SNDRV_PCM_TRIGGER_START: - /* Soft Reset */ - if (!rz_ssi_is_stream_running(&ssi->playback) && - !rz_ssi_is_stream_running(&ssi->capture)) { - rz_ssi_reg_mask_setl(ssi, SSIFCR, 0, SSIFCR_SSIRST); - rz_ssi_reg_mask_setl(ssi, SSIFCR, SSIFCR_SSIRST, 0); - udelay(5); - } - rz_ssi_stream_init(strm, substream); if (ssi->dma_rt) { @@ -914,6 +915,7 @@ static int rz_ssi_dai_hw_params(struct snd_pcm_substream *substream, SNDRV_PCM_HW_PARAM_SAMPLE_BITS)->min; unsigned int channels = params_channels(params); unsigned int rate = params_rate(params); + int ret; if (sample_bits != 16) { dev_err(ssi->dev, "Unsupported sample width: %d\n", @@ -940,6 +942,10 @@ static int rz_ssi_dai_hw_params(struct snd_pcm_substream *substream, rz_ssi_cache_hw_params(ssi, rate, channels, strm->sample_width, sample_bits); + ret = rz_ssi_swreset(ssi); + if (ret) + return ret; + return rz_ssi_clk_setup(ssi, rate, channels); } From 1fc778f7c833aeb13041adc06f016f1a2dff7350 Mon Sep 17 00:00:00 2001 From: Claudiu Beznea Date: Tue, 10 Dec 2024 19:09:46 +0200 Subject: [PATCH 14/16] ASoC: renesas: rz-ssi: Add suspend to RAM support The SSIF-2 IP is available on the Renesas RZ/G3S SoC. The Renesas RZ/G3S SoC supports a power-saving mode where power to most of the SoC components is turned off. Add suspend/resume support to the SSIF-2 driver to support this power-saving mode. On SNDRV_PCM_TRIGGER_SUSPEND trigger the SSI is stopped (the stream user pointer is left untouched to avoid breaking user space and the dma buffer pointer is set to zero), on SNDRV_PCM_TRIGGER_RESUME software reset is issued for the SSIF-2 IP and the clocks are re-configured. Signed-off-by: Claudiu Beznea Link: https://patch.msgid.link/20241210170953.2936724-18-claudiu.beznea.uj@bp.renesas.com Signed-off-by: Mark Brown --- sound/soc/renesas/rz-ssi.c | 46 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/sound/soc/renesas/rz-ssi.c b/sound/soc/renesas/rz-ssi.c index 486822d79458..d48e2e7356b6 100644 --- a/sound/soc/renesas/rz-ssi.c +++ b/sound/soc/renesas/rz-ssi.c @@ -782,6 +782,32 @@ no_dma: return -ENODEV; } +static int rz_ssi_trigger_resume(struct rz_ssi_priv *ssi) +{ + int ret; + + if (rz_ssi_is_stream_running(&ssi->playback) || + rz_ssi_is_stream_running(&ssi->capture)) + return 0; + + ret = rz_ssi_swreset(ssi); + if (ret) + return ret; + + return rz_ssi_clk_setup(ssi, ssi->hw_params_cache.rate, + ssi->hw_params_cache.channels); +} + +static void rz_ssi_streams_suspend(struct rz_ssi_priv *ssi) +{ + if (rz_ssi_is_stream_running(&ssi->playback) || + rz_ssi_is_stream_running(&ssi->capture)) + return; + + ssi->playback.dma_buffer_pos = 0; + ssi->capture.dma_buffer_pos = 0; +} + static int rz_ssi_dai_trigger(struct snd_pcm_substream *substream, int cmd, struct snd_soc_dai *dai) { @@ -790,8 +816,16 @@ static int rz_ssi_dai_trigger(struct snd_pcm_substream *substream, int cmd, int ret = 0, i, num_transfer = 1; switch (cmd) { + case SNDRV_PCM_TRIGGER_RESUME: + ret = rz_ssi_trigger_resume(ssi); + if (ret) + return ret; + + fallthrough; + case SNDRV_PCM_TRIGGER_START: - rz_ssi_stream_init(strm, substream); + if (cmd == SNDRV_PCM_TRIGGER_START) + rz_ssi_stream_init(strm, substream); if (ssi->dma_rt) { bool is_playback; @@ -819,6 +853,12 @@ static int rz_ssi_dai_trigger(struct snd_pcm_substream *substream, int cmd, ret = rz_ssi_start(ssi, strm); break; + + case SNDRV_PCM_TRIGGER_SUSPEND: + rz_ssi_stop(ssi, strm); + rz_ssi_streams_suspend(ssi); + break; + case SNDRV_PCM_TRIGGER_STOP: rz_ssi_stop(ssi, strm); rz_ssi_stream_quit(ssi, strm); @@ -958,7 +998,8 @@ static const struct snd_soc_dai_ops rz_ssi_dai_ops = { static const struct snd_pcm_hardware rz_ssi_pcm_hardware = { .info = SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_MMAP | - SNDRV_PCM_INFO_MMAP_VALID, + SNDRV_PCM_INFO_MMAP_VALID | + SNDRV_PCM_INFO_RESUME, .buffer_bytes_max = PREALLOC_BUFFER, .period_bytes_min = 32, .period_bytes_max = 8192, @@ -1201,6 +1242,7 @@ static int rz_ssi_runtime_resume(struct device *dev) static const struct dev_pm_ops rz_ssi_pm_ops = { RUNTIME_PM_OPS(rz_ssi_runtime_suspend, rz_ssi_runtime_resume, NULL) + SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume) }; static struct platform_driver rz_ssi_driver = { From c28dac5d3a6e87615d4c8e50f574c320172a3d55 Mon Sep 17 00:00:00 2001 From: Claudiu Beznea Date: Tue, 10 Dec 2024 19:09:47 +0200 Subject: [PATCH 15/16] ASoC: dt-bindings: renesas,rz-ssi: Remove DMA description Remove the DMA description, as it duplicates content from ../dma/renesas,rz-dma.yaml. Additionally, remove the MID/RID examples mentioned in the dropped description (this information is already documented in the hardware manual). Suggested-by: Geert Uytterhoeven Signed-off-by: Claudiu Beznea Link: https://patch.msgid.link/20241210170953.2936724-19-claudiu.beznea.uj@bp.renesas.com Signed-off-by: Mark Brown --- .../bindings/sound/renesas,rz-ssi.yaml | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/Documentation/devicetree/bindings/sound/renesas,rz-ssi.yaml b/Documentation/devicetree/bindings/sound/renesas,rz-ssi.yaml index f4610eaed1e1..5b42eec864f8 100644 --- a/Documentation/devicetree/bindings/sound/renesas,rz-ssi.yaml +++ b/Documentation/devicetree/bindings/sound/renesas,rz-ssi.yaml @@ -57,24 +57,6 @@ properties: dmas: minItems: 1 maxItems: 2 - description: - The first cell represents a phandle to dmac. - The second cell specifies the encoded MID/RID values of the SSI port - connected to the DMA client and the slave channel configuration - parameters. - bits[0:9] - Specifies MID/RID value of a SSI channel as below - MID/RID value of SSI rx0 = 0x256 - MID/RID value of SSI tx0 = 0x255 - MID/RID value of SSI rx1 = 0x25a - MID/RID value of SSI tx1 = 0x259 - MID/RID value of SSI rt2 = 0x25f - MID/RID value of SSI rx3 = 0x262 - MID/RID value of SSI tx3 = 0x261 - bit[10] - HIEN = 1, Detects a request in response to the rising edge - of the signal - bit[11] - LVL = 0, Detects based on the edge - bits[12:14] - AM = 2, Bus cycle mode - bit[15] - TM = 0, Single transfer mode dma-names: oneOf: From 699a9733a354d74482ae4d4304acdbb0c0318a23 Mon Sep 17 00:00:00 2001 From: Claudiu Beznea Date: Tue, 10 Dec 2024 19:09:48 +0200 Subject: [PATCH 16/16] ASoC: dt-bindings: renesas,rz-ssi: Document the Renesas RZ/G3S SoC The SSI IP variant present on the Renesas RZ/G3S SoC is similar to the one found on the Renesas RZ/G2{UL, L, LC} SoCs. Add documentation for it. Acked-by: Krzysztof Kozlowski Signed-off-by: Claudiu Beznea Reviewed-by: Geert Uytterhoeven Link: https://patch.msgid.link/20241210170953.2936724-20-claudiu.beznea.uj@bp.renesas.com Signed-off-by: Mark Brown --- Documentation/devicetree/bindings/sound/renesas,rz-ssi.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/sound/renesas,rz-ssi.yaml b/Documentation/devicetree/bindings/sound/renesas,rz-ssi.yaml index 5b42eec864f8..e4cdbf2202b9 100644 --- a/Documentation/devicetree/bindings/sound/renesas,rz-ssi.yaml +++ b/Documentation/devicetree/bindings/sound/renesas,rz-ssi.yaml @@ -19,6 +19,7 @@ properties: - renesas,r9a07g043-ssi # RZ/G2UL and RZ/Five - renesas,r9a07g044-ssi # RZ/G2{L,LC} - renesas,r9a07g054-ssi # RZ/V2L + - renesas,r9a08g045-ssi # RZ/G3S - const: renesas,rz-ssi reg: