Re: [PATCH RESEND] ALSA: hda/cs35l41: Fix firmware load work teardown

[Takashi Iwai <tiwai@suse.de>]

On Mon, 11 May 2026 06:29:34 +0200,
Cássio Gabriel wrote:
> 
> cs35l41_hda creates ALSA controls whose private data points at the
> cs35l41_hda object. The firmware load control can also queue
> fw_load_work.
> 
> Those controls are not removed on component unbind, and device remove
> only cancels fw_load_work through cs35l41_remove_dsp(). That helper is
> skipped when halo_initialized is false. With firmware_autostart
> disabled, a firmware load can be requested before the DSP has been
> initialized. If the component or device is removed before the queued
> work runs, the worker can run after teardown and dereference driver
> state that is no longer valid.
> 
> Track the created controls and remove them on unbind so no new control
> callback can reach the driver data or queue more work. Then cancel
> fw_load_work to drain any request that was already queued. Also cancel
> the work unconditionally during device remove before runtime PM teardown.
> 
> Fixes: 47ceabd99a28 ("ALSA: hda: cs35l41: Support Firmware switching and reloading")
> Fixes: 4c870513fbb0 ("ALSA: hda: cs35l41: Add read-only ALSA control for forced mute")
> Cc: stable@vger.kernel.org
> Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>

Cirrus people, could you take a look?  I'd like to let this better
reviewed since it's non-trivial changes.


thanks,

Takashi

