Re: [PATCH v2] ALSA: hda/cs8409: Fix for Dell Cirrus audio jack detect

[Takashi Iwai <tiwai@suse.de>]

On Sat, 18 Jul 2026 04:05:41 +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.
> 
> ---
> This patch switches to a delayed work method for a
> specific Dell machine (Inspiron 15 3520). This fixes
> jack detection for this model.
> 
> Signed-off-by: Steven 'Steve' Kendall <skend@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!

Hmm, not really.  You've put the foreword separated with '---' again,
hence git-am picks up only the foreword, not the actual patch
description.  In general, there is no foreword in the patch.  And,
your foreword texts are obviously a part of the patch description, so
don't use foreword at all.

Please try once to submit to yourself, apply the patch via git-am
locally, and check that everything works as expected, then submit to
the upstream.

About the code change -- there is too little information in the patch,
both as descriptions and comments.  Please elaborate more.  Especially
about why a delayed work is needed and why you use different values
(100 and 2000 msecs) for the delay.  It looks like you picked up
something from the unsol event handling, etc and open-code in the
delayed work, and this should be described, too.


thanks,

Takashi

> - Link to v1: https://lore.kernel.org/r/20260713-fix-headphone-plug-cirrus-dell-v1-1-3c5157cd45cd@chromium.org
> 
> To: Jaroslav Kysela <perex@perex.cz>
> To: Takashi Iwai <tiwai@suse.com>
> Cc: linux-sound@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  sound/hda/codecs/cirrus/cs8409.c | 32 ++++++++++++++++++++++++++++++--
>  sound/hda/codecs/cirrus/cs8409.h |  1 +
>  2 files changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/hda/codecs/cirrus/cs8409.c b/sound/hda/codecs/cirrus/cs8409.c
> index c43ff3ef75b6e..785946e29ce30 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_work(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_work);
>  	snd_hda_gen_spec_init(&spec->gen);
>  
>  	return spec;
> @@ -115,6 +120,27 @@ static void cs8409_disable_i2c_clock_worker(struct work_struct *work)
>  	cs8409_disable_i2c_clock(spec->codec);
>  }
>  
> +static void cs8409_jack_detect_work(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);
> +
> +		if (reg_ts_status < 0)
> +			return;
> +
> +		if (((reg_ts_status & (CS42L42_TS_PLUG_MASK | CS42L42_TS_UNPLUG_MASK))
> +			>> CS42L42_TS_PLUG_SHIFT) == CS42L42_TS_PLUG) {
> +			cs42l42_run_jack_detect(cs42l42);
> +		} else {
> +			/* Make sure Tip Sense interrupts are unmasked */
> +			cs8409_i2c_write(cs42l42, CS42L42_TSRS_PLUG_INT_MASK, 0xF3);
> +		}
> +	}
> +}
> +
>  /*
>   * cs8409_enable_i2c_clock - Enable I2C clocks
>   * @codec: the codec instance
> @@ -953,6 +979,7 @@ 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);
> +	cancel_delayed_work_sync(&spec->jack_detect_work);
>  	cs8409_disable_i2c_clock(codec);
>  
>  	snd_hda_gen_remove(codec);
> @@ -1025,6 +1052,7 @@ 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);
> +	cancel_delayed_work_sync(&spec->jack_detect_work);
>  	cs8409_disable_i2c_clock(codec);
>  
>  	snd_hda_shutup_pins(codec);
> @@ -1198,7 +1226,7 @@ void cs8409_cs42l42_fixups(struct hda_codec *codec, const struct hda_fixup *fix,
>  		spec->init_done = 1;
>  		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(100));
>  		break;
>  	case HDA_FIXUP_ACT_BUILD:
>  		spec->build_ctrl_done = 1;
> @@ -1209,7 +1237,7 @@ void cs8409_cs42l42_fixups(struct hda_codec *codec, const struct hda_fixup *fix,
>  		 */
>  		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(2000));
>  		break;
>  	default:
>  		break;
> diff --git a/sound/hda/codecs/cirrus/cs8409.h b/sound/hda/codecs/cirrus/cs8409.h
> index be1714a84fff4..19ea0affb9764 100644
> --- a/sound/hda/codecs/cirrus/cs8409.h
> +++ b/sound/hda/codecs/cirrus/cs8409.h
> @@ -339,6 +339,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: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
> change-id: 20260710-fix-headphone-plug-cirrus-dell-5e3b49da5f52
> 
> Best regards,
> --  
> Steven 'Steve' Kendall <skend@chromium.org>
>