[alsa-devel] [PATCH] ASoC: rt286: fix headphone click/crack noise on Dell XPS 9343 I2S mode

[Kai-Heng Feng]

On Fri, Feb 17, 2017 at 3:22 AM Mark Brown <broonie at kernel.org> wrote:

> On Mon, Jan 23, 2017 at 01:35:14PM +0800, Kai-Heng Feng wrote:
>
> > +static void dell_dino_mute_hpo(struct snd_soc_codec *codec)
> > +{
> > +     snd_soc_write(codec, RT286_SET_AMP_GAIN_HPO, AMP_OUT_MUTE);
> > +}
> > +
>
> How does this ever get unmuted, should it be an _AUTODISABLE control
> instead?
>

I think the AMP is overrode by another value when playing sound, but I am
not sure.

Can there be multiple callbacks hooked to the same widget?
In this case, in addition to hpol_enable_control(), can I register another
callback to "HPO L"?


>
> > @@ -1204,6 +1261,7 @@ static int rt286_i2c_probe(struct i2c_client *i2c,
> >       mdelay(10);
> >
> >       regmap_write(rt286->regmap, RT286_MISC_CTRL1, 0x0000);
> > +
> >       /* Power down LDO, VREF */
> >       regmap_update_bits(rt286->regmap, RT286_POWER_CTRL2, 0xc, 0x0);
> >       regmap_update_bits(rt286->regmap, RT286_POWER_CTRL1, 0x1001,
> 0x1001);
>
> Random whitespace change.
>
> > @@ -1222,6 +1280,7 @@ static int rt286_i2c_probe(struct i2c_client *i2c,
> >                       RT286_SET_GPIO_DATA, 0x40, 0x40);
> >               regmap_update_bits(rt286->regmap,
> >                       RT286_GPIO_CTRL, 0xc, 0x8);
> > +             rt286->mute_hpo_func = dell_dino_mute_hpo;
> >       }
>
> Why would we only do this on some machines, does this break others?
>

Because there's already an existing dmi quirk "dmi_dell_dino", I think I
should just follow.
I don't know if this may break other machines or not, I only have XPS 9343
to test.


>
> This change does appear to be two different changes merged into one,
> there's the GPIO setup and this automute thing and they don't seem to
> overlap AFAICT - they should be split into separate patches unless
> there's some reason why they overlap but I'm not seeing that.
>

Understood. I'll split them in later version.