Re: [PATCH 2/2] Input: ili210x - add ILI2117 support

[Sven Van Asbroeck]

Hi Adam,

On Mon, Nov 4, 2019 at 4:28 PM Adam Ford <aford173@xxxxxxxxx> wrote:
>
> I am using IRQ_TYPE_EDGE_RISING for the 2117A.  Is that correct?  For
> my touchscreen, the IRQ line is low until a touch is detected, so I
> assume we want to capure on the rising edge.

That is correct for the 2117A, as far as I know. I am using the same
setting.

>
> Regarding Dmitry's patch,
> Is it a good idea to use msleep in an IRQ?  It seems like using the
> schedule_delayed_work() call seems like it will get in and get out of
> the ISR faster.
>
> If we use msleep and scan again, isn't it possible to starve other processes?

I believe using msleep() is ok because this is not a "real" interrupt handler,
but a threaded one. It runs in a regular kernel thread, with its interrupt
turned off (but all other interrupts remain enabled). Its interrupt is
re-enabled automatically after the threaded handler returns.

See
https://elixir.bootlin.com/linux/latest/source/include/linux/interrupt.h#L50

> > @@ -268,7 +278,7 @@ static irqreturn_t ili210x_irq(int irq, void *irq_data)
> >                 }
> >
> >                 touch = ili210x_report_events(priv, touchdata);
> > -               keep_polling = touch || chip->continue_polling(touchdata);
> > +               keep_polling = chip->continue_polling(touchdata, touch);
> >                 if (keep_polling)
>
> Why not just check the value of touch instead of invoking the function
> pointer which takes the value of touch in as a parameter?
>

The value of touch must be checked inside the callback, because
some variants use it to decide if they should poll again, and
some do not, such as the ili211x.

If I have misinterpreted your suggestion, could you perhaps
express it in C, so I can understand better?