On Fri, Nov 1, 2019 at 3:48 PM Sven Van Asbroeck <thesven73@xxxxxxxxx> 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. > If anyone is interested, I posted a patch to add ili2117A. https://patchwork.kernel.org/patch/10849877/ I am not sure if it's compatible with the non-A version. > Suggested replacement: > > touch = ili210x_report_events(priv, touchdata); > if (chip->continue_polling) > touch = chip->continue_polling(touchdata); > > Sven