mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3] ALSA: hda/cs8409: Fix for Dell Cirrus audio jack detect
@ 2026-09-15  2:11 Steven 'Steve' Kendall
  2026-09-15 10:16 ` Takashi Iwai
  0 siblings, 1 reply; 5+ messages in thread
From: Steven 'Steve' Kendall @ 2026-09-15  2:11 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai
  Cc: linux-sound, linux-kernel, Steven 'Steve' Kendall

On some models like the Dell Inspiron 15 3520, jack
detection does not work. This patch switches to
delayed work to fix jack plug on Chrome OS.

Assisted-by: LLM
Signed-off-by: Steven 'Steve' Kendall <skend@chromium.org>
---
Changes in v3:
- Unified sleep time for both cases.
- Removed some superfluous logic after some additional testing.
- Link to v2: https://patch.msgid.link/20260718-fix-headphone-plug-cirrus-dell-v2-1-f88121570d62@chromium.org

Changes in v2:
- Changes from v1 have been removed.
- New approach uses delayed work and also solves the issue on this machine.
- I'm now using the latest release of b4. Hopefully this addresses the formatting issues I was having!
- Link to v1: https://lore.kernel.org/r/20260713-fix-headphone-plug-cirrus-dell-v1-1-3c5157cd45cd@chromium.org
---
 sound/hda/codecs/cirrus/cs8409.c | 43 +++++++++++++++++++++++++++++++++++++---
 sound/hda/codecs/cirrus/cs8409.h |  2 ++
 2 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/sound/hda/codecs/cirrus/cs8409.c b/sound/hda/codecs/cirrus/cs8409.c
index c43ff3ef75b6e..1300571e4b6ae 100644
--- a/sound/hda/codecs/cirrus/cs8409.c
+++ b/sound/hda/codecs/cirrus/cs8409.c
@@ -56,6 +56,10 @@ static int cs8409_parse_auto_config(struct hda_codec *codec)
 }
 
 static void cs8409_disable_i2c_clock_worker(struct work_struct *work);
+static void cs8409_jack_detect_worker(struct work_struct *work);
+static void cs42l42_run_jack_detect(struct sub_codec *cs42l42);
+static int cs8409_i2c_read(struct sub_codec *scodec, unsigned int addr);
+static int cs8409_i2c_write(struct sub_codec *scodec, unsigned int addr, unsigned int value);
 
 static struct cs8409_spec *cs8409_alloc_spec(struct hda_codec *codec)
 {
@@ -69,6 +73,7 @@ static struct cs8409_spec *cs8409_alloc_spec(struct hda_codec *codec)
 	codec->power_save_node = 1;
 	mutex_init(&spec->i2c_mux);
 	INIT_DELAYED_WORK(&spec->i2c_clk_work, cs8409_disable_i2c_clock_worker);
+	INIT_DELAYED_WORK(&spec->jack_detect_work, cs8409_jack_detect_worker);
 	snd_hda_gen_spec_init(&spec->gen);
 
 	return spec;
@@ -115,6 +120,28 @@ static void cs8409_disable_i2c_clock_worker(struct work_struct *work)
 	cs8409_disable_i2c_clock(spec->codec);
 }
 
+/*
+ * cs8409_jack_detect_worker - Perform initial jack detection once hardware is settled
+ *
+ * Defer initial jack detection. On ChromeOS and possibly other platforms this
+ * delay is required for jack detection to work as expected.
+ */
+static void cs8409_jack_detect_worker(struct work_struct *work)
+{
+	struct cs8409_spec *spec = container_of(work, struct cs8409_spec, jack_detect_work.work);
+	struct sub_codec *cs42l42 = spec->scodecs[CS8409_CODEC0];
+
+	if (spec->init_done && spec->build_ctrl_done && !cs42l42->hp_jack_in) {
+		int reg_ts_status = cs8409_i2c_read(cs42l42, CS42L42_TSRS_PLUG_STATUS);
+
+		/* error case */
+		if (reg_ts_status < 0)
+			return;
+
+		cs42l42_run_jack_detect(cs42l42);
+	}
+}
+
 /*
  * cs8409_enable_i2c_clock - Enable I2C clocks
  * @codec: the codec instance
@@ -954,6 +981,8 @@ static void cs8409_remove(struct hda_codec *codec)
 	/* Cancel i2c clock disable timer, and disable clock if left enabled */
 	cancel_delayed_work_sync(&spec->i2c_clk_work);
 	cs8409_disable_i2c_clock(codec);
+	/* Also cancel jack detect work */
+	cancel_delayed_work_sync(&spec->jack_detect_work);
 
 	snd_hda_gen_remove(codec);
 }
@@ -1026,6 +1055,8 @@ static int cs8409_cs42l42_suspend(struct hda_codec *codec)
 	/* Cancel i2c clock disable timer, and disable clock if left enabled */
 	cancel_delayed_work_sync(&spec->i2c_clk_work);
 	cs8409_disable_i2c_clock(codec);
+	/* Also cancel jack detect work */
+	cancel_delayed_work_sync(&spec->jack_detect_work);
 
 	snd_hda_shutup_pins(codec);
 
@@ -1196,20 +1227,26 @@ void cs8409_cs42l42_fixups(struct hda_codec *codec, const struct hda_fixup *fix,
 	case HDA_FIXUP_ACT_INIT:
 		cs8409_cs42l42_hw_init(codec);
 		spec->init_done = 1;
+		/*
+		 * On resume, schedule delayed jack detect to allow the CS42L42
+		 * to settle after reset before running detection.
+		 */
 		if (spec->init_done && spec->build_ctrl_done
 			&& !spec->scodecs[CS8409_CODEC0]->hp_jack_in)
-			cs42l42_run_jack_detect(spec->scodecs[CS8409_CODEC0]);
+			schedule_delayed_work(&spec->jack_detect_work,
+					      msecs_to_jiffies(CS8409_JACK_DETECT_DELAY_MS));
 		break;
 	case HDA_FIXUP_ACT_BUILD:
 		spec->build_ctrl_done = 1;
-		/* Run jack auto detect first time on boot
+		/* Schedule jack auto detect first time on boot
 		 * after controls have been added, to check if jack has
 		 * been already plugged in.
 		 * Run immediately after init.
 		 */
 		if (spec->init_done && spec->build_ctrl_done
 			&& !spec->scodecs[CS8409_CODEC0]->hp_jack_in)
-			cs42l42_run_jack_detect(spec->scodecs[CS8409_CODEC0]);
+			schedule_delayed_work(&spec->jack_detect_work,
+					      msecs_to_jiffies(CS8409_JACK_DETECT_DELAY_MS));
 		break;
 	default:
 		break;
diff --git a/sound/hda/codecs/cirrus/cs8409.h b/sound/hda/codecs/cirrus/cs8409.h
index be1714a84fff4..dca9b31aeebfc 100644
--- a/sound/hda/codecs/cirrus/cs8409.h
+++ b/sound/hda/codecs/cirrus/cs8409.h
@@ -247,6 +247,7 @@ enum cs8409_coefficient_index_registers {
 #define CS8409_CS42L42_AMIC_PIN_NID		CS8409_PIN_ASP1_RECEIVER_A
 #define CS8409_CS42L42_DMIC_PIN_NID		CS8409_PIN_DMIC1_IN
 #define CS8409_CS42L42_DMIC_ADC_PIN_NID		CS8409_PIN_DMIC1
+#define CS8409_JACK_DETECT_DELAY_MS		100
 
 /* Dolphin */
 
@@ -339,6 +340,7 @@ struct cs8409_spec {
 	unsigned int i2c_clck_enabled;
 	unsigned int dev_addr;
 	struct delayed_work i2c_clk_work;
+	struct delayed_work jack_detect_work;
 
 	unsigned int playback_started:1;
 	unsigned int capture_started:1;

---
base-commit: df2908090cda368b01ff43709f51890076c56157
change-id: 20260710-fix-headphone-plug-cirrus-dell-5e3b49da5f52

Best regards,
--  
Steven 'Steve' Kendall <skend@chromium.org>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] ALSA: hda/cs8409: Fix for Dell Cirrus audio jack detect
  2026-09-15  2:11 [PATCH v3] ALSA: hda/cs8409: Fix for Dell Cirrus audio jack detect Steven 'Steve' Kendall
@ 2026-09-15 10:16 ` Takashi Iwai
  2026-09-16  7:33   ` Takashi Iwai
  0 siblings, 1 reply; 5+ messages in thread
From: Takashi Iwai @ 2026-09-15 10:16 UTC (permalink / raw)
  To: Steven 'Steve' Kendall
  Cc: Jaroslav Kysela, Takashi Iwai, linux-sound, linux-kernel

On Tue, 15 Sep 2026 04:11:38 +0200,
Steven 'Steve' Kendall wrote:
> 
> On some models like the Dell Inspiron 15 3520, jack
> detection does not work. This patch switches to
> delayed work to fix jack plug on Chrome OS.
> 
> Assisted-by: LLM
> Signed-off-by: Steven 'Steve' Kendall <skend@chromium.org>

The code change looks acceptable, but the patch description gives too
little information.

And, should we apply it unconditionally all models with this codec?


thanks,

Takashi

> ---
> Changes in v3:
> - Unified sleep time for both cases.
> - Removed some superfluous logic after some additional testing.
> - Link to v2: https://patch.msgid.link/20260718-fix-headphone-plug-cirrus-dell-v2-1-f88121570d62@chromium.org
> 
> Changes in v2:
> - Changes from v1 have been removed.
> - New approach uses delayed work and also solves the issue on this machine.
> - I'm now using the latest release of b4. Hopefully this addresses the formatting issues I was having!
> - Link to v1: https://lore.kernel.org/r/20260713-fix-headphone-plug-cirrus-dell-v1-1-3c5157cd45cd@chromium.org
> ---
>  sound/hda/codecs/cirrus/cs8409.c | 43 +++++++++++++++++++++++++++++++++++++---
>  sound/hda/codecs/cirrus/cs8409.h |  2 ++
>  2 files changed, 42 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/hda/codecs/cirrus/cs8409.c b/sound/hda/codecs/cirrus/cs8409.c
> index c43ff3ef75b6e..1300571e4b6ae 100644
> --- a/sound/hda/codecs/cirrus/cs8409.c
> +++ b/sound/hda/codecs/cirrus/cs8409.c
> @@ -56,6 +56,10 @@ static int cs8409_parse_auto_config(struct hda_codec *codec)
>  }
>  
>  static void cs8409_disable_i2c_clock_worker(struct work_struct *work);
> +static void cs8409_jack_detect_worker(struct work_struct *work);
> +static void cs42l42_run_jack_detect(struct sub_codec *cs42l42);
> +static int cs8409_i2c_read(struct sub_codec *scodec, unsigned int addr);
> +static int cs8409_i2c_write(struct sub_codec *scodec, unsigned int addr, unsigned int value);
>  
>  static struct cs8409_spec *cs8409_alloc_spec(struct hda_codec *codec)
>  {
> @@ -69,6 +73,7 @@ static struct cs8409_spec *cs8409_alloc_spec(struct hda_codec *codec)
>  	codec->power_save_node = 1;
>  	mutex_init(&spec->i2c_mux);
>  	INIT_DELAYED_WORK(&spec->i2c_clk_work, cs8409_disable_i2c_clock_worker);
> +	INIT_DELAYED_WORK(&spec->jack_detect_work, cs8409_jack_detect_worker);
>  	snd_hda_gen_spec_init(&spec->gen);
>  
>  	return spec;
> @@ -115,6 +120,28 @@ static void cs8409_disable_i2c_clock_worker(struct work_struct *work)
>  	cs8409_disable_i2c_clock(spec->codec);
>  }
>  
> +/*
> + * cs8409_jack_detect_worker - Perform initial jack detection once hardware is settled
> + *
> + * Defer initial jack detection. On ChromeOS and possibly other platforms this
> + * delay is required for jack detection to work as expected.
> + */
> +static void cs8409_jack_detect_worker(struct work_struct *work)
> +{
> +	struct cs8409_spec *spec = container_of(work, struct cs8409_spec, jack_detect_work.work);
> +	struct sub_codec *cs42l42 = spec->scodecs[CS8409_CODEC0];
> +
> +	if (spec->init_done && spec->build_ctrl_done && !cs42l42->hp_jack_in) {
> +		int reg_ts_status = cs8409_i2c_read(cs42l42, CS42L42_TSRS_PLUG_STATUS);
> +
> +		/* error case */
> +		if (reg_ts_status < 0)
> +			return;
> +
> +		cs42l42_run_jack_detect(cs42l42);
> +	}
> +}
> +
>  /*
>   * cs8409_enable_i2c_clock - Enable I2C clocks
>   * @codec: the codec instance
> @@ -954,6 +981,8 @@ static void cs8409_remove(struct hda_codec *codec)
>  	/* Cancel i2c clock disable timer, and disable clock if left enabled */
>  	cancel_delayed_work_sync(&spec->i2c_clk_work);
>  	cs8409_disable_i2c_clock(codec);
> +	/* Also cancel jack detect work */
> +	cancel_delayed_work_sync(&spec->jack_detect_work);
>  
>  	snd_hda_gen_remove(codec);
>  }
> @@ -1026,6 +1055,8 @@ static int cs8409_cs42l42_suspend(struct hda_codec *codec)
>  	/* Cancel i2c clock disable timer, and disable clock if left enabled */
>  	cancel_delayed_work_sync(&spec->i2c_clk_work);
>  	cs8409_disable_i2c_clock(codec);
> +	/* Also cancel jack detect work */
> +	cancel_delayed_work_sync(&spec->jack_detect_work);
>  
>  	snd_hda_shutup_pins(codec);
>  
> @@ -1196,20 +1227,26 @@ void cs8409_cs42l42_fixups(struct hda_codec *codec, const struct hda_fixup *fix,
>  	case HDA_FIXUP_ACT_INIT:
>  		cs8409_cs42l42_hw_init(codec);
>  		spec->init_done = 1;
> +		/*
> +		 * On resume, schedule delayed jack detect to allow the CS42L42
> +		 * to settle after reset before running detection.
> +		 */
>  		if (spec->init_done && spec->build_ctrl_done
>  			&& !spec->scodecs[CS8409_CODEC0]->hp_jack_in)
> -			cs42l42_run_jack_detect(spec->scodecs[CS8409_CODEC0]);
> +			schedule_delayed_work(&spec->jack_detect_work,
> +					      msecs_to_jiffies(CS8409_JACK_DETECT_DELAY_MS));
>  		break;
>  	case HDA_FIXUP_ACT_BUILD:
>  		spec->build_ctrl_done = 1;
> -		/* Run jack auto detect first time on boot
> +		/* Schedule jack auto detect first time on boot
>  		 * after controls have been added, to check if jack has
>  		 * been already plugged in.
>  		 * Run immediately after init.
>  		 */
>  		if (spec->init_done && spec->build_ctrl_done
>  			&& !spec->scodecs[CS8409_CODEC0]->hp_jack_in)
> -			cs42l42_run_jack_detect(spec->scodecs[CS8409_CODEC0]);
> +			schedule_delayed_work(&spec->jack_detect_work,
> +					      msecs_to_jiffies(CS8409_JACK_DETECT_DELAY_MS));
>  		break;
>  	default:
>  		break;
> diff --git a/sound/hda/codecs/cirrus/cs8409.h b/sound/hda/codecs/cirrus/cs8409.h
> index be1714a84fff4..dca9b31aeebfc 100644
> --- a/sound/hda/codecs/cirrus/cs8409.h
> +++ b/sound/hda/codecs/cirrus/cs8409.h
> @@ -247,6 +247,7 @@ enum cs8409_coefficient_index_registers {
>  #define CS8409_CS42L42_AMIC_PIN_NID		CS8409_PIN_ASP1_RECEIVER_A
>  #define CS8409_CS42L42_DMIC_PIN_NID		CS8409_PIN_DMIC1_IN
>  #define CS8409_CS42L42_DMIC_ADC_PIN_NID		CS8409_PIN_DMIC1
> +#define CS8409_JACK_DETECT_DELAY_MS		100
>  
>  /* Dolphin */
>  
> @@ -339,6 +340,7 @@ struct cs8409_spec {
>  	unsigned int i2c_clck_enabled;
>  	unsigned int dev_addr;
>  	struct delayed_work i2c_clk_work;
> +	struct delayed_work jack_detect_work;
>  
>  	unsigned int playback_started:1;
>  	unsigned int capture_started:1;
> 
> ---
> base-commit: df2908090cda368b01ff43709f51890076c56157
> change-id: 20260710-fix-headphone-plug-cirrus-dell-5e3b49da5f52
> 
> Best regards,
> --  
> Steven 'Steve' Kendall <skend@chromium.org>
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] ALSA: hda/cs8409: Fix for Dell Cirrus audio jack detect
  2026-09-15 10:16 ` Takashi Iwai
