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

Graeme Russ
2011-11-13T09:17:08Z

Hi Gabe,

On 13/11/11 13:52, Gabe Black wrote:
>
>
> On Sat, Nov 12, 2011 at 2:26 AM, Graeme Russ <graeme.russ@gmail.com
> <mailto:graeme.russ@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@chromium.org
> <mailto:gabeblack@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