Re: [PATCH v3] Staging: media: Replace timeval with ktime_t

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, May 13, 2015 at 9:57 AM, Ksenija Stanojevic
<ksenija.stanojevic@xxxxxxxxx> 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 <ksenija.stanojevic@xxxxxxxxx>
> ---
> Changes in v3:
>         - as John suggested delta function is changed to inline function,
>         checkpatch signals a warning to change min to min_t. Is it a false
>         positive?
>         - change variable names.
>
> Changes in v2:
>         - change subject line
>
>  drivers/staging/media/lirc/lirc_sir.c | 51 +++++++++++++----------------------
>  1 file changed, 18 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/staging/media/lirc/lirc_sir.c b/drivers/staging/media/lirc/lirc_sir.c
> index 29087f6..c98c486 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;
>  /* time of last UART data ready interrupt */
> -static struct timeval last_intr_tv = {0, 0};
> +static ktime_t last_intr_time;
>  static int last_value;
>
>  static DECLARE_WAIT_QUEUE_HEAD(lirc_read_queue);
> @@ -400,18 +400,11 @@ static void drop_chrdev(void)
>  }
>
>  /* SECTION: Hardware */
> -static long delta(struct timeval *tv1, struct timeval *tv2)
> +static inline long delta(ktime_t t1, ktime_t t2)
>  {
> -       unsigned long deltv;
> -
> -       deltv = tv2->tv_sec - tv1->tv_sec;
> -       if (deltv > 15)
> -               deltv = 0xFFFFFF;
> -       else
> -               deltv = deltv*1000000 +
> -                       tv2->tv_usec -
> -                       tv1->tv_usec;
> -       return deltv;
> +       /* return the delta in 32bit usecs, but cap to UINTMAX in case the
> +        * delta is greater then 32bits */
> +       return (long) min((unsigned int) ktime_us_delta(t1, t2), UINT_MAX);
>  }

This probably needs some close review from the media folks. Thinking
about it more, I'm really not certain the 15sec cap was to avoid a
32bit overflow or if there's some other subtle undocumented reason.

thanks
-john
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux