Re: [RFC 1/2] ASoC: core: Reordered DAPM update power on widgets

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

On Thu, Dec 02, 2010 at 02:03:09PM +0200, Peter Ujfalusi wrote:

> +	unsigned int dapm_reorder_pupdate:1; /* Reordered pupdate in widgets */

Please use a more meaningful name than pupdate, I can't parse what that
stands for easily (I'm guessing power update?) and even with that it's
not clear what the option actually does.

> +	unsigned int change, dapm_pupdate_first = 1;

Please split the new variable onto a separate line - mixed initialised
and uninitialised variables are confusing to read.

> -	if (snd_soc_test_bits(widget->codec, reg, val_mask, val)) {
> -		if (val)
> -			/* new connection */
> -			connect = invert ? 0:1;
> -		else
> -			/* old connection must be powered down */
> -			connect = invert ? 1:0;
> +	change = snd_soc_test_bits(widget->codec, reg, val_mask, val);
> +	if (val)
> +		/* new connection */
> +		connect = invert ? 0 : 1;
> +	else
> +		/* old connection must be powered down */

This is really confusing - if there's no change why are we not just
exiting the function immediately?  It makes the rest of the code much
harder to follow as the conditionals all get more complex.