Re: [PATCH] input: touchscreen: exc300: read the second frame during the interrupt line keep low

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

Hi Haibo,

On Mon, Nov 28, 2022 at 06:08:30PM +0800, haibo.chen@nxp.com wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> For the EETI EXC series, it support up to 10 points, two consecutive frames
> need to read. But for each frame, need to be read when the interrupt line
> keep in low level, otherwise, the read frame always get all 0xff data.
> 
> The hardware behavior for this touch device is it only support the low level
> trigger method, when irq line change to low level, usr need to read the
> first frame, and then the irq line return to high level, after about 320us,
> the irq line change to low level, then usr need to read the second frame.
> 
> Without this patch, we always find the second frame is readed during the irq
> line keep in high level, through the i2c logic analyzer, we see the data in
> the second frame is all 0xFF. This is because usr read the second frame
> immediately after the first frame in the same irq handler. But the irq line
> will back to high level after the first frame and keep about 320us. This
> behavior is wrong, and even worse, in some probability, this wrong behavior
> will cause the touch device stuck, can't generate interrupt any more, which
> means the interrupt line always keep high even after touch the screen.
> 
> This patch change to read only one frame in one irq handler. For the case
> need to read two frames, in the first irq handler, save the total contact
> number, then in the second irq handler, report the leftover contact.
> 
> Fixes: 7e577a17f2ee ("Input: add I2C attached EETI EXC3000 multi touch driver")
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> ---
>  drivers/input/touchscreen/exc3000.c | 40 ++++++++++++++---------------
>  1 file changed, 19 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/exc3000.c b/drivers/input/touchscreen/exc3000.c
> index 4b7eee01c6aa..eae7844e33db 100644
> --- a/drivers/input/touchscreen/exc3000.c
> +++ b/drivers/input/touchscreen/exc3000.c
> @@ -75,9 +75,10 @@ struct exc3000_data {
>  	struct touchscreen_properties prop;
>  	struct gpio_desc *reset;
>  	struct timer_list timer;
> -	u8 buf[2 * EXC3000_LEN_FRAME];
> +	u8 buf[EXC3000_LEN_FRAME];

I think this violates the notion of an input frame and will interfere
with the logic of dropping unused slots as the report will be split
across 2 input frames. You need to keep the buffer size large enough to
store both packets and postpone reporting of contacts until you read the
2nd packet (if there are more than one frame worth of contacts).

Thanks.

-- 
Dmitry