[PATCH 1/4] ASoC: wm8960: Let wm8960 codec driver manage its own MCLK

[PATCH 1/4] ASoC: wm8960: Let wm8960 codec driver manage its own MCLK

[Zidan Wang]

When we want to use wm8960 codec, we should enable its MCLK in machine driver.
It's reasonable for wm8960 codec driver to manage its own MCLK.

When current bias_level is SND_SOC_BIAS_ON, it is preparing for a transition
away from ON. In this case, disable the codec mclk. When current bias_level
is not SND_SOC_BIAS_ON, it preparing for a transition to ON. In this case,
enable the codec mclk.

Signed-off-by: Zidan Wang <b50113@freescale.com>
---
 sound/soc/codecs/wm8960.c | 40 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c
index 031a1ae..1a5f47b 100644
--- a/sound/soc/codecs/wm8960.c
+++ b/sound/soc/codecs/wm8960.c
@@ -15,6 +15,7 @@
 #include <linux/init.h>
 #include <linux/delay.h>
 #include <linux/pm.h>
+#include <linux/clk.h>
 #include <linux/i2c.h>
 #include <linux/slab.h>
 #include <sound/core.h>
@@ -117,6 +118,7 @@ static bool wm8960_volatile(struct device *dev, unsigned int reg)
 }
 
 struct wm8960_priv {
+	struct clk *mclk;
 	struct regmap *regmap;
 	int (*set_bias_level)(struct snd_soc_codec *,
 			      enum snd_soc_bias_level level);
@@ -618,6 +620,7 @@ static int wm8960_set_bias_level_out3(struct snd_soc_codec *codec,
 				      enum snd_soc_bias_level level)
 {
 	struct wm8960_priv *wm8960 = snd_soc_codec_get_drvdata(codec);
+	int ret;
 
 	switch (level) {
 	case SND_SOC_BIAS_ON:
@@ -626,6 +629,20 @@ static int wm8960_set_bias_level_out3(struct snd_soc_codec *codec,
 	case SND_SOC_BIAS_PREPARE:
 		/* Set VMID to 2x50k */
 		snd_soc_update_bits(codec, WM8960_POWER1, 0x180, 0x80);
+
+		if (!IS_ERR(wm8960->mclk)) {
+			if (codec->dapm.bias_level == SND_SOC_BIAS_ON)
+				clk_disable_unprepare(wm8960->mclk);
+			else {
+				ret = clk_prepare_enable(wm8960->mclk);
+				if (ret) {
+					dev_err(codec->dev,
+						"Failed to enable MCLK: %d\n",
+						ret);
+					return ret;
+				}
+			}
+		}
 		break;
 
 	case SND_SOC_BIAS_STANDBY:
@@ -661,6 +678,7 @@ static int wm8960_set_bias_level_out3(struct snd_soc_codec *codec,
 
 		/* Disable VMID and VREF, let them discharge */
 		snd_soc_write(codec, WM8960_POWER1, 0);
+
 		msleep(600);
 		break;
 	}
@@ -674,7 +692,7 @@ static int wm8960_set_bias_level_capless(struct snd_soc_codec *codec,
 					 enum snd_soc_bias_level level)
 {
 	struct wm8960_priv *wm8960 = snd_soc_codec_get_drvdata(codec);
-	int reg;
+	int reg, ret;
 
 	switch (level) {
 	case SND_SOC_BIAS_ON:
@@ -715,9 +733,22 @@ static int wm8960_set_bias_level_capless(struct snd_soc_codec *codec,
 					    WM8960_VREF, WM8960_VREF);
 
 			msleep(100);
+
+			if (!IS_ERR(wm8960->mclk)) {
+				ret = clk_prepare_enable(wm8960->mclk);
+				if (ret) {
+					dev_err(codec->dev,
+						"Failed to enable MCLK: %d\n",
+						ret);
+					return ret;
+				}
+			}
 			break;
 
 		case SND_SOC_BIAS_ON:
+			if (!IS_ERR(wm8960->mclk))
+				clk_disable_unprepare(wm8960->mclk);
+
 			/* Enable anti-pop mode */
 			snd_soc_update_bits(codec, WM8960_APOP1,
 					    WM8960_POBCTRL | WM8960_SOFT_ST |
@@ -728,6 +759,7 @@ static int wm8960_set_bias_level_capless(struct snd_soc_codec *codec,
 			/* Disable VMID and VREF */
 			snd_soc_update_bits(codec, WM8960_POWER1,
 					    WM8960_VREF | WM8960_VMID_MASK, 0);
+
 			break;
 
 		case SND_SOC_BIAS_OFF:
@@ -1002,6 +1034,12 @@ static int wm8960_i2c_probe(struct i2c_client *i2c,
 	if (wm8960 == NULL)
 		return -ENOMEM;
 
+	wm8960->mclk = devm_clk_get(&i2c->dev, "codec_mclk");
+	if (IS_ERR(wm8960->mclk)) {
+		if (PTR_ERR(wm8960->mclk) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+	}
+
 	wm8960->regmap = devm_regmap_init_i2c(i2c, &wm8960_regmap);
 	if (IS_ERR(wm8960->regmap))
 		return PTR_ERR(wm8960->regmap);
-- 
1.9.1

[alsa-devel][PATCH 2/4] ASoC: wm8960: Let wm8960 driver configure its bit clock and frame clock

[Zidan Wang]

wm8960 codec driver missing configure its bit clock and frame clock, so add
support for it. It will calculate a appropriate frequency dividing ratio
according to the system clock, bit clock and frame clock, then set the
corresponding registers.

Signed-off-by: Zidan Wang <b50113@freescale.com>
---
 sound/soc/codecs/wm8960.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 108 insertions(+)

diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c
index 1a5f47b..86a5489 100644
--- a/sound/soc/codecs/wm8960.c
+++ b/sound/soc/codecs/wm8960.c
@@ -127,6 +127,8 @@ struct wm8960_priv {
 	struct snd_soc_dapm_widget *out3;
 	bool deemph;
 	int playback_fs;
+	int bclk;
+	int sysclk;
 	struct wm8960_data pdata;
 };
 
@@ -563,6 +565,79 @@ static struct {
 	{  8000, 5 },
 };
 
+/* Multiply 256 for internal 256 div */
+static const int dac_divs[] = { 256, 384, 512, 768, 1024, 1408, 1536 };
+
+/* Multiply 10 to eliminate decimials */
+static const int bclk_divs[] = {
+	10, 15, 20, 30, 40, 55, 60, 80, 110,
+	120, 160, 220, 240, 320, 320, 320
+};
+
+static void wm8960_configure_clocking(struct snd_soc_codec *codec,
+		int stream, int lrclk)
+{
+	struct wm8960_priv *wm8960 = snd_soc_codec_get_drvdata(codec);
+	u16 iface1 = snd_soc_read(codec, WM8960_IFACE1);
+	u16 iface2 = snd_soc_read(codec, WM8960_IFACE2);
+	int i, j;
+
+	if (!(iface1 & (1<<6))) {
+		dev_dbg(codec->dev,
+			"Codec is slave mode, no need to configure clock\n");
+		return;
+	}
+
+	if (!wm8960->sysclk) {
+		dev_dbg(codec->dev, "No SYSCLK configured\n");
+		return;
+	}
+
+	if (!wm8960->bclk || !lrclk) {
+		dev_dbg(codec->dev, "No audio clocks configured\n");
+		return;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(dac_divs); ++i) {
+		if (wm8960->sysclk == lrclk * dac_divs[i]) {
+			for (j = 0; j < ARRAY_SIZE(bclk_divs); ++j) {
+				if (wm8960->sysclk ==  wm8960->bclk *
+						bclk_divs[j] / 10) {
+					/* configure frame clock */
+					if (iface2 & (1<<6))
+						/* If ADCLRC configure as GPIO
+						 * pin, DACLRC pin is used as
+						 * a frame clock for ADCs and
+						 * DACs */
+						snd_soc_update_bits(codec,
+								WM8960_CLOCK1,
+								0x7 << 3,
+								i << 3);
+					else if (SNDRV_PCM_STREAM_PLAYBACK
+								== stream)
+						snd_soc_update_bits(codec,
+								WM8960_CLOCK1,
+								0x7 << 3,
+								i << 3);
+					else if (SNDRV_PCM_STREAM_CAPTURE
+								== stream)
+						snd_soc_update_bits(codec,
+								WM8960_CLOCK1,
+								0x7 << 6,
+								i << 6);
+
+					/* configure bit clock */
+					snd_soc_update_bits(codec,
+							WM8960_CLOCK2, 0xf, j);
+					return;
+				}
+			}
+		}
+	}
+
+	dev_err(codec->dev, "Unsupported sysclk %d\n", wm8960->sysclk);
+}
+
 static int wm8960_hw_params(struct snd_pcm_substream *substream,
 			    struct snd_pcm_hw_params *params,
 			    struct snd_soc_dai *dai)
@@ -572,6 +647,10 @@ static int wm8960_hw_params(struct snd_pcm_substream *substream,
 	u16 iface = snd_soc_read(codec, WM8960_IFACE1) & 0xfff3;
 	int i;
 
+	wm8960->bclk = snd_soc_params_to_bclk(params);
+	if (params_channels(params) == 1)
+		wm8960->bclk *= 2;
+
 	/* bit size */
 	switch (params_width(params)) {
 	case 16:
@@ -602,6 +681,10 @@ static int wm8960_hw_params(struct snd_pcm_substream *substream,
 
 	/* set iface */
 	snd_soc_write(codec, WM8960_IFACE1, iface);
+
+	wm8960_configure_clocking(codec, substream->stream,
+			params_rate(params));
+
 	return 0;
 }
 
@@ -943,6 +1026,30 @@ static int wm8960_set_bias_level(struct snd_soc_codec *codec,
 	return wm8960->set_bias_level(codec, level);
 }
 
+static int wm8960_set_dai_sysclk(struct snd_soc_dai *dai, int clk_id,
+					unsigned int freq, int dir)
+{
+	struct snd_soc_codec *codec = dai->codec;
+	struct wm8960_priv *wm8960 = snd_soc_codec_get_drvdata(codec);
+
+	switch (clk_id) {
+	case WM8960_SYSCLK_MCLK:
+		snd_soc_update_bits(codec, WM8960_CLOCK1,
+					0x1, WM8960_SYSCLK_MCLK);
+		break;
+	case WM8960_SYSCLK_PLL:
+		snd_soc_update_bits(codec, WM8960_CLOCK1,
+					0x1, WM8960_SYSCLK_PLL);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	wm8960->sysclk = freq;
+
+	return 0;
+}
+
 #define WM8960_RATES SNDRV_PCM_RATE_8000_48000
 
 #define WM8960_FORMATS \
@@ -955,6 +1062,7 @@ static const struct snd_soc_dai_ops wm8960_dai_ops = {
 	.set_fmt = wm8960_set_dai_fmt,
 	.set_clkdiv = wm8960_set_dai_clkdiv,
 	.set_pll = wm8960_set_dai_pll,
+	.set_sysclk = wm8960_set_dai_sysclk,
 };
 
 static struct snd_soc_dai_driver wm8960_dai = {
-- 
1.9.1

Re: [PATCH 2/4] ASoC: wm8960: Let wm8960 driver configure its bit clock and frame clock

[Charles Keepax]

On Wed, Dec 31, 2014 at 11:39:12AM +0800, Zidan Wang wrote:
> wm8960 codec driver missing configure its bit clock and frame clock, so add
> support for it. It will calculate a appropriate frequency dividing ratio
> according to the system clock, bit clock and frame clock, then set the
> corresponding registers.
> 
> Signed-off-by: Zidan Wang <b50113@freescale.com>
> ---
>  sound/soc/codecs/wm8960.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 108 insertions(+)
> 
> diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c
> index 1a5f47b..86a5489 100644
> --- a/sound/soc/codecs/wm8960.c
> +++ b/sound/soc/codecs/wm8960.c
> @@ -127,6 +127,8 @@ struct wm8960_priv {
>  	struct snd_soc_dapm_widget *out3;
>  	bool deemph;
>  	int playback_fs;
> +	int bclk;
> +	int sysclk;
>  	struct wm8960_data pdata;
>  };
>  
> @@ -563,6 +565,79 @@ static struct {
>  	{  8000, 5 },
>  };
>  
> +/* Multiply 256 for internal 256 div */
> +static const int dac_divs[] = { 256, 384, 512, 768, 1024, 1408, 1536 };
> +
> +/* Multiply 10 to eliminate decimials */
> +static const int bclk_divs[] = {
> +	10, 15, 20, 30, 40, 55, 60, 80, 110,
> +	120, 160, 220, 240, 320, 320, 320
> +};
> +
> +static void wm8960_configure_clocking(struct snd_soc_codec *codec,
> +		int stream, int lrclk)
> +{
> +	struct wm8960_priv *wm8960 = snd_soc_codec_get_drvdata(codec);
> +	u16 iface1 = snd_soc_read(codec, WM8960_IFACE1);
> +	u16 iface2 = snd_soc_read(codec, WM8960_IFACE2);
> +	int i, j;
> +
> +	if (!(iface1 & (1<<6))) {
> +		dev_dbg(codec->dev,
> +			"Codec is slave mode, no need to configure clock\n");
> +		return;
> +	}
> +
> +	if (!wm8960->sysclk) {
> +		dev_dbg(codec->dev, "No SYSCLK configured\n");
> +		return;
> +	}
> +
> +	if (!wm8960->bclk || !lrclk) {
> +		dev_dbg(codec->dev, "No audio clocks configured\n");
> +		return;
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(dac_divs); ++i) {
> +		if (wm8960->sysclk == lrclk * dac_divs[i]) {
> +			for (j = 0; j < ARRAY_SIZE(bclk_divs); ++j) {
> +				if (wm8960->sysclk ==  wm8960->bclk *
> +						bclk_divs[j] / 10) {
> +					/* configure frame clock */
> +					if (iface2 & (1<<6))
> +						/* If ADCLRC configure as GPIO
> +						 * pin, DACLRC pin is used as
> +						 * a frame clock for ADCs and
> +						 * DACs */
> +						snd_soc_update_bits(codec,
> +								WM8960_CLOCK1,
> +								0x7 << 3,
> +								i << 3);

The indentation is getting pretty horrific here can we do some
things to ease that a little. You could flip the polarity of the
two if statements and use continues instead for example that
would help a lot.

Thanks,
Charles

> +					else if (SNDRV_PCM_STREAM_PLAYBACK
> +								== stream)
> +						snd_soc_update_bits(codec,
> +								WM8960_CLOCK1,
> +								0x7 << 3,
> +								i << 3);
> +					else if (SNDRV_PCM_STREAM_CAPTURE
> +								== stream)
> +						snd_soc_update_bits(codec,
> +								WM8960_CLOCK1,
> +								0x7 << 6,
> +								i << 6);
> +
> +					/* configure bit clock */
> +					snd_soc_update_bits(codec,
> +							WM8960_CLOCK2, 0xf, j);
> +					return;
> +				}
> +			}
> +		}
> +	}
> +
> +	dev_err(codec->dev, "Unsupported sysclk %d\n", wm8960->sysclk);
> +}
> +

[alsa-devel][PATCH 3/4] ASoC: wm8960: use pr_debug instead of pr_err

[Zidan Wang]

In machine driver, we should find a appropriate sysclk for codec driver,
and will use codec set_dai_pll to judge if pll can generate such sysclk.
When sample rate changed, we should look for a useful bclk, lrclk division
ratio, and will try set_dai_pll for several times. So use pr_debug so that
don't generate much error logs.

Signed-off-by: Zidan Wang <b50113@freescale.com>
---
 sound/soc/codecs/wm8960.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c
index 86a5489..cb42385 100644
--- a/sound/soc/codecs/wm8960.c
+++ b/sound/soc/codecs/wm8960.c
@@ -914,7 +914,7 @@ static int pll_factors(unsigned int source, unsigned int target,
 		pll_div->pre_div = 0;
 
 	if ((Ndiv < 6) || (Ndiv > 12)) {
-		pr_err("WM8960 PLL: Unsupported N=%d\n", Ndiv);
+		pr_debug("WM8960 PLL: Unsupported N=%d\n", Ndiv);
 		return -EINVAL;
 	}
 
-- 
1.9.1

[alsa-devel][PATCH 4/4] ASoC: wm8960: Fix capture sample rate from 11250 to 11025

[Zidan Wang]

wm8960 codec can't support sample rate 11250, it must be 11025.

Signed-off-by: Zidan Wang <b50113@freescale.com>
---
 sound/soc/codecs/wm8960.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c
index cb42385..68790f1 100644
--- a/sound/soc/codecs/wm8960.c
+++ b/sound/soc/codecs/wm8960.c
@@ -560,7 +560,7 @@ static struct {
 	{ 22050, 2 },
 	{ 24000, 2 },
 	{ 16000, 3 },
-	{ 11250, 4 },
+	{ 11025, 4 },
 	{ 12000, 4 },
 	{  8000, 5 },
 };
-- 
1.9.1

Re: [alsa-devel][PATCH 4/4] ASoC: wm8960: Fix capture sample rate from 11250 to 11025

[Charles Keepax]

On Wed, Dec 31, 2014 at 11:39:14AM +0800, Zidan Wang wrote:
> wm8960 codec can't support sample rate 11250, it must be 11025.
> 
> Signed-off-by: Zidan Wang <b50113@freescale.com>
> ---

Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>

Thanks,
Charles

Re: [alsa-devel][PATCH 4/4] ASoC: wm8960: Fix capture sample rate from 11250 to 11025

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 144 bytes --]

On Wed, Dec 31, 2014 at 11:39:14AM +0800, Zidan Wang wrote:
> wm8960 codec can't support sample rate 11250, it must be 11025.

Applied, thanks.

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

Re: [PATCH 1/4] ASoC: wm8960: Let wm8960 codec driver manage its own MCLK

[Mark Brown]

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

On Wed, Dec 31, 2014 at 11:39:11AM +0800, Zidan Wang wrote:

> +		if (!IS_ERR(wm8960->mclk)) {
> +			if (codec->dapm.bias_level == SND_SOC_BIAS_ON)
> +				clk_disable_unprepare(wm8960->mclk);
> +			else {

Both sides of the if statement should have braces if one does, though in
this case nesting a second switch statement would probably be clearer.

> @@ -661,6 +678,7 @@ static int wm8960_set_bias_level_out3(struct snd_soc_codec *codec,
>  
>  		/* Disable VMID and VREF, let them discharge */
>  		snd_soc_write(codec, WM8960_POWER1, 0);
> +
>  		msleep(600);
>  		break;
>  	}

Random while space change here and in a couple of other places.

> +	wm8960->mclk = devm_clk_get(&i2c->dev, "codec_mclk");
> +	if (IS_ERR(wm8960->mclk)) {
> +		if (PTR_ERR(wm8960->mclk) == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;
> +	}
> +

Why name this "codec_mclk" - the device calls it just MCLK?

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

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