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

[Paul Cercueil]

Le sam. 18 avril 2020 à 0:52, Andy Shevchenko 
<andy.shevchenko@xxxxxxxxx> a écrit :
> On Sat, Apr 18, 2020 at 12:45 AM Paul Cercueil <paul@xxxxxxxxxxxxxxx> 
> wrote:
> >  Le sam. 18 avril 2020 à 0:42, Andy Shevchenko
> >  <andy.shevchenko@xxxxxxxxx> a écrit :
> >  > On Sat, Apr 18, 2020 at 12:18 AM Paul Cercueil 
> > <paul@xxxxxxxxxxxxxxx>
> >  > wrote:
> >  >>  Le sam. 18 avril 2020 à 0:13, Andy Shevchenko
> >  >>  <andy.shevchenko@xxxxxxxxx> a écrit :
> >  >>  > On Sat, Apr 18, 2020 at 12:05 AM Paul Cercueil
> >  >> <paul@xxxxxxxxxxxxxxx>
> >  >>  > wrote:
> >  >>  >>  Le ven. 17 avril 2020 à 23:59, Andy Shevchenko
> >  >>  >>  <andy.shevchenko@xxxxxxxxx> a écrit :
> >  >>  >>  > On Fri, Apr 17, 2020 at 11:21 PM Artur Rojek
> >  >>  >> <contact@xxxxxxxxxxxxxx>
> >  >>  >>  > 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