ASoC: Correct typo in SOC_VALUE_ENUM_SINGLE macro

PATCH Superseded Charles Keepax 3 participants 9 messages Apr 30, 2015

Subsystems (3)

Sound ASoC Wolfson microelectronics drivers

lore

Share & tools

lore

Charles Keepax

author

2015-04-30 10:46 UTC #0 lore raw

PATCH 1/5 ASoC: Correct typo in SOC_VALUE_ENUM_SINGLE macro

xnitmes is clearly intended to be xnitems, but all other macros just refer to this as xitems, so change it to that.

Signed-off-by Charles Keepax <[email protected]>

---
 include/sound/soc.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

include/sound/soc.h +2 -2

index a73d855..45cfd69 100644
@@ -190,8 +190,8 @@
 #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}
-#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_SINGLE(xreg, xshift, xmask, xitems, xtexts, xvalues) \
+	SOC_VALUE_ENUM_DOUBLE(xreg, xshift, xshift, xmask, xitems, xtexts, xvalues)
 #define SOC_ENUM_SINGLE_VIRT(xitems, xtexts) \
 	SOC_ENUM_SINGLE(SND_SOC_NOPM, 0, xitems, xtexts)
 #define SOC_ENUM(xname, xenum) \

Signature · 1 line

1.7.2.5

Charles Keepax

author

2015-04-30 10:46 UTC #1 lore raw

PATCH 2/5 ASoC: dapm: Remove local OOM error message

The memory subsystem is pretty chatty on failure no need to have local OOM messages as well.

Signed-off-by Charles Keepax <[email protected]>

---
 sound/soc/soc-dapm.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

sound/soc/soc-dapm.c +1 -5

