Re: [PATCH] ASoC: add RT5640 CODEC driver

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 1374 bytes --]

On Mon, Apr 22, 2013 at 03:03:05PM +0800, Bard wrote:

> Please see my reply below.

As you've been told several times now please fix your mailer to quote
the text you're replying to.  You shouldn't need to point out that
you're replying inline, this is how we do things...

> > +static const SOC_ENUM_SINGLE_DECL(
> > +	rt5640_in2_mode_enum, RT5640_IN3_IN4,
> > +	RT5640_IN_SFT2, rt5640_input_mode);

> Platform data...

> Bard: Can I export a function that machine can configure it by this function?

Why on earth would you want to do that?

> > +static int rt5640_vol_rescale_put(struct snd_kcontrol *kcontrol,
> > +		struct snd_ctl_elem_value *ucontrol) {
> > +	struct soc_mixer_control *mc =
> > +		(struct soc_mixer_control *)kcontrol->private_value;
> > +	struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
> > +	unsigned int val, val2;

> > +	val = VOL_RESCALE_MAX_VOL - ucontrol->value.integer.value[0];
> > +	val2 = VOL_RESCALE_MAX_VOL - ucontrol->value.integer.value[1];
> > +	return snd_soc_update_bits_locked(codec, mc->reg, RT5640_L_VOL_MASK |
> > +			RT5640_R_VOL_MASK, val << mc->shift | val2); }

> This looks like a variant on the _RANGE controls?

> Bard: Yes, we want to limit the max value of volume.
>      If it is not good, I will change it.

This shouldn't be open coded in the driver, the functions should be
generic.  Code wise it's fine.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]