[alsa-devel] [PATCH 0/7] Arizona FLL Bug Fixes
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(-)
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;
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;
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.
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)) {
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; }
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);
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);
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)
participants (2)
-
Charles Keepax -
Mark Brown