On Sunday 10 May 2015 18:40:26 Ksenija Stanojevic 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 v2:
> - change subject line
[adding linux-media@xxxxxxxxxxxxxxx to cc]
Your patch looks correct to me, nice work!
> @@ -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;
> static int last_value;
It would probably help to rename the variables here and remove the '_tv'
postfix as this is no longer a timeval.
> 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;
> }
You have three different types here: 'long', 'unsigned long' and 'int'.
While I believe the code to be correct here, it would be easier to
see if you used all the same types, e.g. by making them all 'long'.
> @@ -432,7 +431,7 @@ static void sir_timeout(unsigned long data)
> /* clear unread bits in UART and restart */
> outb(UART_FCR_CLEAR_RCVR, io + UART_FCR);
> /* determine 'virtual' pulse end: */
> - pulse_end = delta(&last_tv, &last_intr_tv);
> + pulse_end = delta(last_tv, last_intr_tv);
> dev_dbg(driver.dev, "timeout add %d for %lu usec\n",
> last_value, pulse_end);
> add_read_queue(last_value, pulse_end);
> @@ -445,7 +444,7 @@ static void sir_timeout(unsigned long data)
> static irqreturn_t sir_interrupt(int irq, void *dev_id)
> {
> unsigned char data;
> - struct timeval curr_tv;
> + ktime_t curr_tv;
> static unsigned long deltv;
> unsigned long deltintrtv;
> unsigned long flags;
> @@ -471,9 +470,9 @@ static irqreturn_t sir_interrupt(int irq, void *dev_id)
> do {
> del_timer(&timerlist);
> data = inb(io + UART_RX);
> - do_gettimeofday(&curr_tv);
> - deltv = delta(&last_tv, &curr_tv);
> - deltintrtv = delta(&last_intr_tv, &curr_tv);
> + curr_tv = ktime_get();
> + deltv = delta(last_tv, curr_tv);
> + deltintrtv = delta(last_intr_tv, curr_tv);
> dev_dbg(driver.dev, "t %lu, d %d\n",
> deltintrtv, (int)data);
> /*
> @@ -488,10 +487,7 @@ static irqreturn_t sir_interrupt(int irq, void *dev_id)
> deltv -
> deltintrtv);
> last_value = 0;
> - last_tv.tv_sec =
> - last_intr_tv.tv_sec;
> - last_tv.tv_usec =
> - last_intr_tv.tv_usec;
> + last_tv = last_intr_tv;
> deltv = deltintrtv;
> }
> }
> @@ -505,13 +501,8 @@ static irqreturn_t sir_interrupt(int irq, void *dev_id)
> deltv-TIME_CONST);
> last_value = data;
> last_tv = curr_tv;
> - if (last_tv.tv_usec >= TIME_CONST) {
> - last_tv.tv_usec -= TIME_CONST;
> - } else {
> - last_tv.tv_sec--;
> - last_tv.tv_usec += 1000000 -
> - TIME_CONST;
> - }
> + last_tv = ktime_sub_us(last_tv,
> + TIME_CONST);
> }
> last_intr_tv = curr_tv;
> if (data) {
>
--
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
