在 2015年10月23日,17:55,Arnd Bergmann arnd@arndb.de 写道:
On Friday 23 October 2015 17:24:59 WEN Pingbo wrote:
Using struct timeval will cause time overflow in 2038, replacing it with ktime_t. And we don't need to handle sec and nsec separately.
This part looks good now, but as I commented in version 1, it should really be a separate patch rather than combined with the rest that is dealing with another use of timeval.
Ok, I will split it in next version.
do_gettimeofday(&tv);
tv.tv_usec += USEC_PER_SEC * (tv.tv_sec - mlc->instart.tv_sec);
tv.tv_usec -= mlc->instart.tv_usec;
if (tv.tv_usec >= mlc->intimeout) goto sched;
tv.tv_usec = (mlc->intimeout - tv.tv_usec) * HZ / USEC_PER_SEC;
if (!tv.tv_usec) goto sched;
mod_timer(&hil_mlcs_kicker, jiffies + tv.tv_usec);
if (tmp.tv64 >= (mlc->intimeout * NSEC_PER_USEC))
goto sched;
tmp.tv64 = mlc->intimeout * NSEC_PER_USEC - tmp.tv64;
if (tmp.tv64 < NSEC_PER_USEC)
goto sched;
mod_timer(&hil_mlcs_kicker,
break; sched: tasklet_schedule(&hil_mlcs_tasklet);jiffies + nsecs_to_jiffies(tmp.tv64));
If I read this right, the code is executed one for each input event such as a keypress or mouse movement. In the latter case, doing nsecs_to_jiffies() here is actually a bit expensive, and I stil think it can be avoided by just using jiffies.
For the (tmp.tv64 < NSEC_PER_USEC) part, did you just do that because I said this, or did you actually prove that it is required? I'm still confused about what the driver is trying to achieve here.
More explanation here:) the judgement here is to prevent mod_timer with zero delta. I can not make sure whether the module have nanosecond precise, so just keep same.
diff --git a/drivers/input/serio/hp_sdc_mlc.c b/drivers/input/serio/hp_sdc_mlc.c index d50f067..0a27b89 100644 --- a/drivers/input/serio/hp_sdc_mlc.c +++ b/drivers/input/serio/hp_sdc_mlc.c @@ -149,7 +149,8 @@ static int hp_sdc_mlc_in(hil_mlc *mlc, suseconds_t timeout)
/* Try to down the semaphore */ if (down_trylock(&mlc->isem)) {
struct timeval tv;
ktime_t tmp = ktime_sub(ktime_get(), mlc->instart);
- if (priv->emtestmode) { mlc->ipacket[0] = HIL_ERR_INT | (mlc->opacket &
Maybe give the variable a more useful name than 'tmp'?
You could also remove the variable entirely if you slightly restructure the condition below.
I see, thanks for point out.
Pingbo