Re: [PATCH 02/11] ASoC: rt5651: Remove is_sys_clk_from_pll, it has ordering issues

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux