> > > +static irqreturn_t ili210x_irq(int irq, void *irq_data)
> > > +{
> > > + struct ili210x *priv = irq_data;
> > > + struct i2c_client *client = priv->client;
> > > + struct input_dev *input = priv->input;
> > > + struct device *dev = &client->dev;
> > > + struct touchdata touchdata;
> > > + int rc;
> > > +
> > > + do {
> > > + rc = ili210x_read_reg(client, REG_TOUCHDATA, &touchdata,
> > > + sizeof(touchdata));
> > > + if (rc < 0) {
> > > + dev_err(dev, "Unable to get touchdata, err = %d\n",
> > > + rc);
> > > + goto end;
> > > + }
> > > +
> > > + ili210x_report_events(input, &touchdata);
> > > +
> > > + usleep_range(100, 1000);
> > > + mod_timer(&priv->timer, jiffies + TS_PEN_UP_TIMEOUT);
> > > + } while (get_pendown_state(priv) && !priv->stopped);
> >
> > It looks odd to loop in an irq handler, even if it is threaded. What
> > is the pdata->get_pendown_state() doing?
> I agree. The reason of the loop is for edge triggered interrupt.
>
> On my hardware I don't have support for level triggered irq. I'm working
> with edge triggered interrupts.
> In the pdata structure I give to the driver it set irq_flags to
> IRQF_TRIGGER_FALLING and the get_pendown_state function implemented is the
> following:
> static int get_pendown_state(void)
> {
> return gpio_get_value(GPIO_TOUCHSCREEN_IRQ) ? 0 : 1;
> }
> The get_pendown_state() function will look at the irq line to see if it's
> still low meaning there is a finger on the screen.
> If it is the case, we loop, otherwise we exit from the thread and the timer
> will fire after TS_PEN_UP_TIMEOUT.
>
> On the contrary if working with a level triggered interrupt, i.e. with
> irqflags set to IRQF_TRIGGER_LOW, there is no need to add the
> get_pendown_state function and the thread will exit after each touch read.
> In this case the usleep_range() is even not needed...
>
> I agree it's maybe not the best solution... Do I've to use something else
> like a workqueue or something else?
I see, tricky. From the code, it looks like you could skip the loop
and start a workqueue there instead. The work function could then poll
touch frames until the gpio line goes high again. This would also give
you a predictable frame rate without the usleep(). The polldev
implementation might be a good template.
Regarding timer vs workqueue, I am not sure how much could be run in
interrupt context here. You could probably skip the threaded irq
either way, if it is only there to start a poll. Dmitry?
Thanks,
Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
