media: rc: Replace timeval with ktime_t in imon.c

This patch changes the 32-bit time type (timeval) to the 64-bit one (ktime_t), since 32-bit time types will break in the year 2038.

I use ktime_t instead of all uses of timeval in imon.c

This patch also changes do_gettimeofday() to ktime_get() accordingly, since ktime_get returns a ktime_t, but do_gettimeofday returns a struct timeval, and the other reason is that ktime_get() uses the monotonic clock.

This patch use a new function which is provided by another patch listed below to get the millisecond time difference.

http://lkml.iu.edu//hypermail/linux/kernel/1412.2/00625.html

Signed-off-by Chunyan Zhang <[email protected]>

Quoted 5 lines · On Thursday 18 December 2014 11:37:13 Chunyan Zhang wrote:

This patch changes the 32-bit time type (timeval) to the 64-bit one (ktime_t), since 32-bit time types will break in the year 2038.

I use ktime_t instead of all uses of timeval in imon.c

Quoted 7 lines

This patch also changes do_gettimeofday() to ktime_get() accordingly, since ktime_get returns a ktime_t, but do_gettimeofday returns a struct timeval, and the other reason is that ktime_get() uses the monotonic clock.

This patch use a new function which is provided by another patch listed below to get the millisecond time difference.

The patch looks great. Just a few small details that could still be improved:

Quoted 3 lines

http://lkml.iu.edu//hypermail/linux/kernel/1412.2/00625.html

Signed-off-by Chunyan Zhang <[email protected]>

In general, when you give a mailing list link, use the 'Link' tag under your Signed-off-by line, like

Link http://lkml.iu.edu//hypermail/linux/kernel/1412.2/00625.html

It's not used much yet, but getting more popular and seems useful to me.

In this particular case, when you have patches that depend on one another, you can make do it even better by sending all three patches as a series with a [PATCH 0/3] cover letter.

If the media maintainers can provide an Ack for this patch, I would suggest to queue it up in the y2038 branch together with your first patch that it depends on.

Quoted 9 lines

drivers/media/rc/imon.c +2 -3

