On Thu, Sep 19, 2019 at 01:58:35PM +0200, Sylwester Nawrocki wrote: > 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? > Yeah I think that looks good, it is very similar to what we did on Arizona and I haven't found any problems with that yet :-) Thanks, Charles
