[PATCH v1 0/3] Allow reconfiguration of clock rate

[PATCH v1 0/3] Allow reconfiguration of clock rate

[Jiada Wang]

There is use case in Gstreamer ALSA sink, in case of changed caps
Gsreatmer reconfigs and it calls snd_pcm_hw_free() before snd_pcm_prepre().
See gstreamer1.0-plugins-base/
gst-libs/gst/audio/gstaudiobasesink.c: gst_audio_base_sink_setcaps():
- gst_audio_ring_buffer_release()
- gst_audio_sink_ring_buffer_release()
- gst_alsasink_unprepare()
- snd_pcm_hw_free()
is called before
- gst_audio_ring_buffer_acquire()
- gst_audio_sink_ring_buffer_acquire()
- gst_alsasink_prepare()
- set_hwparams()
- snd_pcm_hw_params()
- snd_pcm_prepare()

But with current implementation after clock rate is started in .prepare
reconfiguration of clock rate is not allowed, unless the stream is stopped.

This patch set by move stop of clock to .hw_free callback,
to allow reconfiguration of clock rate.

Jiada Wang (1):
  ASoC: rsnd: call .hw_{params,free} in pair for same stream

Timo Wischer (2):
  ASoC: rsnd: Support hw_free() callback at DAI level
  ASoC: rsnd: Allow reconfiguration of clock rate

 sound/soc/sh/rcar/core.c | 22 +++++++++++++--
 sound/soc/sh/rcar/rsnd.h | 36 ++++++++++++++++++++----
 sound/soc/sh/rcar/ssi.c  | 61 +++++++++++++++++++++++++++++-----------
 sound/soc/sh/rcar/ssiu.c |  3 +-
 4 files changed, 96 insertions(+), 26 deletions(-)

-- 
2.19.2

[PATCH v1 0/3] Allow reconfiguration of clock rate

[Jiada Wang]

There is use case in Gstreamer ALSA sink, in case of changed caps
Gsreatmer reconfigs and it calls snd_pcm_hw_free() before snd_pcm_prepre().
See gstreamer1.0-plugins-base/
gst-libs/gst/audio/gstaudiobasesink.c: gst_audio_base_sink_setcaps():
- gst_audio_ring_buffer_release()
- gst_audio_sink_ring_buffer_release()
- gst_alsasink_unprepare()
- snd_pcm_hw_free()
is called before
- gst_audio_ring_buffer_acquire()
- gst_audio_sink_ring_buffer_acquire()
- gst_alsasink_prepare()
- set_hwparams()
- snd_pcm_hw_params()
- snd_pcm_prepare()

But with current implementation after clock rate is started in .prepare
reconfiguration of clock rate is not allowed, unless the stream is stopped.

This patch set by move stop of clock to .hw_free callback,
to allow reconfiguration of clock rate.

Jiada Wang (1):
  ASoC: rsnd: call .hw_{params,free} in pair for same stream

Timo Wischer (2):
  ASoC: rsnd: Support hw_free() callback at DAI level
  ASoC: rsnd: Allow reconfiguration of clock rate

 sound/soc/sh/rcar/core.c | 22 +++++++++++++--
 sound/soc/sh/rcar/rsnd.h | 36 ++++++++++++++++++++----
 sound/soc/sh/rcar/ssi.c  | 61 +++++++++++++++++++++++++++++-----------
 sound/soc/sh/rcar/ssiu.c |  3 +-
 4 files changed, 96 insertions(+), 26 deletions(-)

-- 
2.19.2

[PATCH v1 1/3] ASoC: rsnd: Support hw_free() callback at DAI level

[Jiada Wang]

From: Timo Wischer <twischer@de.adit-jv.com>

This patch provides the needed infrastructure to support calling hw_free()
at the DAI level. This is for example required to free resources allocated
in hw_params() callback.

The modification of __rsnd_mod_add_hw_params does not have any side
effects because rsnd_mod_ops::hw_params callback is not used by anyone
until now.

Signed-off-by: Timo Wischer <twischer@de.adit-jv.com>
Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 sound/soc/sh/rcar/core.c | 16 +++++++++++++++-
 sound/soc/sh/rcar/rsnd.h | 12 +++++++++---
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c
index 56e8dae9a15c..bda5b958d0dc 100644
--- a/sound/soc/sh/rcar/core.c
+++ b/sound/soc/sh/rcar/core.c
@@ -1421,6 +1421,20 @@ static int rsnd_hw_params(struct snd_pcm_substream *substream,
 					params_buffer_bytes(hw_params));
 }
 