@@ -1191,16 +1168,16 @@ static inline int tv2int(const struct timeval *a, const struct timeval *b)
  */
 static int stabilize(int a, int b, u16 timeout, u16 threshold)
 {
-     struct timeval ct;
-     static struct timeval prev_time = {0, 0};
-     static struct timeval hit_time  = {0, 0};
+     ktime_t ct;
+     static ktime_t prev_time = {0};

drivers/media/rc/imon.c static inline int tv2int(const struct timeval *a, const struct timeval *b) · line 1175

Quoted 3 lines

drivers/media/rc/imon.c +1 -0

+     static ktime_t hit_time  = {0};
      static int x, y, prev_result, hits;
      int result = 0;

The "= {0}" part here is redundant, since static variables are always initialized to zero. Normally, adding the explicit initializer can help readability, but in this case I would leave it out because it shows implementation details of ktime_t that are better hidden from drivers.

drivers/media/rc/imon.c static void imon_incoming_packet(struct imon_context *ictx, · line 1579

Quoted 11 lines

drivers/media/rc/imon.c +3 -3

@@ -1596,9 +1573,9 @@ static void imon_incoming_packet(struct imon_context *ictx,
      int i;
      u64 scancode;
      int press_type = 0;
-     int msec;
-     struct timeval t;
-     static struct timeval prev_time = { 0, 0 };
+     long msec;
+     ktime_t t;
+     static ktime_t prev_time = {0};
      u8 ktype;

Same thing here of course.

Arnd

Quoted 11 lines · On Thursday 18 December 2014 11:37:13 Chunyan Zhang wrote:

Quoted 10 lines

This patch changes the 32-bit time type (timeval) to the 64-bit one (ktime_t), since 32-bit time types will break in the year 2038.

I use ktime_t instead of all uses of timeval in imon.c

This patch also changes do_gettimeofday() to ktime_get() accordingly, since ktime_get returns a ktime_t, but do_gettimeofday returns a struct timeval, and the other reason is that ktime_get() uses the monotonic clock.

Quoted 5 lines

Quoted 2 lines

This patch use a new function which is provided by another patch listed below to get the millisecond time difference.

The patch looks great. Just a few small details that could still be improved:

Quoted 8 lines

Quoted 4 lines

http://lkml.iu.edu//hypermail/linux/kernel/1412.2/00625.html

Signed-off-by Chunyan Zhang <[email protected]>

In general, when you give a mailing list link, use the 'Link' tag under your Signed-off-by line, like

Quoted 8 lines

Link http://lkml.iu.edu//hypermail/linux/kernel/1412.2/00625.html

It's not used much yet, but getting more popular and seems useful to me.

In this particular case, when you have patches that depend on one another, you can make do it even better by sending all three patches as a series with a [PATCH 0/3] cover letter.

OK, I'll send a patch-set including these three patches

Quoted 13 lines

If the media maintainers can provide an Ack for this patch, I would suggest to queue it up in the y2038 branch together with your first patch that it depends on.

Quoted 9 lines

drivers/media/rc/imon.c +2 -3

@@ -1191,16 +1168,16 @@ static inline int tv2int(const struct timeval *a, const struct timeval *b)
  */
 static int stabilize(int a, int b, u16 timeout, u16 threshold)
 {
-     struct timeval ct;
-     static struct timeval prev_time = {0, 0};
-     static struct timeval hit_time  = {0, 0};
+     ktime_t ct;
+     static ktime_t prev_time = {0};

Quoted 9 lines

drivers/media/rc/imon.c static inline int tv2int(const struct timeval *a, const struct timeval *b) · line 1175

Quoted 3 lines

drivers/media/rc/imon.c +1 -0

+     static ktime_t hit_time  = {0};
      static int x, y, prev_result, hits;
      int result = 0;

The "= {0}" part here is redundant, since static variables are always initialized to zero. Normally, adding the explicit initializer can help readability, but in this case I would leave it out because it shows implementation details of ktime_t that are better hidden from drivers.

OK, I'll modify them soon

Thanks,
Chunyan

Quoted 15 lines

drivers/media/rc/imon.c static void imon_incoming_packet(struct imon_context *ictx, · line 1579

Quoted 11 lines

drivers/media/rc/imon.c +3 -3

@@ -1596,9 +1573,9 @@ static void imon_incoming_packet(struct imon_context *ictx,
      int i;
      u64 scancode;
      int press_type = 0;
-     int msec;
-     struct timeval t;
-     static struct timeval prev_time = { 0, 0 };
+     long msec;
+     ktime_t t;
+     static ktime_t prev_time = {0};
      u8 ktype;

Same thing here of course.

Arnd

Em Thu, 18 Dec 2014 17:38:14 +0800
Chunyan Zhang <[email protected]> escreveu:

Quoted 15 lines · Arnd Bergmann

Quoted 14 lines · On Thursday 18 December 2014 11:37:13 Chunyan Zhang wrote:

Quoted 12 lines

This patch changes the 32-bit time type (timeval) to the 64-bit one (ktime_t), since 32-bit time types will break in the year 2038.

I use ktime_t instead of all uses of timeval in imon.c

This patch also changes do_gettimeofday() to ktime_get() accordingly, since ktime_get returns a ktime_t, but do_gettimeofday returns a struct timeval, and the other reason is that ktime_get() uses the monotonic clock.

This patch use a new function which is provided by another patch listed below to get the millisecond time difference.

Quoted 2 lines

Quoted 2 lines

The patch looks great. Just a few small details that could still be improved:

Yes, patch looks OK. After addressing the bits pointed by Arnd:

Acked-by Mauro Carvalho Chehab <[email protected]>

Feel free to merge via y2038 tree.