linux-kernel - Re: [RESEND PATCH v5 3/5] IIO: Ingenic JZ47xx: Add touchscreen mode.

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: <DXCY8Q.DV6MO2JLAAHN3@crapouillou.net>
Date:   Fri, 17 Apr 2020 23:56:01 +0200
From:   Paul Cercueil <paul@...pouillou.net>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Artur Rojek <contact@...ur-rojek.eu>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Jonathan Cameron <jic23@...nel.org>,
        Heiko Stuebner <heiko@...ech.de>,
        linux-input <linux-input@...r.kernel.org>,
        devicetree <devicetree@...r.kernel.org>,
        linux-iio <linux-iio@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [RESEND PATCH v5 3/5] IIO: Ingenic JZ47xx: Add touchscreen mode.



Le sam. 18 avril 2020 à 0:52, Andy Shevchenko 
<andy.shevchenko@...il.com> a écrit :
> On Sat, Apr 18, 2020 at 12:45 AM Paul Cercueil <paul@...pouillou.net> 
> wrote:
>>  Le sam. 18 avril 2020 à 0:42, Andy Shevchenko
>>  <andy.shevchenko@...il.com> a écrit :
>>  > On Sat, Apr 18, 2020 at 12:18 AM Paul Cercueil 
>> <paul@...pouillou.net>
>>  > wrote:
>>  >>  Le sam. 18 avril 2020 à 0:13, Andy Shevchenko
>>  >>  <andy.shevchenko@...il.com> a écrit :
>>  >>  > On Sat, Apr 18, 2020 at 12:05 AM Paul Cercueil
>>  >> <paul@...pouillou.net>
>>  >>  > wrote:
>>  >>  >>  Le ven. 17 avril 2020 à 23:59, Andy Shevchenko
>>  >>  >>  <andy.shevchenko@...il.com> a écrit :
>>  >>  >>  > On Fri, Apr 17, 2020 at 11:21 PM Artur Rojek
>>  >>  >> <contact@...ur-rojek.eu>
>>  >>  >>  > wrote:
>>  >>  >
>>  >>  > ...
>>  >>  >
>>  >>  >>  >>  +       irq = platform_get_irq(pdev, 0);
>>  >>  >>  >
>>  >>  >>  > Before it worked w/o IRQ, here is a regression you 
>> introduced.
>>  >>  >>
>>  >>  >>  Before it simply did not need the IRQ, which is provided by 
>> the
>>  >>  >>  devicetree anyway. No regression here.
>>  >>  >
>>  >>  > Does it work without IRQ? Or it was a dead code till now?
>>  >>  > For me it's clear regression. Otherwise something is really 
>> wrong
>>  >> in a
>>  >>  > process of development of this driver.
>>  >>
>>  >>  Nothing wrong here. The IRQ was not used by the driver for the
>>  >>  functionality it provided before. It is required now to support 
>> the
>>  >>  touchscreen channels.
>>  >
>>  > This is exactly what's wrong.
>>  > Previous DTS for my (hypothetical) case has no IRQ defined. 
>> Everything
>>  > works, right?
>>  > Now, due to this change it breaks my setup. Don't you see the 
>> problem?
>> 
>>  The IRQ has been provided by every concerned DTS file since the
>>  introduction of this driver and the related bindings, even though it
>>  was not used by the driver.
> 
> Can you speak for all possible DTSs/DTBs in the wild?
> Okay, in any case it will be problem of maintainers and yours if
> somebody complains.

I can, since I wrote all of them.

-Paul

> I'm not going to push this anyway -- your choice.
> 
> But I see a (potential) regression.
> 
>>  >>  >>  >>  +       if (irq < 0) {
>>  >>  >>  >
>>  >>  >>  >>  +               dev_err(dev, "Failed to get irq: %d\n",
>>  >> irq);
>>  >>  >>  >
>>  >>  >>  > Redundant message.
>>  >>  >>  >
>>  >>  >>  >>  +               return irq;
>>  >>  >>  >>  +       }
> 
> --
> With Best Regards,
> Andy Shevchenko