[PATCH 0/5] ASoC: rsnd: tidyup Renesas sound

[PATCH 0/5] ASoC: rsnd: tidyup Renesas sound

[Kuninori Morimoto]

Hi Mark

These are tidyup patches for Renesas sound drivers.

Kuninori Morimoto (5):
  ASoC: rsnd: call rsnd_ssi_master_clk_start() from rsnd_ssi_init()
  ASoC: rsnd: check all BUSIF status when error
  ASoC: rsnd: add rsnd_ssi_busif_err_status_clear()
  ASoC: rsnd: add rsnd_ssi_busif_err_irq_enable/disable()
  ASoC: rsnd: add usage for SRC

 sound/soc/sh/rcar/src.c |   9 ++
 sound/soc/sh/rcar/ssi.c | 225 ++++++++++++++++++----------------------
 2 files changed, 110 insertions(+), 124 deletions(-)

-- 
2.25.1

[PATCH 1/5] ASoC: rsnd: call rsnd_ssi_master_clk_start() from rsnd_ssi_init()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current rsnd needs to call .prepare (P) for clock settings,
.trigger for playback start (S) and stop (E).
It should be called as below from SSI point of view.

	P -> S -> E -> P -> S -> E -> ...

But, if you used MIXer, below case might happen

	              (2)
	1: P -> S ---> E -> ...
	2:         P ----> S -> ...
	          (1)     (3)

P(1) setups clock, but E(2) resets it. and starts playback (3).
In such case, it will reports "SSI parent/child should use same rate".

