Re: [PATCH] ASoC: Add MAX9850 codec driver

[Christian Glindkamp <christian.glindkamp@taskit.de>]

On 2011-03-08 10:14, Dimitris Papastamos wrote:
> On Mon, Mar 07, 2011 at 06:04:20PM +0100, Christian Glindkamp wrote:
> > > > +static const struct snd_soc_dapm_widget max9850_dapm_widgets[] = {
> > > > +SND_SOC_DAPM_DAC("DAC", "HiFi Playback", MAX9850_ENABLE, 0, 0),
> > > > +SND_SOC_DAPM_SUPPLY("MCLK", MAX9850_ENABLE, 6, 0, NULL, 0),
> > > > +SND_SOC_DAPM_OUTPUT("OUTL"),
> > > > +SND_SOC_DAPM_OUTPUT("OUTR"),
> > > > +SND_SOC_DAPM_OUTPUT("HPL"),
> > > > +SND_SOC_DAPM_OUTPUT("HPR"),
> > > > +SND_SOC_DAPM_INPUT("INL"),
> > > > +SND_SOC_DAPM_INPUT("INR"),
> > > > +SND_SOC_DAPM_PGA("Headphone Output", MAX9850_ENABLE, 3, 0, NULL, 0),
> > > > +SND_SOC_DAPM_MIXER("Line Input", SND_SOC_NOPM, 0, 0, NULL, 0),
> > > > +SND_SOC_DAPM_MIXER_NAMED_CTL("Output Mixer", MAX9850_ENABLE, 2, 0,
> > > > +		&max9850_mixer_controls[0],
> > > > +		ARRAY_SIZE(max9850_mixer_controls)),
> > > > +};
> > > 
> > > Consider grouping the input and output pins logically separately.
> > > 
> > 
> > Do you mean something like that?
> > 
> > SND_SOC_DAPM_SUPPLY("MCLK", MAX9850_ENABLE, 6, 0, NULL, 0),
> > SND_SOC_DAPM_MIXER_NAMED_CTL("Output Mixer", MAX9850_ENABLE, 2, 0,
> > 		&max9850_mixer_controls[0],
> > 		ARRAY_SIZE(max9850_mixer_controls)),
> > SND_SOC_DAPM_PGA("Headphone Output", MAX9850_ENABLE, 3, 0, NULL, 0),
> > SND_SOC_DAPM_DAC("DAC", "HiFi Playback", MAX9850_ENABLE, 0, 0),
> > SND_SOC_DAPM_OUTPUT("OUTL"),
> > SND_SOC_DAPM_OUTPUT("HPL"),
> > SND_SOC_DAPM_OUTPUT("OUTR"),
> > SND_SOC_DAPM_OUTPUT("HPR"),
> > SND_SOC_DAPM_MIXER("Line Input", SND_SOC_NOPM, 0, 0, NULL, 0),
> > SND_SOC_DAPM_INPUT("INL"),
> > SND_SOC_DAPM_INPUT("INR"),
> 
> Yea, that's fine.  Although the standard thing to do is to group all
> the input pins at the beginning and all the output pins at the end.
> However this is a pretty primitive codec so it is not a problem as it
> stands now.
>  
> > Charge pump could also be a supply for the Output Mixer. Have to find
> > out how to toggle two bits at once via dapm (preferably without
> > resorting to callbacks).
> 
> If they are in the same register and same type of widgets then as Mark
> mentioned the core guarantees that it'll be done in one register write.
> 
> I forgot to mention in the previous e-mail, that if you add the cache
> syncing support during resume or whenever we go from BIAS_OFF to
> STANDBY use the standard ASoC function for that, snd_soc_cache_sync().
> Just in case you are looking at older examples.
> 

Do I understand it correctly, that all register writes that I do in the
probe function would be overridden by the syncing so they would have to
be moved to the standby code or put into the register cache array as
default values?

> Thanks,
> Dimitris
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel