Re: [PATCH 1/7] ir-rx51: Handle signals properly
* Tony Lindgren t...@atomide.com [121120 12:00]:
Hi,
* Timo Kokkonen timo.t.kokko...@iki.fi [121118 07:15]:
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -74,6 +74,19 @@ static void lirc_rx51_off(struct lirc_rx51 *lirc_rx51)
OMAP_TIMER_TRIGGER_NONE);
}
+static void lirc_rx51_stop_tx(struct lirc_rx51 *lirc_rx51)
+{
+ if (lirc_rx51-wbuf_index 0)
+ return;
+
+ lirc_rx51_off(lirc_rx51);
+ lirc_rx51-wbuf_index = -1;
+ omap_dm_timer_stop(lirc_rx51-pwm_timer);
+ omap_dm_timer_stop(lirc_rx51-pulse_timer);
+ omap_dm_timer_set_int_enable(lirc_rx51-pulse_timer, 0);
+ wake_up(lirc_rx51-wqueue);
+}
+
static int init_timing_params(struct lirc_rx51 *lirc_rx51)
{
u32 load, match;
Good fixes in general.. But you won't be able to access the
omap_dm_timer functions after we enable ARM multiplatform support
for omap2+. That's for v3.9 probably right after v3.8-rc1.
We need to find some Linux generic API to use hardware timers
like this, so I've added Thomas Gleixner and linux-arm-kernel
mailing list to cc.
If no such API is available, then maybe we can export some of
the omap_dm_timer functions if Thomas is OK with that.
Just to update the status on this.. It seems that we'll be moving
parts of plat/dmtimer into a minimal include/linux/timer-omap.h
unless people have better ideas on what to do with custom
hardware timers for PWM etc.
The other alternative is to set them up as platform_data
function pointers, but that won't work after we make omap2+
device tree only. And that really just postpones the problem.
Cheers,
Tony
@@ -163,13 +176,7 @@ static irqreturn_t lirc_rx51_interrupt_handler(int
irq, void *ptr)
return IRQ_HANDLED;
end:
- /* Stop TX here */
- lirc_rx51_off(lirc_rx51);
- lirc_rx51-wbuf_index = -1;
- omap_dm_timer_stop(lirc_rx51-pwm_timer);
- omap_dm_timer_stop(lirc_rx51-pulse_timer);
- omap_dm_timer_set_int_enable(lirc_rx51-pulse_timer, 0);
- wake_up_interruptible(lirc_rx51-wqueue);
+ lirc_rx51_stop_tx(lirc_rx51);
return IRQ_HANDLED;
}
@@ -249,8 +256,9 @@ static ssize_t lirc_rx51_write(struct file *file, const
char *buf,
if ((count WBUF_LEN) || (count % 2 == 0))
return -EINVAL;
- /* Wait any pending transfers to finish */
- wait_event_interruptible(lirc_rx51-wqueue, lirc_rx51-wbuf_index 0);
+ /* We can have only one transmit at a time */
+ if (lirc_rx51-wbuf_index = 0)
+ return -EBUSY;
if (copy_from_user(lirc_rx51-wbuf, buf, n))
return -EFAULT;
@@ -276,9 +284,18 @@ static ssize_t lirc_rx51_write(struct file *file,
const char *buf,
/*
* Don't return back to the userspace until the transfer has
-* finished
+* finished. However, we wish to not spend any more than 500ms
+* in kernel. No IR code TX should ever take that long.
+*/
+ i = wait_event_timeout(lirc_rx51-wqueue, lirc_rx51-wbuf_index 0,
+ HZ / 2);
+
+ /*
+* Ensure transmitting has really stopped, even if the timers
+* went mad or something else happened that caused it still
+* sending out something.
*/
- wait_event_interruptible(lirc_rx51-wqueue, lirc_rx51-wbuf_index 0);
+ lirc_rx51_stop_tx(lirc_rx51);
/* We can sleep again */
lirc_rx51-pdata-set_max_mpu_wakeup_lat(lirc_rx51-dev, -1);
--
1.8.0
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
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 1/7] ir-rx51: Handle signals properly
Hi,
On Fri, Dec 14, 2012 at 09:28:09AM -0800, Tony Lindgren wrote:
* Tony Lindgren t...@atomide.com [121120 12:00]:
Hi,
* Timo Kokkonen timo.t.kokko...@iki.fi [121118 07:15]:
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -74,6 +74,19 @@ static void lirc_rx51_off(struct lirc_rx51 *lirc_rx51)
OMAP_TIMER_TRIGGER_NONE);
}
+static void lirc_rx51_stop_tx(struct lirc_rx51 *lirc_rx51)
+{
+ if (lirc_rx51-wbuf_index 0)
+ return;
+
+ lirc_rx51_off(lirc_rx51);
+ lirc_rx51-wbuf_index = -1;
+ omap_dm_timer_stop(lirc_rx51-pwm_timer);
+ omap_dm_timer_stop(lirc_rx51-pulse_timer);
+ omap_dm_timer_set_int_enable(lirc_rx51-pulse_timer, 0);
+ wake_up(lirc_rx51-wqueue);
+}
+
static int init_timing_params(struct lirc_rx51 *lirc_rx51)
{
u32 load, match;
Good fixes in general.. But you won't be able to access the
omap_dm_timer functions after we enable ARM multiplatform support
for omap2+. That's for v3.9 probably right after v3.8-rc1.
We need to find some Linux generic API to use hardware timers
like this, so I've added Thomas Gleixner and linux-arm-kernel
mailing list to cc.
If no such API is available, then maybe we can export some of
the omap_dm_timer functions if Thomas is OK with that.
Just to update the status on this.. It seems that we'll be moving
parts of plat/dmtimer into a minimal include/linux/timer-omap.h
unless people have better ideas on what to do with custom
hardware timers for PWM etc.
if it's really for PWM, shouldn't we be using drivers/pwm/ ??
Meaning that $SUBJECT would just request a PWM device and use it. That
doesn't solve the whole problem, however, as pwm-omap.c would still need
access to timer-omap.h.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 1/7] ir-rx51: Handle signals properly
* Felipe Balbi ba...@ti.com [121214 09:36]:
Hi,
On Fri, Dec 14, 2012 at 09:28:09AM -0800, Tony Lindgren wrote:
* Tony Lindgren t...@atomide.com [121120 12:00]:
Hi,
* Timo Kokkonen timo.t.kokko...@iki.fi [121118 07:15]:
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -74,6 +74,19 @@ static void lirc_rx51_off(struct lirc_rx51
*lirc_rx51)
OMAP_TIMER_TRIGGER_NONE);
}
+static void lirc_rx51_stop_tx(struct lirc_rx51 *lirc_rx51)
+{
+ if (lirc_rx51-wbuf_index 0)
+ return;
+
+ lirc_rx51_off(lirc_rx51);
+ lirc_rx51-wbuf_index = -1;
+ omap_dm_timer_stop(lirc_rx51-pwm_timer);
+ omap_dm_timer_stop(lirc_rx51-pulse_timer);
+ omap_dm_timer_set_int_enable(lirc_rx51-pulse_timer, 0);
+ wake_up(lirc_rx51-wqueue);
+}
+
static int init_timing_params(struct lirc_rx51 *lirc_rx51)
{
u32 load, match;
Good fixes in general.. But you won't be able to access the
omap_dm_timer functions after we enable ARM multiplatform support
for omap2+. That's for v3.9 probably right after v3.8-rc1.
We need to find some Linux generic API to use hardware timers
like this, so I've added Thomas Gleixner and linux-arm-kernel
mailing list to cc.
If no such API is available, then maybe we can export some of
the omap_dm_timer functions if Thomas is OK with that.
Just to update the status on this.. It seems that we'll be moving
parts of plat/dmtimer into a minimal include/linux/timer-omap.h
unless people have better ideas on what to do with custom
hardware timers for PWM etc.
if it's really for PWM, shouldn't we be using drivers/pwm/ ??
Meaning that $SUBJECT would just request a PWM device and use it. That
doesn't solve the whole problem, however, as pwm-omap.c would still need
access to timer-omap.h.
That would only help with omap_dm_timer_set_pwm() I think.
The other functions are also needed by the clocksource and clockevent
drivers. And tidspbridge too:
$ grep -r omap_dm_timer drivers/
...
Regards,
Tony
--
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 1/7] ir-rx51: Handle signals properly
Hi,
On Fri, Dec 14, 2012 at 09:46:29AM -0800, Tony Lindgren wrote:
* Felipe Balbi ba...@ti.com [121214 09:36]:
Hi,
On Fri, Dec 14, 2012 at 09:28:09AM -0800, Tony Lindgren wrote:
* Tony Lindgren t...@atomide.com [121120 12:00]:
Hi,
* Timo Kokkonen timo.t.kokko...@iki.fi [121118 07:15]:
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -74,6 +74,19 @@ static void lirc_rx51_off(struct lirc_rx51
*lirc_rx51)
OMAP_TIMER_TRIGGER_NONE);
}
+static void lirc_rx51_stop_tx(struct lirc_rx51 *lirc_rx51)
+{
+ if (lirc_rx51-wbuf_index 0)
+ return;
+
+ lirc_rx51_off(lirc_rx51);
+ lirc_rx51-wbuf_index = -1;
+ omap_dm_timer_stop(lirc_rx51-pwm_timer);
+ omap_dm_timer_stop(lirc_rx51-pulse_timer);
+ omap_dm_timer_set_int_enable(lirc_rx51-pulse_timer, 0);
+ wake_up(lirc_rx51-wqueue);
+}
+
static int init_timing_params(struct lirc_rx51 *lirc_rx51)
{
u32 load, match;
Good fixes in general.. But you won't be able to access the
omap_dm_timer functions after we enable ARM multiplatform support
for omap2+. That's for v3.9 probably right after v3.8-rc1.
We need to find some Linux generic API to use hardware timers
like this, so I've added Thomas Gleixner and linux-arm-kernel
mailing list to cc.
If no such API is available, then maybe we can export some of
the omap_dm_timer functions if Thomas is OK with that.
Just to update the status on this.. It seems that we'll be moving
parts of plat/dmtimer into a minimal include/linux/timer-omap.h
unless people have better ideas on what to do with custom
hardware timers for PWM etc.
if it's really for PWM, shouldn't we be using drivers/pwm/ ??
Meaning that $SUBJECT would just request a PWM device and use it. That
doesn't solve the whole problem, however, as pwm-omap.c would still need
access to timer-omap.h.
That would only help with omap_dm_timer_set_pwm() I think.
The other functions are also needed by the clocksource and clockevent
drivers. And tidspbridge too:
well, we _do_ have drivers/clocksource ;-)
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 1/7] ir-rx51: Handle signals properly
* Felipe Balbi ba...@ti.com [121214 09:59]: On Fri, Dec 14, 2012 at 09:46:29AM -0800, Tony Lindgren wrote: * Felipe Balbi ba...@ti.com [121214 09:36]: if it's really for PWM, shouldn't we be using drivers/pwm/ ?? Meaning that $SUBJECT would just request a PWM device and use it. That doesn't solve the whole problem, however, as pwm-omap.c would still need access to timer-omap.h. That would only help with omap_dm_timer_set_pwm() I think. The other functions are also needed by the clocksource and clockevent drivers. And tidspbridge too: well, we _do_ have drivers/clocksource ;-) That's where the dmtimer code should live. But still it does not help with the header. Thomas, maybe we could use the hrtimer framework for it if there was some way to completely leave out the rb tree for the dedicated hardware timers? There's no queue needed as there's always just one value tied to a specific timer. Regards, Tony -- 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 1/7] ir-rx51: Handle signals properly
Hi, On Fri, Dec 14, 2012 at 10:06:45AM -0800, Tony Lindgren wrote: * Felipe Balbi ba...@ti.com [121214 09:59]: On Fri, Dec 14, 2012 at 09:46:29AM -0800, Tony Lindgren wrote: * Felipe Balbi ba...@ti.com [121214 09:36]: if it's really for PWM, shouldn't we be using drivers/pwm/ ?? Meaning that $SUBJECT would just request a PWM device and use it. That doesn't solve the whole problem, however, as pwm-omap.c would still need access to timer-omap.h. That would only help with omap_dm_timer_set_pwm() I think. The other functions are also needed by the clocksource and clockevent drivers. And tidspbridge too: well, we _do_ have drivers/clocksource ;-) That's where the dmtimer code should live. But still it does not help with the header. yeah, the header should be where you suggested, no doubts. I was actually criticizing the current timer code. cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH 1/7] ir-rx51: Handle signals properly
On 12/14/12 19:26, Felipe Balbi wrote:
Hi,
On Fri, Dec 14, 2012 at 09:28:09AM -0800, Tony Lindgren wrote:
* Tony Lindgren t...@atomide.com [121120 12:00]:
Hi,
* Timo Kokkonen timo.t.kokko...@iki.fi [121118 07:15]:
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -74,6 +74,19 @@ static void lirc_rx51_off(struct lirc_rx51 *lirc_rx51)
OMAP_TIMER_TRIGGER_NONE);
}
+static void lirc_rx51_stop_tx(struct lirc_rx51 *lirc_rx51)
+{
+ if (lirc_rx51-wbuf_index 0)
+ return;
+
+ lirc_rx51_off(lirc_rx51);
+ lirc_rx51-wbuf_index = -1;
+ omap_dm_timer_stop(lirc_rx51-pwm_timer);
+ omap_dm_timer_stop(lirc_rx51-pulse_timer);
+ omap_dm_timer_set_int_enable(lirc_rx51-pulse_timer, 0);
+ wake_up(lirc_rx51-wqueue);
+}
+
static int init_timing_params(struct lirc_rx51 *lirc_rx51)
{
u32 load, match;
Good fixes in general.. But you won't be able to access the
omap_dm_timer functions after we enable ARM multiplatform support
for omap2+. That's for v3.9 probably right after v3.8-rc1.
We need to find some Linux generic API to use hardware timers
like this, so I've added Thomas Gleixner and linux-arm-kernel
mailing list to cc.
If no such API is available, then maybe we can export some of
the omap_dm_timer functions if Thomas is OK with that.
Just to update the status on this.. It seems that we'll be moving
parts of plat/dmtimer into a minimal include/linux/timer-omap.h
unless people have better ideas on what to do with custom
hardware timers for PWM etc.
if it's really for PWM, shouldn't we be using drivers/pwm/ ??
Now that Neil Brown posted the PWM driver for omap, I've been thinking
about whether converting the ir-rx51 into the PWM API would work. Maybe
controlling the PWM itself would be sufficient, but the ir-rx51 uses
also another dmtimer for creating accurate (enough) timing source for
the IR pulse edges.
I haven't tried whether the default 32kHz clock source is enough for
that. Now that I think about it, I don't see why it wouldn't be good
enough. I think it would even be possible to just use the PWM api alone
(plus hr-timers) in order to generate good enough IR pulses.
-Timo
Meaning that $SUBJECT would just request a PWM device and use it. That
doesn't solve the whole problem, however, as pwm-omap.c would still need
access to timer-omap.h.
--
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 1/7] ir-rx51: Handle signals properly
* Timo Kokkonen timo.t.kokko...@iki.fi [121214 11:33]: On 12/14/12 19:26, Felipe Balbi wrote: if it's really for PWM, shouldn't we be using drivers/pwm/ ?? Now that Neil Brown posted the PWM driver for omap, I've been thinking about whether converting the ir-rx51 into the PWM API would work. Maybe controlling the PWM itself would be sufficient, but the ir-rx51 uses also another dmtimer for creating accurate (enough) timing source for the IR pulse edges. OK. I haven't tried whether the default 32kHz clock source is enough for that. Now that I think about it, I don't see why it wouldn't be good enough. I think it would even be possible to just use the PWM api alone Cool. Regards, Tony -- 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 1/7] ir-rx51: Handle signals properly
Hi,
* Timo Kokkonen timo.t.kokko...@iki.fi [121118 07:15]:
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -74,6 +74,19 @@ static void lirc_rx51_off(struct lirc_rx51 *lirc_rx51)
OMAP_TIMER_TRIGGER_NONE);
}
+static void lirc_rx51_stop_tx(struct lirc_rx51 *lirc_rx51)
+{
+ if (lirc_rx51-wbuf_index 0)
+ return;
+
+ lirc_rx51_off(lirc_rx51);
+ lirc_rx51-wbuf_index = -1;
+ omap_dm_timer_stop(lirc_rx51-pwm_timer);
+ omap_dm_timer_stop(lirc_rx51-pulse_timer);
+ omap_dm_timer_set_int_enable(lirc_rx51-pulse_timer, 0);
+ wake_up(lirc_rx51-wqueue);
+}
+
static int init_timing_params(struct lirc_rx51 *lirc_rx51)
{
u32 load, match;
Good fixes in general.. But you won't be able to access the
omap_dm_timer functions after we enable ARM multiplatform support
for omap2+. That's for v3.9 probably right after v3.8-rc1.
We need to find some Linux generic API to use hardware timers
like this, so I've added Thomas Gleixner and linux-arm-kernel
mailing list to cc.
If no such API is available, then maybe we can export some of
the omap_dm_timer functions if Thomas is OK with that.
The other alternative is to set them up as platform_data
function pointers, but that won't work after we make omap2+
device tree only. And that really just postpones the problem.
Cheers,
Tony
@@ -163,13 +176,7 @@ static irqreturn_t lirc_rx51_interrupt_handler(int irq,
void *ptr)
return IRQ_HANDLED;
end:
- /* Stop TX here */
- lirc_rx51_off(lirc_rx51);
- lirc_rx51-wbuf_index = -1;
- omap_dm_timer_stop(lirc_rx51-pwm_timer);
- omap_dm_timer_stop(lirc_rx51-pulse_timer);
- omap_dm_timer_set_int_enable(lirc_rx51-pulse_timer, 0);
- wake_up_interruptible(lirc_rx51-wqueue);
+ lirc_rx51_stop_tx(lirc_rx51);
return IRQ_HANDLED;
}
@@ -249,8 +256,9 @@ static ssize_t lirc_rx51_write(struct file *file, const
char *buf,
if ((count WBUF_LEN) || (count % 2 == 0))
return -EINVAL;
- /* Wait any pending transfers to finish */
- wait_event_interruptible(lirc_rx51-wqueue, lirc_rx51-wbuf_index 0);
+ /* We can have only one transmit at a time */
+ if (lirc_rx51-wbuf_index = 0)
+ return -EBUSY;
if (copy_from_user(lirc_rx51-wbuf, buf, n))
return -EFAULT;
@@ -276,9 +284,18 @@ static ssize_t lirc_rx51_write(struct file *file, const
char *buf,
/*
* Don't return back to the userspace until the transfer has
- * finished
+ * finished. However, we wish to not spend any more than 500ms
+ * in kernel. No IR code TX should ever take that long.
+ */
+ i = wait_event_timeout(lirc_rx51-wqueue, lirc_rx51-wbuf_index 0,
+ HZ / 2);
+
+ /*
+ * Ensure transmitting has really stopped, even if the timers
+ * went mad or something else happened that caused it still
+ * sending out something.
*/
- wait_event_interruptible(lirc_rx51-wqueue, lirc_rx51-wbuf_index 0);
+ lirc_rx51_stop_tx(lirc_rx51);
/* We can sleep again */
lirc_rx51-pdata-set_max_mpu_wakeup_lat(lirc_rx51-dev, -1);
--
1.8.0
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
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
