On Thu, Sep 4, 2008 at 3:26 AM, Jarkko Nikula jarkko.nikula@nokia.com wrote:
On Wed, 3 Sep 2008 22:01:58 -0700 "ext sakoman@gmail.com" sakoman@gmail.com 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