Re: [PATCH v4-resend 1/7] ASoC: sirf: Add SiRF internal audio codec driver

[RongJun Ying <rjying@gmail.com>]

2014-03-05 11:34 GMT+08:00 Mark Brown <broonie@kernel.org>:
> On Tue, Mar 04, 2014 at 01:11:50PM +0800, RongJun Ying wrote:
>> 2014-03-04 12:58 GMT+08:00 Mark Brown <broonie@kernel.org>:
>> > On Mon, Mar 03, 2014 at 03:48:02PM +0800, Barry Song wrote:
>
>> >> here the clock is an internal clock in the internal codec. the
>> >> external clock of audio controller is managed in system suspend/resume
>> >> as you see. but they are not managed in runtime suspend/resume here
>> >> because the external clock is always enabled to make sure the charge
>> >> pump to be working to make the board have stable current and voltage.
>> >> otherwise, touchscreen ADC will not be accurate.
>
>> > Why aren't these drivers keeping the clock enabled themselves?
>
>> The charge pump register is a part of  the audio controller.
>> But this register is impacted the touchscreen ADC stable.
>> So when the audio codec driver probe, It always enable the audio
>> controller's clock,
>> and must be set the charge pump register.
>
> I'm sorry but I just can't follow what you're talking about at all.  In
> one mail above you were talking about an external clock but here you're
> talking about the charge pump.

The SiRF internal audio system has two clocks. The one is the audio
module clock.
The I2S, internal audio and AC97 module use same clock. This clock is
managed by clock
controller. And use the clock API to enable/disable it.
       ret = clk_prepare_enable(sirf_audio_codec->clk);
....
     clk_disable_unprepare(sirf_audio_codec->clk);
This clock need keeping enabled. The reason is the charge pump need
alway set. Otherwise,
the touch ADC  is unstable.

The other clock is the internal audio codec clock. This clock is not
managed by clock controller.
Enable and disable this clock need update the audio codec's register.
When runtime resume, this clock is enabled and reset it:

+static int sirf_audio_codec_runtime_resume(struct device *dev)
+{
+       struct sirf_audio_codec *sirf_audio_codec = dev_get_drvdata(dev);
+       regmap_update_bits(sirf_audio_codec->regmap, AUDIO_IC_CODEC_CTRL1,
+               (1 << sirf_audio_codec->reg_bits->codec_clk_en_bits) |
+               (1 << sirf_audio_codec->reg_bits->por_bits),
+               (1 << sirf_audio_codec->reg_bits->codec_clk_en_bits));
+       msleep(20);
+       regmap_update_bits(sirf_audio_codec->regmap, AUDIO_IC_CODEC_CTRL1,
+               (1 << sirf_audio_codec->reg_bits->por_bits),
+               (1 << sirf_audio_codec->reg_bits->por_bits));
+       return 0;
+}

And runtime suspend, this clock is disabled:

+static int sirf_audio_codec_runtime_suspend(struct device *dev)
+{
+       struct sirf_audio_codec *sirf_audio_codec = dev_get_drvdata(dev);
+       regmap_update_bits(sirf_audio_codec->regmap, AUDIO_IC_CODEC_CTRL1,
+               (1 << sirf_audio_codec->reg_bits->codec_clk_en_bits),
+               0);
+       return 0;
+}

In v5, I use the codecclk supply to instead the runtime resume/suspend:

+static void enable_and_reset_codec(struct regmap *regmap,
+               u32 codec_enable_bits, u32 codec_reset_bits)
+{
+       regmap_update_bits(regmap, AUDIO_IC_CODEC_CTRL1,
+                       codec_enable_bits | codec_reset_bits,
+                       codec_enable_bits | ~codec_reset_bits);
+       msleep(20);
+       regmap_update_bits(regmap, AUDIO_IC_CODEC_CTRL1,
+                       codec_reset_bits, codec_reset_bits);
+}
+
+static int atlas6_codec_enable_and_reset_event(struct snd_soc_dapm_widget *w,
+               struct snd_kcontrol *kcontrol, int event)
+{
+#define ATLAS6_CODEC_ENABLE_BITS (1 << 29)
+#define ATLAS6_CODEC_RESET_BITS (1 << 28)
+       struct sirf_audio_codec *sirf_audio_codec =
dev_get_drvdata(w->codec->dev);
+       switch (event) {
+       case SND_SOC_DAPM_PRE_PMU:
+               enable_and_reset_codec(sirf_audio_codec->regmap,
+                       ATLAS6_CODEC_ENABLE_BITS, ATLAS6_CODEC_RESET_BITS);
+               break;
+       case SND_SOC_DAPM_POST_PMD:
+               regmap_update_bits(sirf_audio_codec->regmap,
+                       AUDIO_IC_CODEC_CTRL1, ATLAS6_CODEC_ENABLE_BITS,
+                       ~ATLAS6_CODEC_ENABLE_BITS);
+               break;
+       default:
+               break;
+       }
+
+       return 0;
+}
+
+static int prima2_codec_enable_and_reset_event(struct snd_soc_dapm_widget *w,
+               struct snd_kcontrol *kcontrol, int event)
+{
+#define PRIMA2_CODEC_ENABLE_BITS (1 << 27)
+#define PRIMA2_CODEC_RESET_BITS (1 << 26)
+       struct sirf_audio_codec *sirf_audio_codec =
dev_get_drvdata(w->codec->dev);
+       switch (event) {
+       case SND_SOC_DAPM_POST_PMU:
+               enable_and_reset_codec(sirf_audio_codec->regmap,
+                       PRIMA2_CODEC_ENABLE_BITS, PRIMA2_CODEC_RESET_BITS);
+               break;
+       case SND_SOC_DAPM_POST_PMD:
+               regmap_update_bits(sirf_audio_codec->regmap,
+                       AUDIO_IC_CODEC_CTRL1, PRIMA2_CODEC_ENABLE_BITS,
+                       ~PRIMA2_CODEC_ENABLE_BITS);
+               break;
+       default:
+               break;
+       }
+
+       return 0;
+}

+static const struct snd_soc_dapm_widget atlas6_codec_clock_dapm_widget =
+       SND_SOC_DAPM_SUPPLY("codecclk", SND_SOC_NOPM, 0, 0,
+                       atlas6_codec_enable_and_reset_event,
+                       SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD);
+
+static const struct snd_soc_dapm_widget prima2_codec_clock_dapm_widget =
+       SND_SOC_DAPM_SUPPLY("codecclk", SND_SOC_NOPM, 0, 0,
+                       prima2_codec_enable_and_reset_event,
+                       SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD);

Thanks
-- 
------------------------------
Rongjun Ying