@ 2026-09-16  7:33   ` Takashi Iwai
  2026-09-21 21:00     ` Steven Kendall
  0 siblings, 1 reply; 5+ messages in thread
From: Takashi Iwai @ 2026-09-16  7:33 UTC (permalink / raw)
  To: Steven 'Steve' Kendall
  Cc: Jaroslav Kysela, Takashi Iwai, linux-sound, linux-kernel

On Tue, 15 Sep 2026 12:16:42 +0200,
Takashi Iwai wrote:
> 
> On Tue, 15 Sep 2026 04:11:38 +0200,
> Steven 'Steve' Kendall wrote:
> > 
> > On some models like the Dell Inspiron 15 3520, jack
> > detection does not work. This patch switches to
> > delayed work to fix jack plug on Chrome OS.
> > 
> > Assisted-by: LLM
> > Signed-off-by: Steven 'Steve' Kendall <skend@chromium.org>
> 
> The code change looks acceptable, but the patch description gives too
> little information.
> 
> And, should we apply it unconditionally all models with this code?

... also please check sashiko's review
  https://sashiko.dev/#/patchset/20260915-fix-headphone-plug-cirrus-dell-v3-1-6dc555b00f61%40chromium.org

They aren't always correct but often give good hints.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] ALSA: hda/cs8409: Fix for Dell Cirrus audio jack detect
  2026-09-16  7:33   ` Takashi Iwai
@ 2026-09-21 21:00     ` Steven Kendall
  2026-10-04 18:49       ` Takashi Iwai
  0 siblings, 1 reply; 5+ messages in thread
From: Steven Kendall @ 2026-09-21 21:00 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: Jaroslav Kysela, Takashi Iwai, linux-sound, linux-kernel

> > The code change looks acceptable, but the patch description gives too
> > little information.
> >
> > And, should we apply it unconditionally all models with this code?

I've submitted a v4 that only applies the delay to this specific model
I've observed the issue on.

> ... also please check sashiko's review
>   https://sashiko.dev/#/patchset/20260915-fix-headphone-plug-cirrus-dell-v3-1-6dc555b00f61%40chromium.org
>
> They aren't always correct but often give good hints.

I looked over the AI feedback and I'm not sure what the best action to
take is. It seemed like it was kind of a broad criticism of delayed
work in general, which already exists in the same file. Now that the
patch only applies to one machine and I've tested on that one machine
I'm more confident in the fix but I'm open to doing additional
testing. Or if there's some tooling I should be using to make the
abstract "timing issues" more observable I would love to learn more
about that too.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] ALSA: hda/cs8409: Fix for Dell Cirrus audio jack detect
  2026-09-21 21:00     ` Steven Kendall
