Re: [PATCH 06/14] ALSA: hda: cs35l56: Use auto-cleanup for firmware loading

[Takashi Iwai <tiwai@suse.de>]

On Thu, 30 Jul 2026 11:35:56 +0200,
Charles Keepax wrote:
> 
> On Wed, Jul 29, 2026 at 10:37:24AM +0200, Takashi Iwai wrote:
> > Simplify the code to manage the firmware loading with auto-cleanup.
> > By the use of __free(firmware), we can replace the manual mutex locks
> > with guard() gracefully, too.
> > 
> > Only the code refactoring, no functional changes.
> > 
> > Cc: patches@opensource.cirrus.com
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >  sound/hda/codecs/side-codecs/cs35l56_hda.c | 35 ++++++----------------
> >  1 file changed, 9 insertions(+), 26 deletions(-)
> > 
> > diff --git a/sound/hda/codecs/side-codecs/cs35l56_hda.c b/sound/hda/codecs/side-codecs/cs35l56_hda.c
> > index 78c2cf387a00..bc207ab5b020 100644
> > --- a/sound/hda/codecs/side-codecs/cs35l56_hda.c
> > +++ b/sound/hda/codecs/side-codecs/cs35l56_hda.c
> > @@ -527,18 +527,6 @@ static void cs35l56_hda_request_firmware_files(struct cs35l56_hda *cs35l56,
> >  						  base_name, NULL, NULL, "bin");
> >  }
> >  
> > -static void cs35l56_hda_release_firmware_files(const struct firmware *wmfw_firmware,
> > -					       char *wmfw_filename,
> > -					       const struct firmware *coeff_firmware,
> > -					       char *coeff_filename)
> > -{
> > -	release_firmware(wmfw_firmware);
> > -	kfree(wmfw_filename);
> > -
> > -	release_firmware(coeff_firmware);
> > -	kfree(coeff_filename);
> > -}
> > -
> >  static int cs35l56_hda_apply_calibration(struct cs35l56_hda *cs35l56)
> >  {
> >  	int ret;
> > @@ -561,10 +549,10 @@ static int cs35l56_hda_apply_calibration(struct cs35l56_hda *cs35l56)
> >  
> >  static void cs35l56_hda_fw_load(struct cs35l56_hda *cs35l56)
> >  {
> > -	const struct firmware *coeff_firmware = NULL;
> > -	const struct firmware *wmfw_firmware = NULL;
> > -	char *coeff_filename = NULL;
> > -	char *wmfw_filename = NULL;
> > +	const struct firmware *coeff_firmware __free(firmware) = NULL;
> > +	const struct firmware *wmfw_firmware __free(firmware) = NULL;
> 
> Is it definitely safe to replace release_firmware with kfree? In
> most cases that is all release_firmware does, but it looks like
> it does different stuff if someone built the firmware into the
> kernel?

It's __free(firmware), not __free(kfree) :)


Takashi