> ---
>  sound/hda/codecs/side-codecs/cs35l41_hda.c | 77 +++++++++++++++++++++---------
>  sound/hda/codecs/side-codecs/cs35l41_hda.h |  5 ++
>  2 files changed, 60 insertions(+), 22 deletions(-)
> 
> diff --git a/sound/hda/codecs/side-codecs/cs35l41_hda.c b/sound/hda/codecs/side-codecs/cs35l41_hda.c
> index b64890006bb7..7f18be0ccf5a 100644
> --- a/sound/hda/codecs/side-codecs/cs35l41_hda.c
> +++ b/sound/hda/codecs/side-codecs/cs35l41_hda.c
> @@ -1325,6 +1325,43 @@ static int cs35l41_fw_type_ctl_info(struct snd_kcontrol *kcontrol, struct snd_ct
>  	return snd_ctl_enum_info(uinfo, 1, ARRAY_SIZE(cs35l41_hda_fw_ids), cs35l41_hda_fw_ids);
>  }
>  
> +static void cs35l41_remove_controls(struct cs35l41_hda *cs35l41)
> +{
> +	if (!cs35l41->codec)
> +		return;
> +
> +	snd_ctl_remove(cs35l41->codec->card, cs35l41->mute_override_ctl);
> +	cs35l41->mute_override_ctl = NULL;
> +
> +	snd_ctl_remove(cs35l41->codec->card, cs35l41->fw_load_ctl);
> +	cs35l41->fw_load_ctl = NULL;
> +
> +	snd_ctl_remove(cs35l41->codec->card, cs35l41->fw_type_ctl);
> +	cs35l41->fw_type_ctl = NULL;
> +}
> +
> +static int cs35l41_add_control(struct cs35l41_hda *cs35l41,
> +			       struct snd_kcontrol_new *ctl,
> +			       struct snd_kcontrol **kctl)
> +{
> +	int ret;
> +
> +	*kctl = snd_ctl_new1(ctl, cs35l41);
> +	if (!*kctl)
> +		return -ENOMEM;
> +
> +	ret = snd_ctl_add(cs35l41->codec->card, *kctl);
> +	if (ret) {
> +		dev_err(cs35l41->dev, "Failed to add KControl %s = %d\n", ctl->name, ret);
> +		*kctl = NULL;
> +		return ret;
> +	}
> +
> +	dev_dbg(cs35l41->dev, "Added Control %s\n", ctl->name);
> +
> +	return 0;
> +}
> +
>  static int cs35l41_create_controls(struct cs35l41_hda *cs35l41)
>  {
>  	char fw_type_ctl_name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
> @@ -1360,32 +1397,23 @@ static int cs35l41_create_controls(struct cs35l41_hda *cs35l41)
>  	scnprintf(mute_override_ctl_name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN, "%s Forced Mute Status",
>  		  cs35l41->amp_name);
>  
> -	ret = snd_ctl_add(cs35l41->codec->card, snd_ctl_new1(&fw_type_ctl, cs35l41));
> -	if (ret) {
> -		dev_err(cs35l41->dev, "Failed to add KControl %s = %d\n", fw_type_ctl.name, ret);
> -		return ret;
> -	}
> -
> -	dev_dbg(cs35l41->dev, "Added Control %s\n", fw_type_ctl.name);
> -
> -	ret = snd_ctl_add(cs35l41->codec->card, snd_ctl_new1(&fw_load_ctl, cs35l41));
> -	if (ret) {
> -		dev_err(cs35l41->dev, "Failed to add KControl %s = %d\n", fw_load_ctl.name, ret);
> -		return ret;
> -	}
> -
> -	dev_dbg(cs35l41->dev, "Added Control %s\n", fw_load_ctl.name);
> +	ret = cs35l41_add_control(cs35l41, &fw_type_ctl, &cs35l41->fw_type_ctl);
> +	if (ret)
> +		goto err;
>  
> -	ret = snd_ctl_add(cs35l41->codec->card, snd_ctl_new1(&mute_override_ctl, cs35l41));
> -	if (ret) {
> -		dev_err(cs35l41->dev, "Failed to add KControl %s = %d\n", mute_override_ctl.name,
> -			ret);
> -		return ret;
> -	}
> +	ret = cs35l41_add_control(cs35l41, &fw_load_ctl, &cs35l41->fw_load_ctl);
> +	if (ret)
> +		goto err;
>  
> -	dev_dbg(cs35l41->dev, "Added Control %s\n", mute_override_ctl.name);
> +	ret = cs35l41_add_control(cs35l41, &mute_override_ctl, &cs35l41->mute_override_ctl);
> +	if (ret)
> +		goto err;
>  
>  	return 0;
> +
> +err:
> +	cs35l41_remove_controls(cs35l41);
> +	return ret;
>  }
>  
>  static bool cs35l41_dsm_supported(acpi_handle handle, unsigned int commands)
> @@ -1522,6 +1550,10 @@ static void cs35l41_hda_unbind(struct device *dev, struct device *master, void *
>  		device_link_remove(&cs35l41->codec->core.dev, cs35l41->dev);
>  		unlock_system_sleep(sleep_flags);
>  		memset(comp, 0, sizeof(*comp));
> +
> +		cs35l41_remove_controls(cs35l41);
> +		cancel_work_sync(&cs35l41->fw_load_work);
> +		cs35l41->codec = NULL;
>  	}
>  }
>  
> @@ -2058,6 +2090,7 @@ void cs35l41_hda_remove(struct device *dev)
>  	struct cs35l41_hda *cs35l41 = dev_get_drvdata(dev);
>  
>  	component_del(cs35l41->dev, &cs35l41_hda_comp_ops);
> +	cancel_work_sync(&cs35l41->fw_load_work);
>  
>  	pm_runtime_get_sync(cs35l41->dev);
>  	pm_runtime_dont_use_autosuspend(cs35l41->dev);
> diff --git a/sound/hda/codecs/side-codecs/cs35l41_hda.h b/sound/hda/codecs/side-codecs/cs35l41_hda.h
> index 7d003c598e93..56ec07c0bb74 100644
> --- a/sound/hda/codecs/side-codecs/cs35l41_hda.h
> +++ b/sound/hda/codecs/side-codecs/cs35l41_hda.h
> @@ -57,6 +57,8 @@ enum control_bus {
>  	SPI
>  };
>  
> +struct snd_kcontrol;
> +
>  struct cs35l41_hda {
>  	struct device *dev;
>  	struct regmap *regmap;
> @@ -75,6 +77,9 @@ struct cs35l41_hda {
>  	int speaker_id;
>  	struct mutex fw_mutex;
>  	struct work_struct fw_load_work;
> +	struct snd_kcontrol *fw_type_ctl;
> +	struct snd_kcontrol *fw_load_ctl;
> +	struct snd_kcontrol *mute_override_ctl;
>  
>  	struct regmap_irq_chip_data *irq_data;
>  	bool firmware_running;
> 
> ---
> base-commit: 1bc46462f4c09f8d429ae8ec17f92886d604659f
> change-id: 20260421-alsa-hda-cs35l41-fw-work-teardown-48cdba14a9cd
> 
> Best regards,
> --
> Cássio Gabriel <cassiogabrielcontato@gmail.com>
> -- 
> Cássio Gabriel <cassiogabrielcontato@gmail.com>
>