diff mbox

[PATCHv3,5/9] Using the constraint based on wm8971_set_dai_sysclk

Message ID 1409628470-13059-5-git-send-email-xavier.hsu@linaro.org
State New
Headers show

Commit Message

Xavier Hsu Sept. 2, 2014, 3:27 a.m. UTC 
This patch improves WM8971.
We use the constraint based on the function of
wm8971_set_dai_sysclk().

Any comments about improving the patch are welcome.
Thanks.

Signed-off-by: Xavier Hsu <xavier.hsu@linaro.org>
Signed-off-by: Andy Green <andy.green@linaro.org>
---
 sound/soc/codecs/wm8971.c |   77 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 71 insertions(+), 6 deletions(-)

Comments

Charles Keepax Sept. 2, 2014, 9:28 a.m. UTC | #1 
On Tue, Sep 02, 2014 at 11:27:46AM +0800, Xavier Hsu wrote:
> This patch improves WM8971.
> We use the constraint based on the function of
> wm8971_set_dai_sysclk().
> 
> Any comments about improving the patch are welcome.
> Thanks.

Comments like this are probably best put after the --- as they
don't need to appear in the change log.

> 
> Signed-off-by: Xavier Hsu <xavier.hsu@linaro.org>
> Signed-off-by: Andy Green <andy.green@linaro.org>
> ---
>  sound/soc/codecs/wm8971.c |   77 +++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 71 insertions(+), 6 deletions(-)
> 
> diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c
> index 64ed226..20cfdd3 100755
> --- a/sound/soc/codecs/wm8971.c
> +++ b/sound/soc/codecs/wm8971.c
> @@ -41,6 +41,7 @@ static struct workqueue_struct *wm8971_workq;
>  /* codec private data */
>  struct wm8971_priv {
>  	unsigned int sysclk;
> +	struct snd_pcm_hw_constraint_list *sysclk_constraints;
>  	int playback_fs;
>  	bool deemph;
>  };
> @@ -528,6 +529,53 @@ static int get_coeff(int mclk, int rate)
>  	return -EINVAL;
>  }
>  
> +/* The set of rates we can generate from the above for each SYSCLK */
> +static const unsigned int rates_12288[] = {
> +	8000, 12000, 16000, 24000, 32000, 48000, 96000
> +};
> +
> +static struct snd_pcm_hw_constraint_list constraints_12288 = {
> +	.count  = ARRAY_SIZE(rates_12288),
> +	.list   = rates_12288,
> +};
> +
> +static const unsigned int rates_112896[] = {
> +	8000, 11025, 22050, 44100, 88200
> +};
> +
> +static struct snd_pcm_hw_constraint_list constraints_112896 = {
> +	.count  = ARRAY_SIZE(rates_112896),
> +	.list   = rates_112896,
> +};
> +
> +static const unsigned int rates_18432[] = {
> +	8000, 12000, 16000, 24000, 32000, 48000, 96000
> +};
> +
> +static struct snd_pcm_hw_constraint_list constraints_18432 = {
> +	.count  = ARRAY_SIZE(rates_18432),
> +	.list   = rates_18432,
> +};

This one is identical to the 12288 array, why not combine them
and call them the 48k array?

> +
> +static const unsigned int rates_169344[] = {
> +	8000, 11025, 22050, 44100, 88200
> +};

This one is identical to the 112896 array, why not combine them
and call them the 44k1 array?

