[PATCH 1/3] ASoC: wm_adsp: Correct control read size when parsing compressed buffer
When parsing the compressed stream the whole buffer descriptor is now read in a single cs_dsp_coeff_read_ctrl; on older firmwares this descriptor is just 4 bytes but on more modern firmwares it is 24 bytes. The current code reads the full 24 bytes regardless, this was working but reading junk for the last 20 bytes. However commit f444da38ac92 ("firmware: cs_dsp: Add offset to cs_dsp read/write") added a size check into cs_dsp_coeff_read_ctrl, causing the older firmwares to now return an error.
Update the code to only read the amount of data appropriate for the firmware loaded.
Fixes: 04ae08596737 ("ASoC: wm_adsp: Switch to using wm_coeff_read_ctrl for compressed buffers") Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com --- sound/soc/codecs/wm_adsp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c index f3672e3d1703e..0582585236a28 100644 --- a/sound/soc/codecs/wm_adsp.c +++ b/sound/soc/codecs/wm_adsp.c @@ -1441,7 +1441,8 @@ static int wm_adsp_buffer_parse_coeff(struct cs_dsp_coeff_ctl *cs_ctl) int ret, i;
for (i = 0; i < 5; ++i) { - ret = cs_dsp_coeff_read_ctrl(cs_ctl, 0, &coeff_v1, sizeof(coeff_v1)); + ret = cs_dsp_coeff_read_ctrl(cs_ctl, 0, &coeff_v1, + min(cs_ctl->len, sizeof(coeff_v1))); if (ret < 0) return ret;
Newer firmwares will support compressed buffers that may or may not exist, for example debugging streams. Update the driver to make a compressed stream optional. A warning will still be generated at DSP boot time and opening the stream will fail if the compressed buffer in question does not exist, however the DSP can still be booted and other features used.
Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com --- sound/soc/codecs/wm_adsp.c | 54 ++++++++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 21 deletions(-)
diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c index 0582585236a28..8e5ebc4838ec3 100644 --- a/sound/soc/codecs/wm_adsp.c +++ b/sound/soc/codecs/wm_adsp.c @@ -1373,8 +1373,6 @@ static struct wm_adsp_compr_buf *wm_adsp_buffer_alloc(struct wm_adsp *dsp)
wm_adsp_buffer_clear(buf);
- list_add_tail(&buf->list, &dsp->buffer_list); - return buf; }
@@ -1391,10 +1389,6 @@ static int wm_adsp_buffer_parse_legacy(struct wm_adsp *dsp) return -EINVAL; }
- buf = wm_adsp_buffer_alloc(dsp); - if (!buf) - return -ENOMEM; - xmalg = dsp->sys_config_size / sizeof(__be32);
addr = alg_region->base + xmalg + ALG_XM_FIELD(magic); @@ -1405,12 +1399,16 @@ static int wm_adsp_buffer_parse_legacy(struct wm_adsp *dsp) if (magic != WM_ADSP_ALG_XM_STRUCT_MAGIC) return -ENODEV;
+ buf = wm_adsp_buffer_alloc(dsp); + if (!buf) + return -ENOMEM; + addr = alg_region->base + xmalg + ALG_XM_FIELD(host_buf_ptr); for (i = 0; i < 5; ++i) { ret = cs_dsp_read_data_word(&dsp->cs_dsp, WMFW_ADSP2_XM, addr, &buf->host_buf_ptr); if (ret < 0) - return ret; + goto err;
if (buf->host_buf_ptr) break; @@ -1418,18 +1416,27 @@ static int wm_adsp_buffer_parse_legacy(struct wm_adsp *dsp) usleep_range(1000, 2000); }
- if (!buf->host_buf_ptr) - return -EIO; + if (!buf->host_buf_ptr) { + ret = -EIO; + goto err; + }
buf->host_buf_mem_type = WMFW_ADSP2_XM;
ret = wm_adsp_buffer_populate(buf); if (ret < 0) - return ret; + goto err; + + list_add_tail(&buf->list, &dsp->buffer_list);
compr_dbg(buf, "legacy host_buf_ptr=%x\n", buf->host_buf_ptr);
return 0; + +err: + kfree(buf); + + return ret; }
static int wm_adsp_buffer_parse_coeff(struct cs_dsp_coeff_ctl *cs_ctl) @@ -1437,7 +1444,7 @@ static int wm_adsp_buffer_parse_coeff(struct cs_dsp_coeff_ctl *cs_ctl) struct wm_adsp_host_buf_coeff_v1 coeff_v1; struct wm_adsp_compr_buf *buf; struct wm_adsp *dsp = container_of(cs_ctl->dsp, struct wm_adsp, cs_dsp); - unsigned int version; + unsigned int version = 0; int ret, i;
for (i = 0; i < 5; ++i) { @@ -1466,16 +1473,14 @@ static int wm_adsp_buffer_parse_coeff(struct cs_dsp_coeff_ctl *cs_ctl)
ret = wm_adsp_buffer_populate(buf); if (ret < 0) - return ret; + goto err;
/* * v0 host_buffer coefficients didn't have versioning, so if the * control is one word, assume version 0. */ - if (cs_ctl->len == 4) { - compr_dbg(buf, "host_buf_ptr=%x\n", buf->host_buf_ptr); - return 0; - } + if (cs_ctl->len == 4) + goto done;
version = be32_to_cpu(coeff_v1.versions) & HOST_BUF_COEFF_COMPAT_VER_MASK; version >>= HOST_BUF_COEFF_COMPAT_VER_SHIFT; @@ -1484,7 +1489,8 @@ static int wm_adsp_buffer_parse_coeff(struct cs_dsp_coeff_ctl *cs_ctl) adsp_err(dsp, "Host buffer coeff ver %u > supported version %u\n", version, HOST_BUF_COEFF_SUPPORTED_COMPAT_VER); - return -EINVAL; + ret = -EINVAL; + goto err; }
cs_dsp_remove_padding((u32 *)&coeff_v1.name, ARRAY_SIZE(coeff_v1.name)); @@ -1492,10 +1498,18 @@ static int wm_adsp_buffer_parse_coeff(struct cs_dsp_coeff_ctl *cs_ctl) buf->name = kasprintf(GFP_KERNEL, "%s-dsp-%s", dsp->part, (char *)&coeff_v1.name);
+done: + list_add_tail(&buf->list, &dsp->buffer_list); + compr_dbg(buf, "host_buf_ptr=%x coeff version %u\n", buf->host_buf_ptr, version);
return version; + +err: + kfree(buf); + + return ret; }
static int wm_adsp_buffer_init(struct wm_adsp *dsp) @@ -1523,10 +1537,8 @@ static int wm_adsp_buffer_init(struct wm_adsp *dsp) if (list_empty(&dsp->buffer_list)) { /* Fall back to legacy support */ ret = wm_adsp_buffer_parse_legacy(dsp); - if (ret) { - adsp_err(dsp, "Failed to parse legacy: %d\n", ret); - goto error; - } + if (ret) + adsp_warn(dsp, "Failed to parse legacy: %d\n", ret); }
return 0;
From: Vlad Karpovich Vlad.Karpovich@cirrus.com
Enable access to the speaker protection firmware debug stream using compress stream API and lower minimum fragment size to 16 words.
Signed-off-by: Vlad Karpovich Vlad.Karpovich@cirrus.com Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com --- sound/soc/codecs/wm_adsp.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c index 8e5ebc4838ec3..7a32efa1fd90e 100644 --- a/sound/soc/codecs/wm_adsp.c +++ b/sound/soc/codecs/wm_adsp.c @@ -180,7 +180,7 @@ struct wm_adsp_compr {
#define WM_ADSP_MIN_FRAGMENTS 1 #define WM_ADSP_MAX_FRAGMENTS 256 -#define WM_ADSP_MIN_FRAGMENT_SIZE (64 * CS_DSP_DATA_WORD_SIZE) +#define WM_ADSP_MIN_FRAGMENT_SIZE (16 * CS_DSP_DATA_WORD_SIZE) #define WM_ADSP_MAX_FRAGMENT_SIZE (4096 * CS_DSP_DATA_WORD_SIZE)
#define WM_ADSP_ALG_XM_STRUCT_MAGIC 0x49aec7 @@ -296,7 +296,12 @@ static const struct { .num_caps = ARRAY_SIZE(trace_caps), .caps = trace_caps, }, - [WM_ADSP_FW_SPK_PROT] = { .file = "spk-prot" }, + [WM_ADSP_FW_SPK_PROT] = { + .file = "spk-prot", + .compr_direction = SND_COMPRESS_CAPTURE, + .num_caps = ARRAY_SIZE(trace_caps), + .caps = trace_caps, + }, [WM_ADSP_FW_SPK_CALI] = { .file = "spk-cali" }, [WM_ADSP_FW_SPK_DIAG] = { .file = "spk-diag" }, [WM_ADSP_FW_MISC] = { .file = "misc" },
On Thu, 10 Feb 2022 17:20:51 +0000, Charles Keepax wrote:
When parsing the compressed stream the whole buffer descriptor is now read in a single cs_dsp_coeff_read_ctrl; on older firmwares this descriptor is just 4 bytes but on more modern firmwares it is 24 bytes. The current code reads the full 24 bytes regardless, this was working but reading junk for the last 20 bytes. However commit f444da38ac92 ("firmware: cs_dsp: Add offset to cs_dsp read/write") added a size check into cs_dsp_coeff_read_ctrl, causing the older firmwares to now return an error.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-linus
Thanks!
[1/3] ASoC: wm_adsp: Correct control read size when parsing compressed buffer commit: a887f9c7a4d37a8e874ba8415a42a92a1b5139fc
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
On Thu, 10 Feb 2022 17:20:51 +0000, Charles Keepax wrote:
When parsing the compressed stream the whole buffer descriptor is now read in a single cs_dsp_coeff_read_ctrl; on older firmwares this descriptor is just 4 bytes but on more modern firmwares it is 24 bytes. The current code reads the full 24 bytes regardless, this was working but reading junk for the last 20 bytes. However commit f444da38ac92 ("firmware: cs_dsp: Add offset to cs_dsp read/write") added a size check into cs_dsp_coeff_read_ctrl, causing the older firmwares to now return an error.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[2/3] ASoC: wm_adsp: Make compressed buffers optional commit: 0f1d41a85bda6f3502634fe15fa21bfee4c668a4 [3/3] ASoC: wm_adsp: Add trace caps to speaker protection FW commit: c55b3e46cb99a8342cad9c1a35485bfe15187832
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
participants (2)
-
Charles Keepax -
Mark Brown