[PATCH] ASoC: UDA134X Codec: Fix mute/unmute code mistake and add ADC/DAC power control support

[PATCH] ASoC: UDA134X Codec: Fix mute/unmute code mistake and add ADC/DAC power control support

[Shine Liu]

There is a mistake in current uda134x_mute function: mute_reg has been
changed in line 162 or line 164, so uda134x_write should write
"mute_reg" but not "mute_reg & ~(1<<2)" to 
UDA134X_DATA010.

Besides, because there is no DAPM configuration for uda134x, when system
starts up, snd_soc_int_card calls snd_soc_dapm_new_widgets, and
snd_soc_dapm_new_widgets calls dapm_power_widgets. In function
dapm_power_widgets, codec->dapm_widgets has no list entry, so sys_power
retains it's original value zero. Then
snd_soc_dapm_set_bias_level(socdev, SND_SOC_BIAS_PREPARE) and  
snd_soc_dapm_set_bias_level(socdev, SND_SOC_BIAS_STANDBY) are called
sequentially. Finally, the uda134x codec goes to STANDBY mode, so the
ADC/DAC power control bits of UDA134X_STATUS1 register keeps the 0
value.

UDA134X has no trigger function currently, and the ADC/DAC power control
bits of UDA134X_STATUS1 register are not exported in
uda1341_snd_controls, so there's no way to enable the ADC/DAC power
control bits when you want to use the codec. When playing with aplay or
recording with arecord, there is no sound output or no wave input.

I have added the uda134x_trigger function, which turns on/off the
ADC/DAC power control bits according the substream type and the command.
I also exported the ADC/DAC power control bits to uda1341_snd_controls,
so we can also turn these bits on/off as we need via a mixer tool like
alsamixer.

The patch created against linux-2.6.31-rc3.
Tested on a s3c2440 development board with UDA1341TS codec.


Signed-off-by: Shine Liu <shinel@foxmail.com> 

--- sound/soc/codecs/uda134x.c.orig	2009-07-14 09:18:52.000000000 +0800
+++ sound/soc/codecs/uda134x.c	2009-08-17 13:46:57.000000000 +0800
@@ -163,7 +163,7 @@
 	else
 		mute_reg &= ~(1<<2);
 
-	uda134x_write(codec, UDA134X_DATA010, mute_reg & ~(1<<2));
+	uda134x_write(codec, UDA134X_DATA010, mute_reg);
 
 	return 0;
 }
@@ -339,6 +339,38 @@
 	return 0;
 }
 
+static int uda134x_trigger(struct snd_pcm_substream *substream,
+			   int cmd, struct snd_soc_dai *dai)
+{
+	int stream = substream->stream;
+	struct snd_soc_codec *codec = dai->codec;
+	u8 power_ctrl_reg = uda134x_read_reg_cache(codec, UDA134X_STATUS1);
+
+	pr_debug("%s stream: %d, cmd: %d\n", __func__, stream, cmd);
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+		if (stream == SNDRV_PCM_STREAM_PLAYBACK)
+			power_ctrl_reg |= (1<<0);
+		else
+			power_ctrl_reg |= (1<<1);
+		break;
+
+	case SNDRV_PCM_TRIGGER_STOP:
+		if (stream == SNDRV_PCM_STREAM_PLAYBACK)
+			power_ctrl_reg &= ~(1<<0);
+		else
+			power_ctrl_reg &= ~(1<<1);
+		break;
+
+	default:
+		return 0;
+	}
+
+	uda134x_write(codec, UDA134X_STATUS1, power_ctrl_reg);
+	return 0;
+}
+
 static int uda134x_set_bias_level(struct snd_soc_codec *codec,
 				  enum snd_soc_bias_level level)
 {
@@ -416,6 +448,8 @@
 SOC_SINGLE("ADC Polarity Switch", UDA134X_STATUS1, 4, 1, 0),
 SOC_SINGLE("DAC Polarity Switch", UDA134X_STATUS1, 3, 1, 0),
 SOC_SINGLE("Double Speed Playback Switch", UDA134X_STATUS1, 2, 1, 0),
+SOC_SINGLE("ADC Power Switch", UDA134X_STATUS1, 1, 1, 0),
+SOC_SINGLE("DAC Power Switch", UDA134X_STATUS1, 0, 1, 0),
 SOC_SINGLE("DC Filter Enable Switch", UDA134X_STATUS0, 0, 1, 0),
 };
 
@@ -438,6 +472,7 @@
 	.digital_mute	= uda134x_mute,
 	.set_sysclk	= uda134x_set_dai_sysclk,
 	.set_fmt	= uda134x_set_dai_fmt,
