[alsa-devel] [PATCH v3 3/3] ASoC: Intel: fixed TI button detection

[Mark Brown]

On Fri, May 29, 2015 at 11:56:12AM -0700, yang.a.fang at intel.com wrote:

> +	if (event & SND_JACK_MICROPHONE) {
> +
> +		pin_status = snd_soc_dapm_get_pin_status(&codec->dapm, "SHDN");
> +		if (!pin_status)
> +			snd_soc_dapm_force_enable_pin(&codec->dapm, "SHDN");

This seems wrong - either we need the pin enabled or we don't.  If it's
currently enabled for something transient like playback then it might
get turned off later so we should still force it on.

> +		snd_soc_dapm_disable_pin(&codec->dapm, "MICBIAS");
> +		snd_soc_dapm_sync(&codec->dapm);
> +		/**
> +		* SHDN is max980090 shutdown pin we can not disable
> +		* it in case we are in the middle of playabck or record
> +		* we mark it unlock only so dapm will take care of it
> +		* next time
> +		*/
> +		snd_soc_dapm_disable_pin_unlocked(&codec->dapm, "SHDN");

This is wrong, you're mixing locked and unlocked versions of the DAPM
operations which can't be right - the difference between locked and
unlocked versions of the operations is that the locked versions is if
the locks for DAPM are already held and clearly there's no locking code
here.  This last operation shuld be a normal _disable_pin().

I'd also expect it to be before the sync - there's no telling how long
it'll be till the next sync otherwise and no reason to leave the pin
forced on if it's not needed.  If it is in use due to some other thing
then DAPM should ensure that the state doesn't get changed by the
disable.

Indentation is broken for the comment too.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20150603/045c6089/attachment-0001.sig>