[PATCH v2 1/2] ASoC: tlv320aic32x4: Fix problem with first capture.

[PATCH v2 1/2] ASoC: tlv320aic32x4: Fix problem with first capture.

[Javier Martin]

In its previous status, the first capture didn't work properly;
nothing was actually recorded from the microphone. This
behaviour was observed using a Visstrim M10 board.

In order to solve this BUG a workaround has been added that,
during the initialization process of the codec, powers on and
off the ADC.

The issue seems related to a HW BUG or some behavior that
is not documented in the datasheet.

Signed-off-by: Javier Martin <javier.martin@vista-silicon.com>
---
Changes since v1:
 - Use a subject line according to the ASoC subsystem.

---
 sound/soc/codecs/tlv320aic32x4.c |   10 ++++++++++
 sound/soc/codecs/tlv320aic32x4.h |    3 +++
 2 files changed, 13 insertions(+)

diff --git a/sound/soc/codecs/tlv320aic32x4.c b/sound/soc/codecs/tlv320aic32x4.c
index b0a73d3..aad92f9 100644
--- a/sound/soc/codecs/tlv320aic32x4.c
+++ b/sound/soc/codecs/tlv320aic32x4.c
@@ -675,6 +675,16 @@ static int aic32x4_probe(struct snd_soc_codec *codec)
 			     ARRAY_SIZE(aic32x4_snd_controls));
 	aic32x4_add_widgets(codec);
 
+	/*
+	 * Workaround: for an unknown reason, the ADC needs to be powered up
+	 * and down for the first capture to work properly. It seems related to
+	 * a HW BUG or some kind of behavior not documented in the datasheet.
+	 */
+	tmp_reg = snd_soc_read(codec, AIC32X4_ADCSETUP);
+	snd_soc_write(codec, AIC32X4_ADCSETUP, tmp_reg |
+				AIC32X4_LADC_EN | AIC32X4_RADC_EN);
+	snd_soc_write(codec, AIC32X4_ADCSETUP, tmp_reg);
+
 	return 0;
 }
 
diff --git a/sound/soc/codecs/tlv320aic32x4.h b/sound/soc/codecs/tlv320aic32x4.h
index aae2b24..3577422 100644
--- a/sound/soc/codecs/tlv320aic32x4.h
+++ b/sound/soc/codecs/tlv320aic32x4.h
@@ -94,6 +94,9 @@
 #define AIC32X4_WORD_LEN_24BITS		0x02
 #define AIC32X4_WORD_LEN_32BITS		0x03
 
+#define AIC32X4_LADC_EN			(1 << 7)
+#define AIC32X4_RADC_EN			(1 << 6)
+
 #define AIC32X4_I2S_MODE		0x00
 #define AIC32X4_DSP_MODE		0x01
 #define AIC32X4_RIGHT_JUSTIFIED_MODE	0x02
-- 
1.7.9.5

[PATCH v2 2/2] ASoC: tlv320aic32x4: Add rstn gpio to platform data.

[Javier Martin]

Add the possibility to specify a gpio through platform data
so that a HW reset can be issued to the codec.

Signed-off-by: Javier Martin <javier.martin@vista-silicon.com>
---
Changes since v1:
 - Use a subject line according to the ASoC subsystem.
 - Use 'devm_gpio_request_one()'
 - Provide correct default value to the GPIO.
---
 include/sound/tlv320aic32x4.h    |    1 +
 sound/soc/codecs/tlv320aic32x4.c |   14 ++++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/include/sound/tlv320aic32x4.h b/include/sound/tlv320aic32x4.h
index c009f70..24e5d99 100644
--- a/include/sound/tlv320aic32x4.h
+++ b/include/sound/tlv320aic32x4.h
@@ -26,6 +26,7 @@ struct aic32x4_pdata {
 	u32 power_cfg;
 	u32 micpga_routing;
 	bool swapdacs;
+	int rstn_gpio;
 };
 
 #endif
diff --git a/sound/soc/codecs/tlv320aic32x4.c b/sound/soc/codecs/tlv320aic32x4.c
index aad92f9..f188aa4 100644
--- a/sound/soc/codecs/tlv320aic32x4.c
+++ b/sound/soc/codecs/tlv320aic32x4.c
@@ -28,6 +28,7 @@
 #include <linux/init.h>
 #include <linux/delay.h>
 #include <linux/pm.h>
