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

[Oleksandr Suvorov]

> On 17/10/2019 01:23, Greg Kroah-Hartman wrote:
> > On Wed, Oct 16, 2019 at 11:35:18PM +0100, Mark Brown wrote:
> >> On Wed, Oct 16, 2019 at 03:10:25PM -0700, Greg Kroah-Hartman wrote:
> >>> On Wed, Oct 16, 2019 at 11:00:44PM +0100, Mark Brown wrote:
> >>>> On Wed, Oct 16, 2019 at 02:51:44PM -0700, Greg Kroah-Hartman wrote:
> >>>>> From: Oleksandr Suvorov <oleksandr.suvorov@xxxxxxxxxxx>
> >>
> >>>>> commit 694b14554d75f2a1ae111202e71860d58b434a21 upstream.
> >>
> >>>>> This control mute/unmute the ADC input of SGTL5000
> >>>>> using its CHIP_ANA_CTRL register.
> >>
> >>>> This seems like a new feature and not an obvious candidate for stable?
> >>
> >>> there was a long email from Richard that said:
> >>>     Upstream commit 631bc8f0134a ("ASoC: sgtl5000: Fix of unmute
> >>>     outputs on probe"), which is e9f621efaebd in v5.3 replaced
> >>>     snd_soc_component_write with snd_soc_component_update_bits and
> >>>     therefore no longer cleared the MUTE_ADC flag. This caused the
> >>>     ADC to stay muted and recording doesn't work any longer. This
> >>>     patch fixes this problem by adding a Switch control for
> >>>     MUTE_ADC.
> >>
> >>> That's why I took this.  If this isn't true, I'll be glad to drop this.
> >>
> >> That's probably not an appropriate fix for stable - it's going to add a
> >> new control which users will need to manually set (or hope their
> >> userspace automatically figures out that it should set for them, more
> >> advanced userspaces like PulseAudio should) which isn't a drop in fix.
> >> You could either drop the backport that was done for zero cross or take
> >> a new patch that clears the MUTE_ADC flag (rather than punting to
> >> userspace to do so), or just be OK with what you've got at the minute
> >> which might be fine given the lack of user reports.

Mark, obviously this is not a NEW feature. This patch adds LOST
standard control.

Please look into other codecs:

$ grep -R 'SOC_SINGLE("Capture Switch"' sound/soc/
sound/soc/codecs/sgtl5000.c.orig: SOC_SINGLE("Capture Switch",
SGTL5000_CHIP_ANA_CTRL, 0, 1, 1),
sound/soc/codecs/wm9705.c: SOC_SINGLE("Capture Switch", AC97_REC_GAIN,
15, 1, 1),
sound/soc/codecs/wm9713.c:SOC_SINGLE("Capture Switch", AC97_CD, 15, 1, 1),
sound/soc/codecs/wm9712.c:SOC_SINGLE("Capture Switch", AC97_REC_GAIN, 15, 1, 1),

And even:

$ grep -R 'SOC_SINGLE(".*Capture Switch"' sound/soc/
sound/soc/codecs/lm49453.c: SOC_SINGLE("Port1 Capture Switch",
LM49453_P0_AUDIO_PORT1_BASIC_REG,
sound/soc/codecs/lm49453.c: SOC_SINGLE("Port2 Capture Switch",
LM49453_P0_AUDIO_PORT2_BASIC_REG,
sound/soc/codecs/sgtl5000.c.orig: SOC_SINGLE("Capture Switch",
SGTL5000_CHIP_ANA_CTRL, 0, 1, 1),
sound/soc/codecs/mc13783.c: SOC_SINGLE("Line in Capture Switch",
MC13783_AUDIO_RX1, 10, 1, 0),
sound/soc/codecs/ak4642.c: SOC_SINGLE("ALC Capture Switch", ALC_CTL1, 5, 1, 0),
sound/soc/codecs/adau1701.c: SOC_SINGLE("Master Capture Switch",
ADAU1701_DSPCTRL, 4, 1, 0),
sound/soc/codecs/alc5632.c: SOC_SINGLE("DMIC En Capture Switch",
sound/soc/codecs/alc5632.c: SOC_SINGLE("DMIC PreFilter Capture Switch",
sound/soc/codecs/wm9705.c: SOC_SINGLE("Capture Switch", AC97_REC_GAIN,
15, 1, 1),
sound/soc/codecs/adau1977.c: SOC_SINGLE("ADC" #x " Highpass-Filter
Capture Switch", \
sound/soc/codecs/adau1977.c: SOC_SINGLE("ADC" #x " DC Subtraction
Capture Switch", \
sound/soc/codecs/isabelle.c: SOC_SINGLE("ULATX12 Capture Switch",
ISABELLE_ULATX12_INTF_CFG_REG,
sound/soc/codecs/ad1980.c:SOC_SINGLE("PCM Capture Switch",
AC97_REC_GAIN, 15, 1, 1),
sound/soc/codecs/ad1980.c:SOC_SINGLE("Phone Capture Switch",
AC97_PHONE, 15, 1, 1),
sound/soc/codecs/wm8731.c:SOC_SINGLE("Mic Capture Switch",
WM8731_APANA, 1, 1, 1),
sound/soc/codecs/uda1380.c:/**/ SOC_SINGLE("ADC Capture Switch",
UDA1380_PGA, 15, 1, 1), /* MT_ADC */
sound/soc/codecs/wm9713.c:SOC_SINGLE("Capture Switch", AC97_CD, 15, 1, 1),
sound/soc/codecs/es8316.c: SOC_SINGLE("ALC Capture Switch",
ES8316_ADC_ALC1, 6, 1, 0),
sound/soc/codecs/sgtl5000.c: SOC_SINGLE("Capture Switch",
SGTL5000_CHIP_ANA_CTRL, 0, 1, 1),
sound/soc/codecs/tlv320aic31xx.c: SOC_SINGLE("ADC Capture Switch",
AIC31XX_ADCFGA, 7, 1, 1),
sound/soc/codecs/da7210.c: SOC_SINGLE("Aux2 Capture Switch",
DA7210_AUX2, 2, 1, 0),
sound/soc/codecs/wm9712.c:SOC_SINGLE("Capture Switch", AC97_REC_GAIN, 15, 1, 1),

git blame sound/soc/codecs/wm9705.c
...
927b0aea93bb3 (Ian Molton         2009-01-19 17:23:11 +0000  87)
 SOC_SINGLE("Capture Switch", AC97_REC_GAIN, 15, 1, 1),
...

This control was added not later than 2009, so I doubt my patch could
break something in current user-land.

> >
> > Ok, I'll gladly go drop it, thanks!
>
> Mark, thanks for the clarification! I haven't thought of breaking
> anything with the backport as it worked fine for our application.

-- 
Best regards
Oleksandr Suvorov

Toradex AG
Altsagenstrasse 5 | 6048 Horw/Luzern | Switzerland | T: +41 41 500
4800 (main line)