[PATCH v2] ALSA: hda/conexant: Fix swapped speaker/headphone outputs on Huawei MateBook 16s

[PATCH v2] ALSA: hda/conexant: Fix swapped speaker/headphone outputs on Huawei MateBook 16s

[Xie Youtian]

On the Huawei MateBook 16s (2023) with a Conexant SN6140 codec (codec
SSID 0x19e5:0x3291), the speaker amplifier and the headphone jack are
both fed from the analog output line of pin 0x16: the jack through a
tap that needs GPIO1 enabled and driven low, and the speaker amp
switched by the EAPD of pin 0x17 - none of which matches the BIOS pin
config.  The generic parser therefore routes audio to the wrong
physical output and never powers the jack:

 * headphones plugged: the headphone path (DAC 0x10 -> pin 0x16)
   drives the speakers while the jack stays silent
 * headphones unplugged: the speaker path (DAC 0x11 -> pin 0x17) feeds
   no transducer at all and the machine stays silent

Route the shared line to whichever DAC the parser leaves unmuted for
the current jack state, keep the jack tap open, and gate the speaker
amp through EAPD.  The verbs are applied from a private jack callback
(the generic unsol->automute path never fires on this machine while a
stream keeps the codec awake) with a debounced presence sample, and a
delayed recheck re-applies everything from a settled sample, since any
reader of the pin sense can see stale data for a short while after a
codec power transition.  The 600 ms recheck delay matches the factory
Windows driver's plug-detection restart window.

The factory Windows driver groups codec SSIDs 0x19e5:0x3291 through
0x3294 into a single audio configuration, so the siblings very likely
share this wiring; only 0x3291 could be tested.

Tested on the machine: headphone/speaker switching in both directions,
during playback, over the bare PCM path and through PipeWire, and
across suspend/resume.

Problem detection and the verb-level hardware experiments were
performed with the help of an AI coding assistant, with all audible
verification done by ear on the actual hardware by the submitter; the
fix was drafted by the assistant and then reviewed, understood,
adjusted and tested by the submitter.

Link: https://github.com/geniusinsanity/huawei-matebook16s-speaker-fix
Signed-off-by: Xie Youtian <xiedada@xiedada.net>

---
Changes in v2 (addressing review comments):

 - Read the pin sense with a raw AC_VERB_GET_PIN_SENSE instead of
   snd_hda_jack_detect(), whose jacktbl cache made both the debounce
   comparison and the 600 ms recheck read stale data.
 - Queue the recheck on system_percpu_wq consistently; system_wq is
   deprecated and mixing it with schedule_delayed_work() broke
   non-reentrancy for the same work item.
 - Re-assert the pin 0x17 connection select and pin control from the
   coerce function instead of the once-only PRE_PROBE writes that the
   generic parser overwrites.
 - Cancel the recheck from cx_suspend() and cx_remove() (before the
   spec is freed) and guard the automute hook and jack callback against
   running after teardown.
---
 sound/hda/codecs/conexant.c | 174 ++++++++++++++++++++++++++++++++++++
 1 file changed, 174 insertions(+)

diff --git a/sound/hda/codecs/conexant.c b/sound/hda/codecs/conexant.c
index 9bfdbf5c9032..7926fdd71131 100644
--- a/sound/hda/codecs/conexant.c
+++ b/sound/hda/codecs/conexant.c
@@ -43,6 +43,10 @@ struct conexant_spec {
 	unsigned int gpio_mute_led_mask;
 	unsigned int gpio_mic_led_mask;
 	bool is_cx11880_sn6140;
+
+	/* quirk for Huawei MateBooks with Raptor Lake */
+	struct hda_codec *rpl_recheck_codec;
+	struct delayed_work rpl_recheck_work;
 };
 
 
@@ -203,6 +207,11 @@ static void cx_auto_shutdown(struct hda_codec *codec)
 
 static void cx_remove(struct hda_codec *codec)
 {
+	struct conexant_spec *spec = codec->spec;
+
+	/* stop the quirk recheck before the spec is freed below */
+	spec->rpl_recheck_codec = NULL;
+	cancel_delayed_work_sync(&spec->rpl_recheck_work);
 	cx_auto_shutdown(codec);
 	snd_hda_gen_remove(codec);
 }
