[PATCH 0/7] Arizona FLL Bug Fixes

[PATCH 0/7] Arizona FLL Bug Fixes

[Charles Keepax]

Hi,

This patch series contains several small bug fixes and
improves the handling of the situation in which an already
running FLL is reconfigured. In the case of a running FLL
only some tranisitions are supported whereas previously all
transitions were allowed.

Thanks,
Charles

Charles Keepax (7):
  ASoC: arizona: Do not test ratio zero as it is not a valid setting
  ASoC: arizona: Correct checking of FLL ratio limitations
  ASoC: arizona: Correct relationship between VCO corner and Fref
  ASoC: arizona: Coding standards, remove unneeded brackets
  ASoC: arizona: Correct return value of arizona_is_enabled_fll
  ASoC: arizona: FLL freerun only required whilst disabling
  ASoC: arizona: Update handling for input change on an active FLL

 sound/soc/codecs/arizona.c |   63 ++++++++++++++++++++++++++++++--------------
 1 files changed, 43 insertions(+), 20 deletions(-)

-- 
1.7.2.5

[PATCH 1/7] ASoC: arizona: Do not test ratio zero as it is not a valid setting

[Charles Keepax]

Zero is not a valid FRATIO for the FLL, as such we shouldn't test it
whilst refining the FRATIO. This patch does just that.

Reported-by: Ryo Tsutsui <ryo.tsutsui@wolfsonmicro.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/arizona.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sound/soc/codecs/arizona.c b/sound/soc/codecs/arizona.c
index c5b6be2..a0252a7 100644
--- a/sound/soc/codecs/arizona.c
+++ b/sound/soc/codecs/arizona.c
@@ -1630,7 +1630,7 @@ static int arizona_calc_fratio(struct arizona_fll *fll,
 			}
 		}
 
