[PATCH] ASoC: SDP3430: Add support for EXTMUTE using TWL GPIO6

[PATCH] ASoC: SDP3430: Add support for EXTMUTE using TWL GPIO6

[Candelaria Villareal, Jorge]

Board sdp3430 has hardware support for EXTMUTE using TWL4030 GPIO6
line, controlled by register INTBR_PMBR1. Machine driver takes care
of enabling gpio line through i2c and codec driver manipulates the
line during headset ramp up/down sequence.

Signed-off-by: Jorge Eduardo Candelaria <x0107209@ti.com>
---
 sound/soc/omap/sdp3430.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/sound/soc/omap/sdp3430.c b/sound/soc/omap/sdp3430.c
index c51594d..f7e5b74 100644
--- a/sound/soc/omap/sdp3430.c
+++ b/sound/soc/omap/sdp3430.c
@@ -24,6 +24,7 @@
 
 #include <linux/clk.h>
 #include <linux/platform_device.h>
+#include <linux/i2c/twl4030.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/soc.h>
@@ -39,6 +40,9 @@
 #include "omap-pcm.h"
 #include "../codecs/twl4030.h"
 
+#define TWL4030_INTBR_PMBR1	0x0D
+#define EXTMUTE(value)		(value << 2)
+
 static struct snd_soc_card snd_soc_sdp3430;
 
 static int sdp3430_hw_params(struct snd_pcm_substream *substream,
@@ -280,6 +284,7 @@ static struct snd_soc_card snd_soc_sdp3430 = {
 static struct twl4030_setup_data twl4030_setup = {
 	.ramp_delay_value = 3,
 	.sysclk = 26000,
+	.hs_extmute = 1,
 };
 
 /* Audio subsystem */
@@ -312,6 +317,10 @@ static int __init sdp3430_soc_init(void)
 	*(unsigned int *)sdp3430_dai[0].cpu_dai->private_data = 1; /* McBSP2 */
 	*(unsigned int *)sdp3430_dai[1].cpu_dai->private_data = 2; /* McBSP3 */
 
+	/* Set TWL4030 GPIO6 as EXTMUTE signal */
+	twl4030_i2c_write_u8(TWL4030_MODULE_INTBR, EXTMUTE(0x02),
+							TWL4030_MODULE_INTBR);
+
 	ret = platform_device_add(sdp3430_snd_device);
 	if (ret)
 		goto err1;
-- 
1.6.0.4

Re: [PATCH] ASoC: SDP3430: Add support for EXTMUTE using TWL GPIO6

[Mark Brown]

On Tue, Jul 14, 2009 at 08:04:08PM -0500, Candelaria Villareal, Jorge wrote:
> Board sdp3430 has hardware support for EXTMUTE using TWL4030 GPIO6
> line, controlled by register INTBR_PMBR1. Machine driver takes care
> of enabling gpio line through i2c and codec driver manipulates the
> line during headset ramp up/down sequence.

> Signed-off-by: Jorge Eduardo Candelaria <x0107209@ti.com>

Applied, thanks.

Re: [PATCH] ASoC: SDP3430: Add support for EXTMUTE using TWL GPIO6

[Peter Ujfalusi]

On Wednesday 15 July 2009 04:04:08 ext Candelaria Villareal, Jorge wrote:
> Board sdp3430 has hardware support for EXTMUTE using TWL4030 GPIO6
> line, controlled by register INTBR_PMBR1. Machine driver takes care
> of enabling gpio line through i2c and codec driver manipulates the
> line during headset ramp up/down sequence.
>
> +#define TWL4030_INTBR_PMBR1	0x0D
> +#define EXTMUTE(value)		(value << 2)

This is confusing, should it be something like this?:
#define GPIO6_PWM0_MUX(value)		(value << 2)

> +	/* Set TWL4030 GPIO6 as EXTMUTE signal */
> +	twl4030_i2c_write_u8(TWL4030_MODULE_INTBR, EXTMUTE(0x02),
> +							TWL4030_MODULE_INTBR);

I think you meant this:
twl4030_i2c_write_u8(TWL4030_MODULE_INTBR, EXTMUTE(0x02),
							TWL4030_INTBR_PMBR1);

Furthermore, since you are modifying pin muxing register, I think it is safer 
to do something like this here to avoid changing other pin's function:

u8 pin_mux;

twl4030_i2c_read_u8(TWL4030_MODULE_INTBR, &pin_mux,
							TWL4030_INTBR_PMBR1);
pin_mux &= ~GPIO6_PWM0_MUX(0x3);
pin_mux |= GPIO6_PWM0_MUX(0x2);
twl4030_i2c_write_u8(TWL4030_MODULE_INTBR, pin_mux,
							TWL4030_INTBR_PMBR1);


-- 
Péter

Re: [PATCH] ASoC: SDP3430: Add support for EXTMUTE using TWL GPIO6

[Candelaria Villareal, Jorge]

> On Wednesday 15 July 2009 04:04:08 ext Candelaria Villareal, 
> Jorge wrote:
> > Board sdp3430 has hardware support for EXTMUTE using TWL4030 GPIO6
> > line, controlled by register INTBR_PMBR1. Machine driver takes care
> > of enabling gpio line through i2c and codec driver manipulates the
> > line during headset ramp up/down sequence.
> >
> > +#define TWL4030_INTBR_PMBR1	0x0D
> > +#define EXTMUTE(value)		(value << 2)
> 
> This is confusing, should it be something like this?:
> #define GPIO6_PWM0_MUX(value)		(value << 2)
> 
> > +	/* Set TWL4030 GPIO6 as EXTMUTE signal */
> > +	twl4030_i2c_write_u8(TWL4030_MODULE_INTBR, EXTMUTE(0x02),
> > +							
> TWL4030_MODULE_INTBR);
> 
> I think you meant this:
> twl4030_i2c_write_u8(TWL4030_MODULE_INTBR, EXTMUTE(0x02),
> 							
> TWL4030_INTBR_PMBR1);

You are right, I don't know how I missed that. I will change that.

> 
> Furthermore, since you are modifying pin muxing register, I 
> think it is safer 
> to do something like this here to avoid changing other pin's function:
> 
> u8 pin_mux;
> 
> twl4030_i2c_read_u8(TWL4030_MODULE_INTBR, &pin_mux,
> 							
> TWL4030_INTBR_PMBR1);
> pin_mux &= ~GPIO6_PWM0_MUX(0x3);
> pin_mux |= GPIO6_PWM0_MUX(0x2);
> twl4030_i2c_write_u8(TWL4030_MODULE_INTBR, pin_mux,
> 							
> TWL4030_INTBR_PMBR1);

Since this patch is already on branch "for-2.6.32", should I submit a new patch for this changes?  Ir should I first revert and submit the modified patch?

Re: [PATCH] ASoC: SDP3430: Add support for EXTMUTE using TWL GPIO6

[Mark Brown]

On 6 Aug 2009, at 16:20, "Candelaria Villareal, Jorge"  
<x0107209@ti.com> wrote:

>> On Wednesday 15 July 2009 04:04:08 ext Candelaria Villareal,
>> Jorge wrote:
>>> Board sdp3430 has hardware support for EXTMUTE using TWL4030 GPIO6
>>> line, controlled by register INTBR_PMBR1. Machine driver takes care
>>> of enabling gpio line through i2c and codec driver manipulates the
>>> line during headset ramp up/down sequence.
>>>
>>> +#define TWL4030_INTBR_PMBR1    0x0D
>>> +#define EXTMUTE(value)        (value << 2)
>>
>> This is confusing, should it be something like this?:
>> #define GPIO6_PWM0_MUX(value)        (value << 2)
>>
>>> +    /* Set TWL4030 GPIO6 as EXTMUTE signal */
>>> +    twl4030_i2c_write_u8(TWL4030_MODULE_INTBR, EXTMUTE(0x02),
>>> +
>> TWL4030_MODULE_INTBR);
>>
>> I think you meant this:
>> twl4030_i2c_write_u8(TWL4030_MODULE_INTBR, EXTMUTE(0x02),
>>
>> TWL4030_INTBR_PMBR1);
>
> You are right, I don't know how I missed that. I will change that.
>
>>
>> Furthermore, since you are modifying pin muxing register, I
>> think it is safer
>> to do something like this here to avoid changing other pin's  
>> function:
>>
>> u8 pin_mux;
>>
>> twl4030_i2c_read_u8(TWL4030_MODULE_INTBR, &pin_mux,
>>
>> TWL4030_INTBR_PMBR1);
>> pin_mux &= ~GPIO6_PWM0_MUX(0x3);
>> pin_mux |= GPIO6_PWM0_MUX(0x2);
>> twl4030_i2c_write_u8(TWL4030_MODULE_INTBR, pin_mux,
>>
>> TWL4030_INTBR_PMBR1);
>
> Since this patch is already on branch "for-2.6.32", should I submit  
> a new patch for this changes?  Ir should I first revert and submit  
> the modified patch?

Incremental platch, please.

Re: [PATCH] ASoC: SDP3430: Add support for EXTMUTE using TWL GPIO6

[Peter Ujfalusi]

On Tuesday 04 August 2009 09:58:15 Ujfalusi Peter (Nokia-D/Tampere) wrote:
> On Wednesday 15 July 2009 04:04:08 ext Candelaria Villareal, Jorge wrote:
> > Board sdp3430 has hardware support for EXTMUTE using TWL4030 GPIO6
> > line, controlled by register INTBR_PMBR1. Machine driver takes care
> > of enabling gpio line through i2c and codec driver manipulates the
> > line during headset ramp up/down sequence.
> >
> > +#define TWL4030_INTBR_PMBR1	0x0D
> > +#define EXTMUTE(value)		(value << 2)
>
> This is confusing, should it be something like this?:
> #define GPIO6_PWM0_MUX(value)		(value << 2)

If you intend to change this, than GPIO6_PWM0_MUX sounds weird as well, in this 
way it should be GPIO6_PWM0_EXTMUTE_MUX, which not so nice...
TWL4030_GPIO6_MUX() seams much better...

-- 
Péter

Re: [PATCH] ASoC: SDP3430: Add support for EXTMUTE using TWL GPIO6

[Candelaria Villareal, Jorge]

 

> -----Original Message-----
> From: Peter Ujfalusi [mailto:peter.ujfalusi@nokia.com] 
> Sent: Friday, August 07, 2009 12:52 AM
> To: alsa-devel@alsa-project.org
> Cc: Candelaria Villareal, Jorge; broonie@opensource.wolfsonmicro.com
> Subject: Re: [alsa-devel] [PATCH] ASoC: SDP3430: Add support 
> for EXTMUTE using TWL GPIO6
> 
> On Tuesday 04 August 2009 09:58:15 Ujfalusi Peter 
> (Nokia-D/Tampere) wrote:
> > On Wednesday 15 July 2009 04:04:08 ext Candelaria 
> Villareal, Jorge wrote:
> > > Board sdp3430 has hardware support for EXTMUTE using TWL4030 GPIO6
> > > line, controlled by register INTBR_PMBR1. Machine driver 
> takes care
> > > of enabling gpio line through i2c and codec driver manipulates the
> > > line during headset ramp up/down sequence.
> > >
> > > +#define TWL4030_INTBR_PMBR1	0x0D
> > > +#define EXTMUTE(value)		(value << 2)
> >
> > This is confusing, should it be something like this?:
> > #define GPIO6_PWM0_MUX(value)		(value << 2)
> 
> If you intend to change this, than GPIO6_PWM0_MUX sounds 
> weird as well, in this 
> way it should be GPIO6_PWM0_EXTMUTE_MUX, which not so nice...
> TWL4030_GPIO6_MUX() seams much better...
> 

How about naming the field as it is named in TRM, GPIO6_PWM0_MUTE.

That way, if there is any confusion TWL4030 TRM should be able to
clear any doubts. Also, it states clearly whicho mode correspond to
each value.