@@ -270,6 +279,9 @@ static void cx_set_power_state(struct hda_codec *codec, hda_nid_t fg,
 
 static int cx_suspend(struct hda_codec *codec)
 {
+	struct conexant_spec *spec = codec->spec;
+
+	cancel_delayed_work_sync(&spec->rpl_recheck_work);
 	cx_auto_shutdown(codec);
 	return 0;
 }
@@ -312,6 +324,7 @@ enum {
 	CXT_FIXUP_HP_A_U,
 	CXT_FIXUP_ACER_SWIFT_HP,
 	CXT_FIXUP_HUAWEI_MATEBOOK_HP,
+	CXT_FIXUP_HUAWEI_MATEBOOK_RPL,
 };
 
 /* for hda_fixup_thinkpad_acpi() */
@@ -863,6 +876,159 @@ static const struct hda_pintbl cxt_pincfg_sws_js201d[] = {
 	{}
 };
 
+/*
+ * Huawei MateBooks with Raptor Lake and SN6140 (e.g. the MateBook 16s
+ * 2023): the speaker amp and the headphone jack are both fed from the
+ * pin 0x16 line.  The speaker amp is gated by the EAPD of pin 0x17, the
+ * jack tap by GPIO1 - none of which matches the BIOS pin config.  Point
+ * pin 0x16 at the DAC the parser leaves unmuted for the current jack
+ * state, keep the tap open, and gate the speaker amp via EAPD.
+ */
+static void cxt_matebook_raptor_lake_coerce(struct hda_codec *codec, bool hp)
+{
+	snd_hda_codec_write_cache(codec, 0x16, 0,
+				  AC_VERB_SET_CONNECT_SEL, hp ? 0x0 : 0x1);
+	/* the tap path needs pin 0x17 selected and driving, whatever the
+	 * parser does to what it thinks is a speaker pin
+	 */
+	snd_hda_codec_write_cache(codec, 0x17, 0,
+				  AC_VERB_SET_CONNECT_SEL, 0x0);
+	snd_hda_codec_write_cache(codec, 0x17, 0,
+				  AC_VERB_SET_PIN_WIDGET_CONTROL, 0x40);
+	snd_hda_codec_write_cache(codec, 0x17, 0,
+				  AC_VERB_SET_EAPD_BTLENABLE, hp ? 0x0 : 0x2);
+	/* keep the jack tap on the shared line open; with the speaker amp
+	 * gated separately this is correct in both jack states
+	 */
+	snd_hda_codec_write_cache(codec, 0x01, 0,
+				  AC_VERB_SET_GPIO_MASK, 0x02);
+	snd_hda_codec_write_cache(codec, 0x01, 0,
+				  AC_VERB_SET_GPIO_DIRECTION, 0x02);
+	snd_hda_codec_write_cache(codec, 0x01, 0,
+				  AC_VERB_SET_GPIO_DATA, 0x00);
+}
+
+/* read the pin sense directly: snd_hda_jack_detect() returns the
+ * cached state, which is exactly what a bounce-healing re-read must
+ * not trust
+ */
+static bool cxt_matebook_raptor_lake_sense(struct hda_codec *codec)
+{
+	return !!(snd_hda_codec_read(codec, 0x16, 0,
+				     AC_VERB_GET_PIN_SENSE, 0) &
+		  AC_PINSENSE_PRESENCE);
+}
+
+/* jack presence right at a plug event bounces for a few milliseconds;
+ * sample it once, debounced, and use that one value everywhere below
+ */
+static bool cxt_matebook_raptor_lake_jack_present(struct hda_codec *codec)
+{
+	bool a = cxt_matebook_raptor_lake_sense(codec);
+
+	if (cxt_matebook_raptor_lake_sense(codec) != a) {
+		msleep(100);
+		return cxt_matebook_raptor_lake_sense(codec);
+	}
+	return a;
+}
+
+static void cxt_matebook_raptor_lake_apply(struct hda_codec *codec, bool hp)
+{
+	struct conexant_spec *spec = codec->spec;
+
+	spec->gen.hp_jack_present = hp;
+	snd_hda_gen_update_outputs(codec);
+	cxt_matebook_raptor_lake_coerce(codec, hp);
+}
+
+/*
+ * Right after the codec powers back up every reader of the pin sense -
+ * ours and the parser's alike - can see garbage.  Re-apply everything
+ * from a settled read a bit later, which heals any misguess made at
+ * init or during a jack event.
+ */
+static void cxt_matebook_raptor_lake_recheck(struct work_struct *work)
+{
+	struct conexant_spec *spec = container_of(work, struct conexant_spec,
+						  rpl_recheck_work.work);
+	struct hda_codec *codec = spec->rpl_recheck_codec;
+
+	if (codec) {
+		bool hp = cxt_matebook_raptor_lake_sense(codec);
+
+		codec_dbg(codec, "matebook-rpl: recheck hp=%d\n", hp);
+		cxt_matebook_raptor_lake_apply(codec, hp);
+	}
+}
+
+/* the parser reprograms pin 0x17 and the DAC amps on jack events; the
+ * recheck heals a stale presence sample taken while the contact still
+ * bounces
+ */
+static void cxt_matebook_raptor_lake_update_outputs(struct hda_codec *codec)
+{
+	struct conexant_spec *spec = codec->spec;
+	bool hp;
+
+	if (!spec->rpl_recheck_codec)
+		return;
+	hp = cxt_matebook_raptor_lake_jack_present(codec);
+	codec_dbg(codec, "matebook-rpl: hook hp=%d\n", hp);
+	cxt_matebook_raptor_lake_apply(codec, hp);
+	mod_delayed_work(system_percpu_wq, &spec->rpl_recheck_work,
+			  msecs_to_jiffies(600));
+}
+
+/* our own jack callback: the unsol -> automute_hook chain is
+ * unreliable on this machine (stays silent while a stream keeps the
+ * codec awake); apply immediately and keep the delayed recheck as the
+ * safety net for anything the debounced sample got wrong
+ */
+static void cxt_matebook_raptor_lake_jack_event(struct hda_codec *codec,
+					       struct hda_jack_callback *jack)
+{
+	struct conexant_spec *spec = codec->spec;
+	bool hp;
+
+	if (!spec->rpl_recheck_codec)
+		return;
+	hp = cxt_matebook_raptor_lake_jack_present(codec);
+	codec_dbg(codec, "matebook-rpl: jack event\n");
+	cxt_matebook_raptor_lake_apply(codec, hp);
+	mod_delayed_work(system_percpu_wq, &spec->rpl_recheck_work,
+			  msecs_to_jiffies(600));
+}
+
+static void cxt_fixup_matebook_raptor_lake(struct hda_codec *codec,
+				  const struct hda_fixup *fix, int action)
+{
+	struct conexant_spec *spec = codec->spec;
+
+	switch (action) {
+	case HDA_FIXUP_ACT_PRE_PROBE:
+		spec->gen.automute_hook = cxt_matebook_raptor_lake_update_outputs;
+		INIT_DELAYED_WORK(&spec->rpl_recheck_work,
+				  cxt_matebook_raptor_lake_recheck);
+		spec->rpl_recheck_codec = codec;
+		break;
+	case HDA_FIXUP_ACT_INIT:
+		/* the parser's own view is the best guess at init; the
+		 * delayed recheck above heals whatever it gets wrong
+		 */
+		snd_hda_jack_detect_enable_callback(codec, 0x16,
+						    cxt_matebook_raptor_lake_jack_event);
+		cxt_matebook_raptor_lake_apply(codec, spec->gen.hp_jack_present);
+		mod_delayed_work(system_percpu_wq, &spec->rpl_recheck_work,
+				 msecs_to_jiffies(600));
+		break;
+	case HDA_FIXUP_ACT_FREE:
+		cancel_delayed_work_sync(&spec->rpl_recheck_work);
+		spec->rpl_recheck_codec = NULL;
+		break;
+	}
+}
+
 static const struct hda_fixup cxt_fixups[] = {
 	[CXT_PINCFG_LENOVO_X200] = {
 		.type = HDA_FIXUP_PINS,
@@ -1062,6 +1228,10 @@ static const struct hda_fixup cxt_fixups[] = {
 			{ }
 		},
 	},
+	[CXT_FIXUP_HUAWEI_MATEBOOK_RPL] = {
+		.type = HDA_FIXUP_FUNC,
+		.v.func = cxt_fixup_matebook_raptor_lake,
+	},
 };
 
 static const struct hda_quirk cxt5045_fixups[] = {
@@ -1164,6 +1334,10 @@ static const struct hda_quirk cxt5066_fixups[] = {
 	SND_PCI_QUIRK(0x17aa, 0x397b, "Lenovo S205", CXT_FIXUP_STEREO_DMIC),
 	SND_PCI_QUIRK_VENDOR(0x17aa, "Thinkpad/Ideapad", CXT_FIXUP_LENOVO_XPAD_ACPI),
 	SND_PCI_QUIRK(0x19e5, 0x3289, "Huawei Matebook", CXT_FIXUP_HUAWEI_MATEBOOK_HP),
+	SND_PCI_QUIRK(0x19e5, 0x3291, "Huawei MateBook 16s (RPL)", CXT_FIXUP_HUAWEI_MATEBOOK_RPL),
+	SND_PCI_QUIRK(0x19e5, 0x3292, "Huawei MateBook 16s (RPL)", CXT_FIXUP_HUAWEI_MATEBOOK_RPL),
+	SND_PCI_QUIRK(0x19e5, 0x3293, "Huawei MateBook 16s (RPL)", CXT_FIXUP_HUAWEI_MATEBOOK_RPL),
+	SND_PCI_QUIRK(0x19e5, 0x3294, "Huawei MateBook 16s (RPL)", CXT_FIXUP_HUAWEI_MATEBOOK_RPL),
 	SND_PCI_QUIRK(0x1c06, 0x2011, "Lemote A1004", CXT_PINCFG_LEMOTE_A1004),
 	SND_PCI_QUIRK(0x1c06, 0x2012, "Lemote A1205", CXT_PINCFG_LEMOTE_A1205),
 	SND_PCI_QUIRK(0x1d05, 0x3012, "MECHREVO Wujie 15X Pro", CXT_FIXUP_HEADSET_MIC),
-- 
2.56.0

Re: [PATCH v2] ALSA: hda/conexant: Fix swapped speaker/headphone outputs on Huawei MateBook 16s

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] ALSA: hda/conexant: teardown and suspend race in MateBook RPL jack callbacks
- [High] ALSA: hda/conexant: uninitialized work cancel in cx_suspend() and cx_remove()
- [High] ALSA: hda/conexant: deadlock in cx_suspend() cancelling rpl_recheck_work
- [Medium] ALSA: hda/conexant: stale jack state in cxt_matebook_raptor_lake_recheck()
--

commit 50725697e44e0b95fb93bf300a4dfcc83b2c212c
Author: Xie Youtian <xiedada@xiedada.net>

ALSA: hda/conexant: Fix swapped speaker/headphone outputs on Huawei MateBook 16s

This commit adds a fixup for Huawei MateBook 16s (Raptor Lake) models with a
Conexant SN6140 codec to route the shared pin 0x16 output line to the active
DAC, enable the headphone jack tap via GPIO1, and gate the speaker amplifier
via pin 0x17 EAPD using a debounced jack callback and a 600 ms delayed
recheck work item.

Link: https://github.com/geniusinsanity/huawei-matebook16s-speaker-fix

> diff --git a/sound/hda/codecs/conexant.c b/sound/hda/codecs/conexant.c
> index 9bfdbf5c90325..7926fdd711310 100644
> --- a/sound/hda/codecs/conexant.c
> +++ b/sound/hda/codecs/conexant.c
> @@ -43,6 +43,10 @@ struct conexant_spec {
>  	unsigned int gpio_mute_led_mask;
>  	unsigned int gpio_mic_led_mask;
>  	bool is_cx11880_sn6140;
> +
> +	/* quirk for Huawei MateBooks with Raptor Lake */
> +	struct hda_codec *rpl_recheck_codec;
> +	struct delayed_work rpl_recheck_work;
>  };
>  
>  
> @@ -203,6 +207,11 @@ static void cx_auto_shutdown(struct hda_codec *codec)
>  
>  static void cx_remove(struct hda_codec *codec)
>  {
> +	struct conexant_spec *spec = codec->spec;
> +
> +	/* stop the quirk recheck before the spec is freed below */
> +	spec->rpl_recheck_codec = NULL;
> +	cancel_delayed_work_sync(&spec->rpl_recheck_work);
>  	cx_auto_shutdown(codec);
>  	snd_hda_gen_remove(codec);
>  }

[Severity: High]
Will calling cancel_delayed_work_sync(&spec->rpl_recheck_work) unconditionally
in cx_remove() and cx_suspend() trigger a warning on all other Conexant
codecs?

In cx_probe(), struct conexant_spec is allocated with kzalloc_obj(*spec),
leaving spec->rpl_recheck_work zero-initialized. INIT_DELAYED_WORK() is only
called inside cxt_fixup_matebook_raptor_lake() under HDA_FIXUP_ACT_PRE_PROBE:

sound/hda/codecs/conexant.c:cxt_fixup_matebook_raptor_lake() {
	switch (action) {
	case HDA_FIXUP_ACT_PRE_PROBE:
		spec->gen.automute_hook = cxt_matebook_raptor_lake_update_outputs;
		INIT_DELAYED_WORK(&spec->rpl_recheck_work,
				  cxt_matebook_raptor_lake_recheck);
		spec->rpl_recheck_codec = codec;
		break;

When any Conexant device not matching CXT_FIXUP_HUAWEI_MATEBOOK_RPL suspends
via cx_suspend() or unbinds via cx_remove() (or if cx_probe() fails before
HDA_FIXUP_ACT_PRE_PROBE and jumps to error: -> cx_remove()),
cancel_delayed_work_sync(&spec->rpl_recheck_work) is called on a zeroed
delayed_work, hitting WARN_ON(!work->func) in __flush_work() and timer
debug-object warnings in __timer_delete().

> @@ -270,6 +279,9 @@ static void cx_set_power_state(struct hda_codec *codec, hda_nid_t fg,
>  
>  static int cx_suspend(struct hda_codec *codec)
>  {
> +	struct conexant_spec *spec = codec->spec;
> +
> +	cancel_delayed_work_sync(&spec->rpl_recheck_work);
>  	cx_auto_shutdown(codec);
>  	return 0;
>  }

[Severity: High]
Can cancelling spec->rpl_recheck_work synchronously inside cx_suspend()
deadlock against cxt_matebook_raptor_lake_recheck()?

When the codec enters runtime suspend, rpm_suspend() sets
dev->power.runtime_status = RPM_SUSPENDING and calls
hda_codec_runtime_suspend() -> hda_call_codec_suspend():

sound/hda/common/codec.c:hda_call_codec_suspend() {
	...
	snd_hdac_enter_pm(&codec->core);
	if (driver->ops->suspend)
		driver->ops->suspend(codec);

If cxt_matebook_raptor_lake_recheck() starts running on system_percpu_wq and
executes an HDA verb via cxt_matebook_raptor_lake_sense() ->
snd_hda_codec_read() -> snd_hdac_power_up_pm(&codec->core) before
snd_hdac_enter_pm() increments codec->core.in_pm, atomic_inc_not_zero()
returns false and snd_hdac_power_up_pm() calls snd_hdac_power_up() ->
pm_runtime_get_sync() -> rpm_resume().

In rpm_resume(), because dev->power.runtime_status is RPM_SUSPENDING, the
workqueue thread sleeps uninterruptibly on dev->power.wait_queue waiting for
rpm_suspend() to finish, while the suspending thread blocks in cx_suspend() ->
cancel_delayed_work_sync(&spec->rpl_recheck_work) waiting for
cxt_matebook_raptor_lake_recheck() to finish.

In addition, because cx_probe() sets codec->bus->allow_bus_reset = 1, a verb
timeout inside cxt_matebook_raptor_lake_recheck() can trigger
codec_exec_verb() -> snd_hda_bus_reset() -> snd_hda_bus_reset_codecs() ->
hda_call_codec_suspend() -> cx_suspend() ->
cancel_delayed_work_sync(&spec->rpl_recheck_work) from within the work
callback itself.

[ ... ]

> @@ -863,6 +876,159 @@ static const struct hda_pintbl cxt_pincfg_sws_js201d[] = {
>  	{}
>  };
>  
> +/*
> + * Huawei MateBooks with Raptor Lake and SN6140 (e.g. the MateBook 16s
> + * 2023): the speaker amp and the headphone jack are both fed from the
> + * pin 0x16 line.  The speaker amp is gated by the EAPD of pin 0x17, the
> + * jack tap by GPIO1 - none of which matches the BIOS pin config.  Point
> + * pin 0x16 at the DAC the parser leaves unmuted for the current jack
> + * state, keep the tap open, and gate the speaker amp via EAPD.
> + */
> +static void cxt_matebook_raptor_lake_coerce(struct hda_codec *codec, bool hp)
> +{
> +	snd_hda_codec_write_cache(codec, 0x16, 0,
> +				  AC_VERB_SET_CONNECT_SEL, hp ? 0x0 : 0x1);
> +	/* the tap path needs pin 0x17 selected and driving, whatever the
> +	 * parser does to what it thinks is a speaker pin
> +	 */
> +	snd_hda_codec_write_cache(codec, 0x17, 0,
> +				  AC_VERB_SET_CONNECT_SEL, 0x0);
> +	snd_hda_codec_write_cache(codec, 0x17, 0,
> +				  AC_VERB_SET_PIN_WIDGET_CONTROL, 0x40);
> +	snd_hda_codec_write_cache(codec, 0x17, 0,
> +				  AC_VERB_SET_EAPD_BTLENABLE, hp ? 0x0 : 0x2);
> +	/* keep the jack tap on the shared line open; with the speaker amp
> +	 * gated separately this is correct in both jack states
> +	 */
> +	snd_hda_codec_write_cache(codec, 0x01, 0,
> +				  AC_VERB_SET_GPIO_MASK, 0x02);
> +	snd_hda_codec_write_cache(codec, 0x01, 0,
> +				  AC_VERB_SET_GPIO_DIRECTION, 0x02);
> +	snd_hda_codec_write_cache(codec, 0x01, 0,
> +				  AC_VERB_SET_GPIO_DATA, 0x00);
> +}
> +
> +/* read the pin sense directly: snd_hda_jack_detect() returns the
> + * cached state, which is exactly what a bounce-healing re-read must
> + * not trust
> + */
> +static bool cxt_matebook_raptor_lake_sense(struct hda_codec *codec)
> +{
> +	return !!(snd_hda_codec_read(codec, 0x16, 0,
> +				     AC_VERB_GET_PIN_SENSE, 0) &
> +		  AC_PINSENSE_PRESENCE);
> +}
> +
> +/* jack presence right at a plug event bounces for a few milliseconds;
> + * sample it once, debounced, and use that one value everywhere below
> + */
> +static bool cxt_matebook_raptor_lake_jack_present(struct hda_codec *codec)
> +{
> +	bool a = cxt_matebook_raptor_lake_sense(codec);
> +
> +	if (cxt_matebook_raptor_lake_sense(codec) != a) {
> +		msleep(100);
> +		return cxt_matebook_raptor_lake_sense(codec);
> +	}
> +	return a;
> +}
> +
> +static void cxt_matebook_raptor_lake_apply(struct hda_codec *codec, bool hp)
> +{
> +	struct conexant_spec *spec = codec->spec;
> +
> +	spec->gen.hp_jack_present = hp;
> +	snd_hda_gen_update_outputs(codec);
> +	cxt_matebook_raptor_lake_coerce(codec, hp);
> +}
> +
> +/*
> + * Right after the codec powers back up every reader of the pin sense -
> + * ours and the parser's alike - can see garbage.  Re-apply everything
> + * from a settled read a bit later, which heals any misguess made at
> + * init or during a jack event.
> + */
> +static void cxt_matebook_raptor_lake_recheck(struct work_struct *work)
> +{
> +	struct conexant_spec *spec = container_of(work, struct conexant_spec,
> +						  rpl_recheck_work.work);
> +	struct hda_codec *codec = spec->rpl_recheck_codec;
> +
> +	if (codec) {
> +		bool hp = cxt_matebook_raptor_lake_sense(codec);
> +
> +		codec_dbg(codec, "matebook-rpl: recheck hp=%d\n", hp);
> +		cxt_matebook_raptor_lake_apply(codec, hp);
> +	}
> +}

[Severity: Medium]
When cxt_matebook_raptor_lake_recheck() heals a stale initial pin-sense read
after 600 ms, does codec->jacktbl and the userspace jack state remain stuck at
the old value?

During codec resume in hda_call_codec_resume() or an unsolicited event in
snd_hda_jack_unsol_event(), snd_hda_jack_report_sync(codec) runs immediately,
caching the initial (potentially stale or bouncing) pin 0x16 sense in
jack->pin_sense, clearing jack->jack_dirty, and reporting that state to
userspace.

When cxt_matebook_raptor_lake_recheck() runs 600 ms later, it reads the
settled state via cxt_matebook_raptor_lake_sense() and updates hardware
routing via cxt_matebook_raptor_lake_apply(codec, hp), but never marks the
jacktbl entry dirty or calls snd_hda_jack_report_sync(codec). If the initial
read was wrong, won't userspace audio servers (such as PipeWire or PulseAudio)
still see the stale Headphone Jack control state and keep the active DAC path
muted?

> +
> +/* the parser reprograms pin 0x17 and the DAC amps on jack events; the
> + * recheck heals a stale presence sample taken while the contact still
> + * bounces
> + */
> +static void cxt_matebook_raptor_lake_update_outputs(struct hda_codec *codec)
> +{
> +	struct conexant_spec *spec = codec->spec;
> +	bool hp;
> +
> +	if (!spec->rpl_recheck_codec)
> +		return;
> +	hp = cxt_matebook_raptor_lake_jack_present(codec);
> +	codec_dbg(codec, "matebook-rpl: hook hp=%d\n", hp);
> +	cxt_matebook_raptor_lake_apply(codec, hp);
> +	mod_delayed_work(system_percpu_wq, &spec->rpl_recheck_work,
> +			  msecs_to_jiffies(600));
> +}
> +
> +/* our own jack callback: the unsol -> automute_hook chain is
> + * unreliable on this machine (stays silent while a stream keeps the
> + * codec awake); apply immediately and keep the delayed recheck as the
> + * safety net for anything the debounced sample got wrong
> + */
> +static void cxt_matebook_raptor_lake_jack_event(struct hda_codec *codec,
> +					       struct hda_jack_callback *jack)
> +{
> +	struct conexant_spec *spec = codec->spec;
> +	bool hp;
> +
> +	if (!spec->rpl_recheck_codec)
> +		return;
> +	hp = cxt_matebook_raptor_lake_jack_present(codec);
> +	codec_dbg(codec, "matebook-rpl: jack event\n");
> +	cxt_matebook_raptor_lake_apply(codec, hp);
> +	mod_delayed_work(system_percpu_wq, &spec->rpl_recheck_work,
> +			  msecs_to_jiffies(600));
> +}

[Severity: Critical]
What prevents cxt_matebook_raptor_lake_update_outputs() or
cxt_matebook_raptor_lake_jack_event() from racing with cx_remove() or
cx_suspend() while sleeping in cxt_matebook_raptor_lake_jack_present()?

Both callbacks check spec->rpl_recheck_codec without holding a lock and then
call cxt_matebook_raptor_lake_jack_present(codec), which performs HDA bus
reads and calls msleep(100) when the pin sense bounces.

In hda_codec_driver_remove(), driver->ops->remove(codec) (cx_remove()) is
called before snd_hda_codec_cleanup_for_unbind() removes ALSA controls,
cancels codec->jackpoll_work, or frees jack callbacks, and bus->unsol_work is
not flushed before cx_remove() runs:

CPU0                                    CPU1
cxt_matebook_raptor_lake_jack_event()
  if (!spec->rpl_recheck_codec)
  cxt_matebook_raptor_lake_jack_present()
    msleep(100);
                                        cx_remove()
                                          spec->rpl_recheck_codec = NULL;
                                          cancel_delayed_work_sync(&spec->rpl_recheck_work);
                                          cx_auto_shutdown(codec);
                                          snd_hda_gen_remove(codec)
                                            kfree(codec->spec);
                                            codec->spec = NULL;
  cxt_matebook_raptor_lake_apply(codec, hp)
    spec = codec->spec; /* NULL */
    spec->gen.hp_jack_present = hp;
  mod_delayed_work(..., &spec->rpl_recheck_work, ...); /* UAF on freed spec */

Furthermore, if either callback is invoked after snd_hda_gen_remove() sets
codec->spec = NULL and before snd_hda_codec_cleanup_for_unbind() completes,
dereferencing spec->rpl_recheck_codec at the start of the callback will
immediately dereference a NULL spec pointer.

Similarly, in cx_suspend(), spec->rpl_recheck_codec is not cleared and
in-flight callbacks are not synchronized, so a callback sleeping in
msleep(100) can finish after cx_suspend(), re-enabling pin 0x17 EAPD after
cx_auto_shutdown() and re-arming spec->rpl_recheck_work after
cancel_delayed_work_sync().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261025054111.31392-1-xiedada@xiedada.net?part=1