Re: [PATCH 5.3 112/112] ASoC: sgtl5000: add ADC mute control

[Oleksandr Suvorov]

On Thu, 2019-10-17 at 17:39 +0100, Mark Brown wrote:
> 
> All versions of driver sgtl5000 (since creating in 2011) has a bug in
> > sgtl5000_probe():
> > ...
> >        snd_soc_write(codec, SGTL5000_CHIP_ANA_CTRL,
> >                        SGTL5000_HP_ZCD_EN |
> >                        SGTL5000_ADC_ZCD_EN);
> > ...
> > This command rewrites the whole register value instead of just
> > enabling
> > ZCD feature for headphone and adc.
> > This register has bits for HP/LineOut/ADC muting, thus
> > sgtl5000_probe()
> > always unmutes HP/LineOut/ADC.
> 
> Yes, or at the very least this is a badly documented bit of
> intentional
> code.  I suspect it may be the latter but at this point we can't
> tell.
> > 2. keep 631bc8f0134ae and add 694b14554d75f to 4.19, 5.2 and 5.3.
> 
> This means the patch that makes ZC only update the ZC bits and also
> the
> patch that makes the mutes user controllable, the default being
> muted.
> As I pointed out up thread this would mean that someone upgrading to
> a
> newer stable may need to change their userspace to do the unmute
> instead
> of having things unconditionally unmuted by the driver.  This is not
> really what people expect from stable updates, we want them to be
> able
> to pull these in without thinking about it.  i

I think now I see, what you mean.

Of corse, we can change the original commit
[ 631bc8f0134ae ASoC: sgtl5000: Fix of unmute outputs on probe ]
for stable branches, unmuting ADC/HP/Lineout at the end of probing.
We keep an original behaviour for users and reduce the possible 'pop'
on probe.

But, to take significant advantage, we should add ADC control too, and,
as this is a feature, it is undesirable for stable branches.

So probably the best decision is to roll original 631bc8f0134ae back in
all stable branches where it was added.

Thank you, Mark. Your explanation is extremely useful.

> To backport the addition of the controls to stable we'd need an
> additional change which sets the default for this control to unmuted,
> there's a case for having such a change upstream regardless but it's
> still not clear if any of these changes are really fixes in the sense
> that we mean for stable.