Re: [PATCH v1 1/2] Input: tsc2007 - convert to GPIO descriptors

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Mon, Mar 08, 2021 at 11:04:59PM +0200, Andy Shevchenko wrote:
> On Mon, Mar 8, 2021 at 9:29 PM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> > On Mon, Mar 08, 2021 at 11:10:38AM +0200, Andy Shevchenko wrote:
> > > On Mon, Mar 8, 2021 at 12:57 AM Dmitry Torokhov
> > > <dmitry.torokhov@gmail.com> wrote:
> > > > On Mon, Mar 08, 2021 at 12:05:48AM +0200, Andy Shevchenko wrote:
> > >
> > > ...
> > >
> > > > > -     return !gpio_get_value(ts->gpio);
> > > > > +     return !gpiod_get_value(ts->gpiod);
> > > >
> > > > This is not correct. gpio_get_value() is raw polarity vs
> > > > gpiod_get_value() using logical active/inactive, and tsc2007 GPIO lines
> > > > are active low. The negation must be dropped after switching to GPIOD
> > > > API.
> > >
> > > Ah, indeed, I missed that, thanks!
> > >
> > > ...
> > >
> > > > > -     ts->gpio = of_get_gpio(np, 0);
> > > > > -     if (gpio_is_valid(ts->gpio))
> > > > > -             ts->get_pendown_state = tsc2007_get_pendown_state_gpio;
> > > > > -     else
> > > > > -             dev_warn(&client->dev,
> > > > > -                      "GPIO not specified in DT (of_get_gpio returned %d)\n",
> > > > > -                      ts->gpio);
> > > > > +     ts->gpiod = devm_gpiod_get_optional(dev, NULL, GPIOD_IN);
> > > >
> > > > GPIO is definitely not optional in DT case, at least in the way the
> > > > driver written right now.
> > >
> > > Can you elaborate this, please? I don't see from the dev_warn() w/o
> > > any error code returned that it's mandatory.
> > > In the bindings one may read:
> > >
> > >   Optional properties:
> > >   - gpios: the interrupt gpio the chip is connected to (trough the penirq pin).
> > >     The penirq pin goes to low when the panel is touched.
> > >     (see GPIO binding[1] for more details).
> > >
> > > Nothing suggested it's mandatory. What have I missed?
> >
> > Ah, indeed, I misread the code and thought we'd abort if there is no
> > pendown GPIO. I wonder if we should remove the warning since we seem to
> > support operations without it.
> 
> But that's what I have done, i.e. removed the warning as well.

Well, what can I say, -ENOCOFFEE.
> 
> So, if there are no other concerns than inverted value, I'll send v2.

Yes, please.

-- 
Dmitry