linux-kernel - Re: [PATCH] ALSA: hda/conexant: Fix headset auto detect fail in cx8070 and SN6140

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <003501da4217$7cfdf4b0$76f9de10$@senarytech.com>
Date: Mon, 8 Jan 2024 17:46:00 +0800
From: 刘博 <bo.liu@...arytech.com>
To: "'Takashi Iwai'" <tiwai@...e.de>
Cc: <perex@...ex.cz>,
	<tiwai@...e.com>,
	<linux-sound@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ALSA: hda/conexant: Fix headset auto detect fail in cx8070 and SN6140

Hi Takashi,
	Thank you for your quick reply. I hope the following response can be
explained clearly, thanks.

> > > +	/* fix some headset type recognize fail issue, such as EDIFIER
> > headset */
> > > +	snd_hda_codec_write(codec, 0x1c, 0, 0x320, 0x010);
> > > +	snd_hda_codec_write(codec, 0x1c, 0, 0x3b0, 0xe10);
> > > +	snd_hda_codec_write(codec, 0x1c, 0, 0x4f0, 0x0eb);
> > 
> > Please use the defined verbs in sound/hda_verbs.h.
> > The arguments (0x320, 0x010) are (AC_VERB_SET_AMP_GAIN_MUTE, 0x2010)
etc.
> > 
> > Re: (0x1c, 0x320) is not amp gain register, but vendor defined 
> > register as rx control register. Use AC_VERB_SET_AMP_GAIN_MUTE will 
> > confused. It's similar to 0x4f0 and 0xca0.
>
> Ah interesting.  But the verb is actually seen as
AC_VERB_SET_AMP_GAIN_MUTE -- although the resultant bits seem invalid.
>
> HD-audio combines the verb and the value into 20 bits, e.g. (0x320,
> 0x10) is composed as 0x32010, and (0x3b0, 0xe10) is 0x3be10.
> 0x3xx is translated as SET_AMP_GAIN_MUTE, but in your case, 0x32010 leaves
0 to both the input/output bits (bits 14 and 15), which makes it as invalid.
>
> 0x3be10 is another invalid verb, which sets SET_AMP_GAIN_MUTE with OUTPUT,
but it sets both LEFT and RIGHT, and passes a high index (14).
>
> And, what actually (0x4f0, 0x0eb) does?  It's composed as 0x4f0eb, and in
this case, it's a valid verb (SET_PROC_COEF + 0xf0eb).  But COEF is
vendor-specific, so it can be translated in everything the chip wants.
>
> So, if those verbs are vendor-specific ones, please define them and/or
give proper comments to explain what they do for each.

Re: In cx8070 and sn6140, (0x1c, 0x320, 0x010) is used to set micbiasd
output current comparator 
threshold from 66% to 55%. (0x1c, 0x3b0, 0xe10) is used to set OFF voltage
for DFET from -1.2V to 
-0.8V, set headset micbias registor value adjustment trim from 2.2K ohms to
2.0K ohms.
(0x1c, 0x4f0, 0x0eb) is used to set headset detect debounce time, this will
impact detection time.
As it needs to be adjusted according to the project, i think it set to bios
may better. So I will remove
the setting from this patch.

> > Also, it's still not clear what if other nodes are used for headphone 
> > and mic pins -- or when either only headphone or only mic is present.
> > A rare case, but we need to cover.
> > 
> > Re: in cx8070 and sn6140, only 0x16 and 0x19 can be used together as 
> > headset. Other nodes can be used separately as headphones or 
> > microphones, but not as headset, so their configuration will not 
> > interfere with the type detection of headset.
>
> OK, then explain this in comments, too (that we blindly assume those
pins).

Re: OK, I will add the description to patch.

> > > +static void cx_process_headset_plugin(struct hda_codec *codec) {
> > > +	unsigned int val;
> > > +	unsigned int count = 0;
> > > +
> > > +	/* Wait headset detect done. */
> > > +	do {
> > > +		val = snd_hda_codec_read(codec, 0x1c, 0, 0xca0, 0x0);
> > 
> > Use the verb: AC_VERB_GET_PROC_COEF, 0xa000.
> > At best, define the COEF values 0xa000 and 0xb000, and the 
> > corresponding value bits, too.
> > 
> > Re: (0x1c, 0xca0) is not COEF register, but vendor defined register as 
> > jacksense register.
> > 
> > > +static void cx_update_headset_mic_vref(struct hda_codec *codec, 
> > > +unsigned int res) {
> > > +	unsigned int phone_present, mic_persent, phone_tag, mic_tag;
> > > +	struct conexant_spec *spec = codec->spec;
> > > +
> > > +	/* In cx8070 and sn6140, headset is fixed to use node 16 and node
> > 19.
> > 
> > Is it really guaranteed?  IMO, we should check the pin configs 
> > beforehand at the parsing time.
> > 
> > Re: in cx8070 and sn6140, only 0x16 and 0x19 can be used together as 
> > headset. The node 16 can only be config to headphone or disable, The 
> > node 19 can only be config to microphone or disable. Only node 16 and 
> > node 19 both enable, the patch will process.
>
> Then we still might need a check for the condition?

Re: because the node 16 and node 19 can only be config to headphone and
microphone, as describe 
before, whether node 16 and node 19 enable, the patch can process, so I
think it's no need to check
their pin configs.

Best Regards
Bo Liu