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

[Charles Keepax <ckeepax@opensource.cirrus.com>]

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?

Thanks,
Charles