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);
>> }
>> + if (mclk)
>> + clk_disable_unprepare(mclk);
>
> 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<----------
