Re: [PATCH] ASoC: add RT5677 CODEC driver

[Mark Brown <broonie@kernel.org>]

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

On Mon, May 26, 2014 at 08:32:33PM +0800, Oder Chiou wrote:
> This patch adds the Realtek ALC5677 codec driver.

This looks nice but some minor things need fixing.  Since they're so
minor especially in the context of such a large and generally good
driver I've gone ahead and applied this, please submit followup patches
for the issues below.

> +	/* ADC Boost Volume Control */
> +	SOC_DOUBLE_TLV("STO1 ADC Boost Gain", RT5677_STO1_2_ADC_BST,
> +		RT5677_STO1_ADC_L_BST_SFT, RT5677_STO1_ADC_R_BST_SFT, 3, 0,
> +		adc_bst_tlv),

Gain controls of all kinds should be called Volume so UIs do the right
thing.

> +static const struct snd_kcontrol_new rt5677_sto2_dmic_mux =
> +	SOC_DAPM_ENUM("Stereo2 DMIC source", rt5677_stereo2_dmic_enum);

The general style is to capitalise each word so "Stereo2 DMIC Source".

> +static int rt5677_set_micbias1_event(struct snd_soc_dapm_widget *w,
> +	struct snd_kcontrol *kcontrol, int event)
> +{
> +	struct snd_soc_codec *codec = w->codec;
> +	struct rt5677_priv *rt5677 = snd_soc_codec_get_drvdata(codec);
> +
> +	switch (event) {
> +	case SND_SOC_DAPM_POST_PMU:
> +		regmap_update_bits(rt5677->regmap, RT5677_PWR_ANLG2,
> +			RT5677_PWR_CLK_MB1 | RT5677_PWR_PP_MB1 |
> +			RT5677_PWR_CLK_MB, RT5677_PWR_CLK_MB1 |
> +			RT5677_PWR_PP_MB1 | RT5677_PWR_CLK_MB);

What does this do - I note we never undo this?

> +	/* Input Side */
> +	/* micbias */
> +	SND_SOC_DAPM_SUPPLY("micbias1", RT5677_PWR_ANLG2, RT5677_PWR_MB1_BIT,
> +		0, rt5677_set_micbias1_event, SND_SOC_DAPM_POST_PMU),

Idiomaticlaly this would be "MICBIAS1".

> +static int __init rt5677_modinit(void)
> +{
> +	return i2c_add_driver(&rt5677_i2c_driver);
> +}
> +module_init(rt5677_modinit);
> +
> +static void __exit rt5677_modexit(void)
> +{
> +	i2c_del_driver(&rt5677_i2c_driver);
> +}
> +module_exit(rt5677_modexit);

module_i2c_driver().

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

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