[RFC PATCH] ASoC: wm8741: Allow master clock switching

[RFC PATCH] ASoC: wm8741: Allow master clock switching

[Sergej Sawazki]

The set of supported sample rates depends on the master clock supplied
to the codec. Allow the machine driver to set the required master clock
in hw_params().

Signed-off-by: Sergej Sawazki <ce3a@gmx.de>
---

This patch has been tested on a board with two master clocks, 22.5792MHz
(for 44.1kHz, 88.2kHz, ...) and 24.576MHz (for 48kHz, 96kHz, ...).

 sound/soc/codecs/wm8741.c | 63 +++++++++++++----------------------------------
 1 file changed, 17 insertions(+), 46 deletions(-)

diff --git a/sound/soc/codecs/wm8741.c b/sound/soc/codecs/wm8741.c
index 09ff01f..47c5577 100644
--- a/sound/soc/codecs/wm8741.c
+++ b/sound/soc/codecs/wm8741.c
@@ -125,18 +125,6 @@ static const struct snd_soc_dapm_route wm8741_dapm_routes[] = {
 	{ "VOUTRN", NULL, "DACR" },
 };
 
-static struct {
-	int value;
-	int ratio;
-} lrclk_ratios[WM8741_NUM_RATES] = {
-	{ 1, 128 },
-	{ 2, 192 },
-	{ 3, 256 },
-	{ 4, 384 },
-	{ 5, 512 },
-	{ 6, 768 },
-};
-
 static const unsigned int rates_11289[] = {
 	44100, 88200,
 };
@@ -209,29 +197,6 @@ static const struct snd_pcm_hw_constraint_list constraints_36864 = {
 	.list	= rates_36864,
 };
 
-
-static int wm8741_startup(struct snd_pcm_substream *substream,
-			  struct snd_soc_dai *dai)
-{
-	struct snd_soc_codec *codec = dai->codec;
-	struct wm8741_priv *wm8741 = snd_soc_codec_get_drvdata(codec);
-
-	/* The set of sample rates that can be supported depends on the
-	 * MCLK supplied to the CODEC - enforce this.
-	 */
-	if (!wm8741->sysclk) {
-		dev_err(codec->dev,
-			"No MCLK configured, call set_sysclk() on init\n");
-		return -EINVAL;
-	}
-
-	snd_pcm_hw_constraint_list(substream->runtime, 0,
-				   SNDRV_PCM_HW_PARAM_RATE,
-				   wm8741->sysclk_constraints);
-
-	return 0;
-}
-
 static int wm8741_hw_params(struct snd_pcm_substream *substream,
 			    struct snd_pcm_hw_params *params,
 			    struct snd_soc_dai *dai)
