On Sun, May 10, 2015 at 9:38 AM, Ksenija Stanojevic
<[email protected]> wrote:
> 'struct timeval last_tv' is used to get the time of last signal change
> and 'struct timeval last_intr_tv' is used to get the time of last UART
> interrupt.
> 32-bit systems using 'struct timeval' will break in the year 2038, so we
> have to replace that code with more appropriate types.
> Here struct timeval is replaced with ktime_t.
>
> Signed-off-by: Ksenija Stanojevic <[email protected]>
> ---
> drivers/staging/media/lirc/lirc_sir.c | 41
> ++++++++++++++---------------------
> 1 file changed, 16 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/staging/media/lirc/lirc_sir.c
> b/drivers/staging/media/lirc/lirc_sir.c
> index 29087f6..a4ea2b1 100644
> --- a/drivers/staging/media/lirc/lirc_sir.c
> +++ b/drivers/staging/media/lirc/lirc_sir.c
> @@ -44,7 +44,7 @@
> #include <linux/ioport.h>
> #include <linux/kernel.h>
> #include <linux/serial_reg.h>
> -#include <linux/time.h>
> +#include <linux/ktime.h>
> #include <linux/string.h>
> #include <linux/types.h>
> #include <linux/wait.h>
> @@ -127,9 +127,9 @@ static int threshold = 3;
> static DEFINE_SPINLOCK(timer_lock);
> static struct timer_list timerlist;
> /* time of last signal change detected */
> -static struct timeval last_tv = {0, 0};
> +static ktime_t last_tv;
> /* time of last UART data ready interrupt */
> -static struct timeval last_intr_tv = {0, 0};
> +static ktime_t last_intr_tv;
So here and for the other values, the _tv postfix means timeval. So
since you're changing the type, it probably would be cleaner to change
the name to something that makes more sense. ie: last_signal_time,
last_interrupt_time, curr_time.
> static int last_value;
>
> static DECLARE_WAIT_QUEUE_HEAD(lirc_read_queue);
> @@ -400,17 +400,16 @@ static void drop_chrdev(void)
> }
>
> /* SECTION: Hardware */
> -static long delta(struct timeval *tv1, struct timeval *tv2)
> +static long delta(ktime_t tv1, ktime_t tv2)
> {
> unsigned long deltv;
> + ktime_t delkt;
>
> - deltv = tv2->tv_sec - tv1->tv_sec;
> - if (deltv > 15)
> + delkt = ktime_sub(tv2, tv1);
> + if (ktime_compare(delkt, ktime_set(15, 0)) > 0)
> deltv = 0xFFFFFF;
> else
> - deltv = deltv*1000000 +
> - tv2->tv_usec -
> - tv1->tv_usec;
> + deltv = (int)ktime_to_us(delkt);
> return deltv;
> }
Can this whole function be killed and replaced w/ a
ktime_to_us(ktime_sub(t1, t2))?
The if (>15 sec) return -1 bit is curious, since it doesn't seem
clearly documented I don't see much error checking in the file for
this case. I'm guessing its just trying to force the 32bit ceiling
instead of overflowing, but that would be easy enough to preserve w/
something like
static inline long delta(ktime_t t1, ktime_t t2)
{
/* return the delta in 32bit usecs, but cap to UINTMAX in case the
delta is greater then 32bits */
return (long) min(ktime_to_us(ktime_sub(t1, t2)), UINT_MAX);
}
_______________________________________________
Y2038 mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/y2038
