[PATCH 1/2] introduce-fiq-hdq.patch

[Werner Almesberger]

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[128]; /* 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
victim.

+       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
could be:

        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
called "end".

- Werner