linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ASoC: AMD: Enable clk in startup intead of hw_params
@ 2019-11-26  7:54 Yu-Hsuan Hsu
  2019-11-26 10:11 ` Agrawal, Akshu
  2019-11-28 13:18 ` Applied "ASoC: AMD: Enable clk in startup intead of hw_params" to the asoc tree Mark Brown
  0 siblings, 2 replies; 3+ messages in thread
From: Yu-Hsuan Hsu @ 2019-11-26  7:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	Agrawal Akshu, Andi Kleen, Yu-Hsuan Hsu, Adam Thomson,
	Kuninori Morimoto, alsa-devel, Tzung-Bi Shih

Some usages only call startup and shutdown without setting hw_params
(e.g. arecord --dump-hw-params). If we don't enable clk in startup, it
will cause ref count error because the clk will be disabled in shutdown.
For this reason, we should move enabling clk from hw_params to startup.

In addition, the hw_params is fixed in this driver(48000 rate, 2
channels, S16_LE format) so we don't need to change the clk rate after
the hw_params is set.

Signed-off-by: Yu-Hsuan Hsu <yuhsuan@chromium.org>
---
 sound/soc/amd/acp-da7219-max98357a.c | 46 +++++++++-------------------
 1 file changed, 14 insertions(+), 32 deletions(-)

diff --git a/sound/soc/amd/acp-da7219-max98357a.c b/sound/soc/amd/acp-da7219-max98357a.c
index f4ee6798154af5..7a5621e5e2330d 100644
--- a/sound/soc/amd/acp-da7219-max98357a.c
+++ b/sound/soc/amd/acp-da7219-max98357a.c
@@ -96,14 +96,19 @@ static int cz_da7219_init(struct snd_soc_pcm_runtime *rtd)
 	return 0;
 }
 
-static int da7219_clk_enable(struct snd_pcm_substream *substream,
-			     int wclk_rate, int bclk_rate)
+static int da7219_clk_enable(struct snd_pcm_substream *substream)
 {
 	int ret = 0;
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 
-	clk_set_rate(da7219_dai_wclk, wclk_rate);
-	clk_set_rate(da7219_dai_bclk, bclk_rate);
+	/*
+	 * Set wclk to 48000 because the rate constraint of this driver is
+	 * 48000. ADAU7002 spec: "The ADAU7002 requires a BCLK rate that is
+	 * minimum of 64x the LRCLK sample rate." DA7219 is the only clk
+	 * source so for all codecs we have to limit bclk to 64X lrclk.
+	 */
+	clk_set_rate(da7219_dai_wclk, 48000);
+	clk_set_rate(da7219_dai_bclk, 48000 * 64);
 	ret = clk_prepare_enable(da7219_dai_bclk);
 	if (ret < 0) {
 		dev_err(rtd->dev, "can't enable master clock %d\n", ret);
@@ -156,7 +161,7 @@ static int cz_da7219_play_startup(struct snd_pcm_substream *substream)
 				   &constraints_rates);
 
 	machine->play_i2s_instance = I2S_SP_INSTANCE;
-	return 0;
+	return da7219_clk_enable(substream);
 }
 
 static int cz_da7219_cap_startup(struct snd_pcm_substream *substream)
@@ -178,7 +183,7 @@ static int cz_da7219_cap_startup(struct snd_pcm_substream *substream)
 
 	machine->cap_i2s_instance = I2S_SP_INSTANCE;
 	machine->capture_channel = CAP_CHANNEL1;
-	return 0;
+	return da7219_clk_enable(substream);
 }
 
 static int cz_max_startup(struct snd_pcm_substream *substream)
@@ -199,7 +204,7 @@ static int cz_max_startup(struct snd_pcm_substream *substream)
 				   &constraints_rates);
 
 	machine->play_i2s_instance = I2S_BT_INSTANCE;
-	return 0;
+	return da7219_clk_enable(substream);
 }
 
 static int cz_dmic0_startup(struct snd_pcm_substream *substream)
@@ -220,7 +225,7 @@ static int cz_dmic0_startup(struct snd_pcm_substream *substream)
 				   &constraints_rates);
 
 	machine->cap_i2s_instance = I2S_BT_INSTANCE;
-	return 0;
+	return da7219_clk_enable(substream);
 }
 
 static int cz_dmic1_startup(struct snd_pcm_substream *substream)
