Re: [alsa-devel] [PATCH v3 2/8] ASoC: da7210: Add support for DAPM

Re: [alsa-devel] [PATCH v3 2/8] ASoC: da7210: Add support for DAPM

[Mark Brown]

On Fri, Oct 14, 2011 at 04:31:38PM +0530, Ashish Chavan wrote:

> +	/* ADCs */
> +	SND_SOC_DAPM_ADC("ADC Left", "Capture", DA7210_STARTUP3, 5, 1),
> +	SND_SOC_DAPM_ADC("ADC Right", "Capture", DA7210_STARTUP3, 6, 1),
> +
> +	/* Output Side */
> +	/* DACs */
> +	SND_SOC_DAPM_DAC("DAC Left", "Playback", DA7210_STARTUP2, 5, 1),
> +	SND_SOC_DAPM_DAC("DAC Right", "Playback", DA7210_STARTUP2, 6, 1),

These are also being enabled in the probe() function using completely
different registers - there the registers used are _ADC and _DAC.
What's going on there?  It's good to see the stuff in the startup()
function gone but almost all of the probe() just shouldn't be there in a
properly written driver and much of it looks like stuff that DAPM should
be taking care of.

Re: [alsa-devel] [PATCH v3 2/8] ASoC: da7210: Add support for DAPM

[Mark Brown]

On Sat, Oct 15, 2011 at 01:50:03PM +0530, Ashish Chavan wrote:

> There are two type of registers for all IOs (HP, MIC, AUX) as well as
> DAC and ADC. One type of registers control individual enable and disable
> of IO, while other set of register controls the standby mode of that
> particular IO. e.g. For ADC,

What are "enable and disable" in this context?  What do they do if not
put the part of the chip into a low power mode?  This is an intriguing
hardware design and it really needs some documentation in the driver,
right now it's just far too obscure and the driver looks buggy on code
inspection.

Re: [alsa-devel] [PATCH v3 2/8] ASoC: da7210: Add support for DAPM

[Mark Brown]

On Wed, Oct 19, 2011 at 12:51:35PM +0530, Ashish Chavan wrote:

> BTW if in case, hardware designers reiterate to use STANDBY registers,
> will it suffice to only document this somewhere is the source?

The main thing is to make sure that the code is understandable and
doesn't look buggy to visual inspection so yes, if there's adequate
documentation that would be OK.

Re: [alsa-devel] [PATCH v3 2/8] ASoC: da7210: Add support for DAPM

[Mark Brown]

On Thu, Oct 20, 2011 at 05:17:45PM +0530, Ashish Chavan wrote:

> Is there anything else that I need to take care of?

Not that I remember but often major issues obscure others so more may
become apparent on more detailed review.