[PATCH] ASoC: DAPM: Add support for multi register mux

[PATCH] ASoC: DAPM: Add support for multi register mux

[Arun Shamanna Lakshmi]

Modify soc_enum struct to handle pointers for reg and mask. Add
dapm get and put APIs for multi register mux with one hot encoding.

Signed-off-by: Arun Shamanna Lakshmi <aruns@nvidia.com>
Signed-off-by: Songhee Baek <sbaek@nvidia.com>
---
 include/sound/soc-dapm.h |   10 ++++
 include/sound/soc.h      |   22 +++++--
 sound/soc/soc-core.c     |   12 ++--
 sound/soc/soc-dapm.c     |  143 +++++++++++++++++++++++++++++++++++++++++-----
 4 files changed, 162 insertions(+), 25 deletions(-)

diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
index ef78f56..983b0ab 100644
--- a/include/sound/soc-dapm.h
+++ b/include/sound/soc-dapm.h
@@ -305,6 +305,12 @@ struct device;
  	.get = snd_soc_dapm_get_enum_double, \
  	.put = snd_soc_dapm_put_enum_double, \
   	.private_value = (unsigned long)&xenum }
+#define SOC_DAPM_ENUM_WIDE(xname, xenum) \
+{	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
+	.info = snd_soc_info_enum_double, \
+	.get = snd_soc_dapm_get_enum_wide, \
+	.put = snd_soc_dapm_put_enum_wide, \
+	.private_value = (unsigned long)&xenum }
 #define SOC_DAPM_ENUM_VIRT(xname, xenum) \
 	SOC_DAPM_ENUM(xname, xenum)
 #define SOC_DAPM_ENUM_EXT(xname, xenum, xget, xput) \
@@ -378,6 +384,10 @@ int snd_soc_dapm_get_enum_double(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_value *ucontrol);
 int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_value *ucontrol);
+int snd_soc_dapm_get_enum_wide(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol);
+int snd_soc_dapm_put_enum_wide(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol);
 int snd_soc_dapm_info_pin_switch(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_info *uinfo);
 int snd_soc_dapm_get_pin_switch(struct snd_kcontrol *kcontrol,
diff --git a/include/sound/soc.h b/include/sound/soc.h
index 5878410..5c274c4 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -177,18 +177,23 @@
 		{.reg = xreg, .min = xmin, .max = xmax, \
 		 .platform_max = xmax} }
 #define SOC_ENUM_DOUBLE(xreg, xshift_l, xshift_r, xitems, xtexts) \
-{	.reg = xreg, .shift_l = xshift_l, .shift_r = xshift_r, \
+{	.reg = &(int){(xreg)}, .shift_l = xshift_l, .shift_r = xshift_r, \
 	.items = xitems, .texts = xtexts, \
-	.mask = xitems ? roundup_pow_of_two(xitems) - 1 : 0}
+	.mask = &(unsigned int){(xitems ? roundup_pow_of_two(xitems) - 1 : 0)}, \
+	.num_regs = 1 }
 #define SOC_ENUM_SINGLE(xreg, xshift, xitems, xtexts) \
 	SOC_ENUM_DOUBLE(xreg, xshift, xshift, xitems, xtexts)
 #define SOC_ENUM_SINGLE_EXT(xitems, xtexts) \
 {	.items = xitems, .texts = xtexts }
 #define SOC_VALUE_ENUM_DOUBLE(xreg, xshift_l, xshift_r, xmask, xitems, xtexts, xvalues) \
-{	.reg = xreg, .shift_l = xshift_l, .shift_r = xshift_r, \
-	.mask = xmask, .items = xitems, .texts = xtexts, .values = xvalues}
+{	.reg = &(int){(xreg)}, .shift_l = xshift_l, .shift_r = xshift_r, \
+	.mask = &(unsigned int){(xmask)}, .items = xitems, .texts = xtexts, \
+	.values = xvalues, .num_regs = 1 }
 #define SOC_VALUE_ENUM_SINGLE(xreg, xshift, xmask, xnitmes, xtexts, xvalues) \
 	SOC_VALUE_ENUM_DOUBLE(xreg, xshift, xshift, xmask, xnitmes, xtexts, xvalues)
+#define SOC_VALUE_ENUM_WIDE(xregs, xmasks, xnum_regs, xitems, xtexts, xvalues) \
+{	.reg = xregs, .mask = xmasks, .num_regs = xnum_regs, \
+	.items = xitems, .texts = xtexts, .values = xvalues, .reg_width = 32 }
 #define SOC_ENUM_SINGLE_VIRT(xitems, xtexts) \
 	SOC_ENUM_SINGLE(SND_SOC_NOPM, 0, xitems, xtexts)
 #define SOC_ENUM(xname, xenum) \
@@ -293,6 +298,9 @@
 #define SOC_VALUE_ENUM_DOUBLE_DECL(name, xreg, xshift_l, xshift_r, xmask, xtexts, xvalues) \
 	const struct soc_enum name = SOC_VALUE_ENUM_DOUBLE(xreg, xshift_l, xshift_r, xmask, \
 							ARRAY_SIZE(xtexts), xtexts, xvalues)
+#define SOC_VALUE_ENUM_WIDE_DECL(name, xregs, xmasks, xnum_regs, xtexts, xvalues) \
+	const struct soc_enum name = SOC_VALUE_ENUM_WIDE(xregs, xmasks, xnum_regs, \
+							ARRAY_SIZE(xtexts), xtexts, xvalues)
 #define SOC_VALUE_ENUM_SINGLE_DECL(name, xreg, xshift, xmask, xtexts, xvalues) \
 	SOC_VALUE_ENUM_DOUBLE_DECL(name, xreg, xshift, xshift, xmask, xtexts, xvalues)
 #define SOC_ENUM_SINGLE_VIRT_DECL(name, xtexts) \
