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

Andy Green andy at openmoko.com
Mon Feb 11 09:13:43 CET 2008

Hash: SHA1

Somebody in the thread at some point said:
> 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.

Oh well, it didn't make any harm... the thought process was "do I need
to & this, no it will get truncated" and that somehow led to me
personally truncating it.  I removed it.

But the space after casts is super over-pedantic, the kernel is
absolutely chock full of the style I used there and CodingStyle is
silent about it, so I will keep that.

> +       gta02hdq_write(address, (u8)atoi(buf));
> Similarly, this isn't necessary in ANSI C. (K&R2 A7.3.2, page 202.)

Yeah same thought process, removed.

> +       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 != ' ')

Visually I can see what goes on with the existing way at a glance, where
as I have to stare at your version.  So since it makes no impact on code
and only improves readability, I will keep this (but I will think about
it from now on in case I am being a stick-in-the-mud).

> 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,

Yeah I used this, buf1 is a copout anyway.

> 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

I find this rationale very hard to believe :-)  But changed.

- -Andy
Version: GnuPG v1.4.7 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org


More information about the openmoko-kernel mailing list