linux-kernel - Re: [PATCH] input: goodix: Poll the 'buffer status' bit before reading data

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1497912102.2559.7.camel@hadess.net>
Date:   Tue, 20 Jun 2017 00:41:42 +0200
From:   Bastien Nocera <hadess@...ess.net>
To:     Paul Cercueil <paul@...pouillou.net>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc:     linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
        Hans de Goede <hdegoede@...hat.com>
Subject: Re: [PATCH] input: goodix: Poll the 'buffer status' bit before
 reading data

Hey,

Sorry I took this long to look into this.

On Thu, 2017-03-30 at 15:33 +0200, Paul Cercueil wrote:
> The Goodix panel triggers an interrupt on touch events. However, its
> registers will contain the valid values a short time after the
> interrupt, and not when it's raised. At that moment, the 'buffer
> status'
> bit is set.
> 
> Previously, if the 'buffer status' bit was not set when the registers
> were read, the data was discarded and no input event was emitted,
> causing "finger down" or "finger up" events to be missed sometimes.
> 
> This went unnoticed until v4.9, as the DesignWare I2C driver commonly
> used with this driver had enough latency for that bug to never
> trigger.
> 
> Now, in the IRQ handler we will poll (with a timeout)

I don't like the fact that the timeout isn't actually a timeout, but a
loop counter...

>  the 'buffer status'
> bit and process the data of the panel as soon as this bit gets set.

OK.

> Note that the Goodix panel will send a few spurious interrupts after
> the
> 'finger up' event, in which the 'buffer status' bit will never be
> set.

Can you please re-send the patch with diff option that makes it easier
to comment on whole patch? There's various block levels being mixed
because they have a linefeed in common...

I'm also CC:ing Hans to test it out as my Goodix tablet is still out of
commission.

Cheers