> +
> +static struct snd_pcm_hw_constraint_list constraints_169344 = {
> +	.count  = ARRAY_SIZE(rates_169344),
> +	.list   = rates_169344,
> +};
> +
> +static const unsigned int rates_12[] = {
> +	8000, 11025, 12000, 16000, 22050, 2400, 32000, 41100, 48000,

Typo 0 missing                            ^^^^

> +	48000, 88235, 96000,
> +};
> +
> +static struct snd_pcm_hw_constraint_list constraints_12 = {
> +	.count  = ARRAY_SIZE(rates_12),
> +	.list   = rates_12,
> +};
> +
>  static int wm8971_set_dai_sysclk(struct snd_soc_dai *codec_dai,
>  				 int clk_id, unsigned int freq, int dir)
>  {
> @@ -535,15 +583,32 @@ static int wm8971_set_dai_sysclk(struct snd_soc_dai *codec_dai,
>  	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
>  
>  	switch (freq) {
> -	case 11289600:
> -	case 12000000:
>  	case 12288000:
> -	case 16934400:
> +	case 24576000:
> +		wm8971->sysclk_constraints = &constraints_12288;
> +		break;
> +	case 11289600:
> +	case 22579200:
> +		wm8971->sysclk_constraints = &constraints_112896;
> +		break;
>  	case 18432000:
> -		wm8971->sysclk = freq;
> -		return 0;
> +		wm8971->sysclk_constraints = &constraints_18432;
> +		break;
> +	case 16934400:
> +	case 33868800:
> +		wm8971->sysclk_constraints = &constraints_169344;
> +		break;
> +	case 12000000:
> +	case 24000000:
> +		wm8971->sysclk_constraints = &constraints_12;
> +		break;
> +	default:
> +		return -EINVAL;
>  	}
> -	return -EINVAL;

You pick out the constraints here but you never actually set them
with snd_pcm_hw_constraint_list?

> +
> +	wm8971->sysclk = freq;
> +
> +	return 0;
>  }
>  
>  static int wm8971_set_dai_fmt(struct snd_soc_dai *codec_dai,
> -- 

Thanks,
Charles
Xavier Hsu Sept. 4, 2014, 9:13 a.m. UTC | #2 
Hi Charles :
Thanks for yours feedback.

According your suggestion, I combine the redundant codes and fix some
mistakes.
(rates_12288[] and rates_18432[]      => rates_48000[]
 rates_112896[] and rates_169344[]  => rates_44100[]
 2400 => 24000
)
I also add snd_pcm_hw_constraint_list() to restrict sysclk.

Thanks.

BR,
Xavier


2014-09-02 17:28 GMT+08:00 Charles Keepax <
ckeepax@opensource.wolfsonmicro.com>:

> On Tue, Sep 02, 2014 at 11:27:46AM +0800, Xavier Hsu wrote:
> > This patch improves WM8971.
> > We use the constraint based on the function of
> > wm8971_set_dai_sysclk().
> >
> > Any comments about improving the patch are welcome.
> > Thanks.
>
> Comments like this are probably best put after the --- as they
> don't need to appear in the change log.
>
> >
> > Signed-off-by: Xavier Hsu <xavier.hsu@linaro.org>
> > Signed-off-by: Andy Green <andy.green@linaro.org>
> > ---
> >  sound/soc/codecs/wm8971.c |   77
> +++++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 71 insertions(+), 6 deletions(-)
> >
> > diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c
> > index 64ed226..20cfdd3 100755
> > --- a/sound/soc/codecs/wm8971.c
> > +++ b/sound/soc/codecs/wm8971.c
> > @@ -41,6 +41,7 @@ static struct workqueue_struct *wm8971_workq;
> >  /* codec private data */
> >  struct wm8971_priv {
> >       unsigned int sysclk;
> > +     struct snd_pcm_hw_constraint_list *sysclk_constraints;
> >       int playback_fs;
> >       bool deemph;
> >  };
> > @@ -528,6 +529,53 @@ static int get_coeff(int mclk, int rate)
> >       return -EINVAL;
> >  }
> >
> > +/* The set of rates we can generate from the above for each SYSCLK */
> > +static const unsigned int rates_12288[] = {
> > +     8000, 12000, 16000, 24000, 32000, 48000, 96000
> > +};
> > +
> > +static struct snd_pcm_hw_constraint_list constraints_12288 = {
> > +     .count  = ARRAY_SIZE(rates_12288),
> > +     .list   = rates_12288,
> > +};
> > +
> > +static const unsigned int rates_112896[] = {
> > +     8000, 11025, 22050, 44100, 88200
> > +};
> > +
> > +static struct snd_pcm_hw_constraint_list constraints_112896 = {
> > +     .count  = ARRAY_SIZE(rates_112896),
> > +     .list   = rates_112896,
> > +};
> > +
> > +static const unsigned int rates_18432[] = {
> > +     8000, 12000, 16000, 24000, 32000, 48000, 96000
> > +};
> > +
> > +static struct snd_pcm_hw_constraint_list constraints_18432 = {
> > +     .count  = ARRAY_SIZE(rates_18432),
> > +     .list   = rates_18432,
> > +};
>
> This one is identical to the 12288 array, why not combine them
> and call them the 48k array?
>
> > +
> > +static const unsigned int rates_169344[] = {
> > +     8000, 11025, 22050, 44100, 88200
> > +};
>
> This one is identical to the 112896 array, why not combine them
> and call them the 44k1 array?
>
> > +
> > +static struct snd_pcm_hw_constraint_list constraints_169344 = {
> > +     .count  = ARRAY_SIZE(rates_169344),
> > +     .list   = rates_169344,
> > +};
> > +
> > +static const unsigned int rates_12[] = {
> > +     8000, 11025, 12000, 16000, 22050, 2400, 32000, 41100, 48000,
>
> Typo 0 missing                            ^^^^
>
> > +     48000, 88235, 96000,
> > +};
> > +
> > +static struct snd_pcm_hw_constraint_list constraints_12 = {
> > +     .count  = ARRAY_SIZE(rates_12),
> > +     .list   = rates_12,
> > +};
> > +
> >  static int wm8971_set_dai_sysclk(struct snd_soc_dai *codec_dai,
> >                                int clk_id, unsigned int freq, int dir)
> >  {
> > @@ -535,15 +583,32 @@ static int wm8971_set_dai_sysclk(struct
> snd_soc_dai *codec_dai,
> >       struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
> >
> >       switch (freq) {
> > -     case 11289600:
> > -     case 12000000:
> >       case 12288000:
> > -     case 16934400:
> > +     case 24576000:
> > +             wm8971->sysclk_constraints = &constraints_12288;
> > +             break;
> > +     case 11289600:
> > +     case 22579200:
> > +             wm8971->sysclk_constraints = &constraints_112896;
> > +             break;
> >       case 18432000:
> > -             wm8971->sysclk = freq;
> > -             return 0;
> > +             wm8971->sysclk_constraints = &constraints_18432;
> > +             break;
> > +     case 16934400:
> > +     case 33868800:
> > +             wm8971->sysclk_constraints = &constraints_169344;
> > +             break;
> > +     case 12000000:
> > +     case 24000000:
> > +             wm8971->sysclk_constraints = &constraints_12;
> > +             break;
> > +     default:
> > +             return -EINVAL;
> >       }
> > -     return -EINVAL;
>
> You pick out the constraints here but you never actually set them
> with snd_pcm_hw_constraint_list?
>
> > +
> > +     wm8971->sysclk = freq;
> > +
> > +     return 0;
> >  }
> >
> >  static int wm8971_set_dai_fmt(struct snd_soc_dai *codec_dai,
> > --
>
> Thanks,
> Charles
>
diff mbox

Patch

diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c
index 64ed226..20cfdd3 100755
--- a/sound/soc/codecs/wm8971.c
+++ b/sound/soc/codecs/wm8971.c
@@ -41,6 +41,7 @@  static struct workqueue_struct *wm8971_workq;
 /* codec private data */
 struct wm8971_priv {
 	unsigned int sysclk;
+	struct snd_pcm_hw_constraint_list *sysclk_constraints;
 	int playback_fs;
 	bool deemph;
 };
@@ -528,6 +529,53 @@  static int get_coeff(int mclk, int rate)
 	return -EINVAL;
 }
 
+/* The set of rates we can generate from the above for each SYSCLK */
+static const unsigned int rates_12288[] = {
+	8000, 12000, 16000, 24000, 32000, 48000, 96000
+};
+
+static struct snd_pcm_hw_constraint_list constraints_12288 = {
+	.count  = ARRAY_SIZE(rates_12288),
+	.list   = rates_12288,
+};
+
+static const unsigned int rates_112896[] = {
+	8000, 11025, 22050, 44100, 88200
+};
+
+static struct snd_pcm_hw_constraint_list constraints_112896 = {
+	.count  = ARRAY_SIZE(rates_112896),
+	.list   = rates_112896,
+};
+
+static const unsigned int rates_18432[] = {
+	8000, 12000, 16000, 24000, 32000, 48000, 96000
+};
+
+static struct snd_pcm_hw_constraint_list constraints_18432 = {
+	.count  = ARRAY_SIZE(rates_18432),
+	.list   = rates_18432,
+};
+
+static const unsigned int rates_169344[] = {
+	8000, 11025, 22050, 44100, 88200
+};
+
+static struct snd_pcm_hw_constraint_list constraints_169344 = {
+	.count  = ARRAY_SIZE(rates_169344),
+	.list   = rates_169344,
+};
+
+static const unsigned int rates_12[] = {
+	8000, 11025, 12000, 16000, 22050, 2400, 32000, 41100, 48000,
+	48000, 88235, 96000,
+};
+
+static struct snd_pcm_hw_constraint_list constraints_12 = {
+	.count  = ARRAY_SIZE(rates_12),
+	.list   = rates_12,
+};
+
 static int wm8971_set_dai_sysclk(struct snd_soc_dai *codec_dai,
 				 int clk_id, unsigned int freq, int dir)
 {
@@ -535,15 +583,32 @@  static int wm8971_set_dai_sysclk(struct snd_soc_dai *codec_dai,
 	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
 
 	switch (freq) {
-	case 11289600:
-	case 12000000:
 	case 12288000:
-	case 16934400:
+	case 24576000:
+		wm8971->sysclk_constraints = &constraints_12288;
+		break;
+	case 11289600:
+	case 22579200:
+		wm8971->sysclk_constraints = &constraints_112896;
+		break;
 	case 18432000:
-		wm8971->sysclk = freq;
-		return 0;
+		wm8971->sysclk_constraints = &constraints_18432;
+		break;
+	case 16934400:
+	case 33868800:
+		wm8971->sysclk_constraints = &constraints_169344;
+		break;
+	case 12000000:
+	case 24000000:
+		wm8971->sysclk_constraints = &constraints_12;
+		break;
+	default:
+		return -EINVAL;
 	}
-	return -EINVAL;
+
+	wm8971->sysclk = freq;
+
+	return 0;
 }
 
 static int wm8971_set_dai_fmt(struct snd_soc_dai *codec_dai,