+	.trigger	= uda134x_trigger,
 };
 
 struct snd_soc_dai uda134x_dai = {

Re: [PATCH] ASoC: UDA134X Codec: Fix mute/unmute code mistake and add ADC/DAC power control support

[Mark Brown]

On Mon, Aug 17, 2009 at 04:36:56PM +0800, Shine Liu wrote:
> There is a mistake in current uda134x_mute function: mute_reg has been
> changed in line 162 or line 164, so uda134x_write should write
> "mute_reg" but not "mute_reg & ~(1<<2)" to 
> UDA134X_DATA010.

This is OK but please split it out into a separate patch and resend
since...

> Besides, because there is no DAPM configuration for uda134x, when system
> starts up, snd_soc_int_card calls snd_soc_dapm_new_widgets, and
> snd_soc_dapm_new_widgets calls dapm_power_widgets. In function
> dapm_power_widgets, codec->dapm_widgets has no list entry, so sys_power
> retains it's original value zero. Then

...this isn't, partly because I can't really follow what you're trying
to do here.  It looks awfully like you're trying to implement DAPM
support - certainly things like adding the switches for DAC and ADC
power shouldn't be there, they should be automatically managed in the
drivers.

[PATCH] ASoC: UDA134X: Fix mistaken mute/unmute code

[Shine Liu]

Resend the patch for the mistaken mute/unmute code.


Signed-off-by: Shine Liu <shinel@foxmail.com>


--- sound/soc/codecs/uda134x.c.orig	2009-07-14 09:18:52.000000000 +0800
+++ sound/soc/codecs/uda134x.c	2009-08-17 13:46:57.000000000 +0800
@@ -163,7 +163,7 @@
 	else
 		mute_reg &= ~(1<<2);
 
-	uda134x_write(codec, UDA134X_DATA010, mute_reg & ~(1<<2));
+	uda134x_write(codec, UDA134X_DATA010, mute_reg);
 
 	return 0;
 }


On Mon, 2009-08-17 at 11:32 +0100, Mark Brown wrote:
> On Mon, Aug 17, 2009 at 04:36:56PM +0800, Shine Liu wrote:
> > There is a mistake in current uda134x_mute function: mute_reg has been
> > changed in line 162 or line 164, so uda134x_write should write
> > "mute_reg" but not "mute_reg & ~(1<<2)" to 
> > UDA134X_DATA010.
> 
> This is OK but please split it out into a separate patch and resend
> since...
> 
> > Besides, because there is no DAPM configuration for uda134x, when system
> > starts up, snd_soc_int_card calls snd_soc_dapm_new_widgets, and
> > snd_soc_dapm_new_widgets calls dapm_power_widgets. In function
> > dapm_power_widgets, codec->dapm_widgets has no list entry, so sys_power
> > retains it's original value zero. Then
> 
> ...this isn't, partly because I can't really follow what you're trying
> to do here.  It looks awfully like you're trying to implement DAPM
> support - certainly things like adding the switches for DAC and ADC
> power shouldn't be there, they should be automatically managed in the
> drivers.
> 
> 
> 

Re: [PATCH] ASoC: UDA134X: Fix mistaken mute/unmute code

[Mark Brown]

On Mon, Aug 17, 2009 at 06:52:01PM +0800, Shine Liu wrote:
> Resend the patch for the mistaken mute/unmute code.

> Signed-off-by: Shine Liu <shinel@foxmail.com>

I've applied this after copying the changelog from your previous patch.

> --- sound/soc/codecs/uda134x.c.orig	2009-07-14 09:18:52.000000000 +0800
> +++ sound/soc/codecs/uda134x.c	2009-08-17 13:46:57.000000000 +0800

Please also see Documentation/SubmittingPatches - patches should be
generated to be applied with patch -p1.

Re: [PATCH] ASoC: UDA134X Codec: Fix mute/unmute code mistake and add ADC/DAC power control support

[Shine Liu]

On Mon, 2009-08-17 at 11:32 +0100, Mark Brown wrote:
> On Mon, Aug 17, 2009 at 04:36:56PM +0800, Shine Liu wrote:
> > There is a mistake in current uda134x_mute function: mute_reg has been
> > changed in line 162 or line 164, so uda134x_write should write
> > "mute_reg" but not "mute_reg & ~(1<<2)" to 
> > UDA134X_DATA010.
> 
> This is OK but please split it out into a separate patch and resend
> since...
> 
> > Besides, because there is no DAPM configuration for uda134x, when system
> > starts up, snd_soc_int_card calls snd_soc_dapm_new_widgets, and
> > snd_soc_dapm_new_widgets calls dapm_power_widgets. In function
> > dapm_power_widgets, codec->dapm_widgets has no list entry, so sys_power
> > retains it's original value zero. Then
> 
> ...this isn't, partly because I can't really follow what you're trying
> to do here.  It looks awfully like you're trying to implement DAPM
> support - certainly things like adding the switches for DAC and ADC
> power shouldn't be there, they should be automatically managed in the
> drivers.
> 

