[PATCH 1/2] introduce-fiq-hdq.patch
werner at openmoko.org
Mon Feb 11 00:36:31 CET 2008
Andy Green wrote:
> This adds a platform driver and device which performs HDQ
> battery protocol using a single GPIO pin which is set
> through platform data.
Cool stuff. Applied in revisions 4044 and ###. Thanks !
I think it could use some cleanup, though:
+ u8 u8a; /* whole address space for HDQ */
+ u8a[n] = (u8)v;
And that's coming from you, great slayer of redundant casts ? :-)
(K&R2 A7.17) Also, K&R style puts a space between the cast and its
+ gta02hdq_write(address, (u8)atoi(buf));
Similarly, this isn't necessary in ANSI C. (K&R2 A7.3.2, page 202.)
+ while ((buf < buf1) && (*buf != ' '))
+ while ((buf < buf1) && (*buf == ' '))
By the gods of Pascal ! Why thith army of parenthetheth ?
Isn't this much tidier ?
while (buf < buf1 && *buf != ' ')
Or, even better, the general convention is to call a pointer to the
end of some region "end", and Linus likes to compare for equality,
where reasonably possible. (Rationale, as far as I remember it: on
some platforms, testing for equality is more efficient than testing
for order. Also, a test for equality is less likely to mask errors
where the size isn't an increment even though it should be). So this
while (buf != end && *buf != ' ')
(with the appropriate changes to its surroundings.) There are a lot
of similarly nondescript "buf1" in that patch that could also be
More information about the openmoko-kernel