Re: [PATCH] [media] ir: IR_RX51 only works on OMAP2
On 03.14 2013 22:56:44, Arnd Bergmann wrote: > This driver can be enabled on OMAP1 at the moment, which breaks > allyesconfig for that platform. Let's mark it OMAP2PLUS-only > in Kconfig, since that is the only thing it builds on. > Acked-by: Timo Kokkonen Thanks! > Signed-off-by: Arnd Bergmann > Cc: Mauro Carvalho Chehab > Cc: Timo Kokkonen > Cc: Tony Lindgren > Cc: Laurent Pinchart > Cc: linux-media@vger.kernel.org > --- > Mauro, please apply for 3.9 > > drivers/media/rc/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig > index 19f3563..5a79c33 100644 > --- a/drivers/media/rc/Kconfig > +++ b/drivers/media/rc/Kconfig > @@ -291,7 +291,7 @@ config IR_TTUSBIR > > config IR_RX51 > tristate "Nokia N900 IR transmitter diode" > - depends on OMAP_DM_TIMER && LIRC && !ARCH_MULTIPLATFORM > + depends on OMAP_DM_TIMER && ARCH_OMAP2PLUS && LIRC && > !ARCH_MULTIPLATFORM > ---help--- > Say Y or M here if you want to enable support for the IR > transmitter diode built in the Nokia N900 (RX51) device. > -- > 1.8.1.2 > -- 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 6/9] [media] ir-rx51: fix clock API related build issues
On 03.05 2013 17:09:53, Tony Lindgren wrote:
> * Mauro Carvalho Chehab [130305 16:28]:
> > Em Tue, 5 Mar 2013 23:16:46 +0100
> > Arnd Bergmann escreveu:
> >
> > > OMAP1 no longer provides its own clock interfaces since patch
> > > a135eaae52 "ARM: OMAP: remove plat/clock.h". This is great, but
> > > we now have to convert the ir-rx51 driver to use the generic
> > > interface from linux/clk.h.
> > >
> > > The driver also uses the omap_dm_timer_get_fclk() function,
> > > which is not exported for OMAP1, so we have to move the
> > > definition out of the OMAP2 specific section.
> > >
> > > Signed-off-by: Arnd Bergmann
> > > Cc: Mauro Carvalho Chehab
> >
> > From my side:
> > Acked-by: Mauro Carvalho Chehab
>
> There's just one issue, this driver most likely only needed on
> rx51 board.. So I suggest we just mark the driver depends on
> ARCH_OMAP2PLUS and let's drop this patch.
>
> This driver is already disabled for ARCH_MULTIPLATFORM
> as we need to move dmtimer.c to drivers and have some minimal
> include/linux/timer-omap.h for it.
>
I've also had this cunning plan that if or when the PWM subsystem
starts supporting the PWM output in OMAP3, I could convert this driver
to generate the IR carrier wave through the PWM subsystem and then use
HR timers to generate the pulses. I think that's much better approach
than trying to depend on interfaces that are not easily
available. Should be possible, but I haven't proven yet that it will
work :)
Unfortunately I haven't got into executing on that plan yet. In
addition to the challenge of scheduling some of my free time for doing
this, my RX51 device is not enumerating the USB with the latest kernel
and I haven't figured out that yet. And because of that, I haven't
been able to get my user space running over nfsroot setup I've been
using..
-Timo
> > > --- a/arch/arm/plat-omap/dmtimer.c
> > > +++ b/arch/arm/plat-omap/dmtimer.c
> > > @@ -333,6 +333,14 @@ int omap_dm_timer_get_irq(struct omap_dm_timer
> > > *timer)
> > > }
> > > EXPORT_SYMBOL_GPL(omap_dm_timer_get_irq);
> > >
> > > +struct clk *omap_dm_timer_get_fclk(struct omap_dm_timer *timer)
> > > +{
> > > + if (timer)
> > > + return timer->fclk;
> > > + return NULL;
> > > +}
> > > +EXPORT_SYMBOL_GPL(omap_dm_timer_get_fclk);
> > > +
> > > #if defined(CONFIG_ARCH_OMAP1)
> > > #include
> > > /**
> > > @@ -371,14 +379,6 @@ EXPORT_SYMBOL_GPL(omap_dm_timer_modify_idlect_mask);
> > >
> > > #else
> > >
> > > -struct clk *omap_dm_timer_get_fclk(struct omap_dm_timer *timer)
> > > -{
> > > - if (timer)
> > > - return timer->fclk;
> > > - return NULL;
> > > -}
> > > -EXPORT_SYMBOL_GPL(omap_dm_timer_get_fclk);
> > > -
> > > __u32 omap_dm_timer_modify_idlect_mask(__u32 inputmask)
> > > {
> > > BUG();
>
> Then omap_dm_timer_get_fclk() won't work on omap1 as there's no
> separate functional clock. We probably should not even export
> this function eventually when things are fixed up.
>
> 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 v2] Media: remove incorrect __init/__exit markups
On 02.25 2013 23:17:27, Dmitry Torokhov wrote:
> Even if bus is not hot-pluggable, the devices can be unbound from the
> driver via sysfs, so we should not be using __exit annotations on
> remove() methods. The only exception is drivers registered with
> platform_driver_probe() which specifically disables sysfs bind/unbind
> attributes.
>
> Similarly probe() methods should not be marked __init unless
> platform_driver_probe() is used.
>
> Signed-off-by: Dmitry Torokhov
> ---
>
> v1->v2: removed __init markup on omap1_cam_probe() that was pointed out
> by Guennadi Liakhovetski.
>
> drivers/media/i2c/adp1653.c | 4 ++--
> drivers/media/i2c/smiapp/smiapp-core.c | 4 ++--
> drivers/media/platform/soc_camera/omap1_camera.c | 6 +++---
> drivers/media/radio/radio-si4713.c | 4 ++--
> drivers/media/rc/ir-rx51.c | 4 ++--
> 5 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
> index 8ead492..31b955b 100644
> --- a/drivers/media/rc/ir-rx51.c
> +++ b/drivers/media/rc/ir-rx51.c
> @@ -464,14 +464,14 @@ static int lirc_rx51_probe(struct platform_device *dev)
> return 0;
> }
>
> -static int __exit lirc_rx51_remove(struct platform_device *dev)
> +static int lirc_rx51_remove(struct platform_device *dev)
> {
> return lirc_unregister_driver(lirc_rx51_driver.minor);
> }
>
> struct platform_driver lirc_rx51_platform_driver = {
> .probe = lirc_rx51_probe,
> - .remove = __exit_p(lirc_rx51_remove),
> + .remove = lirc_rx51_remove,
> .suspend = lirc_rx51_suspend,
> .resume = lirc_rx51_resume,
> .driver = {
For ir-rx51:
Acked-by: Timo Kokkonen
Thanks!
-Timo
--
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
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 [121120 12:00]:
>>> Hi,
>>>
>>> * Timo Kokkonen [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
[PATCH 4/7] ir-rx51: Replace module_{init,exit} macros with module_platform_driver
No reason to avoid using the existing helpers.
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/ir-rx51.c | 12 +---
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 16b3c1f..6e1ffa6 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -495,17 +495,7 @@ struct platform_driver lirc_rx51_platform_driver = {
},
};
-static int __init lirc_rx51_init(void)
-{
- return platform_driver_register(&lirc_rx51_platform_driver);
-}
-module_init(lirc_rx51_init);
-
-static void __exit lirc_rx51_exit(void)
-{
- platform_driver_unregister(&lirc_rx51_platform_driver);
-}
-module_exit(lirc_rx51_exit);
+module_platform_driver(lirc_rx51_platform_driver);
MODULE_DESCRIPTION("LIRC TX driver for Nokia RX51");
MODULE_AUTHOR("Nokia Corporation");
--
1.8.0
--
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 6/7] ir-rx51: Remove useless variable from struct lirc_rx51
As clearly visible from the patch, this variable has no useful purpose
what so ever. Thus, it can be removed altogether without any side
effects.
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/ir-rx51.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 96ed23d..edb1562 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -57,7 +57,6 @@ struct lirc_rx51 {
unsigned intfreq; /* carrier frequency */
unsigned intduty_cycle; /* carrier duty cycle */
unsigned intirq_num;
- unsigned intmatch;
int wbuf[WBUF_LEN];
int wbuf_index;
unsigned long device_is_open;
@@ -102,8 +101,6 @@ static int init_timing_params(struct lirc_rx51 *lirc_rx51)
omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer, 0);
omap_dm_timer_start(lirc_rx51->pulse_timer);
- lirc_rx51->match = 0;
-
return 0;
}
@@ -113,11 +110,7 @@ static int pulse_timer_set_timeout(struct lirc_rx51
*lirc_rx51, int usec)
BUG_ON(usec < 0);
- if (lirc_rx51->match == 0)
- counter = omap_dm_timer_read_counter(lirc_rx51->pulse_timer);
- else
- counter = lirc_rx51->match;
-
+ counter = omap_dm_timer_read_counter(lirc_rx51->pulse_timer);
counter += (u32)(lirc_rx51->fclk_khz * usec / (1000));
omap_dm_timer_set_match(lirc_rx51->pulse_timer, 1, counter);
omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer,
--
1.8.0
--
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 5/7] ir-rx51: Convert latency constraints to PM QoS API
Convert the driver from the obsolete omap_pm_set_max_mpu_wakeup_lat
API to the new PM QoS API. This allows the callback to be removed from
the platform data structure.
The latency requirements are also adjusted to prevent the MPU from
going into sleep mode. This is needed as the GP timers have no means
to wake up the MPU once it has gone into sleep. The "side effect" is
that from now on the driver actually works even if there is no
background load keeping the MPU awake.
Signed-off-by: Timo Kokkonen
Acked-by: Tony Lindgren
Acked-by: Jean Pihet
---
arch/arm/mach-omap2/board-rx51-peripherals.c | 2 --
drivers/media/rc/ir-rx51.c | 17 -
include/media/ir-rx51.h | 2 --
3 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c
b/arch/arm/mach-omap2/board-rx51-peripherals.c
index 020e03c..6d0f29d 100644
--- a/arch/arm/mach-omap2/board-rx51-peripherals.c
+++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
@@ -33,7 +33,6 @@
#include "common.h"
#include
#include
-#include
#include "gpmc-smc91x.h"
#include "board-rx51.h"
@@ -1224,7 +1223,6 @@ static void __init rx51_init_tsc2005(void)
#if defined(CONFIG_IR_RX51) || defined(CONFIG_IR_RX51_MODULE)
static struct lirc_rx51_platform_data rx51_lirc_data = {
- .set_max_mpu_wakeup_lat = omap_pm_set_max_mpu_wakeup_lat,
.pwm_timer = 9, /* Use GPT 9 for CIR */
};
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 6e1ffa6..96ed23d 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include
#include
@@ -49,6 +50,7 @@ struct lirc_rx51 {
struct omap_dm_timer *pulse_timer;
struct device*dev;
struct lirc_rx51_platform_data *pdata;
+ struct pm_qos_request pm_qos_request;
wait_queue_head_t wqueue;
unsigned long fclk_khz;
@@ -268,10 +270,16 @@ static ssize_t lirc_rx51_write(struct file *file, const
char *buf,
lirc_rx51->wbuf[count] = -1; /* Insert termination mark */
/*
-* Adjust latency requirements so the device doesn't go in too
-* deep sleep states
+* If the MPU is going into too deep sleep state while we are
+* transmitting the IR code, timers will not be able to wake
+* up the MPU. Thus, we need to set a strict enough latency
+* requirement in order to ensure the interrupts come though
+* properly. The 10us latency requirement is low enough to
+* keep MPU from sleeping and thus ensures the interrupts are
+* getting through properly.
*/
- lirc_rx51->pdata->set_max_mpu_wakeup_lat(lirc_rx51->dev, 50);
+ pm_qos_add_request(&lirc_rx51->pm_qos_request,
+ PM_QOS_CPU_DMA_LATENCY, 10);
lirc_rx51_on(lirc_rx51);
lirc_rx51->wbuf_index = 1;
@@ -292,8 +300,7 @@ static ssize_t lirc_rx51_write(struct file *file, const
char *buf,
*/
lirc_rx51_stop_tx(lirc_rx51);
- /* We can sleep again */
- lirc_rx51->pdata->set_max_mpu_wakeup_lat(lirc_rx51->dev, -1);
+ pm_qos_remove_request(&lirc_rx51->pm_qos_request);
return n;
}
diff --git a/include/media/ir-rx51.h b/include/media/ir-rx51.h
index 104aa89..57523f2 100644
--- a/include/media/ir-rx51.h
+++ b/include/media/ir-rx51.h
@@ -3,8 +3,6 @@
struct lirc_rx51_platform_data {
int pwm_timer;
-
- int(*set_max_mpu_wakeup_lat)(struct device *dev, long t);
};
#endif
--
1.8.0
--
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 7/7] ir-rx51: Fix sparse warnings
Add missing __user annotation to all of the user space memory
accesses. Otherwise sparse is complainign about address space
difference in types.
Also struct lirc_rx51_platform_driver is missing static keyword even
though it should have it.
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/ir-rx51.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index edb1562..7ed0616 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -233,7 +233,7 @@ static int lirc_rx51_free_port(struct lirc_rx51 *lirc_rx51)
return 0;
}
-static ssize_t lirc_rx51_write(struct file *file, const char *buf,
+static ssize_t lirc_rx51_write(struct file *file, const char __user *buf,
size_t n, loff_t *ppos)
{
int count, i;
@@ -308,13 +308,13 @@ static long lirc_rx51_ioctl(struct file *filep,
switch (cmd) {
case LIRC_GET_SEND_MODE:
- result = put_user(LIRC_MODE_PULSE, (unsigned long *)arg);
+ result = put_user(LIRC_MODE_PULSE, (unsigned long __user *)arg);
if (result)
return result;
break;
case LIRC_SET_SEND_MODE:
- result = get_user(value, (unsigned long *)arg);
+ result = get_user(value, (unsigned long __user *)arg);
if (result)
return result;
@@ -324,7 +324,7 @@ static long lirc_rx51_ioctl(struct file *filep,
break;
case LIRC_GET_REC_MODE:
- result = put_user(0, (unsigned long *) arg);
+ result = put_user(0, (unsigned long __user *)arg);
if (result)
return result;
break;
@@ -334,7 +334,7 @@ static long lirc_rx51_ioctl(struct file *filep,
break;
case LIRC_SET_SEND_DUTY_CYCLE:
- result = get_user(ivalue, (unsigned int *) arg);
+ result = get_user(ivalue, (unsigned int __user *)arg);
if (result)
return result;
@@ -348,7 +348,7 @@ static long lirc_rx51_ioctl(struct file *filep,
break;
case LIRC_SET_SEND_CARRIER:
- result = get_user(ivalue, (unsigned int *) arg);
+ result = get_user(ivalue, (unsigned int __user *)arg);
if (result)
return result;
@@ -363,7 +363,7 @@ static long lirc_rx51_ioctl(struct file *filep,
case LIRC_GET_FEATURES:
result = put_user(LIRC_RX51_DRIVER_FEATURES,
- (unsigned long *) arg);
+ (unsigned long __user *)arg);
if (result)
return result;
break;
@@ -484,7 +484,7 @@ static int __exit lirc_rx51_remove(struct platform_device
*dev)
return lirc_unregister_driver(lirc_rx51_driver.minor);
}
-struct platform_driver lirc_rx51_platform_driver = {
+static struct platform_driver lirc_rx51_platform_driver = {
.probe = lirc_rx51_probe,
.remove = __exit_p(lirc_rx51_remove),
.suspend= lirc_rx51_suspend,
--
1.8.0
--
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 1/7] ir-rx51: Handle signals properly
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
---
drivers/media/rc/ir-rx51.c | 39 ---
1 file changed, 28 insertions(+), 11 deletions(-)
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);
--
1.8.0
--
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 2/7] ir-rx51: Clean up timer initialization code
Remove a redundant macro definition. This is unneeded and becomes more
readable once the actual timer code is refactored a little.
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/ir-rx51.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 125d4c3..f22e5e4 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -105,11 +105,9 @@ static int init_timing_params(struct lirc_rx51 *lirc_rx51)
return 0;
}
-#define tics_after(a, b) ((long)(b) - (long)(a) < 0)
-
static int pulse_timer_set_timeout(struct lirc_rx51 *lirc_rx51, int usec)
{
- int counter;
+ int counter, counter_now;
BUG_ON(usec < 0);
@@ -122,11 +120,8 @@ static int pulse_timer_set_timeout(struct lirc_rx51
*lirc_rx51, int usec)
omap_dm_timer_set_match(lirc_rx51->pulse_timer, 1, counter);
omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer,
OMAP_TIMER_INT_MATCH);
- if (tics_after(omap_dm_timer_read_counter(lirc_rx51->pulse_timer),
- counter)) {
- return 1;
- }
- return 0;
+ counter_now = omap_dm_timer_read_counter(lirc_rx51->pulse_timer);
+ return (counter - counter_now) < 0;
}
static irqreturn_t lirc_rx51_interrupt_handler(int irq, void *ptr)
--
1.8.0
--
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 0/7] ir-rx51: Various fixes
Hi,
Here is a set of fixes that did not make in the last merge
window. Everything else except the last patch that fixes sparse
complaints have been posted before.
Especially the PM QoS conversion patch is important, without that one
the driver does not work at all unless there is some background load
keeping the CPU from sleeping.
The signal handling fixup patches did raise all sorts of discussion
last time, but my conclusion was that the patch itself should be fine
for now.
Please provide feedback and consider accepting them in. Thank you.
Timo Kokkonen (7):
ir-rx51: Handle signals properly
ir-rx51: Clean up timer initialization code
ir-rx51: Move platform data checking into probe function
ir-rx51: Replace module_{init,exit} macros with
module_platform_driver
ir-rx51: Convert latency constraints to PM QoS API
ir-rx51: Remove useless variable from struct lirc_rx51
ir-rx51: Fix sparse warnings
arch/arm/mach-omap2/board-rx51-peripherals.c | 2 -
drivers/media/rc/ir-rx51.c | 108 ++-
include/media/ir-rx51.h | 2 -
3 files changed, 56 insertions(+), 56 deletions(-)
--
1.8.0
--
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 3/7] ir-rx51: Move platform data checking into probe function
This driver is useless without proper platform data. If data is not
available, we should not register the driver at all. Once this check
is done, the BUG_ON check during device open is no longer needed.
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/ir-rx51.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index f22e5e4..16b3c1f 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -378,7 +378,6 @@ static long lirc_rx51_ioctl(struct file *filep,
static int lirc_rx51_open(struct inode *inode, struct file *file)
{
struct lirc_rx51 *lirc_rx51 = lirc_get_pdata(file);
- BUG_ON(!lirc_rx51);
file->private_data = lirc_rx51;
@@ -458,6 +457,9 @@ static int lirc_rx51_resume(struct platform_device *dev)
static int __devinit lirc_rx51_probe(struct platform_device *dev)
{
+ if (!dev->dev.platform_data)
+ return -ENODEV;
+
lirc_rx51_driver.features = LIRC_RX51_DRIVER_FEATURES;
lirc_rx51.pdata = dev->dev.platform_data;
lirc_rx51.pwm_timer_num = lirc_rx51.pdata->pwm_timer;
--
1.8.0
--
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: [PATCHv3 2/9] ir-rx51: Handle signals properly
On 09/03/12 15:36, Sean Young wrote: > On Sun, Sep 02, 2012 at 11:08:20PM +0300, Timo Kokkonen wrote: >> On 09/02/12 22:41, Sakari Ailus wrote: >>> On Sun, Sep 02, 2012 at 06:20:27PM +0300, Timo Kokkonen wrote: >>>> On 09.02 2012 18:06:34, Sakari Ailus wrote: >>>>> Heippa, >>>>> >>>>> Timo Kokkonen wrote: >>>>>> Terve, >>>>>> >>>>>> On 09/01/12 20:14, Sakari Ailus wrote: >>>>>>> Moi, >>>>>>> >>>>>>> On Thu, Aug 30, 2012 at 08:54:24PM +0300, Timo Kokkonen wrote: >>>>>>>> @@ -273,9 +281,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); >>>>>>> >>>>>>> Why such an arbitrary timeout? In reality it might not bite the user >>>>>>> space >>>>>>> in practice ever, but is it (and if so, why) really required in the >>>>>>> first >>>>>>> place? >>>>>> >>>>>> Well, I can think of two cases: >>>>>> >>>>>> 1) Something goes wrong. Such before I converted the patch to use the up >>>>>> to date PM QoS implementation, the transmitting could take very long >>>>>> time because the interrupts were not waking up the MPU. Now that this is >>>>>> sorted out only unknown bugs can cause transmitting to hang indefinitely. >>>>>> >>>>>> 2) User is (intentionally?) doing something wrong. For example by >>>>>> feeding in an IR code that has got very long pulses, he could end up >>>>>> having the lircd process hung in kernel unkillable for long time. That >>>>>> could be avoided quite easily by counting the pulse lengths and >>>>>> rejecting any IR codes that are obviously too long. But since I'd like >>>>>> to also protect against 1) case, I think this solution works just fine. >>>>>> >>>>>> In the end, this is just safety measure that this driver behaves well. >>>>> >>>>> In that case I think you should use wait_event_interruptible() instead. >>>> >>>> Well, that's what I had there in the first place. With interruptible >>>> wait we are left with problem with signals. I was told by Sean Young >>>> that the lirc API expects the write call to finish only after the IR >>>> code is transmitted. >>>> >>>>> It's not the driver's job to decide what the user can do with the >>>>> hardware and what not, is it? >>>> >>>> Yeah, policy should be decided by the user space. However, kernel >>>> should not leave any objvious denial of service holes open >>>> either. Allowing a process to get stuck unkillable within kernel for >>>> long time sounds like one to me. > > It's not elegant, but this can't be used as a denial of service attack. > The driver waits for a maximum of a half a second after which signals > are serviced as normal. > >>> It's interruptible, so the user space can interrupt that wait if it chooses >>> so. Besides, if you call this denial of service, then capturing video on >>> V4L2 is, too, since others can't use the device in the meantime. :-) >>> >> >> Well, of course there is no problem if we use interruptible waits. But I >> was told by Sean that the lirc API expects the IR TX to be finished >> always when the write call returns. > > This is part of the ABI. The lircd deamon might want to do gap calculation > if there are large spaces in the IR code being sent. Maybe others can > enlighten us why such an ABI was choosen. > >> I guess the assumption is to avoid >> breaking the transmission in the middle in case the process is signaled.
Re: [PATCHv3 2/9] ir-rx51: Handle signals properly
On 09/02/12 22:41, Sakari Ailus wrote: > On Sun, Sep 02, 2012 at 06:20:27PM +0300, Timo Kokkonen wrote: >> On 09.02 2012 18:06:34, Sakari Ailus wrote: >>> Heippa, >>> >>> Timo Kokkonen wrote: >>>> Terve, >>>> >>>> On 09/01/12 20:14, Sakari Ailus wrote: >>>>> Moi, >>>>> >>>>> On Thu, Aug 30, 2012 at 08:54:24PM +0300, Timo Kokkonen wrote: >>>>>> @@ -273,9 +281,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); >>>>> >>>>> Why such an arbitrary timeout? In reality it might not bite the user space >>>>> in practice ever, but is it (and if so, why) really required in the first >>>>> place? >>>> >>>> Well, I can think of two cases: >>>> >>>> 1) Something goes wrong. Such before I converted the patch to use the up >>>> to date PM QoS implementation, the transmitting could take very long >>>> time because the interrupts were not waking up the MPU. Now that this is >>>> sorted out only unknown bugs can cause transmitting to hang indefinitely. >>>> >>>> 2) User is (intentionally?) doing something wrong. For example by >>>> feeding in an IR code that has got very long pulses, he could end up >>>> having the lircd process hung in kernel unkillable for long time. That >>>> could be avoided quite easily by counting the pulse lengths and >>>> rejecting any IR codes that are obviously too long. But since I'd like >>>> to also protect against 1) case, I think this solution works just fine. >>>> >>>> In the end, this is just safety measure that this driver behaves well. >>> >>> In that case I think you should use wait_event_interruptible() instead. >> >> Well, that's what I had there in the first place. With interruptible >> wait we are left with problem with signals. I was told by Sean Young >> that the lirc API expects the write call to finish only after the IR >> code is transmitted. >> >>> It's not the driver's job to decide what the user can do with the >>> hardware and what not, is it? >> >> Yeah, policy should be decided by the user space. However, kernel >> should not leave any objvious denial of service holes open >> either. Allowing a process to get stuck unkillable within kernel for >> long time sounds like one to me. > > It's interruptible, so the user space can interrupt that wait if it chooses > so. Besides, if you call this denial of service, then capturing video on > V4L2 is, too, since others can't use the device in the meantime. :-) > Well, of course there is no problem if we use interruptible waits. But I was told by Sean that the lirc API expects the IR TX to be finished always when the write call returns. I guess the assumption is to avoid breaking the transmission in the middle in case the process is signaled. And that's why we shouldn't use interruptible waits. However, if we allow simply breaking the transmitting in case the process is signaled any way during the transmission, then the handling would be trivial in the driver. That is, if someone for example kills or stops the lirc daemon process, then the IR code just wouldn't finish ever. Sean, do you have an opinion how this should or is allowed to work? >> Anyway, we are trying to cover some rare corner cases here, I'm not >> sure how it should work exactly.. > > If there was a generic maximum timeout for sending a code, wouldn't it make > sense to enforce that in the LIRC framework instead? > Yes, I agree it makes sense to leave unrestricted. But in that case we definitely have to use interruptible waits in case user space is doing something stupid and regrets it later :) > Terveisin, > -- 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: [PATCHv3 2/9] ir-rx51: Handle signals properly
On 09.02 2012 18:06:34, Sakari Ailus wrote: > Heippa, > > Timo Kokkonen wrote: > > Terve, > > > > On 09/01/12 20:14, Sakari Ailus wrote: > >> Moi, > >> > >> On Thu, Aug 30, 2012 at 08:54:24PM +0300, Timo Kokkonen wrote: > >>> @@ -273,9 +281,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); > >> > >> Why such an arbitrary timeout? In reality it might not bite the user space > >> in practice ever, but is it (and if so, why) really required in the first > >> place? > > > > Well, I can think of two cases: > > > > 1) Something goes wrong. Such before I converted the patch to use the up > > to date PM QoS implementation, the transmitting could take very long > > time because the interrupts were not waking up the MPU. Now that this is > > sorted out only unknown bugs can cause transmitting to hang indefinitely. > > > > 2) User is (intentionally?) doing something wrong. For example by > > feeding in an IR code that has got very long pulses, he could end up > > having the lircd process hung in kernel unkillable for long time. That > > could be avoided quite easily by counting the pulse lengths and > > rejecting any IR codes that are obviously too long. But since I'd like > > to also protect against 1) case, I think this solution works just fine. > > > > In the end, this is just safety measure that this driver behaves well. > > In that case I think you should use wait_event_interruptible() instead. Well, that's what I had there in the first place. With interruptible wait we are left with problem with signals. I was told by Sean Young that the lirc API expects the write call to finish only after the IR code is transmitted. > It's not the driver's job to decide what the user can do with the > hardware and what not, is it? Yeah, policy should be decided by the user space. However, kernel should not leave any objvious denial of service holes open either. Allowing a process to get stuck unkillable within kernel for long time sounds like one to me. Anyway, we are trying to cover some rare corner cases here, I'm not sure how it should work exactly.. -Timo > > Terveisin, > > -- > Sakari Ailus > sakari.ai...@iki.fi > -- > 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: [PATCHv3 9/9] ir-rx51: Add missing quote mark in Kconfig text
On 09/01/12 20:16, Sakari Ailus wrote: > Moi, > > On Thu, Aug 30, 2012 at 08:54:31PM +0300, Timo Kokkonen wrote: >> This trivial fix cures the following warning message: >> >> drivers/media/rc/Kconfig:275:warning: multi-line strings not supported >> >> Signed-off-by: Timo Kokkonen >> --- >> drivers/media/rc/Kconfig | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig >> index 4a68014..1300655 100644 >> --- a/drivers/media/rc/Kconfig >> +++ b/drivers/media/rc/Kconfig >> @@ -272,7 +272,7 @@ config IR_IGUANA >> be called iguanair. >> >> config IR_RX51 >> -tristate "Nokia N900 IR transmitter diode >> +tristate "Nokia N900 IR transmitter diode" >> depends on OMAP_DM_TIMER && LIRC >> ---help--- >> Say Y or M here if you want to enable support for the IR > > This should be combined with patch 1. > Actually I'd rather keep the patch 1 as is as it has already a purpose. Instead, I'd squash this into patch 3 as it already touches the Kconfig file and it has also the other trivial fixes combined in it. -Timo -- 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: [PATCHv3 2/9] ir-rx51: Handle signals properly
Terve, On 09/01/12 20:14, Sakari Ailus wrote: > Moi, > > On Thu, Aug 30, 2012 at 08:54:24PM +0300, Timo Kokkonen wrote: >> @@ -273,9 +281,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); > > Why such an arbitrary timeout? In reality it might not bite the user space > in practice ever, but is it (and if so, why) really required in the first > place? Well, I can think of two cases: 1) Something goes wrong. Such before I converted the patch to use the up to date PM QoS implementation, the transmitting could take very long time because the interrupts were not waking up the MPU. Now that this is sorted out only unknown bugs can cause transmitting to hang indefinitely. 2) User is (intentionally?) doing something wrong. For example by feeding in an IR code that has got very long pulses, he could end up having the lircd process hung in kernel unkillable for long time. That could be avoided quite easily by counting the pulse lengths and rejecting any IR codes that are obviously too long. But since I'd like to also protect against 1) case, I think this solution works just fine. In the end, this is just safety measure that this driver behaves well. -Timo -- 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
[PATCHv3 4/9] ir-rx51: Clean up timer initialization code
Remove a redundant macro definition. This is unneeded and becomes more
readable once the actual timer code is refactored a little.
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/ir-rx51.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 125d4c3..f22e5e4 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -105,11 +105,9 @@ static int init_timing_params(struct lirc_rx51 *lirc_rx51)
return 0;
}
-#define tics_after(a, b) ((long)(b) - (long)(a) < 0)
-
static int pulse_timer_set_timeout(struct lirc_rx51 *lirc_rx51, int usec)
{
- int counter;
+ int counter, counter_now;
BUG_ON(usec < 0);
@@ -122,11 +120,8 @@ static int pulse_timer_set_timeout(struct lirc_rx51
*lirc_rx51, int usec)
omap_dm_timer_set_match(lirc_rx51->pulse_timer, 1, counter);
omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer,
OMAP_TIMER_INT_MATCH);
- if (tics_after(omap_dm_timer_read_counter(lirc_rx51->pulse_timer),
- counter)) {
- return 1;
- }
- return 0;
+ counter_now = omap_dm_timer_read_counter(lirc_rx51->pulse_timer);
+ return (counter - counter_now) < 0;
}
static irqreturn_t lirc_rx51_interrupt_handler(int irq, void *ptr)
--
1.7.12
--
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
[PATCHv3 0/9] Fixes in response to review comments
These patches fix most of the issues pointed out in the patch review
by Sean Young and Sakari Ailus.
The most noticeable change after these patch set is that the IR
transmission no longer times out even if the timers are not waking up
the MPU as it should be. Now that Jean Pihet kindly instructed me how
to use the PM QoS API for setting the latency constraints, the driver
will now work without any background load. Someone might consider such
restriction a blocker bug, that is fixed on this patch set.
Changes since v2:
- The 10us PM QoS latency requrement is documented in the code
- A missing quote mark is added into the Kconfig text
Changes since v1:
- Replace wake_up_interruptible with wake_up, as the driver is having
non-interruptible sleeps
- Instead of just removing the set_max_mpu_wakeup_lat calls, replace
them with QoS API calls
Timo Kokkonen (9):
ir-rx51: Adjust dependencies
ir-rx51: Handle signals properly
ir-rx51: Trivial fixes
ir-rx51: Clean up timer initialization code
ir-rx51: Move platform data checking into probe function
ir-rx51: Replace module_{init,exit} macros with
module_platform_driver
ir-rx51: Convert latency constraints to PM QoS API
ir-rx51: Remove useless variable from struct lirc_rx51
ir-rx51: Add missing quote mark in Kconfig text
arch/arm/mach-omap2/board-rx51-peripherals.c | 2 -
drivers/media/rc/Kconfig | 6 +-
drivers/media/rc/ir-rx51.c | 102 ++-
include/media/ir-rx51.h | 2 -
4 files changed, 57 insertions(+), 55 deletions(-)
--
1.7.12
--
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
[PATCHv3 5/9] ir-rx51: Move platform data checking into probe function
This driver is useless without proper platform data. If data is not
available, we should not register the driver at all. Once this check
is done, the BUG_ON check during device open is no longer needed.
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/ir-rx51.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index f22e5e4..16b3c1f 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -378,7 +378,6 @@ static long lirc_rx51_ioctl(struct file *filep,
static int lirc_rx51_open(struct inode *inode, struct file *file)
{
struct lirc_rx51 *lirc_rx51 = lirc_get_pdata(file);
- BUG_ON(!lirc_rx51);
file->private_data = lirc_rx51;
@@ -458,6 +457,9 @@ static int lirc_rx51_resume(struct platform_device *dev)
static int __devinit lirc_rx51_probe(struct platform_device *dev)
{
+ if (!dev->dev.platform_data)
+ return -ENODEV;
+
lirc_rx51_driver.features = LIRC_RX51_DRIVER_FEATURES;
lirc_rx51.pdata = dev->dev.platform_data;
lirc_rx51.pwm_timer_num = lirc_rx51.pdata->pwm_timer;
--
1.7.12
--
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
[PATCHv3 6/9] ir-rx51: Replace module_{init,exit} macros with module_platform_driver
No reason to avoid using the existing helpers.
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/ir-rx51.c | 12 +---
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 16b3c1f..6e1ffa6 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -495,17 +495,7 @@ struct platform_driver lirc_rx51_platform_driver = {
},
};
-static int __init lirc_rx51_init(void)
-{
- return platform_driver_register(&lirc_rx51_platform_driver);
-}
-module_init(lirc_rx51_init);
-
-static void __exit lirc_rx51_exit(void)
-{
- platform_driver_unregister(&lirc_rx51_platform_driver);
-}
-module_exit(lirc_rx51_exit);
+module_platform_driver(lirc_rx51_platform_driver);
MODULE_DESCRIPTION("LIRC TX driver for Nokia RX51");
MODULE_AUTHOR("Nokia Corporation");
--
1.7.12
--
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
[PATCHv3 9/9] ir-rx51: Add missing quote mark in Kconfig text
This trivial fix cures the following warning message: drivers/media/rc/Kconfig:275:warning: multi-line strings not supported Signed-off-by: Timo Kokkonen --- drivers/media/rc/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig index 4a68014..1300655 100644 --- a/drivers/media/rc/Kconfig +++ b/drivers/media/rc/Kconfig @@ -272,7 +272,7 @@ config IR_IGUANA be called iguanair. config IR_RX51 - tristate "Nokia N900 IR transmitter diode + tristate "Nokia N900 IR transmitter diode" depends on OMAP_DM_TIMER && LIRC ---help--- Say Y or M here if you want to enable support for the IR -- 1.7.12 -- 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
[PATCHv3 8/9] ir-rx51: Remove useless variable from struct lirc_rx51
As clearly visible from the patch, this variable has no useful purpose
what so ever. Thus, it can be removed altogether without any side
effects.
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/ir-rx51.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 96ed23d..edb1562 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -57,7 +57,6 @@ struct lirc_rx51 {
unsigned intfreq; /* carrier frequency */
unsigned intduty_cycle; /* carrier duty cycle */
unsigned intirq_num;
- unsigned intmatch;
int wbuf[WBUF_LEN];
int wbuf_index;
unsigned long device_is_open;
@@ -102,8 +101,6 @@ static int init_timing_params(struct lirc_rx51 *lirc_rx51)
omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer, 0);
omap_dm_timer_start(lirc_rx51->pulse_timer);
- lirc_rx51->match = 0;
-
return 0;
}
@@ -113,11 +110,7 @@ static int pulse_timer_set_timeout(struct lirc_rx51
*lirc_rx51, int usec)
BUG_ON(usec < 0);
- if (lirc_rx51->match == 0)
- counter = omap_dm_timer_read_counter(lirc_rx51->pulse_timer);
- else
- counter = lirc_rx51->match;
-
+ counter = omap_dm_timer_read_counter(lirc_rx51->pulse_timer);
counter += (u32)(lirc_rx51->fclk_khz * usec / (1000));
omap_dm_timer_set_match(lirc_rx51->pulse_timer, 1, counter);
omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer,
--
1.7.12
--
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
[PATCHv3 7/9] ir-rx51: Convert latency constraints to PM QoS API
Convert the driver from the obsolete omap_pm_set_max_mpu_wakeup_lat
API to the new PM QoS API. This allows the callback to be removed from
the platform data structure.
The latency requirements are also adjusted to prevent the MPU from
going into sleep mode. This is needed as the GP timers have no means
to wake up the MPU once it has gone into sleep. The "side effect" is
that from now on the driver actually works even if there is no
background load keeping the MPU awake.
Signed-off-by: Timo Kokkonen
Acked-by: Tony Lindgren
Acked-by: Jean Pihet
---
arch/arm/mach-omap2/board-rx51-peripherals.c | 2 --
drivers/media/rc/ir-rx51.c | 17 -
include/media/ir-rx51.h | 2 --
3 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c
b/arch/arm/mach-omap2/board-rx51-peripherals.c
index ca07264..e0750cb 100644
--- a/arch/arm/mach-omap2/board-rx51-peripherals.c
+++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
@@ -34,7 +34,6 @@
#include
#include
#include
-#include
#include
@@ -1227,7 +1226,6 @@ static void __init rx51_init_tsc2005(void)
#if defined(CONFIG_IR_RX51) || defined(CONFIG_IR_RX51_MODULE)
static struct lirc_rx51_platform_data rx51_lirc_data = {
- .set_max_mpu_wakeup_lat = omap_pm_set_max_mpu_wakeup_lat,
.pwm_timer = 9, /* Use GPT 9 for CIR */
};
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 6e1ffa6..96ed23d 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include
#include
@@ -49,6 +50,7 @@ struct lirc_rx51 {
struct omap_dm_timer *pulse_timer;
struct device*dev;
struct lirc_rx51_platform_data *pdata;
+ struct pm_qos_request pm_qos_request;
wait_queue_head_t wqueue;
unsigned long fclk_khz;
@@ -268,10 +270,16 @@ static ssize_t lirc_rx51_write(struct file *file, const
char *buf,
lirc_rx51->wbuf[count] = -1; /* Insert termination mark */
/*
-* Adjust latency requirements so the device doesn't go in too
-* deep sleep states
+* If the MPU is going into too deep sleep state while we are
+* transmitting the IR code, timers will not be able to wake
+* up the MPU. Thus, we need to set a strict enough latency
+* requirement in order to ensure the interrupts come though
+* properly. The 10us latency requirement is low enough to
+* keep MPU from sleeping and thus ensures the interrupts are
+* getting through properly.
*/
- lirc_rx51->pdata->set_max_mpu_wakeup_lat(lirc_rx51->dev, 50);
+ pm_qos_add_request(&lirc_rx51->pm_qos_request,
+ PM_QOS_CPU_DMA_LATENCY, 10);
lirc_rx51_on(lirc_rx51);
lirc_rx51->wbuf_index = 1;
@@ -292,8 +300,7 @@ static ssize_t lirc_rx51_write(struct file *file, const
char *buf,
*/
lirc_rx51_stop_tx(lirc_rx51);
- /* We can sleep again */
- lirc_rx51->pdata->set_max_mpu_wakeup_lat(lirc_rx51->dev, -1);
+ pm_qos_remove_request(&lirc_rx51->pm_qos_request);
return n;
}
diff --git a/include/media/ir-rx51.h b/include/media/ir-rx51.h
index 104aa89..57523f2 100644
--- a/include/media/ir-rx51.h
+++ b/include/media/ir-rx51.h
@@ -3,8 +3,6 @@
struct lirc_rx51_platform_data {
int pwm_timer;
-
- int(*set_max_mpu_wakeup_lat)(struct device *dev, long t);
};
#endif
--
1.7.12
--
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
[PATCHv3 2/9] ir-rx51: Handle signals properly
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
---
drivers/media/rc/ir-rx51.c | 39 ---
1 file changed, 28 insertions(+), 11 deletions(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 9487dd3..e2db94e 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;
@@ -160,13 +173,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;
}
@@ -246,8 +253,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;
@@ -273,9 +281,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.7.12
--
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
[PATCHv3 1/9] ir-rx51: Adjust dependencies
Although this kind of IR diode circuitry is known to exist only in N900 hardware, nothing prevents making similar circuitry on any OMAP based board. The MACH_NOKIA_RX51 dependency is thus not something we want to be there. Also, this should depend on LIRC as it is a LIRC driver. Signed-off-by: Timo Kokkonen --- drivers/media/rc/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig index ffef8b4..093982b 100644 --- a/drivers/media/rc/Kconfig +++ b/drivers/media/rc/Kconfig @@ -273,7 +273,7 @@ config IR_IGUANA config IR_RX51 tristate "Nokia N900 IR transmitter diode - depends on MACH_NOKIA_RX51 && OMAP_DM_TIMER + depends on OMAP_DM_TIMER && LIRC ---help--- Say Y or M here if you want to enable support for the IR transmitter diode built in the Nokia N900 (RX51) device. -- 1.7.12 -- 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
[PATCHv3 3/9] ir-rx51: Trivial fixes
-Fix typo
-Change pwm_timer_num type to match type in platform data
-Remove extra parenthesis
-Replace magic constant with proper bit defintions
-Remove duplicate exit pointer
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/Kconfig | 2 +-
drivers/media/rc/ir-rx51.c | 10 ++
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index 093982b..4a68014 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -278,7 +278,7 @@ config IR_RX51
Say Y or M here if you want to enable support for the IR
transmitter diode built in the Nokia N900 (RX51) device.
- The driver uses omap DM timers for gereating the carrier
+ The driver uses omap DM timers for generating the carrier
wave and pulses.
config RC_LOOPBACK
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index e2db94e..125d4c3 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -59,7 +59,7 @@ struct lirc_rx51 {
int wbuf[WBUF_LEN];
int wbuf_index;
unsigned long device_is_open;
- unsigned intpwm_timer_num;
+ int pwm_timer_num;
};
static void lirc_rx51_on(struct lirc_rx51 *lirc_rx51)
@@ -138,11 +138,14 @@ static irqreturn_t lirc_rx51_interrupt_handler(int irq,
void *ptr)
if (!retval)
return IRQ_NONE;
- if ((retval & ~OMAP_TIMER_INT_MATCH))
+ if (retval & ~OMAP_TIMER_INT_MATCH)
dev_err_ratelimited(lirc_rx51->dev,
": Unexpected interrupt source: %x\n", retval);
- omap_dm_timer_write_status(lirc_rx51->pulse_timer, 7);
+ omap_dm_timer_write_status(lirc_rx51->pulse_timer,
+ OMAP_TIMER_INT_MATCH|
+ OMAP_TIMER_INT_OVERFLOW |
+ OMAP_TIMER_INT_CAPTURE);
if (lirc_rx51->wbuf_index < 0) {
dev_err_ratelimited(lirc_rx51->dev,
": BUG wbuf_index has value of %i\n",
@@ -489,7 +492,6 @@ struct platform_driver lirc_rx51_platform_driver = {
.remove = __exit_p(lirc_rx51_remove),
.suspend= lirc_rx51_suspend,
.resume = lirc_rx51_resume,
- .remove = __exit_p(lirc_rx51_remove),
.driver = {
.name = DRIVER_NAME,
.owner = THIS_MODULE,
--
1.7.12
--
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: [PATCHv2 7/8] ir-rx51: Convert latency constraints to PM QoS API
On 08/27/12 12:25, Jean Pihet wrote: > Hi Timo, > > On Fri, Aug 24, 2012 at 10:39 PM, Tony Lindgren wrote: >> * Timo Kokkonen [120824 08:11]: >>> Convert the driver from the obsolete omap_pm_set_max_mpu_wakeup_lat >>> API to the new PM QoS API. This allows the callback to be removed from >>> the platform data structure. >>> >>> The latency requirements are also adjusted to prevent the MPU from >>> going into sleep mode. This is needed as the GP timers have no means >>> to wake up the MPU once it has gone into sleep. The "side effect" is >>> that from now on the driver actually works even if there is no >>> background load keeping the MPU awake. >>> >>> Signed-off-by: Timo Kokkonen >> >> This should get acked by Kevin ideally. Other than that: >> >> Acked-by: Tony Lindgren > > ... > @@ -268,10 +270,14 @@ static ssize_t lirc_rx51_write(struct file > *file, const char *buf, > lirc_rx51->wbuf[count] = -1; /* Insert termination mark */ > > /* > -* Adjust latency requirements so the device doesn't go in too > -* deep sleep states > +* If the MPU is going into too deep sleep state while we are > +* transmitting the IR code, timers will not be able to wake > +* up the MPU. Thus, we need to set a strict enough latency > +* requirement in order to ensure the interrupts come though > +* properly. > */ > - lirc_rx51->pdata->set_max_mpu_wakeup_lat(lirc_rx51->dev, 50); > + pm_qos_add_request(&lirc_rx51->pm_qos_request, > + PM_QOS_CPU_DMA_LATENCY, 10); > Minor remark: it would be nice to have more detail on where the > latency number 10 comes from. Is it fixed, is it linked to the baud > rate etc? > Yeah, it was chosen to be low enough for the MPU to receive the IRQ from the timers. 50us was good enough back then with the original n900 kernel, but nowadays it is not good enough from preventing the MPU from going to sleep where the timer interrupts don't come through. Yes, I should probably have stated that in the comment to make it clear. Can I re-send just this one patch or should I send the entire set again? I'm assuming these go in through Mauro's media tree as these depend on stuff that's already there. So, which ever is easier for him I guess :) Thanks! -Timo > Here is my ack for the PM QoS API part: > Acked-by: Jean Pihet > > Regards, > Jean > -- > 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
[PATCHv2 5/8] ir-rx51: Move platform data checking into probe function
This driver is useless without proper platform data. If data is not
available, we should not register the driver at all. Once this check
is done, the BUG_ON check during device open is no longer needed.
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/ir-rx51.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index f22e5e4..16b3c1f 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -378,7 +378,6 @@ static long lirc_rx51_ioctl(struct file *filep,
static int lirc_rx51_open(struct inode *inode, struct file *file)
{
struct lirc_rx51 *lirc_rx51 = lirc_get_pdata(file);
- BUG_ON(!lirc_rx51);
file->private_data = lirc_rx51;
@@ -458,6 +457,9 @@ static int lirc_rx51_resume(struct platform_device *dev)
static int __devinit lirc_rx51_probe(struct platform_device *dev)
{
+ if (!dev->dev.platform_data)
+ return -ENODEV;
+
lirc_rx51_driver.features = LIRC_RX51_DRIVER_FEATURES;
lirc_rx51.pdata = dev->dev.platform_data;
lirc_rx51.pwm_timer_num = lirc_rx51.pdata->pwm_timer;
--
1.7.12
--
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
[PATCHv2 6/8] ir-rx51: Replace module_{init,exit} macros with module_platform_driver
No reason to avoid using the existing helpers.
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/ir-rx51.c | 12 +---
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 16b3c1f..6e1ffa6 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -495,17 +495,7 @@ struct platform_driver lirc_rx51_platform_driver = {
},
};
-static int __init lirc_rx51_init(void)
-{
- return platform_driver_register(&lirc_rx51_platform_driver);
-}
-module_init(lirc_rx51_init);
-
-static void __exit lirc_rx51_exit(void)
-{
- platform_driver_unregister(&lirc_rx51_platform_driver);
-}
-module_exit(lirc_rx51_exit);
+module_platform_driver(lirc_rx51_platform_driver);
MODULE_DESCRIPTION("LIRC TX driver for Nokia RX51");
MODULE_AUTHOR("Nokia Corporation");
--
1.7.12
--
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
[PATCHv2 7/8] ir-rx51: Convert latency constraints to PM QoS API
Convert the driver from the obsolete omap_pm_set_max_mpu_wakeup_lat
API to the new PM QoS API. This allows the callback to be removed from
the platform data structure.
The latency requirements are also adjusted to prevent the MPU from
going into sleep mode. This is needed as the GP timers have no means
to wake up the MPU once it has gone into sleep. The "side effect" is
that from now on the driver actually works even if there is no
background load keeping the MPU awake.
Signed-off-by: Timo Kokkonen
---
arch/arm/mach-omap2/board-rx51-peripherals.c | 2 --
drivers/media/rc/ir-rx51.c | 15 ++-
include/media/ir-rx51.h | 2 --
3 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c
b/arch/arm/mach-omap2/board-rx51-peripherals.c
index ca07264..e0750cb 100644
--- a/arch/arm/mach-omap2/board-rx51-peripherals.c
+++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
@@ -34,7 +34,6 @@
#include
#include
#include
-#include
#include
@@ -1227,7 +1226,6 @@ static void __init rx51_init_tsc2005(void)
#if defined(CONFIG_IR_RX51) || defined(CONFIG_IR_RX51_MODULE)
static struct lirc_rx51_platform_data rx51_lirc_data = {
- .set_max_mpu_wakeup_lat = omap_pm_set_max_mpu_wakeup_lat,
.pwm_timer = 9, /* Use GPT 9 for CIR */
};
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 6e1ffa6..008cdab 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include
#include
@@ -49,6 +50,7 @@ struct lirc_rx51 {
struct omap_dm_timer *pulse_timer;
struct device*dev;
struct lirc_rx51_platform_data *pdata;
+ struct pm_qos_request pm_qos_request;
wait_queue_head_t wqueue;
unsigned long fclk_khz;
@@ -268,10 +270,14 @@ static ssize_t lirc_rx51_write(struct file *file, const
char *buf,
lirc_rx51->wbuf[count] = -1; /* Insert termination mark */
/*
-* Adjust latency requirements so the device doesn't go in too
-* deep sleep states
+* If the MPU is going into too deep sleep state while we are
+* transmitting the IR code, timers will not be able to wake
+* up the MPU. Thus, we need to set a strict enough latency
+* requirement in order to ensure the interrupts come though
+* properly.
*/
- lirc_rx51->pdata->set_max_mpu_wakeup_lat(lirc_rx51->dev, 50);
+ pm_qos_add_request(&lirc_rx51->pm_qos_request,
+ PM_QOS_CPU_DMA_LATENCY, 10);
lirc_rx51_on(lirc_rx51);
lirc_rx51->wbuf_index = 1;
@@ -292,8 +298,7 @@ static ssize_t lirc_rx51_write(struct file *file, const
char *buf,
*/
lirc_rx51_stop_tx(lirc_rx51);
- /* We can sleep again */
- lirc_rx51->pdata->set_max_mpu_wakeup_lat(lirc_rx51->dev, -1);
+ pm_qos_remove_request(&lirc_rx51->pm_qos_request);
return n;
}
diff --git a/include/media/ir-rx51.h b/include/media/ir-rx51.h
index 104aa89..57523f2 100644
--- a/include/media/ir-rx51.h
+++ b/include/media/ir-rx51.h
@@ -3,8 +3,6 @@
struct lirc_rx51_platform_data {
int pwm_timer;
-
- int(*set_max_mpu_wakeup_lat)(struct device *dev, long t);
};
#endif
--
1.7.12
--
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
[PATCHv2 8/8] ir-rx51: Remove useless variable from struct lirc_rx51
As clearly visible from the patch, this variable has no useful purpose
what so ever. Thus, it can be removed altogether without any side
effects.
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/ir-rx51.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 008cdab..179b64d 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -57,7 +57,6 @@ struct lirc_rx51 {
unsigned intfreq; /* carrier frequency */
unsigned intduty_cycle; /* carrier duty cycle */
unsigned intirq_num;
- unsigned intmatch;
int wbuf[WBUF_LEN];
int wbuf_index;
unsigned long device_is_open;
@@ -102,8 +101,6 @@ static int init_timing_params(struct lirc_rx51 *lirc_rx51)
omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer, 0);
omap_dm_timer_start(lirc_rx51->pulse_timer);
- lirc_rx51->match = 0;
-
return 0;
}
@@ -113,11 +110,7 @@ static int pulse_timer_set_timeout(struct lirc_rx51
*lirc_rx51, int usec)
BUG_ON(usec < 0);
- if (lirc_rx51->match == 0)
- counter = omap_dm_timer_read_counter(lirc_rx51->pulse_timer);
- else
- counter = lirc_rx51->match;
-
+ counter = omap_dm_timer_read_counter(lirc_rx51->pulse_timer);
counter += (u32)(lirc_rx51->fclk_khz * usec / (1000));
omap_dm_timer_set_match(lirc_rx51->pulse_timer, 1, counter);
omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer,
--
1.7.12
--
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
[PATCHv2 4/8] ir-rx51: Clean up timer initialization code
Remove a redundant macro definition. This is unneeded and becomes more
readable once the actual timer code is refactored a little.
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/ir-rx51.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 125d4c3..f22e5e4 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -105,11 +105,9 @@ static int init_timing_params(struct lirc_rx51 *lirc_rx51)
return 0;
}
-#define tics_after(a, b) ((long)(b) - (long)(a) < 0)
-
static int pulse_timer_set_timeout(struct lirc_rx51 *lirc_rx51, int usec)
{
- int counter;
+ int counter, counter_now;
BUG_ON(usec < 0);
@@ -122,11 +120,8 @@ static int pulse_timer_set_timeout(struct lirc_rx51
*lirc_rx51, int usec)
omap_dm_timer_set_match(lirc_rx51->pulse_timer, 1, counter);
omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer,
OMAP_TIMER_INT_MATCH);
- if (tics_after(omap_dm_timer_read_counter(lirc_rx51->pulse_timer),
- counter)) {
- return 1;
- }
- return 0;
+ counter_now = omap_dm_timer_read_counter(lirc_rx51->pulse_timer);
+ return (counter - counter_now) < 0;
}
static irqreturn_t lirc_rx51_interrupt_handler(int irq, void *ptr)
--
1.7.12
--
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
[PATCHv2 2/8] ir-rx51: Handle signals properly
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
---
drivers/media/rc/ir-rx51.c | 39 ---
1 file changed, 28 insertions(+), 11 deletions(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 9487dd3..e2db94e 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;
@@ -160,13 +173,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;
}
@@ -246,8 +253,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;
@@ -273,9 +281,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.7.12
--
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
[PATCHv2 1/8] ir-rx51: Adjust dependencies
Although this kind of IR diode circuitry is known to exist only in N900 hardware, nothing prevents making similar circuitry on any OMAP based board. The MACH_NOKIA_RX51 dependency is thus not something we want to be there. Also, this should depend on LIRC as it is a LIRC driver. Signed-off-by: Timo Kokkonen --- drivers/media/rc/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig index ffef8b4..093982b 100644 --- a/drivers/media/rc/Kconfig +++ b/drivers/media/rc/Kconfig @@ -273,7 +273,7 @@ config IR_IGUANA config IR_RX51 tristate "Nokia N900 IR transmitter diode - depends on MACH_NOKIA_RX51 && OMAP_DM_TIMER + depends on OMAP_DM_TIMER && LIRC ---help--- Say Y or M here if you want to enable support for the IR transmitter diode built in the Nokia N900 (RX51) device. -- 1.7.12 -- 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
[PATCHv2 3/8] ir-rx51: Trivial fixes
-Fix typo
-Change pwm_timer_num type to match type in platform data
-Remove extra parenthesis
-Replace magic constant with proper bit defintions
-Remove duplicate exit pointer
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/Kconfig | 2 +-
drivers/media/rc/ir-rx51.c | 10 ++
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index 093982b..4a68014 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -278,7 +278,7 @@ config IR_RX51
Say Y or M here if you want to enable support for the IR
transmitter diode built in the Nokia N900 (RX51) device.
- The driver uses omap DM timers for gereating the carrier
+ The driver uses omap DM timers for generating the carrier
wave and pulses.
config RC_LOOPBACK
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index e2db94e..125d4c3 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -59,7 +59,7 @@ struct lirc_rx51 {
int wbuf[WBUF_LEN];
int wbuf_index;
unsigned long device_is_open;
- unsigned intpwm_timer_num;
+ int pwm_timer_num;
};
static void lirc_rx51_on(struct lirc_rx51 *lirc_rx51)
@@ -138,11 +138,14 @@ static irqreturn_t lirc_rx51_interrupt_handler(int irq,
void *ptr)
if (!retval)
return IRQ_NONE;
- if ((retval & ~OMAP_TIMER_INT_MATCH))
+ if (retval & ~OMAP_TIMER_INT_MATCH)
dev_err_ratelimited(lirc_rx51->dev,
": Unexpected interrupt source: %x\n", retval);
- omap_dm_timer_write_status(lirc_rx51->pulse_timer, 7);
+ omap_dm_timer_write_status(lirc_rx51->pulse_timer,
+ OMAP_TIMER_INT_MATCH|
+ OMAP_TIMER_INT_OVERFLOW |
+ OMAP_TIMER_INT_CAPTURE);
if (lirc_rx51->wbuf_index < 0) {
dev_err_ratelimited(lirc_rx51->dev,
": BUG wbuf_index has value of %i\n",
@@ -489,7 +492,6 @@ struct platform_driver lirc_rx51_platform_driver = {
.remove = __exit_p(lirc_rx51_remove),
.suspend= lirc_rx51_suspend,
.resume = lirc_rx51_resume,
- .remove = __exit_p(lirc_rx51_remove),
.driver = {
.name = DRIVER_NAME,
.owner = THIS_MODULE,
--
1.7.12
--
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
[PATCHv2 0/8] ir-rx51: Fixes in response to review comments
These patches fix most of the issues pointed out in the patch review
by Sean Young and Sakari Ailus.
The most noticeable change after these patch set is that the IR
transmission no longer times out even if the timers are not waking up
the MPU as it should be. Now that Jean Pihet kindly instructed me how
to use the PM QoS API for setting the latency constraints, the driver
will now work without any background load. Someone might consider such
restriction a blocker bug, that is fixed on this patch set.
Changes since v1:
- Replace wake_up_interruptible with wake_up, as the driver is having
non-interruptible sleeps
- Instead of just removing the set_max_mpu_wakeup_lat calls, replace
them with QoS API calls
Timo Kokkonen (8):
ir-rx51: Adjust dependencies
ir-rx51: Handle signals properly
ir-rx51: Trivial fixes
ir-rx51: Clean up timer initialization code
ir-rx51: Move platform data checking into probe function
ir-rx51: Replace module_{init,exit} macros with
module_platform_driver
ir-rx51: Convert latency constraints to PM QoS API
ir-rx51: Remove useless variable from struct lirc_rx51
arch/arm/mach-omap2/board-rx51-peripherals.c | 2 -
drivers/media/rc/Kconfig | 4 +-
drivers/media/rc/ir-rx51.c | 100 ++-
include/media/ir-rx51.h | 2 -
4 files changed, 54 insertions(+), 54 deletions(-)
--
1.7.12
--
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 7/8] ir-rx51: Remove MPU wakeup latency adjustments
On 08/24/12 12:04, Jean Pihet wrote: > Hi Timo, > > On Fri, Aug 24, 2012 at 10:14 AM, Timo Kokkonen > wrote: >> Hi Jean, >> >> On 08/23/12 14:58, Jean Pihet wrote: >>> Hi Timo, >>> >>> On Wed, Aug 22, 2012 at 9:50 PM, Timo Kokkonen >>> wrote: >>> That is correct. The API to use is the PM QoS API which cpuidle uses >>> to determine the next MPU state based on the allowed latency. >>> >>>> A more appropriate fix for the problem would be to modify the idle >>>> layer so that it does not allow MPU going to too deep sleep modes when >>>> it is expected that the timers need to wake up MPU. >>> The idle layer already uses the PM QoS framework to decide the next >>> MPU state. I think the right solution is to convert from >>> omap_pm_set_max_mpu_wakeup_lat to the PM QoS API. >>> >>> Cf. http://marc.info/?l=linux-omap&m=133968658305580&w=2 for an >>> example of the conversion. >>> >> >> Thanks. It looks like really easy and straightforward conversion. >> However, I couldn't find the patch you were referring to from any trees > Correct, this patch is not applied to the mainline code yet, it is > provided as an example of the conversion. > >> I could find. So, I take that this API does not really have omap2 >> support in it yet? I tried git grepping through the source and to me it >> appears there is nothing in place yet that actually restricts the MPU >> sleep states on omap2 when requested. > The MPU state is controlled from the cpuidle framework, which > retrieves the MPU allowed latency from the PM QoS framework. This is > supported on OMAP2. > Cf. the table of states and the associated latency in > arch/arm/mach-omap2/cpuidle34xx.c. > Thanks for the pointer. I took a look at the state table and adjusted the latency requirements in my code. If I lower the latency from 50us to 10us, the timers are then waking up as they should be. I'll replace this patch with one where I convert it using the new API. Thanks! -Timo -- 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 2/8] ir-rx51: Handle signals properly
On 08/24/12 13:03, Sean Young wrote:
> On Wed, Aug 22, 2012 at 10:50:35PM +0300, Timo Kokkonen wrote:
>> 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
>> ---
>> drivers/media/rc/ir-rx51.c | 39 ---
>> 1 file changed, 28 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
>> index 9487dd3..a7b787a 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_interruptible(&lirc_rx51->wqueue);
>
> Unless I'm mistaken, wait_up_interruptable() won't wake up any
> non-interruptable sleepers. Should this not be wake_up()?
>
Thanks for pointing out that. I guess I never noticed that because I'm
using a timeout with the wait, so it will wake up anyway. I'll fix it.
-Timo
>> +}
>> +
>> static int init_timing_params(struct lirc_rx51 *lirc_rx51)
>> {
>> u32 load, match;
>> @@ -160,13 +173,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;
>> }
>> @@ -246,8 +253,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;
>> @@ -273,9 +281,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,
>
> Note non-interruptable sleeper.
>
>> +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.7.12
>>
>> --
>> 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
> --
> 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 7/8] ir-rx51: Remove MPU wakeup latency adjustments
Hi Jean,
On 08/23/12 14:58, Jean Pihet wrote:
> Hi Timo,
>
> On Wed, Aug 22, 2012 at 9:50 PM, Timo Kokkonen wrote:
> That is correct. The API to use is the PM QoS API which cpuidle uses
> to determine the next MPU state based on the allowed latency.
>
>> A more appropriate fix for the problem would be to modify the idle
>> layer so that it does not allow MPU going to too deep sleep modes when
>> it is expected that the timers need to wake up MPU.
> The idle layer already uses the PM QoS framework to decide the next
> MPU state. I think the right solution is to convert from
> omap_pm_set_max_mpu_wakeup_lat to the PM QoS API.
>
> Cf. http://marc.info/?l=linux-omap&m=133968658305580&w=2 for an
> example of the conversion.
>
Thanks. It looks like really easy and straightforward conversion.
However, I couldn't find the patch you were referring to from any trees
I could find. So, I take that this API does not really have omap2
support in it yet? I tried git grepping through the source and to me it
appears there is nothing in place yet that actually restricts the MPU
sleep states on omap2 when requested.
Which puzzles me.. The patch you are referring to transfers the omap I2C
from the old omap PM API to the new QOS API is not applied yet in
mainline. The I2C is definitely working with the old API too, I'm just
wondering why I can't make it working with either of the APIs.. Am I
missing something here?
>> Therefore, it makes sense to actually remove this call entirely from
>> the ir-rx51 driver as it is both wrong and does nothing useful at the
>> moment.
>>
>> Signed-off-by: Timo Kokkonen
>
> Regards,
> Jean
>
>> ---
>> arch/arm/mach-omap2/board-rx51-peripherals.c | 2 --
>> drivers/media/rc/ir-rx51.c | 9 -
>> include/media/ir-rx51.h | 2 --
>> 3 files changed, 13 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c
>> b/arch/arm/mach-omap2/board-rx51-peripherals.c
>> index ca07264..e0750cb 100644
>> --- a/arch/arm/mach-omap2/board-rx51-peripherals.c
>> +++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
>> @@ -34,7 +34,6 @@
>> #include
>> #include
>> #include
>> -#include
>>
>> #include
>>
>> @@ -1227,7 +1226,6 @@ static void __init rx51_init_tsc2005(void)
>>
>> #if defined(CONFIG_IR_RX51) || defined(CONFIG_IR_RX51_MODULE)
>> static struct lirc_rx51_platform_data rx51_lirc_data = {
>> - .set_max_mpu_wakeup_lat = omap_pm_set_max_mpu_wakeup_lat,
>> .pwm_timer = 9, /* Use GPT 9 for CIR */
>> };
>>
>> diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
>> index 7eed541..ac7d3f0 100644
>> --- a/drivers/media/rc/ir-rx51.c
>> +++ b/drivers/media/rc/ir-rx51.c
>> @@ -267,12 +267,6 @@ static ssize_t lirc_rx51_write(struct file *file, const
>> char *buf,
>> if (count < WBUF_LEN)
>> lirc_rx51->wbuf[count] = -1; /* Insert termination mark */
>>
>> - /*
>> -* Adjust latency requirements so the device doesn't go in too
>> -* deep sleep states
>> -*/
>> - lirc_rx51->pdata->set_max_mpu_wakeup_lat(lirc_rx51->dev, 50);
>> -
>> lirc_rx51_on(lirc_rx51);
>> lirc_rx51->wbuf_index = 1;
>> pulse_timer_set_timeout(lirc_rx51, lirc_rx51->wbuf[0]);
>> @@ -292,9 +286,6 @@ static ssize_t lirc_rx51_write(struct file *file, const
>> char *buf,
>> */
>> lirc_rx51_stop_tx(lirc_rx51);
>>
>> - /* We can sleep again */
>> - lirc_rx51->pdata->set_max_mpu_wakeup_lat(lirc_rx51->dev, -1);
>> -
>> return n;
>> }
>>
>> diff --git a/include/media/ir-rx51.h b/include/media/ir-rx51.h
>> index 104aa89..57523f2 100644
>> --- a/include/media/ir-rx51.h
>> +++ b/include/media/ir-rx51.h
>> @@ -3,8 +3,6 @@
>>
>> struct lirc_rx51_platform_data {
>> int pwm_timer;
>> -
>> - int(*set_max_mpu_wakeup_lat)(struct device *dev, long t);
>> };
>>
>> #endif
>> --
>> 1.7.12
>>
>> --
>> 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
[PATCH 4/8] ir-rx51: Clean up timer initialization code
Remove a redundant macro definition. This is unneeded and becomes more
readable once the actual timer code is refactored a little.
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/ir-rx51.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 468c8a1..3d2911b 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -105,11 +105,9 @@ static int init_timing_params(struct lirc_rx51 *lirc_rx51)
return 0;
}
-#define tics_after(a, b) ((long)(b) - (long)(a) < 0)
-
static int pulse_timer_set_timeout(struct lirc_rx51 *lirc_rx51, int usec)
{
- int counter;
+ int counter, counter_now;
BUG_ON(usec < 0);
@@ -122,11 +120,8 @@ static int pulse_timer_set_timeout(struct lirc_rx51
*lirc_rx51, int usec)
omap_dm_timer_set_match(lirc_rx51->pulse_timer, 1, counter);
omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer,
OMAP_TIMER_INT_MATCH);
- if (tics_after(omap_dm_timer_read_counter(lirc_rx51->pulse_timer),
- counter)) {
- return 1;
- }
- return 0;
+ counter_now = omap_dm_timer_read_counter(lirc_rx51->pulse_timer);
+ return (counter - counter_now) < 0;
}
static irqreturn_t lirc_rx51_interrupt_handler(int irq, void *ptr)
--
1.7.12
--
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 3/8] ir-rx51: Trivial fixes
-Fix typo
-Change pwm_timer_num type to match type in platform data
-Remove extra parenthesis
-Replace magic constant with proper bit defintions
-Remove duplicate exit pointer
Signed-off-by: Timo Kokkonen
trivial
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/Kconfig | 2 +-
drivers/media/rc/ir-rx51.c | 10 ++
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index 093982b..4a68014 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -278,7 +278,7 @@ config IR_RX51
Say Y or M here if you want to enable support for the IR
transmitter diode built in the Nokia N900 (RX51) device.
- The driver uses omap DM timers for gereating the carrier
+ The driver uses omap DM timers for generating the carrier
wave and pulses.
config RC_LOOPBACK
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index a7b787a..468c8a1 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -59,7 +59,7 @@ struct lirc_rx51 {
int wbuf[WBUF_LEN];
int wbuf_index;
unsigned long device_is_open;
- unsigned intpwm_timer_num;
+ int pwm_timer_num;
};
static void lirc_rx51_on(struct lirc_rx51 *lirc_rx51)
@@ -138,11 +138,14 @@ static irqreturn_t lirc_rx51_interrupt_handler(int irq,
void *ptr)
if (!retval)
return IRQ_NONE;
- if ((retval & ~OMAP_TIMER_INT_MATCH))
+ if (retval & ~OMAP_TIMER_INT_MATCH)
dev_err_ratelimited(lirc_rx51->dev,
": Unexpected interrupt source: %x\n", retval);
- omap_dm_timer_write_status(lirc_rx51->pulse_timer, 7);
+ omap_dm_timer_write_status(lirc_rx51->pulse_timer,
+ OMAP_TIMER_INT_MATCH|
+ OMAP_TIMER_INT_OVERFLOW |
+ OMAP_TIMER_INT_CAPTURE);
if (lirc_rx51->wbuf_index < 0) {
dev_err_ratelimited(lirc_rx51->dev,
": BUG wbuf_index has value of %i\n",
@@ -489,7 +492,6 @@ struct platform_driver lirc_rx51_platform_driver = {
.remove = __exit_p(lirc_rx51_remove),
.suspend= lirc_rx51_suspend,
.resume = lirc_rx51_resume,
- .remove = __exit_p(lirc_rx51_remove),
.driver = {
.name = DRIVER_NAME,
.owner = THIS_MODULE,
--
1.7.12
--
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 2/8] ir-rx51: Handle signals properly
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
---
drivers/media/rc/ir-rx51.c | 39 ---
1 file changed, 28 insertions(+), 11 deletions(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 9487dd3..a7b787a 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_interruptible(&lirc_rx51->wqueue);
+}
+
static int init_timing_params(struct lirc_rx51 *lirc_rx51)
{
u32 load, match;
@@ -160,13 +173,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;
}
@@ -246,8 +253,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;
@@ -273,9 +281,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.7.12
--
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 7/8] ir-rx51: Remove MPU wakeup latency adjustments
The ir-rx51 driver calls omap_pm_set_max_mpu_wakeup_lat() in order to
avoid problems that occur when MPU goes to sleep in the middle of
sending an IR code. Without such calls it takes ridiculously long for
the MPU to wake up from a sleep, which distorts the IR signal
completely.
However, the actual problem is that probably the GP timers are not
able to wake up the MPU at all. That is, adjusting the latency
requirements is not the correct way to solve the issue either. The
reason why this used to work with the original 2.6.28 based N900
kernel that is shipped with the product is that placing strict latency
requirements prevents the MPU from going to sleep at all. Furthermore,
the only PM layer imlementation available at the moment for OMAP3
doesn't do anything with the latency requirement placed with
omap_pm_set_max_mpu_wakeup_lat() calls.
A more appropriate fix for the problem would be to modify the idle
layer so that it does not allow MPU going to too deep sleep modes when
it is expected that the timers need to wake up MPU.
Therefore, it makes sense to actually remove this call entirely from
the ir-rx51 driver as it is both wrong and does nothing useful at the
moment.
Signed-off-by: Timo Kokkonen
---
arch/arm/mach-omap2/board-rx51-peripherals.c | 2 --
drivers/media/rc/ir-rx51.c | 9 -
include/media/ir-rx51.h | 2 --
3 files changed, 13 deletions(-)
diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c
b/arch/arm/mach-omap2/board-rx51-peripherals.c
index ca07264..e0750cb 100644
--- a/arch/arm/mach-omap2/board-rx51-peripherals.c
+++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
@@ -34,7 +34,6 @@
#include
#include
#include
-#include
#include
@@ -1227,7 +1226,6 @@ static void __init rx51_init_tsc2005(void)
#if defined(CONFIG_IR_RX51) || defined(CONFIG_IR_RX51_MODULE)
static struct lirc_rx51_platform_data rx51_lirc_data = {
- .set_max_mpu_wakeup_lat = omap_pm_set_max_mpu_wakeup_lat,
.pwm_timer = 9, /* Use GPT 9 for CIR */
};
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 7eed541..ac7d3f0 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -267,12 +267,6 @@ static ssize_t lirc_rx51_write(struct file *file, const
char *buf,
if (count < WBUF_LEN)
lirc_rx51->wbuf[count] = -1; /* Insert termination mark */
- /*
-* Adjust latency requirements so the device doesn't go in too
-* deep sleep states
-*/
- lirc_rx51->pdata->set_max_mpu_wakeup_lat(lirc_rx51->dev, 50);
-
lirc_rx51_on(lirc_rx51);
lirc_rx51->wbuf_index = 1;
pulse_timer_set_timeout(lirc_rx51, lirc_rx51->wbuf[0]);
@@ -292,9 +286,6 @@ static ssize_t lirc_rx51_write(struct file *file, const
char *buf,
*/
lirc_rx51_stop_tx(lirc_rx51);
- /* We can sleep again */
- lirc_rx51->pdata->set_max_mpu_wakeup_lat(lirc_rx51->dev, -1);
-
return n;
}
diff --git a/include/media/ir-rx51.h b/include/media/ir-rx51.h
index 104aa89..57523f2 100644
--- a/include/media/ir-rx51.h
+++ b/include/media/ir-rx51.h
@@ -3,8 +3,6 @@
struct lirc_rx51_platform_data {
int pwm_timer;
-
- int(*set_max_mpu_wakeup_lat)(struct device *dev, long t);
};
#endif
--
1.7.12
--
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 6/8] ir-rx51: Replace module_{init,exit} macros with module_platform_driver
No reason to avoid using the existing helpers.
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/ir-rx51.c | 12 +---
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 46628c0..7eed541 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -495,17 +495,7 @@ struct platform_driver lirc_rx51_platform_driver = {
},
};
-static int __init lirc_rx51_init(void)
-{
- return platform_driver_register(&lirc_rx51_platform_driver);
-}
-module_init(lirc_rx51_init);
-
-static void __exit lirc_rx51_exit(void)
-{
- platform_driver_unregister(&lirc_rx51_platform_driver);
-}
-module_exit(lirc_rx51_exit);
+module_platform_driver(lirc_rx51_platform_driver);
MODULE_DESCRIPTION("LIRC TX driver for Nokia RX51");
MODULE_AUTHOR("Nokia Corporation");
--
1.7.12
--
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 5/8] ir-rx51: Move platform data checking into probe function
This driver is useless without proper platform data. If data is not
available, we should not register the driver at all. Once this check
is done, the BUG_ON check during device open is no longer needed.
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/ir-rx51.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 3d2911b..46628c0 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -378,7 +378,6 @@ static long lirc_rx51_ioctl(struct file *filep,
static int lirc_rx51_open(struct inode *inode, struct file *file)
{
struct lirc_rx51 *lirc_rx51 = lirc_get_pdata(file);
- BUG_ON(!lirc_rx51);
file->private_data = lirc_rx51;
@@ -458,6 +457,9 @@ static int lirc_rx51_resume(struct platform_device *dev)
static int __devinit lirc_rx51_probe(struct platform_device *dev)
{
+ if (!dev->dev.platform_data)
+ return -ENODEV;
+
lirc_rx51_driver.features = LIRC_RX51_DRIVER_FEATURES;
lirc_rx51.pdata = dev->dev.platform_data;
lirc_rx51.pwm_timer_num = lirc_rx51.pdata->pwm_timer;
--
1.7.12
--
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 8/8] ir-rx51: Remove useless variable from struct lirc_rx51
As clearly visible from the patch, this variable has no useful purpose
what so ever. Thus, it can be removed altogether without any side
effects.
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/ir-rx51.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index ac7d3f0..23bc8c0 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -55,7 +55,6 @@ struct lirc_rx51 {
unsigned intfreq; /* carrier frequency */
unsigned intduty_cycle; /* carrier duty cycle */
unsigned intirq_num;
- unsigned intmatch;
int wbuf[WBUF_LEN];
int wbuf_index;
unsigned long device_is_open;
@@ -100,8 +99,6 @@ static int init_timing_params(struct lirc_rx51 *lirc_rx51)
omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer, 0);
omap_dm_timer_start(lirc_rx51->pulse_timer);
- lirc_rx51->match = 0;
-
return 0;
}
@@ -111,11 +108,7 @@ static int pulse_timer_set_timeout(struct lirc_rx51
*lirc_rx51, int usec)
BUG_ON(usec < 0);
- if (lirc_rx51->match == 0)
- counter = omap_dm_timer_read_counter(lirc_rx51->pulse_timer);
- else
- counter = lirc_rx51->match;
-
+ counter = omap_dm_timer_read_counter(lirc_rx51->pulse_timer);
counter += (u32)(lirc_rx51->fclk_khz * usec / (1000));
omap_dm_timer_set_match(lirc_rx51->pulse_timer, 1, counter);
omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer,
--
1.7.12
--
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 0/8] ir-rx51: Fixes in response to review comments
These patches fix most of the issues pointed out in the patch review
by Sean Young and Sakari Ailus. The most noticeable change after these
patch set is that the IR transmission no longer times out even if the
timers are not waking up the MPU as it should be. However, the
transmission itself is still as badly mangled as before, unless there
is some background load preventing the MPU from going into
sleep. Otherwise the patches are mostly clean ups and rather trivial
stuff.
All comments are welcome. Thanks!
Timo Kokkonen (8):
ir-rx51: Adjust dependencies
ir-rx51: Handle signals properly
ir-rx51: Trivial fixes
ir-rx51: Clean up timer initialization code
ir-rx51: Move platform data checking into probe function
ir-rx51: Replace module_{init,exit} macros with
module_platform_driver
ir-rx51: Remove MPU wakeup latency adjustments
ir-rx51: Remove useless variable from struct lirc_rx51
arch/arm/mach-omap2/board-rx51-peripherals.c | 2 -
drivers/media/rc/Kconfig | 4 +-
drivers/media/rc/ir-rx51.c | 92 +---
include/media/ir-rx51.h | 2 -
4 files changed, 43 insertions(+), 57 deletions(-)
--
1.7.12
--
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 1/8] ir-rx51: Adjust dependencies
Although this kind of IR diode circuitry is known to exist only in N900 hardware, nothing prevents making similar circuitry on any OMAP based board. The MACH_NOKIA_RX51 dependency is thus not something we want to be there. Also, this should depend on LIRC as it is a LIRC driver. Signed-off-by: Timo Kokkonen --- drivers/media/rc/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig index ffef8b4..093982b 100644 --- a/drivers/media/rc/Kconfig +++ b/drivers/media/rc/Kconfig @@ -273,7 +273,7 @@ config IR_IGUANA config IR_RX51 tristate "Nokia N900 IR transmitter diode - depends on MACH_NOKIA_RX51 && OMAP_DM_TIMER + depends on OMAP_DM_TIMER && LIRC ---help--- Say Y or M here if you want to enable support for the IR transmitter diode built in the Nokia N900 (RX51) device. -- 1.7.12 -- 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: [git:v4l-dvb/for_v3.7] [media] media: rc: Introduce RX51 IR transmitter driver
On 08/16/12 19:34, Sakari Ailus wrote: > Hi Sebastian, > > On Thu, Aug 16, 2012 at 01:21:04PM +0200, Sebastian Reichel wrote: >> Hi, >> It was an requirement back then that this driver needs to be a module as 99% of the N900 owners still don't even know they have this kind of capability on their devices, so it doesn't make sense to keep the module loaded unless the user actually needs it. >>> >>> I don't think that's so important --- currently the vast majority of the >>> N900 users using the mainline kernel compile it themselves. It's more >>> important to have a clean implementation at this point. >> >> I would like to enable this feature for the Debian OMAP kernel, >> which is not only used for N900, but also for Pandaboard, etc. > > Fair enough. Thanks for the info! > > Timo: thinking this a little more, do you think the call is really needed? > AFAIU it doesn't really achieve what it's supposed to, keeping the CPU from > going to sleep. I noticed exactly the same problem you did, it was bad to > the extent irsend failed due to a timeout unless I kept the CPU busy. Yes, that's right. It's not really useful as is. > So I think we can remove the call, which results in two things: the driver > can be built as a module and the platform data does not contain a function > pointer any longer. Yeah, I agree. Although with the original N900 kernel the call did make it work. But the power management implementation was different there too. Maybe the proper fix for the problem is today something different it was back then. If I have time I'll see if I can figure out something.. -Timo -- 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: [git:v4l-dvb/for_v3.7] [media] media: rc: Introduce RX51 IR transmitter driver
Terve Sakari, Long time no see. On 08/15/12 19:06, Sakari Ailus wrote: > Heippa, > > Thanks for the patch! I know Mauro has already applied this so any changes > would make a separate patch. > Patches are cheap :) I'll have also couple of changes from Sean's comments. > I have tested this up to the point I can see that the IR LED blinks --- > using my phone's camera. :-) But I have no receivers so the testing ends to > this. Thanks for your thorough review and testing. > > On Mon, Aug 13, 2012 at 09:53:45PM +0200, Mauro Carvalho Chehab wrote: >> This is an automatic generated email to let you know that the following >> patch were queued at the >> http://git.linuxtv.org/media_tree.git tree: >> >> Subject: [media] media: rc: Introduce RX51 IR transmitter driver >> Author: Timo Kokkonen >> Date:Fri Aug 10 06:16:36 2012 -0300 >> >> This is the driver for the IR transmitter diode found on the Nokia >> N900 (also known as RX51) device. The driver is mostly the same as >> found in the original 2.6.28 based kernel that comes with the device. >> >> The following modifications have been made compared to the original >> driver version: >> >> - Adopt to the changes that has happen in the kernel during the past >> five years, such as the change in the include paths >> >> - The OMAP DM-timers require much more care nowadays. The timers need >> to be enabled and disabled or otherwise many actions fail. Timers >> must not be freed without first stopping them or otherwise the timer >> cannot be requested again. >> >> The code has been tested with sending IR codes with N900 device >> running Debian userland. The device receiving the codes was Anysee >> DVB-C USB receiver. > > Just a general question: how much this driver actually depends on the N900? > I can see there's a dependency to OMAP DM timers, but couldn't you use the > same driver if you just wired the IR LED there? Even the timer configuration > is there, so it looks a lot more generic than N900-specific. > Yeah, I was thinking that there are no other boards that have an IR diode connected to the PWM timer pin. But that doesn't stop anyone from soldering a diode one to his/her board. >> Signed-off-by: Timo Kokkonen >> Cc: Tony Lindgren >> Cc: linux-o...@vger.kernel.org >> Cc: Sakari Ailus >> Signed-off-by: Mauro Carvalho Chehab >> >> drivers/media/rc/Kconfig | 10 + >> drivers/media/rc/Makefile |1 + >> drivers/media/rc/ir-rx51.c | 496 >> >> include/media/ir-rx51.h| 10 + >> 4 files changed, 517 insertions(+), 0 deletions(-) >> >> --- >> >> http://git.linuxtv.org/media_tree.git?a=commitdiff;h=c332e8472d7db67766bcad33390c607fdd9ac5bc >> >> diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig >> index 64be610..016f9ab 100644 >> --- a/drivers/media/rc/Kconfig >> +++ b/drivers/media/rc/Kconfig >> @@ -287,6 +287,16 @@ config IR_TTUSBIR >> To compile this driver as a module, choose M here: the module will >> be called ttusbir. >> >> +config IR_RX51 >> +tristate "Nokia N900 IR transmitter diode >> +depends on MACH_NOKIA_RX51 && OMAP_DM_TIMER > > You also should depend on LIRC. Yes. And if one had the diode in some other board than RX51, maybe this wouldn't need to depend on MACH_NOKIA_RX51 either. ... Which gave me an idea. We have this new PWM subsystem that I know nothing about. If the omap timer PWM routines were exposed through this API, we could modify this driver to a generic PWM lirc driver. I don't know how well the PWM api suits for that purpose, but it could be an interesting idea.. :) > >> +---help--- >> + Say Y or M here if you want to enable support for the IR >> + transmitter diode built in the Nokia N900 (RX51) device. >> + >> + The driver uses omap DM timers for gereating the carrier > > s/gereating/renerating/ > Uh oh, will fix.. >> + wave and pulses. >> + >> config RC_LOOPBACK >> tristate "Remote Control Loopback Driver" >> depends on RC_CORE >> diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile >> index 66c8bae..56bacf0 100644 >> --- a/drivers/media/rc/Makefile >> +++ b/drivers/media/rc/Makefile >> @@ -23,6 +23,7 @@ obj-$(CONFIG_IR_FINTEK) += fintek-cir.o >> obj-$(CONFIG_IR_NUVOTON) += nuvoton-cir.o >> obj-$(CONFIG_IR_ENE) += ene_ir.o >> obj-$(CONFIG_IR_REDRAT3) += redrat3.o >> +obj-$(CONFIG_I
Re: [PATCHv2 1/2] media: rc: Introduce RX51 IR transmitter driver
On 08/13/12 21:36, Sean Young wrote:
> On Fri, Aug 10, 2012 at 01:16:36PM +0300, Timo Kokkonen wrote:
>> +static ssize_t lirc_rx51_write(struct file *file, const char *buf,
>> + size_t n, loff_t *ppos)
>> +{
>> +int count, i;
>> +struct lirc_rx51 *lirc_rx51 = file->private_data;
>> +
>> +if (n % sizeof(int))
>> +return -EINVAL;
>> +
>> +count = n / sizeof(int);
>> +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);
>
> If a signal arrives then this could return ERESTARTSYS and the condition
> might not have evaluated to true.
hmm.. The whole point of it is to wait if for any possibly pending
transfers to finish. However, we don't allow the device to be opened
more than once and parallel access doesn't make much sense here anyway.
Only way we can end up waiting here is that the process is having
multiple threads writing to the same file descriptor (or has inherited
it from its parent), which doesn't make any sense.
I think we could simply return -EBUSY in case previous transfer is still
going on. I don't see any reason why we should wait here.
>
>> +
>> +if (copy_from_user(lirc_rx51->wbuf, buf, n))
>> +return -EFAULT;
>> +
>> +/* Sanity check the input pulses */
>> +for (i = 0; i < count; i++)
>> +if (lirc_rx51->wbuf[i] < 0)
>> +return -EINVAL;
>> +
>> +init_timing_params(lirc_rx51);
>> +if (count < WBUF_LEN)
>> +lirc_rx51->wbuf[count] = -1; /* Insert termination mark */
>> +
>> +/*
>> + * Adjust latency requirements so the device doesn't go in too
>> + * deep sleep states
>> + */
>> +lirc_rx51->pdata->set_max_mpu_wakeup_lat(lirc_rx51->dev, 50);
>> +
>> +lirc_rx51_on(lirc_rx51);
>> +lirc_rx51->wbuf_index = 1;
>> +pulse_timer_set_timeout(lirc_rx51, lirc_rx51->wbuf[0]);
>> +
>> +/*
>> + * Don't return back to the userspace until the transfer has
>> + * finished
>> + */
>> +wait_event_interruptible(lirc_rx51->wqueue, lirc_rx51->wbuf_index < 0);
>
> same here.
>
> BTW so the semantics for lirc write() are that they complete when the
> data has been transmitted. This doesn't play well with signals, polling
> or non-blocking I/O. Is this deliberate or historical?
>
> I guess a lirc write() handler should ignore signals completely.
>
I'll change it to wait_event_timeout instead.
>> +
>> +struct platform_driver lirc_rx51_platform_driver = {
>> +.probe = lirc_rx51_probe,
>> +.remove = __exit_p(lirc_rx51_remove),
>> +.suspend= lirc_rx51_suspend,
>> +.resume = lirc_rx51_resume,
>> +.remove = __exit_p(lirc_rx51_remove),
>
> .remove is here twice.
hehe.. I remember I was supposed to write a patch for that five years
ago but I was busy :) Thanks for your review. Will send round three.
-Timo
--
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: [PATCHv2 0/2] Add Nokia N900 (RX51) IR diode support
Hi, On 08/10/12 13:16, Timo Kokkonen wrote: > These patches add the support for sending IR remote controller codes > on the Nokia N900 phone. The code is taken from the public N900 kernel > release and modified to work with today's kernel. > > The code has been tested with a real Nokia N900 device and confirmed > to work. I can identify only one known issue; The IR pulses being sent > become *veeery* long if the device chooses to go into any sleep modes > during transmitting the IR pulses. The driver makes an attempt to set > up PM latency constraints, but apparently those don't apply as there > is currently only no-op PM layer available. Therefore, I guess this > driver doesn't actually work properly unless there is some background > load that prevents the device from enterint sleep modes or the sleep > modes are disabled altogether. However, once a proper PM layer > implementation becomes available, I expect this problem to resolve > itself. The same code used to work with the actual N900 kernel that > has those implemented. > > Any comments regarding the patches are welcome. > > I guess media list won't take in omap patches and omap list doesn't > take media patches. So I wrote the patches so that they can be applied > independently. If you want me to remove the #ifdef hacks from the > board file (that is needed to break the build dependency between the > patches), then the ir-rx51.c patch needs to be applied before the > board file patch. But I though it would be more flexible this way. I'm > open to suggestions on how you are willing to accept the patches. > > --- > > Changes since v1: > > - Move ir-rx51.h into include/media directory > Any comments on these? Anything still missing before you can consider accepting these? Thanks! -Timo > > Timo Kokkonen (2): > media: rc: Introduce RX51 IR transmitter driver > ARM: mach-omap2: board-rx51-peripherals: Add lirc-rx51 data > > arch/arm/mach-omap2/board-rx51-peripherals.c | 30 ++ > drivers/media/rc/Kconfig | 10 + > drivers/media/rc/Makefile|1 + > drivers/media/rc/ir-rx51.c | 496 > ++ > include/media/ir-rx51.h | 10 + > 5 files changed, 547 insertions(+), 0 deletions(-) > create mode 100644 drivers/media/rc/ir-rx51.c > create mode 100644 include/media/ir-rx51.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
[PATCHv2 1/2] media: rc: Introduce RX51 IR transmitter driver
This is the driver for the IR transmitter diode found on the Nokia
N900 (also known as RX51) device. The driver is mostly the same as
found in the original 2.6.28 based kernel that comes with the device.
The following modifications have been made compared to the original
driver version:
- Adopt to the changes that has happen in the kernel during the past
five years, such as the change in the include paths
- The OMAP DM-timers require much more care nowadays. The timers need
to be enabled and disabled or otherwise many actions fail. Timers
must not be freed without first stopping them or otherwise the timer
cannot be requested again.
The code has been tested with sending IR codes with N900 device
running Debian userland. The device receiving the codes was Anysee
DVB-C USB receiver.
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/Kconfig | 10 +
drivers/media/rc/Makefile |1 +
drivers/media/rc/ir-rx51.c | 496
include/media/ir-rx51.h| 10 +
4 files changed, 517 insertions(+), 0 deletions(-)
create mode 100644 drivers/media/rc/ir-rx51.c
create mode 100644 include/media/ir-rx51.h
diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index 5180390..ab35d2e 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -270,6 +270,16 @@ config IR_IGUANA
To compile this driver as a module, choose M here: the module will
be called iguanair.
+config IR_RX51
+ tristate "Nokia N900 IR transmitter diode
+ depends on MACH_NOKIA_RX51 && OMAP_DM_TIMER
+ ---help---
+ Say Y or M here if you want to enable support for the IR
+ transmitter diode built in the Nokia N900 (RX51) device.
+
+ The driver uses omap DM timers for gereating the carrier
+ wave and pulses.
+
config RC_LOOPBACK
tristate "Remote Control Loopback Driver"
depends on RC_CORE
diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
index f871d19..d384f30 100644
--- a/drivers/media/rc/Makefile
+++ b/drivers/media/rc/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_IR_FINTEK) += fintek-cir.o
obj-$(CONFIG_IR_NUVOTON) += nuvoton-cir.o
obj-$(CONFIG_IR_ENE) += ene_ir.o
obj-$(CONFIG_IR_REDRAT3) += redrat3.o
+obj-$(CONFIG_IR_RX51) += ir-rx51.o
obj-$(CONFIG_IR_STREAMZAP) += streamzap.o
obj-$(CONFIG_IR_WINBOND_CIR) += winbond-cir.o
obj-$(CONFIG_RC_LOOPBACK) += rc-loopback.o
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
new file mode 100644
index 000..9487dd3
--- /dev/null
+++ b/drivers/media/rc/ir-rx51.c
@@ -0,0 +1,496 @@
+/*
+ * Copyright (C) 2008 Nokia Corporation
+ *
+ * Based on lirc_serial.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+#define LIRC_RX51_DRIVER_FEATURES (LIRC_CAN_SET_SEND_DUTY_CYCLE | \
+ LIRC_CAN_SET_SEND_CARRIER | \
+ LIRC_CAN_SEND_PULSE)
+
+#define DRIVER_NAME "lirc_rx51"
+
+#define WBUF_LEN 256
+
+#define TIMER_MAX_VALUE 0x
+
+struct lirc_rx51 {
+ struct omap_dm_timer *pwm_timer;
+ struct omap_dm_timer *pulse_timer;
+ struct device*dev;
+ struct lirc_rx51_platform_data *pdata;
+ wait_queue_head_t wqueue;
+
+ unsigned long fclk_khz;
+ unsigned intfreq; /* carrier frequency */
+ unsigned intduty_cycle; /* carrier duty cycle */
+ unsigned intirq_num;
+ unsigned intmatch;
+ int wbuf[WBUF_LEN];
+ int wbuf_index;
+ unsigned long device_is_open;
+ unsigned intpwm_timer_num;
+};
+
+static void lirc_rx51_on(struct lirc_rx51 *lirc_rx51)
+{
+ omap_dm_timer_set_pwm(lirc_rx51->pwm_timer, 0, 1,
+ OMAP_TIMER_TRIGGER_OVERFLOW_AND_COMPARE);
+}
+
+static void lirc_rx51_off(struct lirc_rx51 *lirc_rx51)
+{
+ omap_dm_timer_set_pwm(lirc_rx51->pwm_timer, 0, 1,
+ OMAP_TIMER_TRIGGER_NONE);
+}
+
+static int init_timing_params(struct lirc_rx51 *lirc_rx51)
+{
[PATCHv2 2/2] ARM: mach-omap2: board-rx51-peripherals: Add lirc-rx51 data
The IR diode on the RX51 is connected to the GPT9. This data is needed
for the IR driver to function.
Signed-off-by: Timo Kokkonen
---
arch/arm/mach-omap2/board-rx51-peripherals.c | 30 ++
1 files changed, 30 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c
b/arch/arm/mach-omap2/board-rx51-peripherals.c
index df2534d..ca07264 100644
--- a/arch/arm/mach-omap2/board-rx51-peripherals.c
+++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
@@ -34,6 +34,7 @@
#include
#include
#include
+#include
#include
@@ -46,6 +47,10 @@
#include <../drivers/staging/iio/light/tsl2563.h>
#include
+#if defined(CONFIG_IR_RX51) || defined(CONFIG_IR_RX51_MODULE)
+#include
+#endif
+
#include "mux.h"
#include "hsmmc.h"
#include "common-board-devices.h"
@@ -1220,6 +1225,30 @@ static void __init rx51_init_tsc2005(void)
gpio_to_irq(RX51_TSC2005_IRQ_GPIO);
}
+#if defined(CONFIG_IR_RX51) || defined(CONFIG_IR_RX51_MODULE)
+static struct lirc_rx51_platform_data rx51_lirc_data = {
+ .set_max_mpu_wakeup_lat = omap_pm_set_max_mpu_wakeup_lat,
+ .pwm_timer = 9, /* Use GPT 9 for CIR */
+};
+
+static struct platform_device rx51_lirc_device = {
+ .name = "lirc_rx51",
+ .id = -1,
+ .dev= {
+ .platform_data = &rx51_lirc_data,
+ },
+};
+
+static void __init rx51_init_lirc(void)
+{
+ platform_device_register(&rx51_lirc_device);
+}
+#else
+static void __init rx51_init_lirc(void)
+{
+}
+#endif
+
void __init rx51_peripherals_init(void)
{
rx51_i2c_init();
@@ -1230,6 +1259,7 @@ void __init rx51_peripherals_init(void)
rx51_init_wl1251();
rx51_init_tsc2005();
rx51_init_si4713();
+ rx51_init_lirc();
spi_register_board_info(rx51_peripherals_spi_board_info,
ARRAY_SIZE(rx51_peripherals_spi_board_info));
--
1.7.8.6
--
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 0/2] Add Nokia N900 (RX51) IR diode support
These patches add the support for sending IR remote controller codes on the Nokia N900 phone. The code is taken from the public N900 kernel release and modified to work with today's kernel. The code has been tested with a real Nokia N900 device and confirmed to work. I can identify only one known issue; The IR pulses being sent become *veeery* long if the device chooses to go into any sleep modes during transmitting the IR pulses. The driver makes an attempt to set up PM latency constraints, but apparently those don't apply as there is currently only no-op PM layer available. Therefore, I guess this driver doesn't actually work properly unless there is some background load that prevents the device from enterint sleep modes or the sleep modes are disabled altogether. However, once a proper PM layer implementation becomes available, I expect this problem to resolve itself. The same code used to work with the actual N900 kernel that has those implemented. Any comments regarding the patches are welcome. I guess media list won't take in omap patches and omap list doesn't take media patches. So I wrote the patches so that they can be applied independently. If you want me to remove the #ifdef hacks from the board file (that is needed to break the build dependency between the patches), then the ir-rx51.c patch needs to be applied before the board file patch. But I though it would be more flexible this way. I'm open to suggestions on how you are willing to accept the patches. --- Changes since v1: - Move ir-rx51.h into include/media directory Timo Kokkonen (2): media: rc: Introduce RX51 IR transmitter driver ARM: mach-omap2: board-rx51-peripherals: Add lirc-rx51 data arch/arm/mach-omap2/board-rx51-peripherals.c | 30 ++ drivers/media/rc/Kconfig | 10 + drivers/media/rc/Makefile|1 + drivers/media/rc/ir-rx51.c | 496 ++ include/media/ir-rx51.h | 10 + 5 files changed, 547 insertions(+), 0 deletions(-) create mode 100644 drivers/media/rc/ir-rx51.c create mode 100644 include/media/ir-rx51.h -- 1.7.8.6 -- 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 2/2] ARM: mach-omap2: board-rx51-peripherals: Add lirc-rx51 data
On 08/09/12 16:19, Igor Grinberg wrote:
> Hi Timo,
>
> On 08/09/12 15:41, Timo Kokkonen wrote:
>> The IR diode on the RX51 is connected to the GPT9. This data is needed
>> for the IR driver to function.
>>
>> Signed-off-by: Timo Kokkonen
>> ---
>> arch/arm/mach-omap2/board-rx51-peripherals.c | 30
>> ++
>> 1 files changed, 30 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c
>> b/arch/arm/mach-omap2/board-rx51-peripherals.c
>> index df2534d..4a5a71b 100644
>> --- a/arch/arm/mach-omap2/board-rx51-peripherals.c
>> +++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
>> @@ -34,6 +34,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include
>>
>> @@ -46,6 +47,10 @@
>> #include <../drivers/staging/iio/light/tsl2563.h>
>> #include
>>
>> +#if defined(CONFIG_IR_RX51) || defined(CONFIG_IR_RX51_MODULE)
>> +#include "../../../drivers/media/rc/ir-rx51.h"
>> +#endif
>
> That is not really nice...
> You should place the struct lirc_rx51_platform_data and
> other stuff used outside of the driver in: include/media/
> so you will not have to add that long and ugly relative path.
>
Yeah, you're right. I'll change that. Thanks.
-Timo
>> +
>> #include "mux.h"
>> #include "hsmmc.h"
>> #include "common-board-devices.h"
>> @@ -1220,6 +1225,30 @@ static void __init rx51_init_tsc2005(void)
>> gpio_to_irq(RX51_TSC2005_IRQ_GPIO);
>> }
>>
>> +#if defined(CONFIG_IR_RX51) || defined(CONFIG_IR_RX51_MODULE)
>> +static struct lirc_rx51_platform_data rx51_lirc_data = {
>> +.set_max_mpu_wakeup_lat = omap_pm_set_max_mpu_wakeup_lat,
>> +.pwm_timer = 9, /* Use GPT 9 for CIR */
>> +};
>> +
>> +static struct platform_device rx51_lirc_device = {
>> +.name = "lirc_rx51",
>> +.id = -1,
>> +.dev= {
>> +.platform_data = &rx51_lirc_data,
>> +},
>> +};
>> +
>> +static void __init rx51_init_lirc(void)
>> +{
>> +platform_device_register(&rx51_lirc_device);
>> +}
>> +#else
>> +static void __init rx51_init_lirc(void)
>> +{
>> +}
>> +#endif
>> +
>> void __init rx51_peripherals_init(void)
>> {
>> rx51_i2c_init();
>> @@ -1230,6 +1259,7 @@ void __init rx51_peripherals_init(void)
>> rx51_init_wl1251();
>> rx51_init_tsc2005();
>> rx51_init_si4713();
>> +rx51_init_lirc();
>> spi_register_board_info(rx51_peripherals_spi_board_info,
>> ARRAY_SIZE(rx51_peripherals_spi_board_info));
>>
>
--
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 2/2] ARM: mach-omap2: board-rx51-peripherals: Add lirc-rx51 data
The IR diode on the RX51 is connected to the GPT9. This data is needed
for the IR driver to function.
Signed-off-by: Timo Kokkonen
---
arch/arm/mach-omap2/board-rx51-peripherals.c | 30 ++
1 files changed, 30 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c
b/arch/arm/mach-omap2/board-rx51-peripherals.c
index df2534d..4a5a71b 100644
--- a/arch/arm/mach-omap2/board-rx51-peripherals.c
+++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
@@ -34,6 +34,7 @@
#include
#include
#include
+#include
#include
@@ -46,6 +47,10 @@
#include <../drivers/staging/iio/light/tsl2563.h>
#include
+#if defined(CONFIG_IR_RX51) || defined(CONFIG_IR_RX51_MODULE)
+#include "../../../drivers/media/rc/ir-rx51.h"
+#endif
+
#include "mux.h"
#include "hsmmc.h"
#include "common-board-devices.h"
@@ -1220,6 +1225,30 @@ static void __init rx51_init_tsc2005(void)
gpio_to_irq(RX51_TSC2005_IRQ_GPIO);
}
+#if defined(CONFIG_IR_RX51) || defined(CONFIG_IR_RX51_MODULE)
+static struct lirc_rx51_platform_data rx51_lirc_data = {
+ .set_max_mpu_wakeup_lat = omap_pm_set_max_mpu_wakeup_lat,
+ .pwm_timer = 9, /* Use GPT 9 for CIR */
+};
+
+static struct platform_device rx51_lirc_device = {
+ .name = "lirc_rx51",
+ .id = -1,
+ .dev= {
+ .platform_data = &rx51_lirc_data,
+ },
+};
+
+static void __init rx51_init_lirc(void)
+{
+ platform_device_register(&rx51_lirc_device);
+}
+#else
+static void __init rx51_init_lirc(void)
+{
+}
+#endif
+
void __init rx51_peripherals_init(void)
{
rx51_i2c_init();
@@ -1230,6 +1259,7 @@ void __init rx51_peripherals_init(void)
rx51_init_wl1251();
rx51_init_tsc2005();
rx51_init_si4713();
+ rx51_init_lirc();
spi_register_board_info(rx51_peripherals_spi_board_info,
ARRAY_SIZE(rx51_peripherals_spi_board_info));
--
1.7.8.6
--
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 1/2] media: rc: Introduce RX51 IR transmitter driver
This is the driver for the IR transmitter diode found on the Nokia
N900 (also known as RX51) device. The driver is mostly the same as
found in the original 2.6.28 based kernel that comes with the device.
The following modifications have been made compared to the original
driver version:
- Adopt to the changes that has happen in the kernel during the past
five years, such as the change in the include paths
- The OMAP DM-timers require much more care nowadays. The timers need
to be enabled and disabled or otherwise many actions fail. Timers
must not be freed without first stopping them or otherwise the timer
cannot be requested again.
The code has been tested with sending IR codes with N900 device
running Debian userland. The device receiving the codes was Anysee
DVB-C USB receiver.
Signed-off-by: Timo Kokkonen
---
drivers/media/rc/Kconfig | 10 +
drivers/media/rc/Makefile |1 +
drivers/media/rc/ir-rx51.c | 496
drivers/media/rc/ir-rx51.h | 10 +
4 files changed, 517 insertions(+), 0 deletions(-)
create mode 100644 drivers/media/rc/ir-rx51.c
create mode 100644 drivers/media/rc/ir-rx51.h
diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index 5180390..ab35d2e 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -270,6 +270,16 @@ config IR_IGUANA
To compile this driver as a module, choose M here: the module will
be called iguanair.
+config IR_RX51
+ tristate "Nokia N900 IR transmitter diode
+ depends on MACH_NOKIA_RX51 && OMAP_DM_TIMER
+ ---help---
+ Say Y or M here if you want to enable support for the IR
+ transmitter diode built in the Nokia N900 (RX51) device.
+
+ The driver uses omap DM timers for gereating the carrier
+ wave and pulses.
+
config RC_LOOPBACK
tristate "Remote Control Loopback Driver"
depends on RC_CORE
diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
index f871d19..d384f30 100644
--- a/drivers/media/rc/Makefile
+++ b/drivers/media/rc/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_IR_FINTEK) += fintek-cir.o
obj-$(CONFIG_IR_NUVOTON) += nuvoton-cir.o
obj-$(CONFIG_IR_ENE) += ene_ir.o
obj-$(CONFIG_IR_REDRAT3) += redrat3.o
+obj-$(CONFIG_IR_RX51) += ir-rx51.o
obj-$(CONFIG_IR_STREAMZAP) += streamzap.o
obj-$(CONFIG_IR_WINBOND_CIR) += winbond-cir.o
obj-$(CONFIG_RC_LOOPBACK) += rc-loopback.o
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
new file mode 100644
index 000..66d2607
--- /dev/null
+++ b/drivers/media/rc/ir-rx51.c
@@ -0,0 +1,496 @@
+/*
+ * Copyright (C) 2008 Nokia Corporation
+ *
+ * Based on lirc_serial.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+#include
+#include
+#include "ir-rx51.h"
+
+#define LIRC_RX51_DRIVER_FEATURES (LIRC_CAN_SET_SEND_DUTY_CYCLE | \
+ LIRC_CAN_SET_SEND_CARRIER | \
+ LIRC_CAN_SEND_PULSE)
+
+#define DRIVER_NAME "lirc_rx51"
+
+#define WBUF_LEN 256
+
+#define TIMER_MAX_VALUE 0x
+
+struct lirc_rx51 {
+ struct omap_dm_timer *pwm_timer;
+ struct omap_dm_timer *pulse_timer;
+ struct device*dev;
+ struct lirc_rx51_platform_data *pdata;
+ wait_queue_head_t wqueue;
+
+ unsigned long fclk_khz;
+ unsigned intfreq; /* carrier frequency */
+ unsigned intduty_cycle; /* carrier duty cycle */
+ unsigned intirq_num;
+ unsigned intmatch;
+ int wbuf[WBUF_LEN];
+ int wbuf_index;
+ unsigned long device_is_open;
+ unsigned intpwm_timer_num;
+};
+
+static void lirc_rx51_on(struct lirc_rx51 *lirc_rx51)
+{
+ omap_dm_timer_set_pwm(lirc_rx51->pwm_timer, 0, 1,
+ OMAP_TIMER_TRIGGER_OVERFLOW_AND_COMPARE);
+}
+
+static void lirc_rx51_off(struct lirc_rx51 *lirc_rx51)
+{
+ omap_dm_timer_set_pwm(lirc_rx51->pwm_timer, 0, 1,
+ OMAP_TIMER_TRIGGER_NONE);
+}
+
+static int init_timing_params(struct
[PATCH 0/2] Add Nokia N900 (RX51) IR diode support
These patches add the support for sending IR remote controller codes on the Nokia N900 phone. The code is taken from the public N900 kernel release and modified to work with today's kernel. The code has been tested with a real Nokia N900 device and confirmed to work. I can identify only one known issue; The IR pulses being sent become *veeery* long if the device chooses to go into any sleep modes during transmitting the IR pulses. The driver makes an attempt to set up PM latency constraints, but apparently those don't apply as there is currently only no-op PM layer available. Therefore, I guess this driver doesn't actually work properly unless there is some background load that prevents the device from enterint sleep modes or the sleep modes are disabled altogether. However, once a proper PM layer implementation becomes available, I expect this problem to resolve itself. The same code used to work with the actual N900 kernel that has those implemented. Any comments regarding the patches are welcome. I guess media list won't take in omap patches and omap list doesn't take media patches. So I wrote the patches so that they can be applied independently. If you want me to remove the #ifdef hacks from the board file (that is needed to break the build dependency between the patches), then the ir-rx51.c patch needs to be applied before the board file patch. But I though it would be more flexible this way. I'm open to suggestions on how you are willing to accept the patches. Timo Kokkonen (2): media: rc: Introduce RX51 IR transmitter driver ARM: mach-omap2: board-rx51-peripherals: Add lirc-rx51 data arch/arm/mach-omap2/board-rx51-peripherals.c | 27 ++ drivers/media/rc/Kconfig | 10 + drivers/media/rc/Makefile|1 + drivers/media/rc/ir-rx51.c | 496 ++ drivers/media/rc/ir-rx51.h | 10 + 5 files changed, 544 insertions(+), 0 deletions(-) create mode 100644 drivers/media/rc/ir-rx51.c create mode 100644 drivers/media/rc/ir-rx51.h -- 1.7.8.6 -- 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] saa7134.h: Suppress compiler warnings when CONFIG_VIDEO_SAA7134_RC is not set
If the said config optio is not set, the compiler will spill out many warnings about statements with no effect, such as: drivers/media/video/saa7134/saa7134-core.c: In function ‘saa7134_irq’: drivers/media/video/saa7134/saa7134-core.c:569:7: warning: statement with no effect drivers/media/video/saa7134/saa7134-core.c:588:7: warning: statement with no effect Casting the zero to void will cure the warning. Signed-off-by: Timo Kokkonen --- drivers/media/video/saa7134/saa7134.h | 12 ++-- 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/media/video/saa7134/saa7134.h b/drivers/media/video/saa7134/saa7134.h index bc8d6bb..9b55068 100644 --- a/drivers/media/video/saa7134/saa7134.h +++ b/drivers/media/video/saa7134/saa7134.h @@ -843,10 +843,10 @@ void saa7134_probe_i2c_ir(struct saa7134_dev *dev); int saa7134_ir_start(struct saa7134_dev *dev); void saa7134_ir_stop(struct saa7134_dev *dev); #else -#define saa7134_input_init1(dev) (0) -#define saa7134_input_fini(dev)(0) -#define saa7134_input_irq(dev) (0) -#define saa7134_probe_i2c_ir(dev) (0) -#define saa7134_ir_start(dev) (0) -#define saa7134_ir_stop(dev) (0) +#define saa7134_input_init1(dev) ((void)0) +#define saa7134_input_fini(dev)((void)0) +#define saa7134_input_irq(dev) ((void)0) +#define saa7134_probe_i2c_ir(dev) ((void)0) +#define saa7134_ir_start(dev) ((void)0) +#define saa7134_ir_stop(dev) ((void)0) #endif -- 1.7.7 -- 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
