Hi,
On 22-02-18 11:48, Mark Brown wrote:
On Wed, Feb 21, 2018 at 08:38:01PM +0100, Hans de Goede wrote:
On 21-02-18 12:18, Mark Brown wrote:
On Tue, Feb 20, 2018 at 11:15:02PM +0100, Hans de Goede wrote:
Only if jack detection is enabled, if jack detection is not in use for some reason then the PLL isn't required and should be powered off - this is normally handled by having the jack detection code force enable things.
As the commit message tries to explain, the code this removes is fundamentally broken and this is not jack-detection related:
"dapm_power_widgets() first builds a list of which widgets to power-up before actually powering any of them up. For dapm-supply widgets their connected method, in our case is_sys_clk_from_pll() get called at this point.Before this commit is_sys_clk_from_pll() was looking at the actually selected clock in the RT5651_GBL_CLK register. But the sysclk itself is selected by another dapm-supply (the "Platform Clock" supply in the machine driver) and any changes to that supply part of the same power- transition get executed after building the list, causing is_sys_clk_from_pll() to return the wrong value."
Right, but there's a couple of jumps in your reasoning with the actual solution.
As for that the PLL should be powered-off when not needed, I agree but that is controlled by the machine-driver through the "Platform Clock" dapm supply and if that leaves things on for some reason then that is the problem which we actually need to fix, e.g. this will also leave the MCLK input clk enabled needlessly.
I don't follow this bit, sorry. If there's a DAPM supply that's being enabled when it's not needed then we should just disable it.
Right, that is exactly what I'm saying too.
Whether we use PLL1 as sys-clk or the RCCLK is controlled by the "Platform Clock" dapm supply. If we proper disable that supply when we don't need PLL1, then it will switch to RCCLK and we can simply have rt5651_set_dai_sysclk() enable / disable the PLL pwr bit in PWR_ANLG2 when we switch clock rather then using the is_sys_clk_from_pll() check.
I hope this helps clear up the confusion surrounding this patch. And I guess I need to improve the commit message...
Regards,
Hans