[PATCH v2 0/3] Fixes and cleanup for CS35L41 HDA
Several minor issues were found during additional testing and static analysis. These patches fix these minor issues.
CHANGES SINCE V1: Patch 2: - Removed unnecessary cast
Stefan Binding (3): ALSA: hda: cs35l41: Clean up Firmware Load Controls ALSA: hda: cs35l41: Fix endian conversions ALSA: hda/realtek: Delete cs35l41 component master during free
sound/pci/hda/cs35l41_hda.c | 32 ++++++++++++++------------------ sound/pci/hda/patch_realtek.c | 2 ++ 2 files changed, 16 insertions(+), 18 deletions(-)
Ensure Firmware Load control and Firmware Type control returns 1 when the value changes.
Remove fw_mutex from firmware load control put, since it is unnecessary, and prevents any possibility of mutex inversion.
Signed-off-by: Stefan Binding sbinding@opensource.cirrus.com --- sound/pci/hda/cs35l41_hda.c | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c index b5210abb5141f..d100189e15b83 100644 --- a/sound/pci/hda/cs35l41_hda.c +++ b/sound/pci/hda/cs35l41_hda.c @@ -835,34 +835,26 @@ static int cs35l41_fw_load_ctl_put(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { struct cs35l41_hda *cs35l41 = snd_kcontrol_chip(kcontrol); - unsigned int ret = 0; - - mutex_lock(&cs35l41->fw_mutex);
if (cs35l41->request_fw_load == ucontrol->value.integer.value[0]) - goto err; + return 0;
if (cs35l41->fw_request_ongoing) { dev_dbg(cs35l41->dev, "Existing request not complete\n"); - ret = -EBUSY; - goto err; + return -EBUSY; }
/* Check if playback is ongoing when initial request is made */ if (cs35l41->playback_started) { dev_err(cs35l41->dev, "Cannot Load/Unload firmware during Playback\n"); - ret = -EBUSY; - goto err; + return -EBUSY; }
cs35l41->fw_request_ongoing = true; cs35l41->request_fw_load = ucontrol->value.integer.value[0]; schedule_work(&cs35l41->fw_load_work);
-err: - mutex_unlock(&cs35l41->fw_mutex); - - return ret; + return 1; }
static int cs35l41_fw_type_ctl_get(struct snd_kcontrol *kcontrol, @@ -881,8 +873,12 @@ static int cs35l41_fw_type_ctl_put(struct snd_kcontrol *kcontrol, struct cs35l41_hda *cs35l41 = snd_kcontrol_chip(kcontrol);
if (ucontrol->value.enumerated.item[0] < HDA_CS_DSP_NUM_FW) { - cs35l41->firmware_type = ucontrol->value.enumerated.item[0]; - return 0; + if (cs35l41->firmware_type != ucontrol->value.enumerated.item[0]) { + cs35l41->firmware_type = ucontrol->value.enumerated.item[0]; + return 1; + } else { + return 0; + } }
return -EINVAL;
Found during static analysis, ensure variables are correct types before endian conversion.
Signed-off-by: Stefan Binding sbinding@opensource.cirrus.com --- sound/pci/hda/cs35l41_hda.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c index d100189e15b83..ce5faa6205170 100644 --- a/sound/pci/hda/cs35l41_hda.c +++ b/sound/pci/hda/cs35l41_hda.c @@ -308,8 +308,8 @@ static int cs35l41_request_firmware_files(struct cs35l41_hda *cs35l41, }
#if IS_ENABLED(CONFIG_EFI) -static int cs35l41_apply_calibration(struct cs35l41_hda *cs35l41, unsigned int ambient, - unsigned int r0, unsigned int status, unsigned int checksum) +static int cs35l41_apply_calibration(struct cs35l41_hda *cs35l41, __be32 ambient, __be32 r0, + __be32 status, __be32 checksum) { int ret;
@@ -745,7 +745,7 @@ static int cs35l41_runtime_resume(struct device *dev)
static int cs35l41_smart_amp(struct cs35l41_hda *cs35l41) { - int halo_sts; + __be32 halo_sts; int ret;
ret = cs35l41_init_dsp(cs35l41); @@ -773,7 +773,7 @@ static int cs35l41_smart_amp(struct cs35l41_hda *cs35l41) &halo_sts, sizeof(halo_sts));
if (ret) { - dev_err(cs35l41->dev, "Timeout waiting for HALO Core to start. State: %d\n", + dev_err(cs35l41->dev, "Timeout waiting for HALO Core to start. State: %u\n", halo_sts); goto clean_dsp; }
This ensures that the driver is properly cleaned up when freed.
Signed-off-by: Stefan Binding sbinding@opensource.cirrus.com --- sound/pci/hda/patch_realtek.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 7b5f194513c7b..e3774903918fe 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -6757,6 +6757,8 @@ static void cs35l41_generic_fixup(struct hda_codec *cdc, int action, const char else spec->gen.pcm_playback_hook = comp_generic_playback_hook; break; + case HDA_FIXUP_ACT_FREE: + component_master_del(dev, &comp_master_ops); } }
On Mon, 05 Jun 2023 17:28:55 +0200, Stefan Binding wrote:
This ensures that the driver is properly cleaned up when freed.
Signed-off-by: Stefan Binding sbinding@opensource.cirrus.com
sound/pci/hda/patch_realtek.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 7b5f194513c7b..e3774903918fe 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -6757,6 +6757,8 @@ static void cs35l41_generic_fixup(struct hda_codec *cdc, int action, const char else spec->gen.pcm_playback_hook = comp_generic_playback_hook; break;
- case HDA_FIXUP_ACT_FREE:
component_master_del(dev, &comp_master_ops);
Don't forget to add break here.
thanks,
Takashi
participants (2)
-
Stefan Binding -
Takashi Iwai