On Fri, Feb 03, 2012 at 04:19:00AM -0800, Reddy, MR Swami wrote:
Fix your mailer to word wrap within paragraphs. I'm sure I've mentioned this before...
snd_soc_write(codec, LM49453_P0_FLL_REF_FREQL_REG, (state->in & 0xff));snd_soc_write(codec, LM49453_P0_FLL_REF_FREQH_REG,((state->in >> 8) & 0xff));snd_soc_write(codec, LM49453_P0_VCO_TARGETLL_REG,state->out & 0xff);snd_soc_write(codec, LM49453_P0_VCO_TARGETLH_REG,(state->out >> 8) & 0xff);snd_soc_write(codec, LM49453_P0_VCO_TARGETHL_REG,(state->out >> 16) & 0xff);snd_soc_write(codec, LM49453_P0_VCO_TARGETHH_REG,(state->out >> 24) & 0xff);
The above code used to set the reference frequencies for FLL and VCO. Ideally, this code can be removed, because these registers are used for reference only.
You're saying that the PLL is actually not user configurable at all and the above register writes have no effect?
switch (lm49453->sysclk) {case 12288000:case 26000000:case 19000000:/* PLL CLK slection */pll_clk = ~BIT(4);break;case 48000:case 32576:/* FLL CLK slection */pll_clk = BIT(4);break;default:return -EINVAL;}
The above code used to set for CLOCK type (ie either PLL or FLL). The sysclk is PLL (HIGH FREQ mode) or FLL (LOW FREQ mode). So based on the sysclk, the BIT(4) will be set.
This doesn't address my comment (which you've deleted from my reply) at all. Why is this being done in hw_params()? It appears to be totally unrelated to hw_params().
The clocking configuration in this driver seems very confused, the system clock for the device should be being configured by some combinantion of set_sysclk() and set_pll() and hw_params() should do any adjustment of the clock dividers and so on to configure the rest of the chip for the sample rate based on the current clocking configuration.
case SND_SOC_BIAS_STANDBY:if (codec->dapm.bias_level == SND_SOC_BIAS_OFF)regcache_sync(lm49453->regmap);snd_soc_update_bits(codec, LM49453_P0_PMC_SETUP_REG,LM49453_PMC_SETUP_CHIP_EN, 0);break;case SND_SOC_BIAS_OFF:snd_soc_update_bits(codec, LM49453_P0_PMC_SETUP_REG,LM49453_PMC_SETUP_CHIP_EN, 0);break;}
Setting CHIP_EN bit as '0'. In the _STANDBY mode, CHIP_EN bit setting not needed and will remove in the next patch.
This also sounds broken, as I repeatedly said when reviewing previous versions of the driver if disabling the chip does anything at all then why don't you need to reenable the chip later?