[PATCH 0/4] ASoC: da7213: Device clocking updates and fixes

[PATCH 0/4] ASoC: da7213: Device clocking updates and fixes

[Adam Thomson]

This patch set contains a series of patches relating to device clocking. The
changes are as follows:

 1) Set correct default BCLKs per WCLK to achieve all DAI formats. Currently
    only 16-bit formats would operate correctly.
 2) Only use MCLK as required, when an audio stream is active. Can be disabled
    all other times.
 3) Refactoring of sysclk() and pll() functions to make them cleaner, removing
    need for unnecessary private flags, and resolve incorrect 32Khz mode
    configuration.
 4) Improve 32KHz PLL locking with some small register updates when configuring
    PLL and when DAI is enabled.

Patches are based on v4.7 Linux kernel

Adam Thomson (4):
  ASoC: da7213: Default to 64 BCLKs per WCLK to support all formats
  ASoC: da7213: Improve driver efficiency with regards to MCLK usage
  ASoC: da7213: Refactor sysclk(), pll() functions to improve handling
  ASoC: da7213: Improve 32KHz mode PLL locking

 sound/soc/codecs/da7213.c | 134 +++++++++++++++++++++++++++++-----------------
 sound/soc/codecs/da7213.h |  12 +++--
 2 files changed, 94 insertions(+), 52 deletions(-)

--
1.9.3

[PATCH 1/4] ASoC: da7213: Default to 64 BCLKs per WCLK to support all formats

[Adam Thomson]

Previously code defaulted to 32 BCLKS per WCLK which meant 24 and
32 bit DAI formats would not work properly. This patch fixes the
issue by defaulting to 64 BCLKs per WCLK.

Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
---
 sound/soc/codecs/da7213.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/da7213.c b/sound/soc/codecs/da7213.c
index e5527bc..bcf1834 100644
--- a/sound/soc/codecs/da7213.c
+++ b/sound/soc/codecs/da7213.c
@@ -1247,8 +1247,8 @@ static int da7213_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt)
 		return -EINVAL;
 	}
 
