On Thu, Jan 05, 2017 at 02:20:44PM +0800, John Hsu wrote:
This looks good overall, a couple of small points though:
+static int nau8540_config_sysclk(struct nau8540 *nau8540,
int clk_id, unsigned int freq)+{
struct regmap *regmap = nau8540->regmap;switch (clk_id) {case NAU8540_CLK_DIS:case NAU8540_CLK_MCLK:regmap_update_bits(regmap, NAU8540_REG_CLOCK_SRC,NAU8540_CLK_SRC_MASK, NAU8540_CLK_SRC_MCLK);regmap_update_bits(regmap, NAU8540_REG_FLL6,NAU8540_DCO_EN, 0);break;case NAU8540_CLK_INTERNAL:regmap_update_bits(regmap, NAU8540_REG_FLL6,NAU8540_DCO_EN, NAU8540_DCO_EN);regmap_update_bits(regmap, NAU8540_REG_CLOCK_SRC,NAU8540_CLK_SRC_MASK, NAU8540_CLK_SRC_VCO);break;
The above are fine but...
case NAU8540_CLK_FLL_MCLK:regmap_update_bits(regmap, NAU8540_REG_FLL3,NAU8540_FLL_CLK_SRC_MASK, NAU8540_FLL_CLK_SRC_MCLK);break;case NAU8540_CLK_FLL_BLK:regmap_update_bits(regmap, NAU8540_REG_FLL3,NAU8540_FLL_CLK_SRC_MASK, NAU8540_FLL_CLK_SRC_BLK);break;case NAU8540_CLK_FLL_FS:regmap_update_bits(regmap, NAU8540_REG_FLL3,NAU8540_FLL_CLK_SRC_MASK, NAU8540_FLL_CLK_SRC_FS);break;
...these look like they're mixing in FLL configuration which I'd expect to be done with set_pll() via the source argument and I'm not seeing anything that sets the device to use the FLL here. I'd expect the source selection to go into set_pll() and I'd expect this function to have a setting which tells the device to use the FLL.
+static int nau8540_set_sysclk(struct snd_soc_codec *codec,
int clk_id, int source, unsigned int freq, int dir)+{
struct nau8540 *nau8540 = snd_soc_codec_get_drvdata(codec);return nau8540_config_sysclk(nau8540, clk_id, freq);+}
This wrapper isn't really adding anything.
+static void nau8540_init_regs(struct nau8540 *nau8540) +{
struct regmap *regmap = nau8540->regmap;/* Enable Bias/VMID/VMID Tieoff */regmap_update_bits(regmap, NAU8540_REG_VMID_CTRL,NAU8540_VMID_EN | NAU8540_VMID_SEL_MASK,NAU8540_VMID_EN | (0x2 << NAU8540_VMID_SEL_SFT));regmap_update_bits(regmap, NAU8540_REG_REFERENCE,NAU8540_PRECHARGE_DIS | NAU8540_GLOBAL_BIAS_EN,NAU8540_PRECHARGE_DIS | NAU8540_GLOBAL_BIAS_EN);mdelay(2);regmap_update_bits(regmap, NAU8540_REG_MIC_BIAS,NAU8540_PU_PRE, NAU8540_PU_PRE);regmap_update_bits(regmap, NAU8540_REG_CLOCK_CTRL,NAU8540_CLK_ADC_EN | NAU8540_CLK_I2S_EN,NAU8540_CLK_ADC_EN | NAU8540_CLK_I2S_EN);/* ADC OSR selection, CLK_ADC = Fs * OSR */regmap_update_bits(regmap, NAU8540_REG_ADC_SAMPLE_RATE,NAU8540_ADC_OSR_MASK, NAU8540_ADC_OSR_64);+}
Does this sequence need to be done as part of resume as well?