[07/10] ASoC: soc-pcm: add snd_soc_dpcm_can_be() and remove duplicate code
diff mbox series

Message ID 871rqzru2y.wl-kuninori.morimoto.gx@renesas.com
State New
Headers show
Series
  • ASoC: soc-pcm cleanup step3
Related show

Commit Message

Kuninori Morimoto Feb. 13, 2020, 4:26 a.m. UTC
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Below functions are doing very similar things, the difference is
only used state.

	snd_soc_dpcm_can_be_free_stop()
	snd_soc_dpcm_can_be_params()

This patch adds common snd_soc_dpcm_can_be(), and use it from
snd_soc_dpcm_can_be_free_stop() / snd_soc_dpcm_can_be_params().
This can reduce duplicate code.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-pcm.c | 70 ++++++++++++++++++++++-----------------------
 1 file changed, 35 insertions(+), 35 deletions(-)

Comments

Pierre-Louis Bossart Feb. 13, 2020, 3:56 p.m. UTC | #1
> +int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe,
> +		struct snd_soc_pcm_runtime *be, int stream)
> +{
> +	enum snd_soc_dpcm_state state[] = {
> +		SND_SOC_DPCM_STATE_START,
> +		SND_SOC_DPCM_STATE_PAUSED,
> +		SND_SOC_DPCM_STATE_SUSPEND,
> +	};

should this be const?


> +	enum snd_soc_dpcm_state state[] = {
> +		SND_SOC_DPCM_STATE_START,
> +		SND_SOC_DPCM_STATE_PAUSED,
> +		SND_SOC_DPCM_STATE_SUSPEND,
> +		SND_SOC_DPCM_STATE_PREPARE,
> +	};

const as well?
Ranjani Sridharan Feb. 13, 2020, 5:15 p.m. UTC | #2
On Thu, 2020-02-13 at 13:26 +0900, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> Below functions are doing very similar things, the difference is
> only used state.
> 
> 	snd_soc_dpcm_can_be_free_stop()
> 	snd_soc_dpcm_can_be_params()
> 
> This patch adds common snd_soc_dpcm_can_be(), and use it from
> snd_soc_dpcm_can_be_free_stop() / snd_soc_dpcm_can_be_params().
> This can reduce duplicate code.

Morimoto-san,

Only minor but does it make it a bit more intuitive to call this new
function snd_soc_dpcm_check_state() or something equivalent maybe?

Thanks,
Ranjani
Kuninori Morimoto Feb. 14, 2020, 12:04 a.m. UTC | #3
Hi Pierre-Louis

> > +int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe,
> > +		struct snd_soc_pcm_runtime *be, int stream)
> > +{
> > +	enum snd_soc_dpcm_state state[] = {
> > +		SND_SOC_DPCM_STATE_START,
> > +		SND_SOC_DPCM_STATE_PAUSED,
> > +		SND_SOC_DPCM_STATE_SUSPEND,
> > +	};
> 
> should this be const?
> 
> 
> > +	enum snd_soc_dpcm_state state[] = {
> > +		SND_SOC_DPCM_STATE_START,
> > +		SND_SOC_DPCM_STATE_PAUSED,
> > +		SND_SOC_DPCM_STATE_SUSPEND,
> > +		SND_SOC_DPCM_STATE_PREPARE,
> > +	};
> 
> const as well?

Yeah, indeed.
Thanks. Will fix in v2

Thank you for your help !!
Best regards
---
Kuninori Morimoto
Kuninori Morimoto Feb. 14, 2020, 12:08 a.m. UTC | #4
Hi Ranjani

> > Below functions are doing very similar things, the difference is
> > only used state.
> > 
> > 	snd_soc_dpcm_can_be_free_stop()
> > 	snd_soc_dpcm_can_be_params()
> > 
> > This patch adds common snd_soc_dpcm_can_be(), and use it from
> > snd_soc_dpcm_can_be_free_stop() / snd_soc_dpcm_can_be_params().
> > This can reduce duplicate code.
> 
> Morimoto-san,
> 
> Only minor but does it make it a bit more intuitive to call this new
> function snd_soc_dpcm_check_state() or something equivalent maybe?

Thank you for pointing it.
I like understandable naming :)
Will fix it in v2

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

Patch
diff mbox series

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index acc8a7eabfbe..140f07b91d6a 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -2957,17 +2957,17 @@  struct snd_pcm_substream *
 }
 EXPORT_SYMBOL_GPL(snd_soc_dpcm_get_substream);
 
-/*
- * We can only hw_free, stop, pause or suspend a BE DAI if any of it's FE
- * are not running, paused or suspended for the specified stream direction.
- */
-int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe,
-		struct snd_soc_pcm_runtime *be, int stream)
+static int snd_soc_dpcm_can_be(struct snd_soc_pcm_runtime *fe,
+			       struct snd_soc_pcm_runtime *be,
+			       int stream,
+			       enum snd_soc_dpcm_state *states,
+			       int num_states)
 {
 	struct snd_soc_dpcm *dpcm;
 	int state;
 	int ret = 1;
 	unsigned long flags;
+	int i;
 
 	spin_lock_irqsave(&fe->card->dpcm_lock, flags);
 	for_each_dpcm_fe(be, stream, dpcm) {
@@ -2976,18 +2976,34 @@  int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe,
 			continue;
 
 		state = dpcm->fe->dpcm[stream].state;
-		if (state == SND_SOC_DPCM_STATE_START ||
-			state == SND_SOC_DPCM_STATE_PAUSED ||
-			state == SND_SOC_DPCM_STATE_SUSPEND) {
-			ret = 0;
-			break;
+		for (i = 0; i < num_states; i++) {
+			if (state == states[i]) {
+				ret = 0;
+				break;
+			}
 		}
 	}
 	spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
 
-	/* it's safe to free/stop this BE DAI */
+	/* it's safe to do this BE DAI */
 	return ret;
 }
+
+/*
+ * We can only hw_free, stop, pause or suspend a BE DAI if any of it's FE
+ * are not running, paused or suspended for the specified stream direction.
+ */
+int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe,
+		struct snd_soc_pcm_runtime *be, int stream)
+{
+	enum snd_soc_dpcm_state state[] = {
+		SND_SOC_DPCM_STATE_START,
+		SND_SOC_DPCM_STATE_PAUSED,
+		SND_SOC_DPCM_STATE_SUSPEND,
+	};
+
+	return snd_soc_dpcm_can_be(fe, be, stream, state, ARRAY_SIZE(state));
+}
 EXPORT_SYMBOL_GPL(snd_soc_dpcm_can_be_free_stop);
 
 /*
@@ -2997,30 +3013,14 @@  EXPORT_SYMBOL_GPL(snd_soc_dpcm_can_be_free_stop);
 int snd_soc_dpcm_can_be_params(struct snd_soc_pcm_runtime *fe,
 		struct snd_soc_pcm_runtime *be, int stream)
 {
-	struct snd_soc_dpcm *dpcm;
-	int state;
-	int ret = 1;
-	unsigned long flags;
-
-	spin_lock_irqsave(&fe->card->dpcm_lock, flags);
-	for_each_dpcm_fe(be, stream, dpcm) {
-
-		if (dpcm->fe == fe)
-			continue;
+	enum snd_soc_dpcm_state state[] = {
+		SND_SOC_DPCM_STATE_START,
+		SND_SOC_DPCM_STATE_PAUSED,
+		SND_SOC_DPCM_STATE_SUSPEND,
+		SND_SOC_DPCM_STATE_PREPARE,
+	};
 
-		state = dpcm->fe->dpcm[stream].state;
-		if (state == SND_SOC_DPCM_STATE_START ||
-			state == SND_SOC_DPCM_STATE_PAUSED ||
-			state == SND_SOC_DPCM_STATE_SUSPEND ||
-			state == SND_SOC_DPCM_STATE_PREPARE) {
-			ret = 0;
-			break;
-		}
-	}
-	spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
-
-	/* it's safe to change hw_params */
-	return ret;
+	return snd_soc_dpcm_can_be(fe, be, stream, state, ARRAY_SIZE(state));
 }
 EXPORT_SYMBOL_GPL(snd_soc_dpcm_can_be_params);