Re: [PATCH 1/5] I2C: LM8323: Introduce lm8323 keypad driver

[Daniel Stone <daniel.stone@nokia.com>]

[-- Attachment #1: Type: text/plain, Size: 1014 bytes --]

Hi,

On Wed, Apr 09, 2008 at 01:54:16PM +0300, ext Eduardo Valentin wrote:
> ERROR: do not initialise statics to 0 or NULL
> WARNING: braces {} are not necessary for single statement blocks

Yeah, these should be fixed.

> WARNING: consider using strict_strtoul in preference to simple_strtoul
> WARNING: consider using strict_strtoul in preference to simple_strtoul

I don't even know what strict_strtoul is, but okay.

> WARNING: line over 80 characters
> WARNING: do not add new typedefs

Sure.

> daniel.diff has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> 
> I'd suggest using some #defines for lots for magic numbers in this code.

'lots'? The only ones I can see are the magic constant needed for reset
(used in one place and commented), and the default/max values for
debounce/active time, and max x/y, both of which are again only used in
a single place.

Cheers,
Daniel

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]