Input: smtpe-ts: Use msecs_to_jiffies() instead of HZ

Use msecs_to_jiffies(50) instead of plain (HZ / 50) as the former is independent of kernel HZ settings and provides stable delay.

The delay here must be stable, otherwise we might start polling for pen-up event too early. This would in turn render the touchscreen unusable.

Signed-off-by Marek Vasut <[email protected]>

Cc Vipul Kumar Samar <[email protected]>

Cc Viresh Kumar <[email protected]>

Cc Dmitry Torokhov <[email protected]>

---
 drivers/input/touchscreen/stmpe-ts.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

NOTE: This applies all the way back to Linux ~2.6.36, so I should
probably also submit this for -stable if acceptable ?

drivers/input/touchscreen/stmpe-ts.c +1 -1

index 42ce31a..a8f5ea7 100644
@@ -164,7 +164,7 @@ static irqreturn_t stmpe_ts_handler(int irq, void *data)
 			STMPE_TSC_CTRL_TSC_EN, STMPE_TSC_CTRL_TSC_EN);
 
 	/* start polling for touch_det to detect release */
-	schedule_delayed_work(&ts->work, HZ / 50);
+	schedule_delayed_work(&ts->work, msecs_to_jiffies(50));
 
 	return IRQ_HANDLED;
 }

Signature · 1 line

2.1.4

Quoted 7 lines · Marek Vasut

Use msecs_to_jiffies(50) instead of plain (HZ / 50) as the former is independent of kernel HZ settings and provides stable delay.

The delay here must be stable, otherwise we might start polling for pen-up event too early. This would in turn render the touchscreen unusable.

Doesn't jiffy "size" depend on HZ value? I'd expect the delay be "stable" regardless of the value of HZ.

I do not disagree with the
patch, but I think you need better justification here (except I think the delay should be 20 msec, not 50, and we may use a constant instead of reevaluating it all the time).

Quoted 4 lines

Signed-off-by Marek Vasut <[email protected]>

Cc Vipul Kumar Samar <[email protected]>

Cc Viresh Kumar <[email protected]>

Cc Dmitry Torokhov <[email protected]>

Quoted 8 lines

---

 drivers/input/touchscreen/stmpe-ts.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

NOTE: This applies all the way back to Linux ~2.6.36, so I should

probably also submit this for -stable if acceptable ?

No, I do not think it needs to be in stable as it does not really fix any bug as far as I can see.

Quoted 11 lines

(quoted diff) +0 -0

b/drivers/input/touchscreen/stmpe-ts.c index 42ce31a..a8f5ea7 100644

drivers/input/touchscreen/stmpe-ts.c +0 -0

@@ -164,7 +164,7 @@ static irqreturn_t stmpe_ts_handler(int irq, void
 *data)
 
 			STMPE_TSC_CTRL_TSC_EN, STMPE_TSC_CTRL_TSC_EN);
 
 	/* start polling for touch_det to detect release */
 

Quoted 6 lines

drivers/input/touchscreen/stmpe-ts.c +1 -1

-	schedule_delayed_work(&ts->work, HZ / 50);
+	schedule_delayed_work(&ts->work, msecs_to_jiffies(50));

return IRQ_HANDLED;

}

Thanks.

Quoted 8 lines · Dmitry Torokhov

Quoted 7 lines · Marek Vasut

Use msecs_to_jiffies(50) instead of plain (HZ / 50) as the former is independent of kernel HZ settings and provides stable delay.

The delay here must be stable, otherwise we might start polling for pen-up event too early. This would in turn render the touchscreen unusable.

Hi!

Quoted 2 lines

Doesn't jiffy "size" depend on HZ value? I'd expect the delay be "stable" regardless of the value of HZ.

I have to admit I am not sure, but this would make sense.

Quoted 4 lines

I do not disagree with the
patch, but I think you need better justification here (except I think the delay should be 20 msec, not 50, and we may use a constant instead of reevaluating it all the time).

Maybe I'm fixing two bugs here ? The problem I really had is that with kernel HZ set to 250 and the original code, the work was scheduled and executed too quickly, so my touchscreen was bouncing between pendown and penup events back and forth, without doing any usaful work.

I tried changing the code to HZ/5 instead of HZ/50 and that fixes my issue as well.

My impression is that using msec_to_jiffies() is clearer especially if the code is to represent a constant delay.

But maybe I should then
document why I changed the duration (because the original one was too short) ?

Quoted 10 lines

Maybe I'm fixing two bugs here ? The problem I really had is that with kernel HZ set to 250 and the original code, the work was scheduled and executed too quickly, so my touchscreen was bouncing between pendown and penup events back and forth, without doing any usaful work.

I tried changing the code to HZ/5 instead of HZ/50 and that fixes my issue as well.

My impression is that using msec_to_jiffies() is clearer especially if the code is to represent a constant delay.

It is indeed clearer and I would not mind such conversion, we just need to adjust the comment why we do that.

Quoted 3 lines

But maybe I should then
document why I changed the duration (because the original one was too short) ?

Please make a separate patch for that with separate justification.

Thanks!