+static int rsnd_hw_free(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_dai *dai = rsnd_substream_to_dai(substream);
+	struct rsnd_dai *rdai = rsnd_dai_to_rdai(dai);
+	struct rsnd_dai_stream *io = rsnd_rdai_to_io(rdai, substream);
+	int ret;
+
+	ret = rsnd_dai_call(hw_free, io, substream);
+	if (ret)
+		return ret;
+
+	return snd_pcm_lib_free_pages(substream);
+}
+
 static snd_pcm_uframes_t rsnd_pointer(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_dai *dai = rsnd_substream_to_dai(substream);
@@ -1436,7 +1450,7 @@ static snd_pcm_uframes_t rsnd_pointer(struct snd_pcm_substream *substream)
 static const struct snd_pcm_ops rsnd_pcm_ops = {
 	.ioctl		= snd_pcm_lib_ioctl,
 	.hw_params	= rsnd_hw_params,
-	.hw_free	= snd_pcm_lib_free_pages,
+	.hw_free	= rsnd_hw_free,
 	.pointer	= rsnd_pointer,
 };
 
diff --git a/sound/soc/sh/rcar/rsnd.h b/sound/soc/sh/rcar/rsnd.h
index 7727add3eb1a..ea6cbaa9743e 100644
--- a/sound/soc/sh/rcar/rsnd.h
+++ b/sound/soc/sh/rcar/rsnd.h
@@ -327,6 +327,9 @@ struct rsnd_mod_ops {
 	int (*cleanup)(struct rsnd_mod *mod,
 		       struct rsnd_dai_stream *io,
 		       struct rsnd_priv *priv);
+	int (*hw_free)(struct rsnd_mod *mod,
+		       struct rsnd_dai_stream *io,
+		       struct snd_pcm_substream *substream);
 	u32 *(*get_status)(struct rsnd_mod *mod,
 			   struct rsnd_dai_stream *io,
 			   enum rsnd_mod_type type);
@@ -351,12 +354,12 @@ struct rsnd_mod {
  *
  * B	0: init		1: quit
  * C	0: start	1: stop
+ * D	0: hw_params	1: hw_free
  *
  * H is always called (see __rsnd_mod_call)
  * H	0: probe	1: remove
  * H	0: pcm_new
  * H	0: fallback
- * H	0: hw_params
  * H	0: pointer
  * H	0: prepare
  * H	0: cleanup
@@ -365,12 +368,13 @@ struct rsnd_mod {
 #define __rsnd_mod_shift_quit		4
 #define __rsnd_mod_shift_start		8
 #define __rsnd_mod_shift_stop		8
+#define __rsnd_mod_shift_hw_params	12
+#define __rsnd_mod_shift_hw_free	12
 #define __rsnd_mod_shift_probe		28 /* always called */
 #define __rsnd_mod_shift_remove		28 /* always called */
 #define __rsnd_mod_shift_irq		28 /* always called */
 #define __rsnd_mod_shift_pcm_new	28 /* always called */
 #define __rsnd_mod_shift_fallback	28 /* always called */
-#define __rsnd_mod_shift_hw_params	28 /* always called */
 #define __rsnd_mod_shift_pointer	28 /* always called */
 #define __rsnd_mod_shift_prepare	28 /* always called */
 #define __rsnd_mod_shift_cleanup	28 /* always called */
@@ -383,10 +387,11 @@ struct rsnd_mod {
 #define __rsnd_mod_add_quit		-1
 #define __rsnd_mod_add_start		 1
 #define __rsnd_mod_add_stop		-1
+#define __rsnd_mod_add_hw_params	1
+#define __rsnd_mod_add_hw_free		-1
 #define __rsnd_mod_add_irq		0
 #define __rsnd_mod_add_pcm_new		0
 #define __rsnd_mod_add_fallback		0
-#define __rsnd_mod_add_hw_params	0
 #define __rsnd_mod_add_pointer		0
 
 #define __rsnd_mod_call_probe		0
@@ -402,6 +407,7 @@ struct rsnd_mod {
 #define __rsnd_mod_call_fallback	0
 #define __rsnd_mod_call_hw_params	0
 #define __rsnd_mod_call_pointer		0
+#define __rsnd_mod_call_hw_free		1
 
 #define rsnd_mod_to_priv(mod)	((mod)->priv)
 #define rsnd_mod_power_on(mod)	clk_enable((mod)->clk)
-- 
2.19.2

[PATCH v1 1/3] ASoC: rsnd: Support hw_free() callback at DAI level

[Jiada Wang]

From: Timo Wischer <twischer@de.adit-jv.com>

This patch provides the needed infrastructure to support calling hw_free()
at the DAI level. This is for example required to free resources allocated
in hw_params() callback.

The modification of __rsnd_mod_add_hw_params does not have any side
effects because rsnd_mod_ops::hw_params callback is not used by anyone
until now.

Signed-off-by: Timo Wischer <twischer@de.adit-jv.com>
Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 sound/soc/sh/rcar/core.c | 16 +++++++++++++++-
 sound/soc/sh/rcar/rsnd.h | 12 +++++++++---
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c
index 56e8dae9a15c..bda5b958d0dc 100644
--- a/sound/soc/sh/rcar/core.c
+++ b/sound/soc/sh/rcar/core.c
@@ -1421,6 +1421,20 @@ static int rsnd_hw_params(struct snd_pcm_substream *substream,
 					params_buffer_bytes(hw_params));
 }
 
+static int rsnd_hw_free(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_dai *dai = rsnd_substream_to_dai(substream);
+	struct rsnd_dai *rdai = rsnd_dai_to_rdai(dai);
+	struct rsnd_dai_stream *io = rsnd_rdai_to_io(rdai, substream);
+	int ret;
+
+	ret = rsnd_dai_call(hw_free, io, substream);
+	if (ret)
+		return ret;
+
+	return snd_pcm_lib_free_pages(substream);
+}
+
 static snd_pcm_uframes_t rsnd_pointer(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_dai *dai = rsnd_substream_to_dai(substream);
@@ -1436,7 +1450,7 @@ static snd_pcm_uframes_t rsnd_pointer(struct snd_pcm_substream *substream)
 static const struct snd_pcm_ops rsnd_pcm_ops = {
 	.ioctl		= snd_pcm_lib_ioctl,
 	.hw_params	= rsnd_hw_params,
-	.hw_free	= snd_pcm_lib_free_pages,
+	.hw_free	= rsnd_hw_free,
 	.pointer	= rsnd_pointer,
 };
 
diff --git a/sound/soc/sh/rcar/rsnd.h b/sound/soc/sh/rcar/rsnd.h
index 7727add3eb1a..ea6cbaa9743e 100644
--- a/sound/soc/sh/rcar/rsnd.h
+++ b/sound/soc/sh/rcar/rsnd.h
@@ -327,6 +327,9 @@ struct rsnd_mod_ops {
 	int (*cleanup)(struct rsnd_mod *mod,
 		       struct rsnd_dai_stream *io,
 		       struct rsnd_priv *priv);
+	int (*hw_free)(struct rsnd_mod *mod,
+		       struct rsnd_dai_stream *io,
+		       struct snd_pcm_substream *substream);
 	u32 *(*get_status)(struct rsnd_mod *mod,
 			   struct rsnd_dai_stream *io,
 			   enum rsnd_mod_type type);
@@ -351,12 +354,12 @@ struct rsnd_mod {
  *
  * B	0: init		1: quit
  * C	0: start	1: stop
+ * D	0: hw_params	1: hw_free
  *
  * H is always called (see __rsnd_mod_call)
  * H	0: probe	1: remove
  * H	0: pcm_new
  * H	0: fallback
- * H	0: hw_params
  * H	0: pointer
  * H	0: prepare
  * H	0: cleanup
@@ -365,12 +368,13 @@ struct rsnd_mod {
 #define __rsnd_mod_shift_quit		4
 #define __rsnd_mod_shift_start		8
 #define __rsnd_mod_shift_stop		8
+#define __rsnd_mod_shift_hw_params	12
+#define __rsnd_mod_shift_hw_free	12
 #define __rsnd_mod_shift_probe		28 /* always called */
 #define __rsnd_mod_shift_remove		28 /* always called */
 #define __rsnd_mod_shift_irq		28 /* always called */
 #define __rsnd_mod_shift_pcm_new	28 /* always called */
 #define __rsnd_mod_shift_fallback	28 /* always called */
-#define __rsnd_mod_shift_hw_params	28 /* always called */
 #define __rsnd_mod_shift_pointer	28 /* always called */
 #define __rsnd_mod_shift_prepare	28 /* always called */
 #define __rsnd_mod_shift_cleanup	28 /* always called */
@@ -383,10 +387,11 @@ struct rsnd_mod {
 #define __rsnd_mod_add_quit		-1
 #define __rsnd_mod_add_start		 1
 #define __rsnd_mod_add_stop		-1
+#define __rsnd_mod_add_hw_params	1
+#define __rsnd_mod_add_hw_free		-1
 #define __rsnd_mod_add_irq		0
 #define __rsnd_mod_add_pcm_new		0
 #define __rsnd_mod_add_fallback		0
-#define __rsnd_mod_add_hw_params	0
 #define __rsnd_mod_add_pointer		0
 
 #define __rsnd_mod_call_probe		0
@@ -402,6 +407,7 @@ struct rsnd_mod {
 #define __rsnd_mod_call_fallback	0
 #define __rsnd_mod_call_hw_params	0
 #define __rsnd_mod_call_pointer		0
+#define __rsnd_mod_call_hw_free		1
 
 #define rsnd_mod_to_priv(mod)	((mod)->priv)
 #define rsnd_mod_power_on(mod)	clk_enable((mod)->clk)
-- 
2.19.2

[PATCH v1 2/3] ASoC: rsnd: Allow reconfiguration of clock rate

[Jiada Wang]

From: Timo Wischer <twischer@de.adit-jv.com>

Currently after clock rate is started in .prepare reconfiguration
of clock rate is not allowed, unless the stream is stopped.

But there is use case in Gstreamer ALSA sink, in case of changed caps
Gsreatmer reconfigs and it calls snd_pcm_hw_free() before snd_pcm_prepre().
See gstreamer1.0-plugins-base/
gst-libs/gst/audio/gstaudiobasesink.c: gst_audio_base_sink_setcaps():
- gst_audio_ring_buffer_release()
- gst_audio_sink_ring_buffer_release()
- gst_alsasink_unprepare()
- snd_pcm_hw_free()
is called before
- gst_audio_ring_buffer_acquire()
- gst_audio_sink_ring_buffer_acquire()
- gst_alsasink_prepare()
- set_hwparams()
- snd_pcm_hw_params()
- snd_pcm_prepare()

The issue mentioned in this commit can be reproduced with the following
aplay patch:

    >diff --git a/aplay/aplay.c b/aplay/aplay.c
    >@@ -2760,6 +2760,8 @@ static void playback_go(int fd, size_t loaded,
    >      header(rtype, name);
    >      set_params();
    >+     hwparams.rate = (hwparams.rate == 48000) ? 44100 : 48000;
    >+     set_params();
    >
    >      while (loaded > chunk_bytes && written < count && !in_aborting)
    >      {
    >              if (pcm_write(audiobuf + written, chunk_size) <= 0)

$ aplay -Dplughw:0,0,0 -c8 -fS24_LE -r48000 /dev/zero
rcar_sound ec500000.sound: SSI parent/child should use same rate
rcar_sound ec500000.sound: ssi[3] : prepare error -22
rcar_sound ec500000.sound: ASoC: cpu DAI prepare error: -22
rsnd_link0: ASoC: prepare FE rsnd_link0 failed

this patch address the issue by stop clock in .hw_free,
to allow reconfiguration of clock rate without stop of the stream.

Signed-off-by: Timo Wischer <twischer@de.adit-jv.com>
Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 sound/soc/sh/rcar/ssi.c | 53 +++++++++++++++++++++++++++++------------
 1 file changed, 38 insertions(+), 15 deletions(-)

diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
index f6a7466622ea..f43937d2c588 100644
--- a/sound/soc/sh/rcar/ssi.c
+++ b/sound/soc/sh/rcar/ssi.c
@@ -286,7 +286,7 @@ static int rsnd_ssi_master_clk_start(struct rsnd_mod *mod,
 	if (rsnd_ssi_is_multi_slave(mod, io))
 		return 0;
 
-	if (ssi->usrcnt > 0) {
+	if (ssi->rate) {
 		if (ssi->rate != rate) {
 			dev_err(dev, "SSI parent/child should use same rate\n");
 			return -EINVAL;
@@ -471,13 +471,9 @@ static int rsnd_ssi_init(struct rsnd_mod *mod,
 			 struct rsnd_dai_stream *io,
 			 struct rsnd_priv *priv)
 {
-	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
-
 	if (!rsnd_ssi_is_run_mods(mod, io))
 		return 0;
 
-	ssi->usrcnt++;
-
 	rsnd_mod_power_on(mod);
 
 	rsnd_ssi_config_init(mod, io);
@@ -505,18 +501,8 @@ static int rsnd_ssi_quit(struct rsnd_mod *mod,
 		return -EIO;
 	}
 
-	rsnd_ssi_master_clk_stop(mod, io);
-
 	rsnd_mod_power_off(mod);
 
-	ssi->usrcnt--;
-
-	if (!ssi->usrcnt) {
-		ssi->cr_own	= 0;
-		ssi->cr_mode	= 0;
-		ssi->wsr	= 0;
-	}
-
 	return 0;
 }
 
@@ -525,6 +511,7 @@ static int rsnd_ssi_hw_params(struct rsnd_mod *mod,
 			      struct snd_pcm_substream *substream,
 			      struct snd_pcm_hw_params *params)
 {
+	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
 	struct rsnd_dai *rdai = rsnd_io_to_rdai(io);
 	unsigned int fmt_width = snd_pcm_format_width(params_format(params));
 
@@ -536,6 +523,11 @@ static int rsnd_ssi_hw_params(struct rsnd_mod *mod,
 		return -EINVAL;
 	}
 
+	if (!rsnd_ssi_is_run_mods(mod, io))
+		return 0;
+
+	ssi->usrcnt++;
+
 	return 0;
 }
 
@@ -913,6 +905,35 @@ static int rsnd_ssi_prepare(struct rsnd_mod *mod,
 	return rsnd_ssi_master_clk_start(mod, io);
 }
 
+static int rsnd_ssi_hw_free(struct rsnd_mod *mod, struct rsnd_dai_stream *io,
+			    struct snd_pcm_substream *substream)
+{
+	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
+
+	if (!rsnd_ssi_is_run_mods(mod, io))
+		return 0;
+
+	if (!ssi->usrcnt) {
+		struct rsnd_priv *priv = rsnd_mod_to_priv(mod);
+		struct device *dev = rsnd_priv_to_dev(priv);
+
+		dev_err(dev, "%s usrcnt error\n", rsnd_mod_name(mod));
+		return -EIO;
+	}
+
+	rsnd_ssi_master_clk_stop(mod, io);
+
+	ssi->usrcnt--;
+
+	if (!ssi->usrcnt) {
+		ssi->cr_own	= 0;
+		ssi->cr_mode	= 0;
+		ssi->wsr	= 0;
+	}
+
+	return 0;
+}
+
 static struct rsnd_mod_ops rsnd_ssi_pio_ops = {
 	.name		= SSI_NAME,
 	.probe		= rsnd_ssi_common_probe,
@@ -926,6 +947,7 @@ static struct rsnd_mod_ops rsnd_ssi_pio_ops = {
 	.pcm_new	= rsnd_ssi_pcm_new,
 	.hw_params	= rsnd_ssi_hw_params,
 	.prepare	= rsnd_ssi_prepare,
+	.hw_free	= rsnd_ssi_hw_free,
 	.get_status	= rsnd_ssi_get_status,
 };
 
@@ -1012,6 +1034,7 @@ 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,
+	.hw_free	= rsnd_ssi_hw_free,
 	.prepare	= rsnd_ssi_prepare,
 	.get_status	= rsnd_ssi_get_status,
 };
-- 
2.19.2

[PATCH v1 2/3] ASoC: rsnd: Allow reconfiguration of clock rate

[Jiada Wang]

From: Timo Wischer <twischer@de.adit-jv.com>

Currently after clock rate is started in .prepare reconfiguration
of clock rate is not allowed, unless the stream is stopped.

But there is use case in Gstreamer ALSA sink, in case of changed caps
Gsreatmer reconfigs and it calls snd_pcm_hw_free() before snd_pcm_prepre().
See gstreamer1.0-plugins-base/
gst-libs/gst/audio/gstaudiobasesink.c: gst_audio_base_sink_setcaps():
- gst_audio_ring_buffer_release()
- gst_audio_sink_ring_buffer_release()
- gst_alsasink_unprepare()
- snd_pcm_hw_free()
is called before
- gst_audio_ring_buffer_acquire()
- gst_audio_sink_ring_buffer_acquire()
- gst_alsasink_prepare()
- set_hwparams()
- snd_pcm_hw_params()
- snd_pcm_prepare()

The issue mentioned in this commit can be reproduced with the following
aplay patch:

    >diff --git a/aplay/aplay.c b/aplay/aplay.c
    >@@ -2760,6 +2760,8 @@ static void playback_go(int fd, size_t loaded,
    >      header(rtype, name);
    >      set_params();
    >+     hwparams.rate = (hwparams.rate == 48000) ? 44100 : 48000;
    >+     set_params();
    >
    >      while (loaded > chunk_bytes && written < count && !in_aborting)
    >      {
    >              if (pcm_write(audiobuf + written, chunk_size) <= 0)

$ aplay -Dplughw:0,0,0 -c8 -fS24_LE -r48000 /dev/zero
rcar_sound ec500000.sound: SSI parent/child should use same rate
rcar_sound ec500000.sound: ssi[3] : prepare error -22
rcar_sound ec500000.sound: ASoC: cpu DAI prepare error: -22
rsnd_link0: ASoC: prepare FE rsnd_link0 failed

this patch address the issue by stop clock in .hw_free,
to allow reconfiguration of clock rate without stop of the stream.

Signed-off-by: Timo Wischer <twischer@de.adit-jv.com>
Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 sound/soc/sh/rcar/ssi.c | 53 +++++++++++++++++++++++++++++------------
 1 file changed, 38 insertions(+), 15 deletions(-)

diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
index f6a7466622ea..f43937d2c588 100644
--- a/sound/soc/sh/rcar/ssi.c
+++ b/sound/soc/sh/rcar/ssi.c
@@ -286,7 +286,7 @@ static int rsnd_ssi_master_clk_start(struct rsnd_mod *mod,
 	if (rsnd_ssi_is_multi_slave(mod, io))
 		return 0;
 
-	if (ssi->usrcnt > 0) {
+	if (ssi->rate) {
 		if (ssi->rate != rate) {
 			dev_err(dev, "SSI parent/child should use same rate\n");
 			return -EINVAL;
@@ -471,13 +471,9 @@ static int rsnd_ssi_init(struct rsnd_mod *mod,
 			 struct rsnd_dai_stream *io,
 			 struct rsnd_priv *priv)
 {
-	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
-
 	if (!rsnd_ssi_is_run_mods(mod, io))
 		return 0;
 
-	ssi->usrcnt++;
-
 	rsnd_mod_power_on(mod);
 
 	rsnd_ssi_config_init(mod, io);
@@ -505,18 +501,8 @@ static int rsnd_ssi_quit(struct rsnd_mod *mod,
 		return -EIO;
 	}
 
-	rsnd_ssi_master_clk_stop(mod, io);
-
 	rsnd_mod_power_off(mod);
 
-	ssi->usrcnt--;
-
-	if (!ssi->usrcnt) {
-		ssi->cr_own	= 0;
-		ssi->cr_mode	= 0;
-		ssi->wsr	= 0;
-	}
-
 	return 0;
 }
 
@@ -525,6 +511,7 @@ static int rsnd_ssi_hw_params(struct rsnd_mod *mod,
 			      struct snd_pcm_substream *substream,
 			      struct snd_pcm_hw_params *params)
 {
+	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
 	struct rsnd_dai *rdai = rsnd_io_to_rdai(io);
 	unsigned int fmt_width = snd_pcm_format_width(params_format(params));
 
@@ -536,6 +523,11 @@ static int rsnd_ssi_hw_params(struct rsnd_mod *mod,
 		return -EINVAL;
 	}
 
+	if (!rsnd_ssi_is_run_mods(mod, io))
+		return 0;
+
+	ssi->usrcnt++;
+
 	return 0;
 }
 
@@ -913,6 +905,35 @@ static int rsnd_ssi_prepare(struct rsnd_mod *mod,
 	return rsnd_ssi_master_clk_start(mod, io);
 }
 
+static int rsnd_ssi_hw_free(struct rsnd_mod *mod, struct rsnd_dai_stream *io,
+			    struct snd_pcm_substream *substream)
+{
+	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
+
+	if (!rsnd_ssi_is_run_mods(mod, io))
+		return 0;
+
+	if (!ssi->usrcnt) {
+		struct rsnd_priv *priv = rsnd_mod_to_priv(mod);
+		struct device *dev = rsnd_priv_to_dev(priv);
+
+		dev_err(dev, "%s usrcnt error\n", rsnd_mod_name(mod));
+		return -EIO;
+	}
+
+	rsnd_ssi_master_clk_stop(mod, io);
+
+	ssi->usrcnt--;
+
+	if (!ssi->usrcnt) {
+		ssi->cr_own	= 0;
+		ssi->cr_mode	= 0;
+		ssi->wsr	= 0;
+	}
+
+	return 0;
+}
+
 static struct rsnd_mod_ops rsnd_ssi_pio_ops = {
 	.name		= SSI_NAME,
 	.probe		= rsnd_ssi_common_probe,
@@ -926,6 +947,7 @@ static struct rsnd_mod_ops rsnd_ssi_pio_ops = {
 	.pcm_new	= rsnd_ssi_pcm_new,
 	.hw_params	= rsnd_ssi_hw_params,
 	.prepare	= rsnd_ssi_prepare,
+	.hw_free	= rsnd_ssi_hw_free,
 	.get_status	= rsnd_ssi_get_status,
 };
 
@@ -1012,6 +1034,7 @@ 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,
+	.hw_free	= rsnd_ssi_hw_free,
 	.prepare	= rsnd_ssi_prepare,
 	.get_status	= rsnd_ssi_get_status,
 };
-- 
2.19.2

[PATCH v1 3/3] ASoC: rsnd: call .hw_{params,free} in pair for same stream

[Jiada Wang]

Currently usrcnt is {in,de}cremented in .hw_{params,free} callbacks,
but .hw_free may be called multiple times without calling .hw_params.
this causes the usrcnt be decremented wrongly.

This patch allows .hw_{params,free} to be called only in pairs for
the same stream which balances the {in,de}crement of usrcnt.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
Signed-off-by: Timo Wischer <twischer@de.adit-jv.com>
---
 sound/soc/sh/rcar/core.c |  6 ++++--
 sound/soc/sh/rcar/rsnd.h | 24 ++++++++++++++++++++++--
 sound/soc/sh/rcar/ssi.c  |  8 ++++++--
 sound/soc/sh/rcar/ssiu.c |  3 ++-
 4 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c
index bda5b958d0dc..b9330bdadbd3 100644
--- a/sound/soc/sh/rcar/core.c
+++ b/sound/soc/sh/rcar/core.c
@@ -172,7 +172,8 @@ char *rsnd_mod_name(struct rsnd_mod *mod)
 
 u32 *rsnd_mod_get_status(struct rsnd_mod *mod,
 			 struct rsnd_dai_stream *io,
-			 enum rsnd_mod_type type)
+			 enum rsnd_mod_type type,
+			 int flag)
 {
 	return &mod->status;
 }
@@ -548,7 +549,8 @@ static int rsnd_status_update(u32 *status,
 	enum rsnd_mod_type *types = rsnd_mod_sequence[is_play];		\
 	for_each_rsnd_mod_arrays(i, mod, io, types, RSND_MOD_MAX) {	\
 		int tmp = 0;						\
-		u32 *status = mod->ops->get_status(mod, io, types[i]);	\
+		u32 *status = mod->ops->get_status(mod, io, types[i],	\
+						__rsnd_mod_flag_##fn);	\
 		int func_call = rsnd_status_update(status,		\
 						__rsnd_mod_shift_##fn,	\
 						__rsnd_mod_add_##fn,	\
diff --git a/sound/soc/sh/rcar/rsnd.h b/sound/soc/sh/rcar/rsnd.h
index ea6cbaa9743e..b4e3e9289f8a 100644
--- a/sound/soc/sh/rcar/rsnd.h
+++ b/sound/soc/sh/rcar/rsnd.h
@@ -238,6 +238,7 @@ enum rsnd_reg {
 #define SSI9_BUSIF_DALIGN(i)	(SSI9_BUSIF0_DALIGN + (i))
 #define SSI_SYS_STATUS(i)	(SSI_SYS_STATUS0 + (i))
 
+#define RSND_STATUS_ON_IO	BIT(0)
 
 struct rsnd_priv;
 struct rsnd_mod;
@@ -332,7 +333,8 @@ struct rsnd_mod_ops {
 		       struct snd_pcm_substream *substream);
 	u32 *(*get_status)(struct rsnd_mod *mod,
 			   struct rsnd_dai_stream *io,
-			   enum rsnd_mod_type type);
+			   enum rsnd_mod_type type,
+			   int flag);
 	int (*id)(struct rsnd_mod *mod);
 	int (*id_sub)(struct rsnd_mod *mod);
 	int (*id_cmd)(struct rsnd_mod *mod);
@@ -379,6 +381,22 @@ struct rsnd_mod {
 #define __rsnd_mod_shift_prepare	28 /* always called */
 #define __rsnd_mod_shift_cleanup	28 /* always called */
 
+#define __rsnd_mod_flag_init		0
+#define __rsnd_mod_flag_quit		0
+#define __rsnd_mod_flag_start		0
+#define __rsnd_mod_flag_stop		0
+#define __rsnd_mod_flag_hw_params	RSND_STATUS_ON_IO
+#define __rsnd_mod_flag_hw_free		RSND_STATUS_ON_IO
+#define __rsnd_mod_flag_probe		0
+#define __rsnd_mod_flag_remove		0
+#define __rsnd_mod_flag_irq		0
+#define __rsnd_mod_flag_pcm_new		0
+#define __rsnd_mod_flag_fallback	0
+#define __rsnd_mod_flag_pointer		0
+#define __rsnd_mod_flag_prepare		0
+#define __rsnd_mod_flag_cleanup		0
+#define __rsnd_mod_flag_set_fmt		0
+
 #define __rsnd_mod_add_probe		0
 #define __rsnd_mod_add_remove		0
 #define __rsnd_mod_add_prepare		0
@@ -428,7 +446,8 @@ void rsnd_mod_interrupt(struct rsnd_mod *mod,
 					 struct rsnd_dai_stream *io));
 u32 *rsnd_mod_get_status(struct rsnd_mod *mod,
 			 struct rsnd_dai_stream *io,
-			 enum rsnd_mod_type type);
+			 enum rsnd_mod_type type,
+			 int flag);
 int rsnd_mod_id(struct rsnd_mod *mod);
 int rsnd_mod_id_raw(struct rsnd_mod *mod);
 int rsnd_mod_id_sub(struct rsnd_mod *mod);
@@ -496,6 +515,7 @@ struct rsnd_dai_stream {
 	u32 converted_rate;      /* converted sampling rate */
 	int converted_chan;      /* converted channels */
 	u32 parent_ssi_status;
+	u32 status;
 	u32 flags;
 };
 
diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
index f43937d2c588..89b4029b290b 100644
--- a/sound/soc/sh/rcar/ssi.c
+++ b/sound/soc/sh/rcar/ssi.c
@@ -681,7 +681,8 @@ static irqreturn_t rsnd_ssi_interrupt(int irq, void *data)
 
 static u32 *rsnd_ssi_get_status(struct rsnd_mod *mod,
 				struct rsnd_dai_stream *io,
-				enum rsnd_mod_type type)
+				enum rsnd_mod_type type,
+				int flag)
 {
 	/*
 	 * SSIP (= SSI parent) needs to be special, otherwise,
@@ -711,7 +712,10 @@ static u32 *rsnd_ssi_get_status(struct rsnd_mod *mod,
 	if (type == RSND_MOD_SSIP)
 		return &io->parent_ssi_status;
 
-	return rsnd_mod_get_status(mod, io, type);
+	if (flag && RSND_STATUS_ON_IO)
+		return &io->status;
+
+	return rsnd_mod_get_status(mod, io, type, flag);
 }
 
 /*
diff --git a/sound/soc/sh/rcar/ssiu.c b/sound/soc/sh/rcar/ssiu.c
index f35d88211887..45e4cd84fbc4 100644
--- a/sound/soc/sh/rcar/ssiu.c
+++ b/sound/soc/sh/rcar/ssiu.c
@@ -47,7 +47,8 @@ static const int gen3_id[] = { 0, 8, 16, 24, 32, 40, 41, 42, 43, 44 };
 
 static u32 *rsnd_ssiu_get_status(struct rsnd_mod *mod,
 				 struct rsnd_dai_stream *io,
-				 enum rsnd_mod_type type)
+				 enum rsnd_mod_type type,
+				 int flag)
 {
 	struct rsnd_ssiu *ssiu = rsnd_mod_to_ssiu(mod);
 	int busif = rsnd_mod_id_sub(mod);
-- 
2.19.2

[PATCH v1 3/3] ASoC: rsnd: call .hw_{params,free} in pair for same stream

[Jiada Wang]

Currently usrcnt is {in,de}cremented in .hw_{params,free} callbacks,
but .hw_free may be called multiple times without calling .hw_params.
this causes the usrcnt be decremented wrongly.

This patch allows .hw_{params,free} to be called only in pairs for
the same stream which balances the {in,de}crement of usrcnt.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
Signed-off-by: Timo Wischer <twischer@de.adit-jv.com>
---
 sound/soc/sh/rcar/core.c |  6 ++++--
 sound/soc/sh/rcar/rsnd.h | 24 ++++++++++++++++++++++--
 sound/soc/sh/rcar/ssi.c  |  8 ++++++--
 sound/soc/sh/rcar/ssiu.c |  3 ++-
 4 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c
index bda5b958d0dc..b9330bdadbd3 100644
--- a/sound/soc/sh/rcar/core.c
+++ b/sound/soc/sh/rcar/core.c
@@ -172,7 +172,8 @@ char *rsnd_mod_name(struct rsnd_mod *mod)
 
 u32 *rsnd_mod_get_status(struct rsnd_mod *mod,
 			 struct rsnd_dai_stream *io,
-			 enum rsnd_mod_type type)
+			 enum rsnd_mod_type type,
+			 int flag)
 {
 	return &mod->status;
 }
@@ -548,7 +549,8 @@ static int rsnd_status_update(u32 *status,
 	enum rsnd_mod_type *types = rsnd_mod_sequence[is_play];		\
 	for_each_rsnd_mod_arrays(i, mod, io, types, RSND_MOD_MAX) {	\
 		int tmp = 0;						\
-		u32 *status = mod->ops->get_status(mod, io, types[i]);	\
+		u32 *status = mod->ops->get_status(mod, io, types[i],	\
+						__rsnd_mod_flag_##fn);	\
 		int func_call = rsnd_status_update(status,		\
 						__rsnd_mod_shift_##fn,	\
 						__rsnd_mod_add_##fn,	\
diff --git a/sound/soc/sh/rcar/rsnd.h b/sound/soc/sh/rcar/rsnd.h
index ea6cbaa9743e..b4e3e9289f8a 100644
--- a/sound/soc/sh/rcar/rsnd.h
+++ b/sound/soc/sh/rcar/rsnd.h
@@ -238,6 +238,7 @@ enum rsnd_reg {
 #define SSI9_BUSIF_DALIGN(i)	(SSI9_BUSIF0_DALIGN + (i))
 #define SSI_SYS_STATUS(i)	(SSI_SYS_STATUS0 + (i))
 
+#define RSND_STATUS_ON_IO	BIT(0)
 
 struct rsnd_priv;
 struct rsnd_mod;
@@ -332,7 +333,8 @@ struct rsnd_mod_ops {
 		       struct snd_pcm_substream *substream);
 	u32 *(*get_status)(struct rsnd_mod *mod,
 			   struct rsnd_dai_stream *io,
-			   enum rsnd_mod_type type);
+			   enum rsnd_mod_type type,
+			   int flag);
 	int (*id)(struct rsnd_mod *mod);
 	int (*id_sub)(struct rsnd_mod *mod);
 	int (*id_cmd)(struct rsnd_mod *mod);
@@ -379,6 +381,22 @@ struct rsnd_mod {
 #define __rsnd_mod_shift_prepare	28 /* always called */
 #define __rsnd_mod_shift_cleanup	28 /* always called */
 
+#define __rsnd_mod_flag_init		0
+#define __rsnd_mod_flag_quit		0
+#define __rsnd_mod_flag_start		0
+#define __rsnd_mod_flag_stop		0
+#define __rsnd_mod_flag_hw_params	RSND_STATUS_ON_IO
+#define __rsnd_mod_flag_hw_free		RSND_STATUS_ON_IO
+#define __rsnd_mod_flag_probe		0
+#define __rsnd_mod_flag_remove		0
+#define __rsnd_mod_flag_irq		0
+#define __rsnd_mod_flag_pcm_new		0
+#define __rsnd_mod_flag_fallback	0
+#define __rsnd_mod_flag_pointer		0
+#define __rsnd_mod_flag_prepare		0
+#define __rsnd_mod_flag_cleanup		0
+#define __rsnd_mod_flag_set_fmt		0
+
 #define __rsnd_mod_add_probe		0
 #define __rsnd_mod_add_remove		0
 #define __rsnd_mod_add_prepare		0
@@ -428,7 +446,8 @@ void rsnd_mod_interrupt(struct rsnd_mod *mod,
 					 struct rsnd_dai_stream *io));
 u32 *rsnd_mod_get_status(struct rsnd_mod *mod,
 			 struct rsnd_dai_stream *io,
-			 enum rsnd_mod_type type);
+			 enum rsnd_mod_type type,
+			 int flag);
 int rsnd_mod_id(struct rsnd_mod *mod);
 int rsnd_mod_id_raw(struct rsnd_mod *mod);
 int rsnd_mod_id_sub(struct rsnd_mod *mod);
@@ -496,6 +515,7 @@ struct rsnd_dai_stream {
 	u32 converted_rate;      /* converted sampling rate */
 	int converted_chan;      /* converted channels */
 	u32 parent_ssi_status;
+	u32 status;
 	u32 flags;
 };
 
diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
index f43937d2c588..89b4029b290b 100644
--- a/sound/soc/sh/rcar/ssi.c
+++ b/sound/soc/sh/rcar/ssi.c
@@ -681,7 +681,8 @@ static irqreturn_t rsnd_ssi_interrupt(int irq, void *data)
 
 static u32 *rsnd_ssi_get_status(struct rsnd_mod *mod,
 				struct rsnd_dai_stream *io,
-				enum rsnd_mod_type type)
+				enum rsnd_mod_type type,
+				int flag)
 {
 	/*
 	 * SSIP (= SSI parent) needs to be special, otherwise,
@@ -711,7 +712,10 @@ static u32 *rsnd_ssi_get_status(struct rsnd_mod *mod,
 	if (type == RSND_MOD_SSIP)
 		return &io->parent_ssi_status;
 
-	return rsnd_mod_get_status(mod, io, type);
+	if (flag && RSND_STATUS_ON_IO)
+		return &io->status;
+
+	return rsnd_mod_get_status(mod, io, type, flag);
 }
 
 /*
diff --git a/sound/soc/sh/rcar/ssiu.c b/sound/soc/sh/rcar/ssiu.c
index f35d88211887..45e4cd84fbc4 100644
--- a/sound/soc/sh/rcar/ssiu.c
+++ b/sound/soc/sh/rcar/ssiu.c
@@ -47,7 +47,8 @@ static const int gen3_id[] = { 0, 8, 16, 24, 32, 40, 41, 42, 43, 44 };
 
 static u32 *rsnd_ssiu_get_status(struct rsnd_mod *mod,
 				 struct rsnd_dai_stream *io,
-				 enum rsnd_mod_type type)
+				 enum rsnd_mod_type type,
+				 int flag)
 {
 	struct rsnd_ssiu *ssiu = rsnd_mod_to_ssiu(mod);
 	int busif = rsnd_mod_id_sub(mod);
-- 
2.19.2

Re: [PATCH v1 2/3] ASoC: rsnd: Allow reconfiguration of clock rate

[Kuninori Morimoto]

Hi Jiada

The solution looks very over-kill to me,
especiallyq [3/3] patch is too much to me.

1st, can we start clock at .hw_param, instead of .prepare ?
and stop it at .hw_free ?

2nd, can we keep usrcnt setup as-is ?
I guess we can just avoid rsnd_ssi_master_clk_start() if ssi->rate was not 0 ?
similar for rsnd_ssi_master_clk_stop()

	static int rsnd_ssi_master_clk_start(struct rsnd_mod *mod,
				     struct rsnd_dai_stream *io)
	{
		...
		if (ssi->rate)
			return 0;
		...
	}

	static void rsnd_ssi_master_clk_stop(struct rsnd_mod *mod,
				     struct rsnd_dai_stream *io)
	{
		...
-		if (ssi->usrcnt > 1)
+		if (ssi->rate == 0)
			return;
		...
	}


> From: Timo Wischer <twischer@de.adit-jv.com>
> 
> Currently after clock rate is started in .prepare reconfiguration
> of clock rate is not allowed, unless the stream is stopped.
> 
> But there is use case in Gstreamer ALSA sink, in case of changed caps
> Gsreatmer reconfigs and it calls snd_pcm_hw_free() before snd_pcm_prepre().
> See gstreamer1.0-plugins-base/
> gst-libs/gst/audio/gstaudiobasesink.c: gst_audio_base_sink_setcaps():
> - gst_audio_ring_buffer_release()
> - gst_audio_sink_ring_buffer_release()
> - gst_alsasink_unprepare()
> - snd_pcm_hw_free()
> is called before
> - gst_audio_ring_buffer_acquire()
> - gst_audio_sink_ring_buffer_acquire()
> - gst_alsasink_prepare()
> - set_hwparams()
> - snd_pcm_hw_params()
> - snd_pcm_prepare()
> 
> The issue mentioned in this commit can be reproduced with the following
> aplay patch:
> 
>     >diff --git a/aplay/aplay.c b/aplay/aplay.c
>     >@@ -2760,6 +2760,8 @@ static void playback_go(int fd, size_t loaded,
>     >      header(rtype, name);
>     >      set_params();
>     >+     hwparams.rate = (hwparams.rate == 48000) ? 44100 : 48000;
>     >+     set_params();
>     >
>     >      while (loaded > chunk_bytes && written < count && !in_aborting)
>     >      {
>     >              if (pcm_write(audiobuf + written, chunk_size) <= 0)
> 
> $ aplay -Dplughw:0,0,0 -c8 -fS24_LE -r48000 /dev/zero
> rcar_sound ec500000.sound: SSI parent/child should use same rate
> rcar_sound ec500000.sound: ssi[3] : prepare error -22
> rcar_sound ec500000.sound: ASoC: cpu DAI prepare error: -22
> rsnd_link0: ASoC: prepare FE rsnd_link0 failed
> 
> this patch address the issue by stop clock in .hw_free,
> to allow reconfiguration of clock rate without stop of the stream.
> 
> Signed-off-by: Timo Wischer <twischer@de.adit-jv.com>
> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
> ---
>  sound/soc/sh/rcar/ssi.c | 53 +++++++++++++++++++++++++++++------------
>  1 file changed, 38 insertions(+), 15 deletions(-)
> 
> diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
> index f6a7466622ea..f43937d2c588 100644
> --- a/sound/soc/sh/rcar/ssi.c
> +++ b/sound/soc/sh/rcar/ssi.c
> @@ -286,7 +286,7 @@ static int rsnd_ssi_master_clk_start(struct rsnd_mod *mod,
>  	if (rsnd_ssi_is_multi_slave(mod, io))
>  		return 0;
>  
> -	if (ssi->usrcnt > 0) {
> +	if (ssi->rate) {
>  		if (ssi->rate != rate) {
>  			dev_err(dev, "SSI parent/child should use same rate\n");
>  			return -EINVAL;
> @@ -471,13 +471,9 @@ static int rsnd_ssi_init(struct rsnd_mod *mod,
>  			 struct rsnd_dai_stream *io,
>  			 struct rsnd_priv *priv)
>  {
> -	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
> -
>  	if (!rsnd_ssi_is_run_mods(mod, io))
>  		return 0;
>  
> -	ssi->usrcnt++;
> -
>  	rsnd_mod_power_on(mod);
>  
>  	rsnd_ssi_config_init(mod, io);
> @@ -505,18 +501,8 @@ static int rsnd_ssi_quit(struct rsnd_mod *mod,
>  		return -EIO;
>  	}
>  
> -	rsnd_ssi_master_clk_stop(mod, io);
> -
>  	rsnd_mod_power_off(mod);
>  
> -	ssi->usrcnt--;
> -
> -	if (!ssi->usrcnt) {
> -		ssi->cr_own	= 0;
> -		ssi->cr_mode	= 0;
> -		ssi->wsr	= 0;
> -	}
> -
>  	return 0;
>  }
>  
> @@ -525,6 +511,7 @@ static int rsnd_ssi_hw_params(struct rsnd_mod *mod,
>  			      struct snd_pcm_substream *substream,
>  			      struct snd_pcm_hw_params *params)
>  {
> +	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
>  	struct rsnd_dai *rdai = rsnd_io_to_rdai(io);
>  	unsigned int fmt_width = snd_pcm_format_width(params_format(params));
>  
> @@ -536,6 +523,11 @@ static int rsnd_ssi_hw_params(struct rsnd_mod *mod,
>  		return -EINVAL;
>  	}
>  
> +	if (!rsnd_ssi_is_run_mods(mod, io))
> +		return 0;
> +
> +	ssi->usrcnt++;
> +
>  	return 0;
>  }
>  
> @@ -913,6 +905,35 @@ static int rsnd_ssi_prepare(struct rsnd_mod *mod,
>  	return rsnd_ssi_master_clk_start(mod, io);
>  }
>  
> +static int rsnd_ssi_hw_free(struct rsnd_mod *mod, struct rsnd_dai_stream *io,
> +			    struct snd_pcm_substream *substream)
> +{
> +	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
> +
> +	if (!rsnd_ssi_is_run_mods(mod, io))
> +		return 0;
> +
> +	if (!ssi->usrcnt) {
> +		struct rsnd_priv *priv = rsnd_mod_to_priv(mod);
> +		struct device *dev = rsnd_priv_to_dev(priv);
> +
> +		dev_err(dev, "%s usrcnt error\n", rsnd_mod_name(mod));
> +		return -EIO;
> +	}
> +
> +	rsnd_ssi_master_clk_stop(mod, io);
> +
> +	ssi->usrcnt--;
> +
> +	if (!ssi->usrcnt) {
> +		ssi->cr_own	= 0;
> +		ssi->cr_mode	= 0;
> +		ssi->wsr	= 0;
> +	}
> +
> +	return 0;
> +}
> +
>  static struct rsnd_mod_ops rsnd_ssi_pio_ops = {
>  	.name		= SSI_NAME,
>  	.probe		= rsnd_ssi_common_probe,
> @@ -926,6 +947,7 @@ static struct rsnd_mod_ops rsnd_ssi_pio_ops = {
>  	.pcm_new	= rsnd_ssi_pcm_new,
>  	.hw_params	= rsnd_ssi_hw_params,
>  	.prepare	= rsnd_ssi_prepare,
> +	.hw_free	= rsnd_ssi_hw_free,
>  	.get_status	= rsnd_ssi_get_status,
>  };
>  
> @@ -1012,6 +1034,7 @@ 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,
> +	.hw_free	= rsnd_ssi_hw_free,
>  	.prepare	= rsnd_ssi_prepare,
>  	.get_status	= rsnd_ssi_get_status,
>  };
> -- 
> 2.19.2
> 

Re: [PATCH v1 2/3] ASoC: rsnd: Allow reconfiguration of clock rate

[Kuninori Morimoto]

Hi Jiada

The solution looks very over-kill to me,
especiallyq [3/3] patch is too much to me.

1st, can we start clock at .hw_param, instead of .prepare ?
and stop it at .hw_free ?

2nd, can we keep usrcnt setup as-is ?
I guess we can just avoid rsnd_ssi_master_clk_start() if ssi->rate was not 0 ?
similar for rsnd_ssi_master_clk_stop()

	static int rsnd_ssi_master_clk_start(struct rsnd_mod *mod,
				     struct rsnd_dai_stream *io)
	{
		...
		if (ssi->rate)
			return 0;
		...
	}

	static void rsnd_ssi_master_clk_stop(struct rsnd_mod *mod,
				     struct rsnd_dai_stream *io)
	{
		...
-		if (ssi->usrcnt > 1)
+		if (ssi->rate == 0)
			return;
		...
	}


> From: Timo Wischer <twischer@de.adit-jv.com>
> 
> Currently after clock rate is started in .prepare reconfiguration
> of clock rate is not allowed, unless the stream is stopped.
> 
> But there is use case in Gstreamer ALSA sink, in case of changed caps
> Gsreatmer reconfigs and it calls snd_pcm_hw_free() before snd_pcm_prepre().
> See gstreamer1.0-plugins-base/
> gst-libs/gst/audio/gstaudiobasesink.c: gst_audio_base_sink_setcaps():
> - gst_audio_ring_buffer_release()
> - gst_audio_sink_ring_buffer_release()
> - gst_alsasink_unprepare()
> - snd_pcm_hw_free()
> is called before
> - gst_audio_ring_buffer_acquire()
> - gst_audio_sink_ring_buffer_acquire()
> - gst_alsasink_prepare()
> - set_hwparams()
> - snd_pcm_hw_params()
> - snd_pcm_prepare()
> 
> The issue mentioned in this commit can be reproduced with the following
> aplay patch:
> 
>     >diff --git a/aplay/aplay.c b/aplay/aplay.c
>     >@@ -2760,6 +2760,8 @@ static void playback_go(int fd, size_t loaded,
>     >      header(rtype, name);
>     >      set_params();
>     >+     hwparams.rate = (hwparams.rate == 48000) ? 44100 : 48000;
>     >+     set_params();
>     >
>     >      while (loaded > chunk_bytes && written < count && !in_aborting)
>     >      {
>     >              if (pcm_write(audiobuf + written, chunk_size) <= 0)
> 
> $ aplay -Dplughw:0,0,0 -c8 -fS24_LE -r48000 /dev/zero
> rcar_sound ec500000.sound: SSI parent/child should use same rate
> rcar_sound ec500000.sound: ssi[3] : prepare error -22
> rcar_sound ec500000.sound: ASoC: cpu DAI prepare error: -22
> rsnd_link0: ASoC: prepare FE rsnd_link0 failed
> 
> this patch address the issue by stop clock in .hw_free,
> to allow reconfiguration of clock rate without stop of the stream.
> 
> Signed-off-by: Timo Wischer <twischer@de.adit-jv.com>
> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
> ---
>  sound/soc/sh/rcar/ssi.c | 53 +++++++++++++++++++++++++++++------------
>  1 file changed, 38 insertions(+), 15 deletions(-)
> 
> diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
> index f6a7466622ea..f43937d2c588 100644
> --- a/sound/soc/sh/rcar/ssi.c
> +++ b/sound/soc/sh/rcar/ssi.c
> @@ -286,7 +286,7 @@ static int rsnd_ssi_master_clk_start(struct rsnd_mod *mod,
>  	if (rsnd_ssi_is_multi_slave(mod, io))
>  		return 0;
>  
> -	if (ssi->usrcnt > 0) {
> +	if (ssi->rate) {
>  		if (ssi->rate != rate) {
>  			dev_err(dev, "SSI parent/child should use same rate\n");
>  			return -EINVAL;
> @@ -471,13 +471,9 @@ static int rsnd_ssi_init(struct rsnd_mod *mod,
>  			 struct rsnd_dai_stream *io,
>  			 struct rsnd_priv *priv)
>  {
> -	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
> -
>  	if (!rsnd_ssi_is_run_mods(mod, io))
>  		return 0;
>  
> -	ssi->usrcnt++;
> -
>  	rsnd_mod_power_on(mod);
>  
>  	rsnd_ssi_config_init(mod, io);
> @@ -505,18 +501,8 @@ static int rsnd_ssi_quit(struct rsnd_mod *mod,
>  		return -EIO;
>  	}
>  
> -	rsnd_ssi_master_clk_stop(mod, io);
> -
>  	rsnd_mod_power_off(mod);
>  
> -	ssi->usrcnt--;
> -
> -	if (!ssi->usrcnt) {
> -		ssi->cr_own	= 0;
> -		ssi->cr_mode	= 0;
> -		ssi->wsr	= 0;
> -	}
> -
>  	return 0;
>  }
>  
> @@ -525,6 +511,7 @@ static int rsnd_ssi_hw_params(struct rsnd_mod *mod,
>  			      struct snd_pcm_substream *substream,
>  			      struct snd_pcm_hw_params *params)
>  {
> +	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
>  	struct rsnd_dai *rdai = rsnd_io_to_rdai(io);
>  	unsigned int fmt_width = snd_pcm_format_width(params_format(params));
>  
> @@ -536,6 +523,11 @@ static int rsnd_ssi_hw_params(struct rsnd_mod *mod,
>  		return -EINVAL;
>  	}
>  
> +	if (!rsnd_ssi_is_run_mods(mod, io))
> +		return 0;
> +
> +	ssi->usrcnt++;
> +
>  	return 0;
>  }
>  
> @@ -913,6 +905,35 @@ static int rsnd_ssi_prepare(struct rsnd_mod *mod,
>  	return rsnd_ssi_master_clk_start(mod, io);
>  }
>  
> +static int rsnd_ssi_hw_free(struct rsnd_mod *mod, struct rsnd_dai_stream *io,
> +			    struct snd_pcm_substream *substream)
> +{
> +	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
> +
> +	if (!rsnd_ssi_is_run_mods(mod, io))
> +		return 0;
> +
> +	if (!ssi->usrcnt) {
> +		struct rsnd_priv *priv = rsnd_mod_to_priv(mod);
> +		struct device *dev = rsnd_priv_to_dev(priv);
> +
> +		dev_err(dev, "%s usrcnt error\n", rsnd_mod_name(mod));
> +		return -EIO;
> +	}
> +
> +	rsnd_ssi_master_clk_stop(mod, io);
> +
> +	ssi->usrcnt--;
> +
> +	if (!ssi->usrcnt) {
> +		ssi->cr_own	= 0;
> +		ssi->cr_mode	= 0;
> +		ssi->wsr	= 0;
> +	}
> +
> +	return 0;
> +}
> +
>  static struct rsnd_mod_ops rsnd_ssi_pio_ops = {
>  	.name		= SSI_NAME,
>  	.probe		= rsnd_ssi_common_probe,
> @@ -926,6 +947,7 @@ static struct rsnd_mod_ops rsnd_ssi_pio_ops = {
>  	.pcm_new	= rsnd_ssi_pcm_new,
>  	.hw_params	= rsnd_ssi_hw_params,
>  	.prepare	= rsnd_ssi_prepare,
> +	.hw_free	= rsnd_ssi_hw_free,
>  	.get_status	= rsnd_ssi_get_status,
>  };
>  
> @@ -1012,6 +1034,7 @@ 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,
> +	.hw_free	= rsnd_ssi_hw_free,
>  	.prepare	= rsnd_ssi_prepare,
>  	.get_status	= rsnd_ssi_get_status,
>  };
> -- 
> 2.19.2
> 

Re: [PATCH v1 2/3] ASoC: rsnd: Allow reconfiguration of clock rate

[Kuninori Morimoto]

Hi Jiada, again

> The solution looks very over-kill to me,
> especiallyq [3/3] patch is too much to me.
> 
> 1st, can we start clock at .hw_param, instead of .prepare ?
> and stop it at .hw_free ?
> 
> 2nd, can we keep usrcnt setup as-is ?
> I guess we can just avoid rsnd_ssi_master_clk_start() if ssi->rate was not 0 ?
> similar for rsnd_ssi_master_clk_stop()

In my quick check, I used your [1/3] patch and below.
It works for me, but I don't know detail of your test case.

----------
diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
index f6a7466..f26ffd1 100644
--- a/sound/soc/sh/rcar/ssi.c
+++ b/sound/soc/sh/rcar/ssi.c
@@ -286,19 +286,8 @@ static int rsnd_ssi_master_clk_start(struct rsnd_mod *mod,
 	if (rsnd_ssi_is_multi_slave(mod, io))
 		return 0;
 
-	if (ssi->usrcnt > 0) {
-		if (ssi->rate != rate) {
-			dev_err(dev, "SSI parent/child should use same rate\n");
-			return -EINVAL;
-		}
-
-		if (ssi->chan != chan) {
-			dev_err(dev, "SSI parent/child should use same chan\n");
-			return -EINVAL;
-		}
-
+	if (ssi->rate)
 		return 0;
-	}
 
 	if (rsnd_runtime_is_tdm_split(io))
 		chan = rsnd_io_converted_chan(io);
@@ -349,7 +338,7 @@ static void rsnd_ssi_master_clk_stop(struct rsnd_mod *mod,
 	if (!rsnd_ssi_can_output_clk(mod))
 		return;
 
-	if (ssi->usrcnt > 1)
+	if (!ssi->rate)
 		return;
 
 	ssi->cr_clk	= 0;
@@ -539,6 +528,17 @@ static int rsnd_ssi_hw_params(struct rsnd_mod *mod,
 	return 0;
 }
 
+static int rsnd_ssi_hw_free(struct rsnd_mod *mod, struct rsnd_dai_stream *io,
+			    struct snd_pcm_substream *substream)
+{
+	if (!rsnd_ssi_is_run_mods(mod, io))
+		return 0;
+
+	rsnd_ssi_master_clk_stop(mod, io);
+
+	return 0;
+}
+
 static int rsnd_ssi_start(struct rsnd_mod *mod,
 			  struct rsnd_dai_stream *io,
 			  struct rsnd_priv *priv)
@@ -925,6 +925,7 @@ 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,
+	.hw_free	= rsnd_ssi_hw_free,
 	.prepare	= rsnd_ssi_prepare,
 	.get_status	= rsnd_ssi_get_status,
 };
@@ -1012,6 +1013,7 @@ 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,
+	.hw_free	= rsnd_ssi_hw_free,
 	.prepare	= rsnd_ssi_prepare,
 	.get_status	= rsnd_ssi_get_status,
 };
----------

Re: [PATCH v1 2/3] ASoC: rsnd: Allow reconfiguration of clock rate

[Kuninori Morimoto]

Hi Jiada, again

> The solution looks very over-kill to me,
> especiallyq [3/3] patch is too much to me.
> 
> 1st, can we start clock at .hw_param, instead of .prepare ?
> and stop it at .hw_free ?
> 
> 2nd, can we keep usrcnt setup as-is ?
> I guess we can just avoid rsnd_ssi_master_clk_start() if ssi->rate was not 0 ?
> similar for rsnd_ssi_master_clk_stop()

In my quick check, I used your [1/3] patch and below.
It works for me, but I don't know detail of your test case.

----------
diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
index f6a7466..f26ffd1 100644
--- a/sound/soc/sh/rcar/ssi.c
+++ b/sound/soc/sh/rcar/ssi.c
@@ -286,19 +286,8 @@ static int rsnd_ssi_master_clk_start(struct rsnd_mod *mod,
 	if (rsnd_ssi_is_multi_slave(mod, io))
 		return 0;
 
-	if (ssi->usrcnt > 0) {
-		if (ssi->rate != rate) {
-			dev_err(dev, "SSI parent/child should use same rate\n");
-			return -EINVAL;
-		}
-
-		if (ssi->chan != chan) {
-			dev_err(dev, "SSI parent/child should use same chan\n");
-			return -EINVAL;
-		}
-
+	if (ssi->rate)
 		return 0;
-	}
 
 	if (rsnd_runtime_is_tdm_split(io))
 		chan = rsnd_io_converted_chan(io);
@@ -349,7 +338,7 @@ static void rsnd_ssi_master_clk_stop(struct rsnd_mod *mod,
 	if (!rsnd_ssi_can_output_clk(mod))
 		return;
 
-	if (ssi->usrcnt > 1)
+	if (!ssi->rate)
 		return;
 
 	ssi->cr_clk	= 0;
@@ -539,6 +528,17 @@ static int rsnd_ssi_hw_params(struct rsnd_mod *mod,
 	return 0;
 }
 
+static int rsnd_ssi_hw_free(struct rsnd_mod *mod, struct rsnd_dai_stream *io,
+			    struct snd_pcm_substream *substream)
+{
+	if (!rsnd_ssi_is_run_mods(mod, io))
+		return 0;
+
+	rsnd_ssi_master_clk_stop(mod, io);
+
+	return 0;
+}
+
 static int rsnd_ssi_start(struct rsnd_mod *mod,
 			  struct rsnd_dai_stream *io,
 			  struct rsnd_priv *priv)
@@ -925,6 +925,7 @@ 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,
+	.hw_free	= rsnd_ssi_hw_free,
 	.prepare	= rsnd_ssi_prepare,
 	.get_status	= rsnd_ssi_get_status,
 };
@@ -1012,6 +1013,7 @@ 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,
+	.hw_free	= rsnd_ssi_hw_free,
 	.prepare	= rsnd_ssi_prepare,
 	.get_status	= rsnd_ssi_get_status,
 };
----------

Applied "ASoC: rsnd: Support hw_free() callback at DAI level" to the asoc tree

[Mark Brown]

The patch

   ASoC: rsnd: Support hw_free() callback at DAI level

has been applied to the asoc tree at

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

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

From 859fd6cbf1fb32b5428c26f837215c085b8a822e Mon Sep 17 00:00:00 2001
From: Timo Wischer <twischer@de.adit-jv.com>
Date: Mon, 22 Jul 2019 16:24:01 +0900
Subject: [PATCH] ASoC: rsnd: Support hw_free() callback at DAI level

This patch provides the needed infrastructure to support calling hw_free()
at the DAI level. This is for example required to free resources allocated
in hw_params() callback.

The modification of __rsnd_mod_add_hw_params does not have any side
effects because rsnd_mod_ops::hw_params callback is not used by anyone
until now.

Signed-off-by: Timo Wischer <twischer@de.adit-jv.com>
Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
Link: https://lore.kernel.org/r/20190722072403.11008-2-jiada_wang@mentor.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/sh/rcar/core.c | 16 +++++++++++++++-
 sound/soc/sh/rcar/rsnd.h | 12 +++++++++---
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c
index 56e8dae9a15c..bda5b958d0dc 100644
--- a/sound/soc/sh/rcar/core.c
+++ b/sound/soc/sh/rcar/core.c
@@ -1421,6 +1421,20 @@ static int rsnd_hw_params(struct snd_pcm_substream *substream,
 					params_buffer_bytes(hw_params));
 }
 
+static int rsnd_hw_free(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_dai *dai = rsnd_substream_to_dai(substream);
+	struct rsnd_dai *rdai = rsnd_dai_to_rdai(dai);
+	struct rsnd_dai_stream *io = rsnd_rdai_to_io(rdai, substream);
+	int ret;
+
+	ret = rsnd_dai_call(hw_free, io, substream);
+	if (ret)
+		return ret;
+
+	return snd_pcm_lib_free_pages(substream);
+}
+
 static snd_pcm_uframes_t rsnd_pointer(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_dai *dai = rsnd_substream_to_dai(substream);
@@ -1436,7 +1450,7 @@ static snd_pcm_uframes_t rsnd_pointer(struct snd_pcm_substream *substream)
 static const struct snd_pcm_ops rsnd_pcm_ops = {
 	.ioctl		= snd_pcm_lib_ioctl,
 	.hw_params	= rsnd_hw_params,
-	.hw_free	= snd_pcm_lib_free_pages,
+	.hw_free	= rsnd_hw_free,
 	.pointer	= rsnd_pointer,
 };
 
diff --git a/sound/soc/sh/rcar/rsnd.h b/sound/soc/sh/rcar/rsnd.h
index 7727add3eb1a..ea6cbaa9743e 100644
--- a/sound/soc/sh/rcar/rsnd.h
+++ b/sound/soc/sh/rcar/rsnd.h
@@ -327,6 +327,9 @@ struct rsnd_mod_ops {
 	int (*cleanup)(struct rsnd_mod *mod,
 		       struct rsnd_dai_stream *io,
 		       struct rsnd_priv *priv);
+	int (*hw_free)(struct rsnd_mod *mod,
+		       struct rsnd_dai_stream *io,
+		       struct snd_pcm_substream *substream);
 	u32 *(*get_status)(struct rsnd_mod *mod,
 			   struct rsnd_dai_stream *io,
 			   enum rsnd_mod_type type);
@@ -351,12 +354,12 @@ struct rsnd_mod {
  *
  * B	0: init		1: quit
  * C	0: start	1: stop
+ * D	0: hw_params	1: hw_free
  *
  * H is always called (see __rsnd_mod_call)
  * H	0: probe	1: remove
  * H	0: pcm_new
  * H	0: fallback
- * H	0: hw_params
  * H	0: pointer
  * H	0: prepare
  * H	0: cleanup
@@ -365,12 +368,13 @@ struct rsnd_mod {
 #define __rsnd_mod_shift_quit		4
 #define __rsnd_mod_shift_start		8
 #define __rsnd_mod_shift_stop		8
+#define __rsnd_mod_shift_hw_params	12
+#define __rsnd_mod_shift_hw_free	12
 #define __rsnd_mod_shift_probe		28 /* always called */
 #define __rsnd_mod_shift_remove		28 /* always called */
 #define __rsnd_mod_shift_irq		28 /* always called */
 #define __rsnd_mod_shift_pcm_new	28 /* always called */
 #define __rsnd_mod_shift_fallback	28 /* always called */
-#define __rsnd_mod_shift_hw_params	28 /* always called */
 #define __rsnd_mod_shift_pointer	28 /* always called */
 #define __rsnd_mod_shift_prepare	28 /* always called */
 #define __rsnd_mod_shift_cleanup	28 /* always called */
@@ -383,10 +387,11 @@ struct rsnd_mod {
 #define __rsnd_mod_add_quit		-1
 #define __rsnd_mod_add_start		 1
 #define __rsnd_mod_add_stop		-1
+#define __rsnd_mod_add_hw_params	1
+#define __rsnd_mod_add_hw_free		-1
 #define __rsnd_mod_add_irq		0
 #define __rsnd_mod_add_pcm_new		0
 #define __rsnd_mod_add_fallback		0
-#define __rsnd_mod_add_hw_params	0
 #define __rsnd_mod_add_pointer		0
 
 #define __rsnd_mod_call_probe		0
@@ -402,6 +407,7 @@ struct rsnd_mod {
 #define __rsnd_mod_call_fallback	0
 #define __rsnd_mod_call_hw_params	0
 #define __rsnd_mod_call_pointer		0
+#define __rsnd_mod_call_hw_free		1
 
 #define rsnd_mod_to_priv(mod)	((mod)->priv)
 #define rsnd_mod_power_on(mod)	clk_enable((mod)->clk)
-- 
2.20.1

Applied "ASoC: rsnd: Support hw_free() callback at DAI level" to the asoc tree

[Mark Brown]

The patch

   ASoC: rsnd: Support hw_free() callback at DAI level

has been applied to the asoc tree at

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

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

>From 859fd6cbf1fb32b5428c26f837215c085b8a822e Mon Sep 17 00:00:00 2001
From: Timo Wischer <twischer@de.adit-jv.com>
Date: Mon, 22 Jul 2019 16:24:01 +0900
Subject: [PATCH] ASoC: rsnd: Support hw_free() callback at DAI level

This patch provides the needed infrastructure to support calling hw_free()
at the DAI level. This is for example required to free resources allocated
in hw_params() callback.

The modification of __rsnd_mod_add_hw_params does not have any side
effects because rsnd_mod_ops::hw_params callback is not used by anyone
until now.

Signed-off-by: Timo Wischer <twischer@de.adit-jv.com>
Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
Link: https://lore.kernel.org/r/20190722072403.11008-2-jiada_wang@mentor.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/sh/rcar/core.c | 16 +++++++++++++++-
 sound/soc/sh/rcar/rsnd.h | 12 +++++++++---
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c
index 56e8dae9a15c..bda5b958d0dc 100644
--- a/sound/soc/sh/rcar/core.c
+++ b/sound/soc/sh/rcar/core.c
@@ -1421,6 +1421,20 @@ static int rsnd_hw_params(struct snd_pcm_substream *substream,
 					params_buffer_bytes(hw_params));
 }
 
+static int rsnd_hw_free(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_dai *dai = rsnd_substream_to_dai(substream);
+	struct rsnd_dai *rdai = rsnd_dai_to_rdai(dai);
+	struct rsnd_dai_stream *io = rsnd_rdai_to_io(rdai, substream);
+	int ret;
+
+	ret = rsnd_dai_call(hw_free, io, substream);
+	if (ret)
+		return ret;
+
+	return snd_pcm_lib_free_pages(substream);
+}
+
 static snd_pcm_uframes_t rsnd_pointer(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_dai *dai = rsnd_substream_to_dai(substream);
@@ -1436,7 +1450,7 @@ static snd_pcm_uframes_t rsnd_pointer(struct snd_pcm_substream *substream)
 static const struct snd_pcm_ops rsnd_pcm_ops = {
 	.ioctl		= snd_pcm_lib_ioctl,
 	.hw_params	= rsnd_hw_params,
-	.hw_free	= snd_pcm_lib_free_pages,
+	.hw_free	= rsnd_hw_free,
 	.pointer	= rsnd_pointer,
 };
 
diff --git a/sound/soc/sh/rcar/rsnd.h b/sound/soc/sh/rcar/rsnd.h
index 7727add3eb1a..ea6cbaa9743e 100644
--- a/sound/soc/sh/rcar/rsnd.h
+++ b/sound/soc/sh/rcar/rsnd.h
@@ -327,6 +327,9 @@ struct rsnd_mod_ops {
 	int (*cleanup)(struct rsnd_mod *mod,
 		       struct rsnd_dai_stream *io,
 		       struct rsnd_priv *priv);
+	int (*hw_free)(struct rsnd_mod *mod,
+		       struct rsnd_dai_stream *io,
+		       struct snd_pcm_substream *substream);
 	u32 *(*get_status)(struct rsnd_mod *mod,
 			   struct rsnd_dai_stream *io,
 			   enum rsnd_mod_type type);
@@ -351,12 +354,12 @@ struct rsnd_mod {
  *
  * B	0: init		1: quit
  * C	0: start	1: stop
+ * D	0: hw_params	1: hw_free
  *
  * H is always called (see __rsnd_mod_call)
  * H	0: probe	1: remove
  * H	0: pcm_new
  * H	0: fallback
- * H	0: hw_params
  * H	0: pointer
  * H	0: prepare
  * H	0: cleanup
@@ -365,12 +368,13 @@ struct rsnd_mod {
 #define __rsnd_mod_shift_quit		4
 #define __rsnd_mod_shift_start		8
 #define __rsnd_mod_shift_stop		8
+#define __rsnd_mod_shift_hw_params	12
+#define __rsnd_mod_shift_hw_free	12
 #define __rsnd_mod_shift_probe		28 /* always called */
 #define __rsnd_mod_shift_remove		28 /* always called */
 #define __rsnd_mod_shift_irq		28 /* always called */
 #define __rsnd_mod_shift_pcm_new	28 /* always called */
 #define __rsnd_mod_shift_fallback	28 /* always called */
-#define __rsnd_mod_shift_hw_params	28 /* always called */
 #define __rsnd_mod_shift_pointer	28 /* always called */
 #define __rsnd_mod_shift_prepare	28 /* always called */
 #define __rsnd_mod_shift_cleanup	28 /* always called */
@@ -383,10 +387,11 @@ struct rsnd_mod {
 #define __rsnd_mod_add_quit		-1
 #define __rsnd_mod_add_start		 1
 #define __rsnd_mod_add_stop		-1
+#define __rsnd_mod_add_hw_params	1
+#define __rsnd_mod_add_hw_free		-1
 #define __rsnd_mod_add_irq		0
 #define __rsnd_mod_add_pcm_new		0
 #define __rsnd_mod_add_fallback		0
-#define __rsnd_mod_add_hw_params	0
 #define __rsnd_mod_add_pointer		0
 
 #define __rsnd_mod_call_probe		0
@@ -402,6 +407,7 @@ struct rsnd_mod {
 #define __rsnd_mod_call_fallback	0
 #define __rsnd_mod_call_hw_params	0
 #define __rsnd_mod_call_pointer		0
+#define __rsnd_mod_call_hw_free		1
 
 #define rsnd_mod_to_priv(mod)	((mod)->priv)
 #define rsnd_mod_power_on(mod)	clk_enable((mod)->clk)
-- 
2.20.1

Re: [PATCH v1 2/3] ASoC: rsnd: Allow reconfiguration of clock rate

[Jiada Wang]

Hi Morimoto-san

Sorry for the delayed response

On 2019/07/22 17:41, Kuninori Morimoto wrote:
> 
> Hi Jiada
> 
> The solution looks very over-kill to me,
> especiallyq [3/3] patch is too much to me.
> 
> 1st, can we start clock at .hw_param, instead of .prepare ?
> and stop it at .hw_free ?
> 
the reasoning to move start of clock from .init to .prepare by
commit 4d230d1271064 ("ASoC: rsnd: fixup not to call clk_get/set under 
non-atomic")
is to prevent clk_{get, set_rate} be called from atomic context,
since .hw_params is non atomic context,
so I think start of clock can be moved from .prepare to .hw_params

> 2nd, can we keep usrcnt setup as-is ?
> I guess we can just avoid rsnd_ssi_master_clk_start() if ssi->rate was not 0 ?

I don't fully understand your 2nd question,
in case of rsnd_ssi_master_clk_stop(), if avoid 
rsnd_ssi_master_clk_stop() when ssi->rate is 0 by apply following change

  	static void rsnd_ssi_master_clk_stop(struct rsnd_mod *mod,
  				     struct rsnd_dai_stream *io)
  	{
  		...
  -		if (ssi->usrcnt > 1)
  +		if (ssi->rate == 0)
  			return;
  		...
  	}

then when any IO stream with same SSI calls .hw_free,
the other IO stream's clock will be stopped too.

Thanks,
Jiada

> similar for rsnd_ssi_master_clk_stop()
> 
> 	static int rsnd_ssi_master_clk_start(struct rsnd_mod *mod,
> 				     struct rsnd_dai_stream *io)
> 	{
> 		...
> 		if (ssi->rate)
> 			return 0;
> 		...
> 	}
> 
> 	static void rsnd_ssi_master_clk_stop(struct rsnd_mod *mod,
> 				     struct rsnd_dai_stream *io)
> 	{
> 		...
> -		if (ssi->usrcnt > 1)
> +		if (ssi->rate == 0)
> 			return;
> 		...
> 	}
> 
> 
>> From: Timo Wischer <twischer@de.adit-jv.com>
>>
>> Currently after clock rate is started in .prepare reconfiguration
>> of clock rate is not allowed, unless the stream is stopped.
>>
>> But there is use case in Gstreamer ALSA sink, in case of changed caps
>> Gsreatmer reconfigs and it calls snd_pcm_hw_free() before snd_pcm_prepre().
>> See gstreamer1.0-plugins-base/
>> gst-libs/gst/audio/gstaudiobasesink.c: gst_audio_base_sink_setcaps():
>> - gst_audio_ring_buffer_release()
>> - gst_audio_sink_ring_buffer_release()
>> - gst_alsasink_unprepare()
>> - snd_pcm_hw_free()
>> is called before
>> - gst_audio_ring_buffer_acquire()
>> - gst_audio_sink_ring_buffer_acquire()
>> - gst_alsasink_prepare()
>> - set_hwparams()
>> - snd_pcm_hw_params()
>> - snd_pcm_prepare()
>>
>> The issue mentioned in this commit can be reproduced with the following
>> aplay patch:
>>
>>      >diff --git a/aplay/aplay.c b/aplay/aplay.c
>>      >@@ -2760,6 +2760,8 @@ static void playback_go(int fd, size_t loaded,
>>      >      header(rtype, name);
>>      >      set_params();
>>      >+     hwparams.rate = (hwparams.rate == 48000) ? 44100 : 48000;
>>      >+     set_params();
>>      >
>>      >      while (loaded > chunk_bytes && written < count && !in_aborting)
>>      >      {
>>      >              if (pcm_write(audiobuf + written, chunk_size) <= 0)
>>
>> $ aplay -Dplughw:0,0,0 -c8 -fS24_LE -r48000 /dev/zero
>> rcar_sound ec500000.sound: SSI parent/child should use same rate
>> rcar_sound ec500000.sound: ssi[3] : prepare error -22
>> rcar_sound ec500000.sound: ASoC: cpu DAI prepare error: -22
>> rsnd_link0: ASoC: prepare FE rsnd_link0 failed
>>
>> this patch address the issue by stop clock in .hw_free,
>> to allow reconfiguration of clock rate without stop of the stream.
>>
>> Signed-off-by: Timo Wischer <twischer@de.adit-jv.com>
>> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
>> ---
>>   sound/soc/sh/rcar/ssi.c | 53 +++++++++++++++++++++++++++++------------
>>   1 file changed, 38 insertions(+), 15 deletions(-)
>>
>> diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
>> index f6a7466622ea..f43937d2c588 100644
>> --- a/sound/soc/sh/rcar/ssi.c
>> +++ b/sound/soc/sh/rcar/ssi.c
>> @@ -286,7 +286,7 @@ static int rsnd_ssi_master_clk_start(struct rsnd_mod *mod,
>>   	if (rsnd_ssi_is_multi_slave(mod, io))
>>   		return 0;
>>   
>> -	if (ssi->usrcnt > 0) {
>> +	if (ssi->rate) {
>>   		if (ssi->rate != rate) {
>>   			dev_err(dev, "SSI parent/child should use same rate\n");
>>   			return -EINVAL;
>> @@ -471,13 +471,9 @@ static int rsnd_ssi_init(struct rsnd_mod *mod,
>>   			 struct rsnd_dai_stream *io,
>>   			 struct rsnd_priv *priv)
>>   {
>> -	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
>> -
>>   	if (!rsnd_ssi_is_run_mods(mod, io))
>>   		return 0;
>>   
>> -	ssi->usrcnt++;
>> -
>>   	rsnd_mod_power_on(mod);
>>   
>>   	rsnd_ssi_config_init(mod, io);
>> @@ -505,18 +501,8 @@ static int rsnd_ssi_quit(struct rsnd_mod *mod,
>>   		return -EIO;
>>   	}
>>   
>> -	rsnd_ssi_master_clk_stop(mod, io);
>> -
>>   	rsnd_mod_power_off(mod);
>>   
>> -	ssi->usrcnt--;
>> -
>> -	if (!ssi->usrcnt) {
>> -		ssi->cr_own	= 0;
>> -		ssi->cr_mode	= 0;
>> -		ssi->wsr	= 0;
>> -	}
>> -
>>   	return 0;
>>   }
>>   
>> @@ -525,6 +511,7 @@ static int rsnd_ssi_hw_params(struct rsnd_mod *mod,
>>   			      struct snd_pcm_substream *substream,
>>   			      struct snd_pcm_hw_params *params)
>>   {
>> +	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
>>   	struct rsnd_dai *rdai = rsnd_io_to_rdai(io);
>>   	unsigned int fmt_width = snd_pcm_format_width(params_format(params));
>>   
>> @@ -536,6 +523,11 @@ static int rsnd_ssi_hw_params(struct rsnd_mod *mod,
>>   		return -EINVAL;
>>   	}
>>   
>> +	if (!rsnd_ssi_is_run_mods(mod, io))
>> +		return 0;
>> +
>> +	ssi->usrcnt++;
>> +
>>   	return 0;
>>   }
>>   
>> @@ -913,6 +905,35 @@ static int rsnd_ssi_prepare(struct rsnd_mod *mod,
>>   	return rsnd_ssi_master_clk_start(mod, io);
>>   }
>>   
>> +static int rsnd_ssi_hw_free(struct rsnd_mod *mod, struct rsnd_dai_stream *io,
>> +			    struct snd_pcm_substream *substream)
>> +{
>> +	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
>> +
>> +	if (!rsnd_ssi_is_run_mods(mod, io))
>> +		return 0;
>> +
>> +	if (!ssi->usrcnt) {
>> +		struct rsnd_priv *priv = rsnd_mod_to_priv(mod);
>> +		struct device *dev = rsnd_priv_to_dev(priv);
>> +
>> +		dev_err(dev, "%s usrcnt error\n", rsnd_mod_name(mod));
>> +		return -EIO;
>> +	}
>> +
>> +	rsnd_ssi_master_clk_stop(mod, io);
>> +
>> +	ssi->usrcnt--;
>> +
>> +	if (!ssi->usrcnt) {
>> +		ssi->cr_own	= 0;
>> +		ssi->cr_mode	= 0;
>> +		ssi->wsr	= 0;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>   static struct rsnd_mod_ops rsnd_ssi_pio_ops = {
>>   	.name		= SSI_NAME,
>>   	.probe		= rsnd_ssi_common_probe,
>> @@ -926,6 +947,7 @@ static struct rsnd_mod_ops rsnd_ssi_pio_ops = {
>>   	.pcm_new	= rsnd_ssi_pcm_new,
>>   	.hw_params	= rsnd_ssi_hw_params,
>>   	.prepare	= rsnd_ssi_prepare,
>> +	.hw_free	= rsnd_ssi_hw_free,
>>   	.get_status	= rsnd_ssi_get_status,
>>   };
>>   
>> @@ -1012,6 +1034,7 @@ 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,
>> +	.hw_free	= rsnd_ssi_hw_free,
>>   	.prepare	= rsnd_ssi_prepare,
>>   	.get_status	= rsnd_ssi_get_status,
>>   };
>> -- 
>> 2.19.2
>>

Re: [PATCH v1 2/3] ASoC: rsnd: Allow reconfiguration of clock rate

[Jiada Wang]

Hi Morimoto-san

Sorry for the delayed response

On 2019/07/22 17:41, Kuninori Morimoto wrote:
> 
> Hi Jiada
> 
> The solution looks very over-kill to me,
> especiallyq [3/3] patch is too much to me.
> 
> 1st, can we start clock at .hw_param, instead of .prepare ?
> and stop it at .hw_free ?
> 
the reasoning to move start of clock from .init to .prepare by
commit 4d230d1271064 ("ASoC: rsnd: fixup not to call clk_get/set under 
non-atomic")
is to prevent clk_{get, set_rate} be called from atomic context,
since .hw_params is non atomic context,
so I think start of clock can be moved from .prepare to .hw_params

> 2nd, can we keep usrcnt setup as-is ?
> I guess we can just avoid rsnd_ssi_master_clk_start() if ssi->rate was not 0 ?

I don't fully understand your 2nd question,
in case of rsnd_ssi_master_clk_stop(), if avoid 
rsnd_ssi_master_clk_stop() when ssi->rate is 0 by apply following change

  	static void rsnd_ssi_master_clk_stop(struct rsnd_mod *mod,
  				     struct rsnd_dai_stream *io)
  	{
  		...
  -		if (ssi->usrcnt > 1)
  +		if (ssi->rate == 0)
  			return;
  		...
  	}

then when any IO stream with same SSI calls .hw_free,
the other IO stream's clock will be stopped too.

Thanks,
Jiada

> similar for rsnd_ssi_master_clk_stop()
> 
> 	static int rsnd_ssi_master_clk_start(struct rsnd_mod *mod,
> 				     struct rsnd_dai_stream *io)
> 	{
> 		...
> 		if (ssi->rate)
> 			return 0;
> 		...
> 	}
> 
> 	static void rsnd_ssi_master_clk_stop(struct rsnd_mod *mod,
> 				     struct rsnd_dai_stream *io)
> 	{
> 		...
> -		if (ssi->usrcnt > 1)
> +		if (ssi->rate == 0)
> 			return;
> 		...
> 	}
> 
> 
>> From: Timo Wischer <twischer@de.adit-jv.com>
>>
>> Currently after clock rate is started in .prepare reconfiguration
>> of clock rate is not allowed, unless the stream is stopped.
>>
>> But there is use case in Gstreamer ALSA sink, in case of changed caps
>> Gsreatmer reconfigs and it calls snd_pcm_hw_free() before snd_pcm_prepre().
>> See gstreamer1.0-plugins-base/
>> gst-libs/gst/audio/gstaudiobasesink.c: gst_audio_base_sink_setcaps():
>> - gst_audio_ring_buffer_release()
>> - gst_audio_sink_ring_buffer_release()
>> - gst_alsasink_unprepare()
>> - snd_pcm_hw_free()
>> is called before
>> - gst_audio_ring_buffer_acquire()
>> - gst_audio_sink_ring_buffer_acquire()
>> - gst_alsasink_prepare()
>> - set_hwparams()
>> - snd_pcm_hw_params()
>> - snd_pcm_prepare()
>>
>> The issue mentioned in this commit can be reproduced with the following
>> aplay patch:
>>
>>      >diff --git a/aplay/aplay.c b/aplay/aplay.c
>>      >@@ -2760,6 +2760,8 @@ static void playback_go(int fd, size_t loaded,
>>      >      header(rtype, name);
>>      >      set_params();
>>      >+     hwparams.rate = (hwparams.rate == 48000) ? 44100 : 48000;
>>      >+     set_params();
>>      >
>>      >      while (loaded > chunk_bytes && written < count && !in_aborting)
>>      >      {
>>      >              if (pcm_write(audiobuf + written, chunk_size) <= 0)
>>
>> $ aplay -Dplughw:0,0,0 -c8 -fS24_LE -r48000 /dev/zero
>> rcar_sound ec500000.sound: SSI parent/child should use same rate
>> rcar_sound ec500000.sound: ssi[3] : prepare error -22
>> rcar_sound ec500000.sound: ASoC: cpu DAI prepare error: -22
>> rsnd_link0: ASoC: prepare FE rsnd_link0 failed
>>
>> this patch address the issue by stop clock in .hw_free,
>> to allow reconfiguration of clock rate without stop of the stream.
>>
>> Signed-off-by: Timo Wischer <twischer@de.adit-jv.com>
>> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
>> ---
>>   sound/soc/sh/rcar/ssi.c | 53 +++++++++++++++++++++++++++++------------
>>   1 file changed, 38 insertions(+), 15 deletions(-)
>>
>> diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
>> index f6a7466622ea..f43937d2c588 100644
>> --- a/sound/soc/sh/rcar/ssi.c
>> +++ b/sound/soc/sh/rcar/ssi.c
>> @@ -286,7 +286,7 @@ static int rsnd_ssi_master_clk_start(struct rsnd_mod *mod,
>>   	if (rsnd_ssi_is_multi_slave(mod, io))
>>   		return 0;
>>   
>> -	if (ssi->usrcnt > 0) {
>> +	if (ssi->rate) {
>>   		if (ssi->rate != rate) {
>>   			dev_err(dev, "SSI parent/child should use same rate\n");
>>   			return -EINVAL;
>> @@ -471,13 +471,9 @@ static int rsnd_ssi_init(struct rsnd_mod *mod,
>>   			 struct rsnd_dai_stream *io,
>>   			 struct rsnd_priv *priv)
>>   {
>> -	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
>> -
>>   	if (!rsnd_ssi_is_run_mods(mod, io))
>>   		return 0;
>>   
>> -	ssi->usrcnt++;
>> -
>>   	rsnd_mod_power_on(mod);
>>   
>>   	rsnd_ssi_config_init(mod, io);
>> @@ -505,18 +501,8 @@ static int rsnd_ssi_quit(struct rsnd_mod *mod,
>>   		return -EIO;
>>   	}
>>   
>> -	rsnd_ssi_master_clk_stop(mod, io);
>> -
>>   	rsnd_mod_power_off(mod);
>>   
>> -	ssi->usrcnt--;
>> -
>> -	if (!ssi->usrcnt) {
>> -		ssi->cr_own	= 0;
>> -		ssi->cr_mode	= 0;
>> -		ssi->wsr	= 0;
>> -	}
>> -
>>   	return 0;
>>   }
>>   
>> @@ -525,6 +511,7 @@ static int rsnd_ssi_hw_params(struct rsnd_mod *mod,
>>   			      struct snd_pcm_substream *substream,
>>   			      struct snd_pcm_hw_params *params)
>>   {
>> +	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
>>   	struct rsnd_dai *rdai = rsnd_io_to_rdai(io);
>>   	unsigned int fmt_width = snd_pcm_format_width(params_format(params));
>>   
>> @@ -536,6 +523,11 @@ static int rsnd_ssi_hw_params(struct rsnd_mod *mod,
>>   		return -EINVAL;
>>   	}
>>   
>> +	if (!rsnd_ssi_is_run_mods(mod, io))
>> +		return 0;
>> +
>> +	ssi->usrcnt++;
>> +
>>   	return 0;
>>   }
>>   
>> @@ -913,6 +905,35 @@ static int rsnd_ssi_prepare(struct rsnd_mod *mod,
>>   	return rsnd_ssi_master_clk_start(mod, io);
>>   }
>>   
>> +static int rsnd_ssi_hw_free(struct rsnd_mod *mod, struct rsnd_dai_stream *io,
>> +			    struct snd_pcm_substream *substream)
>> +{
>> +	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
>> +
>> +	if (!rsnd_ssi_is_run_mods(mod, io))
>> +		return 0;
>> +
>> +	if (!ssi->usrcnt) {
>> +		struct rsnd_priv *priv = rsnd_mod_to_priv(mod);
>> +		struct device *dev = rsnd_priv_to_dev(priv);
>> +
>> +		dev_err(dev, "%s usrcnt error\n", rsnd_mod_name(mod));
>> +		return -EIO;
>> +	}
>> +
>> +	rsnd_ssi_master_clk_stop(mod, io);
>> +
>> +	ssi->usrcnt--;
>> +
>> +	if (!ssi->usrcnt) {
>> +		ssi->cr_own	= 0;
>> +		ssi->cr_mode	= 0;
>> +		ssi->wsr	= 0;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>   static struct rsnd_mod_ops rsnd_ssi_pio_ops = {
>>   	.name		= SSI_NAME,
>>   	.probe		= rsnd_ssi_common_probe,
>> @@ -926,6 +947,7 @@ static struct rsnd_mod_ops rsnd_ssi_pio_ops = {
>>   	.pcm_new	= rsnd_ssi_pcm_new,
>>   	.hw_params	= rsnd_ssi_hw_params,
>>   	.prepare	= rsnd_ssi_prepare,
>> +	.hw_free	= rsnd_ssi_hw_free,
>>   	.get_status	= rsnd_ssi_get_status,
>>   };
>>   
>> @@ -1012,6 +1034,7 @@ 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,
>> +	.hw_free	= rsnd_ssi_hw_free,
>>   	.prepare	= rsnd_ssi_prepare,
>>   	.get_status	= rsnd_ssi_get_status,
>>   };
>> -- 
>> 2.19.2
>>

Re: [PATCH v1 2/3] ASoC: rsnd: Allow reconfiguration of clock rate

[Kuninori Morimoto]

Hi Jiada

> > 2nd, can we keep usrcnt setup as-is ?
> > I guess we can just avoid rsnd_ssi_master_clk_start() if ssi->rate was not 0 ?
> 
> I don't fully understand your 2nd question,
> in case of rsnd_ssi_master_clk_stop(), if avoid
> rsnd_ssi_master_clk_stop() when ssi->rate is 0 by apply following
> change
> 
>  	static void rsnd_ssi_master_clk_stop(struct rsnd_mod *mod,
>  				     struct rsnd_dai_stream *io)
>  	{
>  		...
>  -		if (ssi->usrcnt > 1)
>  +		if (ssi->rate == 0)
>  			return;
>  		...
>  	}
> 
> then when any IO stream with same SSI calls .hw_free,
> the other IO stream's clock will be stopped too.

I think we can find more simple solution if we can find good ideas.
For example, how about to add new counter for hw_params/hw_free ?
Anyway, [3/3] patch is too much over-kill to me.

And, please don't exchange usrcnt inc/dec position at [2/3].
It is for open/close.

Thank you for your help !!
Best regards
---
Kuninori Morimoto

Re: [PATCH v1 2/3] ASoC: rsnd: Allow reconfiguration of clock rate

[Kuninori Morimoto]

Hi Jiada

> > 2nd, can we keep usrcnt setup as-is ?
> > I guess we can just avoid rsnd_ssi_master_clk_start() if ssi->rate was not 0 ?
> 
> I don't fully understand your 2nd question,
> in case of rsnd_ssi_master_clk_stop(), if avoid
> rsnd_ssi_master_clk_stop() when ssi->rate is 0 by apply following
> change
> 
>  	static void rsnd_ssi_master_clk_stop(struct rsnd_mod *mod,
>  				     struct rsnd_dai_stream *io)
>  	{
>  		...
>  -		if (ssi->usrcnt > 1)
>  +		if (ssi->rate == 0)
>  			return;
>  		...
>  	}
> 
> then when any IO stream with same SSI calls .hw_free,
> the other IO stream's clock will be stopped too.

I think we can find more simple solution if we can find good ideas.
For example, how about to add new counter for hw_params/hw_free ?
Anyway, [3/3] patch is too much over-kill to me.

And, please don't exchange usrcnt inc/dec position at [2/3].
It is for open/close.

Thank you for your help !!
Best regards
---
Kuninori Morimoto