2023-07-31 17:58:07

by Richard Fitzgerald

[permalink] [raw]
Subject: [PATCH 0/9] ALSA: hda/cs35l56: Various bugfixes

A collection of various bugfixes to the cs35l56 driver.

Richard Fitzgerald (9):
ALSA: hda/cs35l56: Complete firmware reboot before calling
cs_dsp_run()
ALSA: hda/cs35l56: Do not mark cache dirty after REINIT
ALSA: hda/cs35l56: Call cs_dsp_power_down() before reloading firmware
ALSA: hda/cs35l56: Always power-up and start cs_dsp
ALSA: hda/cs35l56: Call cs_dsp_power_down() before calling
cs_dsp_remove()
ALSA: hda/cs35l56: cs_dsp_power_down() on cs35l56_hda_fw_load() error
path
ALSA: hda/cs35l56: Do not download firmware over existing RAM firmware
ALSA: hda/cs35l56: Fail if .bin not found and firmware not patched
ALSA: hda/cs35l56: Reject I2C alias addresses

sound/pci/hda/cs35l56_hda.c | 91 ++++++++++++++++++++++++++-----------
1 file changed, 65 insertions(+), 26 deletions(-)

--
2.30.2



2023-07-31 18:15:33

by Richard Fitzgerald

[permalink] [raw]
Subject: [PATCH 6/9] ALSA: hda/cs35l56: cs_dsp_power_down() on cs35l56_hda_fw_load() error path

If cs35l56_hda_fw_load() successfully called cs_dsp_power_up() the error
path must balance that with a call to cs_dsp_power_down().

Signed-off-by: Richard Fitzgerald <[email protected]>
---
sound/pci/hda/cs35l56_hda.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/sound/pci/hda/cs35l56_hda.c b/sound/pci/hda/cs35l56_hda.c
index e8c41a4a0c40..803fa2da9ea4 100644
--- a/sound/pci/hda/cs35l56_hda.c
+++ b/sound/pci/hda/cs35l56_hda.c
@@ -567,20 +567,20 @@ static int cs35l56_hda_fw_load(struct cs35l56_hda *cs35l56)
if (cs35l56->base.secured) {
ret = cs35l56_mbox_send(&cs35l56->base, CS35L56_MBOX_CMD_AUDIO_REINIT);
if (ret)
- goto err;
+ goto err_powered_up;
} else if (wmfw_firmware || coeff_firmware) {
/* If we downloaded firmware, reset the device and wait for it to boot */
cs35l56_system_reset(&cs35l56->base, false);
regcache_mark_dirty(cs35l56->base.regmap);
ret = cs35l56_wait_for_firmware_boot(&cs35l56->base);
if (ret)
- goto err;
+ goto err_powered_up;
}

/* 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;
+ goto err_powered_up;

regcache_sync(cs35l56->base.regmap);

@@ -592,6 +592,9 @@ static int cs35l56_hda_fw_load(struct cs35l56_hda *cs35l56)
if (ret)
dev_dbg(cs35l56->base.dev, "%s: cs_dsp_run ret %d\n", __func__, ret);

+err_powered_up:
+ if (!cs35l56->base.fw_patched)
+ cs_dsp_power_down(&cs35l56->cs_dsp);
err:
pm_runtime_put(cs35l56->base.dev);
mutex_unlock(&cs35l56->base.irq_lock);
--
2.30.2


2023-07-31 19:14:10

by Richard Fitzgerald

[permalink] [raw]
Subject: [PATCH 7/9] ALSA: hda/cs35l56: Do not download firmware over existing RAM firmware

A RAM firmware can only be downloaded if the CS35L56 is currently
running from ROM firmware. The driver must not try to overwrite
the RAM if the CS35L56 is already running from that RAM.

Firmware can be downloaded in these two cases:

- The BIOS has already patched the firmware (secured mode).
In this case the firmware files will only contain tunings that
are safe to overwrite.

- The CS35L56 is running the built-in ROM firmware.

After a RAM firmware has been downloaded it can only be cleared by
hard resetting CS35L56. Some systems only hard-reset during
power-on and do not give the driver control of hard reset.

Signed-off-by: Richard Fitzgerald <[email protected]>
---
sound/pci/hda/cs35l56_hda.c | 27 +++++++++++++++++++++++----
1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/sound/pci/hda/cs35l56_hda.c b/sound/pci/hda/cs35l56_hda.c
index 803fa2da9ea4..8f1665d38c92 100644
--- a/sound/pci/hda/cs35l56_hda.c
+++ b/sound/pci/hda/cs35l56_hda.c
@@ -525,6 +525,7 @@ static int cs35l56_hda_fw_load(struct cs35l56_hda *cs35l56)
const struct firmware *wmfw_firmware = NULL;
char *coeff_filename = NULL;
char *wmfw_filename = NULL;
+ unsigned int firmware_missing;
int ret = 0;

/* Prepare for a new DSP power-up */
@@ -533,11 +534,28 @@ static int cs35l56_hda_fw_load(struct cs35l56_hda *cs35l56)

cs35l56->base.fw_patched = false;

- cs35l56_hda_request_firmware_files(cs35l56, &wmfw_firmware, &wmfw_filename,
- &coeff_firmware, &coeff_filename);
+ pm_runtime_get_sync(cs35l56->base.dev);
+
+ ret = regmap_read(cs35l56->base.regmap, CS35L56_PROTECTION_STATUS, &firmware_missing);
+ if (ret) {
+ dev_err(cs35l56->base.dev, "Failed to read PROTECTION_STATUS: %d\n", ret);
+ goto err_pm_put;
+ }
+
+ firmware_missing &= CS35L56_FIRMWARE_MISSING;
+
+ /*
+ * Firmware can only be downloaded if the CS35L56 is secured or is
+ * running from the built-in ROM. If it is secured the BIOS will have
+ * downloaded firmware, and the wmfw/bin files will only contain
+ * tunings that are safe to download with the firmware running.
+ */
+ if (cs35l56->base.secured || firmware_missing) {
+ cs35l56_hda_request_firmware_files(cs35l56, &wmfw_firmware, &wmfw_filename,
+ &coeff_firmware, &coeff_filename);
+ }

