linux-kernel - Re: [PATCH 3/5] Input: add driver for Ilitek ili2139 touch IC

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

Message-ID: <20161011185114.GB8841@dtor-ws>
Date:   Tue, 11 Oct 2016 11:51:14 -0700
From:   Dmitry Torokhov <dmitry.torokhov@...il.com>
To:     Icenowy Zheng <icenowy@...c.xyz>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Maxime Ripard <maxime.ripard@...e-electrons.com>,
        Chen-Yu Tsai <wens@...e.org>,
        Hans de Goede <hdegoede@...hat.com>,
        Henrik Rydberg <rydberg@...math.org>,
        Mark Rutland <mark.rutland@....com>,
        Russell King <linux@...linux.org.uk>,
        Thierry Reding <treding@...dia.com>,
        Shawn Guo <shawnguo@...nel.org>,
        Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>,
        Marek Vasut <marex@...x.de>,
        Rask Ingemann Lambertsen <ccc94453@....cybercity.dk>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Michael Welling <mwelling@...e.org>,
        Arnd Bergmann <arnd@...db.de>,
        Markus Pargmann <mpa@...gutronix.de>,
        Damien Riegel <damien.riegel@...oirfairelinux.com>,
        Benjamin Tissoires <benjamin.tissoires@...hat.com>,
        Jeffrey Lin <jeffrey.lin@...-ic.com>,
        Javier Martinez Canillas <javier@....samsung.com>,
        Sangwon Jee <jeesw@...fas.com>,
        Siebren Vroegindeweij <siebren.vroegindeweij@...mail.com>,
        "linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-sunxi@...glegroups.com" <linux-sunxi@...glegroups.com>
Subject: Re: [PATCH 3/5] Input: add driver for Ilitek ili2139 touch IC

On Wed, Oct 12, 2016 at 02:34:01AM +0800, Icenowy Zheng wrote:
> 
> 
> 12.10.2016, 01:40, "Dmitry Torokhov" <dmitry.torokhov@...il.com>:
> > Hi Icenowy,
> >
> > On Tue, Oct 11, 2016 at 08:33:57AM +0800, Icenowy Zheng wrote:
> >>  This driver adds support for Ilitek ili2139 touch IC, which is used in
> >>  several Colorfly tablets (for example, Colorfly E708 Q1, which is an
> >>  Allwinner A31s tablet with mainline kernel support).
> >>
> >>  Theortically it may support more Ilitek touch ICs, however, only ili2139
> >>  is used in any mainlined device.
> >>
> >>  It supports device tree enumeration, with screen resolution and axis
> >>  quirks configurable.
> >>
> >>  Signed-off-by: Icenowy Zheng <icenowy@...c.xyz>
> >
> > Please extend ili210x.c instead of adding brand new driver, they look
> > very similar.
> >
> > Thanks.
> 
> The driver is too old, lack of maintaince and needs some platform data hacks.
> (At least makes it not capable to be used on current ARM devices, as they're
> described with device tree)

There are many drivers that can do both platform and dt-setup.

> 
> Maybe I will rename the new driver modified by me to ili210x, add support for
> the old protocol (but I have no chips to test it), and drop the old ili210x.
> (This driver is capable of dt probing, and uses devm_ functions)

You can add "racy on removal" to the list (you need to take care your
work is canceled at right times, and canceling it before interrupt is
freed is not the right time as interrupt might fire and the work get
scheduled again). Also I think your driver is essentially working in
polling mode because you always reschedule the delayed work.

No, like I said, please work with existing driver, adding DT support and
support for the newer version of the protocol.

Thanks.

-- 
Dmitry