On Wednesday, May 06, 2015 at 02:01:35 AM, Dmitry Torokhov wrote: > On Wed, May 06, 2015 at 01:46:36AM +0200, Marek Vasut wrote: > > 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! > 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. > 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) ? Thanks for your help! > > Signed-off-by: Marek Vasut <marex@xxxxxxx> > > Cc: Vipul Kumar Samar <vipulkumar.samar@xxxxxx> > > Cc: Viresh Kumar <viresh.kumar@xxxxxxxxxx> > > Cc: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> > > --- > > > > 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. > > > diff --git a/drivers/input/touchscreen/stmpe-ts.c > > b/drivers/input/touchscreen/stmpe-ts.c index 42ce31a..a8f5ea7 100644 > > --- a/drivers/input/touchscreen/stmpe-ts.c > > +++ b/drivers/input/touchscreen/stmpe-ts.c > > @@ -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; > > > > } > > Thanks. Best regards, Marek Vasut -- 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
