Re: [PATCH 2/2] ASoC: tas2552: Add DAPM calls for amp and PLL

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2056 bytes --]

On Fri, Jul 25, 2014 at 06:54:19PM +0000, Murphy, Dan wrote:
> On 07/21/2014 06:54 AM, Mark Brown wrote:
> > On Fri, Jul 18, 2014 at 12:31:08PM -0500, Dan Murphy wrote:

> >> +static const struct snd_soc_dapm_widget tas2552_dapm_widgets[] =
> >> +{
> >> +SND_SOC_DAPM_PRE("Class D Enable", tas2552_class_d_en),
> >> +SND_SOC_DAPM_POST("Class D Disable", tas2552_class_d_en),
> >> +SND_SOC_DAPM_POST("PLL Disable", tas2552_pll_disable),
> >> +};

> > This seems broken, having to use _PRE or _POST widgets for simple
> > register writes (or almost anything really) should never be required and
> > error prone - what is this actually trying to do?  I'd expect the class
> > D to be a PGA or OUTPUT widget and the PLL to be a SUPPLY widget.

> I need a little help here.
> I am not seeing the PLL being disabled or Class D being disabled when
> I am using the DAPM calls

> My implementation is like this.  And I use set these widgets and audio
> map in the call back functions in the struct snd_soc_codec_driver.

> When I look at the registers after stream playback I see that
> the TAS2552_CFG_2 register is untouched

> static const struct snd_soc_dapm_widget tas2552_dapm_widgets[] =
> {
> SND_SOC_DAPM_AIF_IN("DAC IN", NULL, 0, SND_SOC_NOPM, 0, 0),
> SND_SOC_DAPM_DAC("DAC", NULL, SND_SOC_NOPM, 0, 0),
> SND_SOC_DAPM_OUT_DRV("ClassD Enable", TAS2552_CFG_2, 7, 0, NULL, 0),
> SND_SOC_DAPM_OUT_DRV("ClassD Disable", TAS2552_CFG_2, 7, 1, NULL, 0),
> SND_SOC_DAPM_SUPPLY("PLL Disable", TAS2552_CFG_2, 3, 1, NULL, 0),
> SND_SOC_DAPM_OUTPUT("OUT")
> };

> static const struct snd_soc_dapm_route tas2552_audio_map[] = {
>     {"DAC", NULL, "DAC IN"},
>     {"ClassD Enable", NULL, "DAC"},
>     {"OUT", NULL, "ClassD Enable"},
>     {"ClassD Disable", NULL, "OUT"},
>     {"PLL Disable", NULL, "ClassD Disable"}
> };

This looks very confused, why do you have separate enable and disable
things - what do you think the effect of the above should be?  That's
probably what's going wrong.  I'd expect to see one DAPM widget for the
class D and one for the PLL.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]