Some systems use older firmware which does not support newer commands
which are used to enable external boost. For those systems, we can
workaround this by writing the registers directly.
We can use the firmware version, stored inside cs_dsp, to determine
whether or not the command is supported.
To achieve this, it requires a cleanup in the api, to pass the cs_dsp
struct into the function.
We can also remove the redundant boolean firmware_running from the HDA
driver, and use the equivalent state inside cs_dsp.
Changes since V1:
- Added fixes tag to all patches
- patches rebased after v6.7 release
Stefan Binding (2):
ALSA: hda: cs35l41: Remove unnecessary boolean state variable
firmware_running
ALSA: cs35l41: Fix for old systems which do not support command
include/sound/cs35l41.h | 2 +-
sound/pci/hda/cs35l41_hda.c | 28 ++++++++++++----------------
sound/soc/codecs/cs35l41-lib.c | 6 ++++--
sound/soc/codecs/cs35l41.c | 4 ++--
4 files changed, 19 insertions(+), 21 deletions(-)
--
2.34.1
Some older laptops using cs35l41 use firmware which does not support
the CSPL_MBOX_CMD_SPK_OUT_ENABLE command.
Firmware versions v0.28.0 and older do not support this command.
Fixes: fa3efcc36aac ("ALSA: cs35l41: Use mbox command to enable speaker output for external boost")
Signed-off-by: Stefan Binding <[email protected]>
---
include/sound/cs35l41.h | 2 +-
sound/pci/hda/cs35l41_hda.c | 4 ++--
sound/soc/codecs/cs35l41-lib.c | 6 ++++--
sound/soc/codecs/cs35l41.c | 4 ++--
4 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/include/sound/cs35l41.h b/include/sound/cs35l41.h
index 043f8ac65dbf..68e053fe7340 100644
--- a/include/sound/cs35l41.h
+++ b/include/sound/cs35l41.h
@@ -906,6 +906,6 @@ int cs35l41_init_boost(struct device *dev, struct regmap *regmap,
bool cs35l41_safe_reset(struct regmap *regmap, enum cs35l41_boost_type b_type);
int cs35l41_mdsync_up(struct regmap *regmap);
int cs35l41_global_enable(struct device *dev, struct regmap *regmap, enum cs35l41_boost_type b_type,
- int enable, bool firmware_running);
+ int enable, struct cs_dsp *dsp);
#endif /* __CS35L41_H */
diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c
index b7d2aa18c566..cbd7d8badf91 100644
--- a/sound/pci/hda/cs35l41_hda.c
+++ b/sound/pci/hda/cs35l41_hda.c
@@ -598,7 +598,7 @@ static void cs35l41_hda_play_done(struct device *dev)
dev_dbg(dev, "Play (Complete)\n");
cs35l41_global_enable(dev, reg, cs35l41->hw_cfg.bst_type, 1,
- cs35l41->cs_dsp.running);
+ &cs35l41->cs_dsp);
cs35l41_mute(dev, false);
}
@@ -611,7 +611,7 @@ static void cs35l41_hda_pause_start(struct device *dev)
cs35l41_mute(dev, true);
cs35l41_global_enable(dev, reg, cs35l41->hw_cfg.bst_type, 0,
- cs35l41->cs_dsp.running);
+ &cs35l41->cs_dsp);
}
static void cs35l41_hda_pause_done(struct device *dev)
diff --git a/sound/soc/codecs/cs35l41-lib.c b/sound/soc/codecs/cs35l41-lib.c
index 4569e4f7cf7e..e9993a39f7d0 100644
--- a/sound/soc/codecs/cs35l41-lib.c
+++ b/sound/soc/codecs/cs35l41-lib.c
@@ -16,6 +16,8 @@
#include <sound/cs35l41.h>
+#define CS35L41_FIRMWARE_OLD_VERSION 0x001C00 /* v0.28.0 */
+
static const struct reg_default cs35l41_reg[] = {
{ CS35L41_PWR_CTRL1, 0x00000000 },
{ CS35L41_PWR_CTRL2, 0x00000000 },
@@ -1214,7 +1216,7 @@ EXPORT_SYMBOL_GPL(cs35l41_safe_reset);
* the PLL Lock interrupt, in the IRQ handler.
*/
int cs35l41_global_enable(struct device *dev, struct regmap *regmap, enum cs35l41_boost_type b_type,
- int enable, bool firmware_running)
+ int enable, struct cs_dsp *dsp)
{
int ret;
unsigned int gpio1_func, pad_control, pwr_ctrl1, pwr_ctrl3, int_status, pup_pdn_mask;
@@ -1309,7 +1311,7 @@ int cs35l41_global_enable(struct device *dev, struct regmap *regmap, enum cs35l4
}
regmap_write(regmap, CS35L41_IRQ1_STATUS1, CS35L41_PUP_DONE_MASK);
- if (firmware_running)
+ if (dsp->running && dsp->fw_id_version > CS35L41_FIRMWARE_OLD_VERSION)
ret = cs35l41_set_cspl_mbox_cmd(dev, regmap,
CSPL_MBOX_CMD_SPK_OUT_ENABLE);
else
diff --git a/sound/soc/codecs/cs35l41.c b/sound/soc/codecs/cs35l41.c
index d0e9128ac6d0..dfb4ce53491b 100644
--- a/sound/soc/codecs/cs35l41.c
+++ b/sound/soc/codecs/cs35l41.c
@@ -519,11 +519,11 @@ static int cs35l41_main_amp_event(struct snd_soc_dapm_widget *w,
ARRAY_SIZE(cs35l41_pup_patch));
ret = cs35l41_global_enable(cs35l41->dev, cs35l41->regmap, cs35l41->hw_cfg.bst_type,
- 1, cs35l41->dsp.cs_dsp.running);
+ 1, &cs35l41->dsp.cs_dsp);
break;
case SND_SOC_DAPM_POST_PMD:
ret = cs35l41_global_enable(cs35l41->dev, cs35l41->regmap, cs35l41->hw_cfg.bst_type,
- 0, cs35l41->dsp.cs_dsp.running);
+ 0, &cs35l41->dsp.cs_dsp);
regmap_multi_reg_write_bypassed(cs35l41->regmap,
cs35l41_pdn_patch,
--
2.34.1
This state duplicates the running state inside cs_dsp, so is not necessary.
Remove it, and use cs_dsp.running instead.
This brings the CS35L41 HDA driver more inline with its ASoC version,
allowing the same state to be used when calling library functions.
Fixes: fa3efcc36aac ("ALSA: cs35l41: Use mbox command to enable speaker output for external boost")
Signed-off-by: Stefan Binding <[email protected]>
---
sound/pci/hda/cs35l41_hda.c | 28 ++++++++++++----------------
1 file changed, 12 insertions(+), 16 deletions(-)
diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c
index b2db8091f0ed..b7d2aa18c566 100644
--- a/sound/pci/hda/cs35l41_hda.c
+++ b/sound/pci/hda/cs35l41_hda.c
@@ -504,7 +504,6 @@ static void cs35l41_shutdown_dsp(struct cs35l41_hda *cs35l41)
cs_dsp_stop(dsp);
cs_dsp_power_down(dsp);
- cs35l41->firmware_running = false;
dev_dbg(cs35l41->dev, "Unloaded Firmware\n");
}
@@ -550,7 +549,7 @@ static void cs35l41_hda_play_start(struct device *dev)
cs35l41->playback_started = true;
- if (cs35l41->firmware_running) {
+ if (cs35l41->cs_dsp.running) {
regmap_multi_reg_write(reg, cs35l41_hda_config_dsp,
ARRAY_SIZE(cs35l41_hda_config_dsp));
regmap_update_bits(reg, CS35L41_PWR_CTRL2,
@@ -580,7 +579,7 @@ static void cs35l41_mute(struct device *dev, bool mute)
regmap_multi_reg_write(reg, cs35l41_hda_mute, ARRAY_SIZE(cs35l41_hda_mute));
} else {
dev_dbg(dev, "Unmuting\n");
- if (cs35l41->firmware_running) {
+ if (cs35l41->cs_dsp.running) {
regmap_multi_reg_write(reg, cs35l41_hda_unmute_dsp,
ARRAY_SIZE(cs35l41_hda_unmute_dsp));
} else {
@@ -599,7 +598,7 @@ static void cs35l41_hda_play_done(struct device *dev)
dev_dbg(dev, "Play (Complete)\n");
cs35l41_global_enable(dev, reg, cs35l41->hw_cfg.bst_type, 1,
- cs35l41->firmware_running);
+ cs35l41->cs_dsp.running);
cs35l41_mute(dev, false);
}
@@ -612,7 +611,7 @@ static void cs35l41_hda_pause_start(struct device *dev)
cs35l41_mute(dev, true);
cs35l41_global_enable(dev, reg, cs35l41->hw_cfg.bst_type, 0,
- cs35l41->firmware_running);
+ cs35l41->cs_dsp.running);
}
static void cs35l41_hda_pause_done(struct device *dev)
@@ -625,7 +624,7 @@ static void cs35l41_hda_pause_done(struct device *dev)
regmap_update_bits(reg, CS35L41_PWR_CTRL2, CS35L41_AMP_EN_MASK, 0 << CS35L41_AMP_EN_SHIFT);
if (cs35l41->hw_cfg.bst_type == CS35L41_EXT_BOOST)
regmap_write(reg, CS35L41_GPIO1_CTRL1, 0x00000001);
- if (cs35l41->firmware_running) {
+ if (cs35l41->cs_dsp.running) {
cs35l41_set_cspl_mbox_cmd(dev, reg, CSPL_MBOX_CMD_PAUSE);
regmap_update_bits(reg, CS35L41_PWR_CTRL2,
CS35L41_VMON_EN_MASK | CS35L41_IMON_EN_MASK,
@@ -675,7 +674,7 @@ static void cs35l41_hda_playback_hook(struct device *dev, int action)
break;
case HDA_GEN_PCM_ACT_CLOSE:
mutex_lock(&cs35l41->fw_mutex);
- if (!cs35l41->firmware_running && cs35l41->request_fw_load &&
+ if (!cs35l41->cs_dsp.running && cs35l41->request_fw_load &&
!cs35l41->fw_request_ongoing) {
dev_info(dev, "Requesting Firmware Load after HDA_GEN_PCM_ACT_CLOSE\n");
cs35l41->fw_request_ongoing = true;
@@ -761,10 +760,9 @@ static int cs35l41_verify_id(struct cs35l41_hda *cs35l41, unsigned int *regid, u
static int cs35l41_ready_for_reset(struct cs35l41_hda *cs35l41)
{
mutex_lock(&cs35l41->fw_mutex);
- if (cs35l41->firmware_running) {
+ if (cs35l41->cs_dsp.running) {
cs35l41->cs_dsp.running = false;
cs35l41->cs_dsp.booted = false;
- cs35l41->firmware_running = false;
}
regcache_mark_dirty(cs35l41->regmap);
mutex_unlock(&cs35l41->fw_mutex);
@@ -925,7 +923,7 @@ static int cs35l41_runtime_suspend(struct device *dev)
mutex_lock(&cs35l41->fw_mutex);
- if (cs35l41->firmware_running) {
+ if (cs35l41->cs_dsp.running) {
ret = cs35l41_enter_hibernate(cs35l41->dev, cs35l41->regmap,
cs35l41->hw_cfg.bst_type);
if (ret)
@@ -960,7 +958,7 @@ static int cs35l41_runtime_resume(struct device *dev)
regcache_cache_only(cs35l41->regmap, false);
- if (cs35l41->firmware_running) {
+ if (cs35l41->cs_dsp.running) {
ret = cs35l41_exit_hibernate(cs35l41->dev, cs35l41->regmap);
if (ret) {
dev_warn(cs35l41->dev, "Unable to exit Hibernate.");
@@ -1052,8 +1050,6 @@ static int cs35l41_smart_amp(struct cs35l41_hda *cs35l41)
goto clean_dsp;
}
- cs35l41->firmware_running = true;
-
return 0;
clean_dsp:
@@ -1063,10 +1059,10 @@ static int cs35l41_smart_amp(struct cs35l41_hda *cs35l41)
static void cs35l41_load_firmware(struct cs35l41_hda *cs35l41, bool load)
{
- if (cs35l41->firmware_running && !load) {
+ if (cs35l41->cs_dsp.running && !load) {
dev_dbg(cs35l41->dev, "Unloading Firmware\n");
cs35l41_shutdown_dsp(cs35l41);
- } else if (!cs35l41->firmware_running && load) {
+ } else if (!cs35l41->cs_dsp.running && load) {
dev_dbg(cs35l41->dev, "Loading Firmware\n");
cs35l41_smart_amp(cs35l41);
} else {
@@ -1346,7 +1342,7 @@ static int cs35l41_hda_bind(struct device *dev, struct device *master, void *mas
cs35l41->acpi_subsystem_id, cs35l41->hw_cfg.bst_type,
cs35l41->hw_cfg.gpio1.func == CS35l41_VSPK_SWITCH,
cs35l41->hw_cfg.spk_pos ? 'R' : 'L',
- cs35l41->firmware_running, cs35l41->speaker_id);
+ cs35l41->cs_dsp.running, cs35l41->speaker_id);
return ret;
}
--
2.34.1
On Fri, 17 Nov 2023 17:36:07 +0100,
Stefan Binding wrote:
>
> Some systems use older firmware which does not support newer commands
> which are used to enable external boost. For those systems, we can
> workaround this by writing the registers directly.
>
> We can use the firmware version, stored inside cs_dsp, to determine
> whether or not the command is supported.
> To achieve this, it requires a cleanup in the api, to pass the cs_dsp
> struct into the function.
>
> We can also remove the redundant boolean firmware_running from the HDA
> driver, and use the equivalent state inside cs_dsp.
>
> Changes since V1:
> - Added fixes tag to all patches
> - patches rebased after v6.7 release
>
> Stefan Binding (2):
> ALSA: hda: cs35l41: Remove unnecessary boolean state variable
> firmware_running
> ALSA: cs35l41: Fix for old systems which do not support command
Both patches applied now. Thanks.
Takashi