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.
I really don't see how configuring a broken sysclk (GBL_CLK points to PLL1, but PLL-bit in PWR_ANLG2 is OFF) is ever a good thing, instead we should make sure we always switch to the RCCLK when we don't need the SYSCLK.
Last force-enabling the PLL bit on all machines where we can do jack-detection (not only over-current, but the JD irq in general needs a valid sysclk) would in practice mean always force-enabling the PLL bit since almost all boards have jack-detection once we add the necessary quirks, so this really is not a solution.
A bit confused here as well, sorry - I think you're assuming I'm suggesting some particular solution but I'm not sure what that is.