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

On Thu, Jul 30, 2026 at 11:45:49AM +0200, Takashi Iwai wrote:
> 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: [email protected]
> > > Signed-off-by: Takashi Iwai <[email protected]>
> > > ---
> > >  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) :)

Oops... sorry my bad, must be too early in the morning for me :-)

Thanks,
Charles