[PATCH] media: rc: Replace timeval with ktime_t in imon.c
From: Chunyan Zhang 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. Signed-off-by: Chunyan Zhang Acked-by: Mauro Carvalho Chehab Signed-off-by: Arnd Bergmann --- This patch was part of an old three-patch series, but did not get merged at the time after I dropped the ball on it. --- drivers/media/rc/imon.c | 49 + 1 file changed, 13 insertions(+), 36 deletions(-) diff --git a/drivers/media/rc/imon.c b/drivers/media/rc/imon.c index b25b35b3f6da..6c3ca1fff16b 100644 --- a/drivers/media/rc/imon.c +++ b/drivers/media/rc/imon.c @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -37,7 +38,6 @@ #include #include -#include #include #define MOD_AUTHOR "Jarod Wilson " @@ -1168,29 +1168,6 @@ static int imon_ir_change_protocol(struct rc_dev *rc, u64 *rc_proto) return retval; } -static inline int tv2int(const struct timeval *a, const struct timeval *b) -{ - int usecs = 0; - int sec = 0; - - if (b->tv_usec > a->tv_usec) { - usecs = 100; - sec--; - } - - usecs += a->tv_usec - b->tv_usec; - - sec += a->tv_sec - b->tv_sec; - sec *= 1000; - usecs /= 1000; - sec += usecs; - - if (sec < 0) - sec = 1000; - - return sec; -} - /** * The directional pad behaves a bit differently, depending on whether this is * one of the older ffdc devices or a newer device. Newer devices appear to @@ -1201,16 +1178,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; + static ktime_t hit_time; static int x, y, prev_result, hits; int result = 0; - int msec, msec_hit; + long msec, msec_hit; - do_gettimeofday(&ct); - msec = tv2int(&ct, &prev_time); - msec_hit = tv2int(&ct, &hit_time); + ct = ktime_get(); + msec = ktime_ms_delta(ct, prev_time); + msec_hit = ktime_ms_delta(ct, hit_time); if (msec > 100) { x = 0; @@ -1688,9 +1665,9 @@ static void imon_incoming_scancode(struct imon_context *ictx, u32 kc; 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; u8 ktype; /* filter out junk data on the older 0xffdc imon devices */ @@ -1783,10 +1760,10 @@ static void imon_incoming_scancode(struct imon_context *ictx, /* Only panel type events left to process now */ spin_lock_irqsave(&ictx->kc_lock, flags); - do_gettimeofday(&t); + t = ktime_get(); /* KEY_MUTE repeats from knob need to be suppressed */ if (ictx->kc == KEY_MUTE && ictx->kc == ictx->last_keycode) { - msec = tv2int(&t, &prev_time); + msec = ktime_ms_delta(t, prev_time); if (msec < ictx->idev->rep[REP_DELAY]) { spin_unlock_irqrestore(&ictx->kc_lock, flags); return; -- 2.9.0
Re: [PATCH] media: rc: Replace timeval with ktime_t in imon.c
On Thu, Dec 18, 2014 at 7:00 PM, Mauro Carvalho Chehab wrote: > Em Thu, 18 Dec 2014 17:38:14 +0800 > Chunyan Zhang escreveu: > >> On Thu, Dec 18, 2014 at 3:50 PM, Arnd Bergmann wrote: >> > 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 >> >> >> >> 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: > > Yes, patch looks OK. After addressing the bits pointed by Arnd: > > Acked-by: Mauro Carvalho Chehab > > Feel free to merge via y2038 tree. > Ok, thank you, I'll send the updated patch-set soon. Chunyan -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media: rc: Replace timeval with ktime_t in imon.c
Em Thu, 18 Dec 2014 17:38:14 +0800 Chunyan Zhang escreveu: > On Thu, Dec 18, 2014 at 3:50 PM, Arnd Bergmann wrote: > > 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 > >> > >> 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: Yes, patch looks OK. After addressing the bits pointed by Arnd: Acked-by: Mauro Carvalho Chehab Feel free to merge via y2038 tree. > > > >> http://lkml.iu.edu//hypermail/linux/kernel/1412.2/00625.html > >> > >> Signed-off-by: Chunyan Zhang > > > > 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. > > > OK, I'll send a patch-set including these three patches > > > 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. > > > >> @@ -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}; > >> + 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 > > >> @@ -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 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media: rc: Replace timeval with ktime_t in imon.c
On Thu, Dec 18, 2014 at 3:50 PM, Arnd Bergmann wrote: > 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 >> >> 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: > >> http://lkml.iu.edu//hypermail/linux/kernel/1412.2/00625.html >> >> Signed-off-by: Chunyan Zhang > > 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. > OK, I'll send a patch-set including these three patches > 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. > >> @@ -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}; >> + 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 >> @@ -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 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media: rc: Replace timeval with ktime_t in imon.c
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 > > 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: > http://lkml.iu.edu//hypermail/linux/kernel/1412.2/00625.html > > Signed-off-by: Chunyan Zhang 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. > @@ -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}; > + 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. > @@ -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 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] 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 --- drivers/media/rc/imon.c | 49 +-- 1 file changed, 13 insertions(+), 36 deletions(-) diff --git a/drivers/media/rc/imon.c b/drivers/media/rc/imon.c index b8837dd..a641139 100644 --- a/drivers/media/rc/imon.c +++ b/drivers/media/rc/imon.c @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -41,7 +42,6 @@ #include #include -#include #include #define MOD_AUTHOR "Jarod Wilson " @@ -1158,29 +1158,6 @@ out: return retval; } -static inline int tv2int(const struct timeval *a, const struct timeval *b) -{ - int usecs = 0; - int sec = 0; - - if (b->tv_usec > a->tv_usec) { - usecs = 100; - sec--; - } - - usecs += a->tv_usec - b->tv_usec; - - sec += a->tv_sec - b->tv_sec; - sec *= 1000; - usecs /= 1000; - sec += usecs; - - if (sec < 0) - sec = 1000; - - return sec; -} - /** * The directional pad behaves a bit differently, depending on whether this is * one of the older ffdc devices or a newer device. Newer devices appear to @@ -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}; + static ktime_t hit_time = {0}; static int x, y, prev_result, hits; int result = 0; - int msec, msec_hit; + long msec, msec_hit; - do_gettimeofday(&ct); - msec = tv2int(&ct, &prev_time); - msec_hit = tv2int(&ct, &hit_time); + ct = ktime_get(); + msec = ktime_ms_delta(ct, prev_time); + msec_hit = ktime_ms_delta(ct, hit_time); if (msec > 100) { x = 0; @@ -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; /* filter out junk data on the older 0xffdc imon devices */ @@ -1692,10 +1669,10 @@ static void imon_incoming_packet(struct imon_context *ictx, /* Only panel type events left to process now */ spin_lock_irqsave(&ictx->kc_lock, flags); - do_gettimeofday(&t); + t = ktime_get(); /* KEY_MUTE repeats from knob need to be suppressed */ if (ictx->kc == KEY_MUTE && ictx->kc == ictx->last_keycode) { - msec = tv2int(&t, &prev_time); + msec = ktime_ms_delta(t, prev_time); if (msec < ictx->idev->rep[REP_DELAY]) { spin_unlock_irqrestore(&ictx->kc_lock, flags); return; -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html