[PATCH 00/13] ADSP Firmware Controls Update

[PATCH 00/13] ADSP Firmware Controls Update

[Charles Keepax]

This series of patches starts with a few small clean ups for the
ADSP code, and then finally adds support for the rev 1 and 2
firmware file formats. The revision 1 format is in common usage
in the field so it is long overdue to get support for that merged
to mainline. The revision 2 format is fairly new but is starting
to see some deployment as well.

There is still more work todo here. The main next step is to add
support for arbitary length controls through the TLV binary
control feature that was recently added to the kernel. This patch
series will truncate any long controls to 512 bytes.

Thanks,
Charles

Charles Keepax (13):
  ASoC: wm_adsp: Split out adsp1 & 2 setup algorithms
  ASoC: wm_adsp: Improve variable naming
  ASoC: wm_adsp: Remove len field from wm_adsp_alg_region
  ASoC: wm_adsp: Limit firmware control name to ALSA control name size
  ASoC: wm_adsp: Move temporary control name to the stack
  ASoC: wm_adsp: Clean up low level control read/write functions
  ASoC: wm_adsp: Factor out creation of alg_regions
  ASoC: wm_adsp: Remove private field from wm_coeff_ctl
  ASoC: wm_adsp: Group all the ALSA control functions together
  ASoC: wm_adsp: Add basic support for rev 1 firmware file format
  ASoC: wm_adsp: Add support for DSP control flags
  ASoC: wm_adsp: Add support for rev 2 firmware file format
  ASoC: wm_adsp: Warn that firmware file format 0 is depreciated

 sound/soc/codecs/wm_adsp.c | 1129 +++++++++++++++++++++++++++-----------------
 sound/soc/codecs/wm_adsp.h |    6 +-
 sound/soc/codecs/wmfw.h    |   44 ++-
 3 files changed, 733 insertions(+), 446 deletions(-)

-- 
1.7.2.5

[PATCH 01/13] ASoC: wm_adsp: Split out adsp1 & 2 setup algorithms

[Charles Keepax]

The vast majority of the wm_adsp_setup_algs function is case statements
for ADSP1 or ADSP2, this patch splits this out into two separate
functions wm_adsp1_setup_algs and wm_adsp2_setup_algs. The small amount
of shared code between them is factored out into an extra helper
function. This makes the code a lot cleaner.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm_adsp.c |  499 ++++++++++++++++++++++----------------------
 1 files changed, 248 insertions(+), 251 deletions(-)

diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index d01c209..f421c09 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -876,298 +876,295 @@ err_name:
 	return ret;
 }
 
-static int wm_adsp_setup_algs(struct wm_adsp *dsp)
+static void *wm_adsp_read_algs(struct wm_adsp *dsp, size_t algs,
+			       unsigned int pos, unsigned int len)
 {
-	struct regmap *regmap = dsp->regmap;
-	struct wmfw_adsp1_id_hdr adsp1_id;
-	struct wmfw_adsp2_id_hdr adsp2_id;
-	struct wmfw_adsp1_alg_hdr *adsp1_alg;
-	struct wmfw_adsp2_alg_hdr *adsp2_alg;
-	void *alg, *buf;
-	struct wm_adsp_alg_region *region;
-	const struct wm_adsp_region *mem;
-	unsigned int pos, term;
-	size_t algs, buf_size;
+	void *alg;
+	int ret;
 	__be32 val;
-	int i, ret;
 
-	switch (dsp->type) {
-	case WMFW_ADSP1:
-		mem = wm_adsp_find_region(dsp, WMFW_ADSP1_DM);
-		break;
-	case WMFW_ADSP2:
-		mem = wm_adsp_find_region(dsp, WMFW_ADSP2_XM);
-		break;
-	default:
-		mem = NULL;
-		break;
+	if (algs == 0) {
+		adsp_err(dsp, "No algorithms\n");
+		return ERR_PTR(-EINVAL);
 	}
 
-	if (WARN_ON(!mem))
-		return -EINVAL;
+	if (algs > 1024) {
+		adsp_err(dsp, "Algorithm count %zx excessive\n", algs);
+		return ERR_PTR(-EINVAL);
+	}
 
-	switch (dsp->type) {
-	case WMFW_ADSP1:
-		ret = regmap_raw_read(regmap, mem->base, &adsp1_id,
-				      sizeof(adsp1_id));
-		if (ret != 0) {
-			adsp_err(dsp, "Failed to read algorithm info: %d\n",
-				 ret);
-			return ret;
-		}
+	/* Read the terminator first to validate the length */
+	ret = regmap_raw_read(dsp->regmap, pos + len, &val, sizeof(val));
+	if (ret != 0) {
+		adsp_err(dsp, "Failed to read algorithm list end: %d\n",
+			ret);
+		return ERR_PTR(ret);
+	}
 
-		buf = &adsp1_id;
-		buf_size = sizeof(adsp1_id);
+	if (be32_to_cpu(val) != 0xbedead)
+		adsp_warn(dsp, "Algorithm list end %x 0x%x != 0xbeadead\n",
+			  pos + len, be32_to_cpu(val));
 
-		algs = be32_to_cpu(adsp1_id.algs);
-		dsp->fw_id = be32_to_cpu(adsp1_id.fw.id);
-		adsp_info(dsp, "Firmware: %x v%d.%d.%d, %zu algorithms\n",
-			  dsp->fw_id,
-			  (be32_to_cpu(adsp1_id.fw.ver) & 0xff0000) >> 16,
-			  (be32_to_cpu(adsp1_id.fw.ver) & 0xff00) >> 8,
-			  be32_to_cpu(adsp1_id.fw.ver) & 0xff,
-			  algs);
+	alg = kzalloc(len * 2, GFP_KERNEL | GFP_DMA);
+	if (!alg)
+		return ERR_PTR(-ENOMEM);
 
-		region = kzalloc(sizeof(*region), GFP_KERNEL);
-		if (!region)
-			return -ENOMEM;
-		region->type = WMFW_ADSP1_ZM;
-		region->alg = be32_to_cpu(adsp1_id.fw.id);
-		region->base = be32_to_cpu(adsp1_id.zm);
-		list_add_tail(&region->list, &dsp->alg_regions);
+	ret = regmap_raw_read(dsp->regmap, pos, alg, len * 2);
+	if (ret != 0) {
+		adsp_err(dsp, "Failed to read algorithm list: %d\n",
+			ret);
+		kfree(alg);
+		return ERR_PTR(ret);
+	}
 
-		region = kzalloc(sizeof(*region), GFP_KERNEL);
-		if (!region)
-			return -ENOMEM;
-		region->type = WMFW_ADSP1_DM;
-		region->alg = be32_to_cpu(adsp1_id.fw.id);
-		region->base = be32_to_cpu(adsp1_id.dm);
-		list_add_tail(&region->list, &dsp->alg_regions);
+	return alg;
+}
 
-		pos = sizeof(adsp1_id) / 2;
-		term = pos + ((sizeof(*adsp1_alg) * algs) / 2);
-		break;
+static int wm_adsp1_setup_algs(struct wm_adsp *dsp)
+{
+	struct wmfw_adsp1_id_hdr adsp1_id;
+	struct wmfw_adsp1_alg_hdr *adsp1_alg;
+	struct wm_adsp_alg_region *region;
+	const struct wm_adsp_region *mem;
+	unsigned int pos, len;
+	size_t algs;
+	int i, ret;
 
-	case WMFW_ADSP2:
-		ret = regmap_raw_read(regmap, mem->base, &adsp2_id,
-				      sizeof(adsp2_id));
-		if (ret != 0) {
-			adsp_err(dsp, "Failed to read algorithm info: %d\n",
-				 ret);
-			return ret;
-		}
+	mem = wm_adsp_find_region(dsp, WMFW_ADSP1_DM);
+	if (WARN_ON(!mem))
+		return -EINVAL;
+
+	ret = regmap_raw_read(dsp->regmap, mem->base, &adsp1_id,
+			      sizeof(adsp1_id));
+	if (ret != 0) {
+		adsp_err(dsp, "Failed to read algorithm info: %d\n",
+			 ret);
+		return ret;
+	}
 
-		buf = &adsp2_id;
-		buf_size = sizeof(adsp2_id);
+	algs = be32_to_cpu(adsp1_id.algs);
+	dsp->fw_id = be32_to_cpu(adsp1_id.fw.id);
+	adsp_info(dsp, "Firmware: %x v%d.%d.%d, %zu algorithms\n",
+		  dsp->fw_id,
+		  (be32_to_cpu(adsp1_id.fw.ver) & 0xff0000) >> 16,
+		  (be32_to_cpu(adsp1_id.fw.ver) & 0xff00) >> 8,
+		  be32_to_cpu(adsp1_id.fw.ver) & 0xff,
+		  algs);
+
+	region = kzalloc(sizeof(*region), GFP_KERNEL);
+	if (!region)
+		return -ENOMEM;
+	region->type = WMFW_ADSP1_ZM;
+	region->alg = be32_to_cpu(adsp1_id.fw.id);
+	region->base = be32_to_cpu(adsp1_id.zm);
+	list_add_tail(&region->list, &dsp->alg_regions);
 
-		algs = be32_to_cpu(adsp2_id.algs);
-		dsp->fw_id = be32_to_cpu(adsp2_id.fw.id);
-		adsp_info(dsp, "Firmware: %x v%d.%d.%d, %zu algorithms\n",
-			  dsp->fw_id,
-			  (be32_to_cpu(adsp2_id.fw.ver) & 0xff0000) >> 16,
-			  (be32_to_cpu(adsp2_id.fw.ver) & 0xff00) >> 8,
-			  be32_to_cpu(adsp2_id.fw.ver) & 0xff,
-			  algs);
+	region = kzalloc(sizeof(*region), GFP_KERNEL);
+	if (!region)
+		return -ENOMEM;
+	region->type = WMFW_ADSP1_DM;
+	region->alg = be32_to_cpu(adsp1_id.fw.id);
+	region->base = be32_to_cpu(adsp1_id.dm);
+	list_add_tail(&region->list, &dsp->alg_regions);
 
-		region = kzalloc(sizeof(*region), GFP_KERNEL);
-		if (!region)
-			return -ENOMEM;
-		region->type = WMFW_ADSP2_XM;
-		region->alg = be32_to_cpu(adsp2_id.fw.id);
-		region->base = be32_to_cpu(adsp2_id.xm);
-		list_add_tail(&region->list, &dsp->alg_regions);
+	pos = sizeof(adsp1_id) / 2;
+	len = (sizeof(*adsp1_alg) * algs) / 2;
+
+	adsp1_alg = wm_adsp_read_algs(dsp, algs, mem->base + pos, len);
+	if (IS_ERR(adsp1_alg))
+		return PTR_ERR(adsp1_alg);
+
+	for (i = 0; i < algs; i++) {
+		adsp_info(dsp, "%d: ID %x v%d.%d.%d DM@%x ZM@%x\n",
+			  i, be32_to_cpu(adsp1_alg[i].alg.id),
+			  (be32_to_cpu(adsp1_alg[i].alg.ver) & 0xff0000) >> 16,
+			  (be32_to_cpu(adsp1_alg[i].alg.ver) & 0xff00) >> 8,
+			  be32_to_cpu(adsp1_alg[i].alg.ver) & 0xff,
+			  be32_to_cpu(adsp1_alg[i].dm),
+			  be32_to_cpu(adsp1_alg[i].zm));
 
 		region = kzalloc(sizeof(*region), GFP_KERNEL);
-		if (!region)
-			return -ENOMEM;
-		region->type = WMFW_ADSP2_YM;
-		region->alg = be32_to_cpu(adsp2_id.fw.id);
-		region->base = be32_to_cpu(adsp2_id.ym);
+		if (!region) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		region->type = WMFW_ADSP1_DM;
+		region->alg = be32_to_cpu(adsp1_alg[i].alg.id);
+		region->base = be32_to_cpu(adsp1_alg[i].dm);
+		region->len = 0;
 		list_add_tail(&region->list, &dsp->alg_regions);
+		if (i + 1 < algs) {
+			region->len = be32_to_cpu(adsp1_alg[i + 1].dm);
+			region->len -= be32_to_cpu(adsp1_alg[i].dm);
+			region->len *= 4;
+			wm_adsp_create_control(dsp, region);
+		} else {
+			adsp_warn(dsp, "Missing length info for region DM with ID %x\n",
+				  be32_to_cpu(adsp1_alg[i].alg.id));
+		}
 
 		region = kzalloc(sizeof(*region), GFP_KERNEL);
-		if (!region)
-			return -ENOMEM;
-		region->type = WMFW_ADSP2_ZM;
-		region->alg = be32_to_cpu(adsp2_id.fw.id);
-		region->base = be32_to_cpu(adsp2_id.zm);
+		if (!region) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		region->type = WMFW_ADSP1_ZM;
+		region->alg = be32_to_cpu(adsp1_alg[i].alg.id);
+		region->base = be32_to_cpu(adsp1_alg[i].zm);
+		region->len = 0;
 		list_add_tail(&region->list, &dsp->alg_regions);
-
-		pos = sizeof(adsp2_id) / 2;
-		term = pos + ((sizeof(*adsp2_alg) * algs) / 2);
-		break;
-
-	default:
-		WARN(1, "Unknown DSP type");
-		return -EINVAL;
+		if (i + 1 < algs) {
+			region->len = be32_to_cpu(adsp1_alg[i + 1].zm);
+			region->len -= be32_to_cpu(adsp1_alg[i].zm);
+			region->len *= 4;
+			wm_adsp_create_control(dsp, region);
+		} else {
+			adsp_warn(dsp, "Missing length info for region ZM with ID %x\n",
+				  be32_to_cpu(adsp1_alg[i].alg.id));
+		}
 	}
 
-	if (algs == 0) {
-		adsp_err(dsp, "No algorithms\n");
-		return -EINVAL;
-	}
+out:
+	kfree(adsp1_alg);
+	return ret;
+}
 
-	if (algs > 1024) {
-		adsp_err(dsp, "Algorithm count %zx excessive\n", algs);
-		print_hex_dump_bytes(dev_name(dsp->dev), DUMP_PREFIX_OFFSET,
-				     buf, buf_size);
+static int wm_adsp2_setup_algs(struct wm_adsp *dsp)
+{
+	struct wmfw_adsp2_id_hdr adsp2_id;
+	struct wmfw_adsp2_alg_hdr *adsp2_alg;
+	struct wm_adsp_alg_region *region;
+	const struct wm_adsp_region *mem;
+	unsigned int pos, len;
+	size_t algs;
+	int i, ret;
+
+	mem = wm_adsp_find_region(dsp, WMFW_ADSP2_XM);
+	if (WARN_ON(!mem))
 		return -EINVAL;
-	}
 
-	/* Read the terminator first to validate the length */
-	ret = regmap_raw_read(regmap, mem->base + term, &val, sizeof(val));
+	ret = regmap_raw_read(dsp->regmap, mem->base, &adsp2_id,
+			      sizeof(adsp2_id));
 	if (ret != 0) {
-		adsp_err(dsp, "Failed to read algorithm list end: %d\n",
-			ret);
+		adsp_err(dsp, "Failed to read algorithm info: %d\n",
+			 ret);
 		return ret;
 	}
 
-	if (be32_to_cpu(val) != 0xbedead)
-		adsp_warn(dsp, "Algorithm list end %x 0x%x != 0xbeadead\n",
-			  term, be32_to_cpu(val));
+	algs = be32_to_cpu(adsp2_id.algs);
+	dsp->fw_id = be32_to_cpu(adsp2_id.fw.id);
+	adsp_info(dsp, "Firmware: %x v%d.%d.%d, %zu algorithms\n",
+		  dsp->fw_id,
+		  (be32_to_cpu(adsp2_id.fw.ver) & 0xff0000) >> 16,
+		  (be32_to_cpu(adsp2_id.fw.ver) & 0xff00) >> 8,
+		  be32_to_cpu(adsp2_id.fw.ver) & 0xff,
+		  algs);
+
+	region = kzalloc(sizeof(*region), GFP_KERNEL);
+	if (!region)
+		return -ENOMEM;
+	region->type = WMFW_ADSP2_XM;
+	region->alg = be32_to_cpu(adsp2_id.fw.id);
+	region->base = be32_to_cpu(adsp2_id.xm);
+	list_add_tail(&region->list, &dsp->alg_regions);
 
-	alg = kzalloc((term - pos) * 2, GFP_KERNEL | GFP_DMA);
-	if (!alg)
+	region = kzalloc(sizeof(*region), GFP_KERNEL);
+	if (!region)
 		return -ENOMEM;
+	region->type = WMFW_ADSP2_YM;
+	region->alg = be32_to_cpu(adsp2_id.fw.id);
+	region->base = be32_to_cpu(adsp2_id.ym);
+	list_add_tail(&region->list, &dsp->alg_regions);
 
-	ret = regmap_raw_read(regmap, mem->base + pos, alg, (term - pos) * 2);
-	if (ret != 0) {
-		adsp_err(dsp, "Failed to read algorithm list: %d\n",
-			ret);
-		goto out;
-	}
+	region = kzalloc(sizeof(*region), GFP_KERNEL);
+	if (!region)
+		return -ENOMEM;
+	region->type = WMFW_ADSP2_ZM;
+	region->alg = be32_to_cpu(adsp2_id.fw.id);
+	region->base = be32_to_cpu(adsp2_id.zm);
+	list_add_tail(&region->list, &dsp->alg_regions);
 
-	adsp1_alg = alg;
-	adsp2_alg = alg;
+	pos = sizeof(adsp2_id) / 2;
+	len = (sizeof(*adsp2_alg) * algs) / 2;
 
-	for (i = 0; i < algs; i++) {
-		switch (dsp->type) {
-		case WMFW_ADSP1:
-			adsp_info(dsp, "%d: ID %x v%d.%d.%d DM@%x ZM@%x\n",
-				  i, be32_to_cpu(adsp1_alg[i].alg.id),
-				  (be32_to_cpu(adsp1_alg[i].alg.ver) & 0xff0000) >> 16,
-				  (be32_to_cpu(adsp1_alg[i].alg.ver) & 0xff00) >> 8,
-				  be32_to_cpu(adsp1_alg[i].alg.ver) & 0xff,
-				  be32_to_cpu(adsp1_alg[i].dm),
-				  be32_to_cpu(adsp1_alg[i].zm));
-
-			region = kzalloc(sizeof(*region), GFP_KERNEL);
-			if (!region) {
-				ret = -ENOMEM;
-				goto out;
-			}
-			region->type = WMFW_ADSP1_DM;
-			region->alg = be32_to_cpu(adsp1_alg[i].alg.id);
-			region->base = be32_to_cpu(adsp1_alg[i].dm);
-			region->len = 0;
-			list_add_tail(&region->list, &dsp->alg_regions);
-			if (i + 1 < algs) {
-				region->len = be32_to_cpu(adsp1_alg[i + 1].dm);
-				region->len -= be32_to_cpu(adsp1_alg[i].dm);
-				region->len *= 4;
-				wm_adsp_create_control(dsp, region);
-			} else {
-				adsp_warn(dsp, "Missing length info for region DM with ID %x\n",
-					  be32_to_cpu(adsp1_alg[i].alg.id));
-			}
+	adsp2_alg = wm_adsp_read_algs(dsp, algs, mem->base + pos, len);
+	if (IS_ERR(adsp2_alg))
+		return PTR_ERR(adsp2_alg);
 
-			region = kzalloc(sizeof(*region), GFP_KERNEL);
-			if (!region) {
-				ret = -ENOMEM;
-				goto out;
-			}
-			region->type = WMFW_ADSP1_ZM;
-			region->alg = be32_to_cpu(adsp1_alg[i].alg.id);
-			region->base = be32_to_cpu(adsp1_alg[i].zm);
-			region->len = 0;
-			list_add_tail(&region->list, &dsp->alg_regions);
-			if (i + 1 < algs) {
-				region->len = be32_to_cpu(adsp1_alg[i + 1].zm);
-				region->len -= be32_to_cpu(adsp1_alg[i].zm);
-				region->len *= 4;
-				wm_adsp_create_control(dsp, region);
-			} else {
-				adsp_warn(dsp, "Missing length info for region ZM with ID %x\n",
-					  be32_to_cpu(adsp1_alg[i].alg.id));
-			}
-			break;
+	for (i = 0; i < algs; i++) {
+		adsp_info(dsp,
+			  "%d: ID %x v%d.%d.%d XM@%x YM@%x ZM@%x\n",
+			  i, be32_to_cpu(adsp2_alg[i].alg.id),
+			  (be32_to_cpu(adsp2_alg[i].alg.ver) & 0xff0000) >> 16,
+			  (be32_to_cpu(adsp2_alg[i].alg.ver) & 0xff00) >> 8,
+			  be32_to_cpu(adsp2_alg[i].alg.ver) & 0xff,
+			  be32_to_cpu(adsp2_alg[i].xm),
+			  be32_to_cpu(adsp2_alg[i].ym),
+			  be32_to_cpu(adsp2_alg[i].zm));
 
-		case WMFW_ADSP2:
-			adsp_info(dsp,
-				  "%d: ID %x v%d.%d.%d XM@%x YM@%x ZM@%x\n",
-				  i, be32_to_cpu(adsp2_alg[i].alg.id),
-				  (be32_to_cpu(adsp2_alg[i].alg.ver) & 0xff0000) >> 16,
-				  (be32_to_cpu(adsp2_alg[i].alg.ver) & 0xff00) >> 8,
-				  be32_to_cpu(adsp2_alg[i].alg.ver) & 0xff,
-				  be32_to_cpu(adsp2_alg[i].xm),
-				  be32_to_cpu(adsp2_alg[i].ym),
-				  be32_to_cpu(adsp2_alg[i].zm));
-
-			region = kzalloc(sizeof(*region), GFP_KERNEL);
-			if (!region) {
-				ret = -ENOMEM;
-				goto out;
-			}
-			region->type = WMFW_ADSP2_XM;
-			region->alg = be32_to_cpu(adsp2_alg[i].alg.id);
-			region->base = be32_to_cpu(adsp2_alg[i].xm);
-			region->len = 0;
-			list_add_tail(&region->list, &dsp->alg_regions);
-			if (i + 1 < algs) {
-				region->len = be32_to_cpu(adsp2_alg[i + 1].xm);
-				region->len -= be32_to_cpu(adsp2_alg[i].xm);
-				region->len *= 4;
-				wm_adsp_create_control(dsp, region);
-			} else {
-				adsp_warn(dsp, "Missing length info for region XM with ID %x\n",
-					  be32_to_cpu(adsp2_alg[i].alg.id));
-			}
+		region = kzalloc(sizeof(*region), GFP_KERNEL);
+		if (!region) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		region->type = WMFW_ADSP2_XM;
+		region->alg = be32_to_cpu(adsp2_alg[i].alg.id);
+		region->base = be32_to_cpu(adsp2_alg[i].xm);
+		region->len = 0;
+		list_add_tail(&region->list, &dsp->alg_regions);
+		if (i + 1 < algs) {
+			region->len = be32_to_cpu(adsp2_alg[i + 1].xm);
+			region->len -= be32_to_cpu(adsp2_alg[i].xm);
+			region->len *= 4;
+			wm_adsp_create_control(dsp, region);
+		} else {
+			adsp_warn(dsp, "Missing length info for region XM with ID %x\n",
+				  be32_to_cpu(adsp2_alg[i].alg.id));
+		}
 
-			region = kzalloc(sizeof(*region), GFP_KERNEL);
-			if (!region) {
-				ret = -ENOMEM;
-				goto out;
-			}
-			region->type = WMFW_ADSP2_YM;
-			region->alg = be32_to_cpu(adsp2_alg[i].alg.id);
-			region->base = be32_to_cpu(adsp2_alg[i].ym);
-			region->len = 0;
-			list_add_tail(&region->list, &dsp->alg_regions);
-			if (i + 1 < algs) {
-				region->len = be32_to_cpu(adsp2_alg[i + 1].ym);
-				region->len -= be32_to_cpu(adsp2_alg[i].ym);
-				region->len *= 4;
-				wm_adsp_create_control(dsp, region);
-			} else {
-				adsp_warn(dsp, "Missing length info for region YM with ID %x\n",
-					  be32_to_cpu(adsp2_alg[i].alg.id));
-			}
+		region = kzalloc(sizeof(*region), GFP_KERNEL);
+		if (!region) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		region->type = WMFW_ADSP2_YM;
+		region->alg = be32_to_cpu(adsp2_alg[i].alg.id);
+		region->base = be32_to_cpu(adsp2_alg[i].ym);
+		region->len = 0;
+		list_add_tail(&region->list, &dsp->alg_regions);
+		if (i + 1 < algs) {
+			region->len = be32_to_cpu(adsp2_alg[i + 1].ym);
+			region->len -= be32_to_cpu(adsp2_alg[i].ym);
+			region->len *= 4;
+			wm_adsp_create_control(dsp, region);
+		} else {
+			adsp_warn(dsp, "Missing length info for region YM with ID %x\n",
+				  be32_to_cpu(adsp2_alg[i].alg.id));
+		}
 
-			region = kzalloc(sizeof(*region), GFP_KERNEL);
-			if (!region) {
-				ret = -ENOMEM;
-				goto out;
-			}
-			region->type = WMFW_ADSP2_ZM;
-			region->alg = be32_to_cpu(adsp2_alg[i].alg.id);
-			region->base = be32_to_cpu(adsp2_alg[i].zm);
-			region->len = 0;
-			list_add_tail(&region->list, &dsp->alg_regions);
-			if (i + 1 < algs) {
-				region->len = be32_to_cpu(adsp2_alg[i + 1].zm);
-				region->len -= be32_to_cpu(adsp2_alg[i].zm);
-				region->len *= 4;
-				wm_adsp_create_control(dsp, region);
-			} else {
-				adsp_warn(dsp, "Missing length info for region ZM with ID %x\n",
-					  be32_to_cpu(adsp2_alg[i].alg.id));
-			}
-			break;
+		region = kzalloc(sizeof(*region), GFP_KERNEL);
+		if (!region) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		region->type = WMFW_ADSP2_ZM;
+		region->alg = be32_to_cpu(adsp2_alg[i].alg.id);
+		region->base = be32_to_cpu(adsp2_alg[i].zm);
+		region->len = 0;
+		list_add_tail(&region->list, &dsp->alg_regions);
+		if (i + 1 < algs) {
+			region->len = be32_to_cpu(adsp2_alg[i + 1].zm);
+			region->len -= be32_to_cpu(adsp2_alg[i].zm);
+			region->len *= 4;
+			wm_adsp_create_control(dsp, region);
+		} else {
+			adsp_warn(dsp, "Missing length info for region ZM with ID %x\n",
+				  be32_to_cpu(adsp2_alg[i].alg.id));
 		}
 	}
 
 out:
-	kfree(alg);
+	kfree(adsp2_alg);
 	return ret;
 }
 
@@ -1410,7 +1407,7 @@ int wm_adsp1_event(struct snd_soc_dapm_widget *w,
 		if (ret != 0)
 			goto err;
 
-		ret = wm_adsp_setup_algs(dsp);
+		ret = wm_adsp1_setup_algs(dsp);
 		if (ret != 0)
 			goto err;
 
@@ -1568,7 +1565,7 @@ static void wm_adsp2_boot_work(struct work_struct *work)
 	if (ret != 0)
 		goto err;
 
-	ret = wm_adsp_setup_algs(dsp);
+	ret = wm_adsp2_setup_algs(dsp);
 	if (ret != 0)
 		goto err;
 
-- 
1.7.2.5

[PATCH 02/13] ASoC: wm_adsp: Improve variable naming

[Charles Keepax]

We have wm_adsp_region, wm_adsp_alg_region, and wmfw_region, the
variables for which are all frequently called region, this can get quite
confusing when reviewing the code especially given some functions are
quite long. Consistently use mem for wm_adsp_regions, alg_region for
wm_adsp_alg_region and region for wmfw_region.

Additionally, we use a mix of adsp and dsp for pointers to the wm_adsp
structure standardise this on dsp.

Finally, we use algs to refer to the number of algorithms quite
frequently, change this to the more descriptive n_algs.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm_adsp.c |  356 ++++++++++++++++++++++----------------------
 sound/soc/codecs/wm_adsp.h |    4 +-
 sound/soc/codecs/wmfw.h    |    4 +-
 3 files changed, 182 insertions(+), 182 deletions(-)

diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index f421c09..4201e1f 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -229,9 +229,9 @@ struct wm_coeff_ctl_ops {
 
 struct wm_coeff_ctl {
 	const char *name;
-	struct wm_adsp_alg_region region;
+	struct wm_adsp_alg_region alg_region;
 	struct wm_coeff_ctl_ops ops;
-	struct wm_adsp *adsp;
+	struct wm_adsp *dsp;
 	void *private;
 	unsigned int enabled:1;
 	struct list_head list;
@@ -246,9 +246,9 @@ static int wm_adsp_fw_get(struct snd_kcontrol *kcontrol,
 {
 	struct snd_soc_codec *codec = snd_soc_kcontrol_codec(kcontrol);
 	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
-	struct wm_adsp *adsp = snd_soc_codec_get_drvdata(codec);
+	struct wm_adsp *dsp = snd_soc_codec_get_drvdata(codec);
 
-	ucontrol->value.integer.value[0] = adsp[e->shift_l].fw;
+	ucontrol->value.integer.value[0] = dsp[e->shift_l].fw;
 
 	return 0;
 }
@@ -258,18 +258,18 @@ static int wm_adsp_fw_put(struct snd_kcontrol *kcontrol,
 {
 	struct snd_soc_codec *codec = snd_soc_kcontrol_codec(kcontrol);
 	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
-	struct wm_adsp *adsp = snd_soc_codec_get_drvdata(codec);
+	struct wm_adsp *dsp = snd_soc_codec_get_drvdata(codec);
 
-	if (ucontrol->value.integer.value[0] == adsp[e->shift_l].fw)
+	if (ucontrol->value.integer.value[0] == dsp[e->shift_l].fw)
 		return 0;
 
 	if (ucontrol->value.integer.value[0] >= WM_ADSP_NUM_FW)
 		return -EINVAL;
 
-	if (adsp[e->shift_l].running)
+	if (dsp[e->shift_l].running)
 		return -EBUSY;
 
-	adsp[e->shift_l].fw = ucontrol->value.integer.value[0];
+	dsp[e->shift_l].fw = ucontrol->value.integer.value[0];
 
 	return 0;
 }
@@ -340,22 +340,22 @@ static struct wm_adsp_region const *wm_adsp_find_region(struct wm_adsp *dsp,
 	return NULL;
 }
 
-static unsigned int wm_adsp_region_to_reg(struct wm_adsp_region const *region,
+static unsigned int wm_adsp_region_to_reg(struct wm_adsp_region const *mem,
 					  unsigned int offset)
 {
-	if (WARN_ON(!region))
+	if (WARN_ON(!mem))
 		return offset;
-	switch (region->type) {
+	switch (mem->type) {
 	case WMFW_ADSP1_PM:
-		return region->base + (offset * 3);
+		return mem->base + (offset * 3);
 	case WMFW_ADSP1_DM:
-		return region->base + (offset * 2);
+		return mem->base + (offset * 2);
 	case WMFW_ADSP2_XM:
-		return region->base + (offset * 2);
+		return mem->base + (offset * 2);
 	case WMFW_ADSP2_YM:
-		return region->base + (offset * 2);
+		return mem->base + (offset * 2);
 	case WMFW_ADSP1_ZM:
-		return region->base + (offset * 2);
+		return mem->base + (offset * 2);
 	default:
 		WARN(1, "Unknown memory region type");
 		return offset;
@@ -376,36 +376,36 @@ static int wm_coeff_write_control(struct snd_kcontrol *kcontrol,
 				  const void *buf, size_t len)
 {
 	struct wm_coeff_ctl *ctl = (struct wm_coeff_ctl *)kcontrol->private_value;
-	struct wm_adsp_alg_region *region = &ctl->region;
+	struct wm_adsp_alg_region *alg_region = &ctl->alg_region;
 	const struct wm_adsp_region *mem;
-	struct wm_adsp *adsp = ctl->adsp;
+	struct wm_adsp *dsp = ctl->dsp;
 	void *scratch;
 	int ret;
 	unsigned int reg;
 
-	mem = wm_adsp_find_region(adsp, region->type);
+	mem = wm_adsp_find_region(dsp, alg_region->type);
 	if (!mem) {
-		adsp_err(adsp, "No base for region %x\n",
-			 region->type);
+		adsp_err(dsp, "No base for region %x\n",
+			 alg_region->type);
 		return -EINVAL;
 	}
 
-	reg = ctl->region.base;
+	reg = ctl->alg_region.base;
 	reg = wm_adsp_region_to_reg(mem, reg);
 
 	scratch = kmemdup(buf, ctl->len, GFP_KERNEL | GFP_DMA);
 	if (!scratch)
 		return -ENOMEM;
 
-	ret = regmap_raw_write(adsp->regmap, reg, scratch,
+	ret = regmap_raw_write(dsp->regmap, reg, scratch,
 			       ctl->len);
 	if (ret) {
-		adsp_err(adsp, "Failed to write %zu bytes to %x: %d\n",
+		adsp_err(dsp, "Failed to write %zu bytes to %x: %d\n",
 			 ctl->len, reg, ret);
 		kfree(scratch);
 		return ret;
 	}
-	adsp_dbg(adsp, "Wrote %zu bytes to %x\n", ctl->len, reg);
+	adsp_dbg(dsp, "Wrote %zu bytes to %x\n", ctl->len, reg);
 
 	kfree(scratch);
 
@@ -431,35 +431,35 @@ static int wm_coeff_read_control(struct snd_kcontrol *kcontrol,
 				 void *buf, size_t len)
 {
 	struct wm_coeff_ctl *ctl = (struct wm_coeff_ctl *)kcontrol->private_value;
-	struct wm_adsp_alg_region *region = &ctl->region;
+	struct wm_adsp_alg_region *alg_region = &ctl->alg_region;
 	const struct wm_adsp_region *mem;
-	struct wm_adsp *adsp = ctl->adsp;
+	struct wm_adsp *dsp = ctl->dsp;
 	void *scratch;
 	int ret;
 	unsigned int reg;
 
-	mem = wm_adsp_find_region(adsp, region->type);
+	mem = wm_adsp_find_region(dsp, alg_region->type);
 	if (!mem) {
-		adsp_err(adsp, "No base for region %x\n",
-			 region->type);
+		adsp_err(dsp, "No base for region %x\n",
+			 alg_region->type);
 		return -EINVAL;
 	}
 
-	reg = ctl->region.base;
+	reg = ctl->alg_region.base;
 	reg = wm_adsp_region_to_reg(mem, reg);
 
 	scratch = kmalloc(ctl->len, GFP_KERNEL | GFP_DMA);
 	if (!scratch)
 		return -ENOMEM;
 
-	ret = regmap_raw_read(adsp->regmap, reg, scratch, ctl->len);
+	ret = regmap_raw_read(dsp->regmap, reg, scratch, ctl->len);
 	if (ret) {
-		adsp_err(adsp, "Failed to read %zu bytes from %x: %d\n",
+		adsp_err(dsp, "Failed to read %zu bytes from %x: %d\n",
 			 ctl->len, reg, ret);
 		kfree(scratch);
 		return ret;
 	}
-	adsp_dbg(adsp, "Read %zu bytes from %x\n", ctl->len, reg);
+	adsp_dbg(dsp, "Read %zu bytes from %x\n", ctl->len, reg);
 
 	memcpy(buf, scratch, ctl->len);
 	kfree(scratch);
@@ -478,12 +478,12 @@ static int wm_coeff_get(struct snd_kcontrol *kcontrol,
 }
 
 struct wmfw_ctl_work {
-	struct wm_adsp *adsp;
+	struct wm_adsp *dsp;
 	struct wm_coeff_ctl *ctl;
 	struct work_struct work;
 };
 
-static int wmfw_add_ctl(struct wm_adsp *adsp, struct wm_coeff_ctl *ctl)
+static int wmfw_add_ctl(struct wm_adsp *dsp, struct wm_coeff_ctl *ctl)
 {
 	struct snd_kcontrol_new *kcontrol;
 	int ret;
@@ -502,17 +502,18 @@ static int wmfw_add_ctl(struct wm_adsp *adsp, struct wm_coeff_ctl *ctl)
 	kcontrol->put = wm_coeff_put;
 	kcontrol->private_value = (unsigned long)ctl;
 
-	ret = snd_soc_add_card_controls(adsp->card,
+	ret = snd_soc_add_card_controls(dsp->card,
 					kcontrol, 1);
 	if (ret < 0)
 		goto err_kcontrol;
 
 	kfree(kcontrol);
 
-	ctl->kcontrol = snd_soc_card_get_kcontrol(adsp->card,
+	ctl->kcontrol = snd_soc_card_get_kcontrol(dsp->card,
 						  ctl->name);
 
-	list_add(&ctl->list, &adsp->ctl_list);
+	list_add(&ctl->list, &dsp->ctl_list);
+
 	return 0;
 
 err_kcontrol:
@@ -730,12 +731,12 @@ out:
 	return ret;
 }
 
-static int wm_coeff_init_control_caches(struct wm_adsp *adsp)
+static int wm_coeff_init_control_caches(struct wm_adsp *dsp)
 {
 	struct wm_coeff_ctl *ctl;
 	int ret;
 
-	list_for_each_entry(ctl, &adsp->ctl_list, list) {
+	list_for_each_entry(ctl, &dsp->ctl_list, list) {
 		if (!ctl->enabled || ctl->set)
 			continue;
 		ret = wm_coeff_read_control(ctl->kcontrol,
@@ -748,12 +749,12 @@ static int wm_coeff_init_control_caches(struct wm_adsp *adsp)
 	return 0;
 }
 
-static int wm_coeff_sync_controls(struct wm_adsp *adsp)
+static int wm_coeff_sync_controls(struct wm_adsp *dsp)
 {
 	struct wm_coeff_ctl *ctl;
 	int ret;
 
-	list_for_each_entry(ctl, &adsp->ctl_list, list) {
+	list_for_each_entry(ctl, &dsp->ctl_list, list) {
 		if (!ctl->enabled)
 			continue;
 		if (ctl->set) {
@@ -774,13 +775,12 @@ static void wm_adsp_ctl_work(struct work_struct *work)
 						      struct wmfw_ctl_work,
 						      work);
 
-	wmfw_add_ctl(ctl_work->adsp, ctl_work->ctl);
+	wmfw_add_ctl(ctl_work->dsp, ctl_work->ctl);
 	kfree(ctl_work);
 }
 
 static int wm_adsp_create_control(struct wm_adsp *dsp,
-				  const struct wm_adsp_alg_region *region)
-
+				  const struct wm_adsp_alg_region *alg_region)
 {
 	struct wm_coeff_ctl *ctl;
 	struct wmfw_ctl_work *ctl_work;
@@ -792,7 +792,7 @@ static int wm_adsp_create_control(struct wm_adsp *dsp,
 	if (!name)
 		return -ENOMEM;
 
-	switch (region->type) {
+	switch (alg_region->type) {
 	case WMFW_ADSP1_PM:
 		region_name = "PM";
 		break;
@@ -814,7 +814,7 @@ static int wm_adsp_create_control(struct wm_adsp *dsp,
 	}
 
 	snprintf(name, PAGE_SIZE, "DSP%d %s %x",
-		 dsp->num, region_name, region->alg);
+		 dsp->num, region_name, alg_region->alg);
 
 	list_for_each_entry(ctl, &dsp->ctl_list,
 			    list) {
@@ -830,7 +830,7 @@ static int wm_adsp_create_control(struct wm_adsp *dsp,
 		ret = -ENOMEM;
 		goto err_name;
 	}
-	ctl->region = *region;
+	ctl->alg_region = *alg_region;
 	ctl->name = kmemdup(name, strlen(name) + 1, GFP_KERNEL);
 	if (!ctl->name) {
 		ret = -ENOMEM;
@@ -840,9 +840,9 @@ static int wm_adsp_create_control(struct wm_adsp *dsp,
 	ctl->set = 0;
 	ctl->ops.xget = wm_coeff_get;
 	ctl->ops.xput = wm_coeff_put;
-	ctl->adsp = dsp;
+	ctl->dsp = dsp;
 
-	ctl->len = region->len;
+	ctl->len = alg_region->len;
 	ctl->cache = kzalloc(ctl->len, GFP_KERNEL);
 	if (!ctl->cache) {
 		ret = -ENOMEM;
@@ -855,7 +855,7 @@ static int wm_adsp_create_control(struct wm_adsp *dsp,
 		goto err_ctl_cache;
 	}
 
-	ctl_work->adsp = dsp;
+	ctl_work->dsp = dsp;
 	ctl_work->ctl = ctl;
 	INIT_WORK(&ctl_work->work, wm_adsp_ctl_work);
 	schedule_work(&ctl_work->work);
@@ -876,20 +876,20 @@ err_name:
 	return ret;
 }
 
-static void *wm_adsp_read_algs(struct wm_adsp *dsp, size_t algs,
+static void *wm_adsp_read_algs(struct wm_adsp *dsp, size_t n_algs,
 			       unsigned int pos, unsigned int len)
 {
 	void *alg;
 	int ret;
 	__be32 val;
 
-	if (algs == 0) {
+	if (n_algs == 0) {
 		adsp_err(dsp, "No algorithms\n");
 		return ERR_PTR(-EINVAL);
 	}
 
-	if (algs > 1024) {
-		adsp_err(dsp, "Algorithm count %zx excessive\n", algs);
+	if (n_algs > 1024) {
+		adsp_err(dsp, "Algorithm count %zx excessive\n", n_algs);
 		return ERR_PTR(-EINVAL);
 	}
 
@@ -924,10 +924,10 @@ static int wm_adsp1_setup_algs(struct wm_adsp *dsp)
 {
 	struct wmfw_adsp1_id_hdr adsp1_id;
 	struct wmfw_adsp1_alg_hdr *adsp1_alg;
-	struct wm_adsp_alg_region *region;
+	struct wm_adsp_alg_region *alg_region;
 	const struct wm_adsp_region *mem;
 	unsigned int pos, len;
-	size_t algs;
+	size_t n_algs;
 	int i, ret;
 
 	mem = wm_adsp_find_region(dsp, WMFW_ADSP1_DM);
@@ -942,39 +942,39 @@ static int wm_adsp1_setup_algs(struct wm_adsp *dsp)
 		return ret;
 	}
 
-	algs = be32_to_cpu(adsp1_id.algs);
+	n_algs = be32_to_cpu(adsp1_id.n_algs);
 	dsp->fw_id = be32_to_cpu(adsp1_id.fw.id);
 	adsp_info(dsp, "Firmware: %x v%d.%d.%d, %zu algorithms\n",
 		  dsp->fw_id,
 		  (be32_to_cpu(adsp1_id.fw.ver) & 0xff0000) >> 16,
 		  (be32_to_cpu(adsp1_id.fw.ver) & 0xff00) >> 8,
 		  be32_to_cpu(adsp1_id.fw.ver) & 0xff,
-		  algs);
+		  n_algs);
 
-	region = kzalloc(sizeof(*region), GFP_KERNEL);
-	if (!region)
+	alg_region = kzalloc(sizeof(*alg_region), GFP_KERNEL);
+	if (!alg_region)
 		return -ENOMEM;
-	region->type = WMFW_ADSP1_ZM;
-	region->alg = be32_to_cpu(adsp1_id.fw.id);
-	region->base = be32_to_cpu(adsp1_id.zm);
-	list_add_tail(&region->list, &dsp->alg_regions);
+	alg_region->type = WMFW_ADSP1_ZM;
+	alg_region->alg = be32_to_cpu(adsp1_id.fw.id);
+	alg_region->base = be32_to_cpu(adsp1_id.zm);
+	list_add_tail(&alg_region->list, &dsp->alg_regions);
 
-	region = kzalloc(sizeof(*region), GFP_KERNEL);
-	if (!region)
+	alg_region = kzalloc(sizeof(*alg_region), GFP_KERNEL);
+	if (!alg_region)
 		return -ENOMEM;
-	region->type = WMFW_ADSP1_DM;
-	region->alg = be32_to_cpu(adsp1_id.fw.id);
-	region->base = be32_to_cpu(adsp1_id.dm);
-	list_add_tail(&region->list, &dsp->alg_regions);
+	alg_region->type = WMFW_ADSP1_DM;
+	alg_region->alg = be32_to_cpu(adsp1_id.fw.id);
+	alg_region->base = be32_to_cpu(adsp1_id.dm);
+	list_add_tail(&alg_region->list, &dsp->alg_regions);
 
 	pos = sizeof(adsp1_id) / 2;
-	len = (sizeof(*adsp1_alg) * algs) / 2;
+	len = (sizeof(*adsp1_alg) * n_algs) / 2;
 
-	adsp1_alg = wm_adsp_read_algs(dsp, algs, mem->base + pos, len);
+	adsp1_alg = wm_adsp_read_algs(dsp, n_algs, mem->base + pos, len);
 	if (IS_ERR(adsp1_alg))
 		return PTR_ERR(adsp1_alg);
 
-	for (i = 0; i < algs; i++) {
+	for (i = 0; i < n_algs; i++) {
 		adsp_info(dsp, "%d: ID %x v%d.%d.%d DM@%x ZM@%x\n",
 			  i, be32_to_cpu(adsp1_alg[i].alg.id),
 			  (be32_to_cpu(adsp1_alg[i].alg.ver) & 0xff0000) >> 16,
@@ -983,41 +983,41 @@ static int wm_adsp1_setup_algs(struct wm_adsp *dsp)
 			  be32_to_cpu(adsp1_alg[i].dm),
 			  be32_to_cpu(adsp1_alg[i].zm));
 
-		region = kzalloc(sizeof(*region), GFP_KERNEL);
-		if (!region) {
+		alg_region = kzalloc(sizeof(*alg_region), GFP_KERNEL);
+		if (!alg_region) {
 			ret = -ENOMEM;
 			goto out;
 		}
-		region->type = WMFW_ADSP1_DM;
-		region->alg = be32_to_cpu(adsp1_alg[i].alg.id);
-		region->base = be32_to_cpu(adsp1_alg[i].dm);
-		region->len = 0;
-		list_add_tail(&region->list, &dsp->alg_regions);
-		if (i + 1 < algs) {
-			region->len = be32_to_cpu(adsp1_alg[i + 1].dm);
-			region->len -= be32_to_cpu(adsp1_alg[i].dm);
-			region->len *= 4;
-			wm_adsp_create_control(dsp, region);
+		alg_region->type = WMFW_ADSP1_DM;
+		alg_region->alg = be32_to_cpu(adsp1_alg[i].alg.id);
+		alg_region->base = be32_to_cpu(adsp1_alg[i].dm);
+		alg_region->len = 0;
+		list_add_tail(&alg_region->list, &dsp->alg_regions);
+		if (i + 1 < n_algs) {
+			alg_region->len = be32_to_cpu(adsp1_alg[i + 1].dm);
+			alg_region->len -= be32_to_cpu(adsp1_alg[i].dm);
+			alg_region->len *= 4;
+			wm_adsp_create_control(dsp, alg_region);
 		} else {
 			adsp_warn(dsp, "Missing length info for region DM with ID %x\n",
 				  be32_to_cpu(adsp1_alg[i].alg.id));
 		}
 
-		region = kzalloc(sizeof(*region), GFP_KERNEL);
-		if (!region) {
+		alg_region = kzalloc(sizeof(*alg_region), GFP_KERNEL);
+		if (!alg_region) {
 			ret = -ENOMEM;
 			goto out;
 		}
-		region->type = WMFW_ADSP1_ZM;
-		region->alg = be32_to_cpu(adsp1_alg[i].alg.id);
-		region->base = be32_to_cpu(adsp1_alg[i].zm);
-		region->len = 0;
-		list_add_tail(&region->list, &dsp->alg_regions);
-		if (i + 1 < algs) {
-			region->len = be32_to_cpu(adsp1_alg[i + 1].zm);
-			region->len -= be32_to_cpu(adsp1_alg[i].zm);
-			region->len *= 4;
-			wm_adsp_create_control(dsp, region);
+		alg_region->type = WMFW_ADSP1_ZM;
+		alg_region->alg = be32_to_cpu(adsp1_alg[i].alg.id);
+		alg_region->base = be32_to_cpu(adsp1_alg[i].zm);
+		alg_region->len = 0;
+		list_add_tail(&alg_region->list, &dsp->alg_regions);
+		if (i + 1 < n_algs) {
+			alg_region->len = be32_to_cpu(adsp1_alg[i + 1].zm);
+			alg_region->len -= be32_to_cpu(adsp1_alg[i].zm);
+			alg_region->len *= 4;
+			wm_adsp_create_control(dsp, alg_region);
 		} else {
 			adsp_warn(dsp, "Missing length info for region ZM with ID %x\n",
 				  be32_to_cpu(adsp1_alg[i].alg.id));
@@ -1033,10 +1033,10 @@ static int wm_adsp2_setup_algs(struct wm_adsp *dsp)
 {
 	struct wmfw_adsp2_id_hdr adsp2_id;
 	struct wmfw_adsp2_alg_hdr *adsp2_alg;
-	struct wm_adsp_alg_region *region;
+	struct wm_adsp_alg_region *alg_region;
 	const struct wm_adsp_region *mem;
 	unsigned int pos, len;
-	size_t algs;
+	size_t n_algs;
 	int i, ret;
 
 	mem = wm_adsp_find_region(dsp, WMFW_ADSP2_XM);
@@ -1051,47 +1051,47 @@ static int wm_adsp2_setup_algs(struct wm_adsp *dsp)
 		return ret;
 	}
 
-	algs = be32_to_cpu(adsp2_id.algs);
+	n_algs = be32_to_cpu(adsp2_id.n_algs);
 	dsp->fw_id = be32_to_cpu(adsp2_id.fw.id);
 	adsp_info(dsp, "Firmware: %x v%d.%d.%d, %zu algorithms\n",
 		  dsp->fw_id,
 		  (be32_to_cpu(adsp2_id.fw.ver) & 0xff0000) >> 16,
 		  (be32_to_cpu(adsp2_id.fw.ver) & 0xff00) >> 8,
 		  be32_to_cpu(adsp2_id.fw.ver) & 0xff,
-		  algs);
+		  n_algs);
 
-	region = kzalloc(sizeof(*region), GFP_KERNEL);
-	if (!region)
+	alg_region = kzalloc(sizeof(*alg_region), GFP_KERNEL);
+	if (!alg_region)
 		return -ENOMEM;
-	region->type = WMFW_ADSP2_XM;
-	region->alg = be32_to_cpu(adsp2_id.fw.id);
-	region->base = be32_to_cpu(adsp2_id.xm);
-	list_add_tail(&region->list, &dsp->alg_regions);
+	alg_region->type = WMFW_ADSP2_XM;
+	alg_region->alg = be32_to_cpu(adsp2_id.fw.id);
+	alg_region->base = be32_to_cpu(adsp2_id.xm);
+	list_add_tail(&alg_region->list, &dsp->alg_regions);
 
-	region = kzalloc(sizeof(*region), GFP_KERNEL);
-	if (!region)
+	alg_region = kzalloc(sizeof(*alg_region), GFP_KERNEL);
+	if (!alg_region)
 		return -ENOMEM;
-	region->type = WMFW_ADSP2_YM;
-	region->alg = be32_to_cpu(adsp2_id.fw.id);
-	region->base = be32_to_cpu(adsp2_id.ym);
-	list_add_tail(&region->list, &dsp->alg_regions);
+	alg_region->type = WMFW_ADSP2_YM;
+	alg_region->alg = be32_to_cpu(adsp2_id.fw.id);
+	alg_region->base = be32_to_cpu(adsp2_id.ym);
+	list_add_tail(&alg_region->list, &dsp->alg_regions);
 
-	region = kzalloc(sizeof(*region), GFP_KERNEL);
-	if (!region)
+	alg_region = kzalloc(sizeof(*alg_region), GFP_KERNEL);
+	if (!alg_region)
 		return -ENOMEM;
-	region->type = WMFW_ADSP2_ZM;
-	region->alg = be32_to_cpu(adsp2_id.fw.id);
-	region->base = be32_to_cpu(adsp2_id.zm);
-	list_add_tail(&region->list, &dsp->alg_regions);
+	alg_region->type = WMFW_ADSP2_ZM;
+	alg_region->alg = be32_to_cpu(adsp2_id.fw.id);
+	alg_region->base = be32_to_cpu(adsp2_id.zm);
+	list_add_tail(&alg_region->list, &dsp->alg_regions);
 
 	pos = sizeof(adsp2_id) / 2;
-	len = (sizeof(*adsp2_alg) * algs) / 2;
+	len = (sizeof(*adsp2_alg) * n_algs) / 2;
 
-	adsp2_alg = wm_adsp_read_algs(dsp, algs, mem->base + pos, len);
+	adsp2_alg = wm_adsp_read_algs(dsp, n_algs, mem->base + pos, len);
 	if (IS_ERR(adsp2_alg))
 		return PTR_ERR(adsp2_alg);
 
-	for (i = 0; i < algs; i++) {
+	for (i = 0; i < n_algs; i++) {
 		adsp_info(dsp,
 			  "%d: ID %x v%d.%d.%d XM@%x YM@%x ZM@%x\n",
 			  i, be32_to_cpu(adsp2_alg[i].alg.id),
@@ -1102,61 +1102,61 @@ static int wm_adsp2_setup_algs(struct wm_adsp *dsp)
 			  be32_to_cpu(adsp2_alg[i].ym),
 			  be32_to_cpu(adsp2_alg[i].zm));
 
-		region = kzalloc(sizeof(*region), GFP_KERNEL);
-		if (!region) {
+		alg_region = kzalloc(sizeof(*alg_region), GFP_KERNEL);
+		if (!alg_region) {
 			ret = -ENOMEM;
 			goto out;
 		}
-		region->type = WMFW_ADSP2_XM;
-		region->alg = be32_to_cpu(adsp2_alg[i].alg.id);
-		region->base = be32_to_cpu(adsp2_alg[i].xm);
-		region->len = 0;
-		list_add_tail(&region->list, &dsp->alg_regions);
-		if (i + 1 < algs) {
-			region->len = be32_to_cpu(adsp2_alg[i + 1].xm);
-			region->len -= be32_to_cpu(adsp2_alg[i].xm);
-			region->len *= 4;
-			wm_adsp_create_control(dsp, region);
+		alg_region->type = WMFW_ADSP2_XM;
+		alg_region->alg = be32_to_cpu(adsp2_alg[i].alg.id);
+		alg_region->base = be32_to_cpu(adsp2_alg[i].xm);
+		alg_region->len = 0;
+		list_add_tail(&alg_region->list, &dsp->alg_regions);
+		if (i + 1 < n_algs) {
+			alg_region->len = be32_to_cpu(adsp2_alg[i + 1].xm);
+			alg_region->len -= be32_to_cpu(adsp2_alg[i].xm);
+			alg_region->len *= 4;
+			wm_adsp_create_control(dsp, alg_region);
 		} else {
 			adsp_warn(dsp, "Missing length info for region XM with ID %x\n",
 				  be32_to_cpu(adsp2_alg[i].alg.id));
 		}
 
-		region = kzalloc(sizeof(*region), GFP_KERNEL);
-		if (!region) {
+		alg_region = kzalloc(sizeof(*alg_region), GFP_KERNEL);
+		if (!alg_region) {
 			ret = -ENOMEM;
 			goto out;
 		}
-		region->type = WMFW_ADSP2_YM;
-		region->alg = be32_to_cpu(adsp2_alg[i].alg.id);
-		region->base = be32_to_cpu(adsp2_alg[i].ym);
-		region->len = 0;
-		list_add_tail(&region->list, &dsp->alg_regions);
-		if (i + 1 < algs) {
-			region->len = be32_to_cpu(adsp2_alg[i + 1].ym);
-			region->len -= be32_to_cpu(adsp2_alg[i].ym);
-			region->len *= 4;
-			wm_adsp_create_control(dsp, region);
+		alg_region->type = WMFW_ADSP2_YM;
+		alg_region->alg = be32_to_cpu(adsp2_alg[i].alg.id);
+		alg_region->base = be32_to_cpu(adsp2_alg[i].ym);
+		alg_region->len = 0;
+		list_add_tail(&alg_region->list, &dsp->alg_regions);
+		if (i + 1 < n_algs) {
+			alg_region->len = be32_to_cpu(adsp2_alg[i + 1].ym);
+			alg_region->len -= be32_to_cpu(adsp2_alg[i].ym);
+			alg_region->len *= 4;
+			wm_adsp_create_control(dsp, alg_region);
 		} else {
 			adsp_warn(dsp, "Missing length info for region YM with ID %x\n",
 				  be32_to_cpu(adsp2_alg[i].alg.id));
 		}
 
-		region = kzalloc(sizeof(*region), GFP_KERNEL);
-		if (!region) {
+		alg_region = kzalloc(sizeof(*alg_region), GFP_KERNEL);
+		if (!alg_region) {
 			ret = -ENOMEM;
 			goto out;
 		}
-		region->type = WMFW_ADSP2_ZM;
-		region->alg = be32_to_cpu(adsp2_alg[i].alg.id);
-		region->base = be32_to_cpu(adsp2_alg[i].zm);
-		region->len = 0;
-		list_add_tail(&region->list, &dsp->alg_regions);
-		if (i + 1 < algs) {
-			region->len = be32_to_cpu(adsp2_alg[i + 1].zm);
-			region->len -= be32_to_cpu(adsp2_alg[i].zm);
-			region->len *= 4;
-			wm_adsp_create_control(dsp, region);
+		alg_region->type = WMFW_ADSP2_ZM;
+		alg_region->alg = be32_to_cpu(adsp2_alg[i].alg.id);
+		alg_region->base = be32_to_cpu(adsp2_alg[i].zm);
+		alg_region->len = 0;
+		list_add_tail(&alg_region->list, &dsp->alg_regions);
+		if (i + 1 < n_algs) {
+			alg_region->len = be32_to_cpu(adsp2_alg[i + 1].zm);
+			alg_region->len -= be32_to_cpu(adsp2_alg[i].zm);
+			alg_region->len *= 4;
+			wm_adsp_create_control(dsp, alg_region);
 		} else {
 			adsp_warn(dsp, "Missing length info for region ZM with ID %x\n",
 				  be32_to_cpu(adsp2_alg[i].alg.id));
@@ -1351,9 +1351,9 @@ out:
 	return ret;
 }
 
-int wm_adsp1_init(struct wm_adsp *adsp)
+int wm_adsp1_init(struct wm_adsp *dsp)
 {
-	INIT_LIST_HEAD(&adsp->alg_regions);
+	INIT_LIST_HEAD(&dsp->alg_regions);
 
 	return 0;
 }
@@ -1691,7 +1691,7 @@ err:
 }
 EXPORT_SYMBOL_GPL(wm_adsp2_event);
 
-int wm_adsp2_init(struct wm_adsp *adsp, bool dvfs)
+int wm_adsp2_init(struct wm_adsp *dsp, bool dvfs)
 {
 	int ret;
 
@@ -1699,40 +1699,40 @@ int wm_adsp2_init(struct wm_adsp *adsp, bool dvfs)
 	 * Disable the DSP memory by default when in reset for a small
 	 * power saving.
 	 */
-	ret = regmap_update_bits(adsp->regmap, adsp->base + ADSP2_CONTROL,
+	ret = regmap_update_bits(dsp->regmap, dsp->base + ADSP2_CONTROL,
 				 ADSP2_MEM_ENA, 0);
 	if (ret != 0) {
-		adsp_err(adsp, "Failed to clear memory retention: %d\n", ret);
+		adsp_err(dsp, "Failed to clear memory retention: %d\n", ret);
 		return ret;
 	}
 
-	INIT_LIST_HEAD(&adsp->alg_regions);
-	INIT_LIST_HEAD(&adsp->ctl_list);
-	INIT_WORK(&adsp->boot_work, wm_adsp2_boot_work);
+	INIT_LIST_HEAD(&dsp->alg_regions);
+	INIT_LIST_HEAD(&dsp->ctl_list);
+	INIT_WORK(&dsp->boot_work, wm_adsp2_boot_work);
 
 	if (dvfs) {
-		adsp->dvfs = devm_regulator_get(adsp->dev, "DCVDD");
-		if (IS_ERR(adsp->dvfs)) {
-			ret = PTR_ERR(adsp->dvfs);
-			adsp_err(adsp, "Failed to get DCVDD: %d\n", ret);
+		dsp->dvfs = devm_regulator_get(dsp->dev, "DCVDD");
+		if (IS_ERR(dsp->dvfs)) {
+			ret = PTR_ERR(dsp->dvfs);
+			adsp_err(dsp, "Failed to get DCVDD: %d\n", ret);
 			return ret;
 		}
 
-		ret = regulator_enable(adsp->dvfs);
+		ret = regulator_enable(dsp->dvfs);
 		if (ret != 0) {
-			adsp_err(adsp, "Failed to enable DCVDD: %d\n", ret);
+			adsp_err(dsp, "Failed to enable DCVDD: %d\n", ret);
 			return ret;
 		}
 
-		ret = regulator_set_voltage(adsp->dvfs, 1200000, 1800000);
+		ret = regulator_set_voltage(dsp->dvfs, 1200000, 1800000);
 		if (ret != 0) {
-			adsp_err(adsp, "Failed to initialise DVFS: %d\n", ret);
+			adsp_err(dsp, "Failed to initialise DVFS: %d\n", ret);
 			return ret;
 		}
 
-		ret = regulator_disable(adsp->dvfs);
+		ret = regulator_disable(dsp->dvfs);
 		if (ret != 0) {
-			adsp_err(adsp, "Failed to disable DCVDD: %d\n", ret);
+			adsp_err(dsp, "Failed to disable DCVDD: %d\n", ret);
 			return ret;
 		}
 	}
diff --git a/sound/soc/codecs/wm_adsp.h b/sound/soc/codecs/wm_adsp.h
index a4f6b64..fc75a24 100644
--- a/sound/soc/codecs/wm_adsp.h
+++ b/sound/soc/codecs/wm_adsp.h
@@ -78,8 +78,8 @@ struct wm_adsp {
 extern const struct snd_kcontrol_new wm_adsp1_fw_controls[];
 extern const struct snd_kcontrol_new wm_adsp2_fw_controls[];
 
-int wm_adsp1_init(struct wm_adsp *adsp);
-int wm_adsp2_init(struct wm_adsp *adsp, bool dvfs);
+int wm_adsp1_init(struct wm_adsp *dsp);
+int wm_adsp2_init(struct wm_adsp *dsp, bool dvfs);
 int wm_adsp1_event(struct snd_soc_dapm_widget *w,
 		   struct snd_kcontrol *kcontrol, int event);
 int wm_adsp2_early_event(struct snd_soc_dapm_widget *w,
diff --git a/sound/soc/codecs/wmfw.h b/sound/soc/codecs/wmfw.h
index ef16336..34c14b5 100644
--- a/sound/soc/codecs/wmfw.h
+++ b/sound/soc/codecs/wmfw.h
@@ -61,7 +61,7 @@ struct wmfw_adsp1_id_hdr {
 	struct wmfw_id_hdr fw;
 	__be32 zm;
 	__be32 dm;
-	__be32 algs;
+	__be32 n_algs;
 } __packed;
 
 struct wmfw_adsp2_id_hdr {
@@ -69,7 +69,7 @@ struct wmfw_adsp2_id_hdr {
 	__be32 zm;
 	__be32 xm;
 	__be32 ym;
-	__be32 algs;
+	__be32 n_algs;
 } __packed;
 
 struct wmfw_alg_hdr {
-- 
1.7.2.5

[PATCH 03/13] ASoC: wm_adsp: Remove len field from wm_adsp_alg_region

[Charles Keepax]

The algorithm region information in the firmware doesn't contain a
length field, explicitly pass this to the create_control function rather
than bundling into wm_adsp_alg_region.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm_adsp.c |   55 ++++++++++++++++++++++---------------------
 sound/soc/codecs/wm_adsp.h |    1 -
 2 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index 4201e1f..3f6b49d 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -780,7 +780,8 @@ static void wm_adsp_ctl_work(struct work_struct *work)
 }
 
 static int wm_adsp_create_control(struct wm_adsp *dsp,
-				  const struct wm_adsp_alg_region *alg_region)
+				  const struct wm_adsp_alg_region *alg_region,
+				  unsigned int len)
 {
 	struct wm_coeff_ctl *ctl;
 	struct wmfw_ctl_work *ctl_work;
@@ -842,7 +843,12 @@ static int wm_adsp_create_control(struct wm_adsp *dsp,
 	ctl->ops.xput = wm_coeff_put;
 	ctl->dsp = dsp;
 
-	ctl->len = alg_region->len;
+	if (len > 512) {
+		adsp_warn(dsp, "Truncating control %s from %d\n",
+			  ctl->name, len);
+		len = 512;
+	}
+	ctl->len = len;
 	ctl->cache = kzalloc(ctl->len, GFP_KERNEL);
 	if (!ctl->cache) {
 		ret = -ENOMEM;
@@ -991,13 +997,12 @@ static int wm_adsp1_setup_algs(struct wm_adsp *dsp)
 		alg_region->type = WMFW_ADSP1_DM;
 		alg_region->alg = be32_to_cpu(adsp1_alg[i].alg.id);
 		alg_region->base = be32_to_cpu(adsp1_alg[i].dm);
-		alg_region->len = 0;
 		list_add_tail(&alg_region->list, &dsp->alg_regions);
 		if (i + 1 < n_algs) {
-			alg_region->len = be32_to_cpu(adsp1_alg[i + 1].dm);
-			alg_region->len -= be32_to_cpu(adsp1_alg[i].dm);
-			alg_region->len *= 4;
-			wm_adsp_create_control(dsp, alg_region);
+			len = be32_to_cpu(adsp1_alg[i + 1].dm);
+			len -= be32_to_cpu(adsp1_alg[i].dm);
+			len *= 4;
+			wm_adsp_create_control(dsp, alg_region, len);
 		} else {
 			adsp_warn(dsp, "Missing length info for region DM with ID %x\n",
 				  be32_to_cpu(adsp1_alg[i].alg.id));
@@ -1011,13 +1016,12 @@ static int wm_adsp1_setup_algs(struct wm_adsp *dsp)
 		alg_region->type = WMFW_ADSP1_ZM;
 		alg_region->alg = be32_to_cpu(adsp1_alg[i].alg.id);
 		alg_region->base = be32_to_cpu(adsp1_alg[i].zm);
-		alg_region->len = 0;
 		list_add_tail(&alg_region->list, &dsp->alg_regions);
 		if (i + 1 < n_algs) {
-			alg_region->len = be32_to_cpu(adsp1_alg[i + 1].zm);
-			alg_region->len -= be32_to_cpu(adsp1_alg[i].zm);
-			alg_region->len *= 4;
-			wm_adsp_create_control(dsp, alg_region);
+			len = be32_to_cpu(adsp1_alg[i + 1].zm);
+			len -= be32_to_cpu(adsp1_alg[i].zm);
+			len *= 4;
+			wm_adsp_create_control(dsp, alg_region, len);
 		} else {
 			adsp_warn(dsp, "Missing length info for region ZM with ID %x\n",
 				  be32_to_cpu(adsp1_alg[i].alg.id));
@@ -1110,13 +1114,12 @@ static int wm_adsp2_setup_algs(struct wm_adsp *dsp)
 		alg_region->type = WMFW_ADSP2_XM;
 		alg_region->alg = be32_to_cpu(adsp2_alg[i].alg.id);
 		alg_region->base = be32_to_cpu(adsp2_alg[i].xm);
-		alg_region->len = 0;
 		list_add_tail(&alg_region->list, &dsp->alg_regions);
 		if (i + 1 < n_algs) {
-			alg_region->len = be32_to_cpu(adsp2_alg[i + 1].xm);
-			alg_region->len -= be32_to_cpu(adsp2_alg[i].xm);
-			alg_region->len *= 4;
-			wm_adsp_create_control(dsp, alg_region);
+			len = be32_to_cpu(adsp2_alg[i + 1].xm);
+			len -= be32_to_cpu(adsp2_alg[i].xm);
+			len *= 4;
+			wm_adsp_create_control(dsp, alg_region, len);
 		} else {
 			adsp_warn(dsp, "Missing length info for region XM with ID %x\n",
 				  be32_to_cpu(adsp2_alg[i].alg.id));
@@ -1130,13 +1133,12 @@ static int wm_adsp2_setup_algs(struct wm_adsp *dsp)
 		alg_region->type = WMFW_ADSP2_YM;
 		alg_region->alg = be32_to_cpu(adsp2_alg[i].alg.id);
 		alg_region->base = be32_to_cpu(adsp2_alg[i].ym);
-		alg_region->len = 0;
 		list_add_tail(&alg_region->list, &dsp->alg_regions);
 		if (i + 1 < n_algs) {
-			alg_region->len = be32_to_cpu(adsp2_alg[i + 1].ym);
-			alg_region->len -= be32_to_cpu(adsp2_alg[i].ym);
-			alg_region->len *= 4;
-			wm_adsp_create_control(dsp, alg_region);
+			len = be32_to_cpu(adsp2_alg[i + 1].ym);
+			len -= be32_to_cpu(adsp2_alg[i].ym);
+			len *= 4;
+			wm_adsp_create_control(dsp, alg_region, len);
 		} else {
 			adsp_warn(dsp, "Missing length info for region YM with ID %x\n",
 				  be32_to_cpu(adsp2_alg[i].alg.id));
@@ -1150,13 +1152,12 @@ static int wm_adsp2_setup_algs(struct wm_adsp *dsp)
 		alg_region->type = WMFW_ADSP2_ZM;
 		alg_region->alg = be32_to_cpu(adsp2_alg[i].alg.id);
 		alg_region->base = be32_to_cpu(adsp2_alg[i].zm);
-		alg_region->len = 0;
 		list_add_tail(&alg_region->list, &dsp->alg_regions);
 		if (i + 1 < n_algs) {
-			alg_region->len = be32_to_cpu(adsp2_alg[i + 1].zm);
-			alg_region->len -= be32_to_cpu(adsp2_alg[i].zm);
-			alg_region->len *= 4;
-			wm_adsp_create_control(dsp, alg_region);
+			len = be32_to_cpu(adsp2_alg[i + 1].zm);
+			len -= be32_to_cpu(adsp2_alg[i].zm);
+			len *= 4;
+			wm_adsp_create_control(dsp, alg_region, len);
 		} else {
 			adsp_warn(dsp, "Missing length info for region ZM with ID %x\n",
 				  be32_to_cpu(adsp2_alg[i].alg.id));
diff --git a/sound/soc/codecs/wm_adsp.h b/sound/soc/codecs/wm_adsp.h
index fc75a24..0ad14e0 100644
--- a/sound/soc/codecs/wm_adsp.h
+++ b/sound/soc/codecs/wm_adsp.h
@@ -30,7 +30,6 @@ struct wm_adsp_alg_region {
 	unsigned int alg;
 	int type;
 	unsigned int base;
-	size_t len;
 };
 
 struct wm_adsp {
-- 
1.7.2.5

[PATCH 04/13] ASoC: wm_adsp: Limit firmware control name to ALSA control name size

[Charles Keepax]

ALSA only supports control names up to 44 bytes, so there is no point
allocating a whole page of memory to hold the control name, just limit
the control name to 44 bytes.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm_adsp.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index 3f6b49d..c291203 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -789,7 +789,7 @@ static int wm_adsp_create_control(struct wm_adsp *dsp,
 	char *region_name;
 	int ret;
 
-	name = kmalloc(PAGE_SIZE, GFP_KERNEL);
+	name = kmalloc(SNDRV_CTL_ELEM_ID_NAME_MAXLEN, GFP_KERNEL);
 	if (!name)
 		return -ENOMEM;
 
@@ -814,7 +814,7 @@ static int wm_adsp_create_control(struct wm_adsp *dsp,
 		goto err_name;
 	}
 
-	snprintf(name, PAGE_SIZE, "DSP%d %s %x",
+	snprintf(name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN, "DSP%d %s %x",
 		 dsp->num, region_name, alg_region->alg);
 
 	list_for_each_entry(ctl, &dsp->ctl_list,
-- 
1.7.2.5

[PATCH 05/13] ASoC: wm_adsp: Move temporary control name to the stack

[Charles Keepax]

Now we only allocate 44 bytes for the control name keep it on the stack
to avoid a lot of pointless memory allocation.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm_adsp.c |   23 ++++++-----------------
 1 files changed, 6 insertions(+), 17 deletions(-)

diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index c291203..6c4f013 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -785,14 +785,10 @@ static int wm_adsp_create_control(struct wm_adsp *dsp,
 {
 	struct wm_coeff_ctl *ctl;
 	struct wmfw_ctl_work *ctl_work;
-	char *name;
+	char name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
 	char *region_name;
 	int ret;
 
-	name = kmalloc(SNDRV_CTL_ELEM_ID_NAME_MAXLEN, GFP_KERNEL);
-	if (!name)
-		return -ENOMEM;
-
 	switch (alg_region->type) {
 	case WMFW_ADSP1_PM:
 		region_name = "PM";
@@ -810,8 +806,7 @@ static int wm_adsp_create_control(struct wm_adsp *dsp,
 		region_name = "ZM";
 		break;
 	default:
-		ret = -EINVAL;
-		goto err_name;
+		return -EINVAL;
 	}
 
 	snprintf(name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN, "DSP%d %s %x",
@@ -822,15 +817,13 @@ static int wm_adsp_create_control(struct wm_adsp *dsp,
 		if (!strcmp(ctl->name, name)) {
 			if (!ctl->enabled)
 				ctl->enabled = 1;
-			goto found;
+			return 0;
 		}
 	}
 
 	ctl = kzalloc(sizeof(*ctl), GFP_KERNEL);
-	if (!ctl) {
-		ret = -ENOMEM;
-		goto err_name;
-	}
+	if (!ctl)
+		return -ENOMEM;
 	ctl->alg_region = *alg_region;
 	ctl->name = kmemdup(name, strlen(name) + 1, GFP_KERNEL);
 	if (!ctl->name) {
@@ -866,9 +859,6 @@ static int wm_adsp_create_control(struct wm_adsp *dsp,
 	INIT_WORK(&ctl_work->work, wm_adsp_ctl_work);
 	schedule_work(&ctl_work->work);
 
-found:
-	kfree(name);
-
 	return 0;
 
 err_ctl_cache:
@@ -877,8 +867,7 @@ err_ctl_name:
 	kfree(ctl->name);
 err_ctl:
 	kfree(ctl);
-err_name:
-	kfree(name);
+
 	return ret;
 }
 
-- 
1.7.2.5

[PATCH 06/13] ASoC: wm_adsp: Clean up low level control read/write functions

[Charles Keepax]

Physically reading and writing controls to/from the DSP are handled by
two low level functions (wm_coeff_{write|read}_control, these currently
take in a snd_kcontrol pointer but immediately pull out a wm_coeff_ctl
pointer from the private data. These functions don't handle the kcontrols
at all they just shuttle data to and from the chip and all the call
sites have a wm_coeff_ctl pointer available. This patch just passes the
wm_coeff_ctl pointer straight into these functions.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm_adsp.c |   12 +++++-------
 1 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index 6c4f013..37e01b0 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -372,10 +372,9 @@ static int wm_coeff_info(struct snd_kcontrol *kcontrol,
 	return 0;
 }
 
-static int wm_coeff_write_control(struct snd_kcontrol *kcontrol,
+static int wm_coeff_write_control(struct wm_coeff_ctl *ctl,
 				  const void *buf, size_t len)
 {
-	struct wm_coeff_ctl *ctl = (struct wm_coeff_ctl *)kcontrol->private_value;
 	struct wm_adsp_alg_region *alg_region = &ctl->alg_region;
 	const struct wm_adsp_region *mem;
 	struct wm_adsp *dsp = ctl->dsp;
@@ -424,13 +423,12 @@ static int wm_coeff_put(struct snd_kcontrol *kcontrol,
 	if (!ctl->enabled)
 		return 0;
 
-	return wm_coeff_write_control(kcontrol, p, ctl->len);
+	return wm_coeff_write_control(ctl, p, ctl->len);
 }
 
-static int wm_coeff_read_control(struct snd_kcontrol *kcontrol,
+static int wm_coeff_read_control(struct wm_coeff_ctl *ctl,
 				 void *buf, size_t len)
 {
-	struct wm_coeff_ctl *ctl = (struct wm_coeff_ctl *)kcontrol->private_value;
 	struct wm_adsp_alg_region *alg_region = &ctl->alg_region;
 	const struct wm_adsp_region *mem;
 	struct wm_adsp *dsp = ctl->dsp;
@@ -739,7 +737,7 @@ static int wm_coeff_init_control_caches(struct wm_adsp *dsp)
 	list_for_each_entry(ctl, &dsp->ctl_list, list) {
 		if (!ctl->enabled || ctl->set)
 			continue;
-		ret = wm_coeff_read_control(ctl->kcontrol,
+		ret = wm_coeff_read_control(ctl,
 					    ctl->cache,
 					    ctl->len);
 		if (ret < 0)
@@ -758,7 +756,7 @@ static int wm_coeff_sync_controls(struct wm_adsp *dsp)
 		if (!ctl->enabled)
 			continue;
 		if (ctl->set) {
-			ret = wm_coeff_write_control(ctl->kcontrol,
+			ret = wm_coeff_write_control(ctl,
 						     ctl->cache,
 						     ctl->len);
 			if (ret < 0)
-- 
1.7.2.5

[PATCH 07/13] ASoC: wm_adsp: Factor out creation of alg_regions

[Charles Keepax]

Tidy up the code a little by factoring out the creation of the algorithm
regions.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm_adsp.c |  134 +++++++++++++++++++++-----------------------
 1 files changed, 64 insertions(+), 70 deletions(-)

diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index 37e01b0..9283d08 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -913,6 +913,25 @@ static void *wm_adsp_read_algs(struct wm_adsp *dsp, size_t n_algs,
 	return alg;
 }
 
+static struct wm_adsp_alg_region *wm_adsp_create_region(struct wm_adsp *dsp,
+							int type, __be32 id,
+							__be32 base)
+{
+	struct wm_adsp_alg_region *alg_region;
+
+	alg_region = kzalloc(sizeof(*alg_region), GFP_KERNEL);
+	if (!alg_region)
+		return ERR_PTR(-ENOMEM);
+
+	alg_region->type = type;
+	alg_region->alg = be32_to_cpu(id);
+	alg_region->base = be32_to_cpu(base);
+
+	list_add_tail(&alg_region->list, &dsp->alg_regions);
+
+	return alg_region;
+}
+
 static int wm_adsp1_setup_algs(struct wm_adsp *dsp)
 {
 	struct wmfw_adsp1_id_hdr adsp1_id;
@@ -944,21 +963,15 @@ static int wm_adsp1_setup_algs(struct wm_adsp *dsp)
 		  be32_to_cpu(adsp1_id.fw.ver) & 0xff,
 		  n_algs);
 
-	alg_region = kzalloc(sizeof(*alg_region), GFP_KERNEL);
-	if (!alg_region)
-		return -ENOMEM;
-	alg_region->type = WMFW_ADSP1_ZM;
-	alg_region->alg = be32_to_cpu(adsp1_id.fw.id);
-	alg_region->base = be32_to_cpu(adsp1_id.zm);
-	list_add_tail(&alg_region->list, &dsp->alg_regions);
+	alg_region = wm_adsp_create_region(dsp, WMFW_ADSP1_ZM,
+					   adsp1_id.fw.id, adsp1_id.zm);
+	if (IS_ERR(alg_region))
+		return PTR_ERR(alg_region);
 
-	alg_region = kzalloc(sizeof(*alg_region), GFP_KERNEL);
-	if (!alg_region)
-		return -ENOMEM;
-	alg_region->type = WMFW_ADSP1_DM;
-	alg_region->alg = be32_to_cpu(adsp1_id.fw.id);
-	alg_region->base = be32_to_cpu(adsp1_id.dm);
-	list_add_tail(&alg_region->list, &dsp->alg_regions);
+	alg_region = wm_adsp_create_region(dsp, WMFW_ADSP1_DM,
+					   adsp1_id.fw.id, adsp1_id.dm);
+	if (IS_ERR(alg_region))
+		return PTR_ERR(alg_region);
 
 	pos = sizeof(adsp1_id) / 2;
 	len = (sizeof(*adsp1_alg) * n_algs) / 2;
@@ -976,15 +989,13 @@ static int wm_adsp1_setup_algs(struct wm_adsp *dsp)
 			  be32_to_cpu(adsp1_alg[i].dm),
 			  be32_to_cpu(adsp1_alg[i].zm));
 
-		alg_region = kzalloc(sizeof(*alg_region), GFP_KERNEL);
-		if (!alg_region) {
-			ret = -ENOMEM;
+		alg_region = wm_adsp_create_region(dsp, WMFW_ADSP1_DM,
+						   adsp1_alg[i].alg.id,
+						   adsp1_alg[i].dm);
+		if (IS_ERR(alg_region)) {
+			ret = PTR_ERR(alg_region);
 			goto out;
 		}
-		alg_region->type = WMFW_ADSP1_DM;
-		alg_region->alg = be32_to_cpu(adsp1_alg[i].alg.id);
-		alg_region->base = be32_to_cpu(adsp1_alg[i].dm);
-		list_add_tail(&alg_region->list, &dsp->alg_regions);
 		if (i + 1 < n_algs) {
 			len = be32_to_cpu(adsp1_alg[i + 1].dm);
 			len -= be32_to_cpu(adsp1_alg[i].dm);
@@ -995,15 +1006,13 @@ static int wm_adsp1_setup_algs(struct wm_adsp *dsp)
 				  be32_to_cpu(adsp1_alg[i].alg.id));
 		}
 
-		alg_region = kzalloc(sizeof(*alg_region), GFP_KERNEL);
-		if (!alg_region) {
-			ret = -ENOMEM;
+		alg_region = wm_adsp_create_region(dsp, WMFW_ADSP1_ZM,
+						   adsp1_alg[i].alg.id,
+						   adsp1_alg[i].zm);
+		if (IS_ERR(alg_region)) {
+			ret = PTR_ERR(alg_region);
 			goto out;
 		}
-		alg_region->type = WMFW_ADSP1_ZM;
-		alg_region->alg = be32_to_cpu(adsp1_alg[i].alg.id);
-		alg_region->base = be32_to_cpu(adsp1_alg[i].zm);
-		list_add_tail(&alg_region->list, &dsp->alg_regions);
 		if (i + 1 < n_algs) {
 			len = be32_to_cpu(adsp1_alg[i + 1].zm);
 			len -= be32_to_cpu(adsp1_alg[i].zm);
@@ -1051,29 +1060,20 @@ static int wm_adsp2_setup_algs(struct wm_adsp *dsp)
 		  be32_to_cpu(adsp2_id.fw.ver) & 0xff,
 		  n_algs);
 
-	alg_region = kzalloc(sizeof(*alg_region), GFP_KERNEL);
-	if (!alg_region)
-		return -ENOMEM;
-	alg_region->type = WMFW_ADSP2_XM;
-	alg_region->alg = be32_to_cpu(adsp2_id.fw.id);
-	alg_region->base = be32_to_cpu(adsp2_id.xm);
-	list_add_tail(&alg_region->list, &dsp->alg_regions);
+	alg_region = wm_adsp_create_region(dsp, WMFW_ADSP2_XM,
+					   adsp2_id.fw.id, adsp2_id.xm);
+	if (IS_ERR(alg_region))
+		return PTR_ERR(alg_region);
 
-	alg_region = kzalloc(sizeof(*alg_region), GFP_KERNEL);
-	if (!alg_region)
-		return -ENOMEM;
-	alg_region->type = WMFW_ADSP2_YM;
-	alg_region->alg = be32_to_cpu(adsp2_id.fw.id);
-	alg_region->base = be32_to_cpu(adsp2_id.ym);
-	list_add_tail(&alg_region->list, &dsp->alg_regions);
+	alg_region = wm_adsp_create_region(dsp, WMFW_ADSP2_YM,
+					   adsp2_id.fw.id, adsp2_id.ym);
+	if (IS_ERR(alg_region))
+		return PTR_ERR(alg_region);
 
-	alg_region = kzalloc(sizeof(*alg_region), GFP_KERNEL);
-	if (!alg_region)
-		return -ENOMEM;
-	alg_region->type = WMFW_ADSP2_ZM;
-	alg_region->alg = be32_to_cpu(adsp2_id.fw.id);
-	alg_region->base = be32_to_cpu(adsp2_id.zm);
-	list_add_tail(&alg_region->list, &dsp->alg_regions);
+	alg_region = wm_adsp_create_region(dsp, WMFW_ADSP2_ZM,
+					   adsp2_id.fw.id, adsp2_id.zm);
+	if (IS_ERR(alg_region))
+		return PTR_ERR(alg_region);
 
 	pos = sizeof(adsp2_id) / 2;
 	len = (sizeof(*adsp2_alg) * n_algs) / 2;
@@ -1093,15 +1093,13 @@ static int wm_adsp2_setup_algs(struct wm_adsp *dsp)
 			  be32_to_cpu(adsp2_alg[i].ym),
 			  be32_to_cpu(adsp2_alg[i].zm));
 
-		alg_region = kzalloc(sizeof(*alg_region), GFP_KERNEL);
-		if (!alg_region) {
-			ret = -ENOMEM;
+		alg_region = wm_adsp_create_region(dsp, WMFW_ADSP2_XM,
+						   adsp2_alg[i].alg.id,
+						   adsp2_alg[i].xm);
+		if (IS_ERR(alg_region)) {
+			ret = PTR_ERR(alg_region);
 			goto out;
 		}
-		alg_region->type = WMFW_ADSP2_XM;
-		alg_region->alg = be32_to_cpu(adsp2_alg[i].alg.id);
-		alg_region->base = be32_to_cpu(adsp2_alg[i].xm);
-		list_add_tail(&alg_region->list, &dsp->alg_regions);
 		if (i + 1 < n_algs) {
 			len = be32_to_cpu(adsp2_alg[i + 1].xm);
 			len -= be32_to_cpu(adsp2_alg[i].xm);
@@ -1112,15 +1110,13 @@ static int wm_adsp2_setup_algs(struct wm_adsp *dsp)
 				  be32_to_cpu(adsp2_alg[i].alg.id));
 		}
 
-		alg_region = kzalloc(sizeof(*alg_region), GFP_KERNEL);
-		if (!alg_region) {
-			ret = -ENOMEM;
+		alg_region = wm_adsp_create_region(dsp, WMFW_ADSP2_YM,
+						   adsp2_alg[i].alg.id,
+						   adsp2_alg[i].ym);
+		if (IS_ERR(alg_region)) {
+			ret = PTR_ERR(alg_region);
 			goto out;
 		}
-		alg_region->type = WMFW_ADSP2_YM;
-		alg_region->alg = be32_to_cpu(adsp2_alg[i].alg.id);
-		alg_region->base = be32_to_cpu(adsp2_alg[i].ym);
-		list_add_tail(&alg_region->list, &dsp->alg_regions);
 		if (i + 1 < n_algs) {
 			len = be32_to_cpu(adsp2_alg[i + 1].ym);
 			len -= be32_to_cpu(adsp2_alg[i].ym);
@@ -1131,15 +1127,13 @@ static int wm_adsp2_setup_algs(struct wm_adsp *dsp)
 				  be32_to_cpu(adsp2_alg[i].alg.id));
 		}
 
-		alg_region = kzalloc(sizeof(*alg_region), GFP_KERNEL);
-		if (!alg_region) {
-			ret = -ENOMEM;
+		alg_region = wm_adsp_create_region(dsp, WMFW_ADSP2_ZM,
+						   adsp2_alg[i].alg.id,
+						   adsp2_alg[i].zm);
+		if (IS_ERR(alg_region)) {
+			ret = PTR_ERR(alg_region);
 			goto out;
 		}
-		alg_region->type = WMFW_ADSP2_ZM;
-		alg_region->alg = be32_to_cpu(adsp2_alg[i].alg.id);
-		alg_region->base = be32_to_cpu(adsp2_alg[i].zm);
-		list_add_tail(&alg_region->list, &dsp->alg_regions);
 		if (i + 1 < n_algs) {
 			len = be32_to_cpu(adsp2_alg[i + 1].zm);
 			len -= be32_to_cpu(adsp2_alg[i].zm);
-- 
1.7.2.5

[PATCH 08/13] ASoC: wm_adsp: Remove private field from wm_coeff_ctl

[Charles Keepax]

The private field in wm_coeff_ctl is currently unused and given the
controls are entirely handled within the ADSP code it is not clear what
it would be used for in the future. Remove the field for now it can be
readded if it is ever required.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm_adsp.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index 9283d08..d6e8913 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -232,7 +232,6 @@ struct wm_coeff_ctl {
 	struct wm_adsp_alg_region alg_region;
 	struct wm_coeff_ctl_ops ops;
 	struct wm_adsp *dsp;
-	void *private;
 	unsigned int enabled:1;
 	struct list_head list;
 	void *cache;
-- 
1.7.2.5

[PATCH 09/13] ASoC: wm_adsp: Group all the ALSA control functions together

[Charles Keepax]

This is slightly logically better and avoids some unnecessary forward
declarations in the following refactoring.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm_adsp.c |  280 ++++++++++++++++++++++----------------------
 1 files changed, 140 insertions(+), 140 deletions(-)

diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index d6e8913..f42b453 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -518,6 +518,146 @@ err_kcontrol:
 	return ret;
 }
 
+static int wm_coeff_init_control_caches(struct wm_adsp *dsp)
+{
+	struct wm_coeff_ctl *ctl;
+	int ret;
+
+	list_for_each_entry(ctl, &dsp->ctl_list, list) {
+		if (!ctl->enabled || ctl->set)
+			continue;
+		ret = wm_coeff_read_control(ctl,
+					    ctl->cache,
+					    ctl->len);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int wm_coeff_sync_controls(struct wm_adsp *dsp)
+{
+	struct wm_coeff_ctl *ctl;
+	int ret;
+
+	list_for_each_entry(ctl, &dsp->ctl_list, list) {
+		if (!ctl->enabled)
+			continue;
+		if (ctl->set) {
+			ret = wm_coeff_write_control(ctl,
+						     ctl->cache,
+						     ctl->len);
+			if (ret < 0)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+
+static void wm_adsp_ctl_work(struct work_struct *work)
+{
+	struct wmfw_ctl_work *ctl_work = container_of(work,
+						      struct wmfw_ctl_work,
+						      work);
+
+	wmfw_add_ctl(ctl_work->dsp, ctl_work->ctl);
+	kfree(ctl_work);
+}
+
+static int wm_adsp_create_control(struct wm_adsp *dsp,
+				  const struct wm_adsp_alg_region *alg_region,
+				  unsigned int len)
+{
+	struct wm_coeff_ctl *ctl;
+	struct wmfw_ctl_work *ctl_work;
+	char name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
+	char *region_name;
+	int ret;
+
+	switch (alg_region->type) {
+	case WMFW_ADSP1_PM:
+		region_name = "PM";
+		break;
+	case WMFW_ADSP1_DM:
+		region_name = "DM";
+		break;
+	case WMFW_ADSP2_XM:
+		region_name = "XM";
+		break;
+	case WMFW_ADSP2_YM:
+		region_name = "YM";
+		break;
+	case WMFW_ADSP1_ZM:
+		region_name = "ZM";
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	snprintf(name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN, "DSP%d %s %x",
+		 dsp->num, region_name, alg_region->alg);
+
+	list_for_each_entry(ctl, &dsp->ctl_list,
+			    list) {
+		if (!strcmp(ctl->name, name)) {
+			if (!ctl->enabled)
+				ctl->enabled = 1;
+			return 0;
+		}
+	}
+
+	ctl = kzalloc(sizeof(*ctl), GFP_KERNEL);
+	if (!ctl)
+		return -ENOMEM;
+	ctl->alg_region = *alg_region;
+	ctl->name = kmemdup(name, strlen(name) + 1, GFP_KERNEL);
+	if (!ctl->name) {
+		ret = -ENOMEM;
+		goto err_ctl;
+	}
+	ctl->enabled = 1;
+	ctl->set = 0;
+	ctl->ops.xget = wm_coeff_get;
+	ctl->ops.xput = wm_coeff_put;
+	ctl->dsp = dsp;
+
+	if (len > 512) {
+		adsp_warn(dsp, "Truncating control %s from %d\n",
+			  ctl->name, len);
+		len = 512;
+	}
+	ctl->len = len;
+	ctl->cache = kzalloc(ctl->len, GFP_KERNEL);
+	if (!ctl->cache) {
+		ret = -ENOMEM;
+		goto err_ctl_name;
+	}
+
+	ctl_work = kzalloc(sizeof(*ctl_work), GFP_KERNEL);
+	if (!ctl_work) {
+		ret = -ENOMEM;
+		goto err_ctl_cache;
+	}
+
+	ctl_work->dsp = dsp;
+	ctl_work->ctl = ctl;
+	INIT_WORK(&ctl_work->work, wm_adsp_ctl_work);
+	schedule_work(&ctl_work->work);
+
+	return 0;
+
+err_ctl_cache:
+	kfree(ctl->cache);
+err_ctl_name:
+	kfree(ctl->name);
+err_ctl:
+	kfree(ctl);
+
+	return ret;
+}
+
 static int wm_adsp_load(struct wm_adsp *dsp)
 {
 	LIST_HEAD(buf_list);
@@ -728,146 +868,6 @@ out:
 	return ret;
 }
 
-static int wm_coeff_init_control_caches(struct wm_adsp *dsp)
-{
-	struct wm_coeff_ctl *ctl;
-	int ret;
-
-	list_for_each_entry(ctl, &dsp->ctl_list, list) {
-		if (!ctl->enabled || ctl->set)
-			continue;
-		ret = wm_coeff_read_control(ctl,
-					    ctl->cache,
-					    ctl->len);
-		if (ret < 0)
-			return ret;
-	}
-
-	return 0;
-}
-
-static int wm_coeff_sync_controls(struct wm_adsp *dsp)
-{
-	struct wm_coeff_ctl *ctl;
-	int ret;
-
-	list_for_each_entry(ctl, &dsp->ctl_list, list) {
-		if (!ctl->enabled)
-			continue;
-		if (ctl->set) {
-			ret = wm_coeff_write_control(ctl,
-						     ctl->cache,
-						     ctl->len);
-			if (ret < 0)
-				return ret;
-		}
-	}
-
-	return 0;
-}
-
-static void wm_adsp_ctl_work(struct work_struct *work)
-{
-	struct wmfw_ctl_work *ctl_work = container_of(work,
-						      struct wmfw_ctl_work,
-						      work);
-
-	wmfw_add_ctl(ctl_work->dsp, ctl_work->ctl);
-	kfree(ctl_work);
-}
-
-static int wm_adsp_create_control(struct wm_adsp *dsp,
-				  const struct wm_adsp_alg_region *alg_region,
-				  unsigned int len)
-{
-	struct wm_coeff_ctl *ctl;
-	struct wmfw_ctl_work *ctl_work;
-	char name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
-	char *region_name;
-	int ret;
-
-	switch (alg_region->type) {
-	case WMFW_ADSP1_PM:
-		region_name = "PM";
-		break;
-	case WMFW_ADSP1_DM:
-		region_name = "DM";
-		break;
-	case WMFW_ADSP2_XM:
-		region_name = "XM";
-		break;
-	case WMFW_ADSP2_YM:
-		region_name = "YM";
-		break;
-	case WMFW_ADSP1_ZM:
-		region_name = "ZM";
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	snprintf(name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN, "DSP%d %s %x",
-		 dsp->num, region_name, alg_region->alg);
-
-	list_for_each_entry(ctl, &dsp->ctl_list,
-			    list) {
-		if (!strcmp(ctl->name, name)) {
-			if (!ctl->enabled)
-				ctl->enabled = 1;
-			return 0;
-		}
-	}
-
-	ctl = kzalloc(sizeof(*ctl), GFP_KERNEL);
-	if (!ctl)
-		return -ENOMEM;
-	ctl->alg_region = *alg_region;
-	ctl->name = kmemdup(name, strlen(name) + 1, GFP_KERNEL);
-	if (!ctl->name) {
-		ret = -ENOMEM;
-		goto err_ctl;
-	}
-	ctl->enabled = 1;
-	ctl->set = 0;
-	ctl->ops.xget = wm_coeff_get;
-	ctl->ops.xput = wm_coeff_put;
-	ctl->dsp = dsp;
-
-	if (len > 512) {
-		adsp_warn(dsp, "Truncating control %s from %d\n",
-			  ctl->name, len);
-		len = 512;
-	}
-	ctl->len = len;
-	ctl->cache = kzalloc(ctl->len, GFP_KERNEL);
-	if (!ctl->cache) {
-		ret = -ENOMEM;
-		goto err_ctl_name;
-	}
-
-	ctl_work = kzalloc(sizeof(*ctl_work), GFP_KERNEL);
-	if (!ctl_work) {
-		ret = -ENOMEM;
-		goto err_ctl_cache;
-	}
-
-	ctl_work->dsp = dsp;
-	ctl_work->ctl = ctl;
-	INIT_WORK(&ctl_work->work, wm_adsp_ctl_work);
-	schedule_work(&ctl_work->work);
-
-	return 0;
-
-err_ctl_cache:
-	kfree(ctl->cache);
-err_ctl_name:
-	kfree(ctl->name);
-err_ctl:
-	kfree(ctl);
-
-	return ret;
-}
-
 static void *wm_adsp_read_algs(struct wm_adsp *dsp, size_t n_algs,
 			       unsigned int pos, unsigned int len)
 {
-- 
1.7.2.5

[PATCH 10/13] ASoC: wm_adsp: Add basic support for rev 1 firmware file format

[Charles Keepax]

Revision one of the file format includes new algorithm and coefficient
blocks which provide additional information about the controls exported
by the firmware. This patch updates the processing to handle this
version of the file format. Note that whilst this version of the format
adds support for specifying a name for the control through the firmware
file this has not been used and to keep compatibility with existing
deployments no changes to the firmware control naming are made by this
patch.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm_adsp.c |  239 +++++++++++++++++++++++++++++++++++---------
 sound/soc/codecs/wm_adsp.h |    1 +
 sound/soc/codecs/wmfw.h    |   35 ++++++-
 3 files changed, 226 insertions(+), 49 deletions(-)

diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index f42b453..16b308a 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -229,12 +229,14 @@ struct wm_coeff_ctl_ops {
 
 struct wm_coeff_ctl {
 	const char *name;
+	const char *fw_name;
 	struct wm_adsp_alg_region alg_region;
 	struct wm_coeff_ctl_ops ops;
 	struct wm_adsp *dsp;
 	unsigned int enabled:1;
 	struct list_head list;
 	void *cache;
+	unsigned int offset;
 	size_t len;
 	unsigned int set:1;
 	struct snd_kcontrol *kcontrol;
@@ -388,7 +390,7 @@ static int wm_coeff_write_control(struct wm_coeff_ctl *ctl,
 		return -EINVAL;
 	}
 
-	reg = ctl->alg_region.base;
+	reg = ctl->alg_region.base + ctl->offset;
 	reg = wm_adsp_region_to_reg(mem, reg);
 
 	scratch = kmemdup(buf, ctl->len, GFP_KERNEL | GFP_DMA);
@@ -442,7 +444,7 @@ static int wm_coeff_read_control(struct wm_coeff_ctl *ctl,
 		return -EINVAL;
 	}
 
-	reg = ctl->alg_region.base;
+	reg = ctl->alg_region.base + ctl->offset;
 	reg = wm_adsp_region_to_reg(mem, reg);
 
 	scratch = kmalloc(ctl->len, GFP_KERNEL | GFP_DMA);
@@ -509,8 +511,6 @@ static int wmfw_add_ctl(struct wm_adsp *dsp, struct wm_coeff_ctl *ctl)
 	ctl->kcontrol = snd_soc_card_get_kcontrol(dsp->card,
 						  ctl->name);
 
-	list_add(&ctl->list, &dsp->ctl_list);
-
 	return 0;
 
 err_kcontrol:
@@ -568,7 +568,8 @@ static void wm_adsp_ctl_work(struct work_struct *work)
 
 static int wm_adsp_create_control(struct wm_adsp *dsp,
 				  const struct wm_adsp_alg_region *alg_region,
-				  unsigned int len)
+				  unsigned int offset, unsigned int len,
+				  const char *subname, unsigned int subname_len)
 {
 	struct wm_coeff_ctl *ctl;
 	struct wmfw_ctl_work *ctl_work;
@@ -593,6 +594,7 @@ static int wm_adsp_create_control(struct wm_adsp *dsp,
 		region_name = "ZM";
 		break;
 	default:
+		adsp_err(dsp, "Unknown region type: %d\n", alg_region->type);
 		return -EINVAL;
 	}
 
@@ -611,6 +613,7 @@ static int wm_adsp_create_control(struct wm_adsp *dsp,
 	ctl = kzalloc(sizeof(*ctl), GFP_KERNEL);
 	if (!ctl)
 		return -ENOMEM;
+	ctl->fw_name = wm_adsp_fw_text[dsp->fw];
 	ctl->alg_region = *alg_region;
 	ctl->name = kmemdup(name, strlen(name) + 1, GFP_KERNEL);
 	if (!ctl->name) {
@@ -623,6 +626,7 @@ static int wm_adsp_create_control(struct wm_adsp *dsp,
 	ctl->ops.xput = wm_coeff_put;
 	ctl->dsp = dsp;
 
+	ctl->offset = offset;
 	if (len > 512) {
 		adsp_warn(dsp, "Truncating control %s from %d\n",
 			  ctl->name, len);
@@ -635,6 +639,8 @@ static int wm_adsp_create_control(struct wm_adsp *dsp,
 		goto err_ctl_name;
 	}
 
+	list_add(&ctl->list, &dsp->ctl_list);
+
 	ctl_work = kzalloc(sizeof(*ctl_work), GFP_KERNEL);
 	if (!ctl_work) {
 		ret = -ENOMEM;
@@ -658,6 +664,103 @@ err_ctl:
 	return ret;
 }
 
+struct wm_coeff_parsed_alg {
+	int id;
+	const u8 *name;
+	int name_len;
+	int ncoeff;
+};
+
+struct wm_coeff_parsed_coeff {
+	int offset;
+	int mem_type;
+	const u8 *name;
+	int name_len;
+	int ctl_type;
+	int flags;
+	int len;
+};
+
+static inline void wm_coeff_parse_alg(struct wm_adsp *dsp, const u8 **data,
+				      struct wm_coeff_parsed_alg *blk)
+{
+	const struct wmfw_adsp_alg_data *raw;
+
+	raw = (const struct wmfw_adsp_alg_data *)*data;
+	*data = raw->data;
+
+	blk->id = le32_to_cpu(raw->id);
+	blk->name = raw->name;
+	blk->name_len = strlen(raw->name);
+	blk->ncoeff = le32_to_cpu(raw->ncoeff);
+
+	adsp_dbg(dsp, "Algorithm ID: %#x\n", blk->id);
+	adsp_dbg(dsp, "Algorithm name: %.*s\n", blk->name_len, blk->name);
+	adsp_dbg(dsp, "# of coefficient descriptors: %#x\n", blk->ncoeff);
+}
+
+static inline void wm_coeff_parse_coeff(struct wm_adsp *dsp, const u8 **data,
+					struct wm_coeff_parsed_coeff *blk)
+{
+	const struct wmfw_adsp_coeff_data *raw;
+
+	raw = (const struct wmfw_adsp_coeff_data *)*data;
+	*data = *data + sizeof(raw->hdr) + le32_to_cpu(raw->hdr.size);
+
+	blk->offset = le16_to_cpu(raw->hdr.offset);
+	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->flags = le16_to_cpu(raw->flags);
+	blk->len = le32_to_cpu(raw->len);
+
+	adsp_dbg(dsp, "\tCoefficient type: %#x\n", blk->mem_type);
+	adsp_dbg(dsp, "\tCoefficient offset: %#x\n", blk->offset);
+	adsp_dbg(dsp, "\tCoefficient name: %.*s\n", blk->name_len, blk->name);
+	adsp_dbg(dsp, "\tCoefficient flags: %#x\n", blk->flags);
+	adsp_dbg(dsp, "\tALSA control type: %#x\n", blk->ctl_type);
+	adsp_dbg(dsp, "\tALSA control len: %#x\n", blk->len);
+}
+
+static int wm_adsp_parse_coeff(struct wm_adsp *dsp,
+			       const struct wmfw_region *region)
+{
+	struct wm_adsp_alg_region alg_region = {};
+	struct wm_coeff_parsed_alg alg_blk;
+	struct wm_coeff_parsed_coeff coeff_blk;
+	const u8 *data = region->data;
+	int i, ret;
+
+	wm_coeff_parse_alg(dsp, &data, &alg_blk);
+	for (i = 0; i < alg_blk.ncoeff; i++) {
+		wm_coeff_parse_coeff(dsp, &data, &coeff_blk);
+
+		switch (coeff_blk.ctl_type) {
+		case SNDRV_CTL_ELEM_TYPE_BYTES:
+			break;
+		default:
+			adsp_err(dsp, "Unknown control type: %d\n",
+				 coeff_blk.ctl_type);
+			return -EINVAL;
+		}
+
+		alg_region.type = coeff_blk.mem_type;
+		alg_region.alg = alg_blk.id;
+
+		ret = wm_adsp_create_control(dsp, &alg_region,
+					     coeff_blk.offset,
+					     coeff_blk.len,
+					     coeff_blk.name,
+					     coeff_blk.name_len);
+		if (ret < 0)
+			adsp_err(dsp, "Failed to create control: %.*s, %d\n",
+				 coeff_blk.name_len, coeff_blk.name, ret);
+	}
+
+	return 0;
+}
+
 static int wm_adsp_load(struct wm_adsp *dsp)
 {
 	LIST_HEAD(buf_list);
@@ -706,12 +809,18 @@ static int wm_adsp_load(struct wm_adsp *dsp)
 		goto out_fw;
 	}
 
-	if (header->ver != 0) {
+	switch (header->ver) {
+	case 0:
+	case 1:
+		break;
+	default:
 		adsp_err(dsp, "%s: unknown file format %d\n",
 			 file, header->ver);
 		goto out_fw;
 	}
+
 	adsp_info(dsp, "Firmware version: %d\n", header->ver);
+	dsp->fw_ver = header->ver;
 
 	if (header->core != dsp->type) {
 		adsp_err(dsp, "%s: invalid core %d != %d\n",
@@ -776,6 +885,12 @@ static int wm_adsp_load(struct wm_adsp *dsp)
 			text = kzalloc(le32_to_cpu(region->len) + 1,
 				       GFP_KERNEL);
 			break;
+		case WMFW_ALGORITHM_DATA:
+			region_name = "Algorithm";
+			ret = wm_adsp_parse_coeff(dsp, region);
+			if (ret != 0)
+				goto out_fw;
+			break;
 		case WMFW_INFO_TEXT:
 			region_name = "Information";
 			text = kzalloc(le32_to_cpu(region->len) + 1,
@@ -868,6 +983,20 @@ out:
 	return ret;
 }
 
+static void wm_adsp_ctl_fixup_base(struct wm_adsp *dsp,
+				  const struct wm_adsp_alg_region *alg_region)
+{
+	struct wm_coeff_ctl *ctl;
+
+	list_for_each_entry(ctl, &dsp->ctl_list, list) {
+		if (ctl->fw_name == wm_adsp_fw_text[dsp->fw] &&
+		    alg_region->alg == ctl->alg_region.alg &&
+		    alg_region->type == ctl->alg_region.type) {
+			ctl->alg_region.base = alg_region->base;
+		}
+	}
+}
+
 static void *wm_adsp_read_algs(struct wm_adsp *dsp, size_t n_algs,
 			       unsigned int pos, unsigned int len)
 {
@@ -928,6 +1057,9 @@ static struct wm_adsp_alg_region *wm_adsp_create_region(struct wm_adsp *dsp,
 
 	list_add_tail(&alg_region->list, &dsp->alg_regions);
 
+	if (dsp->fw_ver > 0)
+		wm_adsp_ctl_fixup_base(dsp, alg_region);
+
 	return alg_region;
 }
 
@@ -995,14 +1127,17 @@ static int wm_adsp1_setup_algs(struct wm_adsp *dsp)
 			ret = PTR_ERR(alg_region);
 			goto out;
 		}
-		if (i + 1 < n_algs) {
-			len = be32_to_cpu(adsp1_alg[i + 1].dm);
-			len -= be32_to_cpu(adsp1_alg[i].dm);
-			len *= 4;
-			wm_adsp_create_control(dsp, alg_region, len);
-		} else {
-			adsp_warn(dsp, "Missing length info for region DM with ID %x\n",
-				  be32_to_cpu(adsp1_alg[i].alg.id));
+		if (dsp->fw_ver == 0) {
+			if (i + 1 < n_algs) {
+				len = be32_to_cpu(adsp1_alg[i + 1].dm);
+				len -= be32_to_cpu(adsp1_alg[i].dm);
+				len *= 4;
+				wm_adsp_create_control(dsp, alg_region, 0,
+						       len, NULL, 0);
+			} else {
+				adsp_warn(dsp, "Missing length info for region DM with ID %x\n",
+					  be32_to_cpu(adsp1_alg[i].alg.id));
+			}
 		}
 
 		alg_region = wm_adsp_create_region(dsp, WMFW_ADSP1_ZM,
@@ -1012,14 +1147,17 @@ static int wm_adsp1_setup_algs(struct wm_adsp *dsp)
 			ret = PTR_ERR(alg_region);
 			goto out;
 		}
-		if (i + 1 < n_algs) {
-			len = be32_to_cpu(adsp1_alg[i + 1].zm);
-			len -= be32_to_cpu(adsp1_alg[i].zm);
-			len *= 4;
-			wm_adsp_create_control(dsp, alg_region, len);
-		} else {
-			adsp_warn(dsp, "Missing length info for region ZM with ID %x\n",
-				  be32_to_cpu(adsp1_alg[i].alg.id));
+		if (dsp->fw_ver == 0) {
+			if (i + 1 < n_algs) {
+				len = be32_to_cpu(adsp1_alg[i + 1].zm);
+				len -= be32_to_cpu(adsp1_alg[i].zm);
+				len *= 4;
+				wm_adsp_create_control(dsp, alg_region, 0,
+						       len, NULL, 0);
+			} else {
+				adsp_warn(dsp, "Missing length info for region ZM with ID %x\n",
+					  be32_to_cpu(adsp1_alg[i].alg.id));
+			}
 		}
 	}
 
@@ -1099,14 +1237,17 @@ static int wm_adsp2_setup_algs(struct wm_adsp *dsp)
 			ret = PTR_ERR(alg_region);
 			goto out;
 		}
-		if (i + 1 < n_algs) {
-			len = be32_to_cpu(adsp2_alg[i + 1].xm);
-			len -= be32_to_cpu(adsp2_alg[i].xm);
-			len *= 4;
-			wm_adsp_create_control(dsp, alg_region, len);
-		} else {
-			adsp_warn(dsp, "Missing length info for region XM with ID %x\n",
-				  be32_to_cpu(adsp2_alg[i].alg.id));
+		if (dsp->fw_ver == 0) {
+			if (i + 1 < n_algs) {
+				len = be32_to_cpu(adsp2_alg[i + 1].xm);
+				len -= be32_to_cpu(adsp2_alg[i].xm);
+				len *= 4;
+				wm_adsp_create_control(dsp, alg_region, 0,
+						       len, NULL, 0);
+			} else {
+				adsp_warn(dsp, "Missing length info for region XM with ID %x\n",
+					  be32_to_cpu(adsp2_alg[i].alg.id));
+			}
 		}
 
 		alg_region = wm_adsp_create_region(dsp, WMFW_ADSP2_YM,
@@ -1116,14 +1257,17 @@ static int wm_adsp2_setup_algs(struct wm_adsp *dsp)
 			ret = PTR_ERR(alg_region);
 			goto out;
 		}
-		if (i + 1 < n_algs) {
-			len = be32_to_cpu(adsp2_alg[i + 1].ym);
-			len -= be32_to_cpu(adsp2_alg[i].ym);
-			len *= 4;
-			wm_adsp_create_control(dsp, alg_region, len);
-		} else {
-			adsp_warn(dsp, "Missing length info for region YM with ID %x\n",
-				  be32_to_cpu(adsp2_alg[i].alg.id));
+		if (dsp->fw_ver == 0) {
+			if (i + 1 < n_algs) {
+				len = be32_to_cpu(adsp2_alg[i + 1].ym);
+				len -= be32_to_cpu(adsp2_alg[i].ym);
+				len *= 4;
+				wm_adsp_create_control(dsp, alg_region, 0,
+						       len, NULL, 0);
+			} else {
+				adsp_warn(dsp, "Missing length info for region YM with ID %x\n",
+					  be32_to_cpu(adsp2_alg[i].alg.id));
+			}
 		}
 
 		alg_region = wm_adsp_create_region(dsp, WMFW_ADSP2_ZM,
@@ -1133,14 +1277,17 @@ static int wm_adsp2_setup_algs(struct wm_adsp *dsp)
 			ret = PTR_ERR(alg_region);
 			goto out;
 		}
-		if (i + 1 < n_algs) {
-			len = be32_to_cpu(adsp2_alg[i + 1].zm);
-			len -= be32_to_cpu(adsp2_alg[i].zm);
-			len *= 4;
-			wm_adsp_create_control(dsp, alg_region, len);
-		} else {
-			adsp_warn(dsp, "Missing length info for region ZM with ID %x\n",
-				  be32_to_cpu(adsp2_alg[i].alg.id));
+		if (dsp->fw_ver == 0) {
+			if (i + 1 < n_algs) {
+				len = be32_to_cpu(adsp2_alg[i + 1].zm);
+				len -= be32_to_cpu(adsp2_alg[i].zm);
+				len *= 4;
+				wm_adsp_create_control(dsp, alg_region, 0,
+						       len, NULL, 0);
+			} else {
+				adsp_warn(dsp, "Missing length info for region ZM with ID %x\n",
+					  be32_to_cpu(adsp2_alg[i].alg.id));
+			}
 		}
 	}
 
diff --git a/sound/soc/codecs/wm_adsp.h b/sound/soc/codecs/wm_adsp.h
index 0ad14e0..4fe0667 100644
--- a/sound/soc/codecs/wm_adsp.h
+++ b/sound/soc/codecs/wm_adsp.h
@@ -53,6 +53,7 @@ struct wm_adsp {
 	int num_mems;
 
 	int fw;
+	int fw_ver;
 	bool running;
 
 	struct regulator *dvfs;
diff --git a/sound/soc/codecs/wmfw.h b/sound/soc/codecs/wmfw.h
index 34c14b5..04690b2 100644
--- a/sound/soc/codecs/wmfw.h
+++ b/sound/soc/codecs/wmfw.h
@@ -15,6 +15,12 @@
 
 #include <linux/types.h>
 
+#define WMFW_MAX_ALG_NAME         256
+#define WMFW_MAX_ALG_DESCR_NAME   256
+
+#define WMFW_MAX_COEFF_NAME       256
+#define WMFW_MAX_COEFF_DESCR_NAME 256
+
 struct wmfw_header {
 	char magic[4];
 	__le32 len;
@@ -90,6 +96,28 @@ struct wmfw_adsp2_alg_hdr {
 	__be32 ym;
 } __packed;
 
+struct wmfw_adsp_alg_data {
+	__le32 id;
+	u8 name[WMFW_MAX_ALG_NAME];
+	u8 descr[WMFW_MAX_ALG_DESCR_NAME];
+	__le32 ncoeff;
+	u8 data[];
+} __packed;
+
+struct wmfw_adsp_coeff_data {
+	struct {
+		__le16 offset;
+		__le16 type;
+		__le32 size;
+	} hdr;
+	u8 name[WMFW_MAX_COEFF_NAME];
+	u8 descr[WMFW_MAX_COEFF_DESCR_NAME];
+	__le16 ctl_type;
+	__le16 flags;
+	__le32 len;
+	u8 data[];
+} __packed;
+
 struct wmfw_coeff_hdr {
 	u8 magic[4];
 	__le32 len;
@@ -117,9 +145,10 @@ struct wmfw_coeff_item {
 #define WMFW_ADSP1 1
 #define WMFW_ADSP2 2
 
-#define WMFW_ABSOLUTE  0xf0
-#define WMFW_NAME_TEXT 0xfe
-#define WMFW_INFO_TEXT 0xff
+#define WMFW_ABSOLUTE         0xf0
+#define WMFW_ALGORITHM_DATA   0xf2
+#define WMFW_NAME_TEXT        0xfe
+#define WMFW_INFO_TEXT        0xff
 
 #define WMFW_ADSP1_PM 2
 #define WMFW_ADSP1_DM 3
-- 
1.7.2.5

Re: [PATCH 10/13] ASoC: wm_adsp: Add basic support for rev 1 firmware file format

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 309 bytes --]

On Mon, Apr 13, 2015 at 01:28:02PM +0100, Charles Keepax wrote:
> Revision one of the file format includes new algorithm and coefficient
> blocks which provide additional information about the controls exported
> by the firmware. This patch updates the processing to handle this.

Applied up to here, thanks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH 11/13] ASoC: wm_adsp: Add support for DSP control flags

[Charles Keepax]

The DSP control information contains various hints about the usage of
the controls use these when handling the control.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm_adsp.c |   40 ++++++++++++++++++++++++++++++++--------
 sound/soc/codecs/wmfw.h    |    5 +++++
 2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index 16b308a..edf3ba3 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -240,6 +240,7 @@ struct wm_coeff_ctl {
 	size_t len;
 	unsigned int set:1;
 	struct snd_kcontrol *kcontrol;
+	unsigned int flags;
 };
 
 static int wm_adsp_fw_get(struct snd_kcontrol *kcontrol,
@@ -418,6 +419,9 @@ static int wm_coeff_put(struct snd_kcontrol *kcontrol,
 	struct wm_coeff_ctl *ctl = (struct wm_coeff_ctl *)kcontrol->private_value;
 	char *p = ucontrol->value.bytes.data;
 
+	if (ctl->flags && !(ctl->flags & WMFW_CTL_FLAG_WRITEABLE))
+		return -EPERM;
+
 	memcpy(ctl->cache, p, ctl->len);
 
 	ctl->set = 1;
@@ -472,7 +476,18 @@ static int wm_coeff_get(struct snd_kcontrol *kcontrol,
 	struct wm_coeff_ctl *ctl = (struct wm_coeff_ctl *)kcontrol->private_value;
 	char *p = ucontrol->value.bytes.data;
 
+	if (ctl->flags && !(ctl->flags & WMFW_CTL_FLAG_READABLE))
+		return -EPERM;
+
+	if (ctl->flags & WMFW_CTL_FLAG_VOLATILE) {
+		if (ctl->enabled)
+			return wm_coeff_read_control(ctl, p, ctl->len);
+		else
+			return -EPERM;
+	}
+
 	memcpy(p, ctl->cache, ctl->len);
+
 	return 0;
 }
 
@@ -526,6 +541,9 @@ static int wm_coeff_init_control_caches(struct wm_adsp *dsp)
 	list_for_each_entry(ctl, &dsp->ctl_list, list) {
 		if (!ctl->enabled || ctl->set)
 			continue;
+		if (ctl->flags & WMFW_CTL_FLAG_VOLATILE)
+			continue;
+
 		ret = wm_coeff_read_control(ctl,
 					    ctl->cache,
 					    ctl->len);
@@ -544,7 +562,7 @@ static int wm_coeff_sync_controls(struct wm_adsp *dsp)
 	list_for_each_entry(ctl, &dsp->ctl_list, list) {
 		if (!ctl->enabled)
 			continue;
-		if (ctl->set) {
+		if (ctl->set && !(ctl->flags & WMFW_CTL_FLAG_VOLATILE)) {
 			ret = wm_coeff_write_control(ctl,
 						     ctl->cache,
 						     ctl->len);
@@ -569,7 +587,8 @@ static void wm_adsp_ctl_work(struct work_struct *work)
 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)
+				  const char *subname, unsigned int subname_len,
+				  unsigned int flags)
 {
 	struct wm_coeff_ctl *ctl;
 	struct wmfw_ctl_work *ctl_work;
@@ -577,6 +596,9 @@ static int wm_adsp_create_control(struct wm_adsp *dsp,
 	char *region_name;
 	int ret;
 
+	if (flags & WMFW_CTL_FLAG_SYS)
+		return 0;
+
 	switch (alg_region->type) {
 	case WMFW_ADSP1_PM:
 		region_name = "PM";
@@ -626,6 +648,7 @@ static int wm_adsp_create_control(struct wm_adsp *dsp,
 	ctl->ops.xput = wm_coeff_put;
 	ctl->dsp = dsp;
 
+	ctl->flags = flags;
 	ctl->offset = offset;
 	if (len > 512) {
 		adsp_warn(dsp, "Truncating control %s from %d\n",
@@ -752,7 +775,8 @@ static int wm_adsp_parse_coeff(struct wm_adsp *dsp,
 					     coeff_blk.offset,
 					     coeff_blk.len,
 					     coeff_blk.name,
-					     coeff_blk.name_len);
+					     coeff_blk.name_len,
+					     coeff_blk.flags);
 		if (ret < 0)
 			adsp_err(dsp, "Failed to create control: %.*s, %d\n",
 				 coeff_blk.name_len, coeff_blk.name, ret);
@@ -1133,7 +1157,7 @@ static int wm_adsp1_setup_algs(struct wm_adsp *dsp)
 				len -= be32_to_cpu(adsp1_alg[i].dm);
 				len *= 4;
 				wm_adsp_create_control(dsp, alg_region, 0,
-						       len, NULL, 0);
+						       len, NULL, 0, 0);
 			} else {
 				adsp_warn(dsp, "Missing length info for region DM with ID %x\n",
 					  be32_to_cpu(adsp1_alg[i].alg.id));
@@ -1153,7 +1177,7 @@ static int wm_adsp1_setup_algs(struct wm_adsp *dsp)
 				len -= be32_to_cpu(adsp1_alg[i].zm);
 				len *= 4;
 				wm_adsp_create_control(dsp, alg_region, 0,
-						       len, NULL, 0);
+						       len, NULL, 0, 0);
 			} else {
 				adsp_warn(dsp, "Missing length info for region ZM with ID %x\n",
 					  be32_to_cpu(adsp1_alg[i].alg.id));
@@ -1243,7 +1267,7 @@ static int wm_adsp2_setup_algs(struct wm_adsp *dsp)
 				len -= be32_to_cpu(adsp2_alg[i].xm);
 				len *= 4;
 				wm_adsp_create_control(dsp, alg_region, 0,
-						       len, NULL, 0);
+						       len, NULL, 0, 0);
 			} else {
 				adsp_warn(dsp, "Missing length info for region XM with ID %x\n",
 					  be32_to_cpu(adsp2_alg[i].alg.id));
@@ -1263,7 +1287,7 @@ static int wm_adsp2_setup_algs(struct wm_adsp *dsp)
 				len -= be32_to_cpu(adsp2_alg[i].ym);
 				len *= 4;
 				wm_adsp_create_control(dsp, alg_region, 0,
-						       len, NULL, 0);
+						       len, NULL, 0, 0);
 			} else {
 				adsp_warn(dsp, "Missing length info for region YM with ID %x\n",
 					  be32_to_cpu(adsp2_alg[i].alg.id));
@@ -1283,7 +1307,7 @@ static int wm_adsp2_setup_algs(struct wm_adsp *dsp)
 				len -= be32_to_cpu(adsp2_alg[i].zm);
 				len *= 4;
 				wm_adsp_create_control(dsp, alg_region, 0,
-						       len, NULL, 0);
+						       len, NULL, 0, 0);
 			} else {
 				adsp_warn(dsp, "Missing length info for region ZM with ID %x\n",
 					  be32_to_cpu(adsp2_alg[i].alg.id));
diff --git a/sound/soc/codecs/wmfw.h b/sound/soc/codecs/wmfw.h
index 04690b2..7613d60 100644
--- a/sound/soc/codecs/wmfw.h
+++ b/sound/soc/codecs/wmfw.h
@@ -21,6 +21,11 @@
 #define WMFW_MAX_COEFF_NAME       256
 #define WMFW_MAX_COEFF_DESCR_NAME 256
 
+#define WMFW_CTL_FLAG_SYS         0x8000
+#define WMFW_CTL_FLAG_VOLATILE    0x0004
+#define WMFW_CTL_FLAG_WRITEABLE   0x0002
+#define WMFW_CTL_FLAG_READABLE    0x0001
+
 struct wmfw_header {
 	char magic[4];
 	__le32 len;
-- 
1.7.2.5

Re: [PATCH 11/13] ASoC: wm_adsp: Add support for DSP control flags

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 908 bytes --]

On Mon, Apr 13, 2015 at 01:28:03PM +0100, Charles Keepax wrote:
> The DSP control information contains various hints about the usage of
> the controls use these when handling the control.

> @@ -418,6 +419,9 @@ static int wm_coeff_put(struct snd_kcontrol *kcontrol,
>  	struct wm_coeff_ctl *ctl = (struct wm_coeff_ctl *)kcontrol->private_value;
>  	char *p = ucontrol->value.bytes.data;
>  
> +	if (ctl->flags && !(ctl->flags & WMFW_CTL_FLAG_WRITEABLE))
> +		return -EPERM;
> +
>  	memcpy(ctl->cache, p, ctl->len);
>  
>  	ctl->set = 1;

What would be even better for these would be to apply these at control
creation time so that controls that lack read or write support just
don't have the relevant operations in the first place.  That would not
be simpler and would allow the rest of the stack to do the right thing,
for example setting SNDRV_CTL_ELEM_ACCESS_READ and _WRITE properly.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 11/13] ASoC: wm_adsp: Add support for DSP control flags

[Charles Keepax]

On Sat, Apr 18, 2015 at 06:23:06PM +0100, Mark Brown wrote:
> On Mon, Apr 13, 2015 at 01:28:03PM +0100, Charles Keepax wrote:
> > The DSP control information contains various hints about the usage of
> > the controls use these when handling the control.
> 
> > @@ -418,6 +419,9 @@ static int wm_coeff_put(struct snd_kcontrol *kcontrol,
> >  	struct wm_coeff_ctl *ctl = (struct wm_coeff_ctl *)kcontrol->private_value;
> >  	char *p = ucontrol->value.bytes.data;
> >  
> > +	if (ctl->flags && !(ctl->flags & WMFW_CTL_FLAG_WRITEABLE))
> > +		return -EPERM;
> > +
> >  	memcpy(ctl->cache, p, ctl->len);
> >  
> >  	ctl->set = 1;
> 
> What would be even better for these would be to apply these at control
> creation time so that controls that lack read or write support just
> don't have the relevant operations in the first place.  That would not
> be simpler and would allow the rest of the stack to do the right thing,
> for example setting SNDRV_CTL_ELEM_ACCESS_READ and _WRITE properly.

That is indeed much more sensible I will respin this patch.

Thanks,
Charles

[PATCH 12/13] ASoC: wm_adsp: Add support for rev 2 firmware file format

[Charles Keepax]

Version 2 of the firmware file format includes length fields for the
various strings associated with control creation, to reduce file size.
However this does increase the parsing complexity slightly. This patch
adds support for the revision of the file format.

This patch also adds a new naming scheme for controls created from rev 2
firmware files. This version of the file format is commonly used to
add multiple controls per algorithm per memory region and the old
control naming scheme would cause multiple controls to have the same
name in this case.. Note that the naming scheme for older firmware
versions is left intact to ensure backwards compatibility.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm_adsp.c |  137 ++++++++++++++++++++++++++++++++++++++------
 1 files changed, 119 insertions(+), 18 deletions(-)

diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index edf3ba3..a5dd22e 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -620,8 +620,31 @@ static int wm_adsp_create_control(struct wm_adsp *dsp,
 		return -EINVAL;
 	}
 
-	snprintf(name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN, "DSP%d %s %x",
-		 dsp->num, region_name, alg_region->alg);
+	switch (dsp->fw_ver) {
+	case 0:
+	case 1:
+		snprintf(name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN, "DSP%d %s %x",
+			 dsp->num, region_name, alg_region->alg);
+		break;
+	default:
+		ret = snprintf(name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN,
+				"DSP%d%c %.12s %x", dsp->num, *region_name,
+				wm_adsp_fw_text[dsp->fw], alg_region->alg);
+
+		/* Truncate the subname from the start if it is too long */
+		if (subname) {
+			int avail = SNDRV_CTL_ELEM_ID_NAME_MAXLEN - ret - 2;
+			int skip = 0;
+
+			if (subname_len > avail)
+				skip = subname_len - avail;
+
+			snprintf(name + ret,
+				 SNDRV_CTL_ELEM_ID_NAME_MAXLEN - ret, " %.*s",
+				 subname_len - skip, subname + skip);
+		}
+		break;
+	}
 
 	list_for_each_entry(ctl, &dsp->ctl_list,
 			    list) {
@@ -704,18 +727,73 @@ struct wm_coeff_parsed_coeff {
 	int len;
 };
 
+static int wm_coeff_parse_string(int bytes, const u8 **pos, const u8 **str)
+{
+	int length;
+
+	switch (bytes) {
+	case 1:
+		length = **pos;
+		break;
+	case 2:
+		length = le16_to_cpu(*((u16 *)*pos));
+		break;
+	default:
+		return 0;
+	}
+
+	if (str)
+		*str = *pos + bytes;
+
+	*pos += ((length + bytes) + 3) & ~0x03;
+
+	return length;
+}
+
+static int wm_coeff_parse_int(int bytes, const u8 **pos)
+{
+	int val = 0;
+
+	switch (bytes) {
+	case 2:
+		val = le16_to_cpu(*((u16 *)*pos));
+		break;
+	case 4:
+		val = le32_to_cpu(*((u32 *)*pos));
+		break;
+	default:
+		break;
+	}
+
+	*pos += bytes;
+
+	return val;
+}
+
 static inline void wm_coeff_parse_alg(struct wm_adsp *dsp, const u8 **data,
 				      struct wm_coeff_parsed_alg *blk)
 {
 	const struct wmfw_adsp_alg_data *raw;
 
-	raw = (const struct wmfw_adsp_alg_data *)*data;
-	*data = raw->data;
+	switch (dsp->fw_ver) {
+	case 0:
+	case 1:
+		raw = (const struct wmfw_adsp_alg_data *)*data;
+		*data = raw->data;
 
-	blk->id = le32_to_cpu(raw->id);
-	blk->name = raw->name;
-	blk->name_len = strlen(raw->name);
-	blk->ncoeff = le32_to_cpu(raw->ncoeff);
+		blk->id = le32_to_cpu(raw->id);
+		blk->name = raw->name;
+		blk->name_len = strlen(raw->name);
+		blk->ncoeff = le32_to_cpu(raw->ncoeff);
+		break;
+	default:
+		blk->id = wm_coeff_parse_int(sizeof(raw->id), data);
+		blk->name_len = wm_coeff_parse_string(sizeof(u8), data,
+						      &blk->name);
+		wm_coeff_parse_string(sizeof(u16), data, NULL);
+		blk->ncoeff = wm_coeff_parse_int(sizeof(raw->ncoeff), data);
+		break;
+	}
 
 	adsp_dbg(dsp, "Algorithm ID: %#x\n", blk->id);
 	adsp_dbg(dsp, "Algorithm name: %.*s\n", blk->name_len, blk->name);
@@ -726,17 +804,39 @@ static inline void wm_coeff_parse_coeff(struct wm_adsp *dsp, const u8 **data,
 					struct wm_coeff_parsed_coeff *blk)
 {
 	const struct wmfw_adsp_coeff_data *raw;
+	const u8 *tmp;
+	int length;
 
-	raw = (const struct wmfw_adsp_coeff_data *)*data;
-	*data = *data + sizeof(raw->hdr) + le32_to_cpu(raw->hdr.size);
-
-	blk->offset = le16_to_cpu(raw->hdr.offset);
-	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->flags = le16_to_cpu(raw->flags);
-	blk->len = le32_to_cpu(raw->len);
+	switch (dsp->fw_ver) {
+	case 0:
+	case 1:
+		raw = (const struct wmfw_adsp_coeff_data *)*data;
+		*data = *data + sizeof(raw->hdr) + le32_to_cpu(raw->hdr.size);
+
+		blk->offset = le16_to_cpu(raw->hdr.offset);
+		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->flags = le16_to_cpu(raw->flags);
+		blk->len = le32_to_cpu(raw->len);
+		break;
+	default:
+		tmp = *data;
+		blk->offset = wm_coeff_parse_int(sizeof(raw->hdr.offset), &tmp);
+		blk->mem_type = wm_coeff_parse_int(sizeof(raw->hdr.type), &tmp);
+		length = wm_coeff_parse_int(sizeof(raw->hdr.size), &tmp);
+		blk->name_len = wm_coeff_parse_string(sizeof(u8), &tmp,
+						      &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->flags = wm_coeff_parse_int(sizeof(raw->flags), &tmp);
+		blk->len = wm_coeff_parse_int(sizeof(raw->len), &tmp);
+
+		*data = *data + sizeof(raw->hdr) + length;
+		break;
+	}
 
 	adsp_dbg(dsp, "\tCoefficient type: %#x\n", blk->mem_type);
 	adsp_dbg(dsp, "\tCoefficient offset: %#x\n", blk->offset);
@@ -836,6 +936,7 @@ static int wm_adsp_load(struct wm_adsp *dsp)
 	switch (header->ver) {
 	case 0:
 	case 1:
+	case 2:
 		break;
 	default:
 		adsp_err(dsp, "%s: unknown file format %d\n",
-- 
1.7.2.5

Re: [PATCH 12/13] ASoC: wm_adsp: Add support for rev 2 firmware file format

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 351 bytes --]

On Mon, Apr 13, 2015 at 01:28:04PM +0100, Charles Keepax wrote:
> Version 2 of the firmware file format includes length fields for the
> various strings associated with control creation, to reduce file size.
> However this does increase the parsing complexity slightly. This patch
> adds support for the revision of the file format.

Applied, thanks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH 13/13] ASoC: wm_adsp: Warn that firmware file format 0 is depreciated

[Charles Keepax]

There are very few version 0 firmwares in the wild and at some point in
the future it would be nice to remove support for them from the driver,
as they require several work arounds to be present to create controls
properly.

This patch adds a depreciated warning if someone is using this file
format.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm_adsp.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index a5dd22e..173f332 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -935,6 +935,9 @@ static int wm_adsp_load(struct wm_adsp *dsp)
 
 	switch (header->ver) {
 	case 0:
+		adsp_warn(dsp, "%s: Depreciated file format %d\n",
+			  file, header->ver);
+		break;
 	case 1:
 	case 2:
 		break;
-- 
1.7.2.5

Re: [PATCH 13/13] ASoC: wm_adsp: Warn that firmware file format 0 is depreciated

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 314 bytes --]

On Mon, Apr 13, 2015 at 01:28:05PM +0100, Charles Keepax wrote:
> There are very few version 0 firmwares in the wild and at some point in
> the future it would be nice to remove support for them from the driver,
> as they require several work arounds to be present to create controls
> properly.

Applied, thanks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]