mutex_lock(&cs35l56->base.irq_lock);
- pm_runtime_get_sync(cs35l56->base.dev);

/*
* When the device is running in secure mode the firmware files can
@@ -596,11 +614,12 @@ static int cs35l56_hda_fw_load(struct cs35l56_hda *cs35l56)
if (!cs35l56->base.fw_patched)
cs_dsp_power_down(&cs35l56->cs_dsp);
err:
- pm_runtime_put(cs35l56->base.dev);
mutex_unlock(&cs35l56->base.irq_lock);

cs35l56_hda_release_firmware_files(wmfw_firmware, wmfw_filename,
coeff_firmware, coeff_filename);
+err_pm_put:
+ pm_runtime_put(cs35l56->base.dev);

return ret;
}
--
2.30.2


2023-07-31 19:18:59

by Richard Fitzgerald

[permalink] [raw]
Subject: [PATCH 2/9] ALSA: hda/cs35l56: Do not mark cache dirty after REINIT

Only call regcache_mark_dirty() in cs35l56_hda_fw_load() if
the CS35L56 was SYSTEM_RESET.

recache_mark_dirty() changes the behaviour of regcache_sync()
to write out cache values that are not the default value, and
skip cache values that are the default.

AUDIO_REINIT does not reset the registers. regcache_mark_dirty()
after AUDIO_REINIT could cause the regcache_sync() to sync
registers incorrectly because it will assume that all registers
have reset to default.

Signed-off-by: Richard Fitzgerald <[email protected]>
---
sound/pci/hda/cs35l56_hda.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/pci/hda/cs35l56_hda.c b/sound/pci/hda/cs35l56_hda.c
index c3acda2daeeb..fda716e0db09 100644
--- a/sound/pci/hda/cs35l56_hda.c
+++ b/sound/pci/hda/cs35l56_hda.c
@@ -569,6 +569,7 @@ static int cs35l56_hda_fw_load(struct cs35l56_hda *cs35l56)
} else {
/* Reset the device and wait for it to boot */
cs35l56_system_reset(&cs35l56->base, false);
+ regcache_mark_dirty(cs35l56->base.regmap);
ret = cs35l56_wait_for_firmware_boot(&cs35l56->base);
if (ret)
goto err;
@@ -579,7 +580,6 @@ static int cs35l56_hda_fw_load(struct cs35l56_hda *cs35l56)
if (ret)
goto err;

- regcache_mark_dirty(cs35l56->base.regmap);
regcache_sync(cs35l56->base.regmap);

regmap_clear_bits(cs35l56->base.regmap, CS35L56_PROTECTION_STATUS,
--
2.30.2


2023-07-31 20:10:12

by Richard Fitzgerald

[permalink] [raw]
Subject: [PATCH 3/9] ALSA: hda/cs35l56: Call cs_dsp_power_down() before reloading firmware

When firmware is reloaded after a system resume cs_dsp_power_down() should
be called before calling cs_dsp_power_up().

The fw_patched flag should also be cleared and only set again if the
firmware download succeeded.

Signed-off-by: Richard Fitzgerald <[email protected]>
---
sound/pci/hda/cs35l56_hda.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/sound/pci/hda/cs35l56_hda.c b/sound/pci/hda/cs35l56_hda.c
index fda716e0db09..b6b8cb21da75 100644
--- a/sound/pci/hda/cs35l56_hda.c
+++ b/sound/pci/hda/cs35l56_hda.c
@@ -527,6 +527,12 @@ static int cs35l56_hda_fw_load(struct cs35l56_hda *cs35l56)
char *wmfw_filename = NULL;
int ret = 0;

+ /* Prepare for a new DSP power-up */
+ if (cs35l56->base.fw_patched)
+ cs_dsp_power_down(&cs35l56->cs_dsp);
+
+ cs35l56->base.fw_patched = false;
+
cs35l56_hda_request_firmware_files(cs35l56, &wmfw_firmware, &wmfw_filename,
&coeff_firmware, &coeff_filename);

--
2.30.2


2023-08-01 07:31:34

by Takashi Iwai

[permalink] [raw]
Subject: Re: [PATCH 0/9] ALSA: hda/cs35l56: Various bugfixes

On Mon, 31 Jul 2023 18:57:17 +0200,
Richard Fitzgerald wrote:
>
> A collection of various bugfixes to the cs35l56 driver.
>
> Richard Fitzgerald (9):
> ALSA: hda/cs35l56: Complete firmware reboot before calling
> cs_dsp_run()
> ALSA: hda/cs35l56: Do not mark cache dirty after REINIT
> ALSA: hda/cs35l56: Call cs_dsp_power_down() before reloading firmware
> ALSA: hda/cs35l56: Always power-up and start cs_dsp
> ALSA: hda/cs35l56: Call cs_dsp_power_down() before calling
> cs_dsp_remove()
> ALSA: hda/cs35l56: cs_dsp_power_down() on cs35l56_hda_fw_load() error
> path
> ALSA: hda/cs35l56: Do not download firmware over existing RAM firmware
> ALSA: hda/cs35l56: Fail if .bin not found and firmware not patched
> ALSA: hda/cs35l56: Reject I2C alias addresses

Applied all patches now. Thanks.


Takashi