Re: [PATCH] ASoC: intel: cht_bsw_max98090_ti: Enable codec clock once and keep it enabled

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

 




On 1/24/19 4:34 AM, Hans de Goede wrote:
Users have been seeing sound stability issues with max98090 codecs since:
commit 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL")

At first that commit broke sound for Chromebook Swanky and Clapper models,
the problem was that the machine-driver has been controlling the wrong
clock on those models since support for them was added. This was hidden by
clk-pmc-atom.c keeping the actual clk on unconditionally.

With the machine-driver controlling the proper clock, sound works again
but we are seeing bug reports describing it as: low volume,
"sounds like played at 10x speed" and instable.

When these issues are hit the following message is seen in dmesg:
"max98090 i2c-193C9890:00: PLL unlocked".

Attempts have been made to fix this by inserting a delay between enabling
the clk and enabling and checking the pll, but this has not helped.

It seems that if we ever disable the clk, the pll looses its lock and
then we get various issues.

This commit fixes this by enabling the clock once at probe time,
in essence restoring the old behavior of clk-pmc-atom.c always keeping
the clk on, but then only on machines with a max98090 codec.

Fixes: 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL")
Reported-and-tested-by: Mogens Jensen <mogens-jensen@xxxxxxxxxxxxxx>
Reported-and-tested-by: Dean Wallace <duffydack73@xxxxxxxxx>
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>

Sorry, I am not really convinced this patch can be applied as is.

1. All these clock changes were introduced to solve S0i1 issues, so now if we keep the clocks on isn't this going to impact suspend/resume usages? Has anyone looked into this? I am also wondering what the assumptions were on what the firmware does, and if the issues occur with the legacy firmware or Coreboot?

2. Also the fix is incorrect in that the clock needs to be set to 19.2 MHz for Baytrail devices (Rambi and friends) which reuse this machine driver. Even if there was agreement on leaving the clocks on, the clk_set_rate() call needs to be added back (default is 25 MHz on Baytrail).

_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel



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

  Powered by Linux