@@ -242,25 +247,7 @@ static int cz_dmic1_startup(struct snd_pcm_substream *substream)
 
 	machine->cap_i2s_instance = I2S_SP_INSTANCE;
 	machine->capture_channel = CAP_CHANNEL0;
-	return 0;
-}
-
-static int cz_da7219_params(struct snd_pcm_substream *substream,
-				      struct snd_pcm_hw_params *params)
-{
-	int wclk, bclk;
-
-	wclk = params_rate(params);
-	bclk = wclk * params_channels(params) *
-		snd_pcm_format_width(params_format(params));
-	/* ADAU7002 spec: "The ADAU7002 requires a BCLK rate
-	 * that is minimum of 64x the LRCLK sample rate."
-	 * DA7219 is the only clk source so for all codecs
-	 * we have to limit bclk to 64X lrclk.
-	 */
-	if (bclk < (wclk * 64))
-		bclk = wclk * 64;
-	return da7219_clk_enable(substream, wclk, bclk);
+	return da7219_clk_enable(substream);
 }
 
 static void cz_da7219_shutdown(struct snd_pcm_substream *substream)
@@ -271,31 +258,26 @@ static void cz_da7219_shutdown(struct snd_pcm_substream *substream)
 static const struct snd_soc_ops cz_da7219_play_ops = {
 	.startup = cz_da7219_play_startup,
 	.shutdown = cz_da7219_shutdown,
-	.hw_params = cz_da7219_params,
 };
 
 static const struct snd_soc_ops cz_da7219_cap_ops = {
 	.startup = cz_da7219_cap_startup,
 	.shutdown = cz_da7219_shutdown,
-	.hw_params = cz_da7219_params,
 };
 
 static const struct snd_soc_ops cz_max_play_ops = {
 	.startup = cz_max_startup,
 	.shutdown = cz_da7219_shutdown,
-	.hw_params = cz_da7219_params,
 };
 
 static const struct snd_soc_ops cz_dmic0_cap_ops = {
 	.startup = cz_dmic0_startup,
 	.shutdown = cz_da7219_shutdown,
-	.hw_params = cz_da7219_params,
 };
 
 static const struct snd_soc_ops cz_dmic1_cap_ops = {
 	.startup = cz_dmic1_startup,
 	.shutdown = cz_da7219_shutdown,
-	.hw_params = cz_da7219_params,
 };
 
 SND_SOC_DAILINK_DEF(designware1,
-- 
2.24.0.432.g9d3f5f5b63-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] ASoC: AMD: Enable clk in startup intead of hw_params
  2019-11-26  7:54 [PATCH] ASoC: AMD: Enable clk in startup intead of hw_params Yu-Hsuan Hsu
@ 2019-11-26 10:11 ` Agrawal, Akshu
  2019-11-28 13:18 ` Applied "ASoC: AMD: Enable clk in startup intead of hw_params" to the asoc tree Mark Brown
  1 sibling, 0 replies; 3+ messages in thread
From: Agrawal, Akshu @ 2019-11-26 10:11 UTC (permalink / raw)
  To: Yu-Hsuan Hsu, linux-kernel
  Cc: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	Agrawal Akshu, Andi Kleen, Adam Thomson, Kuninori Morimoto,
	alsa-devel, Tzung-Bi Shih


On 11/26/2019 1:24 PM, Yu-Hsuan Hsu wrote:
> Some usages only call startup and shutdown without setting hw_params
> (e.g. arecord --dump-hw-params). If we don't enable clk in startup, it
> will cause ref count error because the clk will be disabled in shutdown.
> For this reason, we should move enabling clk from hw_params to startup.
>
> In addition, the hw_params is fixed in this driver(48000 rate, 2
> channels, S16_LE format) so we don't need to change the clk rate after
> the hw_params is set.
>
> Signed-off-by: Yu-Hsuan Hsu <yuhsuan@chromium.org>
> ---
>   sound/soc/amd/acp-da7219-max98357a.c | 46 +++++++++-------------------
>   1 file changed, 14 insertions(+), 32 deletions(-)
>
> diff --git a/sound/soc/amd/acp-da7219-max98357a.c b/sound/soc/amd/acp-da7219-max98357a.c
> index f4ee6798154af5..7a5621e5e2330d 100644
> --- a/sound/soc/amd/acp-da7219-max98357a.c
> +++ b/sound/soc/amd/acp-da7219-max98357a.c
> @@ -96,14 +96,19 @@ static int cz_da7219_init(struct snd_soc_pcm_runtime *rtd)
>   	return 0;
>   }
>   
> -static int da7219_clk_enable(struct snd_pcm_substream *substream,
> -			     int wclk_rate, int bclk_rate)
> +static int da7219_clk_enable(struct snd_pcm_substream *substream)
>   {
>   	int ret = 0;
>   	struct snd_soc_pcm_runtime *rtd = substream->private_data;
>   
> -	clk_set_rate(da7219_dai_wclk, wclk_rate);
> -	clk_set_rate(da7219_dai_bclk, bclk_rate);
> +	/*
> +	 * Set wclk to 48000 because the rate constraint of this driver is
> +	 * 48000. ADAU7002 spec: "The ADAU7002 requires a BCLK rate that is
> +	 * minimum of 64x the LRCLK sample rate." DA7219 is the only clk
> +	 * source so for all codecs we have to limit bclk to 64X lrclk.
> +	 */
> +	clk_set_rate(da7219_dai_wclk, 48000);
> +	clk_set_rate(da7219_dai_bclk, 48000 * 64);
>   	ret = clk_prepare_enable(da7219_dai_bclk);
>   	if (ret < 0) {
>   		dev_err(rtd->dev, "can't enable master clock %d\n", ret);
> @@ -156,7 +161,7 @@ static int cz_da7219_play_startup(struct snd_pcm_substream *substream)
>   				   &constraints_rates);
>   
>   	machine->play_i2s_instance = I2S_SP_INSTANCE;
> -	return 0;
> +	return da7219_clk_enable(substream);
>   }
>   
>   static int cz_da7219_cap_startup(struct snd_pcm_substream *substream)
> @@ -178,7 +183,7 @@ static int cz_da7219_cap_startup(struct snd_pcm_substream *substream)
>   
>   	machine->cap_i2s_instance = I2S_SP_INSTANCE;
>   	machine->capture_channel = CAP_CHANNEL1;
> -	return 0;
> +	return da7219_clk_enable(substream);
>   }
>   
>   static int cz_max_startup(struct snd_pcm_substream *substream)
> @@ -199,7 +204,7 @@ static int cz_max_startup(struct snd_pcm_substream *substream)
>   				   &constraints_rates);
>   
>   	machine->play_i2s_instance = I2S_BT_INSTANCE;
> -	return 0;
> +	return da7219_clk_enable(substream);
>   }
>   
>   static int cz_dmic0_startup(struct snd_pcm_substream *substream)
> @@ -220,7 +225,7 @@ static int cz_dmic0_startup(struct snd_pcm_substream *substream)
>   				   &constraints_rates);
>   
>   	machine->cap_i2s_instance = I2S_BT_INSTANCE;
> -	return 0;
> +	return da7219_clk_enable(substream);
>   }
>   
>   static int cz_dmic1_startup(struct snd_pcm_substream *substream)
> @@ -242,25 +247,7 @@ static int cz_dmic1_startup(struct snd_pcm_substream *substream)
>   
>   	machine->cap_i2s_instance = I2S_SP_INSTANCE;
>   	machine->capture_channel = CAP_CHANNEL0;
> -	return 0;
> -}
> -
> -static int cz_da7219_params(struct snd_pcm_substream *substream,
> -				      struct snd_pcm_hw_params *params)
> -{
> -	int wclk, bclk;
> -
> -	wclk = params_rate(params);
> -	bclk = wclk * params_channels(params) *
> -		snd_pcm_format_width(params_format(params));
> -	/* ADAU7002 spec: "The ADAU7002 requires a BCLK rate
> -	 * that is minimum of 64x the LRCLK sample rate."
> -	 * DA7219 is the only clk source so for all codecs
> -	 * we have to limit bclk to 64X lrclk.
> -	 */
> -	if (bclk < (wclk * 64))
> -		bclk = wclk * 64;
> -	return da7219_clk_enable(substream, wclk, bclk);
> +	return da7219_clk_enable(substream);
>   }
>   
>   static void cz_da7219_shutdown(struct snd_pcm_substream *substream)
> @@ -271,31 +258,26 @@ static void cz_da7219_shutdown(struct snd_pcm_substream *substream)
>   static const struct snd_soc_ops cz_da7219_play_ops = {
>   	.startup = cz_da7219_play_startup,
>   	.shutdown = cz_da7219_shutdown,
> -	.hw_params = cz_da7219_params,
>   };
>   
>   static const struct snd_soc_ops cz_da7219_cap_ops = {
>   	.startup = cz_da7219_cap_startup,
>   	.shutdown = cz_da7219_shutdown,
> -	.hw_params = cz_da7219_params,
>   };
>   
>   static const struct snd_soc_ops cz_max_play_ops = {
>   	.startup = cz_max_startup,
>   	.shutdown = cz_da7219_shutdown,
> -	.hw_params = cz_da7219_params,
>   };
>   
>   static const struct snd_soc_ops cz_dmic0_cap_ops = {
>   	.startup = cz_dmic0_startup,
>   	.shutdown = cz_da7219_shutdown,
> -	.hw_params = cz_da7219_params,
>   };
>   
>   static const struct snd_soc_ops cz_dmic1_cap_ops = {
>   	.startup = cz_dmic1_startup,
>   	.shutdown = cz_da7219_shutdown,
> -	.hw_params = cz_da7219_params,
>   };
>   
>   SND_SOC_DAILINK_DEF(designware1,

Acked-by: Akshu Agrawal <akshu.agarawal@amd.com>


^ permalink raw reply	[flat|nested] 3+ messages in thread
* Applied "ASoC: AMD: Enable clk in startup intead of hw_params" to the asoc tree
  2019-11-26  7:54 [PATCH] ASoC: AMD: Enable clk in startup intead of hw_params Yu-Hsuan Hsu
  2019-11-26 10:11 ` Agrawal, Akshu
@ 2019-11-28 13:18 ` Mark Brown
  1 sibling, 0 replies; 3+ messages in thread
From: Mark Brown @ 2019-11-28 13:18 UTC (permalink / raw)
  To: Yu-Hsuan Hsu
  Cc: Adam Thomson, Agrawal Akshu, Akshu Agrawal, alsa-devel,
	Andi Kleen, Jaroslav Kysela, Kuninori Morimoto, Liam Girdwood,
	linux-kernel, Mark Brown, Takashi Iwai, Tzung-Bi Shih

The patch

   ASoC: AMD: Enable clk in startup intead of hw_params

has been applied to the asoc tree at

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

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 756ae8f237e19a014a1c20ad5a51b0686463d1f7 Mon Sep 17 00:00:00 2001
From: Yu-Hsuan Hsu <yuhsuan@chromium.org>
Date: Tue, 26 Nov 2019 15:54:24 +0800
Subject: [PATCH] ASoC: AMD: Enable clk in startup intead of hw_params

Some usages only call startup and shutdown without setting hw_params
(e.g. arecord --dump-hw-params). If we don't enable clk in startup, it
will cause ref count error because the clk will be disabled in shutdown.
For this reason, we should move enabling clk from hw_params to startup.

In addition, the hw_params is fixed in this driver(48000 rate, 2
channels, S16_LE format) so we don't need to change the clk rate after
the hw_params is set.

Signed-off-by: Yu-Hsuan Hsu <yuhsuan@chromium.org>
Acked-by: Akshu Agrawal <akshu.agarawal@amd.com>
Link: https://lore.kernel.org/r/20191126075424.80668-1-yuhsuan@chromium.org
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/amd/acp-da7219-max98357a.c | 46 +++++++++-------------------
 1 file changed, 14 insertions(+), 32 deletions(-)

diff --git a/sound/soc/amd/acp-da7219-max98357a.c b/sound/soc/amd/acp-da7219-max98357a.c
index f4ee6798154a..7a5621e5e233 100644
--- a/sound/soc/amd/acp-da7219-max98357a.c
+++ b/sound/soc/amd/acp-da7219-max98357a.c
@@ -96,14 +96,19 @@ static int cz_da7219_init(struct snd_soc_pcm_runtime *rtd)
 	return 0;
 }
 