@ 2026-10-04 18:49       ` Takashi Iwai
  0 siblings, 0 replies; 5+ messages in thread
From: Takashi Iwai @ 2026-10-04 18:49 UTC (permalink / raw)
  To: Steven Kendall
  Cc: Takashi Iwai, Jaroslav Kysela, Takashi Iwai, linux-sound, linux-kernel

On Mon, 21 Sep 2026 23:00:28 +0200,
Steven Kendall wrote:
> 
> > > The code change looks acceptable, but the patch description gives too
> > > little information.
> > >
> > > And, should we apply it unconditionally all models with this code?
> 
> I've submitted a v4 that only applies the delay to this specific model
> I've observed the issue on.
> 
> > ... also please check sashiko's review
> >   https://sashiko.dev/#/patchset/20260915-fix-headphone-plug-cirrus-dell-v3-1-6dc555b00f61%40chromium.org
> >
> > They aren't always correct but often give good hints.
> 
> I looked over the AI feedback and I'm not sure what the best action to
> take is. It seemed like it was kind of a broad criticism of delayed
> work in general, which already exists in the same file. Now that the
> patch only applies to one machine and I've tested on that one machine
> I'm more confident in the fix but I'm open to doing additional
> testing. Or if there's some tooling I should be using to make the
> abstract "timing issues" more observable I would love to learn more
> about that too.

The review to v4 showed about only the usage of cancel_work_*(),
and the suggestion sounds reasonable.  Those new cancel_work_*()
should be put before disabling i2c.

BTW, at the remove patch, maybe better to use disable_work*() variant
than cancel_work*().  The existing code is old and they were before
disable_*() was introduced (so they should be replaced, too).


thanks,

Takashi

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-04 18:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-15  2:11 [PATCH v3] ALSA: hda/cs8409: Fix for Dell Cirrus audio jack detect Steven 'Steve' Kendall
2026-09-15 10:16 ` Takashi Iwai
2026-09-16  7:33   ` Takashi Iwai
2026-09-21 21:00     ` Steven Kendall
2026-10-04 18:49       ` Takashi Iwai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®