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.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
