Re: [PATCH v4-resend 1/7] ASoC: sirf: Add SiRF internal audio codec driver

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 1378 bytes --]

On Fri, Feb 28, 2014 at 01:52:32AM +0000, Rongjun Ying wrote:

> > > +static struct sirf_audio_codec_reg_bits
> > sirf_audio_codec_reg_bits_prima2 = {
> > > +	.dig_mic_en_bits = 20,
> > > +	.dig_mic_freq_bits = 21,

> > This looks like the sort of thing that the regmap_field layer was
> > supposed to hide?

> Do you mean I need use the regmap_field_read/write to hide these?

You should certainly consider it and if it's not a good approach
understanding why may help us improve that API.

> > > +static int sirf_audio_codec_runtime_suspend(struct device *dev) {
> > > +	struct sirf_audio_codec *sirf_audio_codec = dev_get_drvdata(dev);
> > > +	regmap_update_bits(sirf_audio_codec->regmap, AUDIO_IC_CODEC_CTRL1,
> > > +		(1 << sirf_audio_codec->reg_bits->codec_clk_en_bits),
> > > +		0);
> > > +	return 0;
> > > +}

> > Can you disable the clock in the clock API as well?  This might allow
> > further supply clocks to be disabled and is genarally good practice.

> This is a workaround. The capture ADC and touch ADC use a same power supply,
> The codec ADC's change pump set or clear will impact touch ADC unstable.
> So It's need enable codec clock and set change pump when the Audio driver init.

That doesn't really answer the question for me - I wasn't asking why the
clock was being disabled, I was asking why the disable wasn't also being
done at the clock API level.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]