@@ -241,17 +206,24 @@ static int wm8741_hw_params(struct snd_pcm_substream *substream,
 	u16 iface = snd_soc_read(codec, WM8741_FORMAT_CONTROL) & 0x1FC;
 	int i;
 
-	/* Find a supported LRCLK ratio */
-	for (i = 0; i < ARRAY_SIZE(lrclk_ratios); i++) {
-		if (wm8741->sysclk / params_rate(params) ==
-		    lrclk_ratios[i].ratio)
+	/* The set of sample rates that can be supported depends on the
+	 * MCLK supplied to the CODEC - enforce this.
+	 */
+	if (!wm8741->sysclk) {
+		dev_err(codec->dev,
+			"No MCLK configured, call set_sysclk() on hw_params\n");
+		return -EINVAL;
+	}
+
+	/* Find a supported LRCLK rate */
+	for (i = 0; i < wm8741->sysclk_constraints->count; i++) {
+		if (wm8741->sysclk_constraints->list[i] == params_rate(params))
 			break;
 	}
 
-	/* Should never happen, should be handled by constraints */
-	if (i == ARRAY_SIZE(lrclk_ratios)) {
-		dev_err(codec->dev, "MCLK/fs ratio %d unsupported\n",
-			wm8741->sysclk / params_rate(params));
+	if (i == wm8741->sysclk_constraints->count) {
+		dev_err(codec->dev, "LRCLK %d unsupported with MCLK %d\n",
+			params_rate(params), wm8741->sysclk);
 		return -EINVAL;
 	}
 
@@ -274,8 +246,8 @@ static int wm8741_hw_params(struct snd_pcm_substream *substream,
 		return -EINVAL;
 	}
 
-	dev_dbg(codec->dev, "wm8741_hw_params:    bit size param = %d",
-		params_width(params));
+	dev_dbg(codec->dev, "wm8741_hw_params:    bit size param = %d, rate param = %d",
+		params_width(params), params_rate(params));
 
 	snd_soc_write(codec, WM8741_FORMAT_CONTROL, iface);
 	return 0;
@@ -403,7 +375,6 @@ static int wm8741_set_dai_fmt(struct snd_soc_dai *codec_dai,
 			SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE)
 
 static const struct snd_soc_dai_ops wm8741_dai_ops = {
-	.startup	= wm8741_startup,
 	.hw_params	= wm8741_hw_params,
 	.set_sysclk	= wm8741_set_dai_sysclk,
 	.set_fmt	= wm8741_set_dai_fmt,
-- 
1.9.1

Re: [RFC PATCH] ASoC: wm8741: Allow master clock switching

[Charles Keepax]

On Fri, Jun 05, 2015 at 02:05:44PM +0200, Sergej Sawazki wrote:
> The set of supported sample rates depends on the master clock supplied
> to the codec. Allow the machine driver to set the required master clock
> in hw_params().
> 
> Signed-off-by: Sergej Sawazki <ce3a@gmx.de>
> ---
> 
> This patch has been tested on a board with two master clocks, 22.5792MHz
> (for 44.1kHz, 88.2kHz, ...) and 24.576MHz (for 48kHz, 96kHz, ...).
> 
>  sound/soc/codecs/wm8741.c | 63 +++++++++++++----------------------------------
>  1 file changed, 17 insertions(+), 46 deletions(-)
> 
> diff --git a/sound/soc/codecs/wm8741.c b/sound/soc/codecs/wm8741.c
> index 09ff01f..47c5577 100644
> --- a/sound/soc/codecs/wm8741.c
> +++ b/sound/soc/codecs/wm8741.c
> @@ -125,18 +125,6 @@ static const struct snd_soc_dapm_route wm8741_dapm_routes[] = {
>  	{ "VOUTRN", NULL, "DACR" },
>  };
>  
> -static struct {
> -	int value;
> -	int ratio;
> -} lrclk_ratios[WM8741_NUM_RATES] = {
> -	{ 1, 128 },
> -	{ 2, 192 },
> -	{ 3, 256 },
> -	{ 4, 384 },
> -	{ 5, 512 },
> -	{ 6, 768 },
> -};
> -
>  static const unsigned int rates_11289[] = {
>  	44100, 88200,
>  };
> @@ -209,29 +197,6 @@ static const struct snd_pcm_hw_constraint_list constraints_36864 = {
>  	.list	= rates_36864,
>  };
>  
> -
> -static int wm8741_startup(struct snd_pcm_substream *substream,
> -			  struct snd_soc_dai *dai)
> -{
> -	struct snd_soc_codec *codec = dai->codec;
> -	struct wm8741_priv *wm8741 = snd_soc_codec_get_drvdata(codec);
> -
> -	/* The set of sample rates that can be supported depends on the
> -	 * MCLK supplied to the CODEC - enforce this.
> -	 */
> -	if (!wm8741->sysclk) {
> -		dev_err(codec->dev,
> -			"No MCLK configured, call set_sysclk() on init\n");
> -		return -EINVAL;
> -	}
> -
> -	snd_pcm_hw_constraint_list(substream->runtime, 0,
> -				   SNDRV_PCM_HW_PARAM_RATE,
> -				   wm8741->sysclk_constraints);
> -
> -	return 0;
> -}

I think rather than removing the constraints entirely here,
perhaps just don't set any constraints if there is no SYSCLK yet.
That way we get the benefits of the constraints for single clock
systems (such as user-space being able to arrange appropriate
software resampling) but those like yours can configure the clock
later.

> -
>  static int wm8741_hw_params(struct snd_pcm_substream *substream,
>  			    struct snd_pcm_hw_params *params,
>  			    struct snd_soc_dai *dai)
> @@ -241,17 +206,24 @@ static int wm8741_hw_params(struct snd_pcm_substream *substream,
>  	u16 iface = snd_soc_read(codec, WM8741_FORMAT_CONTROL) & 0x1FC;
>  	int i;
>  
> -	/* Find a supported LRCLK ratio */
> -	for (i = 0; i < ARRAY_SIZE(lrclk_ratios); i++) {
> -		if (wm8741->sysclk / params_rate(params) ==
> -		    lrclk_ratios[i].ratio)
> +	/* The set of sample rates that can be supported depends on the
> +	 * MCLK supplied to the CODEC - enforce this.
> +	 */
> +	if (!wm8741->sysclk) {
> +		dev_err(codec->dev,
> +			"No MCLK configured, call set_sysclk() on hw_params\n");
> +		return -EINVAL;
> +	}

Then you can keep this error check here and fail if we reach
hw_params and still don't have a SYSCLK. You will probably need
to provide a way to clear the SYSCLK though set_dai_sysclk as
well.

Thanks,
Charles

> +
> +	/* Find a supported LRCLK rate */
> +	for (i = 0; i < wm8741->sysclk_constraints->count; i++) {
> +		if (wm8741->sysclk_constraints->list[i] == params_rate(params))
>  			break;
>  	}
>  
> -	/* Should never happen, should be handled by constraints */
> -	if (i == ARRAY_SIZE(lrclk_ratios)) {
> -		dev_err(codec->dev, "MCLK/fs ratio %d unsupported\n",
> -			wm8741->sysclk / params_rate(params));
> +	if (i == wm8741->sysclk_constraints->count) {
> +		dev_err(codec->dev, "LRCLK %d unsupported with MCLK %d\n",
> +			params_rate(params), wm8741->sysclk);
>  		return -EINVAL;
>  	}
>  
> @@ -274,8 +246,8 @@ static int wm8741_hw_params(struct snd_pcm_substream *substream,
>  		return -EINVAL;
>  	}
>  
> -	dev_dbg(codec->dev, "wm8741_hw_params:    bit size param = %d",
> -		params_width(params));
> +	dev_dbg(codec->dev, "wm8741_hw_params:    bit size param = %d, rate param = %d",
> +		params_width(params), params_rate(params));
>  
>  	snd_soc_write(codec, WM8741_FORMAT_CONTROL, iface);
>  	return 0;
> @@ -403,7 +375,6 @@ static int wm8741_set_dai_fmt(struct snd_soc_dai *codec_dai,
>  			SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE)
>  
>  static const struct snd_soc_dai_ops wm8741_dai_ops = {
> -	.startup	= wm8741_startup,
>  	.hw_params	= wm8741_hw_params,
>  	.set_sysclk	= wm8741_set_dai_sysclk,
>  	.set_fmt	= wm8741_set_dai_fmt,
> -- 
> 1.9.1
> 

Re: [RFC PATCH] ASoC: wm8741: Allow master clock switching

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 775 bytes --]

On Fri, Jun 05, 2015 at 04:22:19PM +0100, Charles Keepax wrote:
> On Fri, Jun 05, 2015 at 02:05:44PM +0200, Sergej Sawazki wrote:
> > The set of supported sample rates depends on the master clock supplied
> > to the codec. Allow the machine driver to set the required master clock

Please delete undeleted context when replying.

> I think rather than removing the constraints entirely here,
> perhaps just don't set any constraints if there is no SYSCLK yet.
> That way we get the benefits of the constraints for single clock
> systems (such as user-space being able to arrange appropriate
> software resampling) but those like yours can configure the clock
> later.

Yes, that's the ideal thing.  The machine driver should reset the clock
back to zero when things go idle.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC PATCH] ASoC: wm8741: Allow master clock switching

[Sergej Sawazki]

On 2015-06-05 at 05:22PM, Charles Keepax wrote:
>
> I think rather than removing the constraints entirely here,
> perhaps just don't set any constraints if there is no SYSCLK yet.
> That way we get the benefits of the constraints for single clock
> systems (such as user-space being able to arrange appropriate
> software resampling) but those like yours can configure the clock
> later.
>

Good point, I will add it to v2.

>> +	if (!wm8741->sysclk) {
>> +		dev_err(codec->dev,
>> +			"No MCLK configured, call set_sysclk() on hw_params\n");
>> +		return -EINVAL;
>> +	}
>
> Then you can keep this error check here and fail if we reach
> hw_params and still don't have a SYSCLK. You will probably need
> to provide a way to clear the SYSCLK though set_dai_sysclk as
> well.

Is snd_soc_dai_set_sysclk( , , 0, ) the right way to clear the SYSCLK?

Thanks,
Sergej

Re: [RFC PATCH] ASoC: wm8741: Allow master clock switching

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 400 bytes --]

On Fri, Jun 05, 2015 at 08:56:10PM +0200, Sergej Sawazki wrote:
> On 2015-06-05 at 05:22PM, Charles Keepax wrote:

> >Then you can keep this error check here and fail if we reach
> >hw_params and still don't have a SYSCLK. You will probably need
> >to provide a way to clear the SYSCLK though set_dai_sysclk as
> >well.

> Is snd_soc_dai_set_sysclk( , , 0, ) the right way to clear the SYSCLK?

Yes.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]