-static int da7219_clk_enable(struct snd_pcm_substream *substream,
-			     int wclk_rate, int bclk_rate)
+static int da7219_clk_enable(struct snd_pcm_substream *substream)
 {
 	int ret = 0;
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 
-	clk_set_rate(da7219_dai_wclk, wclk_rate);
-	clk_set_rate(da7219_dai_bclk, bclk_rate);
+	/*
+	 * Set wclk to 48000 because the rate constraint of this driver is
+	 * 48000. ADAU7002 spec: "The ADAU7002 requires a BCLK rate that is
+	 * minimum of 64x the LRCLK sample rate." DA7219 is the only clk
+	 * source so for all codecs we have to limit bclk to 64X lrclk.
+	 */
+	clk_set_rate(da7219_dai_wclk, 48000);
+	clk_set_rate(da7219_dai_bclk, 48000 * 64);
 	ret = clk_prepare_enable(da7219_dai_bclk);
 	if (ret < 0) {
 		dev_err(rtd->dev, "can't enable master clock %d\n", ret);
@@ -156,7 +161,7 @@ static int cz_da7219_play_startup(struct snd_pcm_substream *substream)
 				   &constraints_rates);
 
 	machine->play_i2s_instance = I2S_SP_INSTANCE;
-	return 0;
+	return da7219_clk_enable(substream);
 }
 
 static int cz_da7219_cap_startup(struct snd_pcm_substream *substream)
