[alsa-devel] [PATCH 01/19] ASoC: upd9976: Add Renesas uPD9976 codec driver

[Lu Guanqun]

On Wed, May 04, 2011 at 10:34:51PM +0800, Mark Brown wrote:
> On Wed, May 04, 2011 at 09:44:58PM +0800, Lu Guanqun wrote:
> 
> Overall this looks very good, a few minor points below.
> 
> > +static inline unsigned int upd9976_read(struct snd_soc_codec *codec,
> > +					unsigned int reg)
> > +{
> 
> I guess these SCU write functions are going to be shared with other
> CODECs for this CPU - we should probably push this into the soc-cache
> for them.

Yes, currently it's the same read/write operation as in sn95031 codec.

> 
> > +static const struct snd_kcontrol_new upd9976_snd_controls[] = {
> > +	SOC_DOUBLE_R_TLV("Headphone & Speaker Volume",
> 
> Master Volume would be a better name.

OK. I was trying to be more clear on what this volume represents.

> 
> > +static const struct snd_kcontrol_new upd9976_hp_spkr_mixer_left_controls[] = {
> > +	SOC_DAPM_SINGLE("Audio DAC Left", UPD9976_HPLMIXSEL, 4, 1, 1),
> > +	SOC_DAPM_SINGLE("Audio DAC Right", UPD9976_HPLMIXSEL, 3, 1, 1),
> 
> These need Switch at the end of the name.

OK.

> 
> > +static struct snd_soc_dai_driver upd9976_dais[] = {
> > +{
> > +	.name = "upd9976-audio",
> > +	.playback = {
> 
> Just drop the audio from the name, it's a CODEC so it's obviously audio.

OK. Thanks for the review.

-- 
guanqun