linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC/RFT][PATCH] Input: tsc2007 - convert to threaded IRQ
@ 2011-08-23  6:17 Dmitry Torokhov
  2011-08-25  6:21 ` Thierry Reding
  0 siblings, 1 reply; 9+ messages in thread
From: Dmitry Torokhov @ 2011-08-23  6:17 UTC (permalink / raw)
  To: Linux Input; +Cc: Thierry Reding, Richard Röjfors

This is completely untested but I believe it should work better than original
workqueue solution, especially when we do not have get_pendown_state method
available. I wonder if it will help Richard's issue with IRQ getting re-raised
after pen up event.

Anyone who has hardware - please give it a spin.

Thanks!

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/input/touchscreen/tsc2007.c |  150 +++++++++++++++--------------------
 1 files changed, 66 insertions(+), 84 deletions(-)


diff --git a/drivers/input/touchscreen/tsc2007.c b/drivers/input/touchscreen/tsc2007.c
index fadc115..0fbc530 100644
--- a/drivers/input/touchscreen/tsc2007.c
+++ b/drivers/input/touchscreen/tsc2007.c
@@ -66,7 +66,6 @@ struct ts_event {
 struct tsc2007 {
 	struct input_dev	*input;
 	char			phys[32];
-	struct delayed_work	work;
 
 	struct i2c_client	*client;
 
@@ -76,9 +75,11 @@ struct tsc2007 {
 	unsigned long		poll_delay;
 	unsigned long		poll_period;
 
-	bool			pendown;
 	int			irq;
 
+	wait_queue_head_t	wait;
+	bool			stopped;
+
 	int			(*get_pendown_state)(void);
 	void			(*clear_penirq)(void);
 };
@@ -141,25 +142,8 @@ static u32 tsc2007_calculate_pressure(struct tsc2007 *tsc, struct ts_event *tc)
 	return rt;
 }
 
-static void tsc2007_send_up_event(struct tsc2007 *tsc)
-{
-	struct input_dev *input = tsc->input;
-
-	dev_dbg(&tsc->client->dev, "UP\n");
-
-	input_report_key(input, BTN_TOUCH, 0);
-	input_report_abs(input, ABS_PRESSURE, 0);
-	input_sync(input);
-}
-
-static void tsc2007_work(struct work_struct *work)
+static bool tsc2007_is_pen_down(struct tsc2007 *ts)
 {
-	struct tsc2007 *ts =
-		container_of(to_delayed_work(work), struct tsc2007, work);
-	bool debounced = false;
-	struct ts_event tc;
-	u32 rt;
-
 	/*
 	 * NOTE: We can't rely on the pressure to determine the pen down
 	 * state, even though this controller has a pressure sensor.
@@ -170,79 +154,82 @@ static void tsc2007_work(struct work_struct *work)
 	 * The only safe way to check for the pen up condition is in the
 	 * work function by reading the pen signal state (it's a GPIO
 	 * and IRQ). Unfortunately such callback is not always available,
-	 * in that case we have rely on the pressure anyway.
+	 * in that case we assume that the pen is down and expect caller
+	 * to fall back on the pressure reading.
 	 */
-	if (ts->get_pendown_state) {
-		if (unlikely(!ts->get_pendown_state())) {
-			tsc2007_send_up_event(ts);
-			ts->pendown = false;
-			goto out;
-		}
 
-		dev_dbg(&ts->client->dev, "pen is still down\n");
-	}
+	if (!ts->get_pendown_state)
+		return true;
 
-	tsc2007_read_values(ts, &tc);
+	return ts->get_pendown_state();
+}
 
-	rt = tsc2007_calculate_pressure(ts, &tc);
-	if (rt > ts->max_rt) {
-		/*
-		 * Sample found inconsistent by debouncing or pressure is
-		 * beyond the maximum. Don't report it to user space,
-		 * repeat at least once more the measurement.
-		 */
-		dev_dbg(&ts->client->dev, "ignored pressure %d\n", rt);
-		debounced = true;
-		goto out;
+static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
+{
+	struct tsc2007 *ts = handle;
+	struct input_dev *input = ts->input;
+	struct ts_event tc;
+	u32 rt;
 
-	}
+	while (!ts->stopped && tsc2007_is_pen_down(ts)) {
+
+		/* pen is down, continue with the measurement */
+		tsc2007_read_values(ts, &tc);
 
-	if (rt) {
-		struct input_dev *input = ts->input;
+		rt = tsc2007_calculate_pressure(ts, &tc);
 
-		if (!ts->pendown) {
-			dev_dbg(&ts->client->dev, "DOWN\n");
+		if (rt == 0 && !ts->get_pendown_state) {
+			/*
+			 * If pressure reported is 0 and we don't have
+			 * callback to check pendown state, we have to
+			 * assume that pen was lifted up.
+			 */
+			break;
+		}
+
+		if (rt <= ts->max_rt) {
+			dev_dbg(&ts->client->dev,
+				"DOWN point(%4d,%4d), pressure (%4u)\n",
+				tc.x, tc.y, rt);
 
 			input_report_key(input, BTN_TOUCH, 1);
-			ts->pendown = true;
+			input_report_abs(input, ABS_X, tc.x);
+			input_report_abs(input, ABS_Y, tc.y);
+			input_report_abs(input, ABS_PRESSURE, rt);
+
+			input_sync(input);
+
+		} else {
+			/*
+			 * Sample found inconsistent by debouncing or pressure is
+			 * beyond the maximum. Don't report it to user space,
+			 * repeat at least once more the measurement.
+			 */
+			dev_dbg(&ts->client->dev, "ignored pressure %d\n", rt);
 		}
 
-		input_report_abs(input, ABS_X, tc.x);
-		input_report_abs(input, ABS_Y, tc.y);
-		input_report_abs(input, ABS_PRESSURE, rt);
+		wait_event_timeout(ts->wait, ts->stopped,
+				   msecs_to_jiffies(ts->poll_period));
+	}
 
-		input_sync(input);
+	dev_dbg(&ts->client->dev, "UP\n");
 
-		dev_dbg(&ts->client->dev, "point(%4d,%4d), pressure (%4u)\n",
-			tc.x, tc.y, rt);
+	input_report_key(input, BTN_TOUCH, 0);
+	input_report_abs(input, ABS_PRESSURE, 0);
+	input_sync(input);
 
-	} else if (!ts->get_pendown_state && ts->pendown) {
-		/*
-		 * We don't have callback to check pendown state, so we
-		 * have to assume that since pressure reported is 0 the
-		 * pen was lifted up.
-		 */
-		tsc2007_send_up_event(ts);
-		ts->pendown = false;
-	}
+	if (ts->clear_penirq)
+		ts->clear_penirq();
 
- out:
-	if (ts->pendown || debounced)
-		schedule_delayed_work(&ts->work,
-				      msecs_to_jiffies(ts->poll_period));
-	else
-		enable_irq(ts->irq);
+	return IRQ_HANDLED;
 }
 
-static irqreturn_t tsc2007_irq(int irq, void *handle)
+static irqreturn_t tsc2007_hard_irq(int irq, void *handle)
 {
 	struct tsc2007 *ts = handle;
 
-	if (!ts->get_pendown_state || likely(ts->get_pendown_state())) {
-		disable_irq_nosync(ts->irq);
-		schedule_delayed_work(&ts->work,
-				      msecs_to_jiffies(ts->poll_delay));
-	}
+	if (!ts->get_pendown_state || likely(ts->get_pendown_state()))
+		return IRQ_WAKE_THREAD;
 
 	if (ts->clear_penirq)
 		ts->clear_penirq();
@@ -252,15 +239,10 @@ static irqreturn_t tsc2007_irq(int irq, void *handle)
 
 static void tsc2007_free_irq(struct tsc2007 *ts)
 {
+	ts->stopped = true;
+	mb();
+	wake_up(&ts->wait);
 	free_irq(ts->irq, ts);
-	if (cancel_delayed_work_sync(&ts->work)) {
-		/*
-		 * Work was pending, therefore we need to enable
-		 * IRQ here to balance the disable_irq() done in the
-		 * interrupt handler.
-		 */
-		enable_irq(ts->irq);
-	}
 }
 
 static int __devinit tsc2007_probe(struct i2c_client *client,
@@ -290,7 +272,7 @@ static int __devinit tsc2007_probe(struct i2c_client *client,
 	ts->client = client;
 	ts->irq = client->irq;
 	ts->input = input_dev;
-	INIT_DELAYED_WORK(&ts->work, tsc2007_work);
+	init_waitqueue_head(&ts->wait);
 
 	ts->model             = pdata->model;
 	ts->x_plate_ohms      = pdata->x_plate_ohms;
@@ -318,8 +300,8 @@ static int __devinit tsc2007_probe(struct i2c_client *client,
 	if (pdata->init_platform_hw)
 		pdata->init_platform_hw();
 
-	err = request_irq(ts->irq, tsc2007_irq, 0,
-			client->dev.driver->name, ts);
+	err = request_threaded_irq(ts->irq, tsc2007_hard_irq, tsc2007_soft_irq,
+				   0, client->dev.driver->name, ts);
 	if (err < 0) {
 		dev_err(&client->dev, "irq %d busy?\n", ts->irq);
 		goto err_free_mem;
-- 
Dmitry

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [RFC/RFT][PATCH] Input: tsc2007 - convert to threaded IRQ
  2011-08-23  6:17 [RFC/RFT][PATCH] Input: tsc2007 - convert to threaded IRQ Dmitry Torokhov
@ 2011-08-25  6:21 ` Thierry Reding
  2011-08-25  6:43   ` Dmitry Torokhov
  0 siblings, 1 reply; 9+ messages in thread
From: Thierry Reding @ 2011-08-25  6:21 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Linux Input, Richard Röjfors

[-- Attachment #1: Type: text/plain, Size: 463 bytes --]

* Dmitry Torokhov wrote:
> This is completely untested but I believe it should work better than original
> workqueue solution, especially when we do not have get_pendown_state method
> available. I wonder if it will help Richard's issue with IRQ getting re-raised
> after pen up event.
> 
> Anyone who has hardware - please give it a spin.

I can see no noticeable changes with this patch.

Tested-by: Thierry Reding <thierry.reding@avionic-design.de>

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC/RFT][PATCH] Input: tsc2007 - convert to threaded IRQ
  2011-08-25  6:21 ` Thierry Reding
