[PATCH 1/2] ASoC: wm_adsp: Only use __be32 for big-endian data
This fixes some minor cases where u32 or unsigned int types were used to store big-endian data, and __be32 types used to store both big-endian and cpu-endian data. This was producing sparse warnings.
Most cases resulted from using the same variable to hold the big-endian value and its converted cpu-endian value. These can be simply fixed by introducing another local variable, or avoiding storing the intermediate value back into the original variable.
One special case is the raw_buf used in the compressed streams to transfer data from DSP to user-side. The endian conversion happens in-place (as there's no point introducing another buffer) so a cast to (__be32 *) is added when passing it to wm_adsp_read_raw_data_block().
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com --- sound/soc/codecs/wm_adsp.c | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c index 5b7d81a91df3..8cfa8ac1b8c4 100644 --- a/sound/soc/codecs/wm_adsp.c +++ b/sound/soc/codecs/wm_adsp.c @@ -980,7 +980,7 @@ static int wm_coeff_write_acked_control(struct wm_coeff_ctl *ctl, unsigned int event_id) { struct wm_adsp *dsp = ctl->dsp; - u32 val = cpu_to_be32(event_id); + __be32 val = cpu_to_be32(event_id); unsigned int reg; int i, ret;
@@ -3704,6 +3704,7 @@ static int wm_adsp_write_data_word(struct wm_adsp *dsp, int mem_type, unsigned int mem_addr, u32 data) { struct wm_adsp_region const *mem = wm_adsp_find_region(dsp, mem_type); + __be32 val = cpu_to_be32(data & 0x00ffffffu); unsigned int reg;
if (!mem) @@ -3711,9 +3712,7 @@ static int wm_adsp_write_data_word(struct wm_adsp *dsp, int mem_type,
reg = dsp->ops->region_to_reg(mem, mem_addr);
- data = cpu_to_be32(data & 0x00ffffffu); - - return regmap_raw_write(dsp->regmap, reg, &data, sizeof(data)); + return regmap_raw_write(dsp->regmap, reg, &val, sizeof(val)); }
static inline int wm_adsp_buffer_read(struct wm_adsp_compr_buf *buf, @@ -3870,7 +3869,8 @@ static int wm_adsp_buffer_parse_coeff(struct wm_coeff_ctl *ctl) { struct wm_adsp_host_buf_coeff_v1 coeff_v1; struct wm_adsp_compr_buf *buf; - unsigned int val, reg; + unsigned int reg, version; + __be32 bufp; int ret, i;
ret = wm_coeff_base_reg(ctl, ®); @@ -3878,17 +3878,17 @@ static int wm_adsp_buffer_parse_coeff(struct wm_coeff_ctl *ctl) return ret;
for (i = 0; i < 5; ++i) { - ret = regmap_raw_read(ctl->dsp->regmap, reg, &val, sizeof(val)); + ret = regmap_raw_read(ctl->dsp->regmap, reg, &bufp, sizeof(bufp)); if (ret < 0) return ret;
- if (val) + if (bufp) break;
usleep_range(1000, 2000); }
- if (!val) { + if (!bufp) { adsp_err(ctl->dsp, "Failed to acquire host buffer\n"); return -EIO; } @@ -3898,7 +3898,7 @@ static int wm_adsp_buffer_parse_coeff(struct wm_coeff_ctl *ctl) return -ENOMEM;
buf->host_buf_mem_type = ctl->alg_region.type; - buf->host_buf_ptr = be32_to_cpu(val); + buf->host_buf_ptr = be32_to_cpu(bufp);
ret = wm_adsp_buffer_populate(buf); if (ret < 0) @@ -3918,14 +3918,13 @@ static int wm_adsp_buffer_parse_coeff(struct wm_coeff_ctl *ctl) if (ret < 0) return ret;
- coeff_v1.versions = be32_to_cpu(coeff_v1.versions); - val = coeff_v1.versions & HOST_BUF_COEFF_COMPAT_VER_MASK; - val >>= HOST_BUF_COEFF_COMPAT_VER_SHIFT; + version = be32_to_cpu(coeff_v1.versions) & HOST_BUF_COEFF_COMPAT_VER_MASK; + version >>= HOST_BUF_COEFF_COMPAT_VER_SHIFT;
- if (val > HOST_BUF_COEFF_SUPPORTED_COMPAT_VER) { + if (version > HOST_BUF_COEFF_SUPPORTED_COMPAT_VER) { adsp_err(ctl->dsp, "Host buffer coeff ver %u > supported version %u\n", - val, HOST_BUF_COEFF_SUPPORTED_COMPAT_VER); + version, HOST_BUF_COEFF_SUPPORTED_COMPAT_VER); return -EINVAL; }
@@ -3935,9 +3934,9 @@ static int wm_adsp_buffer_parse_coeff(struct wm_coeff_ctl *ctl) (char *)&coeff_v1.name);
compr_dbg(buf, "host_buf_ptr=%x coeff version %u\n", - buf->host_buf_ptr, val); + buf->host_buf_ptr, version);
- return val; + return version; }
static int wm_adsp_buffer_init(struct wm_adsp *dsp) @@ -4269,7 +4268,7 @@ static int wm_adsp_buffer_capture_block(struct wm_adsp_compr *compr, int target)
/* Read data from DSP */ ret = wm_adsp_read_raw_data_block(buf->dsp, mem_type, adsp_addr, - nwords, compr->raw_buf); + nwords, (__be32 *)compr->raw_buf); if (ret < 0) return ret;
Sparse will complain about trying to convert between values declared as snd_ctl_elem_type_t and other types. This patch converts to consistently use snd_ctl_elem_type_t for control type values. A __force cast is needed in a couple of cases where the control type value is parsed out of a DSP data block.
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com --- sound/soc/codecs/wm_adsp.c | 12 +++++++----- sound/soc/codecs/wmfw.h | 6 +++--- 2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c index 8cfa8ac1b8c4..f8ad768364c2 100644 --- a/sound/soc/codecs/wm_adsp.c +++ b/sound/soc/codecs/wm_adsp.c @@ -619,7 +619,7 @@ struct wm_coeff_ctl { unsigned int set:1; struct soc_bytes_ext bytes_ext; unsigned int flags; - unsigned int type; + snd_ctl_elem_type_t type; };
static const char *wm_adsp_mem_region_name(unsigned int type) @@ -1420,7 +1420,7 @@ static int wm_adsp_create_control(struct wm_adsp *dsp, const struct wm_adsp_alg_region *alg_region, unsigned int offset, unsigned int len, const char *subname, unsigned int subname_len, - unsigned int flags, unsigned int type) + unsigned int flags, snd_ctl_elem_type_t type) { struct wm_coeff_ctl *ctl; struct wmfw_ctl_work *ctl_work; @@ -1554,7 +1554,7 @@ struct wm_coeff_parsed_coeff { int mem_type; const u8 *name; int name_len; - int ctl_type; + snd_ctl_elem_type_t ctl_type; int flags; int len; }; @@ -1649,7 +1649,7 @@ static inline void wm_coeff_parse_coeff(struct wm_adsp *dsp, const u8 **data, blk->mem_type = le16_to_cpu(raw->hdr.type); blk->name = raw->name; blk->name_len = strlen(raw->name); - blk->ctl_type = le16_to_cpu(raw->ctl_type); + blk->ctl_type = (__force snd_ctl_elem_type_t)le16_to_cpu(raw->ctl_type); blk->flags = le16_to_cpu(raw->flags); blk->len = le32_to_cpu(raw->len); break; @@ -1662,7 +1662,9 @@ static inline void wm_coeff_parse_coeff(struct wm_adsp *dsp, const u8 **data, &blk->name); wm_coeff_parse_string(sizeof(u8), &tmp, NULL); wm_coeff_parse_string(sizeof(u16), &tmp, NULL); - blk->ctl_type = wm_coeff_parse_int(sizeof(raw->ctl_type), &tmp); + blk->ctl_type = + (__force snd_ctl_elem_type_t)wm_coeff_parse_int(sizeof(raw->ctl_type), + &tmp); blk->flags = wm_coeff_parse_int(sizeof(raw->flags), &tmp); blk->len = wm_coeff_parse_int(sizeof(raw->len), &tmp);
diff --git a/sound/soc/codecs/wmfw.h b/sound/soc/codecs/wmfw.h index 7423272c30e9..f3d51602f85c 100644 --- a/sound/soc/codecs/wmfw.h +++ b/sound/soc/codecs/wmfw.h @@ -24,9 +24,9 @@ #define WMFW_CTL_FLAG_READABLE 0x0001
/* Non-ALSA coefficient types start at 0x1000 */ -#define WMFW_CTL_TYPE_ACKED 0x1000 /* acked control */ -#define WMFW_CTL_TYPE_HOSTEVENT 0x1001 /* event control */ -#define WMFW_CTL_TYPE_HOST_BUFFER 0x1002 /* host buffer pointer */ +#define WMFW_CTL_TYPE_ACKED ((__force snd_ctl_elem_type_t)0x1000) /* acked control */ +#define WMFW_CTL_TYPE_HOSTEVENT ((__force snd_ctl_elem_type_t)0x1001) /* event control */ +#define WMFW_CTL_TYPE_HOST_BUFFER ((__force snd_ctl_elem_type_t)0x1002) /* host buffer pointer */
struct wmfw_header { char magic[4];
On Wed, 30 Dec 2020 17:24:26 +0000, Richard Fitzgerald wrote:
This fixes some minor cases where u32 or unsigned int types were used to store big-endian data, and __be32 types used to store both big-endian and cpu-endian data. This was producing sparse warnings.
Most cases resulted from using the same variable to hold the big-endian value and its converted cpu-endian value. These can be simply fixed by introducing another local variable, or avoiding storing the intermediate value back into the original variable.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/2] ASoC: wm_adsp: Only use __be32 for big-endian data commit: a0b653e89a3afd2a833f23589a83488fac842ddb [2/2] ASoC: wm_adsp: Use snd_ctl_elem_type_t for control types commit: f6212e0ab3ff28d4e2e53a520496a052c241df39
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)
-
Mark Brown
-
Richard Fitzgerald