-		for (ratio = init_ratio - 1; ratio >= 0; ratio--) {
+		for (ratio = init_ratio - 1; ratio > 0; ratio--) {
 			if (ARIZONA_FLL_VCO_CORNER / (fll->vco_mult * ratio) <
 			    Fref)
 				break;
-- 
1.7.2.5

[PATCH 2/7] ASoC: arizona: Correct checking of FLL ratio limitations

[Charles Keepax]

The check to ensure the Fref frequency is within the bounds for the
current ratio, was placed in the wrong loop. The initial configuration
will always be valid and the loop lowering the ratio will only reinforce
this validity. The check should be on the loop increasing the ratio.
This could on occasion cause an invalid ratio/Fref combination to be
selected.

Reported-by: Ryo Tsutsui <ryo.tsutsui@wolfsonmicro.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/arizona.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/arizona.c b/sound/soc/codecs/arizona.c
index a0252a7..e9e0b6b 100644
--- a/sound/soc/codecs/arizona.c
+++ b/sound/soc/codecs/arizona.c
@@ -1623,6 +1623,10 @@ static int arizona_calc_fratio(struct arizona_fll *fll,
 	while (div <= ARIZONA_FLL_MAX_REFDIV) {
 		for (ratio = init_ratio; ratio <= ARIZONA_FLL_MAX_FRATIO;
 		     ratio++) {
+			if (ARIZONA_FLL_VCO_CORNER / (fll->vco_mult * ratio) <
+			    Fref)
+				break;
+
 			if (target % (ratio * Fref)) {
 				cfg->refdiv = refdiv;
 				cfg->fratio = ratio - 1;
@@ -1631,10 +1635,6 @@ static int arizona_calc_fratio(struct arizona_fll *fll,
 		}
 
 		for (ratio = init_ratio - 1; ratio > 0; ratio--) {
-			if (ARIZONA_FLL_VCO_CORNER / (fll->vco_mult * ratio) <
-			    Fref)
-				break;
-
 			if (target % (ratio * Fref)) {
 				cfg->refdiv = refdiv;
 				cfg->fratio = ratio - 1;
-- 
1.7.2.5

[PATCH 3/7] ASoC: arizona: Correct relationship between VCO corner and Fref

[Charles Keepax]

When configuring the FLL we must ensure that the reference clock passed
to the FLL is under a certain limit. This limit was specified
incorrectly in the current code, this patch corrects this. Although the
error will only be encountered in some edge cases.

Reported-by: Ryo Tsutsui <ryo.tsutsui@wolfsonmicro.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/arizona.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/arizona.c b/sound/soc/codecs/arizona.c
index e9e0b6b..97cc80d 100644
--- a/sound/soc/codecs/arizona.c
+++ b/sound/soc/codecs/arizona.c
@@ -1623,8 +1623,8 @@ static int arizona_calc_fratio(struct arizona_fll *fll,
 	while (div <= ARIZONA_FLL_MAX_REFDIV) {
 		for (ratio = init_ratio; ratio <= ARIZONA_FLL_MAX_FRATIO;
 		     ratio++) {
-			if (ARIZONA_FLL_VCO_CORNER / (fll->vco_mult * ratio) <
-			    Fref)
+			if ((ARIZONA_FLL_VCO_CORNER / 2) /
+			    (fll->vco_mult * ratio) < Fref)
 				break;
 
 			if (target % (ratio * Fref)) {
-- 
1.7.2.5

[PATCH 4/7] ASoC: arizona: Coding standards, remove unneeded brackets

[Charles Keepax]

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/arizona.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/arizona.c b/sound/soc/codecs/arizona.c
index 97cc80d..9a369bc 100644
--- a/sound/soc/codecs/arizona.c
+++ b/sound/soc/codecs/arizona.c
@@ -1913,11 +1913,10 @@ int arizona_set_fll(struct arizona_fll *fll, int source,
 	fll->sync_freq = Fref;
 	fll->fout = Fout;
 
-	if (Fout) {
+	if (Fout)
 		arizona_enable_fll(fll);
-	} else {
+	else
 		arizona_disable_fll(fll);
-	}
 
 	return 0;
 }
-- 
1.7.2.5

[PATCH 5/7] ASoC: arizona: Correct return value of arizona_is_enabled_fll

[Charles Keepax]

arizona_is_enabled_fll currently returns a bool, but can throw an error.
The error will be basically ignored and we will treat the FLL as already
on. This patch changes the return to be an int and adds error code to
propagate the error up to the callback.

Reported-by: Anil Kumar <anil.kumar@wolfsonmicro.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/arizona.c |   26 ++++++++++++++++----------
 1 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/sound/soc/codecs/arizona.c b/sound/soc/codecs/arizona.c
index 9a369bc..1daa5cc 100644
--- a/sound/soc/codecs/arizona.c
+++ b/sound/soc/codecs/arizona.c
@@ -1761,7 +1761,7 @@ static void arizona_apply_fll(struct arizona *arizona, unsigned int base,
 				 ARIZONA_FLL1_CTRL_UPD | cfg->n);
 }
 
-static bool arizona_is_enabled_fll(struct arizona_fll *fll)
+static int arizona_is_enabled_fll(struct arizona_fll *fll)
 {
 	struct arizona *arizona = fll->arizona;
 	unsigned int reg;
@@ -1777,13 +1777,17 @@ static bool arizona_is_enabled_fll(struct arizona_fll *fll)
 	return reg & ARIZONA_FLL1_ENA;
 }
 
-static void arizona_enable_fll(struct arizona_fll *fll)
+static int arizona_enable_fll(struct arizona_fll *fll)
 {
 	struct arizona *arizona = fll->arizona;
 	int ret;
 	bool use_sync = false;
+	int already_enabled = arizona_is_enabled_fll(fll);
 	struct arizona_fll_cfg cfg;
 
+	if (already_enabled < 0)
+		return already_enabled;
+
 	/*
 	 * If we have both REFCLK and SYNCCLK then enable both,
 	 * otherwise apply the SYNCCLK settings to REFCLK.
@@ -1811,7 +1815,7 @@ static void arizona_enable_fll(struct arizona_fll *fll)
 					 ARIZONA_FLL1_SYNC_ENA, 0);
 	} else {
 		arizona_fll_err(fll, "No clocks provided\n");
-		return;
+		return -EINVAL;
 	}
 
 	/*
@@ -1826,7 +1830,7 @@ static void arizona_enable_fll(struct arizona_fll *fll)
 					 ARIZONA_FLL1_SYNC_BW,
 					 ARIZONA_FLL1_SYNC_BW);
 
-	if (!arizona_is_enabled_fll(fll))
+	if (!already_enabled)
 		pm_runtime_get(arizona->dev);
 
 	/* Clear any pending completions */
@@ -1845,6 +1849,8 @@ static void arizona_enable_fll(struct arizona_fll *fll)
 					  msecs_to_jiffies(250));
 	if (ret == 0)
 		arizona_fll_warn(fll, "Timed out waiting for lock\n");
+
+	return 0;
 }
 
 static void arizona_disable_fll(struct arizona_fll *fll)
@@ -1866,7 +1872,7 @@ static void arizona_disable_fll(struct arizona_fll *fll)
 int arizona_set_fll_refclk(struct arizona_fll *fll, int source,
 			   unsigned int Fref, unsigned int Fout)
 {
-	int ret;
+	int ret = 0;
 
 	if (fll->ref_src == source && fll->ref_freq == Fref)
 		return 0;
@@ -1881,17 +1887,17 @@ int arizona_set_fll_refclk(struct arizona_fll *fll, int source,
 	fll->ref_freq = Fref;
 
 	if (fll->fout && Fref > 0) {
-		arizona_enable_fll(fll);
+		ret = arizona_enable_fll(fll);
 	}
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(arizona_set_fll_refclk);
 
 int arizona_set_fll(struct arizona_fll *fll, int source,
 		    unsigned int Fref, unsigned int Fout)
 {
-	int ret;
+	int ret = 0;
 
 	if (fll->sync_src == source &&
 	    fll->sync_freq == Fref && fll->fout == Fout)
@@ -1914,11 +1920,11 @@ int arizona_set_fll(struct arizona_fll *fll, int source,
 	fll->fout = Fout;
 
 	if (Fout)
-		arizona_enable_fll(fll);
+		ret = arizona_enable_fll(fll);
 	else
 		arizona_disable_fll(fll);
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(arizona_set_fll);
 
-- 
1.7.2.5

[PATCH 6/7] ASoC: arizona: FLL freerun only required whilst disabling

[Charles Keepax]

The FLL freerun is only required whilst we disable the FLL not the
entire time the FLL is disabled. This patch moves the FLL freerun
disable from the enable sequence to the disable sequence.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/arizona.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/arizona.c b/sound/soc/codecs/arizona.c
index 1daa5cc..a044e29 100644
--- a/sound/soc/codecs/arizona.c
+++ b/sound/soc/codecs/arizona.c
@@ -1837,8 +1837,6 @@ static int arizona_enable_fll(struct arizona_fll *fll)
 	try_wait_for_completion(&fll->ok);
 
 	regmap_update_bits_async(arizona->regmap, fll->base + 1,
-				 ARIZONA_FLL1_FREERUN, 0);
-	regmap_update_bits_async(arizona->regmap, fll->base + 1,
 				 ARIZONA_FLL1_ENA, ARIZONA_FLL1_ENA);
 	if (use_sync)
 		regmap_update_bits_async(arizona->regmap, fll->base + 0x11,
@@ -1864,6 +1862,8 @@ static void arizona_disable_fll(struct arizona_fll *fll)
 				 ARIZONA_FLL1_ENA, 0, &change);
 	regmap_update_bits(arizona->regmap, fll->base + 0x11,
 			   ARIZONA_FLL1_SYNC_ENA, 0);
+	regmap_update_bits_async(arizona->regmap, fll->base + 1,
+				 ARIZONA_FLL1_FREERUN, 0);
 
 	if (change)
 		pm_runtime_put_autosuspend(arizona->dev);
-- 
1.7.2.5

[PATCH 7/7] ASoC: arizona: Update handling for input change on an active FLL

[Charles Keepax]

Currently, the driver places no restrictions on changes that can be
applied to an active FLL. However, it is only possible to change the
input for an active FLL, to change the output the FLL should be stopped
and then recofigured. This patch disallows changes in output frequency
and adds some additional handling to ensure the output remains
consistent across an input transition.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/arizona.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/sound/soc/codecs/arizona.c b/sound/soc/codecs/arizona.c
index a044e29..ca41586 100644
--- a/sound/soc/codecs/arizona.c
+++ b/sound/soc/codecs/arizona.c
@@ -1545,6 +1545,12 @@ static int arizona_validate_fll(struct arizona_fll *fll,
 {
 	unsigned int Fvco_min;
 
+	if (fll->fout && Fout != fll->fout) {
+		arizona_fll_err(fll,
+				"Can't change output on active FLL\n");
+		return -EINVAL;
+	}
+
 	if (Fref / ARIZONA_FLL_MAX_REFDIV > ARIZONA_FLL_MAX_FREF) {
 		arizona_fll_err(fll,
 				"Can't scale %dMHz in to <=13.5MHz\n",
@@ -1788,6 +1794,15 @@ static int arizona_enable_fll(struct arizona_fll *fll)
 	if (already_enabled < 0)
 		return already_enabled;
 
+	if (already_enabled) {
+		/* Facilitate smooth refclk across the transition */
+		regmap_update_bits_async(fll->arizona->regmap, fll->base + 0x7,
+					 ARIZONA_FLL1_GAIN_MASK, 0);
+		regmap_update_bits_async(fll->arizona->regmap, fll->base + 1,
+					 ARIZONA_FLL1_FREERUN,
+					 ARIZONA_FLL1_FREERUN);
+	}
+
 	/*
 	 * If we have both REFCLK and SYNCCLK then enable both,
 	 * otherwise apply the SYNCCLK settings to REFCLK.
@@ -1843,6 +1858,10 @@ static int arizona_enable_fll(struct arizona_fll *fll)
 					 ARIZONA_FLL1_SYNC_ENA,
 					 ARIZONA_FLL1_SYNC_ENA);
 
+	if (already_enabled)
+		regmap_update_bits_async(arizona->regmap, fll->base + 1,
+					 ARIZONA_FLL1_FREERUN, 0);
+
 	ret = wait_for_completion_timeout(&fll->ok,
 					  msecs_to_jiffies(250));
 	if (ret == 0)
-- 
1.7.2.5

Re: [PATCH 2/7] ASoC: arizona: Correct checking of FLL ratio limitations

[Mark Brown]

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

On Wed, Jul 09, 2014 at 05:41:44PM +0100, Charles Keepax wrote:
> The check to ensure the Fref frequency is within the bounds for the
> current ratio, was placed in the wrong loop. The initial configuration
> will always be valid and the loop lowering the ratio will only reinforce
> this validity. The check should be on the loop increasing the ratio.
> This could on occasion cause an invalid ratio/Fref combination to be
> selected.

Applied all, thanks.  You should really have put this and patch 3 first
since they're bug fixes.

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

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