@ 2011-08-25  6:43   ` Dmitry Torokhov
  2011-08-25  7:06     ` Thierry Reding
  0 siblings, 1 reply; 9+ messages in thread
From: Dmitry Torokhov @ 2011-08-25  6:43 UTC (permalink / raw)
  To: Thierry Reding; +Cc: Linux Input, Richard Röjfors

On Thu, Aug 25, 2011 at 08:21:59AM +0200, Thierry Reding wrote:
> * Dmitry Torokhov wrote:
> > This is completely untested but I believe it should work better than original
> > workqueue solution, especially when we do not have get_pendown_state method
> > available. I wonder if it will help Richard's issue with IRQ getting re-raised
> > after pen up event.
> > 
> > Anyone who has hardware - please give it a spin.
> 
> I can see no noticeable changes with this patch.
> 

That's the result I was aiming for ;) Thanks for testing.

Oh, crap... Does it still work if you change IRQ flags in
request_threaded_irq() from 0 to IRQF_ONESHOT? 

Also, while we are at it would you ming trying out another patch
(below)?

Thanks!

-- 
Dmitry


Input: tsc2007 - add open and close methods

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/input/touchscreen/tsc2007.c |   46 +++++++++++++++++++++++++++++------
 1 files changed, 38 insertions(+), 8 deletions(-)