-	/* By default only 32 BCLK per WCLK is supported */
-	dai_clk_mode |= DA7213_DAI_BCLKS_PER_WCLK_32;
+	/* By default only 64 BCLK per WCLK is supported */
+	dai_clk_mode |= DA7213_DAI_BCLKS_PER_WCLK_64;
 
 	snd_soc_write(codec, DA7213_DAI_CLK_MODE, dai_clk_mode);
 	snd_soc_update_bits(codec, DA7213_DAI_CTRL, DA7213_DAI_FORMAT_MASK,
-- 
1.9.3

Re: [alsa-devel] [PATCH 1/4] ASoC: da7213: Default to 64 BCLKs per WCLK to support all formats

[Peter Meerwald-Stadler]

> Previously code defaulted to 32 BCLKS per WCLK which meant 24 and
> 32 bit DAI formats would not work properly. This patch fixes the
> issue by defaulting to 64 BCLKs per WCLK.

Tested-by: Peter Meerwald-Stadler <pmeerw@pmeerw.net>
 
> Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> ---
>  sound/soc/codecs/da7213.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/codecs/da7213.c b/sound/soc/codecs/da7213.c
> index e5527bc..bcf1834 100644
> --- a/sound/soc/codecs/da7213.c
> +++ b/sound/soc/codecs/da7213.c
> @@ -1247,8 +1247,8 @@ static int da7213_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt)
>  		return -EINVAL;
>  	}
>  
> -	/* By default only 32 BCLK per WCLK is supported */
> -	dai_clk_mode |= DA7213_DAI_BCLKS_PER_WCLK_32;
> +	/* By default only 64 BCLK per WCLK is supported */
> +	dai_clk_mode |= DA7213_DAI_BCLKS_PER_WCLK_64;
>  
>  	snd_soc_write(codec, DA7213_DAI_CLK_MODE, dai_clk_mode);
>  	snd_soc_update_bits(codec, DA7213_DAI_CTRL, DA7213_DAI_FORMAT_MASK,
> 

-- 

Peter Meerwald-Stadler
+43-664-2444418 (mobile)

[PATCH 2/4] ASoC: da7213: Improve driver efficiency with regards to MCLK usage

[Adam Thomson]

Currently MCLK remains enabled during bias STANDBY state, and this
is not necessary. This patch updates the code to handle enabling
and disabling of MCLK, if provided, when moving between STANDBY
and PREPARE states, therefore saving power when no active streams
present.

Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
---
 sound/soc/codecs/da7213.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/sound/soc/codecs/da7213.c b/sound/soc/codecs/da7213.c
index bcf1834..7701f4e 100644
--- a/sound/soc/codecs/da7213.c
+++ b/sound/soc/codecs/da7213.c
@@ -1454,11 +1454,10 @@ static int da7213_set_bias_level(struct snd_soc_codec *codec,
 
 	switch (level) {
 	case SND_SOC_BIAS_ON:
-	case SND_SOC_BIAS_PREPARE:
 		break;
-	case SND_SOC_BIAS_STANDBY:
-		if (snd_soc_codec_get_bias_level(codec) == SND_SOC_BIAS_OFF) {
-			/* MCLK */
+	case SND_SOC_BIAS_PREPARE:
+		/* Enable MCLK for transition to ON state */
+		if (snd_soc_codec_get_bias_level(codec) == SND_SOC_BIAS_STANDBY) {
 			if (da7213->mclk) {
 				ret = clk_prepare_enable(da7213->mclk);
 				if (ret) {
@@ -1467,21 +1466,24 @@ static int da7213_set_bias_level(struct snd_soc_codec *codec,
 					return ret;
 				}
 			}
-
+		}
+		break;
+	case SND_SOC_BIAS_STANDBY:
+		if (snd_soc_codec_get_bias_level(codec) == SND_SOC_BIAS_OFF) {
 			/* Enable VMID reference & master bias */
 			snd_soc_update_bits(codec, DA7213_REFERENCES,
 					    DA7213_VMID_EN | DA7213_BIAS_EN,
 					    DA7213_VMID_EN | DA7213_BIAS_EN);
+		} else {
+			/* Remove MCLK */
+			if (da7213->mclk)
+				clk_disable_unprepare(da7213->mclk);
 		}
 		break;
 	case SND_SOC_BIAS_OFF:
 		/* Disable VMID reference & master bias */
 		snd_soc_update_bits(codec, DA7213_REFERENCES,
 				    DA7213_VMID_EN | DA7213_BIAS_EN, 0);
-
-		/* MCLK */
-		if (da7213->mclk)
-			clk_disable_unprepare(da7213->mclk);
 		break;
 	}
 	return 0;
-- 
1.9.3

Re: [alsa-devel] [PATCH 2/4] ASoC: da7213: Improve driver efficiency with regards to MCLK usage

[Peter Meerwald-Stadler]

> Currently MCLK remains enabled during bias STANDBY state, and this
> is not necessary. This patch updates the code to handle enabling
> and disabling of MCLK, if provided, when moving between STANDBY
> and PREPARE states, therefore saving power when no active streams
> present.
> 
> Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>

Tested-by: Peter Meerwald-Stadler <pmeerw@pmeerw.net>

> ---
>  sound/soc/codecs/da7213.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/sound/soc/codecs/da7213.c b/sound/soc/codecs/da7213.c
> index bcf1834..7701f4e 100644
> --- a/sound/soc/codecs/da7213.c
> +++ b/sound/soc/codecs/da7213.c
> @@ -1454,11 +1454,10 @@ static int da7213_set_bias_level(struct snd_soc_codec *codec,
>  
>  	switch (level) {
>  	case SND_SOC_BIAS_ON:
> -	case SND_SOC_BIAS_PREPARE:
>  		break;
> -	case SND_SOC_BIAS_STANDBY:
> -		if (snd_soc_codec_get_bias_level(codec) == SND_SOC_BIAS_OFF) {
> -			/* MCLK */
> +	case SND_SOC_BIAS_PREPARE:
> +		/* Enable MCLK for transition to ON state */
> +		if (snd_soc_codec_get_bias_level(codec) == SND_SOC_BIAS_STANDBY) {
>  			if (da7213->mclk) {
>  				ret = clk_prepare_enable(da7213->mclk);
>  				if (ret) {
> @@ -1467,21 +1466,24 @@ static int da7213_set_bias_level(struct snd_soc_codec *codec,
>  					return ret;
>  				}
>  			}
> -
> +		}
> +		break;
> +	case SND_SOC_BIAS_STANDBY:
> +		if (snd_soc_codec_get_bias_level(codec) == SND_SOC_BIAS_OFF) {
>  			/* Enable VMID reference & master bias */
>  			snd_soc_update_bits(codec, DA7213_REFERENCES,
>  					    DA7213_VMID_EN | DA7213_BIAS_EN,
>  					    DA7213_VMID_EN | DA7213_BIAS_EN);
> +		} else {
> +			/* Remove MCLK */
> +			if (da7213->mclk)
> +				clk_disable_unprepare(da7213->mclk);
>  		}
>  		break;
>  	case SND_SOC_BIAS_OFF:
>  		/* Disable VMID reference & master bias */
>  		snd_soc_update_bits(codec, DA7213_REFERENCES,
>  				    DA7213_VMID_EN | DA7213_BIAS_EN, 0);
> -
> -		/* MCLK */
> -		if (da7213->mclk)
> -			clk_disable_unprepare(da7213->mclk);
>  		break;
>  	}
>  	return 0;
> 

-- 

Peter Meerwald-Stadler
+43-664-2444418 (mobile)

[PATCH 3/4] ASoC: da7213: Refactor sysclk(), pll() functions to improve handling

[Adam Thomson]

Currently the handling of the PLL in the driver is a little clunky,
and not ideal for all modes. This patch updates the code to make it
cleaner and more sensible for the various PLL states.

Key items of note are:
 - MCLK squaring is now handled directly as part of the sysclk()
   function, removing the need for a private flag to set this feature.
 - All PLL modes are defined as an enum, and are handled as a case
   statement in pll() function to clean up configuration. This also
   removes any need for a private flag for SRM.
 - For 32KHz mode, checks are made on codec master mode and correct
   MCLK rates, to avoid incorrect usage of PLL for this operation.
 - For 32KHz mode, SRM flag now correctly enabled and fout set to
   sensible value to achieve appropriate PLL dividers.

Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
---
 sound/soc/codecs/da7213.c | 85 +++++++++++++++++++++++++++--------------------
 sound/soc/codecs/da7213.h | 12 ++++---
 2 files changed, 57 insertions(+), 40 deletions(-)

diff --git a/sound/soc/codecs/da7213.c b/sound/soc/codecs/da7213.c
index 7701f4e..79b8324 100644
--- a/sound/soc/codecs/da7213.c
+++ b/sound/soc/codecs/da7213.c
@@ -1297,10 +1297,13 @@ static int da7213_set_dai_sysclk(struct snd_soc_dai *codec_dai,
 
 	switch (clk_id) {
 	case DA7213_CLKSRC_MCLK:
-		da7213->mclk_squarer_en = false;
+		snd_soc_update_bits(codec, DA7213_PLL_CTRL,
+				    DA7213_PLL_MCLK_SQR_EN, 0);
 		break;
 	case DA7213_CLKSRC_MCLK_SQR:
-		da7213->mclk_squarer_en = true;
+		snd_soc_update_bits(codec, DA7213_PLL_CTRL,
+				    DA7213_PLL_MCLK_SQR_EN,
+				    DA7213_PLL_MCLK_SQR_EN);
 		break;
 	default:
 		dev_err(codec_dai->dev, "Unknown clock source %d\n", clk_id);
@@ -1324,7 +1327,7 @@ static int da7213_set_dai_sysclk(struct snd_soc_dai *codec_dai,
 	return 0;
 }
 
-/* Supported PLL input frequencies are 5MHz - 54MHz. */
+/* Supported PLL input frequencies are 32KHz, 5MHz - 54MHz. */
 static int da7213_set_dai_pll(struct snd_soc_dai *codec_dai, int pll_id,
 			      int source, unsigned int fref, unsigned int fout)
 {
@@ -1336,22 +1339,26 @@ static int da7213_set_dai_pll(struct snd_soc_dai *codec_dai, int pll_id,
 	u32 freq_ref;
 	u64 frac_div;
 
-	/* Reset PLL configuration */
-	snd_soc_write(codec, DA7213_PLL_CTRL, 0);
-
-	pll_ctrl = 0;
-
 	/* Workout input divider based on MCLK rate */
 	if (da7213->mclk_rate == 32768) {
+		if (!da7213->master) {
+			dev_err(codec->dev,
+				"32KHz only valid if codec is clock master\n");
+			return -EINVAL;
+		}
+
 		/* 32KHz PLL Mode */
 		indiv_bits = DA7213_PLL_INDIV_9_TO_18_MHZ;
 		indiv = DA7213_PLL_INDIV_9_TO_18_MHZ_VAL;
+		source = DA7213_SYSCLK_PLL_32KHZ;
 		freq_ref = 3750000;
-		pll_ctrl |= DA7213_PLL_32K_MODE;
+
 	} else {
-		/* 5 - 54MHz MCLK */
 		if (da7213->mclk_rate < 5000000) {
-			goto pll_err;
+			dev_err(codec->dev,
+				"PLL input clock %d below valid range\n",
+				da7213->mclk_rate);
+			return -EINVAL;
 		} else if (da7213->mclk_rate <= 9000000) {
 			indiv_bits = DA7213_PLL_INDIV_5_TO_9_MHZ;
 			indiv = DA7213_PLL_INDIV_5_TO_9_MHZ_VAL;
@@ -1365,32 +1372,44 @@ static int da7213_set_dai_pll(struct snd_soc_dai *codec_dai, int pll_id,
 			indiv_bits = DA7213_PLL_INDIV_36_TO_54_MHZ;
 			indiv = DA7213_PLL_INDIV_36_TO_54_MHZ_VAL;
 		} else {
-			goto pll_err;
+			dev_err(codec->dev,
+				"PLL input clock %d above valid range\n",
+				da7213->mclk_rate);
+			return -EINVAL;
 		}
 		freq_ref = (da7213->mclk_rate / indiv);
 	}
 
-	pll_ctrl |= indiv_bits;
+	pll_ctrl = indiv_bits;
 
-	/* PLL Bypass mode */
-	if (source == DA7213_SYSCLK_MCLK) {
-		snd_soc_write(codec, DA7213_PLL_CTRL, pll_ctrl);
+	/* Configure PLL */
+	switch (source) {
+	case DA7213_SYSCLK_MCLK:
+		snd_soc_update_bits(codec, DA7213_PLL_CTRL,
+				    DA7213_PLL_INDIV_MASK |
+				    DA7213_PLL_MODE_MASK, pll_ctrl);
 		return 0;
-	}
+	case DA7213_SYSCLK_PLL:
+		break;
+	case DA7213_SYSCLK_PLL_SRM:
+		pll_ctrl |= DA7213_PLL_SRM_EN;
+		fout = DA7213_PLL_FREQ_OUT_94310400;
+		break;
+	case DA7213_SYSCLK_PLL_32KHZ:
+		if (da7213->mclk_rate != 32768) {
+			dev_err(codec->dev,
+				"32KHz mode only valid with 32KHz MCLK\n");
+			return -EINVAL;
+		}
 
-	/*
-	 * If Codec is slave and SRM enabled,
-	 * freq_out is (98304000 + 90316800)/2 = 94310400
-	 */
-	if (!da7213->master && da7213->srm_en) {
+		pll_ctrl |= DA7213_PLL_32K_MODE | DA7213_PLL_SRM_EN;
 		fout = DA7213_PLL_FREQ_OUT_94310400;
-		pll_ctrl |= DA7213_PLL_SRM_EN;
+		break;
+	default:
+		dev_err(codec->dev, "Invalid PLL config\n");
+		return -EINVAL;
 	}
 
-	/* Enable MCLK squarer if required */
-	if (da7213->mclk_squarer_en)
-		pll_ctrl |= DA7213_PLL_MCLK_SQR_EN;
-
 	/* Calculate dividers for PLL */
 	pll_integer = fout / freq_ref;
 	frac_div = (u64)(fout % freq_ref) * 8192ULL;
@@ -1405,14 +1424,11 @@ static int da7213_set_dai_pll(struct snd_soc_dai *codec_dai, int pll_id,
 
 	/* Enable PLL */
 	pll_ctrl |= DA7213_PLL_EN;
-	snd_soc_write(codec, DA7213_PLL_CTRL, pll_ctrl);
+	snd_soc_update_bits(codec, DA7213_PLL_CTRL,
+			    DA7213_PLL_INDIV_MASK | DA7213_PLL_MODE_MASK,
+			    pll_ctrl);
 
 	return 0;
-
-pll_err:
-	dev_err(codec_dai->dev, "Unsupported PLL input frequency %d\n",
-		da7213->mclk_rate);
-	return -EINVAL;
 }
 
 /* DAI operations */
@@ -1607,9 +1623,6 @@ static int da7213_probe(struct snd_soc_codec *codec)
 			    DA7213_ALC_CALIB_MODE_MAN, 0);
 	da7213->alc_calib_auto = true;
 
-	/* Default to using SRM for slave mode */
-	da7213->srm_en = true;
-
 	/* Default PC counter to free-running */
 	snd_soc_update_bits(codec, DA7213_PC_COUNT, DA7213_PC_FREERUN_MASK,
 			    DA7213_PC_FREERUN_MASK);
diff --git a/sound/soc/codecs/da7213.h b/sound/soc/codecs/da7213.h
index fbb7a35..16ef56f 100644
--- a/sound/soc/codecs/da7213.h
+++ b/sound/soc/codecs/da7213.h
@@ -172,6 +172,7 @@
 #define DA7213_PLL_32K_MODE					(0x1 << 5)
 #define DA7213_PLL_SRM_EN					(0x1 << 6)
 #define DA7213_PLL_EN						(0x1 << 7)
+#define DA7213_PLL_MODE_MASK					(0x7 << 5)
 
 /* DA7213_DAI_CLK_MODE = 0x28 */
 #define DA7213_DAI_BCLKS_PER_WCLK_32				(0x0 << 0)
@@ -499,8 +500,6 @@
 #define DA7213_ALC_AVG_ITERATIONS	5
 
 /* PLL related */
-#define DA7213_SYSCLK_MCLK			0
-#define DA7213_SYSCLK_PLL			1
 #define DA7213_PLL_FREQ_OUT_90316800		90316800
 #define DA7213_PLL_FREQ_OUT_98304000		98304000
 #define DA7213_PLL_FREQ_OUT_94310400		94310400
@@ -515,6 +514,13 @@ enum da7213_clk_src {
 	DA7213_CLKSRC_MCLK_SQR,
 };
 
+enum da7213_sys_clk {
+	DA7213_SYSCLK_MCLK = 0,
+	DA7213_SYSCLK_PLL,
+	DA7213_SYSCLK_PLL_SRM,
+	DA7213_SYSCLK_PLL_32KHZ
+};
+
 /* Codec private data */
 struct da7213_priv {
 	struct regmap *regmap;
@@ -522,8 +528,6 @@ struct da7213_priv {
 	unsigned int mclk_rate;
 	int clk_src;
 	bool master;
-	bool mclk_squarer_en;
-	bool srm_en;
 	bool alc_calib_auto;
 	bool alc_en;
 	struct da7213_platform_data *pdata;
-- 
1.9.3

Re: [alsa-devel] [PATCH 3/4] ASoC: da7213: Refactor sysclk(), pll() functions to improve handling

[Peter Meerwald-Stadler]

> Currently the handling of the PLL in the driver is a little clunky,
> and not ideal for all modes. This patch updates the code to make it
> cleaner and more sensible for the various PLL states.
> 
> Key items of note are:
>  - MCLK squaring is now handled directly as part of the sysclk()
>    function, removing the need for a private flag to set this feature.
>  - All PLL modes are defined as an enum, and are handled as a case
>    statement in pll() function to clean up configuration. This also
>    removes any need for a private flag for SRM.
>  - For 32KHz mode, checks are made on codec master mode and correct
>    MCLK rates, to avoid incorrect usage of PLL for this operation.
>  - For 32KHz mode, SRM flag now correctly enabled and fout set to
>    sensible value to achieve appropriate PLL dividers.

thanks, looks good
Tested-by: Peter Meerwald-Stadler <pmeerw@pmeerw.net>

nitpick: add extra newline at the end of 
if (da7213->mclk_rate == 32768) block
 
> Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> ---
>  sound/soc/codecs/da7213.c | 85 +++++++++++++++++++++++++++--------------------
>  sound/soc/codecs/da7213.h | 12 ++++---
>  2 files changed, 57 insertions(+), 40 deletions(-)
> 
> diff --git a/sound/soc/codecs/da7213.c b/sound/soc/codecs/da7213.c
> index 7701f4e..79b8324 100644
> --- a/sound/soc/codecs/da7213.c
> +++ b/sound/soc/codecs/da7213.c
> @@ -1297,10 +1297,13 @@ static int da7213_set_dai_sysclk(struct snd_soc_dai *codec_dai,
>  
>  	switch (clk_id) {
>  	case DA7213_CLKSRC_MCLK:
> -		da7213->mclk_squarer_en = false;
> +		snd_soc_update_bits(codec, DA7213_PLL_CTRL,
> +				    DA7213_PLL_MCLK_SQR_EN, 0);
>  		break;
>  	case DA7213_CLKSRC_MCLK_SQR:
> -		da7213->mclk_squarer_en = true;
> +		snd_soc_update_bits(codec, DA7213_PLL_CTRL,
> +				    DA7213_PLL_MCLK_SQR_EN,
> +				    DA7213_PLL_MCLK_SQR_EN);
>  		break;
>  	default:
>  		dev_err(codec_dai->dev, "Unknown clock source %d\n", clk_id);
> @@ -1324,7 +1327,7 @@ static int da7213_set_dai_sysclk(struct snd_soc_dai *codec_dai,
>  	return 0;
>  }
>  
> -/* Supported PLL input frequencies are 5MHz - 54MHz. */
> +/* Supported PLL input frequencies are 32KHz, 5MHz - 54MHz. */
>  static int da7213_set_dai_pll(struct snd_soc_dai *codec_dai, int pll_id,
>  			      int source, unsigned int fref, unsigned int fout)
>  {
> @@ -1336,22 +1339,26 @@ static int da7213_set_dai_pll(struct snd_soc_dai *codec_dai, int pll_id,
>  	u32 freq_ref;
>  	u64 frac_div;
>  
> -	/* Reset PLL configuration */
> -	snd_soc_write(codec, DA7213_PLL_CTRL, 0);
> -
> -	pll_ctrl = 0;
> -
>  	/* Workout input divider based on MCLK rate */
>  	if (da7213->mclk_rate == 32768) {
> +		if (!da7213->master) {
> +			dev_err(codec->dev,
> +				"32KHz only valid if codec is clock master\n");
> +			return -EINVAL;
> +		}
> +
>  		/* 32KHz PLL Mode */
>  		indiv_bits = DA7213_PLL_INDIV_9_TO_18_MHZ;
>  		indiv = DA7213_PLL_INDIV_9_TO_18_MHZ_VAL;
> +		source = DA7213_SYSCLK_PLL_32KHZ;
>  		freq_ref = 3750000;
> -		pll_ctrl |= DA7213_PLL_32K_MODE;
> +
>  	} else {
> -		/* 5 - 54MHz MCLK */
>  		if (da7213->mclk_rate < 5000000) {
> -			goto pll_err;
> +			dev_err(codec->dev,
> +				"PLL input clock %d below valid range\n",
> +				da7213->mclk_rate);
> +			return -EINVAL;
>  		} else if (da7213->mclk_rate <= 9000000) {
>  			indiv_bits = DA7213_PLL_INDIV_5_TO_9_MHZ;
>  			indiv = DA7213_PLL_INDIV_5_TO_9_MHZ_VAL;
> @@ -1365,32 +1372,44 @@ static int da7213_set_dai_pll(struct snd_soc_dai *codec_dai, int pll_id,
>  			indiv_bits = DA7213_PLL_INDIV_36_TO_54_MHZ;
>  			indiv = DA7213_PLL_INDIV_36_TO_54_MHZ_VAL;
>  		} else {
> -			goto pll_err;
> +			dev_err(codec->dev,
> +				"PLL input clock %d above valid range\n",
> +				da7213->mclk_rate);
> +			return -EINVAL;
>  		}
>  		freq_ref = (da7213->mclk_rate / indiv);
>  	}
>  
> -	pll_ctrl |= indiv_bits;
> +	pll_ctrl = indiv_bits;
>  
> -	/* PLL Bypass mode */
> -	if (source == DA7213_SYSCLK_MCLK) {
> -		snd_soc_write(codec, DA7213_PLL_CTRL, pll_ctrl);
> +	/* Configure PLL */
> +	switch (source) {
> +	case DA7213_SYSCLK_MCLK:
> +		snd_soc_update_bits(codec, DA7213_PLL_CTRL,
> +				    DA7213_PLL_INDIV_MASK |
> +				    DA7213_PLL_MODE_MASK, pll_ctrl);
>  		return 0;
> -	}
> +	case DA7213_SYSCLK_PLL:
> +		break;
> +	case DA7213_SYSCLK_PLL_SRM:
> +		pll_ctrl |= DA7213_PLL_SRM_EN;
> +		fout = DA7213_PLL_FREQ_OUT_94310400;
> +		break;
> +	case DA7213_SYSCLK_PLL_32KHZ:
> +		if (da7213->mclk_rate != 32768) {
> +			dev_err(codec->dev,
> +				"32KHz mode only valid with 32KHz MCLK\n");
> +			return -EINVAL;
> +		}
>  
> -	/*
> -	 * If Codec is slave and SRM enabled,
> -	 * freq_out is (98304000 + 90316800)/2 = 94310400
> -	 */
> -	if (!da7213->master && da7213->srm_en) {
> +		pll_ctrl |= DA7213_PLL_32K_MODE | DA7213_PLL_SRM_EN;
>  		fout = DA7213_PLL_FREQ_OUT_94310400;
> -		pll_ctrl |= DA7213_PLL_SRM_EN;
> +		break;
> +	default:
> +		dev_err(codec->dev, "Invalid PLL config\n");
> +		return -EINVAL;
>  	}
>  
> -	/* Enable MCLK squarer if required */
> -	if (da7213->mclk_squarer_en)
> -		pll_ctrl |= DA7213_PLL_MCLK_SQR_EN;
> -
>  	/* Calculate dividers for PLL */
>  	pll_integer = fout / freq_ref;
>  	frac_div = (u64)(fout % freq_ref) * 8192ULL;
> @@ -1405,14 +1424,11 @@ static int da7213_set_dai_pll(struct snd_soc_dai *codec_dai, int pll_id,
>  
>  	/* Enable PLL */
>  	pll_ctrl |= DA7213_PLL_EN;
> -	snd_soc_write(codec, DA7213_PLL_CTRL, pll_ctrl);
> +	snd_soc_update_bits(codec, DA7213_PLL_CTRL,
> +			    DA7213_PLL_INDIV_MASK | DA7213_PLL_MODE_MASK,
> +			    pll_ctrl);
>  
>  	return 0;
> -
> -pll_err:
> -	dev_err(codec_dai->dev, "Unsupported PLL input frequency %d\n",
> -		da7213->mclk_rate);
> -	return -EINVAL;
>  }
>  
>  /* DAI operations */
> @@ -1607,9 +1623,6 @@ static int da7213_probe(struct snd_soc_codec *codec)
>  			    DA7213_ALC_CALIB_MODE_MAN, 0);
>  	da7213->alc_calib_auto = true;
>  
> -	/* Default to using SRM for slave mode */
> -	da7213->srm_en = true;
> -
>  	/* Default PC counter to free-running */
>  	snd_soc_update_bits(codec, DA7213_PC_COUNT, DA7213_PC_FREERUN_MASK,
>  			    DA7213_PC_FREERUN_MASK);
> diff --git a/sound/soc/codecs/da7213.h b/sound/soc/codecs/da7213.h
> index fbb7a35..16ef56f 100644
> --- a/sound/soc/codecs/da7213.h
> +++ b/sound/soc/codecs/da7213.h
> @@ -172,6 +172,7 @@
>  #define DA7213_PLL_32K_MODE					(0x1 << 5)
>  #define DA7213_PLL_SRM_EN					(0x1 << 6)
>  #define DA7213_PLL_EN						(0x1 << 7)
> +#define DA7213_PLL_MODE_MASK					(0x7 << 5)
>  
>  /* DA7213_DAI_CLK_MODE = 0x28 */
>  #define DA7213_DAI_BCLKS_PER_WCLK_32				(0x0 << 0)
> @@ -499,8 +500,6 @@
>  #define DA7213_ALC_AVG_ITERATIONS	5
>  
>  /* PLL related */
> -#define DA7213_SYSCLK_MCLK			0
> -#define DA7213_SYSCLK_PLL			1
>  #define DA7213_PLL_FREQ_OUT_90316800		90316800
>  #define DA7213_PLL_FREQ_OUT_98304000		98304000
>  #define DA7213_PLL_FREQ_OUT_94310400		94310400
> @@ -515,6 +514,13 @@ enum da7213_clk_src {
>  	DA7213_CLKSRC_MCLK_SQR,
>  };
>  
> +enum da7213_sys_clk {
> +	DA7213_SYSCLK_MCLK = 0,
> +	DA7213_SYSCLK_PLL,
> +	DA7213_SYSCLK_PLL_SRM,
> +	DA7213_SYSCLK_PLL_32KHZ
> +};
> +
>  /* Codec private data */
>  struct da7213_priv {
>  	struct regmap *regmap;
> @@ -522,8 +528,6 @@ struct da7213_priv {
>  	unsigned int mclk_rate;
>  	int clk_src;
>  	bool master;
> -	bool mclk_squarer_en;
> -	bool srm_en;
>  	bool alc_calib_auto;
>  	bool alc_en;
>  	struct da7213_platform_data *pdata;
> 

-- 

Peter Meerwald-Stadler
+43-664-2444418 (mobile)

RE: [alsa-devel] [PATCH 3/4] ASoC: da7213: Refactor sysclk(), pll() functions to improve handling

[Adam Thomson]

On 08 August 2016 08:43, Peter Meerwald-Stadler wrote:

> > Currently the handling of the PLL in the driver is a little clunky,
> > and not ideal for all modes. This patch updates the code to make it
> > cleaner and more sensible for the various PLL states.
> >
> > Key items of note are:
> >  - MCLK squaring is now handled directly as part of the sysclk()
> >    function, removing the need for a private flag to set this feature.
> >  - All PLL modes are defined as an enum, and are handled as a case
> >    statement in pll() function to clean up configuration. This also
> >    removes any need for a private flag for SRM.
> >  - For 32KHz mode, checks are made on codec master mode and correct
> >    MCLK rates, to avoid incorrect usage of PLL for this operation.
> >  - For 32KHz mode, SRM flag now correctly enabled and fout set to
> >    sensible value to achieve appropriate PLL dividers.
> 
> thanks, looks good
> Tested-by: Peter Meerwald-Stadler <pmeerw@pmeerw.net>
> 
> nitpick: add extra newline at the end of
> if (da7213->mclk_rate == 32768) block
> 

Thanks. Glad the patch set has resolved issues you were seeing. Patches have
been pulled by Mark.

[PATCH 4/4] ASoC: da7213: Improve 32KHz mode PLL locking

[Adam Thomson]

To aid PLL in locking on to a 32KHz MCLK, some register mods
are made during PLL configuration, and when enabling the DAI,
to achieve the full range of sample rates.

Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
---
 sound/soc/codecs/da7213.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/da7213.c b/sound/soc/codecs/da7213.c
index 79b8324..095fe40 100644
--- a/sound/soc/codecs/da7213.c
+++ b/sound/soc/codecs/da7213.c
@@ -750,11 +750,18 @@ static int da7213_dai_event(struct snd_soc_dapm_widget *w,
 		snd_soc_update_bits(codec, DA7213_PC_COUNT,
 				    DA7213_PC_FREERUN_MASK, 0);

-		/* Slave mode, if SRM not enabled no need for status checks */
+		/* If SRM not enabled then nothing more to do */
 		pll_ctrl = snd_soc_read(codec, DA7213_PLL_CTRL);
 		if (!(pll_ctrl & DA7213_PLL_SRM_EN))
 			return 0;

+		/* Assist 32KHz mode PLL lock */
+		if (pll_ctrl & DA7213_PLL_32K_MODE) {
+			snd_soc_write(codec, 0xF0, 0x8B);
+			snd_soc_write(codec, 0xF2, 0x03);
+			snd_soc_write(codec, 0xF0, 0x00);
+		}
+
 		/* Check SRM has locked */
 		do {
 			pll_status = snd_soc_read(codec, DA7213_PLL_STATUS);
@@ -771,6 +778,14 @@ static int da7213_dai_event(struct snd_soc_dapm_widget *w,

 		return 0;
 	case SND_SOC_DAPM_POST_PMD:
+		/* Revert 32KHz PLL lock udpates if applied previously */
+		pll_ctrl = snd_soc_read(codec, DA7213_PLL_CTRL);
+		if (pll_ctrl & DA7213_PLL_32K_MODE) {
+			snd_soc_write(codec, 0xF0, 0x8B);
+			snd_soc_write(codec, 0xF2, 0x01);
+			snd_soc_write(codec, 0xF0, 0x00);
+		}
+
 		/* PC free-running */
 		snd_soc_update_bits(codec, DA7213_PC_COUNT,
 				    DA7213_PC_FREERUN_MASK,
@@ -1428,6 +1443,14 @@ static int da7213_set_dai_pll(struct snd_soc_dai *codec_dai, int pll_id,
 			    DA7213_PLL_INDIV_MASK | DA7213_PLL_MODE_MASK,
 			    pll_ctrl);

+	/* Assist 32KHz mode PLL lock */
+	if (source == DA7213_SYSCLK_PLL_32KHZ) {
+		snd_soc_write(codec, 0xF0, 0x8B);
+		snd_soc_write(codec, 0xF1, 0x03);
+		snd_soc_write(codec, 0xF1, 0x01);
+		snd_soc_write(codec, 0xF0, 0x00);
+	}
+
 	return 0;
 }

--
1.9.3

Re: [alsa-devel] [PATCH 4/4] ASoC: da7213: Improve 32KHz mode PLL locking

[Peter Meerwald-Stadler]

> To aid PLL in locking on to a 32KHz MCLK, some register mods
> are made during PLL configuration, and when enabling the DAI,
> to achieve the full range of sample rates.

thanks for the patch series; we are about to test...

some comments below
 
> Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> ---
>  sound/soc/codecs/da7213.c | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/codecs/da7213.c b/sound/soc/codecs/da7213.c
> index 79b8324..095fe40 100644
> --- a/sound/soc/codecs/da7213.c
> +++ b/sound/soc/codecs/da7213.c
> @@ -750,11 +750,18 @@ static int da7213_dai_event(struct snd_soc_dapm_widget *w,
>  		snd_soc_update_bits(codec, DA7213_PC_COUNT,
>  				    DA7213_PC_FREERUN_MASK, 0);
> 
> -		/* Slave mode, if SRM not enabled no need for status checks */
> +		/* If SRM not enabled then nothing more to do */
>  		pll_ctrl = snd_soc_read(codec, DA7213_PLL_CTRL);
>  		if (!(pll_ctrl & DA7213_PLL_SRM_EN))
>  			return 0;
> 
> +		/* Assist 32KHz mode PLL lock */
> +		if (pll_ctrl & DA7213_PLL_32K_MODE) {

these registers cannot not found in the datasheet;
maybe add descriptive #defines in da7213.h

> +			snd_soc_write(codec, 0xF0, 0x8B);
> +			snd_soc_write(codec, 0xF2, 0x03);
> +			snd_soc_write(codec, 0xF0, 0x00);
> +		}
> +
>  		/* Check SRM has locked */
>  		do {
>  			pll_status = snd_soc_read(codec, DA7213_PLL_STATUS);
> @@ -771,6 +778,14 @@ static int da7213_dai_event(struct snd_soc_dapm_widget *w,
> 
>  		return 0;
>  	case SND_SOC_DAPM_POST_PMD:
> +		/* Revert 32KHz PLL lock udpates if applied previously */
> +		pll_ctrl = snd_soc_read(codec, DA7213_PLL_CTRL);
> +		if (pll_ctrl & DA7213_PLL_32K_MODE) {
> +			snd_soc_write(codec, 0xF0, 0x8B);
> +			snd_soc_write(codec, 0xF2, 0x01);
> +			snd_soc_write(codec, 0xF0, 0x00);
> +		}
> +
>  		/* PC free-running */
>  		snd_soc_update_bits(codec, DA7213_PC_COUNT,
>  				    DA7213_PC_FREERUN_MASK,
> @@ -1428,6 +1443,14 @@ static int da7213_set_dai_pll(struct snd_soc_dai *codec_dai, int pll_id,
>  			    DA7213_PLL_INDIV_MASK | DA7213_PLL_MODE_MASK,
>  			    pll_ctrl);
> 
> +	/* Assist 32KHz mode PLL lock */
> +	if (source == DA7213_SYSCLK_PLL_32KHZ) {
> +		snd_soc_write(codec, 0xF0, 0x8B);
> +		snd_soc_write(codec, 0xF1, 0x03);
> +		snd_soc_write(codec, 0xF1, 0x01);
> +		snd_soc_write(codec, 0xF0, 0x00);
> +	}
> +
>  	return 0;
>  }

-- 

Peter Meerwald-Stadler
+43-664-2444418 (mobile)

Re: [alsa-devel] [PATCH 4/4] ASoC: da7213: Improve 32KHz mode PLL locking

[Mark Brown]

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

On Fri, Aug 05, 2016 at 11:04:51AM +0200, Peter Meerwald-Stadler wrote:

> these registers cannot not found in the datasheet;
> maybe add descriptive #defines in da7213.h

> > +			snd_soc_write(codec, 0xF0, 0x8B);
> > +			snd_soc_write(codec, 0xF2, 0x03);
> > +			snd_soc_write(codec, 0xF0, 0x00);

It is very common for chips to have undocumented write sequences that
make tweaks based on test registers that are deliberately not
documented.  This looks like such a case, I'd imagine that register 0xf0
is a test key and 0xf2 contains a value being tweaked.

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