* [PATCH] cx88: High resolution timer for Remote Controls
@ 2009-06-04 16:49 AH
0 siblings, 0 replies; 3+ messages in thread
From: AH @ 2009-06-04 16:49 UTC (permalink / raw)
To: linux-media
Patch solves problem of missed keystrokes on some remote controls,
as reported on http://bugzilla.kernel.org/show_bug.cgi?id=9637 .
---
Signed-off-by: Andrzej Hajda <andrzej.hajda@wp.pl <mailto:andrzej.hajda@wp.pl>>
Acked-by: Jean Delvare <khali@linux-fr.org <mailto:khali@linux-fr.org>>
---
diff -r 315bc4b65b4f linux/drivers/media/video/cx88/cx88-input.c
--- a/linux/drivers/media/video/cx88/cx88-input.c Sun May 17 12:28:55
2009 +0000
+++ b/linux/drivers/media/video/cx88/cx88-input.c Sat May 30 14:16:24
2009 +0200
@@ -23,10 +23,10 @@
*/
#include <linux/init.h>
-#include <linux/delay.h>
#include <linux/input.h>
#include <linux/pci.h>
#include <linux/module.h>
+#include <linux/hrtimer.h>
#include "compat.h"
#include "cx88.h"
@@ -49,7 +49,7 @@ struct cx88_IR {
/* poll external decoder */
int polling;
- struct delayed_work work;
+ struct hrtimer timer;
u32 gpio_addr;
u32 last_gpio;
u32 mask_keycode;
@@ -144,31 +144,27 @@ static void cx88_ir_handle_key(struct cx
}
}
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20)
-static void cx88_ir_work(void *data)
-#else
-static void cx88_ir_work(struct work_struct *work)
-#endif
+enum hrtimer_restart cx88_ir_work(struct hrtimer *timer)
{
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20)
- struct cx88_IR *ir = data;
-#else
- struct cx88_IR *ir = container_of(work, struct cx88_IR, work.work);
-#endif
+ unsigned long missed;
+ struct cx88_IR *ir = container_of(timer, struct cx88_IR, timer);
cx88_ir_handle_key(ir);
- schedule_delayed_work(&ir->work, msecs_to_jiffies(ir->polling));
+ missed = hrtimer_forward_now(&ir->timer,
+ ktime_set(0, ir->polling * 1000000));
+ if (missed > 1)
+ ir_dprintk("Missed ticks %ld\n", missed - 1);
+
+ return HRTIMER_RESTART;
}
void cx88_ir_start(struct cx88_core *core, struct cx88_IR *ir)
{
if (ir->polling) {
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20)
- INIT_DELAYED_WORK(&ir->work, cx88_ir_work, ir);
-#else
- INIT_DELAYED_WORK(&ir->work, cx88_ir_work);
-#endif
- schedule_delayed_work(&ir->work, 0);
+ hrtimer_init(&ir->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+ ir->timer.function = cx88_ir_work;
+ hrtimer_start(&ir->timer, ktime_set(0, ir->polling * 1000000),
+ HRTIMER_MODE_REL);
}
if (ir->sampling) {
core->pci_irqmask |= PCI_INT_IR_SMPINT;
@@ -185,7 +181,7 @@ void cx88_ir_stop(struct cx88_core *core
}
if (ir->polling)
- cancel_delayed_work_sync(&ir->work);
+ hrtimer_cancel(&ir->timer);
}
/*
---------------------------------------------------------------------- */
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] cx88: High resolution timer for Remote Controls
@ 2009-07-02 14:50 Jean Delvare
2009-07-03 20:37 ` Jean Delvare
0 siblings, 1 reply; 3+ messages in thread
From: Jean Delvare @ 2009-07-02 14:50 UTC (permalink / raw)
To: LMML; +Cc: Andrzej Hajda, Trent Piepho
From: Andrzej Hajda <andrzej.hajda@wp.pl>
Patch solves problem of missed keystrokes on some remote controls,
as reported on http://bugzilla.kernel.org/show_bug.cgi?id=9637 .
Signed-off-by: Andrzej Hajda <andrzej.hajda@wp.pl>
Signed-off-by: Jean Delvare <khali@linux-fr.org>
---
Resending because last attempt resulted in folded lines:
http://www.spinics.net/lists/linux-media/msg06884.html
Patch was already resent by Andrzej on June 4th but apparently it was
overlooked.
Trent Piepho commented on the compatibility with kernels older than
2.6.20 being possibly broken:
http://www.spinics.net/lists/linux-media/msg06885.html
I don't think this is the case. The kernel version test was there
because the workqueue API changed in 2.6.20, but the hrtimer API did
not have such a change. This is why the version check has gone.
It is highly probable that the hrtimer API had its own incompatible
changes since it was introduced in kernel 2.6.16. By looking at the
code, I found the following ones:
* hrtimer_forward_now() was added with kernel 2.6.25 only:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=5e05ad7d4e3b11f935998882b5d9c3b257137f1b
But this is an inline function, so I presume this shouldn't be too
difficult to add to a compatibility header.
* Before 2.6.21, HRTIMER_MODE_REL was named HRTIMER_REL:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=c9cb2e3d7c9178ab75d0942f96abb3abe0369906
This too should be solvable in a compatibility header.
The rest doesn't seem to cause compatibility issues, but only actual
testing would confirm that.
This bug affects me, which is why I am motivated to get this fix
upstream. Please let me know how I can help.
linux/drivers/media/video/cx88/cx88-input.c | 37 ++++++++++++---------------
1 file changed, 17 insertions(+), 20 deletions(-)
--- v4l-dvb.orig/linux/drivers/media/video/cx88/cx88-input.c 2009-07-02 15:13:08.000000000 +0200
+++ v4l-dvb/linux/drivers/media/video/cx88/cx88-input.c 2009-07-02 15:35:04.000000000 +0200
@@ -23,10 +23,10 @@
*/
#include <linux/init.h>
-#include <linux/delay.h>
#include <linux/input.h>
#include <linux/pci.h>
#include <linux/module.h>
+#include <linux/hrtimer.h>
#include "compat.h"
#include "cx88.h"
@@ -49,7 +49,7 @@ struct cx88_IR {
/* poll external decoder */
int polling;
- struct delayed_work work;
+ struct hrtimer timer;
u32 gpio_addr;
u32 last_gpio;
u32 mask_keycode;
@@ -145,31 +145,28 @@ static void cx88_ir_handle_key(struct cx
}
}
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20)
-static void cx88_ir_work(void *data)
-#else
-static void cx88_ir_work(struct work_struct *work)
-#endif
+enum hrtimer_restart cx88_ir_work(struct hrtimer *timer)
{
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20)
- struct cx88_IR *ir = data;
-#else
- struct cx88_IR *ir = container_of(work, struct cx88_IR, work.work);
-#endif
+ unsigned long missed;
+ struct cx88_IR *ir = container_of(timer, struct cx88_IR, timer);
cx88_ir_handle_key(ir);
- schedule_delayed_work(&ir->work, msecs_to_jiffies(ir->polling));
+ missed = hrtimer_forward_now(&ir->timer,
+ ktime_set(0, ir->polling * 1000000));
+ if (missed > 1)
+ ir_dprintk("Missed ticks %ld\n", missed - 1);
+
+ return HRTIMER_RESTART;
}
void cx88_ir_start(struct cx88_core *core, struct cx88_IR *ir)
{
if (ir->polling) {
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20)
- INIT_DELAYED_WORK(&ir->work, cx88_ir_work, ir);
-#else
- INIT_DELAYED_WORK(&ir->work, cx88_ir_work);
-#endif
- schedule_delayed_work(&ir->work, 0);
+ hrtimer_init(&ir->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+ ir->timer.function = cx88_ir_work;
+ hrtimer_start(&ir->timer,
+ ktime_set(0, ir->polling * 1000000),
+ HRTIMER_MODE_REL);
}
if (ir->sampling) {
core->pci_irqmask |= PCI_INT_IR_SMPINT;
@@ -186,7 +183,7 @@ void cx88_ir_stop(struct cx88_core *core
}
if (ir->polling)
- cancel_delayed_work_sync(&ir->work);
+ hrtimer_cancel(&ir->timer);
}
/* ---------------------------------------------------------------------- */
--
Jean Delvare
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] cx88: High resolution timer for Remote Controls
2009-07-02 14:50 Jean Delvare
@ 2009-07-03 20:37 ` Jean Delvare
0 siblings, 0 replies; 3+ messages in thread
From: Jean Delvare @ 2009-07-03 20:37 UTC (permalink / raw)
To: LMML; +Cc: Andrzej Hajda, Trent Piepho
On Thu, 2 Jul 2009 16:50:35 +0200, Jean Delvare wrote:
> From: Andrzej Hajda <andrzej.hajda@wp.pl>
>
> Patch solves problem of missed keystrokes on some remote controls,
> as reported on http://bugzilla.kernel.org/show_bug.cgi?id=9637 .
>
> Signed-off-by: Andrzej Hajda <andrzej.hajda@wp.pl>
> Signed-off-by: Jean Delvare <khali@linux-fr.org>
> ---
> Resending because last attempt resulted in folded lines:
> http://www.spinics.net/lists/linux-media/msg06884.html
> Patch was already resent by Andrzej on June 4th but apparently it was
> overlooked.
>
> Trent Piepho commented on the compatibility with kernels older than
> 2.6.20 being possibly broken:
> http://www.spinics.net/lists/linux-media/msg06885.html
> I don't think this is the case. The kernel version test was there
> because the workqueue API changed in 2.6.20, but the hrtimer API did
> not have such a change. This is why the version check has gone.
>
> It is highly probable that the hrtimer API had its own incompatible
> changes since it was introduced in kernel 2.6.16. By looking at the
> code, I found the following ones:
>
> * hrtimer_forward_now() was added with kernel 2.6.25 only:
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=5e05ad7d4e3b11f935998882b5d9c3b257137f1b
> But this is an inline function, so I presume this shouldn't be too
> difficult to add to a compatibility header.
>
> * Before 2.6.21, HRTIMER_MODE_REL was named HRTIMER_REL:
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=c9cb2e3d7c9178ab75d0942f96abb3abe0369906
> This too should be solvable in a compatibility header.
>
> The rest doesn't seem to cause compatibility issues, but only actual
> testing would confirm that.
Actually there were more compatibility issues, the most important one
being that not all functions of the hrtimer API are exported before
2.6.22. So unfortunately this bug fix means that the cx88 driver will
no longer build on kernels < 2.6.22. I'll post a new patch with this
change, and another one for the hrtimer compatibility code.
--
Jean Delvare
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2009-07-03 20:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-06-04 16:49 [PATCH] cx88: High resolution timer for Remote Controls AH
-- strict thread matches above, loose matches on Subject: below --
2009-07-02 14:50 Jean Delvare
2009-07-03 20:37 ` Jean Delvare
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox