[PATCH] ASoC: dapm - Refactor widget IO functions in preparation for platform widgets.

[PATCH] ASoC: dapm - Refactor widget IO functions in preparation for platform widgets.

[Liam Girdwood]

Currently widget IO is tightly coupled to the CODEC drivers. Future platform DSP
devices have mixer components that can alter power usage and hence require full
DAPM support.

This provides a generic widget IO operation wrapper in preparation for
future patches that implement platform driver DAPM.

Signed-off-by: Liam Girdwood <lrg@ti.com>
---
 sound/soc/soc-dapm.c |   55 ++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 34106bc..7b7af9f 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -124,6 +124,49 @@ static inline struct snd_soc_dapm_widget *dapm_cnew_widget(
 	return kmemdup(_widget, sizeof(*_widget), GFP_KERNEL);
 }
 
+static int soc_widget_read(struct snd_soc_dapm_widget *w, int reg)
+{
+	if (w->codec)
+		return snd_soc_read(w->codec, reg);
+	return 0;
+}
+
+static int soc_widget_write(struct snd_soc_dapm_widget *w,int reg, int val)
+{
+	if (w->codec)
+		return snd_soc_write(w->codec, reg, val);
+	return 0;
+}
+
+int soc_widget_update_bits(struct snd_soc_dapm_widget *w, unsigned short reg,
+				unsigned int mask, unsigned int value)
+{
+	int change;
+	unsigned int old, new;
+
+	old = soc_widget_read(w, reg);
+	new = (old & ~mask) | value;
+	change = old != new;
+	if (change)
+		soc_widget_write(w, reg, new);
+
+	return change;
+}
+
+int soc_widget_test_bits(struct snd_soc_dapm_widget *w, unsigned short reg,
+				unsigned int mask, unsigned int value)
+{
+	int change;
+	unsigned int old, new;
+
+	old = soc_widget_read(w, reg);
+	new = (old & ~mask) | value;
+	change = old != new;
+
+	return change;
+}
+
+
 /**
  * snd_soc_dapm_set_bias_level - set the bias level for the system
  * @dapm: DAPM context
@@ -181,7 +224,7 @@ static void dapm_set_path_status(struct snd_soc_dapm_widget *w,
 		unsigned int mask = (1 << fls(max)) - 1;
 		unsigned int invert = mc->invert;
 
-		val = snd_soc_read(w->codec, reg);
+		val = soc_widget_read(w, reg);
 		val = (val >> shift) & mask;
 
 		if ((invert && !val) || (!invert && val))
@@ -197,7 +240,7 @@ static void dapm_set_path_status(struct snd_soc_dapm_widget *w,
 
 		for (bitmask = 1; bitmask < e->max; bitmask <<= 1)
 			;
-		val = snd_soc_read(w->codec, e->reg);
+		val = soc_widget_read(w, e->reg);
 		item = (val >> e->shift_l) & (bitmask - 1);
 
 		p->connect = 0;
@@ -227,7 +270,7 @@ static void dapm_set_path_status(struct snd_soc_dapm_widget *w,
 			w->kcontrol_news[i].private_value;
 		int val, item;
 
-		val = snd_soc_read(w->codec, e->reg);
+		val = soc_widget_read(w, e->reg);
 		val = (val >> e->shift_l) & e->mask;
 		for (item = 0; item < e->max; item++) {
 			if (val == e->values[item])
@@ -669,7 +712,7 @@ int dapm_reg_event(struct snd_soc_dapm_widget *w,
 	else
 		val = w->off_val;
 
-	snd_soc_update_bits(w->codec, -(w->reg + 1),
+	soc_widget_update_bits(w, -(w->reg + 1),
 			    w->mask << w->shift, val << w->shift);
 
 	return 0;
@@ -877,7 +920,7 @@ static void dapm_seq_run_coalesced(struct snd_soc_dapm_context *dapm,
 			"pop test : Applying 0x%x/0x%x to %x in %dms\n",
 			value, mask, reg, card->pop_time);
 		pop_wait(card->pop_time);
-		snd_soc_update_bits(dapm->codec, reg, mask, value);
+		soc_widget_update_bits(w, reg, mask, value);
 	}
 
 	list_for_each_entry(w, pending, power_list) {
@@ -1878,7 +1921,7 @@ int snd_soc_dapm_new_widgets(struct snd_soc_dapm_context *dapm)
 
 		/* Read the initial power state from the device */
 		if (w->reg >= 0) {
-			val = snd_soc_read(w->codec, w->reg);
+			val = soc_widget_read(w, w->reg);
 			val &= 1 << w->shift;
 			if (w->invert)
 				val = !val;
-- 
1.7.4.1

Re: [PATCH] ASoC: dapm - Refactor widget IO functions in preparation for platform widgets.

[Mark Brown]

On Thu, Jun 09, 2011 at 07:10:49PM +0100, Liam Girdwood wrote:

> +static int soc_widget_read(struct snd_soc_dapm_widget *w, int reg)
> +{
> +	if (w->codec)
> +		return snd_soc_read(w->codec, reg);
> +	return 0;
> +}

I guess this and write() should return an error if they can't find
something to interact with.  And realistically to also complain in the
logs given how usefully we can actualy handle errors.

I'm tempted to say let me post my regmap API that I've had sitting in my
-next queue over the weekend then we can just unconditionally use a
regmap here.  I'd not been too happy with how it was handling mutli-word
I/O but there's only one user of that in ASoC so far.  We still need the
_widget and so on versions wrapping them in ASoC of course so that we
can pass sensible objects in.

> +int soc_widget_update_bits(struct snd_soc_dapm_widget *w, unsigned short reg,
> +				unsigned int mask, unsigned int value)
> +{
> +	int change;
> +	unsigned int old, new;
> +
> +	old = soc_widget_read(w, reg);
> +	new = (old & ~mask) | value;

Should do the & mask on value that I added to the vanilla one last week
- that would've avoided a genuine bug.

Re: [PATCH] ASoC: dapm - Refactor widget IO functions in preparation for platform widgets.

[Takashi Iwai]

At Thu, 9 Jun 2011 19:10:49 +0100,
Liam Girdwood wrote:
> 
> Currently widget IO is tightly coupled to the CODEC drivers. Future platform DSP
> devices have mixer components that can alter power usage and hence require full
> DAPM support.
> 
> This provides a generic widget IO operation wrapper in preparation for
> future patches that implement platform driver DAPM.
> 
> Signed-off-by: Liam Girdwood <lrg@ti.com>
> ---
>  sound/soc/soc-dapm.c |   55 ++++++++++++++++++++++++++++++++++++++++++++-----
>  1 files changed, 49 insertions(+), 6 deletions(-)
> 
> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
> index 34106bc..7b7af9f 100644
> --- a/sound/soc/soc-dapm.c
> +++ b/sound/soc/soc-dapm.c
> @@ -124,6 +124,49 @@ static inline struct snd_soc_dapm_widget *dapm_cnew_widget(
>  	return kmemdup(_widget, sizeof(*_widget), GFP_KERNEL);
>  }
>  
> +static int soc_widget_read(struct snd_soc_dapm_widget *w, int reg)
> +{
> +	if (w->codec)
> +		return snd_soc_read(w->codec, reg);
> +	return 0;
> +}
> +
> +static int soc_widget_write(struct snd_soc_dapm_widget *w,int reg, int val)

Missing space after comma.

> +{
> +	if (w->codec)
> +		return snd_soc_write(w->codec, reg, val);
> +	return 0;
> +}
> +
> +int soc_widget_update_bits(struct snd_soc_dapm_widget *w, unsigned short reg,
> +				unsigned int mask, unsigned int value)

Do you want to expose this and soc_widget_test_bits()?
Or just forgotten static?


Takashi

Re: [PATCH] ASoC: dapm - Refactor widget IO functions in preparation for platform widgets.

[Liam Girdwood]

On 10/06/11 13:11, Takashi Iwai wrote:
> At Thu, 9 Jun 2011 19:10:49 +0100,
> Liam Girdwood wrote:
>>
>> Currently widget IO is tightly coupled to the CODEC drivers. Future platform DSP
>> devices have mixer components that can alter power usage and hence require full
>> DAPM support.
>>
>> This provides a generic widget IO operation wrapper in preparation for
>> future patches that implement platform driver DAPM.
>>
>> Signed-off-by: Liam Girdwood <lrg@ti.com>
>> ---
>>  sound/soc/soc-dapm.c |   55 ++++++++++++++++++++++++++++++++++++++++++++-----
>>  1 files changed, 49 insertions(+), 6 deletions(-)
>>
>> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
>> index 34106bc..7b7af9f 100644
>> --- a/sound/soc/soc-dapm.c
>> +++ b/sound/soc/soc-dapm.c
>> @@ -124,6 +124,49 @@ static inline struct snd_soc_dapm_widget *dapm_cnew_widget(
>>  	return kmemdup(_widget, sizeof(*_widget), GFP_KERNEL);
>>  }
>>  
>> +static int soc_widget_read(struct snd_soc_dapm_widget *w, int reg)
>> +{
>> +	if (w->codec)
>> +		return snd_soc_read(w->codec, reg);
>> +	return 0;
>> +}
>> +
>> +static int soc_widget_write(struct snd_soc_dapm_widget *w,int reg, int val)
> 
> Missing space after comma.

Thanks, didn't see that.

> 
>> +{
>> +	if (w->codec)
>> +		return snd_soc_write(w->codec, reg, val);
>> +	return 0;
>> +}
>> +
>> +int soc_widget_update_bits(struct snd_soc_dapm_widget *w, unsigned short reg,
>> +				unsigned int mask, unsigned int value)
> 
> Do you want to expose this and soc_widget_test_bits()?
> Or just forgotten static?
> 

The original intention was that they may have been used by the ABE but this seems unlikely now.

I'll make them static for V2 and remove test_bits since it's not used.

Liam

Re: [PATCH] ASoC: dapm - Refactor widget IO functions in preparation for platform widgets.

[Takashi Iwai]

At Fri, 10 Jun 2011 15:44:30 +0100,
Liam Girdwood wrote:
> 
> On 10/06/11 13:11, Takashi Iwai wrote:
> > At Thu, 9 Jun 2011 19:10:49 +0100,
> > Liam Girdwood wrote:
> >>
> >> Currently widget IO is tightly coupled to the CODEC drivers. Future platform DSP
> >> devices have mixer components that can alter power usage and hence require full
> >> DAPM support.
> >>
> >> This provides a generic widget IO operation wrapper in preparation for
> >> future patches that implement platform driver DAPM.
> >>
> >> Signed-off-by: Liam Girdwood <lrg@ti.com>
> >> ---
> >>  sound/soc/soc-dapm.c |   55 ++++++++++++++++++++++++++++++++++++++++++++-----
> >>  1 files changed, 49 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
> >> index 34106bc..7b7af9f 100644
> >> --- a/sound/soc/soc-dapm.c
> >> +++ b/sound/soc/soc-dapm.c
> >> @@ -124,6 +124,49 @@ static inline struct snd_soc_dapm_widget *dapm_cnew_widget(
> >>  	return kmemdup(_widget, sizeof(*_widget), GFP_KERNEL);
> >>  }
> >>  
> >> +static int soc_widget_read(struct snd_soc_dapm_widget *w, int reg)
> >> +{
> >> +	if (w->codec)
> >> +		return snd_soc_read(w->codec, reg);
> >> +	return 0;
> >> +}
> >> +
> >> +static int soc_widget_write(struct snd_soc_dapm_widget *w,int reg, int val)
> > 
> > Missing space after comma.
> 
> Thanks, didn't see that.
> 
> > 
> >> +{
> >> +	if (w->codec)
> >> +		return snd_soc_write(w->codec, reg, val);
> >> +	return 0;
> >> +}
> >> +
> >> +int soc_widget_update_bits(struct snd_soc_dapm_widget *w, unsigned short reg,
> >> +				unsigned int mask, unsigned int value)
> > 
> > Do you want to expose this and soc_widget_test_bits()?
> > Or just forgotten static?
> > 
> 
> The original intention was that they may have been used by the ABE but this seems unlikely now.
> 
> I'll make them static for V2 and remove test_bits since it's not used.

Sounds reasonable.


thanks,

Takashi