Hi Dmitry,

Am Mittwoch, 29. Juli 2015, 10:53:04 schrieb Dmitry Torokhov:

On Tue, Jul 28, 2015 at 11:43:20PM +0200, Heiko Stübner wrote:

[snip]

quoted

quoted

quoted

@@ -510,7 +511,18 @@ static irqreturn_t zforce_irq_thread(int irq,

void
*dev_id)>

 	if (!ts->suspending && device_may_wakeup(&client->dev))
 	
 		pm_stay_awake(&client->dev);

-	while (gpiod_get_value_cansleep(ts->gpio_int)) {
+	while (run) {
+		/*
+		 * Exit the loop if either
+		 * - the optional interrupt GPIO isn't specified
+		 *   (there is only one packet read per ISR invocation, 

then)

quoted

quoted

quoted

+		 * or
+		 * - the GPIO isn't active any more
+		 *   (packet read until the level GPIO indicates that there 

is

quoted

quoted

quoted

+		 *    no IRQ any more)
+		 */
+		run = gpiod_get_value_cansleep(ts->gpio_int);
+

alteratively you could simply convert to a

	/* Run at least once, or as long as the interrupt gpio is active. */
	do {
	
		...
	
	} while(gpiod_get_value_cansleep(ts->gpio_int));

saving the additional variable run and a lot of lines, while still running
at least once.

But I think that means that we'll try to read even if gpio is not active
anymore on the first iteration.

I don't think this is a possible scenario :-)

The interrupt is only used for transmission of touch-data and is pulled low as 
long as the host hasn't fetched all touch-packages. So if nothing else except 
the zforce-driver talks to the chip, the gpio will stay on.

And when we're entering the interrupt handler, there is at least one touch-
data package waiting.

So maybe we need to factor out the body
and do:

	if (!ts->gpio_int)
		zforce_report_data(...)
	else
		while (gpiod_get_value_cansleep(ts->gpio_int))
			zforce_report_data(...);

Or we could install 2 different interrupt handlers, depending on the
presence of interrupt gpio.

Heiko
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html