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

From: Mark Brown <broonie@kernel.org>
To: Rongjun Ying <Rongjun.Ying@csr.com>
Cc: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	Takashi Iwai <tiwai@suse.de>,
	DL-SHA-WorkGroupLinux <DL-SHA-WorkGroupLinux@internal.csr.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	RongJun Ying <rjying@gmail.com>
Subject: Re: [PATCH v4-resend 1/7] ASoC: sirf: Add SiRF internal audio codec driver
Date: Sat, 1 Mar 2014 12:16:46 +0900	[thread overview]
Message-ID: <20140301031646.GJ29849@sirena.org.uk> (raw)
In-Reply-To: <86130EF012BDF348AA0D464A4F449478012AE51755@SHAASIEXM01.ASIA.ROOT.PRI>

[-- 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 --]