On 9/18/19 16:31, Charles Keepax wrote:
@@ -2315,6 +2396,8 @@ static int _wm8994_set_fll(struct snd_soc_component *component, int id, int src,
active_dereference(component);
}
I don't think this works in the case of changing active FLLs.
The driver looks like it allows changing the FLL configuration
whilst the FLL is already active in which case it you would have
two wm8994_set_fll calls enabling the FLL but only a single one
disabling it. Resulting in the FLL being off but the MCLK being
left enabled.
Indeed I missed this scenario, or rather assumed it won't be used.
But since the driver allows reconfiguring active FLLs we should make
sure such use case remains properly supported.
What I came up so far as a fix is reading current FLL refclk source and
if FLL was enabled with that source disabling refclk, before we change FLL
configuration to new one. So we have clk_disable_unprepare(MCLK) more
closely following FLL enable bit changes. I have tested it and it seems
to work - something like below. Do you think it makes sense?
--
Regards,
Sylwester
---------8<----------
diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c
index bf02e8908d5a..78a0835a095e 100644
--- a/sound/soc/codecs/wm8994.c
+++ b/sound/soc/codecs/wm8994.c
@@ -2277,6 +2277,31 @@ static int _wm8994_set_fll(struct snd_soc_component *component, int id, int src,
snd_soc_component_update_bits(component, WM8994_FLL1_CONTROL_1 + reg_offset,
WM8994_FLL1_ENA, 0);
+ /* Disable MCLK if needed before we possibly change to new clock parent */
+ if (was_enabled) {
+ reg = snd_soc_component_read32(component, WM8994_FLL1_CONTROL_5
+ + reg_offset);
+ reg = ((reg & WM8994_FLL1_REFCLK_SRC_MASK)
+ >> WM8994_FLL1_REFCLK_SRC_SHIFT) + 1;
+
+ switch (reg) {
+ case WM8994_FLL_SRC_MCLK1:
+ mclk = control->mclk[WM8994_MCLK1].clk;
+ break;
+ case WM8994_FLL_SRC_MCLK2:
+ mclk = control->mclk[WM8994_MCLK2].clk;
+ break;
+ default:
+ mclk = NULL;
+ }
+
+ clk_disable_unprepare(mclk);
+ }
+
if (wm8994->fll_byp && src == WM8994_FLL_SRC_BCLK &&
freq_in == freq_out && freq_out) {
dev_dbg(component->dev, "Bypassing FLL%d\n", id + 1);
@@ -2396,8 +2420,6 @@ static int _wm8994_set_fll(struct snd_soc_component *component, int id, int src,
active_dereference(component);
}
- if (mclk)
- clk_disable_unprepare(mclk);
}
out:
---------8<----------
