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

Andy Shevchenko
2020-04-17T21:53:09Z

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