[alsa-devel] [PATCH 1/5] ASoC: twl4030 - AIF/APLL clock fix for all DAPM routes.

Peter Ujfalusi peter.ujfalusi at nokia.com
Wed Apr 28 08:46:03 CEST 2010


Hi,

one more thing...

On Tuesday 27 April 2010 17:54:16 ext Liam Girdwood wrote:
> This allows the AIF and APLL clocks to continue running in
> loopback mode and when invalid DAPM routes are selected at
> stream runtime.
> 
> Acked-by: Peter Ujfalusi <peter.ujfalusi at nokia.com>
> Acked-by: Mark Brown <broonie at opensource.wolfsonmicro.com>
> Signed-off-by: Liam Girdwood <lrg at slimlogic.co.uk>
> ---
>  sound/soc/codecs/twl4030.c |  123
> +++++++++++++++++++++++++++++-------------- 1 files changed, 83
> insertions(+), 40 deletions(-)
> 
> diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c
> index dc26b71..1a3e7f6 100644
> --- a/sound/soc/codecs/twl4030.c
> +++ b/sound/soc/codecs/twl4030.c
> @@ -123,7 +123,10 @@ struct twl4030_priv {
>  	struct snd_soc_codec codec;
> 
>  	unsigned int codec_powered;
> +
> +	/* reference counts of AIF/APLL users */
>  	unsigned int apll_enabled;
> +	unsigned int aif_enabled;
> 
>  	struct snd_pcm_substream *master_substream;
>  	struct snd_pcm_substream *slave_substream;
> @@ -259,22 +262,50 @@ static void twl4030_init_chip(struct snd_soc_codec
> *codec) static void twl4030_apll_enable(struct snd_soc_codec *codec, int
> enable) {
>  	struct twl4030_priv *twl4030 = snd_soc_codec_get_drvdata(codec);
> -	int status;
> -
> -	if (enable == twl4030->apll_enabled)
> -		return;
> +	int status = -1;
> 
> +	/* update the user count */
>  	if (enable)
> +		twl4030->apll_enabled++;
> +	else
> +		twl4030->apll_enabled--;
> +
> +	if (enable && twl4030->apll_enabled == 1)
>  		/* Enable PLL */
>  		status = twl4030_codec_enable_resource(TWL4030_CODEC_RES_APLL);
> -	else
> +	else if (!enable && twl4030->apll_enabled == 0)
>  		/* Disable PLL */
>  		status = twl4030_codec_disable_resource(TWL4030_CODEC_RES_APLL);
> 
>  	if (status >= 0)
>  		twl4030_write_reg_cache(codec, TWL4030_REG_APLL_CTL, status);
> +}
> 
> -	twl4030->apll_enabled = enable;
> +static void  twl4030_aif_enable(struct snd_soc_codec *codec, int enable)
> +{
> +	struct twl4030_priv *twl4030 = codec->private_data;
> +
> +	/* update the user count */
> +	if (enable)
> +		twl4030->aif_enabled++;
> +	else
> +		twl4030->aif_enabled--;
> +
> +	if (enable && twl4030->aif_enabled == 1) {
> +		/* Enable AIF */
> +		/* enable the PLL before we use it to clock the DAI */
> +		twl4030_apll_enable(codec, 1);
> +
> +		twl4030_write(codec, TWL4030_REG_AUDIO_IF,
> +							twl4030_read_reg_cache(codec, TWL4030_REG_AUDIO_IF)
> +							| TWL4030_AIF_EN);
> +	} else if (!enable && twl4030->aif_enabled == 0) {
> +		/* disable the DAI before we stop it's source PLL */
> +		twl4030_write(codec, TWL4030_REG_AUDIO_IF,
> +							twl4030_read_reg_cache(codec, TWL4030_REG_AUDIO_IF)
> +							& ~TWL4030_AIF_EN);
> +		twl4030_apll_enable(codec, 0);
> +	}
>  }

These lines are quite long, would it be better to introduce a variable to make 
the code look better? Something like this instead:

-       twl4030->apll_enabled = enable;
+static void  twl4030_aif_enable(struct snd_soc_codec *codec, int enable)
+{
+       struct twl4030_priv *twl4030 = snd_soc_codec_get_drvdata(codec);
+       u8 audio_if;
+
+       /* update the user count */
+       if (enable)
+               twl4030->aif_enabled++;
+       else
+               twl4030->aif_enabled--;
+
+       audio_if = twl4030_read_reg_cache(codec, TWL4030_REG_AUDIO_IF);
+       if (enable && twl4030->aif_enabled == 1) {
+               /* Enable AIF */
+               /* enable the PLL before we use it to clock the DAI */
+               twl4030_apll_enable(codec, 1);
+
+               twl4030_write(codec, TWL4030_REG_AUDIO_IF,
+                                               audio_if | TWL4030_AIF_EN);
+       } else if (!enable && twl4030->aif_enabled == 0) {
+               /* disable the DAI before we stop it's source PLL */
+               twl4030_write(codec, TWL4030_REG_AUDIO_IF,
+                                               audio_if &  ~TWL4030_AIF_EN);
+               twl4030_apll_enable(codec, 0);
+       }
 }

> 
>  static void twl4030_power_up(struct snd_soc_codec *codec)
> @@ -672,6 +703,20 @@ static int apll_event(struct snd_soc_dapm_widget *w,
>  	return 0;
>  }
> 
> +static int aif_event(struct snd_soc_dapm_widget *w,
> +		struct snd_kcontrol *kcontrol, int event)
> +{
> +	switch (event) {
> +	case SND_SOC_DAPM_PRE_PMU:
> +		twl4030_aif_enable(w->codec, 1);
> +		break;
> +	case SND_SOC_DAPM_POST_PMD:
> +		twl4030_aif_enable(w->codec, 0);
> +		break;
> +	}
> +	return 0;
> +}
> +
>  static void headset_ramp(struct snd_soc_codec *codec, int ramp)
>  {
>  	struct snd_soc_device *socdev = codec->socdev;
> @@ -1178,6 +1223,11 @@ static const struct snd_soc_dapm_widget
> twl4030_dapm_widgets[] = { SND_SOC_DAPM_OUTPUT("HFR"),
>  	SND_SOC_DAPM_OUTPUT("VIBRA"),
> 
> +	/* AIF and APLL clocks for running DAIs (including loopback) */
> +	SND_SOC_DAPM_OUTPUT("AIF DAC"),
> +	SND_SOC_DAPM_INPUT("AIF ADC"),
> +	SND_SOC_DAPM_OUTPUT("APLL"),
> +
>  	/* DACs */
>  	SND_SOC_DAPM_DAC("DAC Right1", "Right Front HiFi Playback",
>  			SND_SOC_NOPM, 0, 0),
> @@ -1215,16 +1265,21 @@ static const struct snd_soc_dapm_widget
> twl4030_dapm_widgets[] = { &twl4030_dapm_dbypassv_control),
> 
>  	/* Digital mixers, power control for the physical DACs */
> -	SND_SOC_DAPM_MIXER("Digital R1 Playback Mixer",
> -			TWL4030_REG_AVDAC_CTL, 0, 0, NULL, 0),
> -	SND_SOC_DAPM_MIXER("Digital L1 Playback Mixer",
> -			TWL4030_REG_AVDAC_CTL, 1, 0, NULL, 0),
> -	SND_SOC_DAPM_MIXER("Digital R2 Playback Mixer",
> -			TWL4030_REG_AVDAC_CTL, 2, 0, NULL, 0),
> -	SND_SOC_DAPM_MIXER("Digital L2 Playback Mixer",
> -			TWL4030_REG_AVDAC_CTL, 3, 0, NULL, 0),
> -	SND_SOC_DAPM_MIXER("Digital Voice Playback Mixer",
> -			TWL4030_REG_AVDAC_CTL, 4, 0, NULL, 0),
> +	SND_SOC_DAPM_MIXER_E("Digital R1 Playback Mixer",
> +			TWL4030_REG_AVDAC_CTL, 0, 0, NULL, 0, aif_event,
> +			SND_SOC_DAPM_PRE_PMU|SND_SOC_DAPM_POST_PMD),
> +	SND_SOC_DAPM_MIXER_E("Digital L1 Playback Mixer",
> +			TWL4030_REG_AVDAC_CTL, 1, 0, NULL, 0, aif_event,
> +			SND_SOC_DAPM_PRE_PMU|SND_SOC_DAPM_POST_PMD),
> +	SND_SOC_DAPM_MIXER_E("Digital R2 Playback Mixer",
> +			TWL4030_REG_AVDAC_CTL, 2, 0, NULL, 0, aif_event,
> +			SND_SOC_DAPM_PRE_PMU|SND_SOC_DAPM_POST_PMD),
> +	SND_SOC_DAPM_MIXER_E("Digital L2 Playback Mixer",
> +			TWL4030_REG_AVDAC_CTL, 3, 0, NULL, 0, aif_event,
> +			SND_SOC_DAPM_PRE_PMU|SND_SOC_DAPM_POST_PMD),
> +	SND_SOC_DAPM_MIXER_E("Digital Voice Playback Mixer",
> +			TWL4030_REG_AVDAC_CTL, 4, 0, NULL, 0, apll_event,
> +			SND_SOC_DAPM_PRE_PMU|SND_SOC_DAPM_POST_PMD),
> 
>  	/* Analog mixers, power control for the physical PGAs */
>  	SND_SOC_DAPM_MIXER("Analog R1 Playback Mixer",
> @@ -1238,11 +1293,6 @@ static const struct snd_soc_dapm_widget
> twl4030_dapm_widgets[] = { SND_SOC_DAPM_MIXER("Analog Voice Playback
> Mixer",
>  			TWL4030_REG_VDL_APGA_CTL, 0, 0, NULL, 0),
> 
> -	SND_SOC_DAPM_SUPPLY("APLL Enable", SND_SOC_NOPM, 0, 0, apll_event,
> -			    SND_SOC_DAPM_PRE_PMU|SND_SOC_DAPM_POST_PMD),
> -
> -	SND_SOC_DAPM_SUPPLY("AIF Enable", TWL4030_REG_AUDIO_IF, 0, 0, NULL, 0),
> -
>  	/* Output MIXER controls */
>  	/* Earpiece */
>  	SND_SOC_DAPM_MIXER("Earpiece Mixer", SND_SOC_NOPM, 0, 0,
> @@ -1370,17 +1420,14 @@ static const struct snd_soc_dapm_route intercon[] =
> { {"Digital R2 Playback Mixer", NULL, "DAC Right2"},
>  	{"Digital Voice Playback Mixer", NULL, "DAC Voice"},
> 
> -	/* Supply for the digital part (APLL) */
> -	{"Digital R1 Playback Mixer", NULL, "APLL Enable"},
> -	{"Digital L1 Playback Mixer", NULL, "APLL Enable"},
> -	{"Digital R2 Playback Mixer", NULL, "APLL Enable"},
> -	{"Digital L2 Playback Mixer", NULL, "APLL Enable"},
> -	{"Digital Voice Playback Mixer", NULL, "APLL Enable"},
> +	/* Route for the digital part (APLL) */
> +	{"APLL", NULL, "Digital Voice Playback Mixer"},
> 
> -	{"Digital R1 Playback Mixer", NULL, "AIF Enable"},
> -	{"Digital L1 Playback Mixer", NULL, "AIF Enable"},
> -	{"Digital R2 Playback Mixer", NULL, "AIF Enable"},
> -	{"Digital L2 Playback Mixer", NULL, "AIF Enable"},
> +	/* Route for digital part (AIF) */
> +	{"AIF DAC", NULL, "Digital L1 Playback Mixer"},
> +	{"AIF DAC", NULL, "Digital R1 Playback Mixer"},
> +	{"AIF DAC", NULL, "Digital L2 Playback Mixer"},
> +	{"AIF DAC", NULL, "Digital R2 Playback Mixer"},
> 
>  	{"Analog L1 Playback Mixer", NULL, "Digital L1 Playback Mixer"},
>  	{"Analog R1 Playback Mixer", NULL, "Digital R1 Playback Mixer"},
> @@ -1493,15 +1540,11 @@ static const struct snd_soc_dapm_route intercon[] =
> { {"ADC Virtual Left2", NULL, "TX2 Capture Route"},
>  	{"ADC Virtual Right2", NULL, "TX2 Capture Route"},
> 
> -	{"ADC Virtual Left1", NULL, "APLL Enable"},
> -	{"ADC Virtual Right1", NULL, "APLL Enable"},
> -	{"ADC Virtual Left2", NULL, "APLL Enable"},
> -	{"ADC Virtual Right2", NULL, "APLL Enable"},
> -
> -	{"ADC Virtual Left1", NULL, "AIF Enable"},
> -	{"ADC Virtual Right1", NULL, "AIF Enable"},
> -	{"ADC Virtual Left2", NULL, "AIF Enable"},
> -	{"ADC Virtual Right2", NULL, "AIF Enable"},
> +	/* Route for digital part AIF */
> +	{"ADC Virtual Left1", NULL, "AIF ADC"},
> +	{"ADC Virtual Right1", NULL, "AIF ADC"},
> +	{"ADC Virtual Left2", NULL, "AIF ADC"},
> +	{"ADC Virtual Right2", NULL, "AIF ADC"},
> 
>  	/* Analog bypass routes */
>  	{"Right1 Analog Loopback", "Switch", "Analog Right"},

-- 
Péter


More information about the Alsa-devel mailing list