rsnd_ssi_master_clk_start() which is the main function at (P)
was called from rsnd_ssi_init() (= S) before,
but was moved by below patch to rsnd_soc_dai_prepare() (= P) to avoid
using clk_get_rate() which shouldn't be used under atomic context.

	commit 4d230d1271064 ("ASoC: rsnd: fixup not to call clk_get/set
				under non-atomic")

Because of above patch, rsnd_ssi_master_clk_start() is now called at (P)
which is for non atomic context. But (P) is assuming that spin lock is
*not* used.
One issue now is rsnd_ssi_master_clk_start() is checking ssi->xxx
which should be protected by spin lock.

After above patch, adg.c had below patch for other reasons.

	commit 06e8f5c842f2d ("ASoC: rsnd: don't call clk_get_rate()
				under atomic context")

clk_get_rate() is used at probe() timing by this patch.
In other words, rsnd_ssi_master_clk_start() is no longer using
clk_get_rate() any more.

This means we can call it from rsnd_ssi_init() (= S) again which is
protected by spin lock.
This patch re-move it to under spin lock, and solves
1. checking ssi->xxx without spin lock issue.
2. clk setting / device start / device stop race condition.

Reported-by: Linh Phung T. Y. <linh.phung.jy@renesas.com>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/sh/rcar/ssi.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
index d071cec25f71..048d53566127 100644
--- a/sound/soc/sh/rcar/ssi.c
+++ b/sound/soc/sh/rcar/ssi.c
@@ -506,10 +506,15 @@ static int rsnd_ssi_init(struct rsnd_mod *mod,
 			 struct rsnd_priv *priv)
 {
 	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
+	int ret;
 
 	if (!rsnd_ssi_is_run_mods(mod, io))
 		return 0;
 
+	ret = rsnd_ssi_master_clk_start(mod, io);
+	if (ret < 0)
+		return ret;
+
 	ssi->usrcnt++;
 
 	rsnd_mod_power_on(mod);
@@ -1060,13 +1065,6 @@ static int rsnd_ssi_pio_pointer(struct rsnd_mod *mod,
 	return 0;
 }
 
-static int rsnd_ssi_prepare(struct rsnd_mod *mod,
-			    struct rsnd_dai_stream *io,
-			    struct rsnd_priv *priv)
-{
-	return rsnd_ssi_master_clk_start(mod, io);
-}
-
 static struct rsnd_mod_ops rsnd_ssi_pio_ops = {
 	.name		= SSI_NAME,
 	.probe		= rsnd_ssi_common_probe,
@@ -1079,7 +1077,6 @@ static struct rsnd_mod_ops rsnd_ssi_pio_ops = {
 	.pointer	= rsnd_ssi_pio_pointer,
 	.pcm_new	= rsnd_ssi_pcm_new,
 	.hw_params	= rsnd_ssi_hw_params,
-	.prepare	= rsnd_ssi_prepare,
 	.get_status	= rsnd_ssi_get_status,
 };
 
@@ -1166,7 +1163,6 @@ static struct rsnd_mod_ops rsnd_ssi_dma_ops = {
 	.pcm_new	= rsnd_ssi_pcm_new,
 	.fallback	= rsnd_ssi_fallback,
 	.hw_params	= rsnd_ssi_hw_params,
-	.prepare	= rsnd_ssi_prepare,
 	.get_status	= rsnd_ssi_get_status,
 };
 
-- 
2.25.1

[PATCH 2/5] ASoC: rsnd: check all BUSIF status when error

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

commit 66c705d07d784 ("SoC: rsnd: add interrupt support for SSI BUSIF
buffer") adds __rsnd_ssi_interrupt() checks for BUSIF status,
but is using "break" at for loop.
This means it is not checking all status. Let's check all BUSIF status.

Fixes: commit 66c705d07d784 ("SoC: rsnd: add interrupt support for SSI BUSIF buffer")
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/sh/rcar/ssi.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
index 048d53566127..9a3310393989 100644
--- a/sound/soc/sh/rcar/ssi.c
+++ b/sound/soc/sh/rcar/ssi.c
@@ -797,7 +797,6 @@ static void __rsnd_ssi_interrupt(struct rsnd_mod *mod,
 						       SSI_SYS_STATUS(i * 2),
 						       0xf << (id * 4));
 					stop = true;
-					break;
 				}
 			}
 			break;
@@ -815,7 +814,6 @@ static void __rsnd_ssi_interrupt(struct rsnd_mod *mod,
 						SSI_SYS_STATUS((i * 2) + 1),
 						0xf << 4);
 					stop = true;
-					break;
 				}
 			}
 			break;
-- 
2.25.1

[PATCH 3/5] ASoC: rsnd: add rsnd_ssi_busif_err_status_clear()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current ssi.c clears BUSIF error status at __rsnd_ssi_interrupt(),
but its code is verbose.
This patch off-load it to rsnd_ssi_busif_err_status_clear().

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/sh/rcar/ssi.c | 97 ++++++++++++++++++++++-------------------
 1 file changed, 51 insertions(+), 46 deletions(-)

diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
index 9a3310393989..4af024917f03 100644
--- a/sound/soc/sh/rcar/ssi.c
+++ b/sound/soc/sh/rcar/ssi.c
@@ -359,6 +359,55 @@ static void rsnd_ssi_master_clk_stop(struct rsnd_mod *mod,
 	rsnd_adg_ssi_clk_stop(mod);
 }
 
+static bool rsnd_ssi_busif_err_status_clear(struct rsnd_mod *mod)
+{
+	struct rsnd_priv *priv = rsnd_mod_to_priv(mod);
+	struct device *dev = rsnd_priv_to_dev(priv);
+	u32 status;
+	bool stop = false;
+	int id = rsnd_mod_id(mod);
+	int i;
+
+	switch (id) {
+	case 0:
+	case 1:
+	case 2:
+	case 3:
+	case 4:
+		for (i = 0; i < 4; i++) {
+			status = rsnd_mod_read(mod, SSI_SYS_STATUS(i * 2));
+			status &= 0xf << (id * 4);
+
+			if (status) {
+				rsnd_dbg_irq_status(dev, "%s err status : 0x%08x\n",
+						    rsnd_mod_name(mod), status);
+				rsnd_mod_write(mod,
+					       SSI_SYS_STATUS(i * 2),
+					       0xf << (id * 4));
+				stop = true;
+			}
+		}
+		break;
+	case 9:
+		for (i = 0; i < 4; i++) {
+			status = rsnd_mod_read(mod, SSI_SYS_STATUS((i * 2) + 1));
+			status &= 0xf << 4;
+
+			if (status) {
+				rsnd_dbg_irq_status(dev, "%s err status : 0x%08x\n",
+						    rsnd_mod_name(mod), status);
+				rsnd_mod_write(mod,
+					       SSI_SYS_STATUS((i * 2) + 1),
+					       0xf << 4);
+				stop = true;
+			}
+		}
+		break;
+	}
+
+	return stop;
+}
+
 static void rsnd_ssi_config_init(struct rsnd_mod *mod,
 				struct rsnd_dai_stream *io)
 {
@@ -748,8 +797,6 @@ static void __rsnd_ssi_interrupt(struct rsnd_mod *mod,
 	u32 status;
 	bool elapsed = false;
 	bool stop = false;
-	int id = rsnd_mod_id(mod);
-	int i;
 	int is_tdm, is_tdm_split;
 
 	is_tdm		= rsnd_runtime_is_tdm(io);
@@ -775,50 +822,8 @@ static void __rsnd_ssi_interrupt(struct rsnd_mod *mod,
 		stop = true;
 	}
 
-	status = 0;
-
-	if (is_tdm || is_tdm_split) {
-		switch (id) {
-		case 0:
-		case 1:
-		case 2:
-		case 3:
-		case 4:
-			for (i = 0; i < 4; i++) {
-				status = rsnd_mod_read(mod,
-						       SSI_SYS_STATUS(i * 2));
-				status &= 0xf << (id * 4);
-
-				if (status) {
-					rsnd_dbg_irq_status(dev,
-						"%s err status : 0x%08x\n",
-						rsnd_mod_name(mod), status);
-					rsnd_mod_write(mod,
-						       SSI_SYS_STATUS(i * 2),
-						       0xf << (id * 4));
-					stop = true;
-				}
-			}
-			break;
-		case 9:
-			for (i = 0; i < 4; i++) {
-				status = rsnd_mod_read(mod,
-						SSI_SYS_STATUS((i * 2) + 1));
-				status &= 0xf << 4;
-
-				if (status) {
-					rsnd_dbg_irq_status(dev,
-						"%s err status : 0x%08x\n",
-						rsnd_mod_name(mod), status);
-					rsnd_mod_write(mod,
-						SSI_SYS_STATUS((i * 2) + 1),
-						0xf << 4);
-					stop = true;
-				}
-			}
-			break;
-		}
-	}
+	if (is_tdm || is_tdm_split)
+		stop |= rsnd_ssi_busif_err_status_clear(mod);
 
 	rsnd_ssi_status_clear(mod);
 rsnd_ssi_interrupt_out:
-- 
2.25.1

[PATCH 4/5] ASoC: rsnd: add rsnd_ssi_busif_err_irq_enable/disable()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current ssi.c has duplicated code to control BUSIF
over/under run interrupt.
This patch adds new rsnd_ssi_busif_err_irq_enable/disable()
and share the code.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Cc: Yongbo Zhang <giraffesnn123@gmail.com>
Cc: Chen Li <licheng0822@thundersoft.com>
---
 sound/soc/sh/rcar/ssi.c | 112 ++++++++++++++++------------------------
 1 file changed, 45 insertions(+), 67 deletions(-)

diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
index 4af024917f03..e29482c26d6a 100644
--- a/sound/soc/sh/rcar/ssi.c
+++ b/sound/soc/sh/rcar/ssi.c
@@ -359,6 +359,47 @@ static void rsnd_ssi_master_clk_stop(struct rsnd_mod *mod,
 	rsnd_adg_ssi_clk_stop(mod);
 }
 
+/* enable busif buffer over/under run interrupt. */
+#define rsnd_ssi_busif_err_irq_enable(mod)  rsnd_ssi_busif_err_irq_ctrl(mod, 1)
+#define rsnd_ssi_busif_err_irq_disable(mod) rsnd_ssi_busif_err_irq_ctrl(mod, 0)
+static void rsnd_ssi_busif_err_irq_ctrl(struct rsnd_mod *mod, int enable)
+{
+	u32 sys_int_enable = 0;
+	int id = rsnd_mod_id(mod);
+	int i;
+
+	switch (id) {
+	case 0:
+	case 1:
+	case 2:
+	case 3:
+	case 4:
+		for (i = 0; i < 4; i++) {
+			sys_int_enable = rsnd_mod_read(mod, SSI_SYS_INT_ENABLE(i * 2));
+			if (enable)
+				sys_int_enable |= 0xf << (id * 4);
+			else
+				sys_int_enable &= ~(0xf << (id * 4));
+			rsnd_mod_write(mod,
+				       SSI_SYS_INT_ENABLE(i * 2),
+				       sys_int_enable);
+		}
+		break;
+	case 9:
+		for (i = 0; i < 4; i++) {
+			sys_int_enable = rsnd_mod_read(mod, SSI_SYS_INT_ENABLE((i * 2) + 1));
+			if (enable)
+				sys_int_enable |= 0xf << 4;
+			else
+				sys_int_enable &= ~(0xf << 4);
+			rsnd_mod_write(mod,
+				       SSI_SYS_INT_ENABLE((i * 2) + 1),
+				       sys_int_enable);
+		}
+		break;
+	}
+}
+
 static bool rsnd_ssi_busif_err_status_clear(struct rsnd_mod *mod)
 {
 	struct rsnd_priv *priv = rsnd_mod_to_priv(mod);
@@ -421,9 +462,6 @@ static void rsnd_ssi_config_init(struct rsnd_mod *mod,
 	u32 wsr		= ssi->wsr;
 	int width;
 	int is_tdm, is_tdm_split;
-	int id = rsnd_mod_id(mod);
-	int i;
-	u32 sys_int_enable = 0;
 
 	is_tdm		= rsnd_runtime_is_tdm(io);
 	is_tdm_split	= rsnd_runtime_is_tdm_split(io);
@@ -499,36 +537,8 @@ static void rsnd_ssi_config_init(struct rsnd_mod *mod,
 	}
 
 	/* enable busif buffer over/under run interrupt. */
-	if (is_tdm || is_tdm_split) {
-		switch (id) {
-		case 0:
-		case 1:
-		case 2:
-		case 3:
-		case 4:
-			for (i = 0; i < 4; i++) {
-				sys_int_enable = rsnd_mod_read(mod,
-					SSI_SYS_INT_ENABLE(i * 2));
-				sys_int_enable |= 0xf << (id * 4);
-				rsnd_mod_write(mod,
-					       SSI_SYS_INT_ENABLE(i * 2),
-					       sys_int_enable);
-			}
-
-			break;
-		case 9:
-			for (i = 0; i < 4; i++) {
-				sys_int_enable = rsnd_mod_read(mod,
-					SSI_SYS_INT_ENABLE((i * 2) + 1));
-				sys_int_enable |= 0xf << 4;
-				rsnd_mod_write(mod,
-					       SSI_SYS_INT_ENABLE((i * 2) + 1),
-					       sys_int_enable);
-			}
-
-			break;
-		}
-	}
+	if (is_tdm || is_tdm_split)
+		rsnd_ssi_busif_err_irq_enable(mod);
 
 init_end:
 	ssi->cr_own	= cr_own;
@@ -585,8 +595,6 @@ static int rsnd_ssi_quit(struct rsnd_mod *mod,
 	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
 	struct device *dev = rsnd_priv_to_dev(priv);
 	int is_tdm, is_tdm_split;
-	int id = rsnd_mod_id(mod);
-	u32 sys_int_enable = 0;
 
 	is_tdm		= rsnd_runtime_is_tdm(io);
 	is_tdm_split	= rsnd_runtime_is_tdm_split(io);
@@ -612,38 +620,8 @@ static int rsnd_ssi_quit(struct rsnd_mod *mod,
 	}
 
 	/* disable busif buffer over/under run interrupt. */
-	if (is_tdm || is_tdm_split) {
-		int i;
-
-		switch (id) {
-		case 0:
-		case 1:
-		case 2:
-		case 3:
-		case 4:
-			for (i = 0; i < 4; i++) {
-				sys_int_enable = rsnd_mod_read(mod,
-						SSI_SYS_INT_ENABLE(i * 2));
-				sys_int_enable &= ~(0xf << (id * 4));
-				rsnd_mod_write(mod,
-					       SSI_SYS_INT_ENABLE(i * 2),
-					       sys_int_enable);
-			}
-
-			break;
-		case 9:
-			for (i = 0; i < 4; i++) {
-				sys_int_enable = rsnd_mod_read(mod,
-					SSI_SYS_INT_ENABLE((i * 2) + 1));
-				sys_int_enable &= ~(0xf << 4);
-				rsnd_mod_write(mod,
-					       SSI_SYS_INT_ENABLE((i * 2) + 1),
-					       sys_int_enable);
-			}
-
-			break;
-		}
-	}
+	if (is_tdm || is_tdm_split)
+		rsnd_ssi_busif_err_irq_disable(mod);
 
 	return 0;
 }
-- 
2.25.1

[PATCH 5/5] ASoC: rsnd: add usage for SRC

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

This patch add missing usage comment for SRC.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/sh/rcar/src.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/sound/soc/sh/rcar/src.c b/sound/soc/sh/rcar/src.c
index 585ffba0244b..628af8f3920d 100644
--- a/sound/soc/sh/rcar/src.c
+++ b/sound/soc/sh/rcar/src.c
@@ -5,6 +5,15 @@
 // Copyright (C) 2013 Renesas Solutions Corp.
 // Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
 
+/*
+ * You can use Synchronous Sampling Rate Convert (if no DVC)
+ *
+ *	amixer set "SRC Out Rate" on
+ *	aplay xxx.wav &
+ *	amixer set "SRC Out Rate" 96000 // convert rate to 96000Hz
+ *	amixer set "SRC Out Rate" 22050 // convert rate to 22050Hz
+ */
+
 /*
  * you can enable below define if you don't need
  * SSI interrupt status debug message when debugging
-- 
2.25.1

Re: [PATCH 0/5] ASoC: rsnd: tidyup Renesas sound

[Mark Brown]

On 08 Apr 2021 13:27:54 +0900, Kuninori Morimoto wrote:
> These are tidyup patches for Renesas sound drivers.
> 
> Kuninori Morimoto (5):
>   ASoC: rsnd: call rsnd_ssi_master_clk_start() from rsnd_ssi_init()
>   ASoC: rsnd: check all BUSIF status when error
>   ASoC: rsnd: add rsnd_ssi_busif_err_status_clear()
>   ASoC: rsnd: add rsnd_ssi_busif_err_irq_enable/disable()
>   ASoC: rsnd: add usage for SRC
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/5] ASoC: rsnd: call rsnd_ssi_master_clk_start() from rsnd_ssi_init()
      commit: a122a116fc6d8fcf2f202dcd185173a54268f239
[2/5] ASoC: rsnd: check all BUSIF status when error
      commit: a4856e15e58b54977f1c0c0299309ad4d1f13365
[3/5] ASoC: rsnd: add rsnd_ssi_busif_err_status_clear()
      commit: 691b379cbe348fbead33e49d1c1d0f045a2e3446
[4/5] ASoC: rsnd: add rsnd_ssi_busif_err_irq_enable/disable()
      commit: 15c57ce07ce207069f1cd57ec117b11871f3afa6
[5/5] ASoC: rsnd: add usage for SRC
      commit: 63346d3d2f7cd96746149b9710a4cc5401c0cb5c

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark