Re: [alsa-devel] [PATCH v7 2/3] ASoC: da7210: Add support for line out and DAC

Re: [alsa-devel] [PATCH v7 2/3] ASoC: da7210: Add support for line out and DAC

[Girdwood, Liam]

On 20 October 2011 15:42, Ashish Chavan <ashish.chavan@kpitcummins.com> wrote:
> DA7210 has three line outputs. OUT1 Left, OUT1 Right and OUT2 (mono).
> This patch adds support for gain controls for these three line outs.
> It also adds support for overall DAC gain control.
>
> Signed-off-by: Ashish Chavan <ashish.chavan@kpitcummins.com>
> Signed-off-by: David Dajun Chen <dchen@diasemi.com>
> ---
> Changes since v1:
> - Removed explicit setting of default gains
> - Renamed "DAC Playback" to "Digital Playback"
> ---
>  sound/soc/codecs/da7210.c |   42 ++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 42 insertions(+), 0 deletions(-)
>

>
> +       /* Enable Line out amplifiers */
> +       snd_soc_write(codec, DA7210_OUT1_L, DA7210_OUT1_L_EN);
> +       snd_soc_write(codec, DA7210_OUT1_R, DA7210_OUT1_R_EN);
> +       snd_soc_write(codec, DA7210_OUT2, DA7210_OUT2_EN |
> +                    DA7210_OUT2_OUTMIX_L | DA7210_OUT2_OUTMIX_R);
> +

Any reason for not using DAPM here ? Otherwise they are always on
making your chip burn mW.....

Thanks

Liam

Re: [alsa-devel] [PATCH v7 2/3] ASoC: da7210: Add support for line out and DAC

[Mark Brown]

On Fri, Oct 21, 2011 at 02:07:40PM +0530, Ashish Chavan wrote:
> On Thu, 2011-10-20 at 18:10 +0100, Girdwood, Liam wrote:

> > Any reason for not using DAPM here ? Otherwise they are always on
> > making your chip burn mW.....

> Not really. As explained in the comment in DAPM patch, this enables will
> be effective (burn mW) only after respective IO is taken out from
> STANDBY mode. DAPM takes care of managing STANDBY mode for all IOs and
> ADC/DAC. I know this way of handling low power mode is bit non-trivial,
> but it is what is recommended by chip designers. Actually Mark was also
> initially confused with this and requested to put enough documentation
> into code. After that I tried explaining the logic in source code
> comment in DAPM patch. Let me know if it is not clear and doesn't convey
> clearly what is should.

That'd be fine but there's no DAPM code in the patch, only code to
unconditionally enable on init.

Re: [alsa-devel] [PATCH v7 2/3] ASoC: da7210: Add support for line out and DAC

[Mark Brown]

On Fri, Oct 21, 2011 at 03:19:15PM +0530, Ashish Chavan wrote:
> On Fri, 2011-10-21 at 10:00 +0100, Mark Brown wrote:

> > That'd be fine but there's no DAPM code in the patch, only code to
> > unconditionally enable on init.

> Yes, because DAPM part for those IOs is already covered by DAPM patch.

Ah, you should split that out of the DAPM patch into here - the DAPM bit
won't work without the enable bits (as I understand it).

Re: [alsa-devel] [PATCH v7 2/3] ASoC: da7210: Add support for line out and DAC

[Mark Brown]

On Fri, Oct 21, 2011 at 04:35:32PM +0530, Ashish Chavan wrote:

> Yes, your understanding is correct. Actually I acted bit lazy here by
> not splitting, assuming that both patches are going to get merged
> shortly. Do you think is it mandatory to do that? Or can I repost the
> patch after just resolving merge issue?

It's better to refactor, it makes review much easier.