When I play a wav file using aplay, I found there's no sound. After some
debug work, I noticed that the DAC power control bit keeps zero since
the system started up.

DAC power control bit should be set when the codec is about to excute a
playback task. But current code doesn't set this bit on before playback.
So I do the work in the codec's trigger operation.

So is the wave recording. There's no code to set the ADC power control
bit on when to record.

Yes, it's true that we should not expose the power control to the end
user. So I should the remove the two lines in the last patch.

Re: [PATCH] ASoC: UDA134X Codec: Fix mute/unmute code mistake and add ADC/DAC power control support

[Mark Brown]

On Mon, Aug 17, 2009 at 07:22:34PM +0800, Shine Liu wrote:

> When I play a wav file using aplay, I found there's no sound. After some
> debug work, I noticed that the DAC power control bit keeps zero since
> the system started up.

Please try the patch below.

commit 6294523841a61cd1ad24f220daf6e73e9cc83184
Author: Mark Brown <broonie@opensource.wolfsonmicro.com>
Date:   Mon Aug 17 11:55:38 2009 +0100

    ASoC: Fix handling of bias levels for non-DAPM codecs
    
    If the system doesn't have any DAPM widgets then we can't use their
    state to check if the bias level for the codec should be up.
    
    Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index c68c204..4a21a5e 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -966,6 +966,22 @@ static int dapm_power_widgets(struct snd_soc_codec *codec, int event)
 		}
 	}
 
+	/* If there are no DAPM widgets then try to figure out power from the
+	 * event type.
+	 */
+	if (list_empty(&codec->dapm_widgets)) {
+		switch (event) {
+		case SND_SOC_DAPM_STREAM_START:
+		case SND_SOC_DAPM_STREAM_RESUME:
+			sys_power = 1;
+			break;
+		case SND_SOC_DAPM_STREAM_NOP:
+			sys_power == codec->bias_level != SND_SOC_BIAS_STANDBY;
+		default:
+			break;
+		}
+	}
+
 	/* If we're changing to all on or all off then prepare */
 	if ((sys_power && codec->bias_level == SND_SOC_BIAS_STANDBY) ||
 	    (!sys_power && codec->bias_level == SND_SOC_BIAS_ON)) {

Re: [PATCH] ASoC: UDA134X: Fix mistaken mute/unmute code

[Shine Liu]

Thank you for your remainder.
Using -p1 regenerate the patch.


--- a/sound/soc/codecs/uda134x.c	2009-07-14 09:18:52.000000000 +0800
+++ b/sound/soc/codecs/uda134x.c	2009-08-17 13:46:57.000000000 +0800
@@ -163,7 +163,7 @@
 	else
 		mute_reg &= ~(1<<2);
 
-	uda134x_write(codec, UDA134X_DATA010, mute_reg & ~(1<<2));
+	uda134x_write(codec, UDA134X_DATA010, mute_reg);
 
 	return 0;
 }

Re: [PATCH] ASoC: UDA134X Codec: Fix mute/unmute code mistake and add ADC/DAC power control support

[Shine Liu]

On Mon, 2009-08-17 at 12:27 +0100, Mark Brown wrote:

> +	/* If there are no DAPM widgets then try to figure out power from the
> +	 * event type.
> +	 */
> +	if (list_empty(&codec->dapm_widgets)) {
> +		switch (event) {
> +		case SND_SOC_DAPM_STREAM_START:
> +		case SND_SOC_DAPM_STREAM_RESUME:
> +			sys_power = 1;
> +			break;
> +		case SND_SOC_DAPM_STREAM_NOP:
> +			sys_power == codec->bias_level != SND_SOC_BIAS_STANDBY;

Should be "sys_power = codec->bias_level != SND_SOC_BIAS_STANDBY;" ?

> +		default:
> +			break;
> +		}
> +	}
> +
>  	/* If we're changing to all on or all off then prepare */
>  	if ((sys_power && codec->bias_level == SND_SOC_BIAS_STANDBY) ||
>  	    (!sys_power && codec->bias_level == SND_SOC_BIAS_ON)) {
> 
> 
> 

Re: [PATCH] ASoC: UDA134X Codec: Fix mute/unmute code mistake and add ADC/DAC power control support

[Mark Brown]

On Mon, Aug 17, 2009 at 07:53:15PM +0800, Shine Liu wrote:

> Should be "sys_power = codec->bias_level != SND_SOC_BIAS_STANDBY;" ?

Yes.