diff --git a/drivers/input/touchscreen/tsc2007.c b/drivers/input/touchscreen/tsc2007.c
index 0fbc530..494b175 100644
--- a/drivers/input/touchscreen/tsc2007.c
+++ b/drivers/input/touchscreen/tsc2007.c
@@ -237,12 +237,40 @@ static irqreturn_t tsc2007_hard_irq(int irq, void *handle)
 	return IRQ_HANDLED;
 }
 
-static void tsc2007_free_irq(struct tsc2007 *ts)
+static void tsc2007_stop(struct tsc2007 *ts)
 {
 	ts->stopped = true;
 	mb();
 	wake_up(&ts->wait);
-	free_irq(ts->irq, ts);
+
+	disable_irq(ts->irq);
+}
+
+static int tsc2007_open(struct input_dev *input_dev)
+{
+	struct tsc2007 *ts = input_get_drvdata(input_dev);
+	int err;
+
+	ts->stopped = false;
+	mb();
+
+	enable_irq(ts->irq);
+
+	/* Prepare for touch readings - power down ADC and enable PENIRQ */
+	err = tsc2007_xfer(ts, PWRDOWN);
+	if (err) {
+		tsc2007_stop(ts);
+		return err;
+	}
+
+	return 0;
+}
+
+static void tsc2007_close(struct input_dev *input_dev)
+{
+	struct tsc2007 *ts = input_get_drvdata(input_dev);
+
+	tsc2007_stop(ts);
 }
 
 static int __devinit tsc2007_probe(struct i2c_client *client,
@@ -289,6 +317,11 @@ static int __devinit tsc2007_probe(struct i2c_client *client,
 	input_dev->phys = ts->phys;
 	input_dev->id.bustype = BUS_I2C;
 
+	input_dev->open = tsc2007_open;
+	input_dev->close = tsc2007_close;
+
+	input_set_drvdata(input_dev, ts);
+
 	input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
 	input_dev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
 
@@ -307,10 +340,7 @@ static int __devinit tsc2007_probe(struct i2c_client *client,
 		goto err_free_mem;
 	}
 
-	/* Prepare for touch readings - power down ADC and enable PENIRQ */
-	err = tsc2007_xfer(ts, PWRDOWN);
-	if (err < 0)
-		goto err_free_irq;
+	tsc2007_stop(ts);
 
 	err = input_register_device(input_dev);
 	if (err)
@@ -321,7 +351,7 @@ static int __devinit tsc2007_probe(struct i2c_client *client,
 	return 0;
 
  err_free_irq:
-	tsc2007_free_irq(ts);
+	free_irq(ts->irq, ts);
 	if (pdata->exit_platform_hw)
 		pdata->exit_platform_hw();
  err_free_mem:
@@ -335,7 +365,7 @@ static int __devexit tsc2007_remove(struct i2c_client *client)
 	struct tsc2007	*ts = i2c_get_clientdata(client);
 	struct tsc2007_platform_data *pdata = client->dev.platform_data;
 
-	tsc2007_free_irq(ts);
+	free_irq(ts->irq, ts);
 
 	if (pdata->exit_platform_hw)
 		pdata->exit_platform_hw();

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [RFC/RFT][PATCH] Input: tsc2007 - convert to threaded IRQ
  2011-08-25  6:43   ` Dmitry Torokhov
@ 2011-08-25  7:06     ` Thierry Reding
  2011-08-25  7:34       ` Dmitry Torokhov
  0 siblings, 1 reply; 9+ messages in thread
From: Thierry Reding @ 2011-08-25  7:06 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Linux Input, Richard Röjfors

[-- Attachment #1: Type: text/plain, Size: 1032 bytes --]

* Dmitry Torokhov wrote:
> On Thu, Aug 25, 2011 at 08:21:59AM +0200, Thierry Reding wrote:
> > * Dmitry Torokhov wrote:
> > > This is completely untested but I believe it should work better than original
> > > workqueue solution, especially when we do not have get_pendown_state method
> > > available. I wonder if it will help Richard's issue with IRQ getting re-raised
> > > after pen up event.
> > > 
> > > Anyone who has hardware - please give it a spin.
> > 
> > I can see no noticeable changes with this patch.
> > 
> 
> That's the result I was aiming for ;) Thanks for testing.
> 
> Oh, crap... Does it still work if you change IRQ flags in
> request_threaded_irq() from 0 to IRQF_ONESHOT? 
> 
> Also, while we are at it would you ming trying out another patch
> (below)?

With both your patch applied and the change from 0 to IRQF_ONESHOT the driver
oopses when loaded. I didn't have time to investigate further, but I think I
could perhaps test a couple more patches if you provide them.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC/RFT][PATCH] Input: tsc2007 - convert to threaded IRQ
  2011-08-25  7:06     ` Thierry Reding
@ 2011-08-25  7:34       ` Dmitry Torokhov
  2011-08-25  7:52         ` Thierry Reding
  0 siblings, 1 reply; 9+ messages in thread
From: Dmitry Torokhov @ 2011-08-25  7:34 UTC (permalink / raw)
  To: Thierry Reding; +Cc: Linux Input, Richard Röjfors

On Thu, Aug 25, 2011 at 09:06:57AM +0200, Thierry Reding wrote:
> * Dmitry Torokhov wrote:
> > On Thu, Aug 25, 2011 at 08:21:59AM +0200, Thierry Reding wrote:
> > > * Dmitry Torokhov wrote:
> > > > This is completely untested but I believe it should work better than original
> > > > workqueue solution, especially when we do not have get_pendown_state method
> > > > available. I wonder if it will help Richard's issue with IRQ getting re-raised
> > > > after pen up event.
> > > > 
> > > > Anyone who has hardware - please give it a spin.
> > > 
> > > I can see no noticeable changes with this patch.
> > > 
> > 
> > That's the result I was aiming for ;) Thanks for testing.
> > 
> > Oh, crap... Does it still work if you change IRQ flags in
> > request_threaded_irq() from 0 to IRQF_ONESHOT? 
> > 
> > Also, while we are at it would you ming trying out another patch
> > (below)?
> 
> With both your patch applied and the change from 0 to IRQF_ONESHOT the driver
> oopses when loaded. I didn't have time to investigate further, but I think I
> could perhaps test a couple more patches if you provide them.

Hmm, can't spot the problem... Where does it die? If you could try just
IRFQ_ONESHOT change that would be helpful.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC/RFT][PATCH] Input: tsc2007 - convert to threaded IRQ
  2011-08-25  7:34       ` Dmitry Torokhov
@ 2011-08-25  7:52         ` Thierry Reding
  2011-08-25  7:56           ` Dmitry Torokhov
  0 siblings, 1 reply; 9+ messages in thread
From: Thierry Reding @ 2011-08-25  7:52 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Linux Input, Richard Röjfors

[-- Attachment #1: Type: text/plain, Size: 1463 bytes --]

* Dmitry Torokhov wrote:
> On Thu, Aug 25, 2011 at 09:06:57AM +0200, Thierry Reding wrote:
> > * Dmitry Torokhov wrote:
> > > On Thu, Aug 25, 2011 at 08:21:59AM +0200, Thierry Reding wrote:
> > > > * Dmitry Torokhov wrote:
> > > > > This is completely untested but I believe it should work better than original
> > > > > workqueue solution, especially when we do not have get_pendown_state method
> > > > > available. I wonder if it will help Richard's issue with IRQ getting re-raised
> > > > > after pen up event.
> > > > > 
> > > > > Anyone who has hardware - please give it a spin.
> > > > 
> > > > I can see no noticeable changes with this patch.
> > > > 
> > > 
> > > That's the result I was aiming for ;) Thanks for testing.
> > > 
> > > Oh, crap... Does it still work if you change IRQ flags in
> > > request_threaded_irq() from 0 to IRQF_ONESHOT? 
> > > 
> > > Also, while we are at it would you ming trying out another patch
> > > (below)?
> > 
> > With both your patch applied and the change from 0 to IRQF_ONESHOT the driver
> > oopses when loaded. I didn't have time to investigate further, but I think I
> > could perhaps test a couple more patches if you provide them.
> 
> Hmm, can't spot the problem... Where does it die?

I'll see if I can track it down, but I have very limited time.

> If you could try just IRFQ_ONESHOT change that would be helpful.

With only just this change, things work okay.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC/RFT][PATCH] Input: tsc2007 - convert to threaded IRQ
  2011-08-25  7:52         ` Thierry Reding
@ 2011-08-25  7:56           ` Dmitry Torokhov
  2011-08-25  8:05             ` Dmitry Torokhov
  0 siblings, 1 reply; 9+ messages in thread
From: Dmitry Torokhov @ 2011-08-25  7:56 UTC (permalink / raw)
  To: Thierry Reding; +Cc: Linux Input, Richard Röjfors

On Thu, Aug 25, 2011 at 09:52:54AM +0200, Thierry Reding wrote:
> * Dmitry Torokhov wrote:
> > On Thu, Aug 25, 2011 at 09:06:57AM +0200, Thierry Reding wrote:
> > > * Dmitry Torokhov wrote:
> > > > On Thu, Aug 25, 2011 at 08:21:59AM +0200, Thierry Reding wrote:
> > > > > * Dmitry Torokhov wrote:
> > > > > > This is completely untested but I believe it should work better than original
> > > > > > workqueue solution, especially when we do not have get_pendown_state method
> > > > > > available. I wonder if it will help Richard's issue with IRQ getting re-raised
> > > > > > after pen up event.
> > > > > > 
> > > > > > Anyone who has hardware - please give it a spin.
> > > > > 
> > > > > I can see no noticeable changes with this patch.
> > > > > 
> > > > 
> > > > That's the result I was aiming for ;) Thanks for testing.
> > > > 
> > > > Oh, crap... Does it still work if you change IRQ flags in
> > > > request_threaded_irq() from 0 to IRQF_ONESHOT? 
> > > > 
> > > > Also, while we are at it would you ming trying out another patch
> > > > (below)?
> > > 
> > > With both your patch applied and the change from 0 to IRQF_ONESHOT the driver
> > > oopses when loaded. I didn't have time to investigate further, but I think I
> > > could perhaps test a couple more patches if you provide them.
> > 
> > Hmm, can't spot the problem... Where does it die?
> 
> I'll see if I can track it down, but I have very limited time.

Don't worry, there is no rush whatsoever... I'll try to ponder the code
some more.

> 
> > If you could try just IRFQ_ONESHOT change that would be helpful.
> 
> With only just this change, things work okay.

Excellent, thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC/RFT][PATCH] Input: tsc2007 - convert to threaded IRQ
  2011-08-25  7:56           ` Dmitry Torokhov
@ 2011-08-25  8:05             ` Dmitry Torokhov
  2011-08-25  8:25               ` Thierry Reding
  0 siblings, 1 reply; 9+ messages in thread
From: Dmitry Torokhov @ 2011-08-25  8:05 UTC (permalink / raw)
  To: Thierry Reding; +Cc: Linux Input, Richard Röjfors

On Thu, Aug 25, 2011 at 12:56:25AM -0700, Dmitry Torokhov wrote:
> On Thu, Aug 25, 2011 at 09:52:54AM +0200, Thierry Reding wrote:
> > * Dmitry Torokhov wrote:
> > > On Thu, Aug 25, 2011 at 09:06:57AM +0200, Thierry Reding wrote:
> > > > * Dmitry Torokhov wrote:
> > > > > On Thu, Aug 25, 2011 at 08:21:59AM +0200, Thierry Reding wrote:
> > > > > > * Dmitry Torokhov wrote:
> > > > > > > This is completely untested but I believe it should work better than original
> > > > > > > workqueue solution, especially when we do not have get_pendown_state method
> > > > > > > available. I wonder if it will help Richard's issue with IRQ getting re-raised
> > > > > > > after pen up event.
> > > > > > > 
> > > > > > > Anyone who has hardware - please give it a spin.
> > > > > > 
> > > > > > I can see no noticeable changes with this patch.
> > > > > > 
> > > > > 
> > > > > That's the result I was aiming for ;) Thanks for testing.
> > > > > 
> > > > > Oh, crap... Does it still work if you change IRQ flags in
> > > > > request_threaded_irq() from 0 to IRQF_ONESHOT? 
> > > > > 
> > > > > Also, while we are at it would you ming trying out another patch
> > > > > (below)?
> > > > 
> > > > With both your patch applied and the change from 0 to IRQF_ONESHOT the driver
> > > > oopses when loaded. I didn't have time to investigate further, but I think I
> > > > could perhaps test a couple more patches if you provide them.
> > > 
> > > Hmm, can't spot the problem... Where does it die?
> > 
> > I'll see if I can track it down, but I have very limited time.
> 
> Don't worry, there is no rush whatsoever... I'll try to ponder the code
> some more.

I think we fail in tsc2007_open(), instead of "if (err) {" it should say
"if (err < 0) {"... but I still wonder why this causes oops...

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC/RFT][PATCH] Input: tsc2007 - convert to threaded IRQ
  2011-08-25  8:05             ` Dmitry Torokhov
@ 2011-08-25  8:25               ` Thierry Reding
  0 siblings, 0 replies; 9+ messages in thread
From: Thierry Reding @ 2011-08-25  8:25 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Linux Input, Richard Röjfors

[-- Attachment #1: Type: text/plain, Size: 2193 bytes --]

* Dmitry Torokhov wrote:
> On Thu, Aug 25, 2011 at 12:56:25AM -0700, Dmitry Torokhov wrote:
> > On Thu, Aug 25, 2011 at 09:52:54AM +0200, Thierry Reding wrote:
> > > * Dmitry Torokhov wrote:
> > > > On Thu, Aug 25, 2011 at 09:06:57AM +0200, Thierry Reding wrote:
> > > > > * Dmitry Torokhov wrote:
> > > > > > On Thu, Aug 25, 2011 at 08:21:59AM +0200, Thierry Reding wrote:
> > > > > > > * Dmitry Torokhov wrote:
> > > > > > > > This is completely untested but I believe it should work better than original
> > > > > > > > workqueue solution, especially when we do not have get_pendown_state method
> > > > > > > > available. I wonder if it will help Richard's issue with IRQ getting re-raised
> > > > > > > > after pen up event.
> > > > > > > > 
> > > > > > > > Anyone who has hardware - please give it a spin.
> > > > > > > 
> > > > > > > I can see no noticeable changes with this patch.
> > > > > > > 
> > > > > > 
> > > > > > That's the result I was aiming for ;) Thanks for testing.
> > > > > > 
> > > > > > Oh, crap... Does it still work if you change IRQ flags in
> > > > > > request_threaded_irq() from 0 to IRQF_ONESHOT? 
> > > > > > 
> > > > > > Also, while we are at it would you ming trying out another patch
> > > > > > (below)?
> > > > > 
> > > > > With both your patch applied and the change from 0 to IRQF_ONESHOT the driver
> > > > > oopses when loaded. I didn't have time to investigate further, but I think I
> > > > > could perhaps test a couple more patches if you provide them.
> > > > 
> > > > Hmm, can't spot the problem... Where does it die?
> > > 
> > > I'll see if I can track it down, but I have very limited time.
> > 
> > Don't worry, there is no rush whatsoever... I'll try to ponder the code
> > some more.
> 
> I think we fail in tsc2007_open(), instead of "if (err) {" it should say
> "if (err < 0) {"... but I still wonder why this causes oops...

It was indeed the open() call that was failing and changing "if (err) {" to
"if (err < 0) {" does indeed fix the issue.

Again, no negative nor positive changes in the touchscreen behaviour. So
again,

Tested-by: Thierry Reding <thierry.reding@avionic-design.de>

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2011-08-25  8:25 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-08-23  6:17 [RFC/RFT][PATCH] Input: tsc2007 - convert to threaded IRQ Dmitry Torokhov
2011-08-25  6:21 ` Thierry Reding
2011-08-25  6:43   ` Dmitry Torokhov
2011-08-25  7:06     ` Thierry Reding
2011-08-25  7:34       ` Dmitry Torokhov
2011-08-25  7:52         ` Thierry Reding
2011-08-25  7:56           ` Dmitry Torokhov
2011-08-25  8:05             ` Dmitry Torokhov
2011-08-25  8:25               ` Thierry Reding

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).