linux-kernel - Re: [PATCH 2/2] Input: ili210x

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191104070116.GM57214@dtor-ws>
Date:   Sun, 3 Nov 2019 23:01:16 -0800
From:   Dmitry Torokhov <dmitry.torokhov@...il.com>
To:     Sven Van Asbroeck <thesven73@...il.com>
Cc:     Marek Vasut <marex@...x.de>, Adam Ford <aford173@...il.com>,
        linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] Input: ili210x - add ILI2117 support

Hi Sven,

On Fri, Nov 01, 2019 at 04:48:01PM -0400, Sven Van Asbroeck wrote:
> Dmitry / Marek,
> 
> There have been two attempts to add ILI2117 touch controller support.
> I was about to add a third, but luckily I checked the mailing list
> before writing any code :)
> 
> Adding this support would clearly be beneficial for the common good.
> What can we do to get this in motion again?
> 
> Last time I checked, Marek posted a patch which added the 2117, but Dmitry
> objected, because the code became too unwieldy. Dmitry then posted a cleanup
> patch, which did not work for Marek. So everything came to a halt.
> See:
> https://patchwork.kernel.org/patch/10836651/
> https://www.spinics.net/lists/linux-input/msg62670.html
> 
> Dmitry, would you perhaps be willing to accept Marek's patch, and perform the
> cleanup later?
> 
> Marek, would you perhaps be willing to invest some time to debug Dmitry's
> cleanup patch?
> 
> On my end, I've reviewed Dmitry's patch and it looks mostly ok. I saw one
> difference with ILI210X which could explain Marek's results, but I can't be
> sure - because I could not locate the 210X's register layout on the web.
> 
> In Dmitry's patch, we see:
> 
> 	touch = ili210x_report_events(priv, touchdata);
> 	if (touch || chip->continue_polling(touchdata))
> 		schedule_delayed_work(&priv->dwork,
> 				      msecs_to_jiffies(priv->poll_period));
> 
> but this is not exactly equivalent to the original. Because in the original,
> the 210X's decision to kick off delayed work is completely independent of
> the value of touch.

No, it is not independent really. Bits 0 and 1 in the first byte
correspond to touches with first and 2nd finger, so checking for touch
in addition to 0xf3 mask is not incorrect.

Can you please tell me what device you have? Do the patches work for
you?

Marek, sorry for letting the patches linger. Can you please tell me what
touch controller did you test with that failed for you? I think I see at
least one issue in ili251x_read_touch_data() - the check whether we
should read the second part of the packet should check if data[0] == 2,
not 0.

Thanks.

-- 
Dmitry