mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Steven 'Steve' Kendall <skend@chromium.org>
Cc: Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
	linux-sound@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] ALSA: hda/cs8409: Fix for Dell Cirrus audio jack detect
Date: Tue, 15 Sep 2026 12:16:42 +0200	[thread overview]
Message-ID: <87fqzan7jp.wl-tiwai@suse.de> (raw)
In-Reply-To: <20260915-fix-headphone-plug-cirrus-dell-v3-1-6dc555b00f61@chromium.org>

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>
> 

  reply	other threads:[~2026-09-15 10:16 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-15  2:11 Steven 'Steve' Kendall
2026-09-15 10:16 ` Takashi Iwai [this message]
2026-09-16  7:33   ` Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87fqzan7jp.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=perex@perex.cz \
    --cc=skend@chromium.org \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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®