| Message ID | 1353251589-26143-2-git-send-email-timo.t.kokkonen@iki.fi (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, * Timo Kokkonen <timo.t.kokkonen@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 majordomo@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Fri, Dec 14, 2012 at 09:28:09AM -0800, Tony Lindgren wrote: > * Tony Lindgren <tony@atomide.com> [121120 12:00]: > > Hi, > > > > * Timo Kokkonen <timo.t.kokkonen@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.
* Tony Lindgren <tony@atomide.com> [121120 12:00]: > Hi, > > * Timo Kokkonen <timo.t.kokkonen@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 majordomo@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 majordomo@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Felipe Balbi <balbi@ti.com> [121214 09:36]: > Hi, > > On Fri, Dec 14, 2012 at 09:28:09AM -0800, Tony Lindgren wrote: > > * Tony Lindgren <tony@atomide.com> [121120 12:00]: > > > Hi, > > > > > > * Timo Kokkonen <timo.t.kokkonen@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Fri, Dec 14, 2012 at 09:46:29AM -0800, Tony Lindgren wrote: > * Felipe Balbi <balbi@ti.com> [121214 09:36]: > > Hi, > > > > On Fri, Dec 14, 2012 at 09:28:09AM -0800, Tony Lindgren wrote: > > > * Tony Lindgren <tony@atomide.com> [121120 12:00]: > > > > Hi, > > > > > > > > * Timo Kokkonen <timo.t.kokkonen@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 ;-)
* Felipe Balbi <balbi@ti.com> [121214 09:59]: > On Fri, Dec 14, 2012 at 09:46:29AM -0800, Tony Lindgren wrote: > > * Felipe Balbi <balbi@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Fri, Dec 14, 2012 at 10:06:45AM -0800, Tony Lindgren wrote: > * Felipe Balbi <balbi@ti.com> [121214 09:59]: > > On Fri, Dec 14, 2012 at 09:46:29AM -0800, Tony Lindgren wrote: > > > * Felipe Balbi <balbi@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
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 <tony@atomide.com> [121120 12:00]: >>> Hi, >>> >>> * Timo Kokkonen <timo.t.kokkonen@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Timo Kokkonen <timo.t.kokkonen@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c index 546199e..125d4c3 100644 --- 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; @@ -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);
The lirc-dev expects the ir-code to be transmitted when the write call returns back to the user space. We should not leave TX ongoing no matter what is the reason we return to the user space. Easiest solution for that is to simply remove interruptible sleeps. The first wait_event_interruptible is thus replaced with return -EBUSY in case there is still ongoing transfer. This should suffice as the concept of sending multiple codes in parallel does not make sense. The second wait_event_interruptible call is replaced with wait_even_timeout with a fixed and safe timeout that should prevent the process from getting stuck in kernel for too long. Also, from now on we will force the TX to stop before we return from write call. If the TX happened to time out for some reason, we should not leave the HW transmitting anything. Signed-off-by: Timo Kokkonen <timo.t.kokkonen@iki.fi> --- drivers/media/rc/ir-rx51.c | 39 ++++++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 11 deletions(-)