-----Original Message----- From: Mark Brown [mailto:broonie@kernel.org] Sent: Monday, March 09, 2015 8:20 PM To: Bard Liao Cc: lgirdwood@gmail.com; alsa-devel@alsa-project.org; lars@metafoo.de; Flove; Oder Chiou; John Lin; mengdong.lin@intel.com; yao.jin@intel.com Subject: Re: [PATCH 1/2] ASoC: rt5670: Revert Keep sysclk on patch
On Mon, Mar 09, 2015 at 12:13:04PM +0000, Bard Liao wrote:
System clock is needed for rt5670 jack detection function. Usually, the system clock source is from MCLK. But some platform such as Intel BSW will turn off MCLK in idle mode. So rt5670 need to switch the system clock source to its internal clock. The action is done by machine driver.
OK... but if this is the case why does your patch not update any machine drivers?
The sysclk source switching is already in cht_bsw_rt5672.c. But it switches to internal clock only but doesn't switch back to PLL. Jin Yao will send a patch to fix it.
There is a PLL power in rt5670. It should be on if we want to use PLL as the system clock source. So we call force enable/disable pin in rt5670_set_dai_sysclk. However, cht_bsw_rt5672.c call snd_soc_dai_set_sysclk in platform_clock_control which is the dapm event of "Platform Clock" widget. In that case, snd_soc_dapm_disable_pin and snd_soc_dapm_sync are called within a dapm event. I think it is not valid.
This is very unusual, normally such system clock reconfiguration would be done in the set_bias_level() callback not in a DAPM event. Doing it in a DAPM event sounds unreliable as there are only weak guarantees about the state of the system.
------Please consider the environment before printing this e-mail.