@@ -1098,13 +1106,15 @@ struct soc_mreg_control {
 
 /* enumerated kcontrol */
 struct soc_enum {
-	int reg;
+	int *reg;
 	unsigned char shift_l;
 	unsigned char shift_r;
 	unsigned int items;
-	unsigned int mask;
+	unsigned int *mask;
 	const char * const *texts;
 	const unsigned int *values;
+	unsigned int reg_width;
+	unsigned int num_regs;
 };
 
 /**
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index cd52d52..aba0094 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2601,12 +2601,12 @@ int snd_soc_get_enum_double(struct snd_kcontrol *kcontrol,
 	unsigned int val, item;
 	unsigned int reg_val;
 
-	reg_val = snd_soc_read(codec, e->reg);
-	val = (reg_val >> e->shift_l) & e->mask;
+	reg_val = snd_soc_read(codec, e->reg[0]);
+	val = (reg_val >> e->shift_l) & e->mask[0];
 	item = snd_soc_enum_val_to_item(e, val);
 	ucontrol->value.enumerated.item[0] = item;
 	if (e->shift_l != e->shift_r) {
-		val = (reg_val >> e->shift_l) & e->mask;
+		val = (reg_val >> e->shift_l) & e->mask[0];
 		item = snd_soc_enum_val_to_item(e, val);
 		ucontrol->value.enumerated.item[1] = item;
 	}
@@ -2636,15 +2636,15 @@ int snd_soc_put_enum_double(struct snd_kcontrol *kcontrol,
 	if (item[0] >= e->items)
 		return -EINVAL;
 	val = snd_soc_enum_item_to_val(e, item[0]) << e->shift_l;
-	mask = e->mask << e->shift_l;
+	mask = e->mask[0] << e->shift_l;
 	if (e->shift_l != e->shift_r) {
 		if (item[1] >= e->items)
 			return -EINVAL;
 		val |= snd_soc_enum_item_to_val(e, item[1]) << e->shift_r;
-		mask |= e->mask << e->shift_r;
+		mask |= e->mask[0] << e->shift_r;
 	}
 
-	return snd_soc_update_bits_locked(codec, e->reg, mask, val);
+	return snd_soc_update_bits_locked(codec, e->reg[0], mask, val);
 }
 EXPORT_SYMBOL_GPL(snd_soc_put_enum_double);
 
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index c8a780d..4d2b35c 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -514,9 +514,9 @@ static int dapm_connect_mux(struct snd_soc_dapm_context *dapm,
 	unsigned int val, item;
 	int i;
 
-	if (e->reg != SND_SOC_NOPM) {
-		soc_widget_read(dest, e->reg, &val);
-		val = (val >> e->shift_l) & e->mask;
+	if (e->reg[0] != SND_SOC_NOPM) {
+		soc_widget_read(dest, e->reg[0], &val);
+		val = (val >> e->shift_l) & e->mask[0];
 		item = snd_soc_enum_val_to_item(e, val);
 	} else {
 		/* since a virtual mux has no backing registers to
@@ -2903,15 +2903,15 @@ int snd_soc_dapm_get_enum_double(struct snd_kcontrol *kcontrol,
 	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
 	unsigned int reg_val, val;
 
-	if (e->reg != SND_SOC_NOPM)
-		reg_val = snd_soc_read(codec, e->reg);
+	if (e->reg[0] != SND_SOC_NOPM)
+		reg_val = snd_soc_read(codec, e->reg[0]);
 	else
 		reg_val = dapm_kcontrol_get_value(kcontrol);
 
-	val = (reg_val >> e->shift_l) & e->mask;
+	val = (reg_val >> e->shift_l) & e->mask[0];
 	ucontrol->value.enumerated.item[0] = snd_soc_enum_val_to_item(e, val);
 	if (e->shift_l != e->shift_r) {
-		val = (reg_val >> e->shift_r) & e->mask;
+		val = (reg_val >> e->shift_r) & e->mask[0];
 		val = snd_soc_enum_val_to_item(e, val);
 		ucontrol->value.enumerated.item[1] = val;
 	}
@@ -2945,25 +2945,25 @@ int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol,
 		return -EINVAL;
 
 	val = snd_soc_enum_item_to_val(e, item[0]) << e->shift_l;
-	mask = e->mask << e->shift_l;
+	mask = e->mask[0] << e->shift_l;
 	if (e->shift_l != e->shift_r) {
 		if (item[1] > e->items)
 			return -EINVAL;
 		val |= snd_soc_enum_item_to_val(e, item[1]) << e->shift_l;
-		mask |= e->mask << e->shift_r;
+		mask |= e->mask[0] << e->shift_r;
 	}
 
 	mutex_lock_nested(&card->dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME);
 
-	if (e->reg != SND_SOC_NOPM)
-		change = snd_soc_test_bits(codec, e->reg, mask, val);
+	if (e->reg[0] != SND_SOC_NOPM)
+		change = snd_soc_test_bits(codec, e->reg[0], mask, val);
 	else
 		change = dapm_kcontrol_set_value(kcontrol, val);
 
 	if (change) {
-		if (e->reg != SND_SOC_NOPM) {
+		if (e->reg[0] != SND_SOC_NOPM) {
 			update.kcontrol = kcontrol;
-			update.reg = e->reg;
+			update.reg = e->reg[0];
 			update.mask = mask;
 			update.val = val;
 			card->update = &update;
@@ -2984,6 +2984,123 @@ int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol,
 EXPORT_SYMBOL_GPL(snd_soc_dapm_put_enum_double);
 
 /**
+ * snd_soc_dapm_get_enum_wide - dapm semi enumerated multiple registers
+ *					mixer get callback
+ * @kcontrol: mixer control
+ * @ucontrol: control element information
+ *
+ * Callback to get the value of a dapm semi enumerated multiple register mixer
+ * control.
+ *
+ * semi enumerated multiple registers mixer:
+ * the mixer has multiple registers to set the enumerated items. The enumerated
+ * items are referred as values.
+ * Can be used for handling bit field coded enumeration for example.
+ *
+ * Returns 0 for success.
+ */
+int snd_soc_dapm_get_enum_wide(struct snd_kcontrol *kcontrol,
+			struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_codec *codec = snd_soc_dapm_kcontrol_codec(kcontrol);
+	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
+	unsigned int reg_val, val, bit_pos = 0, reg_idx;
+
+	for (reg_idx = 0; reg_idx < e->num_regs; reg_idx++) {
+		reg_val = snd_soc_read(codec, e->reg[reg_idx]);
+		val = reg_val & e->mask[reg_idx];
+		if (val != 0) {
+			bit_pos = ffs(val) + (e->reg_width * reg_idx);
+			break;
+		}
+	}
+
+	ucontrol->value.enumerated.item[0] =
+			snd_soc_enum_val_to_item(e, bit_pos);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_soc_dapm_get_enum_wide);
+
+/**
+ * snd_soc_dapm_put_enum_wide - dapm semi enumerated multiple registers
+ *					mixer put callback
+ * @kcontrol: mixer control
+ * @ucontrol: control element information
+ *
+ * Callback to put the value of a dapm semi enumerated multiple register mixer
+ * control.
+ *
+ * semi enumerated multiple registers mixer:
+ * the mixer has multiple registers to set the enumerated items. The enumerated
+ * items are referred as values.
+ * Can be used for handling bit field coded enumeration for example.
+ *
+ * Returns 0 for success.
+ */
+int snd_soc_dapm_put_enum_wide(struct snd_kcontrol *kcontrol,
+			struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_codec *codec = snd_soc_dapm_kcontrol_codec(kcontrol);
+	struct snd_soc_card *card = codec->card;
+	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
+	unsigned int *item = ucontrol->value.enumerated.item;
+	unsigned int change = 0, reg_idx = 0, value, bit_pos;
+	struct snd_soc_dapm_update update;
+	int ret = 0, reg_val = 0, i;
+
+	if (item[0] >= e->items)
+		return -EINVAL;
+
+	value = snd_soc_enum_item_to_val(e, item[0]);
+
+	if (value) {
+		/* get the register index and value to set */
+		reg_idx = (value - 1) / e->reg_width;
+		bit_pos = (value - 1) % e->reg_width;
+		reg_val = BIT(bit_pos);
+	}
+
+	for (i = 0; i < e->num_regs; i++) {
+		if (i == reg_idx) {
+			change = snd_soc_test_bits(codec, e->reg[i],
+							e->mask[i], reg_val);
+
+		} else {
+			/* accumulate the change to update the DAPM path
+			    when none is selected */
+			change += snd_soc_test_bits(codec, e->reg[i],
+							e->mask[i], 0);
+
+			/* clear the register when not selected */
+			snd_soc_write(codec, e->reg[i], 0);
+		}
+	}
+
+	mutex_lock_nested(&card->dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME);
+
+	if (change) {
+		update.kcontrol = kcontrol;
+		update.reg = e->reg[reg_idx];
+		update.mask = e->mask[reg_idx];
+		update.val = reg_val;
+		card->update = &update;
+
+		ret = soc_dapm_mux_update_power(card, kcontrol, item[0], e);
+
+		card->update = NULL;
+	}
+
+	mutex_unlock(&card->dapm_mutex);
+
+	if (ret > 0)
+		soc_dpcm_runtime_update(card);
+
+	return change;
+}
+EXPORT_SYMBOL_GPL(snd_soc_dapm_put_enum_wide);
+
+/**
  * snd_soc_dapm_info_pin_switch - Info for a pin switch
  *
  * @kcontrol: mixer control
-- 
1.7.9.5

Re: [PATCH] ASoC: DAPM: Add support for multi register mux

[Lars-Peter Clausen]

On 04/01/2014 08:21 AM, Arun Shamanna Lakshmi wrote:
> Modify soc_enum struct to handle pointers for reg and mask. Add
> dapm get and put APIs for multi register mux with one hot encoding.
>
> Signed-off-by: Arun Shamanna Lakshmi <aruns@nvidia.com>
> Signed-off-by: Songhee Baek <sbaek@nvidia.com>

Looks in my opinion much better than the previous version :) Just a few 
minor issues, comments inline

> ---
>   include/sound/soc-dapm.h |   10 ++++
>   include/sound/soc.h      |   22 +++++--
>   sound/soc/soc-core.c     |   12 ++--
>   sound/soc/soc-dapm.c     |  143 +++++++++++++++++++++++++++++++++++++++++-----
>   4 files changed, 162 insertions(+), 25 deletions(-)
>
> diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
> index ef78f56..983b0ab 100644
> --- a/include/sound/soc-dapm.h
> +++ b/include/sound/soc-dapm.h
> @@ -305,6 +305,12 @@ struct device;
>    	.get = snd_soc_dapm_get_enum_double, \
>    	.put = snd_soc_dapm_put_enum_double, \
>     	.private_value = (unsigned long)&xenum }
> +#define SOC_DAPM_ENUM_WIDE(xname, xenum) \

maybe just call it ENUM_ONEHOT, since it doesn't actually have to be more 
than one register.

[...]
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index cd52d52..aba0094 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -2601,12 +2601,12 @@ int snd_soc_get_enum_double(struct snd_kcontrol *kcontrol,
>   	unsigned int val, item;
>   	unsigned int reg_val;
>
> -	reg_val = snd_soc_read(codec, e->reg);
> -	val = (reg_val >> e->shift_l) & e->mask;
> +	reg_val = snd_soc_read(codec, e->reg[0]);
> +	val = (reg_val >> e->shift_l) & e->mask[0];
>   	item = snd_soc_enum_val_to_item(e, val);
>   	ucontrol->value.enumerated.item[0] = item;
>   	if (e->shift_l != e->shift_r) {
> -		val = (reg_val >> e->shift_l) & e->mask;
> +		val = (reg_val >> e->shift_l) & e->mask[0];
>   		item = snd_soc_enum_val_to_item(e, val);
>   		ucontrol->value.enumerated.item[1] = item;
>   	}
> @@ -2636,15 +2636,15 @@ int snd_soc_put_enum_double(struct snd_kcontrol *kcontrol,
>   	if (item[0] >= e->items)
>   		return -EINVAL;
>   	val = snd_soc_enum_item_to_val(e, item[0]) << e->shift_l;
> -	mask = e->mask << e->shift_l;
> +	mask = e->mask[0] << e->shift_l;
>   	if (e->shift_l != e->shift_r) {
>   		if (item[1] >= e->items)
>   			return -EINVAL;
>   		val |= snd_soc_enum_item_to_val(e, item[1]) << e->shift_r;
> -		mask |= e->mask << e->shift_r;
> +		mask |= e->mask[0] << e->shift_r;
>   	}
>
> -	return snd_soc_update_bits_locked(codec, e->reg, mask, val);
> +	return snd_soc_update_bits_locked(codec, e->reg[0], mask, val);
>   }
>   EXPORT_SYMBOL_GPL(snd_soc_put_enum_double);
>
> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
> index c8a780d..4d2b35c 100644
> --- a/sound/soc/soc-dapm.c
> +++ b/sound/soc/soc-dapm.c
> @@ -514,9 +514,9 @@ static int dapm_connect_mux(struct snd_soc_dapm_context *dapm,
>   	unsigned int val, item;
>   	int i;
>
> -	if (e->reg != SND_SOC_NOPM) {
> -		soc_widget_read(dest, e->reg, &val);
> -		val = (val >> e->shift_l) & e->mask;
> +	if (e->reg[0] != SND_SOC_NOPM) {
> +		soc_widget_read(dest, e->reg[0], &val);
> +		val = (val >> e->shift_l) & e->mask[0];
>   		item = snd_soc_enum_val_to_item(e, val);

This probably should handle the new enum type as well. You'll probably need 
some kind of flag in the struct to distinguish between the two enum types.

>   	} else {
>   		/* since a virtual mux has no backing registers to
[...]
>   /**
> + * snd_soc_dapm_get_enum_wide - dapm semi enumerated multiple registers

What's a semi-enumerated register?

> + *					mixer get callback
> + * @kcontrol: mixer control
> + * @ucontrol: control element information
> + *
> + * Callback to get the value of a dapm semi enumerated multiple register mixer
> + * control.
> + *
> + * semi enumerated multiple registers mixer:
> + * the mixer has multiple registers to set the enumerated items. The enumerated
> + * items are referred as values.
> + * Can be used for handling bit field coded enumeration for example.
> + *
> + * Returns 0 for success.
> + */
> +int snd_soc_dapm_get_enum_wide(struct snd_kcontrol *kcontrol,
> +			struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_codec *codec = snd_soc_dapm_kcontrol_codec(kcontrol);
> +	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
> +	unsigned int reg_val, val, bit_pos = 0, reg_idx;
> +
> +	for (reg_idx = 0; reg_idx < e->num_regs; reg_idx++) {
> +		reg_val = snd_soc_read(codec, e->reg[reg_idx]);
> +		val = reg_val & e->mask[reg_idx];
> +		if (val != 0) {
> +			bit_pos = ffs(val) + (e->reg_width * reg_idx);

Should be __ffs. __ffs returns the bits zero-indexed and ffs one-indexed. 
That will work better for cases where there is not additional value table 
necessary, since it means bit 1 maps to value 0.

> +			break;
> +		}
> +	}
> +
> +	ucontrol->value.enumerated.item[0] =
> +			snd_soc_enum_val_to_item(e, bit_pos);
> +
> +	return 0;
> +}
[...]
> +int snd_soc_dapm_put_enum_wide(struct snd_kcontrol *kcontrol,
> +			struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_codec *codec = snd_soc_dapm_kcontrol_codec(kcontrol);
> +	struct snd_soc_card *card = codec->card;
> +	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
> +	unsigned int *item = ucontrol->value.enumerated.item;
> +	unsigned int change = 0, reg_idx = 0, value, bit_pos;
> +	struct snd_soc_dapm_update update;
> +	int ret = 0, reg_val = 0, i;
> +
> +	if (item[0] >= e->items)
> +		return -EINVAL;
> +
> +	value = snd_soc_enum_item_to_val(e, item[0]);
> +
> +	if (value) {
> +		/* get the register index and value to set */
> +		reg_idx = (value - 1) / e->reg_width;
> +		bit_pos = (value - 1) % e->reg_width;

Changing the ffs to __ffs also means you can drop the ' - 1' here.

Also e->reg_width should be (codec->val_bytes * 8) and reg_width field 
should be dropped from the enum struct.

> +		reg_val = BIT(bit_pos);
> +	}
> +
> +	for (i = 0; i < e->num_regs; i++) {
> +		if (i == reg_idx) {
> +			change = snd_soc_test_bits(codec, e->reg[i],
> +							e->mask[i], reg_val);
> +
> +		} else {
> +			/* accumulate the change to update the DAPM path
> +			    when none is selected */
> +			change += snd_soc_test_bits(codec, e->reg[i],
> +							e->mask[i], 0);

change |=

> +
> +			/* clear the register when not selected */
> +			snd_soc_write(codec, e->reg[i], 0);

I think this should happen as part of the DAPM update sequence like you had 
earlier. Some special care should probably be take to make sure that you 
de-select the previous mux input before selecting the new one if the new one 
is in a different register than the previous one.

> +		}
> +	}
> +
> +	mutex_lock_nested(&card->dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME);
> +
[...]

Re: [PATCH] ASoC: DAPM: Add support for multi register mux

[Arun Shamanna Lakshmi]

> -----Original Message-----
> From: Lars-Peter Clausen [mailto:lars@metafoo.de]
> Sent: Tuesday, April 01, 2014 12:48 AM
> To: Arun Shamanna Lakshmi
> Cc: lgirdwood@gmail.com; broonie@kernel.org;
swarren@wwwdotorg.org;
> perex@perex.cz; tiwai@suse.de; alsa- devel@alsa-project.org;
> linux-kernel@vger.kernel.org; Songhee Baek
> Subject: Re: [PATCH] ASoC: DAPM: Add support for multi register mux
>
> On 04/01/2014 08:21 AM, Arun Shamanna Lakshmi wrote:
> > Modify soc_enum struct to handle pointers for reg and mask. Add
dapm
> > get and put APIs for multi register mux with one hot encoding.
> >
> > Signed-off-by: Arun Shamanna Lakshmi <aruns@nvidia.com>
> > Signed-off-by: Songhee Baek <sbaek@nvidia.com>
>
> Looks in my opinion much better than the previous version :) Just a
> few minor issues, comments inline
>
> > ---
> >   include/sound/soc-dapm.h |   10 ++++
> >   include/sound/soc.h      |   22 +++++--
> >   sound/soc/soc-core.c     |   12 ++--
> >   sound/soc/soc-dapm.c     |  143
> +++++++++++++++++++++++++++++++++++++++++-----
> >   4 files changed, 162 insertions(+), 25 deletions(-)
> >
> > diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
> index
> > ef78f56..983b0ab 100644
> > --- a/include/sound/soc-dapm.h
> > +++ b/include/sound/soc-dapm.h
> > @@ -305,6 +305,12 @@ struct device;
> >    	.get = snd_soc_dapm_get_enum_double, \
> >    	.put = snd_soc_dapm_put_enum_double, \
> >     	.private_value = (unsigned long)&xenum }
> > +#define SOC_DAPM_ENUM_WIDE(xname, xenum) \
>
> maybe just call it ENUM_ONEHOT, since it doesn't actually have to be
> more than one register.
>
> [...]
> > diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index
> > cd52d52..aba0094 100644
> > --- a/sound/soc/soc-core.c
> > +++ b/sound/soc/soc-core.c
> > @@ -2601,12 +2601,12 @@ int snd_soc_get_enum_double(struct
> snd_kcontrol *kcontrol,
> >   	unsigned int val, item;
> >   	unsigned int reg_val;
> >
> > -	reg_val = snd_soc_read(codec, e->reg);
> > -	val = (reg_val >> e->shift_l) & e->mask;
> > +	reg_val = snd_soc_read(codec, e->reg[0]);
> > +	val = (reg_val >> e->shift_l) & e->mask[0];
> >   	item = snd_soc_enum_val_to_item(e, val);
> >   	ucontrol->value.enumerated.item[0] = item;
> >   	if (e->shift_l != e->shift_r) {
> > -		val = (reg_val >> e->shift_l) & e->mask;
> > +		val = (reg_val >> e->shift_l) & e->mask[0];
> >   		item = snd_soc_enum_val_to_item(e, val);
> >   		ucontrol->value.enumerated.item[1] = item;
> >   	}
> > @@ -2636,15 +2636,15 @@ int snd_soc_put_enum_double(struct
> snd_kcontrol *kcontrol,
> >   	if (item[0] >= e->items)
> >   		return -EINVAL;
> >   	val = snd_soc_enum_item_to_val(e, item[0]) << e->shift_l;
> > -	mask = e->mask << e->shift_l;
> > +	mask = e->mask[0] << e->shift_l;
> >   	if (e->shift_l != e->shift_r) {
> >   		if (item[1] >= e->items)
> >   			return -EINVAL;
> >   		val |= snd_soc_enum_item_to_val(e, item[1]) << e-
shift_r;
> > -		mask |= e->mask << e->shift_r;
> > +		mask |= e->mask[0] << e->shift_r;
> >   	}
> >
> > -	return snd_soc_update_bits_locked(codec, e->reg, mask, val);
> > +	return snd_soc_update_bits_locked(codec, e->reg[0], mask, val);
> >   }
> >   EXPORT_SYMBOL_GPL(snd_soc_put_enum_double);
> >
> > diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index
> > c8a780d..4d2b35c 100644
> > --- a/sound/soc/soc-dapm.c
> > +++ b/sound/soc/soc-dapm.c
> > @@ -514,9 +514,9 @@ static int dapm_connect_mux(struct
> snd_soc_dapm_context *dapm,
> >   	unsigned int val, item;
> >   	int i;
> >
> > -	if (e->reg != SND_SOC_NOPM) {
> > -		soc_widget_read(dest, e->reg, &val);
> > -		val = (val >> e->shift_l) & e->mask;
> > +	if (e->reg[0] != SND_SOC_NOPM) {
> > +		soc_widget_read(dest, e->reg[0], &val);
> > +		val = (val >> e->shift_l) & e->mask[0];
> >   		item = snd_soc_enum_val_to_item(e, val);
>
> This probably should handle the new enum type as well. You'll probably
> need some kind of flag in the struct to distinguish between the two
> enum types.

Any suggestion on the flag name ?

>
> >   	} else {
> >   		/* since a virtual mux has no backing registers to
> [...]
> >   /**
> > + * snd_soc_dapm_get_enum_wide - dapm semi enumerated multiple
> > + registers
>
> What's a semi-enumerated register?
>
> > + *					mixer get callback
> > + * @kcontrol: mixer control
> > + * @ucontrol: control element information
> > + *
> > + * Callback to get the value of a dapm semi enumerated multiple
> > +register mixer
> > + * control.
> > + *
> > + * semi enumerated multiple registers mixer:
> > + * the mixer has multiple registers to set the enumerated items.
> > +The enumerated
> > + * items are referred as values.
> > + * Can be used for handling bit field coded enumeration for example.
> > + *
> > + * Returns 0 for success.
> > + */
> > +int snd_soc_dapm_get_enum_wide(struct snd_kcontrol *kcontrol,
> > +			struct snd_ctl_elem_value *ucontrol) {
> > +	struct snd_soc_codec *codec =
> snd_soc_dapm_kcontrol_codec(kcontrol);
> > +	struct soc_enum *e = (struct soc_enum *)kcontrol-
> >private_value;
> > +	unsigned int reg_val, val, bit_pos = 0, reg_idx;
> > +
> > +	for (reg_idx = 0; reg_idx < e->num_regs; reg_idx++) {
> > +		reg_val = snd_soc_read(codec, e->reg[reg_idx]);
> > +		val = reg_val & e->mask[reg_idx];
> > +		if (val != 0) {
> > +			bit_pos = ffs(val) + (e->reg_width * reg_idx);
>
> Should be __ffs. __ffs returns the bits zero-indexed and ffs one-indexed.
> That will work better for cases where there is not additional value
> table necessary, since it means bit 1 maps to value 0.
>
> > +			break;
> > +		}
> > +	}
> > +
> > +	ucontrol->value.enumerated.item[0] =
> > +			snd_soc_enum_val_to_item(e, bit_pos);
> > +
> > +	return 0;
> > +}
> [...]
> > +int snd_soc_dapm_put_enum_wide(struct snd_kcontrol *kcontrol,
> > +			struct snd_ctl_elem_value *ucontrol) {
> > +	struct snd_soc_codec *codec =
> snd_soc_dapm_kcontrol_codec(kcontrol);
> > +	struct snd_soc_card *card = codec->card;
> > +	struct soc_enum *e = (struct soc_enum *)kcontrol-
> >private_value;
> > +	unsigned int *item = ucontrol->value.enumerated.item;
> > +	unsigned int change = 0, reg_idx = 0, value, bit_pos;
> > +	struct snd_soc_dapm_update update;
> > +	int ret = 0, reg_val = 0, i;
> > +
> > +	if (item[0] >= e->items)
> > +		return -EINVAL;
> > +
> > +	value = snd_soc_enum_item_to_val(e, item[0]);
> > +
> > +	if (value) {
> > +		/* get the register index and value to set */
> > +		reg_idx = (value - 1) / e->reg_width;
> > +		bit_pos = (value - 1) % e->reg_width;
>
> Changing the ffs to __ffs also means you can drop the ' - 1' here.
>
> Also e->reg_width should be (codec->val_bytes * 8) and reg_width field
> should be dropped from the enum struct.
>
> > +		reg_val = BIT(bit_pos);
> > +	}
> > +
> > +	for (i = 0; i < e->num_regs; i++) {
> > +		if (i == reg_idx) {
> > +			change = snd_soc_test_bits(codec, e->reg[i],
> > +							e->mask[i],
> reg_val);
> > +
> > +		} else {
> > +			/* accumulate the change to update the DAPM
> path
> > +			    when none is selected */
> > +			change += snd_soc_test_bits(codec, e->reg[i],
> > +							e->mask[i], 0);
>
> change |=
>
> > +
> > +			/* clear the register when not selected */
> > +			snd_soc_write(codec, e->reg[i], 0);
>
> I think this should happen as part of the DAPM update sequence like
> you had earlier. Some special care should probably be take to make
> sure that you de-select the previous mux input before selecting the
> new one if the new one is in a different register than the previous one.

I am not sure I follow this part. We are clearing the 'not selected'
registers before we set the one we want. Do you want us to loop the
logic of soc_dapm_mux_update_power for each register ? or do you
want to change the dapm_update structure so that it takes all the regs,
masks, and values together ?
>
> > +		}
> > +	}
> > +
> > +	mutex_lock_nested(&card->dapm_mutex,
> SND_SOC_DAPM_CLASS_RUNTIME);
> > +
> [...]

Re: [PATCH] ASoC: DAPM: Add support for multi register mux

[Lars-Peter Clausen]

On 04/01/2014 08:26 PM, Arun Shamanna Lakshmi wrote:
[...]
>>> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index
>>> c8a780d..4d2b35c 100644
>>> --- a/sound/soc/soc-dapm.c
>>> +++ b/sound/soc/soc-dapm.c
>>> @@ -514,9 +514,9 @@ static int dapm_connect_mux(struct
>> snd_soc_dapm_context *dapm,
>>>    	unsigned int val, item;
>>>    	int i;
>>>
>>> -	if (e->reg != SND_SOC_NOPM) {
>>> -		soc_widget_read(dest, e->reg, &val);
>>> -		val = (val >> e->shift_l) & e->mask;
>>> +	if (e->reg[0] != SND_SOC_NOPM) {
>>> +		soc_widget_read(dest, e->reg[0], &val);
>>> +		val = (val >> e->shift_l) & e->mask[0];
>>>    		item = snd_soc_enum_val_to_item(e, val);
>>
>> This probably should handle the new enum type as well. You'll probably
>> need some kind of flag in the struct to distinguish between the two
>> enum types.
>
> Any suggestion on the flag name ?
>

How about 'onehot'?

[...]
>>> +		reg_val = BIT(bit_pos);
>>> +	}
>>> +
>>> +	for (i = 0; i < e->num_regs; i++) {
>>> +		if (i == reg_idx) {
>>> +			change = snd_soc_test_bits(codec, e->reg[i],
>>> +							e->mask[i],
>> reg_val);
>>> +
>>> +		} else {
>>> +			/* accumulate the change to update the DAPM
>> path
>>> +			    when none is selected */
>>> +			change += snd_soc_test_bits(codec, e->reg[i],
>>> +							e->mask[i], 0);
>>
>> change |=
>>
>>> +
>>> +			/* clear the register when not selected */
>>> +			snd_soc_write(codec, e->reg[i], 0);
>>
>> I think this should happen as part of the DAPM update sequence like
>> you had earlier. Some special care should probably be take to make
>> sure that you de-select the previous mux input before selecting the
>> new one if the new one is in a different register than the previous one.
>
> I am not sure I follow this part. We are clearing the 'not selected'
> registers before we set the one we want. Do you want us to loop the
> logic of soc_dapm_mux_update_power for each register ? or do you
> want to change the dapm_update structure so that it takes all the regs,
> masks, and values together ?

The idea with the dapm_update struct is that the register updates are done 
in the middle of the power-down and power-up sequence. So yes, change the 
dapm_update struct to be able to hold all register updates and do all 
register updates in dapm_widget_update. I think an earlier version of your 
patch already had this.

Re: [PATCH] ASoC: DAPM: Add support for multi register mux

[Songhee Baek]

> -----Original Message-----
> From: Lars-Peter Clausen [mailto:lars@metafoo.de]
> Sent: Tuesday, April 01, 2014 11:00 PM
> To: Arun Shamanna Lakshmi
> Cc: lgirdwood@gmail.com; broonie@kernel.org; swarren@wwwdotorg.org;
> perex@perex.cz; tiwai@suse.de; alsa-devel@alsa-project.org; linux-
> kernel@vger.kernel.org; Songhee Baek
> Subject: Re: [PATCH] ASoC: DAPM: Add support for multi register mux
> 
> On 04/01/2014 08:26 PM, Arun Shamanna Lakshmi wrote:
> [...]
> >>> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index
> >>> c8a780d..4d2b35c 100644
> >>> --- a/sound/soc/soc-dapm.c
> >>> +++ b/sound/soc/soc-dapm.c
> >>> @@ -514,9 +514,9 @@ static int dapm_connect_mux(struct
> >> snd_soc_dapm_context *dapm,
> >>>    	unsigned int val, item;
> >>>    	int i;
> >>>
> >>> -	if (e->reg != SND_SOC_NOPM) {
> >>> -		soc_widget_read(dest, e->reg, &val);
> >>> -		val = (val >> e->shift_l) & e->mask;
> >>> +	if (e->reg[0] != SND_SOC_NOPM) {
> >>> +		soc_widget_read(dest, e->reg[0], &val);
> >>> +		val = (val >> e->shift_l) & e->mask[0];
> >>>    		item = snd_soc_enum_val_to_item(e, val);
> >>
> >> This probably should handle the new enum type as well. You'll
> >> probably need some kind of flag in the struct to distinguish between
> >> the two enum types.
> >
> > Any suggestion on the flag name ?
> >
> 
> How about 'onehot'?
> 
> [...]
> >>> +		reg_val = BIT(bit_pos);
> >>> +	}
> >>> +
> >>> +	for (i = 0; i < e->num_regs; i++) {
> >>> +		if (i == reg_idx) {
> >>> +			change = snd_soc_test_bits(codec, e->reg[i],
> >>> +							e->mask[i],
> >> reg_val);
> >>> +
> >>> +		} else {
> >>> +			/* accumulate the change to update the DAPM
> >> path
> >>> +			    when none is selected */
> >>> +			change += snd_soc_test_bits(codec, e->reg[i],
> >>> +							e->mask[i], 0);
> >>
> >> change |=
> >>
> >>> +
> >>> +			/* clear the register when not selected */
> >>> +			snd_soc_write(codec, e->reg[i], 0);
> >>
> >> I think this should happen as part of the DAPM update sequence like
> >> you had earlier. Some special care should probably be take to make
> >> sure that you de-select the previous mux input before selecting the
> >> new one if the new one is in a different register than the previous one.
> >
> > I am not sure I follow this part. We are clearing the 'not selected'
> > registers before we set the one we want. Do you want us to loop the
> > logic of soc_dapm_mux_update_power for each register ? or do you want
> > to change the dapm_update structure so that it takes all the regs,
> > masks, and values together ?
> 
> The idea with the dapm_update struct is that the register updates are done
> in the middle of the power-down and power-up sequence. So yes, change
> the dapm_update struct to be able to hold all register updates and do all
> register updates in dapm_widget_update. I think an earlier version of your
> patch already had this.

Is the change similar to as shown below?

for (reg_idx = 0; reg_idx < e->num_regs; reg_idx++) {
	val = e->values[item * e->num_regs + reg_idx];
	ret = snd_soc_update_bits_locked(codec, e->reg[reg_idx],
				e->mask[reg_idx], val);
	if (ret)
	return ret;
}

During updating of the register's value, the above change can create non-zero
value in two different registers (very short transition) as Mark mentioned for
that change so we need to clear register first before writing the desired value
in the register.

Should we add the clearing all registers and write the mux value in desired
register in the update function?

Re: [PATCH] ASoC: DAPM: Add support for multi register mux

[Lars-Peter Clausen]

On 04/02/2014 08:17 AM, Songhee Baek wrote:
>> -----Original Message-----
>> From: Lars-Peter Clausen [mailto:lars@metafoo.de]
>> Sent: Tuesday, April 01, 2014 11:00 PM
>> To: Arun Shamanna Lakshmi
>> Cc: lgirdwood@gmail.com; broonie@kernel.org; swarren@wwwdotorg.org;
>> perex@perex.cz; tiwai@suse.de; alsa-devel@alsa-project.org; linux-
>> kernel@vger.kernel.org; Songhee Baek
>> Subject: Re: [PATCH] ASoC: DAPM: Add support for multi register mux
>>
>> On 04/01/2014 08:26 PM, Arun Shamanna Lakshmi wrote:
>> [...]
>>>>> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index
>>>>> c8a780d..4d2b35c 100644
>>>>> --- a/sound/soc/soc-dapm.c
>>>>> +++ b/sound/soc/soc-dapm.c
>>>>> @@ -514,9 +514,9 @@ static int dapm_connect_mux(struct
>>>> snd_soc_dapm_context *dapm,
>>>>>     	unsigned int val, item;
>>>>>     	int i;
>>>>>
>>>>> -	if (e->reg != SND_SOC_NOPM) {
>>>>> -		soc_widget_read(dest, e->reg, &val);
>>>>> -		val = (val >> e->shift_l) & e->mask;
>>>>> +	if (e->reg[0] != SND_SOC_NOPM) {
>>>>> +		soc_widget_read(dest, e->reg[0], &val);
>>>>> +		val = (val >> e->shift_l) & e->mask[0];
>>>>>     		item = snd_soc_enum_val_to_item(e, val);
>>>>
>>>> This probably should handle the new enum type as well. You'll
>>>> probably need some kind of flag in the struct to distinguish between
>>>> the two enum types.
>>>
>>> Any suggestion on the flag name ?
>>>
>>
>> How about 'onehot'?
>>
>> [...]
>>>>> +		reg_val = BIT(bit_pos);
>>>>> +	}
>>>>> +
>>>>> +	for (i = 0; i < e->num_regs; i++) {
>>>>> +		if (i == reg_idx) {
>>>>> +			change = snd_soc_test_bits(codec, e->reg[i],
>>>>> +							e->mask[i],
>>>> reg_val);
>>>>> +
>>>>> +		} else {
>>>>> +			/* accumulate the change to update the DAPM
>>>> path
>>>>> +			    when none is selected */
>>>>> +			change += snd_soc_test_bits(codec, e->reg[i],
>>>>> +							e->mask[i], 0);
>>>>
>>>> change |=
>>>>
>>>>> +
>>>>> +			/* clear the register when not selected */
>>>>> +			snd_soc_write(codec, e->reg[i], 0);
>>>>
>>>> I think this should happen as part of the DAPM update sequence like
>>>> you had earlier. Some special care should probably be take to make
>>>> sure that you de-select the previous mux input before selecting the
>>>> new one if the new one is in a different register than the previous one.
>>>
>>> I am not sure I follow this part. We are clearing the 'not selected'
>>> registers before we set the one we want. Do you want us to loop the
>>> logic of soc_dapm_mux_update_power for each register ? or do you want
>>> to change the dapm_update structure so that it takes all the regs,
>>> masks, and values together ?
>>
>> The idea with the dapm_update struct is that the register updates are done
>> in the middle of the power-down and power-up sequence. So yes, change
>> the dapm_update struct to be able to hold all register updates and do all
>> register updates in dapm_widget_update. I think an earlier version of your
>> patch already had this.
>
> Is the change similar to as shown below?
>
> for (reg_idx = 0; reg_idx < e->num_regs; reg_idx++) {
> 	val = e->values[item * e->num_regs + reg_idx];
> 	ret = snd_soc_update_bits_locked(codec, e->reg[reg_idx],
> 				e->mask[reg_idx], val);
> 	if (ret)
> 	return ret;
> }
>
> During updating of the register's value, the above change can create non-zero
> value in two different registers (very short transition) as Mark mentioned for
> that change so we need to clear register first before writing the desired value
> in the register.
>
> Should we add the clearing all registers and write the mux value in desired
> register in the update function?
>

In dapm_update_widget() you have this line:

  ret = soc_widget_update_bits(w, update->reg, update->mask, update->val);

That needs to be done for every register update. When you setup the update 
struct you need to make sure that the register clears come before the 
register set.

E.g. if you have register 0x3, 0x4, 0x5 and you select a bit in register 0x4 
it should look like this.

update->reg[0] = 0x3;
update->val[0] = 0x0;
update->reg[1] = 0x5;
update->val[1] = 0x0;
update->reg[2] = 0x4;
update->val[2] = 0x8;

When you set a bit in register 0x3 it should look like this:

update->reg[0] = 0x4;
update->val[0] = 0x0;
update->reg[1] = 0x5;
update->val[1] = 0x0;
update->reg[2] = 0x3;
update->val[2] = 0x1;

So basically the write operation goes into update->reg[e->num_regs-1] the 
clear operations go into the other slots before that.

- Lars

RE: [PATCH] ASoC: DAPM: Add support for multi register mux

[Songhee Baek]

> -----Original Message-----
> From: Lars-Peter Clausen [mailto:lars@metafoo.de]
> Sent: Tuesday, April 01, 2014 11:47 PM
> To: Songhee Baek
> Cc: Arun Shamanna Lakshmi; lgirdwood@gmail.com; broonie@kernel.org;
> swarren@wwwdotorg.org; perex@perex.cz; tiwai@suse.de; alsa-
> devel@alsa-project.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] ASoC: DAPM: Add support for multi register mux
> 
> On 04/02/2014 08:17 AM, Songhee Baek wrote:
> >> -----Original Message-----
> >> From: Lars-Peter Clausen [mailto:lars@metafoo.de]
> >> Sent: Tuesday, April 01, 2014 11:00 PM
> >> To: Arun Shamanna Lakshmi
> >> Cc: lgirdwood@gmail.com; broonie@kernel.org;
> swarren@wwwdotorg.org;
> >> perex@perex.cz; tiwai@suse.de; alsa-devel@alsa-project.org; linux-
> >> kernel@vger.kernel.org; Songhee Baek
> >> Subject: Re: [PATCH] ASoC: DAPM: Add support for multi register mux
> >>
> >> On 04/01/2014 08:26 PM, Arun Shamanna Lakshmi wrote:
> >> [...]
> >>>>> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index
> >>>>> c8a780d..4d2b35c 100644
> >>>>> --- a/sound/soc/soc-dapm.c
> >>>>> +++ b/sound/soc/soc-dapm.c
> >>>>> @@ -514,9 +514,9 @@ static int dapm_connect_mux(struct
> >>>> snd_soc_dapm_context *dapm,
> >>>>>     	unsigned int val, item;
> >>>>>     	int i;
> >>>>>
> >>>>> -	if (e->reg != SND_SOC_NOPM) {
> >>>>> -		soc_widget_read(dest, e->reg, &val);
> >>>>> -		val = (val >> e->shift_l) & e->mask;
> >>>>> +	if (e->reg[0] != SND_SOC_NOPM) {
> >>>>> +		soc_widget_read(dest, e->reg[0], &val);
> >>>>> +		val = (val >> e->shift_l) & e->mask[0];
> >>>>>     		item = snd_soc_enum_val_to_item(e, val);
> >>>>
> >>>> This probably should handle the new enum type as well. You'll
> >>>> probably need some kind of flag in the struct to distinguish
> >>>> between the two enum types.
> >>>
> >>> Any suggestion on the flag name ?
> >>>
> >>
> >> How about 'onehot'?
> >>
> >> [...]
> >>>>> +		reg_val = BIT(bit_pos);
> >>>>> +	}
> >>>>> +
> >>>>> +	for (i = 0; i < e->num_regs; i++) {
> >>>>> +		if (i == reg_idx) {
> >>>>> +			change = snd_soc_test_bits(codec, e->reg[i],
> >>>>> +							e->mask[i],
> >>>> reg_val);
> >>>>> +
> >>>>> +		} else {
> >>>>> +			/* accumulate the change to update the
> DAPM
> >>>> path
> >>>>> +			    when none is selected */
> >>>>> +			change += snd_soc_test_bits(codec, e-
> >reg[i],
> >>>>> +							e->mask[i], 0);
> >>>>
> >>>> change |=
> >>>>
> >>>>> +
> >>>>> +			/* clear the register when not selected */
> >>>>> +			snd_soc_write(codec, e->reg[i], 0);
> >>>>
> >>>> I think this should happen as part of the DAPM update sequence like
> >>>> you had earlier. Some special care should probably be take to make
> >>>> sure that you de-select the previous mux input before selecting the
> >>>> new one if the new one is in a different register than the previous one.
> >>>
> >>> I am not sure I follow this part. We are clearing the 'not selected'
> >>> registers before we set the one we want. Do you want us to loop the
> >>> logic of soc_dapm_mux_update_power for each register ? or do you
> >>> want to change the dapm_update structure so that it takes all the
> >>> regs, masks, and values together ?
> >>
> >> The idea with the dapm_update struct is that the register updates are
> >> done in the middle of the power-down and power-up sequence. So yes,
> >> change the dapm_update struct to be able to hold all register updates
> >> and do all register updates in dapm_widget_update. I think an earlier
> >> version of your patch already had this.
> >
> > Is the change similar to as shown below?
> >
> > for (reg_idx = 0; reg_idx < e->num_regs; reg_idx++) {
> > 	val = e->values[item * e->num_regs + reg_idx];
> > 	ret = snd_soc_update_bits_locked(codec, e->reg[reg_idx],
> > 				e->mask[reg_idx], val);
> > 	if (ret)
> > 	return ret;
> > }
> >
> > During updating of the register's value, the above change can create
> > non-zero value in two different registers (very short transition) as
> > Mark mentioned for that change so we need to clear register first
> > before writing the desired value in the register.
> >
> > Should we add the clearing all registers and write the mux value in
> > desired register in the update function?
> >
> 
> In dapm_update_widget() you have this line:
> 
>   ret = soc_widget_update_bits(w, update->reg, update->mask, update-
> >val);
> 
> That needs to be done for every register update. When you setup the
> update struct you need to make sure that the register clears come before
> the register set.
> 
> E.g. if you have register 0x3, 0x4, 0x5 and you select a bit in register 0x4 it
> should look like this.
> 
> update->reg[0] = 0x3;
> update->val[0] = 0x0;
> update->reg[1] = 0x5;
> update->val[1] = 0x0;
> update->reg[2] = 0x4;
> update->val[2] = 0x8;
> 
> When you set a bit in register 0x3 it should look like this:
> 
> update->reg[0] = 0x4;
> update->val[0] = 0x0;
> update->reg[1] = 0x5;
> update->val[1] = 0x0;
> update->reg[2] = 0x3;
> update->val[2] = 0x1;
> 
> So basically the write operation goes into update->reg[e->num_regs-1] the
> clear operations go into the other slots before that.

Does update reg/val array have the writing sequence, is it correct?
And can I assume that update struct has reg/val/mask arrays not pointers?

> 
> - Lars

Re: [PATCH] ASoC: DAPM: Add support for multi register mux

[Lars-Peter Clausen]

On 04/02/2014 08:56 AM, Songhee Baek wrote:
>
>
>> -----Original Message-----
>> From: Lars-Peter Clausen [mailto:lars@metafoo.de]
>> Sent: Tuesday, April 01, 2014 11:47 PM
>> To: Songhee Baek
>> Cc: Arun Shamanna Lakshmi; lgirdwood@gmail.com; broonie@kernel.org;
>> swarren@wwwdotorg.org; perex@perex.cz; tiwai@suse.de; alsa-
>> devel@alsa-project.org; linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH] ASoC: DAPM: Add support for multi register mux
>>
>> On 04/02/2014 08:17 AM, Songhee Baek wrote:
>>>> -----Original Message-----
>>>> From: Lars-Peter Clausen [mailto:lars@metafoo.de]
>>>> Sent: Tuesday, April 01, 2014 11:00 PM
>>>> To: Arun Shamanna Lakshmi
>>>> Cc: lgirdwood@gmail.com; broonie@kernel.org;
>> swarren@wwwdotorg.org;
>>>> perex@perex.cz; tiwai@suse.de; alsa-devel@alsa-project.org; linux-
>>>> kernel@vger.kernel.org; Songhee Baek
>>>> Subject: Re: [PATCH] ASoC: DAPM: Add support for multi register mux
>>>>
>>>> On 04/01/2014 08:26 PM, Arun Shamanna Lakshmi wrote:
>>>> [...]
>>>>>>> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index
>>>>>>> c8a780d..4d2b35c 100644
>>>>>>> --- a/sound/soc/soc-dapm.c
>>>>>>> +++ b/sound/soc/soc-dapm.c
>>>>>>> @@ -514,9 +514,9 @@ static int dapm_connect_mux(struct
>>>>>> snd_soc_dapm_context *dapm,
>>>>>>>      	unsigned int val, item;
>>>>>>>      	int i;
>>>>>>>
>>>>>>> -	if (e->reg != SND_SOC_NOPM) {
>>>>>>> -		soc_widget_read(dest, e->reg, &val);
>>>>>>> -		val = (val >> e->shift_l) & e->mask;
>>>>>>> +	if (e->reg[0] != SND_SOC_NOPM) {
>>>>>>> +		soc_widget_read(dest, e->reg[0], &val);
>>>>>>> +		val = (val >> e->shift_l) & e->mask[0];
>>>>>>>      		item = snd_soc_enum_val_to_item(e, val);
>>>>>>
>>>>>> This probably should handle the new enum type as well. You'll
>>>>>> probably need some kind of flag in the struct to distinguish
>>>>>> between the two enum types.
>>>>>
>>>>> Any suggestion on the flag name ?
>>>>>
>>>>
>>>> How about 'onehot'?
>>>>
>>>> [...]
>>>>>>> +		reg_val = BIT(bit_pos);
>>>>>>> +	}
>>>>>>> +
>>>>>>> +	for (i = 0; i < e->num_regs; i++) {
>>>>>>> +		if (i == reg_idx) {
>>>>>>> +			change = snd_soc_test_bits(codec, e->reg[i],
>>>>>>> +							e->mask[i],
>>>>>> reg_val);
>>>>>>> +
>>>>>>> +		} else {
>>>>>>> +			/* accumulate the change to update the
>> DAPM
>>>>>> path
>>>>>>> +			    when none is selected */
>>>>>>> +			change += snd_soc_test_bits(codec, e-
>>> reg[i],
>>>>>>> +							e->mask[i], 0);
>>>>>>
>>>>>> change |=
>>>>>>
>>>>>>> +
>>>>>>> +			/* clear the register when not selected */
>>>>>>> +			snd_soc_write(codec, e->reg[i], 0);
>>>>>>
>>>>>> I think this should happen as part of the DAPM update sequence like
>>>>>> you had earlier. Some special care should probably be take to make
>>>>>> sure that you de-select the previous mux input before selecting the
>>>>>> new one if the new one is in a different register than the previous one.
>>>>>
>>>>> I am not sure I follow this part. We are clearing the 'not selected'
>>>>> registers before we set the one we want. Do you want us to loop the
>>>>> logic of soc_dapm_mux_update_power for each register ? or do you
>>>>> want to change the dapm_update structure so that it takes all the
>>>>> regs, masks, and values together ?
>>>>
>>>> The idea with the dapm_update struct is that the register updates are
>>>> done in the middle of the power-down and power-up sequence. So yes,
>>>> change the dapm_update struct to be able to hold all register updates
>>>> and do all register updates in dapm_widget_update. I think an earlier
>>>> version of your patch already had this.
>>>
>>> Is the change similar to as shown below?
>>>
>>> for (reg_idx = 0; reg_idx < e->num_regs; reg_idx++) {
>>> 	val = e->values[item * e->num_regs + reg_idx];
>>> 	ret = snd_soc_update_bits_locked(codec, e->reg[reg_idx],
>>> 				e->mask[reg_idx], val);
>>> 	if (ret)
>>> 	return ret;
>>> }
>>>
>>> During updating of the register's value, the above change can create
>>> non-zero value in two different registers (very short transition) as
>>> Mark mentioned for that change so we need to clear register first
>>> before writing the desired value in the register.
>>>
>>> Should we add the clearing all registers and write the mux value in
>>> desired register in the update function?
>>>
>>
>> In dapm_update_widget() you have this line:
>>
>>    ret = soc_widget_update_bits(w, update->reg, update->mask, update-
>>> val);
>>
>> That needs to be done for every register update. When you setup the
>> update struct you need to make sure that the register clears come before
>> the register set.
>>
>> E.g. if you have register 0x3, 0x4, 0x5 and you select a bit in register 0x4 it
>> should look like this.
>>
>> update->reg[0] = 0x3;
>> update->val[0] = 0x0;
>> update->reg[1] = 0x5;
>> update->val[1] = 0x0;
>> update->reg[2] = 0x4;
>> update->val[2] = 0x8;
>>
>> When you set a bit in register 0x3 it should look like this:
>>
>> update->reg[0] = 0x4;
>> update->val[0] = 0x0;
>> update->reg[1] = 0x5;
>> update->val[1] = 0x0;
>> update->reg[2] = 0x3;
>> update->val[2] = 0x1;
>>
>> So basically the write operation goes into update->reg[e->num_regs-1] the
>> clear operations go into the other slots before that.
>
> Does update reg/val array have the writing sequence, is it correct?
> And can I assume that update struct has reg/val/mask arrays not pointers?

Right now the update struct does not have support for multiple register 
writes. That's up to you to implement this. I guess making it an array for 
now should be fine. But you need to add some safety checks to make sure that 
num_regs is not larger or equal to the array size.

Re: [PATCH] ASoC: DAPM: Add support for multi register mux

[Songhee Baek]

> -----Original Message-----
> From: Lars-Peter Clausen [mailto:lars@metafoo.de]
> Sent: Wednesday, April 02, 2014 12:02 AM
> To: Songhee Baek
> Cc: Arun Shamanna Lakshmi; lgirdwood@gmail.com; broonie@kernel.org;
> swarren@wwwdotorg.org; perex@perex.cz; tiwai@suse.de; alsa-
> devel@alsa-project.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] ASoC: DAPM: Add support for multi register mux
> 
> On 04/02/2014 08:56 AM, Songhee Baek wrote:
> >
> >
> >> -----Original Message-----
> >> From: Lars-Peter Clausen [mailto:lars@metafoo.de]
> >> Sent: Tuesday, April 01, 2014 11:47 PM
> >> To: Songhee Baek
> >> Cc: Arun Shamanna Lakshmi; lgirdwood@gmail.com; broonie@kernel.org;
> >> swarren@wwwdotorg.org; perex@perex.cz; tiwai@suse.de; alsa-
> >> devel@alsa-project.org; linux-kernel@vger.kernel.org
> >> Subject: Re: [PATCH] ASoC: DAPM: Add support for multi register mux
> >>
> >> On 04/02/2014 08:17 AM, Songhee Baek wrote:
> >>>> -----Original Message-----
> >>>> From: Lars-Peter Clausen [mailto:lars@metafoo.de]
> >>>> Sent: Tuesday, April 01, 2014 11:00 PM
> >>>> To: Arun Shamanna Lakshmi
> >>>> Cc: lgirdwood@gmail.com; broonie@kernel.org;
> >> swarren@wwwdotorg.org;
> >>>> perex@perex.cz; tiwai@suse.de; alsa-devel@alsa-project.org; linux-
> >>>> kernel@vger.kernel.org; Songhee Baek
> >>>> Subject: Re: [PATCH] ASoC: DAPM: Add support for multi register mux
> >>>>
> >>>> On 04/01/2014 08:26 PM, Arun Shamanna Lakshmi wrote:
> >>>> [...]
> >>>>>>> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index
> >>>>>>> c8a780d..4d2b35c 100644
> >>>>>>> --- a/sound/soc/soc-dapm.c
> >>>>>>> +++ b/sound/soc/soc-dapm.c
> >>>>>>> @@ -514,9 +514,9 @@ static int dapm_connect_mux(struct
> >>>>>> snd_soc_dapm_context *dapm,
> >>>>>>>      	unsigned int val, item;
> >>>>>>>      	int i;
> >>>>>>>
> >>>>>>> -	if (e->reg != SND_SOC_NOPM) {
> >>>>>>> -		soc_widget_read(dest, e->reg, &val);
> >>>>>>> -		val = (val >> e->shift_l) & e->mask;
> >>>>>>> +	if (e->reg[0] != SND_SOC_NOPM) {
> >>>>>>> +		soc_widget_read(dest, e->reg[0], &val);
> >>>>>>> +		val = (val >> e->shift_l) & e->mask[0];
> >>>>>>>      		item = snd_soc_enum_val_to_item(e, val);
> >>>>>>
> >>>>>> This probably should handle the new enum type as well. You'll
> >>>>>> probably need some kind of flag in the struct to distinguish
> >>>>>> between the two enum types.
> >>>>>
> >>>>> Any suggestion on the flag name ?
> >>>>>
> >>>>
> >>>> How about 'onehot'?
> >>>>
> >>>> [...]
> >>>>>>> +		reg_val = BIT(bit_pos);
> >>>>>>> +	}
> >>>>>>> +
> >>>>>>> +	for (i = 0; i < e->num_regs; i++) {
> >>>>>>> +		if (i == reg_idx) {
> >>>>>>> +			change = snd_soc_test_bits(codec, e->reg[i],
> >>>>>>> +							e->mask[i],
> >>>>>> reg_val);
> >>>>>>> +
> >>>>>>> +		} else {
> >>>>>>> +			/* accumulate the change to update the
> >> DAPM
> >>>>>> path
> >>>>>>> +			    when none is selected */
> >>>>>>> +			change += snd_soc_test_bits(codec, e-
> >>> reg[i],
> >>>>>>> +							e->mask[i], 0);
> >>>>>>
> >>>>>> change |=
> >>>>>>
> >>>>>>> +
> >>>>>>> +			/* clear the register when not selected */
> >>>>>>> +			snd_soc_write(codec, e->reg[i], 0);
> >>>>>>
> >>>>>> I think this should happen as part of the DAPM update sequence
> >>>>>> like you had earlier. Some special care should probably be take
> >>>>>> to make sure that you de-select the previous mux input before
> >>>>>> selecting the new one if the new one is in a different register than
> the previous one.
> >>>>>
> >>>>> I am not sure I follow this part. We are clearing the 'not selected'
> >>>>> registers before we set the one we want. Do you want us to loop
> >>>>> the logic of soc_dapm_mux_update_power for each register ? or do
> >>>>> you want to change the dapm_update structure so that it takes all
> >>>>> the regs, masks, and values together ?
> >>>>
> >>>> The idea with the dapm_update struct is that the register updates
> >>>> are done in the middle of the power-down and power-up sequence.
> So
> >>>> yes, change the dapm_update struct to be able to hold all register
> >>>> updates and do all register updates in dapm_widget_update. I think
> >>>> an earlier version of your patch already had this.
> >>>
> >>> Is the change similar to as shown below?
> >>>
> >>> for (reg_idx = 0; reg_idx < e->num_regs; reg_idx++) {
> >>> 	val = e->values[item * e->num_regs + reg_idx];
> >>> 	ret = snd_soc_update_bits_locked(codec, e->reg[reg_idx],
> >>> 				e->mask[reg_idx], val);
> >>> 	if (ret)
> >>> 	return ret;
> >>> }
> >>>
> >>> During updating of the register's value, the above change can create
> >>> non-zero value in two different registers (very short transition) as
> >>> Mark mentioned for that change so we need to clear register first
> >>> before writing the desired value in the register.
> >>>
> >>> Should we add the clearing all registers and write the mux value in
> >>> desired register in the update function?
> >>>
> >>
> >> In dapm_update_widget() you have this line:
> >>
> >>    ret = soc_widget_update_bits(w, update->reg, update->mask, update-
> >>> val);
> >>
> >> That needs to be done for every register update. When you setup the
> >> update struct you need to make sure that the register clears come
> >> before the register set.
> >>
> >> E.g. if you have register 0x3, 0x4, 0x5 and you select a bit in
> >> register 0x4 it should look like this.
> >>
> >> update->reg[0] = 0x3;
> >> update->val[0] = 0x0;
> >> update->reg[1] = 0x5;
> >> update->val[1] = 0x0;
> >> update->reg[2] = 0x4;
> >> update->val[2] = 0x8;
> >>
> >> When you set a bit in register 0x3 it should look like this:
> >>
> >> update->reg[0] = 0x4;
> >> update->val[0] = 0x0;
> >> update->reg[1] = 0x5;
> >> update->val[1] = 0x0;
> >> update->reg[2] = 0x3;
> >> update->val[2] = 0x1;
> >>
> >> So basically the write operation goes into update->reg[e->num_regs-1]
> >> the clear operations go into the other slots before that.
> >
> > Does update reg/val array have the writing sequence, is it correct?
> > And can I assume that update struct has reg/val/mask arrays not pointers?
> 
> Right now the update struct does not have support for multiple register
> writes. That's up to you to implement this. I guess making it an array for now
> should be fine. But you need to add some safety checks to make sure that
> num_regs is not larger or equal to the array size.

Thank you for the clarification. We will add this in dapm update struct.

Re: [PATCH] ASoC: DAPM: Add support for multi register mux

[Songhee Baek]

> -----Original Message-----
> From: Lars-Peter Clausen [mailto:lars@metafoo.de]
> Sent: Wednesday, April 02, 2014 12:02 AM
> To: Songhee Baek
> Cc: Arun Shamanna Lakshmi; lgirdwood@gmail.com; broonie@kernel.org;
> swarren@wwwdotorg.org; perex@perex.cz; tiwai@suse.de; alsa-
> devel@alsa-project.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] ASoC: DAPM: Add support for multi register mux
> 
> On 04/02/2014 08:56 AM, Songhee Baek wrote:
> >
> >
> >> -----Original Message-----
> >> From: Lars-Peter Clausen [mailto:lars@metafoo.de]
> >> Sent: Tuesday, April 01, 2014 11:47 PM
> >> To: Songhee Baek
> >> Cc: Arun Shamanna Lakshmi; lgirdwood@gmail.com; broonie@kernel.org;
> >> swarren@wwwdotorg.org; perex@perex.cz; tiwai@suse.de; alsa-
> >> devel@alsa-project.org; linux-kernel@vger.kernel.org
> >> Subject: Re: [PATCH] ASoC: DAPM: Add support for multi register mux
> >>
> >> On 04/02/2014 08:17 AM, Songhee Baek wrote:
> >>>> -----Original Message-----
> >>>> From: Lars-Peter Clausen [mailto:lars@metafoo.de]
> >>>> Sent: Tuesday, April 01, 2014 11:00 PM
> >>>> To: Arun Shamanna Lakshmi
> >>>> Cc: lgirdwood@gmail.com; broonie@kernel.org;
> >> swarren@wwwdotorg.org;
> >>>> perex@perex.cz; tiwai@suse.de; alsa-devel@alsa-project.org; linux-
> >>>> kernel@vger.kernel.org; Songhee Baek
> >>>> Subject: Re: [PATCH] ASoC: DAPM: Add support for multi register mux
> >>>>
> >>>> On 04/01/2014 08:26 PM, Arun Shamanna Lakshmi wrote:
> >>>> [...]
> >>>>>>> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index
> >>>>>>> c8a780d..4d2b35c 100644
> >>>>>>> --- a/sound/soc/soc-dapm.c
> >>>>>>> +++ b/sound/soc/soc-dapm.c
> >>>>>>> @@ -514,9 +514,9 @@ static int dapm_connect_mux(struct
> >>>>>> snd_soc_dapm_context *dapm,
> >>>>>>>      	unsigned int val, item;
> >>>>>>>      	int i;
> >>>>>>>
> >>>>>>> -	if (e->reg != SND_SOC_NOPM) {
> >>>>>>> -		soc_widget_read(dest, e->reg, &val);
> >>>>>>> -		val = (val >> e->shift_l) & e->mask;
> >>>>>>> +	if (e->reg[0] != SND_SOC_NOPM) {
> >>>>>>> +		soc_widget_read(dest, e->reg[0], &val);
> >>>>>>> +		val = (val >> e->shift_l) & e->mask[0];
> >>>>>>>      		item = snd_soc_enum_val_to_item(e, val);
> >>>>>>
> >>>>>> This probably should handle the new enum type as well. You'll
> >>>>>> probably need some kind of flag in the struct to distinguish
> >>>>>> between the two enum types.
> >>>>>
> >>>>> Any suggestion on the flag name ?
> >>>>>
> >>>>
> >>>> How about 'onehot'?
> >>>>
> >>>> [...]
> >>>>>>> +		reg_val = BIT(bit_pos);
> >>>>>>> +	}
> >>>>>>> +
> >>>>>>> +	for (i = 0; i < e->num_regs; i++) {
> >>>>>>> +		if (i == reg_idx) {
> >>>>>>> +			change = snd_soc_test_bits(codec, e->reg[i],
> >>>>>>> +							e->mask[i],
> >>>>>> reg_val);
> >>>>>>> +
> >>>>>>> +		} else {
> >>>>>>> +			/* accumulate the change to update the
> >> DAPM
> >>>>>> path
> >>>>>>> +			    when none is selected */
> >>>>>>> +			change += snd_soc_test_bits(codec, e-
> >>> reg[i],
> >>>>>>> +							e->mask[i], 0);
> >>>>>>
> >>>>>> change |=
> >>>>>>
> >>>>>>> +
> >>>>>>> +			/* clear the register when not selected */
> >>>>>>> +			snd_soc_write(codec, e->reg[i], 0);
> >>>>>>
> >>>>>> I think this should happen as part of the DAPM update sequence
> >>>>>> like you had earlier. Some special care should probably be take
> >>>>>> to make sure that you de-select the previous mux input before
> >>>>>> selecting the new one if the new one is in a different register than
> the previous one.
> >>>>>
> >>>>> I am not sure I follow this part. We are clearing the 'not selected'
> >>>>> registers before we set the one we want. Do you want us to loop
> >>>>> the logic of soc_dapm_mux_update_power for each register ? or do
> >>>>> you want to change the dapm_update structure so that it takes all
> >>>>> the regs, masks, and values together ?
> >>>>
> >>>> The idea with the dapm_update struct is that the register updates
> >>>> are done in the middle of the power-down and power-up sequence.
> So
> >>>> yes, change the dapm_update struct to be able to hold all register
> >>>> updates and do all register updates in dapm_widget_update. I think
> >>>> an earlier version of your patch already had this.
> >>>
> >>> Is the change similar to as shown below?
> >>>
> >>> for (reg_idx = 0; reg_idx < e->num_regs; reg_idx++) {
> >>> 	val = e->values[item * e->num_regs + reg_idx];
> >>> 	ret = snd_soc_update_bits_locked(codec, e->reg[reg_idx],
> >>> 				e->mask[reg_idx], val);
> >>> 	if (ret)
> >>> 	return ret;
> >>> }
> >>>
> >>> During updating of the register's value, the above change can create
> >>> non-zero value in two different registers (very short transition) as
> >>> Mark mentioned for that change so we need to clear register first
> >>> before writing the desired value in the register.
> >>>
> >>> Should we add the clearing all registers and write the mux value in
> >>> desired register in the update function?
> >>>
> >>
> >> In dapm_update_widget() you have this line:
> >>
> >>    ret = soc_widget_update_bits(w, update->reg, update->mask, update-
> >>> val);
> >>
> >> That needs to be done for every register update. When you setup the
> >> update struct you need to make sure that the register clears come
> >> before the register set.
> >>
> >> E.g. if you have register 0x3, 0x4, 0x5 and you select a bit in
> >> register 0x4 it should look like this.
> >>
> >> update->reg[0] = 0x3;
> >> update->val[0] = 0x0;
> >> update->reg[1] = 0x5;
> >> update->val[1] = 0x0;
> >> update->reg[2] = 0x4;
> >> update->val[2] = 0x8;
> >>
> >> When you set a bit in register 0x3 it should look like this:
> >>
> >> update->reg[0] = 0x4;
> >> update->val[0] = 0x0;
> >> update->reg[1] = 0x5;
> >> update->val[1] = 0x0;
> >> update->reg[2] = 0x3;
> >> update->val[2] = 0x1;
> >>
> >> So basically the write operation goes into update->reg[e->num_regs-1]
> >> the clear operations go into the other slots before that.
> >
> > Does update reg/val array have the writing sequence, is it correct?
> > And can I assume that update struct has reg/val/mask arrays not pointers?
> 
> Right now the update struct does not have support for multiple register
> writes. That's up to you to implement this. I guess making it an array for now
> should be fine. But you need to add some safety checks to make sure that
> num_regs is not larger or equal to the array size.

I think that the dapm update struct needs to have reg[2]/val[2]/mask[2].
Because the mux is one-hot coded, only one register has a non-zero value.
So reg[0] will contain the register to be clear and reg[2] has selected register
to be set.
How about your opinion for this?

Re: [PATCH] ASoC: DAPM: Add support for multi register mux

[Lars-Peter Clausen]

On 04/02/2014 05:26 PM, Songhee Baek wrote:
>> -----Original Message-----
>> From: Lars-Peter Clausen [mailto:lars@metafoo.de]
>> Sent: Wednesday, April 02, 2014 12:02 AM
>> To: Songhee Baek
>> Cc: Arun Shamanna Lakshmi; lgirdwood@gmail.com; broonie@kernel.org;
>> swarren@wwwdotorg.org; perex@perex.cz; tiwai@suse.de; alsa-
>> devel@alsa-project.org; linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH] ASoC: DAPM: Add support for multi register mux
>>
>> On 04/02/2014 08:56 AM, Songhee Baek wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Lars-Peter Clausen [mailto:lars@metafoo.de]
>>>> Sent: Tuesday, April 01, 2014 11:47 PM
>>>> To: Songhee Baek
>>>> Cc: Arun Shamanna Lakshmi; lgirdwood@gmail.com; broonie@kernel.org;
>>>> swarren@wwwdotorg.org; perex@perex.cz; tiwai@suse.de; alsa-
>>>> devel@alsa-project.org; linux-kernel@vger.kernel.org
>>>> Subject: Re: [PATCH] ASoC: DAPM: Add support for multi register mux
>>>>
>>>> On 04/02/2014 08:17 AM, Songhee Baek wrote:
>>>>>> -----Original Message-----
>>>>>> From: Lars-Peter Clausen [mailto:lars@metafoo.de]
>>>>>> Sent: Tuesday, April 01, 2014 11:00 PM
>>>>>> To: Arun Shamanna Lakshmi
>>>>>> Cc: lgirdwood@gmail.com; broonie@kernel.org;
>>>> swarren@wwwdotorg.org;
>>>>>> perex@perex.cz; tiwai@suse.de; alsa-devel@alsa-project.org; linux-
>>>>>> kernel@vger.kernel.org; Songhee Baek
>>>>>> Subject: Re: [PATCH] ASoC: DAPM: Add support for multi register mux
>>>>>>
>>>>>> On 04/01/2014 08:26 PM, Arun Shamanna Lakshmi wrote:
>>>>>> [...]
>>>>>>>>> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index
>>>>>>>>> c8a780d..4d2b35c 100644
>>>>>>>>> --- a/sound/soc/soc-dapm.c
>>>>>>>>> +++ b/sound/soc/soc-dapm.c
>>>>>>>>> @@ -514,9 +514,9 @@ static int dapm_connect_mux(struct
>>>>>>>> snd_soc_dapm_context *dapm,
>>>>>>>>>       	unsigned int val, item;
>>>>>>>>>       	int i;
>>>>>>>>>
>>>>>>>>> -	if (e->reg != SND_SOC_NOPM) {
>>>>>>>>> -		soc_widget_read(dest, e->reg, &val);
>>>>>>>>> -		val = (val >> e->shift_l) & e->mask;
>>>>>>>>> +	if (e->reg[0] != SND_SOC_NOPM) {
>>>>>>>>> +		soc_widget_read(dest, e->reg[0], &val);
>>>>>>>>> +		val = (val >> e->shift_l) & e->mask[0];
>>>>>>>>>       		item = snd_soc_enum_val_to_item(e, val);
>>>>>>>>
>>>>>>>> This probably should handle the new enum type as well. You'll
>>>>>>>> probably need some kind of flag in the struct to distinguish
>>>>>>>> between the two enum types.
>>>>>>>
>>>>>>> Any suggestion on the flag name ?
>>>>>>>
>>>>>>
>>>>>> How about 'onehot'?
>>>>>>
>>>>>> [...]
>>>>>>>>> +		reg_val = BIT(bit_pos);
>>>>>>>>> +	}
>>>>>>>>> +
>>>>>>>>> +	for (i = 0; i < e->num_regs; i++) {
>>>>>>>>> +		if (i == reg_idx) {
>>>>>>>>> +			change = snd_soc_test_bits(codec, e->reg[i],
>>>>>>>>> +							e->mask[i],
>>>>>>>> reg_val);
>>>>>>>>> +
>>>>>>>>> +		} else {
>>>>>>>>> +			/* accumulate the change to update the
>>>> DAPM
>>>>>>>> path
>>>>>>>>> +			    when none is selected */
>>>>>>>>> +			change += snd_soc_test_bits(codec, e-
>>>>> reg[i],
>>>>>>>>> +							e->mask[i], 0);
>>>>>>>>
>>>>>>>> change |=
>>>>>>>>
>>>>>>>>> +
>>>>>>>>> +			/* clear the register when not selected */
>>>>>>>>> +			snd_soc_write(codec, e->reg[i], 0);
>>>>>>>>
>>>>>>>> I think this should happen as part of the DAPM update sequence
>>>>>>>> like you had earlier. Some special care should probably be take
>>>>>>>> to make sure that you de-select the previous mux input before
>>>>>>>> selecting the new one if the new one is in a different register than
>> the previous one.
>>>>>>>
>>>>>>> I am not sure I follow this part. We are clearing the 'not selected'
>>>>>>> registers before we set the one we want. Do you want us to loop
>>>>>>> the logic of soc_dapm_mux_update_power for each register ? or do
>>>>>>> you want to change the dapm_update structure so that it takes all
>>>>>>> the regs, masks, and values together ?
>>>>>>
>>>>>> The idea with the dapm_update struct is that the register updates
>>>>>> are done in the middle of the power-down and power-up sequence.
>> So
>>>>>> yes, change the dapm_update struct to be able to hold all register
>>>>>> updates and do all register updates in dapm_widget_update. I think
>>>>>> an earlier version of your patch already had this.
>>>>>
>>>>> Is the change similar to as shown below?
>>>>>
>>>>> for (reg_idx = 0; reg_idx < e->num_regs; reg_idx++) {
>>>>> 	val = e->values[item * e->num_regs + reg_idx];
>>>>> 	ret = snd_soc_update_bits_locked(codec, e->reg[reg_idx],
>>>>> 				e->mask[reg_idx], val);
>>>>> 	if (ret)
>>>>> 	return ret;
>>>>> }
>>>>>
>>>>> During updating of the register's value, the above change can create
>>>>> non-zero value in two different registers (very short transition) as
>>>>> Mark mentioned for that change so we need to clear register first
>>>>> before writing the desired value in the register.
>>>>>
>>>>> Should we add the clearing all registers and write the mux value in
>>>>> desired register in the update function?
>>>>>
>>>>
>>>> In dapm_update_widget() you have this line:
>>>>
>>>>     ret = soc_widget_update_bits(w, update->reg, update->mask, update-
>>>>> val);
>>>>
>>>> That needs to be done for every register update. When you setup the
>>>> update struct you need to make sure that the register clears come
>>>> before the register set.
>>>>
>>>> E.g. if you have register 0x3, 0x4, 0x5 and you select a bit in
>>>> register 0x4 it should look like this.
>>>>
>>>> update->reg[0] = 0x3;
>>>> update->val[0] = 0x0;
>>>> update->reg[1] = 0x5;
>>>> update->val[1] = 0x0;
>>>> update->reg[2] = 0x4;
>>>> update->val[2] = 0x8;
>>>>
>>>> When you set a bit in register 0x3 it should look like this:
>>>>
>>>> update->reg[0] = 0x4;
>>>> update->val[0] = 0x0;
>>>> update->reg[1] = 0x5;
>>>> update->val[1] = 0x0;
>>>> update->reg[2] = 0x3;
>>>> update->val[2] = 0x1;
>>>>
>>>> So basically the write operation goes into update->reg[e->num_regs-1]
>>>> the clear operations go into the other slots before that.
>>>
>>> Does update reg/val array have the writing sequence, is it correct?
>>> And can I assume that update struct has reg/val/mask arrays not pointers?
>>
>> Right now the update struct does not have support for multiple register
>> writes. That's up to you to implement this. I guess making it an array for now
>> should be fine. But you need to add some safety checks to make sure that
>> num_regs is not larger or equal to the array size.
>
> I think that the dapm update struct needs to have reg[2]/val[2]/mask[2].
> Because the mux is one-hot coded, only one register has a non-zero value.
> So reg[0] will contain the register to be clear and reg[2] has selected register
> to be set.
> How about your opinion for this?

Should work (in theory).

- Lars

Re: [PATCH] ASoC: dapm: Add support for multi register mux

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 973 bytes --]

On Fri, Apr 04, 2014 at 05:12:10PM -0700, Arun Shamanna Lakshmi wrote:
> 1. Modify soc_enum struct to handle pointers for reg and mask
> 2. Add dapm get and put APIs for multi register one hot encoded mux
> 3. Update snd_soc_dapm_update struct to support multiple reg update

If you've got several changes like this it's probably a sign that the
change ought to be split into a patch series.

I'm still not seeing any handling of the issues with having invalid
configurations written to the device during the process of carrying out
multi register updates; I did raise this with one of the earlier
versions but don't recall any response.

I also think I agree with Takashi on this one - trying to implement this
without adding an abstraction for the control values is making the code
more complex than it needs to be, all the conditional paths for _ONEHOT
aren't pretty (and don't use switches which is the usual idiom for this
stuff if it's not indirected via functions).

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] ASoC: dapm: Add support for multi register mux

[Takashi Iwai]

At Mon, 07 Apr 2014 14:54:11 +0200,
Lars-Peter Clausen wrote:
> 
> On 04/05/2014 02:12 AM, Arun Shamanna Lakshmi wrote:
> > 1. Modify soc_enum struct to handle pointers for reg and mask
> > 2. Add dapm get and put APIs for multi register one hot encoded mux
> > 3. Update snd_soc_dapm_update struct to support multiple reg update
> >
> > Signed-off-by: Arun S L <aruns@nvidia.com>
> > Signed-off-by: Songhee Baek <sbaek@nvidia.com>
> 
> Looks good to me, so:
> 
> Reviewed-by: Lars-Peter Clausen <lars@metafoo.de>
> 
> 
> As Takashi said it is not that nice that there is a bit of code churn by 
> having to update all the existing users of e->reg and e->mask. But 
> implementing this properly seems to cause even more code churn.

Well, I'm not sure about it.  It's already too late for 3.15, so we
have some time to enhance for this feature properly.

I personally like a hackish solution, too, but only if it really gives
a clear benefit over the generic solution.  In this case, it changes
the data type and yet increases the data size significantly, which
isn't a good sign.  (Note that replacing int with pointer can be twice
on 64bit, and there will be pads for alignment, in addition to the
extra space for number instances by this implementation.)

Also, as the data handling is branched per type, a more readable
implementation would be to impose a union between normal and onehot
types in soc_enum struct, I guess.  But, then a question comes to the
point whether we really want such a unification at all.

Last but not least, another concern is that there can be multiple
onehot types: with 0 or without 0 handling.  It'd be easy to add the
implementation, but it indicates that the implementation isn't generic
enough in comparison with the existing snd_soc code for single
register.

> And I think 
> it will be done best in an effort to consolidate the kcontrol helpers and 
> the DAPM kcontrol helpers by adding an additional layer of abstraction 
> between the kcontrols and the hardware access that translates between the 
> logical value and the physical value(s).
> 
> E.g. something like
> 
> struct kcontrol_ops {
> 	int (*read)(...);
> 	int (*write)(...);
> };
> 
> And then have one kind of ops for each kind of control type and at the high 
> level only have put and get handlers for enums and for switches/volumes.

This might work well, indeed.  But, let the actual code talk :)


thanks,

Takashi

Re: [PATCH] ASoC: dapm: Add support for multi register mux

[Lars-Peter Clausen]

On 04/05/2014 02:12 AM, Arun Shamanna Lakshmi wrote:
> 1. Modify soc_enum struct to handle pointers for reg and mask
> 2. Add dapm get and put APIs for multi register one hot encoded mux
> 3. Update snd_soc_dapm_update struct to support multiple reg update
>
> Signed-off-by: Arun S L <aruns@nvidia.com>
> Signed-off-by: Songhee Baek <sbaek@nvidia.com>

Looks good to me, so:

Reviewed-by: Lars-Peter Clausen <lars@metafoo.de>


As Takashi said it is not that nice that there is a bit of code churn by 
having to update all the existing users of e->reg and e->mask. But 
implementing this properly seems to cause even more code churn. And I think 
it will be done best in an effort to consolidate the kcontrol helpers and 
the DAPM kcontrol helpers by adding an additional layer of abstraction 
between the kcontrols and the hardware access that translates between the 
logical value and the physical value(s).

E.g. something like

struct kcontrol_ops {
	int (*read)(...);
	int (*write)(...);
};

And then have one kind of ops for each kind of control type and at the high 
level only have put and get handlers for enums and for switches/volumes.

- Lars

[PATCH] ASoC: dapm: Add support for multi register mux

[Arun Shamanna Lakshmi]

1. Modify soc_enum struct to handle pointers for reg and mask
2. Add dapm get and put APIs for multi register one hot encoded mux
3. Update snd_soc_dapm_update struct to support multiple reg update

Signed-off-by: Arun S L <aruns@nvidia.com>
Signed-off-by: Songhee Baek <sbaek@nvidia.com>
---
 include/sound/soc-dapm.h |   20 ++++-
 include/sound/soc.h      |   33 +++++++--
 sound/soc/soc-core.c     |   12 +--
 sound/soc/soc-dapm.c     |  181 +++++++++++++++++++++++++++++++++++++++-------
 4 files changed, 203 insertions(+), 43 deletions(-)

diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
index ef78f56..c255349 100644
--- a/include/sound/soc-dapm.h
+++ b/include/sound/soc-dapm.h
@@ -305,6 +305,12 @@ struct device;
  	.get = snd_soc_dapm_get_enum_double, \
  	.put = snd_soc_dapm_put_enum_double, \
   	.private_value = (unsigned long)&xenum }
+#define SOC_DAPM_ENUM_ONEHOT(xname, xenum) \
+{	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
+	.info = snd_soc_info_enum_double, \
+	.get = snd_soc_dapm_get_enum_onehot, \
+	.put = snd_soc_dapm_put_enum_onehot, \
+	.private_value = (unsigned long)&xenum }
 #define SOC_DAPM_ENUM_VIRT(xname, xenum) \
 	SOC_DAPM_ENUM(xname, xenum)
 #define SOC_DAPM_ENUM_EXT(xname, xenum, xget, xput) \
@@ -352,6 +358,9 @@ struct device;
 /* regulator widget flags */
 #define SND_SOC_DAPM_REGULATOR_BYPASS     0x1     /* bypass when disabled */
 
+/* maximum registers to update */
+#define SND_SOC_DAPM_UPDATE_MAX_REG		3
+
 struct snd_soc_dapm_widget;
 enum snd_soc_dapm_type;
 struct snd_soc_dapm_path;
@@ -378,6 +387,10 @@ int snd_soc_dapm_get_enum_double(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_value *ucontrol);
 int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_value *ucontrol);
+int snd_soc_dapm_get_enum_onehot(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol);
+int snd_soc_dapm_put_enum_onehot(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol);
 int snd_soc_dapm_info_pin_switch(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_info *uinfo);
 int snd_soc_dapm_get_pin_switch(struct snd_kcontrol *kcontrol,
@@ -590,9 +603,10 @@ struct snd_soc_dapm_widget {
 
 struct snd_soc_dapm_update {
 	struct snd_kcontrol *kcontrol;
-	int reg;
-	int mask;
-	int val;
+	int reg[SND_SOC_DAPM_UPDATE_MAX_REG];
+	int mask[SND_SOC_DAPM_UPDATE_MAX_REG];
+	int val[SND_SOC_DAPM_UPDATE_MAX_REG];
+	unsigned int num_regs;
 };
 
 /* DAPM context */
diff --git a/include/sound/soc.h b/include/sound/soc.h
index 0b83168..dcdf1cb 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -177,18 +177,24 @@
 		{.reg = xreg, .min = xmin, .max = xmax, \
 		 .platform_max = xmax} }
 #define SOC_ENUM_DOUBLE(xreg, xshift_l, xshift_r, xitems, xtexts) \
-{	.reg = xreg, .shift_l = xshift_l, .shift_r = xshift_r, \
+{	.reg = &(int){(xreg)}, .shift_l = xshift_l, .shift_r = xshift_r, \
 	.items = xitems, .texts = xtexts, \
-	.mask = xitems ? roundup_pow_of_two(xitems) - 1 : 0}
+	.mask = &(unsigned int){(xitems ? roundup_pow_of_two(xitems) - 1 : 0)}, \
+	.num_regs = 1, .type = SND_SOC_ENUM_BINARY }
 #define SOC_ENUM_SINGLE(xreg, xshift, xitems, xtexts) \
 	SOC_ENUM_DOUBLE(xreg, xshift, xshift, xitems, xtexts)
 #define SOC_ENUM_SINGLE_EXT(xitems, xtexts) \
 {	.items = xitems, .texts = xtexts }
 #define SOC_VALUE_ENUM_DOUBLE(xreg, xshift_l, xshift_r, xmask, xitems, xtexts, xvalues) \
-{	.reg = xreg, .shift_l = xshift_l, .shift_r = xshift_r, \
-	.mask = xmask, .items = xitems, .texts = xtexts, .values = xvalues}
+{	.reg = &(int){(xreg)}, .shift_l = xshift_l, .shift_r = xshift_r, \
+	.mask = &(unsigned int){(xmask)}, .items = xitems, .texts = xtexts, \
+	.values = xvalues, .num_regs = 1, .type = SND_SOC_ENUM_BINARY }
 #define SOC_VALUE_ENUM_SINGLE(xreg, xshift, xmask, xnitmes, xtexts, xvalues) \
 	SOC_VALUE_ENUM_DOUBLE(xreg, xshift, xshift, xmask, xnitmes, xtexts, xvalues)
+#define SOC_VALUE_ENUM_ONEHOT(xregs, xmasks, xnum_regs, xitems, xtexts, xvalues) \
+{	.reg = xregs, .mask = xmasks, .num_regs = xnum_regs, \
+	.items = xitems, .texts = xtexts, .values = xvalues, \
+	.type = SND_SOC_ENUM_ONEHOT }
 #define SOC_ENUM_SINGLE_VIRT(xitems, xtexts) \
 	SOC_ENUM_SINGLE(SND_SOC_NOPM, 0, xitems, xtexts)
 #define SOC_ENUM(xname, xenum) \
@@ -293,6 +299,9 @@
 #define SOC_VALUE_ENUM_DOUBLE_DECL(name, xreg, xshift_l, xshift_r, xmask, xtexts, xvalues) \
 	const struct soc_enum name = SOC_VALUE_ENUM_DOUBLE(xreg, xshift_l, xshift_r, xmask, \
 							ARRAY_SIZE(xtexts), xtexts, xvalues)
+#define SOC_VALUE_ENUM_ONEHOT_DECL(name, xregs, xmasks, xnum_regs, xtexts, xvalues) \
+	const struct soc_enum name = SOC_VALUE_ENUM_ONEHOT(xregs, xmasks, xnum_regs, \
+							ARRAY_SIZE(xtexts), xtexts, xvalues)
 #define SOC_VALUE_ENUM_SINGLE_DECL(name, xreg, xshift, xmask, xtexts, xvalues) \
 	SOC_VALUE_ENUM_DOUBLE_DECL(name, xreg, xshift, xshift, xmask, xtexts, xvalues)
 #define SOC_ENUM_SINGLE_VIRT_DECL(name, xtexts) \
@@ -326,6 +335,16 @@ enum snd_soc_bias_level {
 	SND_SOC_BIAS_ON = 3,
 };
 
+/*
+ * enum snd_soc_enum_type - Type of ASoC enum control
+ * @SND_SOC_ENUM_BINARY: soc_enum type for SINGLE, DOUBLE or VIRTUAL mux
+ * @SND_SOC_ENUM_ONEHOT: soc_enum type for ONEHOT encoding mux
+ */
+enum snd_soc_enum_type {
+	SND_SOC_ENUM_BINARY = 0,
+	SND_SOC_ENUM_ONEHOT = 1,
+};
+
 struct device_node;
 struct snd_jack;
 struct snd_soc_card;
@@ -1098,13 +1117,15 @@ struct soc_mreg_control {
 
 /* enumerated kcontrol */
 struct soc_enum {
-	int reg;
+	int *reg;
 	unsigned char shift_l;
 	unsigned char shift_r;
 	unsigned int items;
-	unsigned int mask;
+	unsigned int *mask;
 	const char * const *texts;
 	const unsigned int *values;
+	enum snd_soc_enum_type type;
+	unsigned int num_regs;
 };
 
 /**
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index caebd63..cf29722 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2601,12 +2601,12 @@ int snd_soc_get_enum_double(struct snd_kcontrol *kcontrol,
 	unsigned int val, item;
 	unsigned int reg_val;
 
-	reg_val = snd_soc_read(codec, e->reg);
-	val = (reg_val >> e->shift_l) & e->mask;
+	reg_val = snd_soc_read(codec, e->reg[0]);
+	val = (reg_val >> e->shift_l) & e->mask[0];
 	item = snd_soc_enum_val_to_item(e, val);
 	ucontrol->value.enumerated.item[0] = item;
 	if (e->shift_l != e->shift_r) {
-		val = (reg_val >> e->shift_l) & e->mask;
+		val = (reg_val >> e->shift_l) & e->mask[0];
 		item = snd_soc_enum_val_to_item(e, val);
 		ucontrol->value.enumerated.item[1] = item;
 	}
@@ -2636,15 +2636,15 @@ int snd_soc_put_enum_double(struct snd_kcontrol *kcontrol,
 	if (item[0] >= e->items)
 		return -EINVAL;
 	val = snd_soc_enum_item_to_val(e, item[0]) << e->shift_l;
-	mask = e->mask << e->shift_l;
+	mask = e->mask[0] << e->shift_l;
 	if (e->shift_l != e->shift_r) {
 		if (item[1] >= e->items)
 			return -EINVAL;
 		val |= snd_soc_enum_item_to_val(e, item[1]) << e->shift_r;
-		mask |= e->mask << e->shift_r;
+		mask |= e->mask[0] << e->shift_r;
 	}
 
-	return snd_soc_update_bits_locked(codec, e->reg, mask, val);
+	return snd_soc_update_bits_locked(codec, e->reg[0], mask, val);
 }
 EXPORT_SYMBOL_GPL(snd_soc_put_enum_double);
 
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index c8a780d..be9cd63 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -511,13 +511,26 @@ static int dapm_connect_mux(struct snd_soc_dapm_context *dapm,
 	const struct snd_kcontrol_new *kcontrol)
 {
 	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
-	unsigned int val, item;
+	unsigned int val, item, bit_pos = 0;
 	int i;
 
-	if (e->reg != SND_SOC_NOPM) {
-		soc_widget_read(dest, e->reg, &val);
-		val = (val >> e->shift_l) & e->mask;
-		item = snd_soc_enum_val_to_item(e, val);
+	if (e->reg[0] != SND_SOC_NOPM) {
+		if (e->type == SND_SOC_ENUM_ONEHOT) {
+			for (i = 0; i < e->num_regs; i++) {
+				soc_widget_read(dest, e->reg[i], &val);
+				val = val & e->mask[i];
+				if (val != 0) {
+					bit_pos = ffs(val) +
+						(8 * dest->codec->val_bytes * i);
+					break;
+				}
+			}
+			item = snd_soc_enum_val_to_item(e, bit_pos);
+		} else {
+			soc_widget_read(dest, e->reg[0], &val);
+			val = (val >> e->shift_l) & e->mask[0];
+			item = snd_soc_enum_val_to_item(e, val);
+		}
 	} else {
 		/* since a virtual mux has no backing registers to
 		 * decide which path to connect, it will try to match
@@ -1553,8 +1566,8 @@ static void dapm_widget_update(struct snd_soc_card *card)
 	struct snd_soc_dapm_update *update = card->update;
 	struct snd_soc_dapm_widget_list *wlist;
 	struct snd_soc_dapm_widget *w = NULL;
-	unsigned int wi;
-	int ret;
+	unsigned int wi, i;
+	int ret = 0;
 
 	if (!update || !dapm_kcontrol_is_powered(update->kcontrol))
 		return;
@@ -1575,11 +1588,16 @@ static void dapm_widget_update(struct snd_soc_card *card)
 	if (!w)
 		return;
 
-	ret = soc_widget_update_bits_locked(w, update->reg, update->mask,
-				  update->val);
-	if (ret < 0)
-		dev_err(w->dapm->dev, "ASoC: %s DAPM update failed: %d\n",
-			w->name, ret);
+	/* dapm update for multiple registers */
+	for (i = 0; i < update->num_regs; i++) {
+		ret = soc_widget_update_bits_locked(w, update->reg[i],
+					update->mask[i], update->val[i]);
+		if (ret < 0) {
+			dev_err(w->dapm->dev, "ASoC: %s DAPM update failed: %d\n",
+				w->name, ret);
+			break;
+		}
+	}
 
 	for (wi = 0; wi < wlist->num_widgets; wi++) {
 		w = wlist->widgets[wi];
@@ -2866,10 +2884,10 @@ int snd_soc_dapm_put_volsw(struct snd_kcontrol *kcontrol,
 	if (change) {
 		if (reg != SND_SOC_NOPM) {
 			update.kcontrol = kcontrol;
-			update.reg = reg;
-			update.mask = mask;
-			update.val = val;
-
+			update.reg[0] = reg;
+			update.mask[0] = mask;
+			update.val[0] = val;
+			update.num_regs = 1;
 			card->update = &update;
 		}
 
@@ -2903,15 +2921,15 @@ int snd_soc_dapm_get_enum_double(struct snd_kcontrol *kcontrol,
 	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
 	unsigned int reg_val, val;
 
-	if (e->reg != SND_SOC_NOPM)
-		reg_val = snd_soc_read(codec, e->reg);
+	if (e->reg[0] != SND_SOC_NOPM)
+		reg_val = snd_soc_read(codec, e->reg[0]);
 	else
 		reg_val = dapm_kcontrol_get_value(kcontrol);
 
-	val = (reg_val >> e->shift_l) & e->mask;
+	val = (reg_val >> e->shift_l) & e->mask[0];
 	ucontrol->value.enumerated.item[0] = snd_soc_enum_val_to_item(e, val);
 	if (e->shift_l != e->shift_r) {
-		val = (reg_val >> e->shift_r) & e->mask;
+		val = (reg_val >> e->shift_r) & e->mask[0];
 		val = snd_soc_enum_val_to_item(e, val);
 		ucontrol->value.enumerated.item[1] = val;
 	}
@@ -2945,27 +2963,28 @@ int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol,
 		return -EINVAL;
 
 	val = snd_soc_enum_item_to_val(e, item[0]) << e->shift_l;
-	mask = e->mask << e->shift_l;
+	mask = e->mask[0] << e->shift_l;
 	if (e->shift_l != e->shift_r) {
 		if (item[1] > e->items)
 			return -EINVAL;
 		val |= snd_soc_enum_item_to_val(e, item[1]) << e->shift_l;
-		mask |= e->mask << e->shift_r;
+		mask |= e->mask[0] << e->shift_r;
 	}
 
 	mutex_lock_nested(&card->dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME);
 
-	if (e->reg != SND_SOC_NOPM)
-		change = snd_soc_test_bits(codec, e->reg, mask, val);
+	if (e->reg[0] != SND_SOC_NOPM)
+		change = snd_soc_test_bits(codec, e->reg[0], mask, val);
 	else
 		change = dapm_kcontrol_set_value(kcontrol, val);
 
 	if (change) {
-		if (e->reg != SND_SOC_NOPM) {
+		if (e->reg[0] != SND_SOC_NOPM) {
 			update.kcontrol = kcontrol;
-			update.reg = e->reg;
-			update.mask = mask;
-			update.val = val;
+			update.reg[0] = e->reg[0];
+			update.mask[0] = mask;
+			update.val[0] = val;
+			update.num_regs = 1;
 			card->update = &update;
 		}
 
@@ -2984,6 +3003,112 @@ int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol,
 EXPORT_SYMBOL_GPL(snd_soc_dapm_put_enum_double);
 
 /**
+ * snd_soc_dapm_get_enum_onehot - dapm enumerated onehot mixer get callback
+ * @kcontrol: mixer control
+ * @ucontrol: control element information
+ *
+ * Callback to get the value of a dapm enumerated onehot encoded mixer control
+ *
+ * Returns 0 for success.
+ */
+int snd_soc_dapm_get_enum_onehot(struct snd_kcontrol *kcontrol,
+			struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_codec *codec = snd_soc_dapm_kcontrol_codec(kcontrol);
+	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
+	unsigned int reg_val, val, bit_pos = 0, reg_idx;
+
+	for (reg_idx = 0; reg_idx < e->num_regs; reg_idx++) {
+		reg_val = snd_soc_read(codec, e->reg[reg_idx]);
+		val = reg_val & e->mask[reg_idx];
+		if (val != 0) {
+			bit_pos = ffs(val) + (8 * codec->val_bytes * reg_idx);
+			break;
+		}
+	}
+
+	ucontrol->value.enumerated.item[0] =
+			snd_soc_enum_val_to_item(e, bit_pos);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_soc_dapm_get_enum_onehot);
+
+/**
+ * snd_soc_dapm_put_enum_onehot - dapm enumerated onehot mixer put callback
+ * @kcontrol: mixer control
+ * @ucontrol: control element information
+ *
+ * Callback to put the value of a dapm enumerated onehot encoded mixer control
+ *
+ * Returns 0 for success.
+ */
+int snd_soc_dapm_put_enum_onehot(struct snd_kcontrol *kcontrol,
+			struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_codec *codec = snd_soc_dapm_kcontrol_codec(kcontrol);
+	struct snd_soc_card *card = codec->card;
+	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
+	unsigned int *item = ucontrol->value.enumerated.item;
+	unsigned int change = 0, reg_idx = 0, value, bit_pos;
+	unsigned int i, reg_val = 0, update_idx = 0;
+	struct snd_soc_dapm_update update;
+	int ret = 0;
+
+	if (item[0] >= e->items || e->num_regs > SND_SOC_DAPM_UPDATE_MAX_REG)
+		return -EINVAL;
+
+	value = snd_soc_enum_item_to_val(e, item[0]);
+
+	if (value) {
+		/* get the register index and value to set */
+		reg_idx = (value - 1) / (8 * codec->val_bytes);
+		bit_pos = (value - 1) % (8 * codec->val_bytes);
+		reg_val = BIT(bit_pos);
+	}
+
+	for (i = 0; i < e->num_regs; i++) {
+		if (i == reg_idx) {
+			change |= snd_soc_test_bits(codec, e->reg[i],
+							e->mask[i], reg_val);
+			/* set the selected register */
+			update.reg[e->num_regs - 1] = e->reg[reg_idx];
+			update.mask[e->num_regs - 1] = e->mask[reg_idx];
+			update.val[e->num_regs - 1] = reg_val;
+		} else {
+			/* accumulate the change to update the DAPM path
+			    when none is selected */
+			change |= snd_soc_test_bits(codec, e->reg[i],
+							e->mask[i], 0);
+
+			/* clear the register when not selected */
+			update.reg[update_idx] = e->reg[i];
+			update.mask[update_idx] = e->mask[i];
+			update.val[update_idx++] = 0;
+		}
+	}
+
+	mutex_lock_nested(&card->dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME);
+
+	if (change) {
+		update.kcontrol = kcontrol;
+		update.num_regs = e->num_regs;
+		card->update = &update;
+
+		ret = soc_dapm_mux_update_power(card, kcontrol, item[0], e);
+		card->update = NULL;
+	}
+
+	mutex_unlock(&card->dapm_mutex);
+
+	if (ret > 0)
+		soc_dpcm_runtime_update(card);
+
+	return change;
+}
+EXPORT_SYMBOL_GPL(snd_soc_dapm_put_enum_onehot);
+
+/**
  * snd_soc_dapm_info_pin_switch - Info for a pin switch
  *
  * @kcontrol: mixer control
-- 
1.7.9.5

Re: [PATCH] ASoC: dapm: Add support for multi register mux

[Lars-Peter Clausen]

On 04/04/2014 09:34 AM, Arun Shamanna Lakshmi wrote:
>
>> -----Original Message-----
>> From: Lars-Peter Clausen [mailto:lars@metafoo.de]
>> Sent: Friday, April 04, 2014 12:32 AM
>> To: Arun Shamanna Lakshmi
>> Cc: lgirdwood@gmail.com; broonie@kernel.org;
>> swarren@wwwdotorg.org; perex@perex.cz; tiwai@suse.de; alsa-
>> devel@alsa-project.org; linux-kernel@vger.kernel.org; Songhee Baek
>> Subject: Re: [PATCH] ASoC: dapm: Add support for multi register mux
>>
>> On 04/03/2014 10:11 PM, Arun Shamanna Lakshmi wrote:
>> [...]
>>>> Here as well, default for bit_pos should be 0.
>>>
>>> This means when 'None' of the options are selected, by default, it
>>> enumerates to 0. Since we are using __ffs, BIT(0) of Register-0 also
>>> enumerates to 0. That's the reason why I used just ffs in the first place.
>>> Let me know your opinion. My value table looks like below.
>>>
>>> #define MUX_VALUE(npart, nbit)	(nbit + 32 * npart)
>>> static const int mux_values[] = {
>>> 	0,
>>> 	MUX_VALUE(0, 0),
>>> 	.
>>> 	.
>>> 	.
>>> 	MUX_VALUE(0, 31),
>>> 	/* above inputs are for part0 mux */
>>> 	MUX_VALUE(1, 0),
>>> 	.
>>> 	.
>>> 	.
>>> 	MUX_VALUE(1, 31),
>>> 	/* above inputs are for part1 mux */
>>> 	MUX_VALUE(2, 0),
>>> 	.
>>> 	.
>>> 	.
>>> 	MUX_VALUE(2, 31),
>>> 	/* above inputs are for part2 mux */
>>> };
>>
>> Ok, so having none of the input selected should be a valid user selectable
>> option?
>
> Yes. If 'None' is selected, it goes and clears the register. So, can we have ffs( )
> instead of __ffs( ) ? It would fix this case.

Yes, but you need to make sure to handle it also correctly in the put 
handler, since all of the registers need to be written to 0 in that case.

RE: [PATCH] ASoC: dapm: Add support for multi register mux

[Arun Shamanna Lakshmi]

> -----Original Message-----
> From: Lars-Peter Clausen [mailto:lars@metafoo.de]
> Sent: Friday, April 04, 2014 12:32 AM
> To: Arun Shamanna Lakshmi
> Cc: lgirdwood@gmail.com; broonie@kernel.org;
> swarren@wwwdotorg.org; perex@perex.cz; tiwai@suse.de; alsa-
> devel@alsa-project.org; linux-kernel@vger.kernel.org; Songhee Baek
> Subject: Re: [PATCH] ASoC: dapm: Add support for multi register mux
> 
> On 04/03/2014 10:11 PM, Arun Shamanna Lakshmi wrote:
> [...]
> >> Here as well, default for bit_pos should be 0.
> >
> > This means when 'None' of the options are selected, by default, it
> > enumerates to 0. Since we are using __ffs, BIT(0) of Register-0 also
> > enumerates to 0. That's the reason why I used just ffs in the first place.
> > Let me know your opinion. My value table looks like below.
> >
> > #define MUX_VALUE(npart, nbit)	(nbit + 32 * npart)
> > static const int mux_values[] = {
> > 	0,
> > 	MUX_VALUE(0, 0),
> > 	.
> > 	.
> > 	.
> > 	MUX_VALUE(0, 31),
> > 	/* above inputs are for part0 mux */
> > 	MUX_VALUE(1, 0),
> > 	.
> > 	.
> > 	.
> > 	MUX_VALUE(1, 31),
> > 	/* above inputs are for part1 mux */
> > 	MUX_VALUE(2, 0),
> > 	.
> > 	.
> > 	.
> > 	MUX_VALUE(2, 31),
> > 	/* above inputs are for part2 mux */
> > };
> 
> Ok, so having none of the input selected should be a valid user selectable
> option?

Yes. If 'None' is selected, it goes and clears the register. So, can we have ffs( )
instead of __ffs( ) ? It would fix this case.

- Arun

Re: [PATCH] ASoC: dapm: Add support for multi register mux

[Lars-Peter Clausen]

On 04/03/2014 10:11 PM, Arun Shamanna Lakshmi wrote:
[...]
>> Here as well, default for bit_pos should be 0.
>
> This means when 'None' of the options are selected, by default, it
> enumerates to 0. Since we are using __ffs, BIT(0) of Register-0 also
> enumerates to 0. That's the reason why I used just ffs in the first place.
> Let me know your opinion. My value table looks like below.
>
> #define MUX_VALUE(npart, nbit)	(nbit + 32 * npart)
> static const int mux_values[] = {
> 	0,
> 	MUX_VALUE(0, 0),
> 	.
> 	.
> 	.
> 	MUX_VALUE(0, 31),
> 	/* above inputs are for part0 mux */
> 	MUX_VALUE(1, 0),
> 	.
> 	.
> 	.
> 	MUX_VALUE(1, 31),
> 	/* above inputs are for part1 mux */
> 	MUX_VALUE(2, 0),
> 	.
> 	.
> 	.
> 	MUX_VALUE(2, 31),
> 	/* above inputs are for part2 mux */
> };

Ok, so having none of the input selected should be a valid user selectable 
option?

RE: [PATCH] ASoC: dapm: Add support for multi register mux

[Arun Shamanna Lakshmi]

> -----Original Message-----
> From: Lars-Peter Clausen [mailto:lars@metafoo.de]
> Sent: Thursday, April 03, 2014 1:27 AM
> To: Arun Shamanna Lakshmi
> Cc: lgirdwood@gmail.com; broonie@kernel.org;
swarren@wwwdotorg.org;
> perex@perex.cz; tiwai@suse.de; alsa- devel@alsa-project.org;
> linux-kernel@vger.kernel.org; Songhee Baek
> Subject: Re: [PATCH] ASoC: dapm: Add support for multi register mux
>
> On 04/03/2014 05:11 AM, Arun Shamanna Lakshmi wrote:
>
> This looks essentially good to me. A few minor issues, once those are
> fixed things should be good to go.
>
> [...]
> > @@ -2984,6 +3002,112 @@ int
> snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol,
> >   EXPORT_SYMBOL_GPL(snd_soc_dapm_put_enum_double);
> >
> >   /**
> > + * snd_soc_dapm_get_enum_onehot - dapm enumerated onehot
> mixer get
> > +callback
> > + * @kcontrol: mixer control
> > + * @ucontrol: control element information
> > + *
> > + * Callback to get the value of a dapm enumerated onehot encoded
> > +mixer control
> > + *
> > + * Returns 0 for success.
> > + */
> > +int snd_soc_dapm_get_enum_onehot(struct snd_kcontrol *kcontrol,
> > +			struct snd_ctl_elem_value *ucontrol) {
> > +	struct snd_soc_codec *codec =
> snd_soc_dapm_kcontrol_codec(kcontrol);
> > +	struct soc_enum *e = (struct soc_enum *)kcontrol-
> >private_value;
> > +	unsigned int reg_val, val, bit_pos = -1, reg_idx;
>
> Here as well, default for bit_pos should be 0.

This means when 'None' of the options are selected, by default, it
enumerates to 0. Since we are using __ffs, BIT(0) of Register-0 also
enumerates to 0. That's the reason why I used just ffs in the first place.
Let me know your opinion. My value table looks like below.

#define MUX_VALUE(npart, nbit)	(nbit + 32 * npart)
static const int mux_values[] = {
	0,
	MUX_VALUE(0, 0),
	.
	.
	.
	MUX_VALUE(0, 31),
	/* above inputs are for part0 mux */
	MUX_VALUE(1, 0),
	.
	.
	.
	MUX_VALUE(1, 31),
	/* above inputs are for part1 mux */
	MUX_VALUE(2, 0),
	.
	.
	.
	MUX_VALUE(2, 31),
	/* above inputs are for part2 mux */
};

>
> > +
> > +	for (reg_idx = 0; reg_idx < e->num_regs; reg_idx++) {
> > +		reg_val = snd_soc_read(codec, e->reg[reg_idx]);
> > +		val = reg_val & e->mask[reg_idx];
> > +		if (val != 0) {
> > +			bit_pos = __ffs(val) + (8 * codec->val_bytes *
> reg_idx);
> > +			break;
> > +		}
> > +	}
> > +
> > +	ucontrol->value.enumerated.item[0] =
> > +			snd_soc_enum_val_to_item(e, bit_pos);
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(snd_soc_dapm_get_enum_onehot);
> > +
> > +/**
> > +

Re: [PATCH] ASoC: dapm: Add support for multi register mux

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1383 bytes --]

On Thu, Apr 03, 2014 at 05:06:28PM +0200, Takashi Iwai wrote:
> Lars-Peter Clausen wrote:

> > It would be nice, but it also requires some slight restructuring. The issue 
> > we have right now is that there is  strictly speaking a bit of a layering 
> > violation. The DAPM widgets should not need to know how the kcontrols that 
> > are attached to the widget do their IO. What we essentially do in 
> > dapm_connect_mux() (and also dapm_connect_mixer) is an open-coded version of 
> > the controls get handler. Replacing that by calling the get handler instead 
> > should allow us to use different structs for enums and onehot enums.

Right, that's because DAPM has always just reimplemented everything and
the virtual controls were grafted on the side of the existing register
stuff rather than adding an abstraction layer.

> So, something like below?  It's totally untested, just a proof of
> concept.

Yes, that looks sensible to me but I didn't test it yet either.

> If the performance matters, we can optimize it by checking
> kcontrol->get == snd_soc_dapm_get_enum_double or kcontrol->get ==
> snd_soc_dapm_get_volsw and bypass to the current open-code functions
> instead of the generic get/put callers.

Performance isn't that big a deal, probably avoiding the alloc/frees
by just keeping one around somewhere convenient would be useful too.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] ASoC: dapm: Add support for multi register mux

[Takashi Iwai]

At Thu, 03 Apr 2014 15:31:58 +0200,
Lars-Peter Clausen wrote:
> 
> On 04/03/2014 11:53 AM, Mark Brown wrote:
> > On Thu, Apr 03, 2014 at 11:47:15AM +0200, Takashi Iwai wrote:
> >
> >> I'm a bit late in the game, but I feel a bit uneasy through looking
> >> at the whole changes.  My primary question is, whether do we really
> >> need to share the same struct soc_enum for the onehot type?  What
> >> makes hard to use a struct soc_enum_onehot for them?  You need
> >> different individual get/put for each type.  We may still need to
> >> change soc_dapm_update stuff, but it's different from sharing
> >> soc_enum.
> >
> > Indeed, I had thought this was where the discussion was heading - not
> > looked at this version of the patch yet.
> >
> 
> It would be nice, but it also requires some slight restructuring. The issue 
> we have right now is that there is  strictly speaking a bit of a layering 
> violation. The DAPM widgets should not need to know how the kcontrols that 
> are attached to the widget do their IO. What we essentially do in 
> dapm_connect_mux() (and also dapm_connect_mixer) is an open-coded version of 
> the controls get handler. Replacing that by calling the get handler instead 
> should allow us to use different structs for enums and onehot enums.

So, something like below?  It's totally untested, just a proof of
concept.

If the performance matters, we can optimize it by checking
kcontrol->get == snd_soc_dapm_get_enum_double or kcontrol->get ==
snd_soc_dapm_get_volsw and bypass to the current open-code functions
instead of the generic get/put callers.


thanks,

Takashi

---
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index c8a780d0d057..5947c6e2fcc8 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -508,64 +508,71 @@ out:
 static int dapm_connect_mux(struct snd_soc_dapm_context *dapm,
 	struct snd_soc_dapm_widget *src, struct snd_soc_dapm_widget *dest,
 	struct snd_soc_dapm_path *path, const char *control_name,
-	const struct snd_kcontrol_new *kcontrol)
+	struct snd_kcontrol *kcontrol)
 {
-	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
-	unsigned int val, item;
-	int i;
+	struct snd_ctl_elem_info *uinfo;
+	struct snd_ctl_elem_value *ucontrol;
+	unsigned int i, item, items;
+	int err;
 
-	if (e->reg != SND_SOC_NOPM) {
-		soc_widget_read(dest, e->reg, &val);
-		val = (val >> e->shift_l) & e->mask;
-		item = snd_soc_enum_val_to_item(e, val);
-	} else {
-		/* since a virtual mux has no backing registers to
-		 * decide which path to connect, it will try to match
-		 * with the first enumeration.  This is to ensure
-		 * that the default mux choice (the first) will be
-		 * correctly powered up during initialization.
-		 */
-		item = 0;
+	uinfo = kzalloc(sizeof(*uinfo), GFP_KERNEL);
+	ucontrol = kzalloc(sizeof(*ucontrol), GFP_KERNEL);
+	if (!uinfo || !ucontrol) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	err = kcontrol->info(kcontrol, uinfo);
+	if (err < 0)
+		goto out;
+	if (WARN_ON(uinfo->type != SNDRV_CTL_ELEM_TYPE_ENUMERATED)) {
+		err = -EINVAL;
+		goto out;
 	}
+	items = uinfo->value.enumerated.items;
 
-	for (i = 0; i < e->items; i++) {
-		if (!(strcmp(control_name, e->texts[i]))) {
+	err = kcontrol->get(kcontrol, ucontrol);
+	if (err < 0)
+		goto out;
+	item = ucontrol->value.enumerated.item[0];
+
+	for (i = 0; i < items; i++) {
+		uinfo->value.enumerated.item = i;
+		err = kcontrol->info(kcontrol, uinfo);
+		if (err < 0)
+			goto out;
+		if (!(strcmp(control_name, uinfo->value.enumerated.name))) {
 			list_add(&path->list, &dapm->card->paths);
 			list_add(&path->list_sink, &dest->sources);
 			list_add(&path->list_source, &src->sinks);
-			path->name = (char*)e->texts[i];
+			path->name = control_name;
 			if (i == item)
 				path->connect = 1;
 			else
 				path->connect = 0;
-			return 0;
+			goto out;
 		}
 	}
 
-	return -ENODEV;
+	err = -ENODEV;
+ out:
+	kfree(ucontrol);
+	kfree(uinfo);
+	return err < 0 ? err : 0;
 }
 
 /* set up initial codec paths */
 static void dapm_set_mixer_path_status(struct snd_soc_dapm_widget *w,
 	struct snd_soc_dapm_path *p, int i)
 {
-	struct soc_mixer_control *mc = (struct soc_mixer_control *)
-		w->kcontrol_news[i].private_value;
-	unsigned int reg = mc->reg;
-	unsigned int shift = mc->shift;
-	unsigned int max = mc->max;
-	unsigned int mask = (1 << fls(max)) - 1;
-	unsigned int invert = mc->invert;
-	unsigned int val;
+	struct snd_kcontrol *kcontrol = w->kcontrols[i];
+	struct snd_ctl_elem_value *ucontrol =
+		kzalloc(sizeof(*ucontrol), GFP_KERNEL);
 
-	if (reg != SND_SOC_NOPM) {
-		soc_widget_read(w, reg, &val);
-		val = (val >> shift) & mask;
-		if (invert)
-			val = max - val;
-		p->connect = !!val;
-	} else {
-		p->connect = 0;
+	if (ucontrol) {
+		if (kcontrol->get(kcontrol, ucontrol) >= 0)
+			p->connect = !!ucontrol->value.integer.value[0];
+		kfree(ucontrol);
 	}
 }
 
@@ -2415,7 +2422,7 @@ static int snd_soc_dapm_add_path(struct snd_soc_dapm_context *dapm,
 		return 0;
 	case snd_soc_dapm_mux:
 		ret = dapm_connect_mux(dapm, wsource, wsink, path, control,
-			&wsink->kcontrol_news[0]);
+				       wsink->kcontrols[0]);
 		if (ret != 0)
 			goto err;
 		break;

Re: [PATCH] ASoC: dapm: Add support for multi register mux

[Lars-Peter Clausen]

On 04/03/2014 11:53 AM, Mark Brown wrote:
> On Thu, Apr 03, 2014 at 11:47:15AM +0200, Takashi Iwai wrote:
>
>> I'm a bit late in the game, but I feel a bit uneasy through looking
>> at the whole changes.  My primary question is, whether do we really
>> need to share the same struct soc_enum for the onehot type?  What
>> makes hard to use a struct soc_enum_onehot for them?  You need
>> different individual get/put for each type.  We may still need to
>> change soc_dapm_update stuff, but it's different from sharing
>> soc_enum.
>
> Indeed, I had thought this was where the discussion was heading - not
> looked at this version of the patch yet.
>

It would be nice, but it also requires some slight restructuring. The issue 
we have right now is that there is  strictly speaking a bit of a layering 
violation. The DAPM widgets should not need to know how the kcontrols that 
are attached to the widget do their IO. What we essentially do in 
dapm_connect_mux() (and also dapm_connect_mixer) is an open-coded version of 
the controls get handler. Replacing that by calling the get handler instead 
should allow us to use different structs for enums and onehot enums.

- Lars

Re: [PATCH] ASoC: dapm: Add support for multi register mux

[Mark Brown]

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

On Thu, Apr 03, 2014 at 11:47:15AM +0200, Takashi Iwai wrote:

> I'm a bit late in the game, but I feel a bit uneasy through looking
> at the whole changes.  My primary question is, whether do we really
> need to share the same struct soc_enum for the onehot type?  What
> makes hard to use a struct soc_enum_onehot for them?  You need
> different individual get/put for each type.  We may still need to
> change soc_dapm_update stuff, but it's different from sharing
> soc_enum.

Indeed, I had thought this was where the discussion was heading - not
looked at this version of the patch yet.

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

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

Re: [PATCH] ASoC: dapm: Add support for multi register mux

[Takashi Iwai]

At Wed, 2 Apr 2014 20:11:50 -0700,
Arun Shamanna Lakshmi wrote:
> 
> - Modify soc_enum struct to handle pointers for reg and mask
> - Add dapm get and put APIs for multi register mux with one hot encoding
> - Update snd_soc_dapm_update struct to support multiple reg update
> 
> Signed-off-by: Arun Shamanna Lakshmi <aruns@nvidia.com>
> Signed-off-by: Songhee Baek <sbaek@nvidia.com>

I'm a bit late in the game, but I feel a bit uneasy through looking
at the whole changes.  My primary question is, whether do we really
need to share the same struct soc_enum for the onehot type?  What
makes hard to use a struct soc_enum_onehot for them?  You need
different individual get/put for each type.  We may still need to
change soc_dapm_update stuff, but it's different from sharing
soc_enum.


thanks,

Takashi

> ---
>  include/sound/soc-dapm.h |   17 ++++-
>  include/sound/soc.h      |   34 +++++++--
>  sound/soc/soc-core.c     |   12 ++--
>  sound/soc/soc-dapm.c     |  174 +++++++++++++++++++++++++++++++++++++++-------
>  4 files changed, 197 insertions(+), 40 deletions(-)
> 
> diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
> index ef78f56..ded46732 100644
> --- a/include/sound/soc-dapm.h
> +++ b/include/sound/soc-dapm.h
> @@ -305,6 +305,12 @@ struct device;
>   	.get = snd_soc_dapm_get_enum_double, \
>   	.put = snd_soc_dapm_put_enum_double, \
>    	.private_value = (unsigned long)&xenum }
> +#define SOC_DAPM_ENUM_ONEHOT(xname, xenum) \
> +{	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
> +	.info = snd_soc_info_enum_double, \
> +	.get = snd_soc_dapm_get_enum_onehot, \
> +	.put = snd_soc_dapm_put_enum_onehot, \
> +	.private_value = (unsigned long)&xenum }
>  #define SOC_DAPM_ENUM_VIRT(xname, xenum) \
>  	SOC_DAPM_ENUM(xname, xenum)
>  #define SOC_DAPM_ENUM_EXT(xname, xenum, xget, xput) \
> @@ -378,6 +384,10 @@ int snd_soc_dapm_get_enum_double(struct snd_kcontrol *kcontrol,
>  	struct snd_ctl_elem_value *ucontrol);
>  int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol,
>  	struct snd_ctl_elem_value *ucontrol);
> +int snd_soc_dapm_get_enum_onehot(struct snd_kcontrol *kcontrol,
> +	struct snd_ctl_elem_value *ucontrol);
> +int snd_soc_dapm_put_enum_onehot(struct snd_kcontrol *kcontrol,
> +	struct snd_ctl_elem_value *ucontrol);
>  int snd_soc_dapm_info_pin_switch(struct snd_kcontrol *kcontrol,
>  	struct snd_ctl_elem_info *uinfo);
>  int snd_soc_dapm_get_pin_switch(struct snd_kcontrol *kcontrol,
> @@ -590,9 +600,10 @@ struct snd_soc_dapm_widget {
>  
>  struct snd_soc_dapm_update {
>  	struct snd_kcontrol *kcontrol;
> -	int reg;
> -	int mask;
> -	int val;
> +	int reg[3];
> +	int mask[3];
> +	int val[3];
> +	int num_regs;
>  };
>  
>  /* DAPM context */
> diff --git a/include/sound/soc.h b/include/sound/soc.h
> index 0b83168..add326a 100644
> --- a/include/sound/soc.h
> +++ b/include/sound/soc.h
> @@ -177,18 +177,24 @@
>  		{.reg = xreg, .min = xmin, .max = xmax, \
>  		 .platform_max = xmax} }
>  #define SOC_ENUM_DOUBLE(xreg, xshift_l, xshift_r, xitems, xtexts) \
> -{	.reg = xreg, .shift_l = xshift_l, .shift_r = xshift_r, \
> +{	.reg = &(int){(xreg)}, .shift_l = xshift_l, .shift_r = xshift_r, \
>  	.items = xitems, .texts = xtexts, \
> -	.mask = xitems ? roundup_pow_of_two(xitems) - 1 : 0}
> +	.mask = &(unsigned int){(xitems ? roundup_pow_of_two(xitems) - 1 : 0)}, \
> +	.num_regs = 1, .type = SND_SOC_ENUM_NONE }
>  #define SOC_ENUM_SINGLE(xreg, xshift, xitems, xtexts) \
>  	SOC_ENUM_DOUBLE(xreg, xshift, xshift, xitems, xtexts)
>  #define SOC_ENUM_SINGLE_EXT(xitems, xtexts) \
>  {	.items = xitems, .texts = xtexts }
>  #define SOC_VALUE_ENUM_DOUBLE(xreg, xshift_l, xshift_r, xmask, xitems, xtexts, xvalues) \
> -{	.reg = xreg, .shift_l = xshift_l, .shift_r = xshift_r, \
> -	.mask = xmask, .items = xitems, .texts = xtexts, .values = xvalues}
> +{	.reg = &(int){(xreg)}, .shift_l = xshift_l, .shift_r = xshift_r, \
> +	.mask = &(unsigned int){(xmask)}, .items = xitems, .texts = xtexts, \
> +	.values = xvalues, .num_regs = 1, .type = SND_SOC_ENUM_NONE }
>  #define SOC_VALUE_ENUM_SINGLE(xreg, xshift, xmask, xnitmes, xtexts, xvalues) \
>  	SOC_VALUE_ENUM_DOUBLE(xreg, xshift, xshift, xmask, xnitmes, xtexts, xvalues)
> +#define SOC_VALUE_ENUM_ONEHOT(xregs, xmasks, xnum_regs, xitems, xtexts, xvalues) \
> +{	.reg = xregs, .mask = xmasks, .num_regs = xnum_regs, \
> +	.items = xitems, .texts = xtexts, .values = xvalues, \
> +	.type = SND_SOC_ENUM_ONEHOT }
>  #define SOC_ENUM_SINGLE_VIRT(xitems, xtexts) \
>  	SOC_ENUM_SINGLE(SND_SOC_NOPM, 0, xitems, xtexts)
>  #define SOC_ENUM(xname, xenum) \
> @@ -293,6 +299,9 @@
>  #define SOC_VALUE_ENUM_DOUBLE_DECL(name, xreg, xshift_l, xshift_r, xmask, xtexts, xvalues) \
>  	const struct soc_enum name = SOC_VALUE_ENUM_DOUBLE(xreg, xshift_l, xshift_r, xmask, \
>  							ARRAY_SIZE(xtexts), xtexts, xvalues)
> +#define SOC_VALUE_ENUM_ONEHOT_DECL(name, xregs, xmasks, xnum_regs, xtexts, xvalues) \
> +	const struct soc_enum name = SOC_VALUE_ENUM_ONEHOT(xregs, xmasks, xnum_regs, \
> +							ARRAY_SIZE(xtexts), xtexts, xvalues)
>  #define SOC_VALUE_ENUM_SINGLE_DECL(name, xreg, xshift, xmask, xtexts, xvalues) \
>  	SOC_VALUE_ENUM_DOUBLE_DECL(name, xreg, xshift, xshift, xmask, xtexts, xvalues)
>  #define SOC_ENUM_SINGLE_VIRT_DECL(name, xtexts) \
> @@ -326,6 +335,17 @@ enum snd_soc_bias_level {
>  	SND_SOC_BIAS_ON = 3,
>  };
>  
> +/*
> + * Soc Enum Type
> + *
> + * @NONE:    soc_enum type for SINGLE, DOUBLE or VIRTUAL mux
> + * @ONEHOT:  soc_enum type for one hot encoding mux
> + */
> +enum snd_soc_enum_type {
> +	SND_SOC_ENUM_NONE = 0,
> +	SND_SOC_ENUM_ONEHOT = 1,
> +};
> +
>  struct device_node;
>  struct snd_jack;
>  struct snd_soc_card;
> @@ -1098,13 +1118,15 @@ struct soc_mreg_control {
>  
>  /* enumerated kcontrol */
>  struct soc_enum {
> -	int reg;
> +	int *reg;
>  	unsigned char shift_l;
>  	unsigned char shift_r;
>  	unsigned int items;
> -	unsigned int mask;
> +	unsigned int *mask;
>  	const char * const *texts;
>  	const unsigned int *values;
> +	enum snd_soc_enum_type type;
> +	unsigned int num_regs;
>  };
>  
>  /**
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index caebd63..cf29722 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -2601,12 +2601,12 @@ int snd_soc_get_enum_double(struct snd_kcontrol *kcontrol,
>  	unsigned int val, item;
>  	unsigned int reg_val;
>  
> -	reg_val = snd_soc_read(codec, e->reg);
> -	val = (reg_val >> e->shift_l) & e->mask;
> +	reg_val = snd_soc_read(codec, e->reg[0]);
> +	val = (reg_val >> e->shift_l) & e->mask[0];
>  	item = snd_soc_enum_val_to_item(e, val);
>  	ucontrol->value.enumerated.item[0] = item;
>  	if (e->shift_l != e->shift_r) {
> -		val = (reg_val >> e->shift_l) & e->mask;
> +		val = (reg_val >> e->shift_l) & e->mask[0];
>  		item = snd_soc_enum_val_to_item(e, val);
>  		ucontrol->value.enumerated.item[1] = item;
>  	}
> @@ -2636,15 +2636,15 @@ int snd_soc_put_enum_double(struct snd_kcontrol *kcontrol,
>  	if (item[0] >= e->items)
>  		return -EINVAL;
>  	val = snd_soc_enum_item_to_val(e, item[0]) << e->shift_l;
> -	mask = e->mask << e->shift_l;
> +	mask = e->mask[0] << e->shift_l;
>  	if (e->shift_l != e->shift_r) {
>  		if (item[1] >= e->items)
>  			return -EINVAL;
>  		val |= snd_soc_enum_item_to_val(e, item[1]) << e->shift_r;
> -		mask |= e->mask << e->shift_r;
> +		mask |= e->mask[0] << e->shift_r;
>  	}
>  
> -	return snd_soc_update_bits_locked(codec, e->reg, mask, val);
> +	return snd_soc_update_bits_locked(codec, e->reg[0], mask, val);
>  }
>  EXPORT_SYMBOL_GPL(snd_soc_put_enum_double);
>  
> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
> index c8a780d..19b004a 100644
> --- a/sound/soc/soc-dapm.c
> +++ b/sound/soc/soc-dapm.c
> @@ -511,13 +511,26 @@ static int dapm_connect_mux(struct snd_soc_dapm_context *dapm,
>  	const struct snd_kcontrol_new *kcontrol)
>  {
>  	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
> -	unsigned int val, item;
> +	unsigned int val, item, bit_pos = -1;
>  	int i;
>  
> -	if (e->reg != SND_SOC_NOPM) {
> -		soc_widget_read(dest, e->reg, &val);
> -		val = (val >> e->shift_l) & e->mask;
> -		item = snd_soc_enum_val_to_item(e, val);
> +	if (e->reg[0] != SND_SOC_NOPM) {
> +		if (e->type == SND_SOC_ENUM_ONEHOT) {
> +			for (i = 0; i < e->num_regs; i++) {
> +				soc_widget_read(dest, e->reg[i], &val);
> +				val = val & e->mask[i];
> +				if (val != 0) {
> +					bit_pos = __ffs(val) +
> +						(8 * dest->codec->val_bytes * i);
> +					break;
> +				}
> +			}
> +			item = snd_soc_enum_val_to_item(e, bit_pos);
> +		} else {
> +			soc_widget_read(dest, e->reg[0], &val);
> +			val = (val >> e->shift_l) & e->mask[0];
> +			item = snd_soc_enum_val_to_item(e, val);
> +		}
>  	} else {
>  		/* since a virtual mux has no backing registers to
>  		 * decide which path to connect, it will try to match
> @@ -1553,8 +1566,8 @@ static void dapm_widget_update(struct snd_soc_card *card)
>  	struct snd_soc_dapm_update *update = card->update;
>  	struct snd_soc_dapm_widget_list *wlist;
>  	struct snd_soc_dapm_widget *w = NULL;
> -	unsigned int wi;
> -	int ret;
> +	unsigned int wi, i;
> +	int ret = 0;
>  
>  	if (!update || !dapm_kcontrol_is_powered(update->kcontrol))
>  		return;
> @@ -1575,8 +1588,12 @@ static void dapm_widget_update(struct snd_soc_card *card)
>  	if (!w)
>  		return;
>  
> -	ret = soc_widget_update_bits_locked(w, update->reg, update->mask,
> -				  update->val);
> +	/* dapm update for multiple registers */
> +	for (i = 0; i < update->num_regs; i++) {
> +		ret |= soc_widget_update_bits_locked(w, update->reg[i],
> +					update->mask[i], update->val[i]);
> +	}
> +
>  	if (ret < 0)
>  		dev_err(w->dapm->dev, "ASoC: %s DAPM update failed: %d\n",
>  			w->name, ret);
> @@ -2866,10 +2883,10 @@ int snd_soc_dapm_put_volsw(struct snd_kcontrol *kcontrol,
>  	if (change) {
>  		if (reg != SND_SOC_NOPM) {
>  			update.kcontrol = kcontrol;
> -			update.reg = reg;
> -			update.mask = mask;
> -			update.val = val;
> -
> +			update.reg[0] = reg;
> +			update.mask[0] = mask;
> +			update.val[0] = val;
> +			update.num_regs = 1;
>  			card->update = &update;
>  		}
>  
> @@ -2903,15 +2920,15 @@ int snd_soc_dapm_get_enum_double(struct snd_kcontrol *kcontrol,
>  	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
>  	unsigned int reg_val, val;
>  
> -	if (e->reg != SND_SOC_NOPM)
> -		reg_val = snd_soc_read(codec, e->reg);
> +	if (e->reg[0] != SND_SOC_NOPM)
> +		reg_val = snd_soc_read(codec, e->reg[0]);
>  	else
>  		reg_val = dapm_kcontrol_get_value(kcontrol);
>  
> -	val = (reg_val >> e->shift_l) & e->mask;
> +	val = (reg_val >> e->shift_l) & e->mask[0];
>  	ucontrol->value.enumerated.item[0] = snd_soc_enum_val_to_item(e, val);
>  	if (e->shift_l != e->shift_r) {
> -		val = (reg_val >> e->shift_r) & e->mask;
> +		val = (reg_val >> e->shift_r) & e->mask[0];
>  		val = snd_soc_enum_val_to_item(e, val);
>  		ucontrol->value.enumerated.item[1] = val;
>  	}
> @@ -2945,27 +2962,28 @@ int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol,
>  		return -EINVAL;
>  
>  	val = snd_soc_enum_item_to_val(e, item[0]) << e->shift_l;
> -	mask = e->mask << e->shift_l;
> +	mask = e->mask[0] << e->shift_l;
>  	if (e->shift_l != e->shift_r) {
>  		if (item[1] > e->items)
>  			return -EINVAL;
>  		val |= snd_soc_enum_item_to_val(e, item[1]) << e->shift_l;
> -		mask |= e->mask << e->shift_r;
> +		mask |= e->mask[0] << e->shift_r;
>  	}
>  
>  	mutex_lock_nested(&card->dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME);
>  
> -	if (e->reg != SND_SOC_NOPM)
> -		change = snd_soc_test_bits(codec, e->reg, mask, val);
> +	if (e->reg[0] != SND_SOC_NOPM)
> +		change = snd_soc_test_bits(codec, e->reg[0], mask, val);
>  	else
>  		change = dapm_kcontrol_set_value(kcontrol, val);
>  
>  	if (change) {
> -		if (e->reg != SND_SOC_NOPM) {
> +		if (e->reg[0] != SND_SOC_NOPM) {
>  			update.kcontrol = kcontrol;
> -			update.reg = e->reg;
> -			update.mask = mask;
> -			update.val = val;
> +			update.reg[0] = e->reg[0];
> +			update.mask[0] = mask;
> +			update.val[0] = val;
> +			update.num_regs = 1;
>  			card->update = &update;
>  		}
>  
> @@ -2984,6 +3002,112 @@ int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol,
>  EXPORT_SYMBOL_GPL(snd_soc_dapm_put_enum_double);
>  
>  /**
> + * snd_soc_dapm_get_enum_onehot - dapm enumerated onehot mixer get callback
> + * @kcontrol: mixer control
> + * @ucontrol: control element information
> + *
> + * Callback to get the value of a dapm enumerated onehot encoded mixer control
> + *
> + * Returns 0 for success.
> + */
> +int snd_soc_dapm_get_enum_onehot(struct snd_kcontrol *kcontrol,
> +			struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_codec *codec = snd_soc_dapm_kcontrol_codec(kcontrol);
> +	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
> +	unsigned int reg_val, val, bit_pos = -1, reg_idx;
> +
> +	for (reg_idx = 0; reg_idx < e->num_regs; reg_idx++) {
> +		reg_val = snd_soc_read(codec, e->reg[reg_idx]);
> +		val = reg_val & e->mask[reg_idx];
> +		if (val != 0) {
> +			bit_pos = __ffs(val) + (8 * codec->val_bytes * reg_idx);
> +			break;
> +		}
> +	}
> +
> +	ucontrol->value.enumerated.item[0] =
> +			snd_soc_enum_val_to_item(e, bit_pos);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(snd_soc_dapm_get_enum_onehot);
> +
> +/**
> + * snd_soc_dapm_put_enum_onehot - dapm enumerated onehot mixer put callback
> + * @kcontrol: mixer control
> + * @ucontrol: control element information
> + *
> + * Callback to put the value of a dapm enumerated onehot encoded mixer control
> + *
> + * Returns 0 for success.
> + */
> +int snd_soc_dapm_put_enum_onehot(struct snd_kcontrol *kcontrol,
> +			struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_codec *codec = snd_soc_dapm_kcontrol_codec(kcontrol);
> +	struct snd_soc_card *card = codec->card;
> +	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
> +	unsigned int *item = ucontrol->value.enumerated.item;
> +	unsigned int change = 0, reg_idx = 0, value, bit_pos;
> +	struct snd_soc_dapm_update update;
> +	int ret = 0, reg_val = 0, i, update_idx = 0;
> +
> +	if (item[0] >= e->items)
> +		return -EINVAL;
> +
> +	value = snd_soc_enum_item_to_val(e, item[0]);
> +
> +	if (value >= 0) {
> +		/* get the register index and value to set */
> +		reg_idx = value / (8 * codec->val_bytes);
> +		bit_pos = value % (8 * codec->val_bytes);
> +		reg_val = BIT(bit_pos);
> +	}
> +
> +	for (i = 0; i < e->num_regs; i++) {
> +		if (i == reg_idx) {
> +			change = snd_soc_test_bits(codec, e->reg[i],
> +							e->mask[i], reg_val);
> +			/* set the selected register */
> +			update.reg[e->num_regs - 1] = e->reg[reg_idx];
> +			update.mask[e->num_regs - 1] = e->mask[reg_idx];
> +			update.val[e->num_regs - 1] = reg_val;
> +		} else {
> +			/* accumulate the change to update the DAPM path
> +			    when none is selected */
> +			change |= snd_soc_test_bits(codec, e->reg[i],
> +							e->mask[i], 0);
> +
> +			/* clear the register when not selected */
> +			update.reg[update_idx] = e->reg[i];
> +			update.mask[update_idx] = e->mask[i];
> +			update.val[update_idx++] = 0;
> +		}
> +	}
> +
> +	mutex_lock_nested(&card->dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME);
> +
> +	if (change) {
> +		update.kcontrol = kcontrol;
> +		update.num_regs = 3;
> +		card->update = &update;
> +
> +		ret = soc_dapm_mux_update_power(card, kcontrol, item[0], e);
> +
> +		card->update = NULL;
> +	}
> +
> +	mutex_unlock(&card->dapm_mutex);
> +
> +	if (ret > 0)
> +		soc_dpcm_runtime_update(card);
> +
> +	return change;
> +}
> +EXPORT_SYMBOL_GPL(snd_soc_dapm_put_enum_onehot);
> +
> +/**
>   * snd_soc_dapm_info_pin_switch - Info for a pin switch
>   *
>   * @kcontrol: mixer control
> -- 
> 1.7.9.5
> 

Re: [PATCH] ASoC: dapm: Add support for multi register mux

[Mark Brown]

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

On Thu, Apr 03, 2014 at 10:27:17AM +0200, Lars-Peter Clausen wrote:
> On 04/03/2014 05:11 AM, Arun Shamanna Lakshmi wrote:

> >+enum snd_soc_enum_type {
> >+	SND_SOC_ENUM_NONE = 0,

> I'm not sure if NONE is the right term. Maybe BINARY is better.

Yes, it's definitely not none.

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

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

Re: [PATCH] ASoC: dapm: Add support for multi register mux

[Lars-Peter Clausen]

On 04/03/2014 05:11 AM, Arun Shamanna Lakshmi wrote:

This looks essentially good to me. A few minor issues, once those are fixed 
things should be good to go.

[...]
>   struct snd_soc_dapm_update {
>   	struct snd_kcontrol *kcontrol;
> -	int reg;
> -	int mask;
> -	int val;
> +	int reg[3];
> +	int mask[3];
> +	int val[3];

Make the 3 a define and check against it in the put handler.

> +	int num_regs;

unsigned int

>   };
[...]
> +/*
> + * Soc Enum Type
> + *
> + * @NONE:    soc_enum type for SINGLE, DOUBLE or VIRTUAL mux
> + * @ONEHOT:  soc_enum type for one hot encoding mux
> + */


This should be kernel doc style so

/**
  * enum snd_soc_enum_type - Type of the ASoC enum control
  * @SND_SOC_ENUM_NONE: ...
  * ...
  */


> +enum snd_soc_enum_type {
> +	SND_SOC_ENUM_NONE = 0,

I'm not sure if NONE is the right term. Maybe BINARY is better.

> +	SND_SOC_ENUM_ONEHOT = 1,
> +};
> +
[...]
> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
> index c8a780d..19b004a 100644
> --- a/sound/soc/soc-dapm.c
> +++ b/sound/soc/soc-dapm.c
> @@ -511,13 +511,26 @@ static int dapm_connect_mux(struct snd_soc_dapm_context *dapm,
>   	const struct snd_kcontrol_new *kcontrol)
>   {
>   	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
> -	unsigned int val, item;
> +	unsigned int val, item, bit_pos = -1;

default for bit_pos should probably be 0.

[...]
> @@ -1575,8 +1588,12 @@ static void dapm_widget_update(struct snd_soc_card *card)
>   	if (!w)
>   		return;
>
> -	ret = soc_widget_update_bits_locked(w, update->reg, update->mask,
> -				  update->val);
> +	/* dapm update for multiple registers */
> +	for (i = 0; i < update->num_regs; i++) {
> +		ret |= soc_widget_update_bits_locked(w, update->reg[i],
> +					update->mask[i], update->val[i]);

I'd prefer
	ret = soc_widget_update_bits_locked(...);
	if (ret < 0)
		break;

> +	}
> +
>   	if (ret < 0)
>   		dev_err(w->dapm->dev, "ASoC: %s DAPM update failed: %d\n",
>   			w->name, ret);
[...]
> @@ -2984,6 +3002,112 @@ int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol,
>   EXPORT_SYMBOL_GPL(snd_soc_dapm_put_enum_double);
>
>   /**
> + * snd_soc_dapm_get_enum_onehot - dapm enumerated onehot mixer get callback
> + * @kcontrol: mixer control
> + * @ucontrol: control element information
> + *
> + * Callback to get the value of a dapm enumerated onehot encoded mixer control
> + *
> + * Returns 0 for success.
> + */
> +int snd_soc_dapm_get_enum_onehot(struct snd_kcontrol *kcontrol,
> +			struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_codec *codec = snd_soc_dapm_kcontrol_codec(kcontrol);
> +	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
> +	unsigned int reg_val, val, bit_pos = -1, reg_idx;

Here as well, default for bit_pos should be 0.

> +
> +	for (reg_idx = 0; reg_idx < e->num_regs; reg_idx++) {
> +		reg_val = snd_soc_read(codec, e->reg[reg_idx]);
> +		val = reg_val & e->mask[reg_idx];
> +		if (val != 0) {
> +			bit_pos = __ffs(val) + (8 * codec->val_bytes * reg_idx);
> +			break;
> +		}
> +	}
> +
> +	ucontrol->value.enumerated.item[0] =
> +			snd_soc_enum_val_to_item(e, bit_pos);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(snd_soc_dapm_get_enum_onehot);
> +
> +/**
> + * snd_soc_dapm_put_enum_onehot - dapm enumerated onehot mixer put callback
> + * @kcontrol: mixer control
> + * @ucontrol: control element information
> + *
> + * Callback to put the value of a dapm enumerated onehot encoded mixer control
> + *
> + * Returns 0 for success.
> + */
> +int snd_soc_dapm_put_enum_onehot(struct snd_kcontrol *kcontrol,
> +			struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_codec *codec = snd_soc_dapm_kcontrol_codec(kcontrol);
> +	struct snd_soc_card *card = codec->card;
> +	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
> +	unsigned int *item = ucontrol->value.enumerated.item;
> +	unsigned int change = 0, reg_idx = 0, value, bit_pos;
> +	struct snd_soc_dapm_update update;
> +	int ret = 0, reg_val = 0, i, update_idx = 0;
> +
> +	if (item[0] >= e->items)
> +		return -EINVAL;
> +
> +	value = snd_soc_enum_item_to_val(e, item[0]);
> +
> +	if (value >= 0) {

value is unsigned int, so this is never false.

> +		/* get the register index and value to set */
> +		reg_idx = value / (8 * codec->val_bytes);
> +		bit_pos = value % (8 * codec->val_bytes);
> +		reg_val = BIT(bit_pos);
> +	}
> +
> +	for (i = 0; i < e->num_regs; i++) {
> +		if (i == reg_idx) {
> +			change = snd_soc_test_bits(codec, e->reg[i],
> +							e->mask[i], reg_val);

change |=

> +			/* set the selected register */
> +			update.reg[e->num_regs - 1] = e->reg[reg_idx];
> +			update.mask[e->num_regs - 1] = e->mask[reg_idx];
> +			update.val[e->num_regs - 1] = reg_val;
> +		} else {
> +			/* accumulate the change to update the DAPM path
> +			    when none is selected */
> +			change |= snd_soc_test_bits(codec, e->reg[i],
> +							e->mask[i], 0);
> +
> +			/* clear the register when not selected */
> +			update.reg[update_idx] = e->reg[i];
> +			update.mask[update_idx] = e->mask[i];
> +			update.val[update_idx++] = 0;
> +		}
> +	}
> +
> +	mutex_lock_nested(&card->dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME);
> +
> +	if (change) {
> +		update.kcontrol = kcontrol;
> +		update.num_regs = 3;

Should be e->num_regs;

> +		card->update = &update;
> +
> +		ret = soc_dapm_mux_update_power(card, kcontrol, item[0], e);
> +
> +		card->update = NULL;
> +	}
> +

[PATCH] ASoC: dapm: Add support for multi register mux

[Arun Shamanna Lakshmi]

- Modify soc_enum struct to handle pointers for reg and mask
- Add dapm get and put APIs for multi register mux with one hot encoding
- Update snd_soc_dapm_update struct to support multiple reg update

Signed-off-by: Arun Shamanna Lakshmi <aruns@nvidia.com>
Signed-off-by: Songhee Baek <sbaek@nvidia.com>
---
 include/sound/soc-dapm.h |   17 ++++-
 include/sound/soc.h      |   34 +++++++--
 sound/soc/soc-core.c     |   12 ++--
 sound/soc/soc-dapm.c     |  174 +++++++++++++++++++++++++++++++++++++++-------
 4 files changed, 197 insertions(+), 40 deletions(-)

diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
index ef78f56..ded46732 100644
--- a/include/sound/soc-dapm.h
+++ b/include/sound/soc-dapm.h
@@ -305,6 +305,12 @@ struct device;
  	.get = snd_soc_dapm_get_enum_double, \
  	.put = snd_soc_dapm_put_enum_double, \
   	.private_value = (unsigned long)&xenum }
+#define SOC_DAPM_ENUM_ONEHOT(xname, xenum) \
+{	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
+	.info = snd_soc_info_enum_double, \
+	.get = snd_soc_dapm_get_enum_onehot, \
+	.put = snd_soc_dapm_put_enum_onehot, \
+	.private_value = (unsigned long)&xenum }
 #define SOC_DAPM_ENUM_VIRT(xname, xenum) \
 	SOC_DAPM_ENUM(xname, xenum)
 #define SOC_DAPM_ENUM_EXT(xname, xenum, xget, xput) \
@@ -378,6 +384,10 @@ int snd_soc_dapm_get_enum_double(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_value *ucontrol);
 int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_value *ucontrol);
+int snd_soc_dapm_get_enum_onehot(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol);
+int snd_soc_dapm_put_enum_onehot(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol);
 int snd_soc_dapm_info_pin_switch(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_info *uinfo);
 int snd_soc_dapm_get_pin_switch(struct snd_kcontrol *kcontrol,
@@ -590,9 +600,10 @@ struct snd_soc_dapm_widget {
 
 struct snd_soc_dapm_update {
 	struct snd_kcontrol *kcontrol;
-	int reg;
-	int mask;
-	int val;
+	int reg[3];
+	int mask[3];
+	int val[3];
+	int num_regs;
 };
 
 /* DAPM context */
diff --git a/include/sound/soc.h b/include/sound/soc.h
index 0b83168..add326a 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -177,18 +177,24 @@
 		{.reg = xreg, .min = xmin, .max = xmax, \
 		 .platform_max = xmax} }
 #define SOC_ENUM_DOUBLE(xreg, xshift_l, xshift_r, xitems, xtexts) \
-{	.reg = xreg, .shift_l = xshift_l, .shift_r = xshift_r, \
+{	.reg = &(int){(xreg)}, .shift_l = xshift_l, .shift_r = xshift_r, \
 	.items = xitems, .texts = xtexts, \
-	.mask = xitems ? roundup_pow_of_two(xitems) - 1 : 0}
+	.mask = &(unsigned int){(xitems ? roundup_pow_of_two(xitems) - 1 : 0)}, \
+	.num_regs = 1, .type = SND_SOC_ENUM_NONE }
 #define SOC_ENUM_SINGLE(xreg, xshift, xitems, xtexts) \
 	SOC_ENUM_DOUBLE(xreg, xshift, xshift, xitems, xtexts)
 #define SOC_ENUM_SINGLE_EXT(xitems, xtexts) \
 {	.items = xitems, .texts = xtexts }
 #define SOC_VALUE_ENUM_DOUBLE(xreg, xshift_l, xshift_r, xmask, xitems, xtexts, xvalues) \
-{	.reg = xreg, .shift_l = xshift_l, .shift_r = xshift_r, \
-	.mask = xmask, .items = xitems, .texts = xtexts, .values = xvalues}
+{	.reg = &(int){(xreg)}, .shift_l = xshift_l, .shift_r = xshift_r, \
+	.mask = &(unsigned int){(xmask)}, .items = xitems, .texts = xtexts, \
+	.values = xvalues, .num_regs = 1, .type = SND_SOC_ENUM_NONE }
 #define SOC_VALUE_ENUM_SINGLE(xreg, xshift, xmask, xnitmes, xtexts, xvalues) \
 	SOC_VALUE_ENUM_DOUBLE(xreg, xshift, xshift, xmask, xnitmes, xtexts, xvalues)
+#define SOC_VALUE_ENUM_ONEHOT(xregs, xmasks, xnum_regs, xitems, xtexts, xvalues) \
+{	.reg = xregs, .mask = xmasks, .num_regs = xnum_regs, \
+	.items = xitems, .texts = xtexts, .values = xvalues, \
+	.type = SND_SOC_ENUM_ONEHOT }
 #define SOC_ENUM_SINGLE_VIRT(xitems, xtexts) \
 	SOC_ENUM_SINGLE(SND_SOC_NOPM, 0, xitems, xtexts)
 #define SOC_ENUM(xname, xenum) \
@@ -293,6 +299,9 @@
 #define SOC_VALUE_ENUM_DOUBLE_DECL(name, xreg, xshift_l, xshift_r, xmask, xtexts, xvalues) \
 	const struct soc_enum name = SOC_VALUE_ENUM_DOUBLE(xreg, xshift_l, xshift_r, xmask, \
 							ARRAY_SIZE(xtexts), xtexts, xvalues)
+#define SOC_VALUE_ENUM_ONEHOT_DECL(name, xregs, xmasks, xnum_regs, xtexts, xvalues) \
+	const struct soc_enum name = SOC_VALUE_ENUM_ONEHOT(xregs, xmasks, xnum_regs, \
+							ARRAY_SIZE(xtexts), xtexts, xvalues)
 #define SOC_VALUE_ENUM_SINGLE_DECL(name, xreg, xshift, xmask, xtexts, xvalues) \
 	SOC_VALUE_ENUM_DOUBLE_DECL(name, xreg, xshift, xshift, xmask, xtexts, xvalues)
 #define SOC_ENUM_SINGLE_VIRT_DECL(name, xtexts) \
@@ -326,6 +335,17 @@ enum snd_soc_bias_level {
 	SND_SOC_BIAS_ON = 3,
 };
 
+/*
+ * Soc Enum Type
+ *
+ * @NONE:    soc_enum type for SINGLE, DOUBLE or VIRTUAL mux
+ * @ONEHOT:  soc_enum type for one hot encoding mux
+ */
+enum snd_soc_enum_type {
+	SND_SOC_ENUM_NONE = 0,
+	SND_SOC_ENUM_ONEHOT = 1,
+};
+
 struct device_node;
 struct snd_jack;
 struct snd_soc_card;
@@ -1098,13 +1118,15 @@ struct soc_mreg_control {
 
 /* enumerated kcontrol */
 struct soc_enum {
-	int reg;
+	int *reg;
 	unsigned char shift_l;
 	unsigned char shift_r;
 	unsigned int items;
-	unsigned int mask;
+	unsigned int *mask;
 	const char * const *texts;
 	const unsigned int *values;
+	enum snd_soc_enum_type type;
+	unsigned int num_regs;
 };
 
 /**
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index caebd63..cf29722 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2601,12 +2601,12 @@ int snd_soc_get_enum_double(struct snd_kcontrol *kcontrol,
 	unsigned int val, item;
 	unsigned int reg_val;
 
-	reg_val = snd_soc_read(codec, e->reg);
-	val = (reg_val >> e->shift_l) & e->mask;
+	reg_val = snd_soc_read(codec, e->reg[0]);
+	val = (reg_val >> e->shift_l) & e->mask[0];
 	item = snd_soc_enum_val_to_item(e, val);
 	ucontrol->value.enumerated.item[0] = item;
 	if (e->shift_l != e->shift_r) {
-		val = (reg_val >> e->shift_l) & e->mask;
+		val = (reg_val >> e->shift_l) & e->mask[0];
 		item = snd_soc_enum_val_to_item(e, val);
 		ucontrol->value.enumerated.item[1] = item;
 	}
@@ -2636,15 +2636,15 @@ int snd_soc_put_enum_double(struct snd_kcontrol *kcontrol,
 	if (item[0] >= e->items)
 		return -EINVAL;
 	val = snd_soc_enum_item_to_val(e, item[0]) << e->shift_l;
-	mask = e->mask << e->shift_l;
+	mask = e->mask[0] << e->shift_l;
 	if (e->shift_l != e->shift_r) {
 		if (item[1] >= e->items)
 			return -EINVAL;
 		val |= snd_soc_enum_item_to_val(e, item[1]) << e->shift_r;
-		mask |= e->mask << e->shift_r;
+		mask |= e->mask[0] << e->shift_r;
 	}
 
-	return snd_soc_update_bits_locked(codec, e->reg, mask, val);
+	return snd_soc_update_bits_locked(codec, e->reg[0], mask, val);
 }
 EXPORT_SYMBOL_GPL(snd_soc_put_enum_double);
 
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index c8a780d..19b004a 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -511,13 +511,26 @@ static int dapm_connect_mux(struct snd_soc_dapm_context *dapm,
 	const struct snd_kcontrol_new *kcontrol)
 {
 	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
-	unsigned int val, item;
+	unsigned int val, item, bit_pos = -1;
 	int i;
 
-	if (e->reg != SND_SOC_NOPM) {
-		soc_widget_read(dest, e->reg, &val);
-		val = (val >> e->shift_l) & e->mask;
-		item = snd_soc_enum_val_to_item(e, val);
+	if (e->reg[0] != SND_SOC_NOPM) {
+		if (e->type == SND_SOC_ENUM_ONEHOT) {
+			for (i = 0; i < e->num_regs; i++) {
+				soc_widget_read(dest, e->reg[i], &val);
+				val = val & e->mask[i];
+				if (val != 0) {
+					bit_pos = __ffs(val) +
+						(8 * dest->codec->val_bytes * i);
+					break;
+				}
+			}
+			item = snd_soc_enum_val_to_item(e, bit_pos);
+		} else {
+			soc_widget_read(dest, e->reg[0], &val);
+			val = (val >> e->shift_l) & e->mask[0];
+			item = snd_soc_enum_val_to_item(e, val);
+		}
 	} else {
 		/* since a virtual mux has no backing registers to
 		 * decide which path to connect, it will try to match
@@ -1553,8 +1566,8 @@ static void dapm_widget_update(struct snd_soc_card *card)
 	struct snd_soc_dapm_update *update = card->update;
 	struct snd_soc_dapm_widget_list *wlist;
 	struct snd_soc_dapm_widget *w = NULL;
-	unsigned int wi;
-	int ret;
+	unsigned int wi, i;
+	int ret = 0;
 
 	if (!update || !dapm_kcontrol_is_powered(update->kcontrol))
 		return;
@@ -1575,8 +1588,12 @@ static void dapm_widget_update(struct snd_soc_card *card)
 	if (!w)
 		return;
 
-	ret = soc_widget_update_bits_locked(w, update->reg, update->mask,
-				  update->val);
+	/* dapm update for multiple registers */
+	for (i = 0; i < update->num_regs; i++) {
+		ret |= soc_widget_update_bits_locked(w, update->reg[i],
+					update->mask[i], update->val[i]);
+	}
+
 	if (ret < 0)
 		dev_err(w->dapm->dev, "ASoC: %s DAPM update failed: %d\n",
 			w->name, ret);
@@ -2866,10 +2883,10 @@ int snd_soc_dapm_put_volsw(struct snd_kcontrol *kcontrol,
 	if (change) {
 		if (reg != SND_SOC_NOPM) {
 			update.kcontrol = kcontrol;
-			update.reg = reg;
-			update.mask = mask;
-			update.val = val;
-
+			update.reg[0] = reg;
+			update.mask[0] = mask;
+			update.val[0] = val;
+			update.num_regs = 1;
 			card->update = &update;
 		}
 
@@ -2903,15 +2920,15 @@ int snd_soc_dapm_get_enum_double(struct snd_kcontrol *kcontrol,
 	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
 	unsigned int reg_val, val;
 
-	if (e->reg != SND_SOC_NOPM)
-		reg_val = snd_soc_read(codec, e->reg);
+	if (e->reg[0] != SND_SOC_NOPM)
+		reg_val = snd_soc_read(codec, e->reg[0]);
 	else
 		reg_val = dapm_kcontrol_get_value(kcontrol);
 
-	val = (reg_val >> e->shift_l) & e->mask;
+	val = (reg_val >> e->shift_l) & e->mask[0];
 	ucontrol->value.enumerated.item[0] = snd_soc_enum_val_to_item(e, val);
 	if (e->shift_l != e->shift_r) {
-		val = (reg_val >> e->shift_r) & e->mask;
+		val = (reg_val >> e->shift_r) & e->mask[0];
 		val = snd_soc_enum_val_to_item(e, val);
 		ucontrol->value.enumerated.item[1] = val;
 	}
@@ -2945,27 +2962,28 @@ int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol,
 		return -EINVAL;
 
 	val = snd_soc_enum_item_to_val(e, item[0]) << e->shift_l;
-	mask = e->mask << e->shift_l;
+	mask = e->mask[0] << e->shift_l;
 	if (e->shift_l != e->shift_r) {
 		if (item[1] > e->items)
 			return -EINVAL;
 		val |= snd_soc_enum_item_to_val(e, item[1]) << e->shift_l;
-		mask |= e->mask << e->shift_r;
+		mask |= e->mask[0] << e->shift_r;
 	}
 
 	mutex_lock_nested(&card->dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME);
 
-	if (e->reg != SND_SOC_NOPM)
-		change = snd_soc_test_bits(codec, e->reg, mask, val);
+	if (e->reg[0] != SND_SOC_NOPM)
+		change = snd_soc_test_bits(codec, e->reg[0], mask, val);
 	else
 		change = dapm_kcontrol_set_value(kcontrol, val);
 
 	if (change) {
-		if (e->reg != SND_SOC_NOPM) {
+		if (e->reg[0] != SND_SOC_NOPM) {
 			update.kcontrol = kcontrol;
-			update.reg = e->reg;
-			update.mask = mask;
-			update.val = val;
+			update.reg[0] = e->reg[0];
+			update.mask[0] = mask;
+			update.val[0] = val;
+			update.num_regs = 1;
 			card->update = &update;
 		}
 
@@ -2984,6 +3002,112 @@ int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol,
 EXPORT_SYMBOL_GPL(snd_soc_dapm_put_enum_double);
 
 /**
+ * snd_soc_dapm_get_enum_onehot - dapm enumerated onehot mixer get callback
+ * @kcontrol: mixer control
+ * @ucontrol: control element information
+ *
+ * Callback to get the value of a dapm enumerated onehot encoded mixer control
+ *
+ * Returns 0 for success.
+ */
+int snd_soc_dapm_get_enum_onehot(struct snd_kcontrol *kcontrol,
+			struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_codec *codec = snd_soc_dapm_kcontrol_codec(kcontrol);
+	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
+	unsigned int reg_val, val, bit_pos = -1, reg_idx;
+
+	for (reg_idx = 0; reg_idx < e->num_regs; reg_idx++) {
+		reg_val = snd_soc_read(codec, e->reg[reg_idx]);
+		val = reg_val & e->mask[reg_idx];
+		if (val != 0) {
+			bit_pos = __ffs(val) + (8 * codec->val_bytes * reg_idx);
+			break;
+		}
+	}
+
+	ucontrol->value.enumerated.item[0] =
+			snd_soc_enum_val_to_item(e, bit_pos);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_soc_dapm_get_enum_onehot);
+
+/**
+ * snd_soc_dapm_put_enum_onehot - dapm enumerated onehot mixer put callback
+ * @kcontrol: mixer control
+ * @ucontrol: control element information
+ *
+ * Callback to put the value of a dapm enumerated onehot encoded mixer control
+ *
+ * Returns 0 for success.
+ */
+int snd_soc_dapm_put_enum_onehot(struct snd_kcontrol *kcontrol,
+			struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_codec *codec = snd_soc_dapm_kcontrol_codec(kcontrol);
+	struct snd_soc_card *card = codec->card;
+	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
+	unsigned int *item = ucontrol->value.enumerated.item;
+	unsigned int change = 0, reg_idx = 0, value, bit_pos;
+	struct snd_soc_dapm_update update;
+	int ret = 0, reg_val = 0, i, update_idx = 0;
+
+	if (item[0] >= e->items)
+		return -EINVAL;
+
+	value = snd_soc_enum_item_to_val(e, item[0]);
+
+	if (value >= 0) {
+		/* get the register index and value to set */
+		reg_idx = value / (8 * codec->val_bytes);
+		bit_pos = value % (8 * codec->val_bytes);
+		reg_val = BIT(bit_pos);
+	}
+
+	for (i = 0; i < e->num_regs; i++) {
+		if (i == reg_idx) {
+			change = snd_soc_test_bits(codec, e->reg[i],
+							e->mask[i], reg_val);
+			/* set the selected register */
+			update.reg[e->num_regs - 1] = e->reg[reg_idx];
+			update.mask[e->num_regs - 1] = e->mask[reg_idx];
+			update.val[e->num_regs - 1] = reg_val;
+		} else {
+			/* accumulate the change to update the DAPM path
+			    when none is selected */
+			change |= snd_soc_test_bits(codec, e->reg[i],
+							e->mask[i], 0);
+
+			/* clear the register when not selected */
+			update.reg[update_idx] = e->reg[i];
+			update.mask[update_idx] = e->mask[i];
+			update.val[update_idx++] = 0;
+		}
+	}
+
+	mutex_lock_nested(&card->dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME);
+
+	if (change) {
+		update.kcontrol = kcontrol;
+		update.num_regs = 3;
+		card->update = &update;
+
+		ret = soc_dapm_mux_update_power(card, kcontrol, item[0], e);
+
+		card->update = NULL;
+	}
+
+	mutex_unlock(&card->dapm_mutex);
+
+	if (ret > 0)
+		soc_dpcm_runtime_update(card);
+
+	return change;
+}
+EXPORT_SYMBOL_GPL(snd_soc_dapm_put_enum_onehot);
+
+/**
  * snd_soc_dapm_info_pin_switch - Info for a pin switch
  *
  * @kcontrol: mixer control
-- 
1.7.9.5