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

On Mon, Nov 4, 2019 at 3:43 PM Sven Van Asbroeck [off-list ref] wrote:

Hi Adam,

On Mon, Nov 4, 2019 at 4:28 PM Adam Ford [off-list ref] wrote:

quoted

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.

quoted

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

quoted

quoted

@@ -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.

That makes sense.

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

You explained it.
I'm good.

adam