On Thursday 02 December 2010 14:21:28 ext Mark Brown wrote:
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.
I'll try to figure out better name.
- 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.
True.
- 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.
Yes, in that case we do not even need the change variable, since if there is no change I just: goto out; Makes it cleaner.
Thanks, Péter