[U-Boot] [PATCH] [x86] Fix some bugs in the i8402 driver when no controller is present

[Graeme Russ]

Hi Gabe,

On 13/11/11 13:52, Gabe Black wrote:
> 
> 
> On Sat, Nov 12, 2011 at 2:26 AM, Graeme Russ <graeme.russ at gmail.com
> <mailto:graeme.russ at gmail.com>> wrote:
> 
>     Hi Gabe,
> 
>     On 08/11/11 20:48, Gabe Black wrote:
>     > If no controller is present, the i8402 driver should return
>     immediately and
>     > not attempt to operate on the missing hardware.
>     >
>     > In kbd_input_empty, the status register is checked every millisecond
>     to see
>     > whether the input buffer is empty, up to a timeout which is tracked by
>     > decrimenting a counter each time the check is performed. The decrement is
>     > performed with a postfix -- operator, and the value of the counter is
>     > checked in place. That means that when the counter reaches zero and the
>     > loop terminates, it will actually be decrimented one more time and become
>     > -1. That value is returned as the return value of the function. That
>     would
>     > give the right answer if it wasn't for that extra decrement because a
>     > timeout would indicate that the buffer never became empty.
>     >
>     > This change fixes both of those bugs.
>     >
>     > Signed-off-by: Gabe Black <gabeblack at chromium.org
>     <mailto:gabeblack at chromium.org>>
>     > ---
>     >  drivers/input/i8042.c |   12 +++++++++++-
>     >  1 files changed, 11 insertions(+), 1 deletions(-)
>     >
> 
>     total: 1 errors, 5 warnings, 30 lines checked
> 
>     Patch is not checkpatch clean - Can you please fix checkpatch issues and
>     change tag to 'x86:' style and resubmit
> 
>     Thanks,
> 
>     Graeme
> 
> 
> checkpatch doesn't like it because I was consistent with the existing
> incorrect style in the file. There are three or four options for how to
> handle this.
> 
> 1. Match the current style and ignore checkpatch (what I've already done).
> 2. Fix the style on just the lines I've modified.
> 3. Fix the style within a "local" area around my changes, for some
> definition of local, along with my changes.
> 4. Submit a separate cleanup patch which fixes the style first.
> 
> What is the standard way of approaching this sort of situation? I'm ok with
> option 1 if you are since I already have that ready to go, and I'd be fine
> with 4 if you'd like to clean up this file first like you've been cleaning
> up some others. 2 and 3 would make the style in that file inconsistent and
> harder to read, but if that's standard practice I'll do that.

I think the standard we are now adopting (enforcing) is #4 - A two patch
series consisting of a checkpatch cleanup of the file and checkpatch clean
patch for the changes. Of course there are exceptions:

 - The checkpatch cleanup would be too onerous (touches too many files,
   the patched file is too large, too close to release) and the bugfix is
   critical
 - The bugfix covers too many files (again,
 - The file is from another repository (typically Linux) and the patch has
   been applied to that repository (in which case, a commit reference is
   required in the patch summary)

I'm happy for Wolfgang to correct me

Regards,

Graeme