+#include <linux/gpio.h>
 #include <linux/i2c.h>
 #include <linux/cdev.h>
 #include <linux/slab.h>
@@ -65,6 +66,7 @@ struct aic32x4_priv {
 	u32 power_cfg;
 	u32 micpga_routing;
 	bool swapdacs;
+	int rstn_gpio;
 };
 
 /* 0dB min, 1dB steps */
@@ -627,10 +629,20 @@ static int aic32x4_probe(struct snd_soc_codec *codec)
 {
 	struct aic32x4_priv *aic32x4 = snd_soc_codec_get_drvdata(codec);
 	u32 tmp_reg;
+	int ret;
 
 	codec->hw_write = (hw_write_t) i2c_master_send;
 	codec->control_data = aic32x4->control_data;
 
+	if (aic32x4->rstn_gpio >= 0) {
+		ret = devm_gpio_request_one(codec->dev, aic32x4->rstn_gpio,
+				GPIOF_OUT_INIT_LOW, "tlv320aic32x4 rstn");
+		if (ret != 0)
+			return ret;
+		ndelay(10);
+		gpio_set_value(aic32x4->rstn_gpio, 1);
+	}
+
 	snd_soc_write(codec, AIC32X4_RESET, 0x01);
 
 	/* Power platform configuration */
@@ -723,10 +735,12 @@ static __devinit int aic32x4_i2c_probe(struct i2c_client *i2c,
 		aic32x4->power_cfg = pdata->power_cfg;
 		aic32x4->swapdacs = pdata->swapdacs;
 		aic32x4->micpga_routing = pdata->micpga_routing;
+		aic32x4->rstn_gpio = pdata->rstn_gpio;
 	} else {
 		aic32x4->power_cfg = 0;
 		aic32x4->swapdacs = false;
 		aic32x4->micpga_routing = 0;
+		aic32x4->rstn_gpio = -1;
 	}
 
 	ret = snd_soc_register_codec(&i2c->dev,
-- 
1.7.9.5

Re: [PATCH v2 2/2] ASoC: tlv320aic32x4: Add rstn gpio to platform data.

[Mike Looijmans]

On 10/31/2012 11:53 AM, Javier Martin wrote:
> Add the possibility to specify a gpio through platform data
> so that a HW reset can be issued to the codec.
>
> Signed-off-by: Javier Martin <javier.martin@vista-silicon.com>
> ---
> Changes since v1:
>   - Use a subject line according to the ASoC subsystem.
>   - Use 'devm_gpio_request_one()'
>   - Provide correct default value to the GPIO.
>

Now that I see your patch - I modelled the RESET pins (and the power 
supply pins) as power supply inputs, and request them to be active when 
the bias goes into standby or higher. Keeping the chip in RESET is a 
very nice power saving feature. I just cache all the register settings 
up until that point (which required a bit of coding because of the 
"paging" of the I2C address space).

The reason that I also made RESET a "power" rather than a GPIO is that I 
have seven codecs, using three GPIO lines (in 3/3/1 configuration) to 
reset the chips. Having the RESET pin modelled as a powersupply allowed 
the codec instance to blindly turn it on, letting the powersupply keep 
track of the number of clients.

It was the best I could think off at the time, and it made me wonder why 
those reset lines aren't modelled this way more often.

If anyone's interested, I can upload the modified source. A patch is 
difficult, because I did horrible things to the code for our specific needs.

What's your view on this?


Mike.


Met vriendelijke groet / kind regards,

Mike Looijmans

TOPIC Embedded Systems
Eindhovenseweg 32-C, NL-5683 KH Best
Postbus 440, NL-5680 AK Best
Telefoon: (+31) – (0)499 - 33.69.79
Telefax: (+31) - (0)499 - 33.69.70
E-mail: mike.looijmans@topic.nl
Website: www.topic.nl

Dit e-mail bericht en de eventueel daarbij behorende bijlagen zijn uitsluitend bestemd voor de geadresseerde, zoals die blijkt uit het e-mail bericht en/of de bijlagen. Er kunnen gegevens met betrekking tot een derde instaan. Indien u als niet-geadresseerde dit bericht en de bijlagen ontvangt, terwijl u niet bevoegd of gemachtigd bent om dit bericht namens de geadresseerde te ontvangen, wordt u verzocht de afzender hierover direct te informeren en het e-mail bericht met de bijlagen te vernietigen. Ieder gebruik van de inhoud van het e-mail bericht, waaronder de daarbij behorende bijlagen, door een ander dan de geadresseerde is onrechtmatig jegens ons dan wel de eventueel in het e-mail bericht of de bijlagen voorkomende andere personen. TOPIC Embedded Systems is niet aansprakelijk voor enigerlei schade voortvloeiend uit het gebruik en/of acceptatie van dit e-mail bericht of de daarbij behorende bijlagen.

The contents of this message, as well as any enclosures, are addressed personally to, and thus solely intended for the addressee. They may contain information regarding a third party. A recipient who is neither the addressee, nor empowered to receive this message on behalf of the addressee, is kindly requested to immediately inform the sender of receipt, and to destroy the message and the enclosures. Any use of the contents of this message and/or the enclosures by any other person than the addressee or person who is empowered to receive this message, is illegal towards the sender and/or the aforementioned third party. TOPIC Embedded Systems is not  liable for any damage as a result of the use and/or acceptance of this message and as well as any enclosures.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH v2 2/2] ASoC: tlv320aic32x4: Add rstn gpio to platform data.

[Mark Brown]

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

On Wed, Oct 31, 2012 at 03:40:17PM +0100, Mike Looijmans wrote:

> is a very nice power saving feature. I just cache all the register
> settings up until that point (which required a bit of coding because
> of the "paging" of the I2C address space).

Probably easiest to just convert over to regmap, it handles paging fine.

> It was the best I could think off at the time, and it made me wonder
> why those reset lines aren't modelled this way more often.

This breaks down if any of the drivers actually uses the reset pin to
reset the chip - if the reset might not actually happen but the driver
wants it to happen then things are going to go wrong.

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

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

Re: [PATCH v2 1/2] ASoC: tlv320aic32x4: Fix problem with first capture.

[Mark Brown]

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

On Wed, Oct 31, 2012 at 11:53:33AM +0100, Javier Martin wrote:
> In its previous status, the first capture didn't work properly;
> nothing was actually recorded from the microphone. This
> behaviour was observed using a Visstrim M10 board.

Applied both, thanks.

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

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

Re: [PATCH v2 2/2] ASoC: tlv320aic32x4: Add rstn gpio to platform data.

[Mike Looijmans]

On 11/01/2012 03:35 PM, Mark Brown wrote:
>
>> is a very nice power saving feature. I just cache all the register
>> settings up until that point (which required a bit of coding because
>> of the "paging" of the I2C address space).
> Probably easiest to just convert over to regmap, it handles paging fine.
I've backported the driver to 2.6.37 so probably my regmap wasn't smart 
enough yet.

>
>> It was the best I could think off at the time, and it made me wonder
>> why those reset lines aren't modelled this way more often.
> This breaks down if any of the drivers actually uses the reset pin to
> reset the chip - if the reset might not actually happen but the driver
> wants it to happen then things are going to go wrong.

Good point. But in that case, the gpio cannot be "optional", the board 
file MUST supply it.

Most chips - if not all, and this one certainly has it - also have a 
software routine for resetting it.

 From a hardware perspective, when using two codecs to get a 4-channel 
input, it makes perfect sense to use the same GPIO line to reset them 
both. This will never work with two instances of a driver that wants to 
control the GPIO line directly, but it works flawlessly when that line 
is modelled as a power supply. Each instance can request the reset to be 
relaxed, and each instance gets feedback from the supply core when the 
reset line is actually being altered (which can for example be used to 
update the codec's registers from the cache, or to indicate that the 
chip is no longer online and every write must go to the cache only).


Do I understand correctly that I'm totally alone in this and that there 
is no point in uploading the code?


Met vriendelijke groet / kind regards,

Mike Looijmans

TOPIC Embedded Systems
Eindhovenseweg 32-C, NL-5683 KH Best
Postbus 440, NL-5680 AK Best
Telefoon: (+31) – (0)499 - 33.69.79
Telefax: (+31) - (0)499 - 33.69.70
E-mail: mike.looijmans@topic.nl
Website: www.topic.nl

Dit e-mail bericht en de eventueel daarbij behorende bijlagen zijn uitsluitend bestemd voor de geadresseerde, zoals die blijkt uit het e-mail bericht en/of de bijlagen. Er kunnen gegevens met betrekking tot een derde instaan. Indien u als niet-geadresseerde dit bericht en de bijlagen ontvangt, terwijl u niet bevoegd of gemachtigd bent om dit bericht namens de geadresseerde te ontvangen, wordt u verzocht de afzender hierover direct te informeren en het e-mail bericht met de bijlagen te vernietigen. Ieder gebruik van de inhoud van het e-mail bericht, waaronder de daarbij behorende bijlagen, door een ander dan de geadresseerde is onrechtmatig jegens ons dan wel de eventueel in het e-mail bericht of de bijlagen voorkomende andere personen. TOPIC Embedded Systems is niet aansprakelijk voor enigerlei schade voortvloeiend uit het gebruik en/of acceptatie van dit e-mail bericht of de daarbij behorende bijlagen.

The contents of this message, as well as any enclosures, are addressed personally to, and thus solely intended for the addressee. They may contain information regarding a third party. A recipient who is neither the addressee, nor empowered to receive this message on behalf of the addressee, is kindly requested to immediately inform the sender of receipt, and to destroy the message and the enclosures. Any use of the contents of this message and/or the enclosures by any other person than the addressee or person who is empowered to receive this message, is illegal towards the sender and/or the aforementioned third party. TOPIC Embedded Systems is not  liable for any damage as a result of the use and/or acceptance of this message and as well as any enclosures.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH v2 2/2] ASoC: tlv320aic32x4: Add rstn gpio to platform data.