@@ -178,7 +183,7 @@ static int cz_da7219_cap_startup(struct snd_pcm_substream *substream)
 
 	machine->cap_i2s_instance = I2S_SP_INSTANCE;
 	machine->capture_channel = CAP_CHANNEL1;
-	return 0;
+	return da7219_clk_enable(substream);
 }
 
 static int cz_max_startup(struct snd_pcm_substream *substream)
@@ -199,7 +204,7 @@ static int cz_max_startup(struct snd_pcm_substream *substream)
 				   &constraints_rates);
 
 	machine->play_i2s_instance = I2S_BT_INSTANCE;
-	return 0;
+	return da7219_clk_enable(substream);
 }
 
 static int cz_dmic0_startup(struct snd_pcm_substream *substream)
@@ -220,7 +225,7 @@ static int cz_dmic0_startup(struct snd_pcm_substream *substream)
 				   &constraints_rates);
 
 	machine->cap_i2s_instance = I2S_BT_INSTANCE;
-	return 0;
+	return da7219_clk_enable(substream);
 }
 
 static int cz_dmic1_startup(struct snd_pcm_substream *substream)
@@ -242,25 +247,7 @@ static int cz_dmic1_startup(struct snd_pcm_substream *substream)
 
 	machine->cap_i2s_instance = I2S_SP_INSTANCE;
 	machine->capture_channel = CAP_CHANNEL0;
-	return 0;
-}
-
-static int cz_da7219_params(struct snd_pcm_substream *substream,
-				      struct snd_pcm_hw_params *params)
-{
-	int wclk, bclk;
-
-	wclk = params_rate(params);
-	bclk = wclk * params_channels(params) *
-		snd_pcm_format_width(params_format(params));
-	/* ADAU7002 spec: "The ADAU7002 requires a BCLK rate
-	 * that is minimum of 64x the LRCLK sample rate."
-	 * DA7219 is the only clk source so for all codecs
-	 * we have to limit bclk to 64X lrclk.
-	 */
-	if (bclk < (wclk * 64))
-		bclk = wclk * 64;
-	return da7219_clk_enable(substream, wclk, bclk);
+	return da7219_clk_enable(substream);
 }
 
 static void cz_da7219_shutdown(struct snd_pcm_substream *substream)
@@ -271,31 +258,26 @@ static void cz_da7219_shutdown(struct snd_pcm_substream *substream)
 static const struct snd_soc_ops cz_da7219_play_ops = {
 	.startup = cz_da7219_play_startup,
 	.shutdown = cz_da7219_shutdown,
-	.hw_params = cz_da7219_params,
 };
 
 static const struct snd_soc_ops cz_da7219_cap_ops = {
 	.startup = cz_da7219_cap_startup,
 	.shutdown = cz_da7219_shutdown,
-	.hw_params = cz_da7219_params,
 };
 
 static const struct snd_soc_ops cz_max_play_ops = {
 	.startup = cz_max_startup,
 	.shutdown = cz_da7219_shutdown,
-	.hw_params = cz_da7219_params,
 };
 
 static const struct snd_soc_ops cz_dmic0_cap_ops = {
 	.startup = cz_dmic0_startup,
 	.shutdown = cz_da7219_shutdown,
-	.hw_params = cz_da7219_params,
 };
 
 static const struct snd_soc_ops cz_dmic1_cap_ops = {
 	.startup = cz_dmic1_startup,
 	.shutdown = cz_da7219_shutdown,
-	.hw_params = cz_da7219_params,
 };
 
 SND_SOC_DAILINK_DEF(designware1,
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-11-28 13:18 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-26  7:54 [PATCH] ASoC: AMD: Enable clk in startup intead of hw_params Yu-Hsuan Hsu
2019-11-26 10:11 ` Agrawal, Akshu
2019-11-28 13:18 ` Applied "ASoC: AMD: Enable clk in startup intead of hw_params" to the asoc tree Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).