[PATCH 3/5] SOUND: SOC: CODECS: Add support for the TWL4030 audio codec
From: Steve Sakoman <[EMAIL PROTECTED]>
---
sound/soc/codecs/Kconfig |5 +
sound/soc/codecs/Makefile |2 +
sound/soc/codecs/twl4030.c | 628
sound/soc/codecs/twl4030.h | 197 ++
4 files changed, 832 insertions(+), 0 deletions(-)
create mode 100644 sound/soc/codecs/twl4030.c
create mode 100644 sound/soc/codecs/twl4030.h
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 1db04a2..2f00e1e 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -50,3 +50,8 @@ config SND_SOC_CS4270_VD33_ERRATA
config SND_SOC_TLV320AIC3X
tristate
depends on I2C
+
+config SND_SOC_TWL4030
+ tristate
+ depends on SND_SOC && I2C
+
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index d7b97ab..a519ced 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -10,6 +10,7 @@ snd-soc-wm9712-objs := wm9712.o
snd-soc-wm9713-objs := wm9713.o
snd-soc-cs4270-objs := cs4270.o
snd-soc-tlv320aic3x-objs := tlv320aic3x.o
+snd-soc-twl4030-objs := twl4030.o
obj-$(CONFIG_SND_SOC_AC97_CODEC) += snd-soc-ac97.o
obj-$(CONFIG_SND_SOC_AK4535) += snd-soc-ak4535.o
@@ -23,3 +24,4 @@ obj-$(CONFIG_SND_SOC_WM9712) += snd-soc-wm9712.o
obj-$(CONFIG_SND_SOC_WM9713) += snd-soc-wm9713.o
obj-$(CONFIG_SND_SOC_CS4270) += snd-soc-cs4270.o
obj-$(CONFIG_SND_SOC_TLV320AIC3X) += snd-soc-tlv320aic3x.o
+obj-$(CONFIG_SND_SOC_TWL4030) += snd-soc-twl4030.o
diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c
new file mode 100644
index 000..683b8a1
--- /dev/null
+++ b/sound/soc/codecs/twl4030.c
@@ -0,0 +1,628 @@
+/*
+ * ALSA SoC TWL4030 codec driver
+ *
+ * Author: Steve Sakoman, <[EMAIL PROTECTED]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "twl4030.h"
+
+/*
+ * twl4030 register cache & default register settings
+ */
+static const u8 twl4030_reg[TWL4030_CACHEREGNUM] = {
+ 0x00, /* this register not used */
+ 0x93, /* REG_CODEC_MODE (0x1) */
+ 0xc3, /* REG_OPTION (0x2) */
+ 0x00, /* REG_UNKNOWN(0x3) */
+ 0x00, /* REG_MICBIAS_CTL(0x4) */
+ 0x24, /* REG_ANAMICL(0x5) */
+ 0x04, /* REG_ANAMICR(0x6) */
+ 0x0a, /* REG_AVADC_CTL (0x7) */
+ 0x00, /* REG_ADCMICSEL (0x8) */
+ 0x00, /* REG_DIGMIXING (0x9) */
+ 0x0c, /* REG_ATXL1PGA (0xA) */
+ 0x0c, /* REG_ATXR1PGA (0xB) */
+ 0x00, /* REG_AVTXL2PGA (0xC) */
+ 0x00, /* REG_AVTXR2PGA (0xD) */
+ 0x01, /* REG_AUDIO_IF (0xE) */
+ 0x00, /* REG_VOICE_IF (0xF) */
+ 0x00, /* REG_ARXR1PGA (0x10) */
+ 0x00, /* REG_ARXL1PGA (0x11) */
+ 0x6c, /* REG_ARXR2PGA (0x12) */
+ 0x6c, /* REG_ARXL2PGA (0x13) */
+ 0x00, /* REG_VRXPGA (0x14) */
+ 0x00, /* REG_VSTPGA (0x15) */
+ 0x00, /* REG_VRX2ARXPGA (0x16) */
+ 0x0c, /* REG_AVDAC_CTL (0x17) */
+ 0x00, /* REG_ARX2VTXPGA (0x18) */
+ 0x00, /* REG_ARXL1_APGA_CTL (0x19) */
+ 0x00, /* REG_ARXR1_APGA_CTL (0x1A) */
+ 0x4b, /* REG_ARXL2_APGA_CTL (0x1B) */
+ 0x4b, /* REG_ARXR2_APGA_CTL (0x1C) */
+ 0x00, /* REG_ATX2ARXPGA (0x1D) */
+ 0x00, /* REG_BT_IF (0x1E) */
+ 0x00, /* REG_BTPGA (0x1F) */
+ 0x00, /* REG_BTSTPGA(0x20) */
+ 0x00, /* REG_EAR_CTL(0x21) */
+ 0x24, /* REG_HS_SEL (0x22) */
+ 0x0a, /* REG_HS_GAIN_SET(0x23) */
+ 0x00, /* REG_HS_POPN_SET(0x24) */
+ 0x00, /* REG_PREDL_CTL (0x25) */
+ 0x00, /* REG_PREDR_CTL (0x26) */
+ 0x00, /* REG_PRECKL_CTL (0x27) */
+ 0x00, /* REG_PRECKR_CTL (0x28) */
+ 0x00, /* REG_HFL_CTL(0x29) */
+ 0x00, /* REG_HFR_CTL(0x2A) */
+ 0x00, /* REG_ALC_CTL(0x2B) *
Re: [PATCH 3/5] SOUND: SOC: CODECS: Add support for the TWL4030 audio codec
On Wed, 3 Sep 2008 22:01:58 -0700
"ext [EMAIL PROTECTED]" <[EMAIL PROTECTED]> wrote:
> From: Steve Sakoman <[EMAIL PROTECTED]>
>
> ---
> sound/soc/codecs/Kconfig |5 +
> sound/soc/codecs/Makefile |2 +
> sound/soc/codecs/twl4030.c | 628 +++
...
> +static void twl4030_dump_registers(void)
> +{
This is not needed since there is already nice function
for it: sound/soc/soc-core.c: codec_reg_show.
> +static void twl4030_power_up(struct snd_soc_codec *codec)
> +{
> + u8 mode, byte, popn, hsgain;
> +
...
> + /* wait for offset cancellation to complete */
> + twl4030_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE, &byte,
> REG_ANAMICL);
> + while ((byte & CNCL_OFFSET_START) == CNCL_OFFSET_START)
> + twl4030_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE,
> + &byte, REG_ANAMICL);
> +
Probably some timeout escape here.
> +static void twl4030_power_down(struct snd_soc_codec *codec)
> +{
...
> + udelay(10);
> +}
REVISIT comment for these kind of magic delays if doesn't work without.
> +static int twl4030_hw_params(struct snd_pcm_substream *substream,
> +struct snd_pcm_hw_params *params)
> +{
...
> + switch (params_rate(params)) {
> + case 44100:
> + mode |= APLL_RATE_44100;
> + break;
> + case 48000:
> + mode |= APLL_RATE_48000;
> + break;
> + default:
> + printk(KERN_ERR "TWL4030 hw params: unknown rate %d
> \n",
> + params_rate(params));
> + return -EINVAL;
> + }
I checked that chip supports also other standard rates from 8 kHz,
11.025 kHz, etc. However I didn't find from quick look how this relates
to interface rate. I would say that small TODO comment to whom who has
platform to try these combinations would be nice.
> +static int twl4030_set_dai_sysclk(struct snd_soc_dai *codec_dai,
> + int clk_id, unsigned int freq, int dir)
> +{
...
> +
> + infreq |= APLL_EN;
> + twl4030_write(codec, REG_APLL_CTL, infreq);
> +
> + return 0;
> +}
If this actually place for set_pll callback if one wants to manage PLL
(APLL_EN bit) dynamically?
Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] SOUND: SOC: CODECS: Add support for the TWL4030 audio codec
On Thu, Sep 4, 2008 at 3:26 AM, Jarkko Nikula <[EMAIL PROTECTED]> wrote:
> On Wed, 3 Sep 2008 22:01:58 -0700
> "ext [EMAIL PROTECTED]" <[EMAIL PROTECTED]> wrote:
>> +static void twl4030_dump_registers(void)
>> +{
> This is not needed since there is already nice function
> for it: sound/soc/soc-core.c: codec_reg_show.
I could probably get rid of this function. It was quite useful during
debugging and I was not aware of codec_reg_show. IIRC, most of the
other codec drivers also have the equivalent of this function, so we
might want to clean them up too if there is a standard function to
replace them.
>> + /* wait for offset cancellation to complete */
>> + twl4030_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE, &byte,
>> REG_ANAMICL);
>> + while ((byte & CNCL_OFFSET_START) == CNCL_OFFSET_START)
>> + twl4030_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE,
>> + &byte, REG_ANAMICL);
>> +
> Probably some timeout escape here.
Right you are! This is a pet peeve of mine and I am humiliated I let
this sneak in :-)
>
>> +static void twl4030_power_down(struct snd_soc_codec *codec)
>> +{
> ...
>> + udelay(10);
>> +}
> REVISIT comment for these kind of magic delays if doesn't work without.
It *seems* to work without them, but every historic TI driver seemed
to have them. I figured that they might know something not reflected
in the documentation. I will add a REVIST comment.
>> +static int twl4030_hw_params(struct snd_pcm_substream *substream,
>> +struct snd_pcm_hw_params *params)
>> +{
> ...
>> + switch (params_rate(params)) {
>> + case 44100:
>> + mode |= APLL_RATE_44100;
>> + break;
>> + case 48000:
>> + mode |= APLL_RATE_48000;
>> + break;
>> + default:
>> + printk(KERN_ERR "TWL4030 hw params: unknown rate %d
>> \n",
>> + params_rate(params));
>> + return -EINVAL;
>> + }
> I checked that chip supports also other standard rates from 8 kHz,
> 11.025 kHz, etc. However I didn't find from quick look how this relates
> to interface rate. I would say that small TODO comment to whom who has
> platform to try these combinations would be nice.
It would be trivial to add support for the other data rates. IIRC, I
just matched the data rates that were supported by mcbsp.
>> +static int twl4030_set_dai_sysclk(struct snd_soc_dai *codec_dai,
>> + int clk_id, unsigned int freq, int dir)
>> +{
> ...
>> +
>> + infreq |= APLL_EN;
>> + twl4030_write(codec, REG_APLL_CTL, infreq);
>> +
>> + return 0;
>> +}
> If this actually place for set_pll callback if one wants to manage PLL
> (APLL_EN bit) dynamically?
I can add a TODO comment for this. I'm under delivery pressure for
the next few weeks and won't get time to work on this.
Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] SOUND: SOC: CODECS: Add support for the TWL4030 audio codec
From: Steve Sakoman <[EMAIL PROTECTED]>
SOUND: SOC: CODECS: Add support for the TWL4030 audio codec
This version should take into account all feedback received to date
Signed-off-by: Steve Sakoman <[EMAIL PROTECTED]>
---
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 1db04a2..2f00e1e 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -50,3 +50,8 @@ config SND_SOC_CS4270_VD33_ERRATA
config SND_SOC_TLV320AIC3X
tristate
depends on I2C
+
+config SND_SOC_TWL4030
+ tristate
+ depends on SND_SOC && I2C
+
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index d7b97ab..a519ced 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -10,6 +10,7 @@ snd-soc-wm9712-objs := wm9712.o
snd-soc-wm9713-objs := wm9713.o
snd-soc-cs4270-objs := cs4270.o
snd-soc-tlv320aic3x-objs := tlv320aic3x.o
+snd-soc-twl4030-objs := twl4030.o
obj-$(CONFIG_SND_SOC_AC97_CODEC) += snd-soc-ac97.o
obj-$(CONFIG_SND_SOC_AK4535) += snd-soc-ak4535.o
@@ -23,3 +24,4 @@ obj-$(CONFIG_SND_SOC_WM9712) += snd-soc-wm9712.o
obj-$(CONFIG_SND_SOC_WM9713) += snd-soc-wm9713.o
obj-$(CONFIG_SND_SOC_CS4270) += snd-soc-cs4270.o
obj-$(CONFIG_SND_SOC_TLV320AIC3X) += snd-soc-tlv320aic3x.o
+obj-$(CONFIG_SND_SOC_TWL4030) += snd-soc-twl4030.o
diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c
new file mode 100644
index 000..ee2f0d3
--- /dev/null
+++ b/sound/soc/codecs/twl4030.c
@@ -0,0 +1,653 @@
+/*
+ * ALSA SoC TWL4030 codec driver
+ *
+ * Author: Steve Sakoman, <[EMAIL PROTECTED]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "twl4030.h"
+
+/*
+ * twl4030 register cache & default register settings
+ */
+static const u8 twl4030_reg[TWL4030_CACHEREGNUM] = {
+ 0x00, /* this register not used */
+ 0x93, /* REG_CODEC_MODE (0x1) */
+ 0xc3, /* REG_OPTION (0x2) */
+ 0x00, /* REG_UNKNOWN(0x3) */
+ 0x00, /* REG_MICBIAS_CTL(0x4) */
+ 0x24, /* REG_ANAMICL(0x5) */
+ 0x04, /* REG_ANAMICR(0x6) */
+ 0x0a, /* REG_AVADC_CTL (0x7) */
+ 0x00, /* REG_ADCMICSEL (0x8) */
+ 0x00, /* REG_DIGMIXING (0x9) */
+ 0x0c, /* REG_ATXL1PGA (0xA) */
+ 0x0c, /* REG_ATXR1PGA (0xB) */
+ 0x00, /* REG_AVTXL2PGA (0xC) */
+ 0x00, /* REG_AVTXR2PGA (0xD) */
+ 0x01, /* REG_AUDIO_IF (0xE) */
+ 0x00, /* REG_VOICE_IF (0xF) */
+ 0x00, /* REG_ARXR1PGA (0x10) */
+ 0x00, /* REG_ARXL1PGA (0x11) */
+ 0x6c, /* REG_ARXR2PGA (0x12) */
+ 0x6c, /* REG_ARXL2PGA (0x13) */
+ 0x00, /* REG_VRXPGA (0x14) */
+ 0x00, /* REG_VSTPGA (0x15) */
+ 0x00, /* REG_VRX2ARXPGA (0x16) */
+ 0x0c, /* REG_AVDAC_CTL (0x17) */
+ 0x00, /* REG_ARX2VTXPGA (0x18) */
+ 0x00, /* REG_ARXL1_APGA_CTL (0x19) */
+ 0x00, /* REG_ARXR1_APGA_CTL (0x1A) */
+ 0x4b, /* REG_ARXL2_APGA_CTL (0x1B) */
+ 0x4b, /* REG_ARXR2_APGA_CTL (0x1C) */
+ 0x00, /* REG_ATX2ARXPGA (0x1D) */
+ 0x00, /* REG_BT_IF (0x1E) */
+ 0x00, /* REG_BTPGA (0x1F) */
+ 0x00, /* REG_BTSTPGA(0x20) */
+ 0x00, /* REG_EAR_CTL(0x21) */
+ 0x24, /* REG_HS_SEL (0x22) */
+ 0x0a, /* REG_HS_GAIN_SET(0x23) */
+ 0x00, /* REG_HS_POPN_SET(0x24) */
+ 0x00, /* REG_PREDL_CTL (0x25) */
+ 0x00, /* REG_PREDR_CTL (0x26) */
+ 0x00, /* REG_PRECKL_CTL (0x27) */
+ 0x00, /* REG_PRECKR_CTL (0x28) */
+ 0x00, /* REG_HFL_CTL(0x29) */
+ 0x00, /* REG_HFR_CTL(0x2A) */
+ 0x00, /* REG_ALC_CTL(0x2B) */
+ 0x00, /* REG_ALC_SET1 (0x2C) */
+ 0x00, /* REG_ALC_SET2 (0x2D) */
+ 0x00, /* REG_BOOST_CTL (0x2E) */
+ 0x01, /*
Re: [alsa-devel] [PATCH 3/5] SOUND: SOC: CODECS: Add support for the TWL4030 audio codec
On Wed, Sep 03, 2008 at 10:01:58PM -0700, [EMAIL PROTECTED] wrote:
> +static void twl4030_power_up(struct snd_soc_codec *codec)
> +{
> + u8 mode, byte, popn, hsgain;
> +
> + /* set CODECPDZ to turn on codec */
> + mode = twl4030_read_reg_cache(codec, REG_CODEC_MODE);
> + twl4030_write(codec, REG_CODEC_MODE, mode | CODECPDZ);
> + udelay(10);
> +
> + /* initiate offset cancellation */
> + twl4030_write(codec, REG_ANAMICL,
> + twl4030_reg[REG_ANAMICL] | CNCL_OFFSET_START);
It looks a bit odd to see this reading the register defaults rather than
the register cache.
> + /* wait for offset cancellation to complete */
> + twl4030_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE, &byte, REG_ANAMICL);
> + while ((byte & CNCL_OFFSET_START) == CNCL_OFFSET_START)
> + twl4030_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE,
> + &byte, REG_ANAMICL);
How long is this likely to take? If it might take a while then putting
a delay in between reads might be nice. As Jarkko said, a timeout would
be a good idea too in case something went wrong.
> + /* anti-pop when changing analog gain */
> + twl4030_write(codec, REG_MISC_SET_1,
> + twl4030_reg[REG_MISC_SET_1] | SMOOTH_ANAVOL_EN);
Another one reading the register defaults rather than register cache...
> + /* disable bias out */
> + popn &= ~VMID_EN;
> + twl4030_write(codec, REG_HS_POPN_SET, popn);
...
> + case SND_SOC_BIAS_ON:
> + twl4030_power_up(codec);
> + break;
> + case SND_SOC_BIAS_PREPARE:
> + break;
> + case SND_SOC_BIAS_STANDBY:
> + twl4030_power_down(codec);
> + break;
Normally SND_SOC_BIAS_STANDBY would leave Vmid enabled but your power
down function turns it off. The normal thing here is to have standby
bring the codec up and then apply relatively small tweaks in _ON and
_PREPARE that do things like improve the quality of the reference
voltages. Equally well, since the driver does not yet implement DAPM
support doing this will give you power savings that you wouldn't
otherwise get.
Does the codec have any bypass paths that can be set up? If not it
shouldn't do much harm either way until you implement DAPM.
> +/* AUDIO_IF (0x0E) Fields */
> +
> +#define AIF_SLAVE_EN 0x80
> +#define DATA_WIDTH 0x60
These should really all get namespaced - some of them look relatively
likely to collide with registers for CPU side stuff.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH 3/5] SOUND: SOC: CODECS: Add support for the TWL4030 audio codec
On Thu, Sep 04, 2008 at 01:26:32PM +0300, Jarkko Nikula wrote:
> > +static void twl4030_dump_registers(void)
> > +{
> This is not needed since there is already nice function
> for it: sound/soc/soc-core.c: codec_reg_show.
It's doing something slighly different and dumping the chip registers
rather than the register cache (the read() operation for this chip is
_read_reg_cache()) but yes, should be sensible to drop it.
> > +static void twl4030_power_down(struct snd_soc_codec *codec)
> > +{
> ...
> > + udelay(10);
> > +}
> REVISIT comment for these kind of magic delays if doesn't work without.
Alternatively, if writes to CODECPDZ are always supposed to have delays
after them according to the datasheet perhaps a function to wrap up the
write plus delay would be a good idea?
> > +static int twl4030_set_dai_sysclk(struct snd_soc_dai *codec_dai,
> > + int clk_id, unsigned int freq, int dir)
> > +{
> ...
> > +
> > + infreq |= APLL_EN;
> > + twl4030_write(codec, REG_APLL_CTL, infreq);
> > +
> > + return 0;
> > +}
> If this actually place for set_pll callback if one wants to manage PLL
> (APLL_EN bit) dynamically?
Might be nice, yes. That said, how much flexibility is there regarding
the PLL on this part? If the configuration is determined very strongly
by the audio configuration and it's likely that the configuration is
entirely determined by the current audio status then it may not be worth
exposing the decision making to the user, at least for a first pass.
The impression I got from the code was that it was very fixed purpose.
If you do do this then make sure it interacts well with power_up() and
power_down().
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH 3/5] SOUND: SOC: CODECS: Add support for the TWL4030 audio codec
On Thu, Sep 4, 2008 at 5:04 AM, Mark Brown <[EMAIL PROTECTED]> wrote: > On Wed, Sep 03, 2008 at 10:01:58PM -0700, [EMAIL PROTECTED] wrote: >> + /* initiate offset cancellation */ >> + twl4030_write(codec, REG_ANAMICL, >> + twl4030_reg[REG_ANAMICL] | CNCL_OFFSET_START); > > It looks a bit odd to see this reading the register defaults rather than > the register cache. True. I will fix this. > >> + /* wait for offset cancellation to complete */ >> + twl4030_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE, &byte, REG_ANAMICL); >> + while ((byte & CNCL_OFFSET_START) == CNCL_OFFSET_START) >> + twl4030_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE, >> + &byte, REG_ANAMICL); > > How long is this likely to take? If it might take a while then putting > a delay in between reads might be nice. As Jarkko said, a timeout would > be a good idea too in case something went wrong. It happens quite quickly. I'll add an exit to the loop and measure how many passes it takes. If it is a large number I'll add a delay. >> + /* anti-pop when changing analog gain */ >> + twl4030_write(codec, REG_MISC_SET_1, >> + twl4030_reg[REG_MISC_SET_1] | SMOOTH_ANAVOL_EN); > > Another one reading the register defaults rather than register cache... I'll fix this. >> + /* disable bias out */ >> + popn &= ~VMID_EN; >> + twl4030_write(codec, REG_HS_POPN_SET, popn); > > ... > >> + case SND_SOC_BIAS_ON: >> + twl4030_power_up(codec); >> + break; >> + case SND_SOC_BIAS_PREPARE: >> + break; >> + case SND_SOC_BIAS_STANDBY: >> + twl4030_power_down(codec); >> + break; > > Normally SND_SOC_BIAS_STANDBY would leave Vmid enabled but your power > down function turns it off. The normal thing here is to have standby > bring the codec up and then apply relatively small tweaks in _ON and > _PREPARE that do things like improve the quality of the reference > voltages. Equally well, since the driver does not yet implement DAPM > support doing this will give you power savings that you wouldn't > otherwise get. > > Does the codec have any bypass paths that can be set up? If not it > shouldn't do much harm either way until you implement DAPM. How about I add a "TODO" on this? I'm swamped for the next few weeks and won't be able to give it the attention it deserves test-wise. >> +/* AUDIO_IF (0x0E) Fields */ >> + >> +#define AIF_SLAVE_EN 0x80 >> +#define DATA_WIDTH 0x60 > > These should really all get namespaced - some of them look relatively > likely to collide with registers for CPU side stuff. Good point. Steve -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH 3/5] SOUND: SOC: CODECS: Add support for the TWL4030 audio codec
On Thu, Sep 04, 2008 at 07:48:02AM -0700, Steve Sakoman wrote: > On Thu, Sep 4, 2008 at 5:04 AM, Mark Brown <[EMAIL PROTECTED]> wrote: > > Normally SND_SOC_BIAS_STANDBY would leave Vmid enabled but your power > > down function turns it off. The normal thing here is to have standby > > bring the codec up and then apply relatively small tweaks in _ON and ... > > Does the codec have any bypass paths that can be set up? If not it > > shouldn't do much harm either way until you implement DAPM. > How about I add a "TODO" on this? I'm swamped for the next few weeks > and won't be able to give it the attention it deserves test-wise. Like I say, if the codec/driver implement any bypass paths (paths that carry audio from the inputs to the outputs without using the DAC or ADC) then it'll break those - the device will be brought down to standby when there is no playback or record going, which will power the bypass paths down. If that is an issue then documenting it (ideally with printks for users) should cover it for now. If there aren't any bypass paths then it should be fine to leave it as-is. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH 3/5] SOUND: SOC: CODECS: Add support for the TWL4030 audio codec
On Thu, Sep 04, 2008 at 07:32:28AM -0700, Steve Sakoman wrote:
> On Thu, Sep 4, 2008 at 3:26 AM, Jarkko Nikula <[EMAIL PROTECTED]> wrote:
> >> +static void twl4030_dump_registers(void)
> >> +{
> > This is not needed since there is already nice function
> > for it: sound/soc/soc-core.c: codec_reg_show.
> I could probably get rid of this function. It was quite useful during
> debugging and I was not aware of codec_reg_show. IIRC, most of the
> other codec drivers also have the equivalent of this function, so we
> might want to clean them up too if there is a standard function to
> replace them.
Any references? None of the in-tree drivers have them...
> >> +static void twl4030_power_down(struct snd_soc_codec *codec)
> >> +{
> > ...
> >> + udelay(10);
> >> +}
> > REVISIT comment for these kind of magic delays if doesn't work without.
> It *seems* to work without them, but every historic TI driver seemed
> to have them. I figured that they might know something not reflected
> in the documentation. I will add a REVIST comment.
This sort of stuff is very common in codec drivers - normally the delays
are there to allow the analogue side of the system time to settle down
(waiting for capacitors to charge/discharge or reference voltages to
stabalise, for example). Ideally they have comments saying what's going
on, of course. Missing these delays often won't actually stop things
working completely but will instead do things like reduce performance or
generate audio artefacts - and sometimes it's application dependant if
these are important.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH 3/5] SOUND: SOC: CODECS: Add support for the TWL4030 audio codec
On Fri, Sep 5, 2008 at 3:06 AM, Mark Brown <[EMAIL PROTECTED]> wrote: >> How about I add a "TODO" on this? I'm swamped for the next few weeks >> and won't be able to give it the attention it deserves test-wise. > If there aren't any bypass paths then it should be fine to leave it > as-is. There are no bypass paths, so I will leave it as is for now. I plan to get back to this in a few weeks and will then implement proper BIAS_PREPARE and BIAS_STANDBY support. Steve -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH 3/5] SOUND: SOC: CODECS: Add support for the TWL4030 audio codec
On Fri, Sep 5, 2008 at 3:30 AM, Mark Brown <[EMAIL PROTECTED]> wrote: > On Thu, Sep 04, 2008 at 07:32:28AM -0700, Steve Sakoman wrote: >> On Thu, Sep 4, 2008 at 3:26 AM, Jarkko Nikula <[EMAIL PROTECTED]> wrote: > >> I could probably get rid of this function. It was quite useful during >> debugging and I was not aware of codec_reg_show. IIRC, most of the >> other codec drivers also have the equivalent of this function, so we >> might want to clean them up too if there is a standard function to >> replace them. > > Any references? None of the in-tree drivers have them... Heh, you are correct. I could swear I did a cut'n'paste on the reg cache stuff including the dump. Perhaps just old timers disease . . . >> It *seems* to work without them, but every historic TI driver seemed >> to have them. I figured that they might know something not reflected >> in the documentation. I will add a REVIST comment. > > This sort of stuff is very common in codec drivers - normally the delays > are there to allow the analogue side of the system time to settle down > (waiting for capacitors to charge/discharge or reference voltages to > stabalise, for example). Ideally they have comments saying what's going > on, of course. Missing these delays often won't actually stop things > working completely but will instead do things like reduce performance or > generate audio artefacts - and sometimes it's application dependant if > these are important. That was my assumption too, so I left them under the "better safe than sorry" theory. As suggested, I moved all sets and clears of CODEXPDZ to function calls which include the delay. Thanks again for the comments. Steve -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH 3/5] SOUND: SOC: CODECS: Add support for the TWL4030 audio codec
On Fri, Sep 05, 2008 at 07:34:00AM -0700, Steve Sakoman wrote: > From: Steve Sakoman <[EMAIL PROTECTED]> Looks good. Acked-by: Mark Brown <[EMAIL PROTECTED]> > SOUND: SOC: CODECS: Add support for the TWL4030 audio codec Usually it'd be formatted as ALSA: ASoC: Add support for TWL4030 audio codec but that's not too important. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH 3/5] SOUND: SOC: CODECS: Add support for the TWL4030 audio codec
On Fri, Sep 5, 2008 at 7:48 AM, Mark Brown <[EMAIL PROTECTED]> wrote: > Looks good. > > Acked-by: Mark Brown <[EMAIL PROTECTED]> > >> SOUND: SOC: CODECS: Add support for the TWL4030 audio codec > > Usually it'd be formatted as > > ALSA: ASoC: Add support for TWL4030 audio codec Thanks Mark! I'll change the title on my final resubmission of the series Steve -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