[Mark Brown]

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

On Thu, Nov 01, 2012 at 04:28:59PM +0100, Mike Looijmans wrote:

> >This breaks down if any of the drivers actually uses the reset pin to
> >reset the chip - if the reset might not actually happen but the driver
> >wants it to happen then things are going to go wrong.

> Good point. But in that case, the gpio cannot be "optional", the
> board file MUST supply it.

Well, what normally happens is that the driver has some alternative
ways of doing what it wants but if there's a reset signal present it
wants to actually use it and be able to rely on it doing the job.

> Do I understand correctly that I'm totally alone in this and that
> there is no point in uploading the code?

If you want to do something like this you should explictly model reset
lines like this rather than shoehorning them into a different subsystem.

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

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

Re: [PATCH v2 2/2] ASoC: tlv320aic32x4: Add rstn gpio to platform data.

[javier Martin]

Hi,

On 1 November 2012 15:35, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Oct 31, 2012 at 03:40:17PM +0100, Mike Looijmans wrote:
>
>> is a very nice power saving feature. I just cache all the register
>> settings up until that point (which required a bit of coding because
>> of the "paging" of the I2C address space).
>
> Probably easiest to just convert over to regmap, it handles paging fine.

this is something I'd like to do when I have some spare time. Do you
know any example of a codec driver that uses a similar approach for
accessing registers?



-- 
Javier Martin
Vista Silicon S.L.
CDTUC - FASE C - Oficina S-345
Avda de los Castros s/n
39005- Santander. Cantabria. Spain
+34 942 25 32 60
www.vista-silicon.com

Re: [PATCH v2 2/2] ASoC: tlv320aic32x4: Add rstn gpio to platform data.

[Mark Brown]

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

On Mon, Nov 05, 2012 at 09:14:41AM +0100, javier Martin wrote:
> On 1 November 2012 15:35, Mark Brown

> > Probably easiest to just convert over to regmap, it handles paging fine.

> this is something I'd like to do when I have some spare time. Do you
> know any example of a codec driver that uses a similar approach for
> accessing registers?

The WM2200 uses paging, and grepping for regmap will turn up a bunch of
other drivers which use it.

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

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