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

[Stefan Binding (Opensource)]

Hi,

> -----Original Message-----
> From: Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxx>
> Sent: Friday, May 15, 2026 11:12 AM
> To: Cássio Gabriel <cassiogabrielcontato@xxxxxxxxx>
> Cc: David Rhodes <david.rhodes@xxxxxxxxxx>; Richard Fitzgerald
> <rf@xxxxxxxxxxxxxxxxxxxxx>; Takashi Iwai <tiwai@xxxxxxxx>; Stefan Binding
> <sbinding@xxxxxxxxxxxxxxxxxxxxx>; Vitaly Rodionov
> <vitalyr@xxxxxxxxxxxxxxxxxxxxx>; Jaroslav Kysela <perex@xxxxxxxx>; linux-
> sound@xxxxxxxxxxxxxxx; patches@xxxxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; stable@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH RESEND] ALSA: hda/cs35l41: Fix firmware load work
> teardown
> 
> On Mon, May 11, 2026 at 01:29:34AM -0300, 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@xxxxxxxxxxxxxxx
> > Signed-off-by: Cássio Gabriel <cassiogabrielcontato@xxxxxxxxx>
> > ---
> >  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;
> 
> Hmm... are we sure the controls are actually still accessible from
user-space
> here? Feels like generally it would make more sense to make all the cards
> controls inaccessible before we start tearing the card down as a core
feature.
> 
> Adding the cancel works looks very sensible.
> 
> @Stefan, could you also please have a look.

I think this is fine to do, and I did some tests to make sure it doesn?t
break anything.
Reviewed-by: Stefan Binding <sbinding@xxxxxxxxxxxxxxxxxxxxx>

> 
> Thanks,
> Charles