[alsa-devel] [PATCH] ASoC: core: allow control index different from 0

[Takashi Sakamoto]

Sorry to be late. I've review your driver in for-next branch of
maintainer's tree.

On Oct 8 2016 01:41, Arnaud Pouliquen wrote:
> Let clarify the my devs:
> 
> 1) My original implementation is here :
> http://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/tree/sound/soc/sti/sti_uniperif.c?h=sound-4.9-rc1#n232
> this does not work due to index that is overwritten
>  Notice that my issue is not limited to iec958 control, but controls in
> generals
> 
> 2) In parallel, I proposed generic code to implement:
>  IEC control + possibility to attach DAI ctrl to PCM
> device:http://mailman.alsa-project.org/pipermail/alsa-devel/2016-March/105502.html
> 
> Right now, I propose to focus on point 1) that integrates constraints
> for DAIs and forget the patch-set that includes IEC generic control (if
> you interested in this code, i can rework it in a second step)

The center of discussion now becomes transformed.

Anyway, at least, this patch (ASoC: core: allow control index different
from 0) is exaggerated to your aim. The change influences to all of
control element sets added by implementations in ALSA SoC part. (We have
no guarantee that all drivers already set 0 to the field of template
before calling with the template.)

And please write enough information about your aim of this patch, even
if it's a part of your work for something large. It helps reviewers.
Additionally, if you discuss the former patchset, please rebase them to
current upstream, then post them again with enough comments, then start
discussion.

On Oct 8 2016 01:41, Arnaud Pouliquen wrote:
> Perhaps this could be handled in a generic way in control.c?
> Today if control is identical, snd_ctl_add returns an error.
> Instead an auto-incrementation mechanism could be implemented to
> increase index.

Just for ALSA SoC part, something like below (not tested). This never
work well because 0 is still assigned to the index field later.

But I still oppose this idea. This idea allows drivers to add control
element sets of different types (int/int64/bytes etc...) with the same
name and different index number. This certainly brings confusions to
applications.

In a framework of ALSA SoC part, several drivers are associated to one
sound card instance can add their own control element sets. There's no
mechanism to prevent my concern. This idea is bad.

>From 30c6abb20ca548add8cddb6e056831efde365c3e Mon Sep 17 00:00:00 2001
From: Takashi Sakamoto <o-takashi at sakamocchi.jp>
Date: Sun, 9 Oct 2016 11:20:08 +0900
Subject: [PATCH] hoge

---
 sound/soc/soc-core.c | 42 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 38 insertions(+), 4 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index c0bbcd9..573ca73 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2202,15 +2202,49 @@ static int snd_soc_add_controls(struct snd_card
*card, struct device *dev,
 	const struct snd_kcontrol_new *controls, int num_controls,
 	const char *prefix, void *data)
 {
+	struct snd_kcontrol_new template;
+	struct snd_ctl_elem_id id;
+	struct snd_kcontrol *elem_set;
 	int err, i;

 	for (i = 0; i < num_controls; i++) {
-		const struct snd_kcontrol_new *control = &controls[i];
-		err = snd_ctl_add(card, snd_soc_cnew(control, data,
-						     control->name, prefix));
+		template = controls[i];
+
+		id.iface = template.iface;
+		id.device = template.device;
+		id.subdevice = template.subdevice;
+		memcpy(id.name, template.name, sizeof(id.name));
+
+		/*
+		 * Seek duplicated element sets already registered in this
+		 * sound card to use continuous number for index field.
+		 */
+		while (1) {
+			id.index = template.index;
+
+			elem_set = snd_ctl_find_id(card, &id);
+			if (elem_set == NULL)
+				break;
+
+			if (elem_set->id.index > UINT_MAX - elem_set->count) {
+				dev_err(dev, "ASoC: Failed to keep %s: %d\n",
+					template.name, err);
+				return -ENOSPC;
+			}
+			template.index = elem_set->count + elem_set->id.index;
+		}
+
+		elem_set = snd_soc_cnew(&template, data, template.name, prefix);
+		if (elem_set == NULL) {
+			dev_err(dev, "ASoC: Failed to allocate %s: %d\n",
+				template.name, err);
+			return -ENOMEM;
+		}
+
+		err = snd_ctl_add(card, elem_set);
 		if (err < 0) {
 			dev_err(dev, "ASoC: Failed to add %s: %d\n",
-				control->name, err);
+				template.name, err);
 			return err;
 		}
 	}
-- 
2.7.4


Regards

Takashi Sakamoto