On Thu, Sep 4, 2008 at 3:26 AM, Jarkko Nikula <jarkko.nikula@xxxxxxxxx> wrote: > On Wed, 3 Sep 2008 22:01:58 -0700 > "ext sakoman@xxxxxxxxx" <sakoman@xxxxxxxxx> 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 _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel