[PATCH] gta03-pca9632.patch

[Werner Almesberger]

michael wrote:
> Same problem :) *kzalloc*

Yup, assigments in expressions are generally a bad idea.
GCC will warn you if you
	if (foo = bar())
and it will also warn you if you
	foo == bar()
but with
	if ((foo == bar()))
all bets are off. Besides, such "hidden" assignments are easily
overlooked.

Also, a bit more confidence in operator precedence wouldn't be
amiss :-) E.g,
	return ((idc*100)/256);
could just be
	return idc*100/256;

Extra parentheses where they're obviously not needed just confuse
the reader, because they suggest that there is actually some subtle
reason why they're here, and they also make things harder to read.
E.g.,
	led_state = ((led_state >> (2 * ldrx)) & 0x03) ;

Besides this nitpicking, I didn't find anything to complain about :-)
Nice work !

Ah, one thing. It would be good to clearly mark code that's just
for general review/comments and not for inclusion into our tree. How
about using neither [PATCH] in the subject nor Signed-off-by unless
it's ready for inclusion ?

- Werner