Re: [PATCH v3] Add support for the Philips SA56004 temperaturesensor.

[Guenter Roeck]

On Sun, Jun 26, 2011 at 03:54:37PM -0400, Fubo Chen wrote:
> On Mon, Jun 6, 2011 at 12:40 PM,  <sdevrien@xxxxxxxxx> wrote:
> > +       } else
> > +       if (man_id == 0xA1) { /*  NXP Semiconductor/Philips */
> 
> No kernel coder writes like this. Has this ever been run through checkpatch ?
> 
What a nice comment. Quite obviously _you_ did not run the patch through checkpatch
to back up yout claim. I did:

total: 0 errors, 0 warnings, 151 lines checked

The code format is in line with other if/else constructs in the same function,
which coincidentially also pass through checkpatch. Which you did not bother
checking either (neither the format of the code in lm90.c nor the results
of checkpatch over it).

Before making statements like this, you might want to consider spending some time
with the actual code - amongst others to see if the new code fits into the structure
of existing code.

If you want to improve code formatting in this or in other files, I would suggest 
to submit patches.

Thanks,
Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/