[PATCH] ALSA: hda: cs35l56: Fix mixed cleanup and goto in cs35l56_hda_fw_load()

[PATCH] ALSA: hda: cs35l56: Fix mixed cleanup and goto in cs35l56_hda_fw_load()

[Richard Fitzgerald]

Remove the gotos from cs35l56_hda_fw_load() to prevent mixing
auto-cleanup and goto-cleanup in the same function. Replace them with
returns and remove the goto cleanup code, as there's no need to "undo"
a cs_dsp_power_up() with a cs_dsp_power_down() here.

Two previous commits added auto-cleanup to cs35l56_hda_fw_load() but
didn't remove the goto, which leaves mixed cleanup and goto.

commit 0ff22680e51f4 ("ALSA: hda/cs35l56: Clean up with
PM_RUNTIME_ACQUIRE*() macros")

commit 185841c94accc ("ALSA: hda: cs35l56: Use auto-cleanup for firmware
loading")

There's actually no need to call cs_dsp_power_down() here. On the
CS35L54/56/57 amps it doesn't control the DSP subsystem power (that is all
managed by the firmware), so it's perfectly safe to just return if there
are any errors after a successful cs_dsp_power_up(). As cs_dsp_power_up()
was successful it is valid to leave cs_dsp in this state, which matches
the state of the hardware.

The two places that cs_dsp_power_down() is called conditionally have been
changed to check the cs_dsp.booted flag, now the fw_patched flag is not
a guaranteed proxy. The booted flag is directly indicating that there has
been a successful cs_dsp_power_up() so this is more correct anyway.

Fixes: 0ff22680e51f4 ("ALSA: hda/cs35l56: Clean up with PM_RUNTIME_ACQUIRE*() macros")
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
---
 sound/hda/codecs/side-codecs/cs35l56_hda.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/sound/hda/codecs/side-codecs/cs35l56_hda.c b/sound/hda/codecs/side-codecs/cs35l56_hda.c
index bc207ab5b020a..148d760987fdd 100644
--- a/sound/hda/codecs/side-codecs/cs35l56_hda.c
+++ b/sound/hda/codecs/side-codecs/cs35l56_hda.c
@@ -562,7 +562,7 @@ static void cs35l56_hda_fw_load(struct cs35l56_hda *cs35l56)
 	 * downloaded previously then it needs to be powered down so that it
 	 * can be updated.
 	 */
-	if (cs35l56->base.fw_patched)
+	if (cs35l56->cs_dsp.booted)
 		cs_dsp_power_down(&cs35l56->cs_dsp);
 
 	cs35l56->base.fw_patched = false;
@@ -634,7 +634,7 @@ static void cs35l56_hda_fw_load(struct cs35l56_hda *cs35l56)
 		regcache_mark_dirty(cs35l56->base.regmap);
 		ret = cs35l56_wait_for_firmware_boot(&cs35l56->base);
 		if (ret)
-			goto err_powered_up;
+			return;
 
 		regcache_cache_only(cs35l56->base.regmap, false);
 	}
@@ -642,7 +642,7 @@ static void cs35l56_hda_fw_load(struct cs35l56_hda *cs35l56)
 	/* Disable auto-hibernate so that runtime_pm has control */
 	ret = cs35l56_mbox_send(&cs35l56->base, CS35L56_MBOX_CMD_PREVENT_AUTO_HIBERNATE);
 	if (ret)
-		goto err_powered_up;
+		return;
 
 	regcache_sync(cs35l56->base.regmap);
 
@@ -663,10 +663,6 @@ static void cs35l56_hda_fw_load(struct cs35l56_hda *cs35l56)
 		cs_dsp_stop(&cs35l56->cs_dsp);
 
 	cs35l56_log_tuning(&cs35l56->base, &cs35l56->cs_dsp);
-
-err_powered_up:
-	if (!cs35l56->base.fw_patched)
-		cs_dsp_power_down(&cs35l56->cs_dsp);
 }
 
 static void cs35l56_hda_dsp_work(struct work_struct *work)
@@ -814,7 +810,7 @@ static void cs35l56_hda_unbind(struct device *dev, struct device *master, void *
 	debugfs_remove_recursive(cs35l56->debugfs_root);
 #endif
 
-	if (cs35l56->base.fw_patched)
+	if (cs35l56->cs_dsp.booted)
 		cs_dsp_power_down(&cs35l56->cs_dsp);
 
 	comp = hda_component_from_index(parent, cs35l56->index);
-- 
2.47.3

Re: [PATCH] ALSA: hda: cs35l56: Fix mixed cleanup and goto in cs35l56_hda_fw_load()

[Takashi Iwai]

On Wed, 04 Nov 2026 12:11:21 +0100,
Richard Fitzgerald wrote:
> 
> Remove the gotos from cs35l56_hda_fw_load() to prevent mixing
> auto-cleanup and goto-cleanup in the same function. Replace them with
> returns and remove the goto cleanup code, as there's no need to "undo"
> a cs_dsp_power_up() with a cs_dsp_power_down() here.
> 
> Two previous commits added auto-cleanup to cs35l56_hda_fw_load() but
> didn't remove the goto, which leaves mixed cleanup and goto.
> 
> commit 0ff22680e51f4 ("ALSA: hda/cs35l56: Clean up with
> PM_RUNTIME_ACQUIRE*() macros")
> 
> commit 185841c94accc ("ALSA: hda: cs35l56: Use auto-cleanup for firmware
> loading")
> 
> There's actually no need to call cs_dsp_power_down() here. On the
> CS35L54/56/57 amps it doesn't control the DSP subsystem power (that is all
> managed by the firmware), so it's perfectly safe to just return if there
> are any errors after a successful cs_dsp_power_up(). As cs_dsp_power_up()
> was successful it is valid to leave cs_dsp in this state, which matches
> the state of the hardware.
> 
> The two places that cs_dsp_power_down() is called conditionally have been
> changed to check the cs_dsp.booted flag, now the fw_patched flag is not
> a guaranteed proxy. The booted flag is directly indicating that there has
> been a successful cs_dsp_power_up() so this is more correct anyway.
> 
> Fixes: 0ff22680e51f4 ("ALSA: hda/cs35l56: Clean up with PM_RUNTIME_ACQUIRE*() macros")
> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>

Applied now.  Thanks.


Takashi