index 79b9478..beb48b6 100644
@@ -310,12 +310,8 @@ static int dapm_kcontrol_data_alloc(struct snd_soc_dapm_widget *widget,
 	struct soc_mixer_control *mc;
 
 	data = kzalloc(sizeof(*data), GFP_KERNEL);
-	if (!data) {
-		dev_err(widget->dapm->dev,
-				"ASoC: can't allocate kcontrol data for %s\n",
-				widget->name);
+	if (!data)
 		return -ENOMEM;
-	}
 
 	INIT_LIST_HEAD(&data->paths);

Signature · 1 line

1.7.2.5

Charles Keepax

author

2015-04-30 10:46 UTC #2 lore raw

PATCH 3/5 ASoC: dapm: Append "Autodisable" to autodisable widget names

In reply to Charles Keepax

This makes it a little easier to follow what is happening in debugfs. Additionally is also useful in facilitating work to add autodisable muxes because the control name is already used for the mux widget and thus shouldn't be reused for the autodisable widget.

Signed-off-by Charles Keepax <[email protected]>

---
 sound/soc/soc-dapm.c |   22 +++++++++++++++++++---
 1 files changed, 19 insertions(+), 3 deletions(-)

sound/soc/soc-dapm.c +19 -3

index beb48b6..e557018 100644
@@ -308,11 +308,19 @@ static int dapm_kcontrol_data_alloc(struct snd_soc_dapm_widget *widget,
 {
 	struct dapm_kcontrol_data *data;
 	struct soc_mixer_control *mc;
+	const char *name;
+	int ret;
 
 	data = kzalloc(sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
 
+	name = kasprintf(GFP_KERNEL, "%s %s", kcontrol->id.name, "Autodisable");
+	if (!name) {
+		ret = -ENOMEM;
+		goto err_data;
+	}
+
 	INIT_LIST_HEAD(&data->paths);
 
 	switch (widget->id) {

@@ -334,15 +342,15 @@ static int dapm_kcontrol_data_alloc(struct snd_soc_dapm_widget *widget,
 				template.off_val = 0;
 			template.on_val = template.off_val;
 			template.id = snd_soc_dapm_kcontrol;
-			template.name = kcontrol->id.name;
+			template.name = name;
 
 			data->value = template.on_val;
 
 			data->widget = snd_soc_dapm_new_control(widget->dapm,
 				&template);
 			if (!data->widget) {
-				kfree(data);
-				return -ENOMEM;
+				ret = -ENOMEM;
+				goto err_name;
 			}
 		}
 		break;

@@ -353,11 +361,19 @@ static int dapm_kcontrol_data_alloc(struct snd_soc_dapm_widget *widget,
 	kcontrol->private_data = data;
 
 	return 0;
+
+err_name:
+	kfree(name);
+err_data:
+	kfree(data);
+	return ret;
 }
 
 static void dapm_kcontrol_free(struct snd_kcontrol *kctl)
 {
 	struct dapm_kcontrol_data *data = snd_kcontrol_chip(kctl);
+	if (data->widget)
+		kfree(data->widget->name);
 	kfree(data->wlist);
 	kfree(data);
 }

Signature · 1 line

1.7.2.5

Charles Keepax

author

2015-04-30 10:46 UTC #3 lore raw

PATCH 4/5 ASoC: dapm: Add support for autodisable mux controls

In reply to Charles Keepax

Commit 57295073b6ac ("ASoC: dapm: Implement mixer input auto-disable") added support for autodisable controls, controls whose values are only written to the hardware when their respective widgets are powered up. But it only added support for controls based on the mixer abstraction.

This patch add support for mux controls (DAPM controls based on the enum abstraction) to be auto-disabled as well. As each mux can only have a single control, there is no need to tie the autodisable widget to the inputs (as is done for the mixer controls) it can be tided directly to the mux widget itself.

Signed-off-by Charles Keepax <[email protected]>

---
 include/sound/soc.h  |   10 ++++++++
 sound/soc/soc-dapm.c |   62 +++++++++++++++++++++++++++++++++++++------------
 2 files changed, 57 insertions(+), 15 deletions(-)

include/sound/soc.h +10 -0

index 45cfd69..28b8ae6 100644
@@ -192,6 +192,10 @@
 	.mask = xmask, .items = xitems, .texts = xtexts, .values = xvalues}
 #define SOC_VALUE_ENUM_SINGLE(xreg, xshift, xmask, xitems, xtexts, xvalues) \
 	SOC_VALUE_ENUM_DOUBLE(xreg, xshift, xshift, xmask, xitems, xtexts, xvalues)
+#define SOC_VALUE_ENUM_SINGLE_AUTODISABLE(xreg, xshift, xmask, xitems, xtexts, xvalues) \
+{	.reg = xreg, .shift_l = xshift, .shift_r = xshift, \
+	.mask = xmask, .items = xitems, .texts = xtexts, \
+	.values = xvalues, .autodisable = 1}
 #define SOC_ENUM_SINGLE_VIRT(xitems, xtexts) \
 	SOC_ENUM_SINGLE(SND_SOC_NOPM, 0, xitems, xtexts)
 #define SOC_ENUM(xname, xenum) \

@@ -312,6 +316,11 @@
 							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_VALUE_ENUM_SINGLE_AUTODISABLE_DECL(name, xreg, xshift, xmask, xtexts, xvalues) \
+	const struct soc_enum name = SOC_VALUE_ENUM_SINGLE_AUTODISABLE(xreg, \
+		xshift, xmask, ARRAY_SIZE(xtexts), xtexts, xvalues)
+
 #define SOC_ENUM_SINGLE_VIRT_DECL(name, xtexts) \
 	const struct soc_enum name = SOC_ENUM_SINGLE_VIRT(ARRAY_SIZE(xtexts), xtexts)

@@ -1200,6 +1209,7 @@ struct soc_enum {
 	unsigned int mask;
 	const char * const *texts;
 	const unsigned int *values;
+	unsigned int autodisable:1;
 };
 
 /**

sound/soc/soc-dapm.c +47 ~~-15~~

118 lines not on this page · raw message

Signature · 1 line

1.7.2.5

Lars-Peter Clausen

2015-04-30 12:44 UTC #4 lore raw

Re: [PATCH 4/5] ASoC: dapm: Add support for autodisable mux controls

Quoted 5 lines · Charles Keepax

Commit 57295073b6ac ("ASoC: dapm: Implement mixer input auto-disable") added support for autodisable controls, controls whose values are only written to the hardware when their respective widgets are powered up. But it only added support for controls based on the mixer abstraction.

Quoted 7 lines

This patch add support for mux controls (DAPM controls based on the enum abstraction) to be auto-disabled as well. As each mux can only have a single control, there is no need to tie the autodisable widget to the inputs (as is done for the mixer controls) it can be tided directly to the mux widget itself.
Signed-off-by Charles Keepax <[email protected]>

Looks pretty good.

[...]

Quoted 5 lines

sound/soc/soc-dapm.c +1 -0

@@ -354,6 +355,34 @@ static int dapm_kcontrol_data_alloc(struct snd_soc_dapm_widget *widget,
  			}
  		}
  		break;
+	case snd_soc_dapm_mux:

sound/soc/soc-dapm.c static int dapm_kcontrol_data_alloc(struct snd_soc_dapm_widget *widget, · line 368

Quoted 10 lines

sound/soc/soc-dapm.c +10 -0

+		e = (struct soc_enum *)kcontrol->private_value;
+
+		if (e->autodisable) {
+			struct snd_soc_dapm_widget template;
+
+			memset(&template, 0, sizeof(template));
+			template.reg = e->reg;
+			template.mask = e->mask << e->shift_l;
+			template.shift = e->shift_l;
+			template.off_val = 0;

I've though about adding a auto-disable MUX type as well and the chip were I wanted to use it has a non-zero power-off value. So I'd like to be able to somehow specify this. We could for example put it at the end of the values array. E.g.

if (e->values)

	template.off_val = e->values[e->items].
else
	template.off_val = 0;

Quoted 20 lines

sound/soc/soc-dapm.c +17 -0

+			template.on_val = template.off_val;
+			template.id = snd_soc_dapm_kcontrol;
+			template.name = name;
+
+			data->value = template.on_val;
+
+			data->widget = snd_soc_dapm_new_control(widget->dapm,
+					&template);
+			if (!data->widget) {
+				ret = -ENOMEM;
+				goto err_name;
+			}
+
+			snd_soc_dapm_add_path(widget->dapm, data->widget,
+					      widget, NULL, NULL);
+		}
+		break;
  	default:
  		break;
  	}

Charles Keepax

author

2015-04-30 15:00 UTC #5 lore raw

Re: [PATCH 4/5] ASoC: dapm: Add support for autodisable mux controls

Quoted 38 lines · Lars-Peter Clausen

Quoted 5 lines · Charles Keepax
Commit 57295073b6ac ("ASoC: dapm: Implement mixer input auto-disable") added support for autodisable controls, controls whose values are only written to the hardware when their respective widgets are powered up. But it only added support for controls based on the mixer abstraction.
Quoted 7 lines
This patch add support for mux controls (DAPM controls based on the enum abstraction) to be auto-disabled as well. As each mux can only have a single control, there is no need to tie the autodisable widget to the inputs (as is done for the mixer controls) it can be tided directly to the mux widget itself.
Signed-off-by Charles Keepax <[email protected]>
Looks pretty good.
[...]
Quoted 5 lines
sound/soc/soc-dapm.c +1 -0
@@ -354,6 +355,34 @@ static int dapm_kcontrol_data_alloc(struct snd_soc_dapm_widget *widget,
  			}
  		}
  		break;
+	case snd_soc_dapm_mux:
sound/soc/soc-dapm.c static int dapm_kcontrol_data_alloc(struct snd_soc_dapm_widget *widget, · line 368
Quoted 10 lines
sound/soc/soc-dapm.c +10 -0
+		e = (struct soc_enum *)kcontrol->private_value;
+
+		if (e->autodisable) {
+			struct snd_soc_dapm_widget template;
+
+			memset(&template, 0, sizeof(template));
+			template.reg = e->reg;
+			template.mask = e->mask << e->shift_l;
+			template.shift = e->shift_l;
+			template.off_val = 0;
I've though about adding a auto-disable MUX type as well and the chip were I wanted to use it has a non-zero power-off value. So I'd like to be able to somehow specify this. We could for example put it at the end of the values array. E.g.

Quoted 5 lines

if (e->values)

	template.off_val = e->values[e->items].
else
	template.off_val = 0;

Thats a good point something I hadn't thought of. It seems reasonable that if the mux is autodisable that one of the states in the values array should be the disabled state. Either the first or last value would seem the reasonable choices. I would lean towards the first value, as this is already the case in the Arizona code :-).

I will do a respin with that included.

Thanks,
Charles

Lars-Peter Clausen

2015-04-30 15:40 UTC #6 lore raw

Re: [PATCH 4/5] ASoC: dapm: Add support for autodisable mux controls

Quoted 45 lines · Charles Keepax

Quoted 38 lines · Lars-Peter Clausen
Quoted 5 lines · Charles Keepax
Commit 57295073b6ac ("ASoC: dapm: Implement mixer input auto-disable") added support for autodisable controls, controls whose values are only written to the hardware when their respective widgets are powered up. But it only added support for controls based on the mixer abstraction.
Quoted 7 lines
This patch add support for mux controls (DAPM controls based on the enum abstraction) to be auto-disabled as well. As each mux can only have a single control, there is no need to tie the autodisable widget to the inputs (as is done for the mixer controls) it can be tided directly to the mux widget itself.
Signed-off-by Charles Keepax <[email protected]>
Looks pretty good.
[...]
Quoted 5 lines
sound/soc/soc-dapm.c +1 -0
@@ -354,6 +355,34 @@ static int dapm_kcontrol_data_alloc(struct snd_soc_dapm_widget *widget,
   			}
   		}
   		break;
+	case snd_soc_dapm_mux:
sound/soc/soc-dapm.c static int dapm_kcontrol_data_alloc(struct snd_soc_dapm_widget *widget, · line 368
Quoted 10 lines
sound/soc/soc-dapm.c +10 -0
+		e = (struct soc_enum *)kcontrol->private_value;
+
+		if (e->autodisable) {
+			struct snd_soc_dapm_widget template;
+
+			memset(&template, 0, sizeof(template));
+			template.reg = e->reg;
+			template.mask = e->mask << e->shift_l;
+			template.shift = e->shift_l;
+			template.off_val = 0;
I've though about adding a auto-disable MUX type as well and the chip were I wanted to use it has a non-zero power-off value. So I'd like to be able to somehow specify this. We could for example put it at the end of the values array. E.g.
Quoted 5 lines
if (e->values)
	template.off_val = e->values[e->items].
else
	template.off_val = 0;

Quoted 6 lines

Thats a good point something I hadn't thought of. It seems reasonable that if the mux is autodisable that one of the states in the values array should be the disabled state. Either the first or last value would seem the reasonable choices. I would lean towards the first value, as this is already the case in the Arizona code :-).

Hm, right. What I had in mind is to not export the off state to userland, but I guess it makes sense to make it possible to select it explicitly since it is consistent with the auto-disable switches.

- Lars

Mark Brown

maintainer

2015-04-30 15:59 UTC #7 lore raw

Re: [PATCH 4/5] ASoC: dapm: Add support for autodisable mux controls

Quoted 3 lines · Lars-Peter Clausen

Hm, right. What I had in mind is to not export the off state to userland, but I guess it makes sense to make it possible to select it explicitly since it is consistent with the auto-disable switches.

It's not just consistent, you need to be able to do that so that you can set something to a known unused input when it's not in use, otherwise you need to worry about making sure that you don't end up causing audio to flow through the mux unintentionally when you go into some other use case.

Charles Keepax

author

2015-04-30 10:46 UTC #8 lore raw

PATCH 5/5 ASoC: arizona: Use auto disable muxes for routing

In reply to Charles Keepax

The mixer core on the Arizona devices is powered up whenever any routing is non-zero. This patch saves a little power and avoids a few difficult corner cases (around the mixer core being powered whilst there is no clock available), by using the autodisable mux functionality to only write out the settings for the muxes when they are powered up.

Signed-off-by Charles Keepax <[email protected]>

---
 sound/soc/codecs/arizona.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

sound/soc/codecs/arizona.h +2 -2

index 11ff899..bacc296 100644
@@ -107,8 +107,8 @@ extern int arizona_mixer_values[ARIZONA_NUM_MIXER_INPUTS];
 			     arizona_mixer_tlv)
 
 #define ARIZONA_MUX_ENUM_DECL(name, reg) \
-	SOC_VALUE_ENUM_SINGLE_DECL(name, reg, 0, 0xff,			\
-				   arizona_mixer_texts, arizona_mixer_values)
+	SOC_VALUE_ENUM_SINGLE_AUTODISABLE_DECL( \
+		name, reg, 0, 0xff, arizona_mixer_texts, arizona_mixer_values)
 
 #define ARIZONA_MUX_CTL_DECL(name) \
 	const struct snd_kcontrol_new name##_mux =	\

Signature · 1 line

1.7.2.5