Re: [PATCH 1/7] ir-rx51: Handle signals properly
On 12/14/12 19:26, Felipe Balbi wrote:
Hi,
On Fri, Dec 14, 2012 at 09:28:09AM -0800, Tony Lindgren wrote:
* Tony Lindgren t...@atomide.com [121120 12:00]:
Hi,
* Timo Kokkonen timo.t.kokko...@iki.fi [121118 07:15]:
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -74,6 +74,19 @@ static void lirc_rx51_off(struct lirc_rx51 *lirc_rx51)
OMAP_TIMER_TRIGGER_NONE);
}
+static void lirc_rx51_stop_tx(struct lirc_rx51 *lirc_rx51)
+{
+ if (lirc_rx51-wbuf_index 0)
+ return;
+
+ lirc_rx51_off(lirc_rx51);
+ lirc_rx51-wbuf_index = -1;
+ omap_dm_timer_stop(lirc_rx51-pwm_timer);
+ omap_dm_timer_stop(lirc_rx51-pulse_timer);
+ omap_dm_timer_set_int_enable(lirc_rx51-pulse_timer, 0);
+ wake_up(lirc_rx51-wqueue);
+}
+
static int init_timing_params(struct lirc_rx51 *lirc_rx51)
{
u32 load, match;
Good fixes in general.. But you won't be able to access the
omap_dm_timer functions after we enable ARM multiplatform support
for omap2+. That's for v3.9 probably right after v3.8-rc1.
We need to find some Linux generic API to use hardware timers
like this, so I've added Thomas Gleixner and linux-arm-kernel
mailing list to cc.
If no such API is available, then maybe we can export some of
the omap_dm_timer functions if Thomas is OK with that.
Just to update the status on this.. It seems that we'll be moving
parts of plat/dmtimer into a minimal include/linux/timer-omap.h
unless people have better ideas on what to do with custom
hardware timers for PWM etc.
if it's really for PWM, shouldn't we be using drivers/pwm/ ??
Now that Neil Brown posted the PWM driver for omap, I've been thinking
about whether converting the ir-rx51 into the PWM API would work. Maybe
controlling the PWM itself would be sufficient, but the ir-rx51 uses
also another dmtimer for creating accurate (enough) timing source for
the IR pulse edges.
I haven't tried whether the default 32kHz clock source is enough for
that. Now that I think about it, I don't see why it wouldn't be good
enough. I think it would even be possible to just use the PWM api alone
(plus hr-timers) in order to generate good enough IR pulses.
-Timo
Meaning that $SUBJECT would just request a PWM device and use it. That
doesn't solve the whole problem, however, as pwm-omap.c would still need
access to timer-omap.h.
--
To unsubscribe from this list: send the line unsubscribe linux-omap 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-omap 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 timo.t.kokko...@iki.fi
---
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-omap 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 timo.t.kokko...@iki.fi
---
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-omap 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 timo.t.kokko...@iki.fi
---
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-omap 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 timo.t.kokko...@iki.fi
---
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-omap 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 timo.t.kokko...@iki.fi
---
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-omap 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 timo.t.kokko...@iki.fi
Acked-by: Tony Lindgren t...@atomide.com
Acked-by: Jean Pihet j-pi...@ti.com
---
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 plat/dma.h
#include plat/gpmc.h
-#include plat/omap-pm.h
#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 linux/platform_device.h
#include linux/sched.h
#include linux/wait.h
+#include linux/pm_qos.h
#include plat/dmtimer.h
#include plat/clock.h
@@ -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-omap 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 timo.t.kokko...@iki.fi
---
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-omap 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. 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? You want to know when the hardware is done sending the IR. If you return EINTR to user space, how would user space know how much IR has been sent, if any? This ABI is not particularily elegant so there are proposals for a better interface which would obsolete the lirc interface. David Hardeman has worked on this: http://patchwork.linuxtv.org/patch/11411/ It appears that all modern lirc drivers are now using the rc-core functionalities to implement the common stuff. When the rx51 lirc driver was first written, the core was not in place yet. Therefore it is implementing the file operations in the driver, which other rc drivers won't do today. So, I think it would make sense to modify the rx51 driver to use the rc core functionality. But if there is an ABI change ongoing, I could wait until you have that done before I start working on the change? Considering this patch set, I think it makes sense still to apply these as they improve the existing code base. I'll just squash the one patch to the misc fixes, as pointed by Sakari, and then re-send the set. -Timo 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
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-omap 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 timo.t.kokko...@iki.fi --- 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-omap 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-omap 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/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-omap 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 timo.t.kokko...@iki.fi --- 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-omap 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 timo.t.kokko...@iki.fi
---
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-omap 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 timo.t.kokko...@iki.fi
Acked-by: Tony Lindgren t...@atomide.com
Acked-by: Jean Pihet j-pi...@ti.com
---
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 plat/gpmc.h
#include plat/onenand.h
#include plat/gpmc-smc91x.h
-#include plat/omap-pm.h
#include mach/board-rx51.h
@@ -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 linux/platform_device.h
#include linux/sched.h
#include linux/wait.h
+#include linux/pm_qos.h
#include plat/dmtimer.h
#include plat/clock.h
@@ -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-omap 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 timo.t.kokko...@iki.fi
---
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-omap 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 timo.t.kokko...@iki.fi --- 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-omap 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 timo.t.kokko...@iki.fi
---
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-omap 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 timo.t.kokko...@iki.fi
---
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-omap 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 timo.t.kokko...@iki.fi
---
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-omap 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 timo.t.kokko...@iki.fi
---
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-omap 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-omap 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 t...@atomide.com wrote: * Timo Kokkonen timo.t.kokko...@iki.fi [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 timo.t.kokko...@iki.fi This should get acked by Kevin ideally. Other than that: Acked-by: Tony Lindgren t...@atomide.com ... @@ -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 j-pi...@ti.com 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-omap 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 timo.t.kokko...@iki.fi 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-omapm=133968658305580w=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 timo.t.kokko...@iki.fi
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 plat/gpmc.h
#include plat/onenand.h
#include plat/gpmc-smc91x.h
-#include plat/omap-pm.h
#include mach/board-rx51.h
@@ -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-omap 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 timo.t.kokko...@iki.fi
---
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-omap 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 timo.t.kokko...@iki.fi 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 timo.t.kokko...@iki.fi 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-omapm=133968658305580w=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-omap 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-omap 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 timo.t.kokko...@iki.fi --- 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-omap 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 timo.t.kokko...@iki.fi
---
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-omap 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 timo.t.kokko...@iki.fi
---
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-omap 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 timo.t.kokko...@iki.fi
---
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-omap 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 timo.t.kokko...@iki.fi
---
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-omap 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 timo.t.kokko...@iki.fi
---
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 plat/gpmc.h
#include plat/onenand.h
#include plat/gpmc-smc91x.h
-#include plat/omap-pm.h
#include mach/board-rx51.h
@@ -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 linux/platform_device.h
#include linux/sched.h
#include linux/wait.h
+#include linux/pm_qos.h
#include plat/dmtimer.h
#include plat/clock.h
@@ -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-omap 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 timo.t.kokko...@iki.fi
---
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-omap 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 timo.t.kokko...@iki.fi
---
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-omap 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 timo.t.kokko...@iki.fi --- 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-omap 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-omap 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 timo.t.kokko...@iki.fi
---
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-omap 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 timo.t.kokko...@iki.fi
---
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-omap 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 timo.t.kokko...@iki.fi
---
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-omap 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 timo.t.kokko...@iki.fi
---
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 plat/gpmc.h
#include plat/onenand.h
#include plat/gpmc-smc91x.h
-#include plat/omap-pm.h
#include mach/board-rx51.h
@@ -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
[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 timo.t.kokko...@iki.fi
trivial
Signed-off-by: Timo Kokkonen timo.t.kokko...@iki.fi
---
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-omap 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 timo.t.kokko...@iki.fi
---
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-omap 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 timo.t.kokko...@iki.fi
---
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-omap 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-omap 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 timo.t.kokko...@iki.fi 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 timo.t.kokko...@iki.fi Cc: Tony Lindgren t...@atomide.com Cc: linux-omap@vger.kernel.org Cc: Sakari Ailus sakari.ai...@iki.fi Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com 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_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
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-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] omap: dmtimers: Fix locking issue in omap_dm_timer_request*()
Calling omap_dm_timer_prepare while the spinlock is held is not
allowed as sleeping functions are called later on during the
preparation (namely within clk_get()).
dm_timer_lock is only required for protecting the
omap_timer_list. After the timer is marked as reserved, the lock is no
longer needed and should be freed.
Signed-off-by: Timo Kokkonen timo.t.kokko...@iki.fi
Cc: Tony Lindgren t...@atomide.com
Cc: Tarun Kanti DebBarma tarun.ka...@ti.com
---
arch/arm/plat-omap/dmtimer.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index 626ad8c..1f66cac 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -189,6 +189,7 @@ struct omap_dm_timer *omap_dm_timer_request(void)
timer-reserved = 1;
break;
}
+ spin_unlock_irqrestore(dm_timer_lock, flags);
if (timer) {
ret = omap_dm_timer_prepare(timer);
@@ -197,7 +198,6 @@ struct omap_dm_timer *omap_dm_timer_request(void)
timer = NULL;
}
}
- spin_unlock_irqrestore(dm_timer_lock, flags);
if (!timer)
pr_debug(%s: timer request failed!\n, __func__);
@@ -220,6 +220,7 @@ struct omap_dm_timer *omap_dm_timer_request_specific(int id)
break;
}
}
+ spin_unlock_irqrestore(dm_timer_lock, flags);
if (timer) {
ret = omap_dm_timer_prepare(timer);
@@ -228,7 +229,6 @@ struct omap_dm_timer *omap_dm_timer_request_specific(int id)
timer = NULL;
}
}
- spin_unlock_irqrestore(dm_timer_lock, flags);
if (!timer)
pr_debug(%s: timer%d request failed!\n, __func__, id);
--
1.7.8.6
--
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
[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-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[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 timo.t.kokko...@iki.fi
---
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 plat/gpmc.h
#include plat/onenand.h
#include plat/gpmc-smc91x.h
+#include plat/omap-pm.h
#include mach/board-rx51.h
@@ -46,6 +47,10 @@
#include ../drivers/staging/iio/light/tsl2563.h
#include linux/lis3lv02d.h
+#if defined(CONFIG_IR_RX51) || defined(CONFIG_IR_RX51_MODULE)
+#include media/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-omap 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 timo.t.kokko...@iki.fi
---
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 linux/module.h
+#include linux/interrupt.h
+#include linux/uaccess.h
+#include linux/platform_device.h
+#include linux/sched.h
+#include linux/wait.h
+
+#include plat/dmtimer.h
+#include plat/clock.h
+#include plat/omap-pm.h
+
+#include media/lirc.h
+#include media/lirc_dev.h
+#include media/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
Re: [PATCHv2 1/2] media: rc: Introduce RX51 IR transmitter driver
I should have probably tried this before, but when I enabled lock
debugging with this driver, I got this:
[ 663.848083] BUG: sleeping function called from invalid context at
kernel/mutex.c:269
[ 663.856262] in_atomic(): 1, irqs_disabled(): 128, pid: 889, name: lircd
[ 663.863220] 1 lock held by lircd/889:
[ 663.867065] #0: (dm_timer_lock){..}, at: [c00343e4]
omap_dm_timer_request_specific+0x18/0xc4
[ 663.876739] irq event stamp: 1770
[ 663.880218] hardirqs last enabled at (1769): [c050e4fc]
__mutex_unlock_slowpath+0xe0/0x15c
[ 663.889221] hardirqs last disabled at (1770): [c05102e8]
_raw_spin_lock_irqsave+0x1c/0x60
[ 663.898010] softirqs last enabled at (1674): [c04bcc14]
unix_create1+0x148/0x174
[ 663.906066] softirqs last disabled at (1672): [c04bcbfc]
unix_create1+0x130/0x174
[ 663.914154] [c0019a90] (unwind_backtrace+0x0/0xf8) from [c050e11c]
(mutex_lock_nested+0x28/0x328)
[ 663.923858] [c050e11c] (mutex_lock_nested+0x28/0x328) from [c04196bc]
(clk_get_sys+0x1c/0xfc)
[ 663.933197] [c04196bc] (clk_get_sys+0x1c/0xfc) from [c0034344]
(omap_dm_timer_prepare+0xe4/0x16c)
[ 663.942901] [c0034344] (omap_dm_timer_prepare+0xe4/0x16c) from
[c0034440] (omap_dm_timer_request_specific+0x74/0xc4)
[ 663.954345] [c0034440] (omap_dm_timer_request_specific+0x74/0xc4) from
[c03e7554] (lirc_rx51_open+0x50/0x154)
[ 663.965148] [c03e7554] (lirc_rx51_open+0x50/0x154) from [c0103cf0]
(chrdev_open+0x98/0x16c)
[ 663.974304] [c0103cf0] (chrdev_open+0x98/0x16c) from [c00fd608]
(do_dentry_open+0x1dc/0x264)
[ 663.983551] [c00fd608] (do_dentry_open+0x1dc/0x264) from [c00fd9ec]
(finish_open+0x44/0x5c)
[ 663.992706] [c00fd9ec] (finish_open+0x44/0x5c) from [c010e230]
(do_last.isra.21+0x598/0xba8)
[ 664.001953] [c010e230] (do_last.isra.21+0x598/0xba8) from [c010e8e8]
(path_openat+0xa8/0x47c)
[ 664.011291] [c010e8e8] (path_openat+0xa8/0x47c) from [c010efb4]
(do_filp_open+0x2c/0x80)
[ 664.020172] [c010efb4] (do_filp_open+0x2c/0x80) from [c00fe674]
(do_sys_open+0xe0/0x174)
[ 664.029052] [c00fe674] (do_sys_open+0xe0/0x174) from [c0013a60]
(ret_fast_syscall+0x0/0x3c)
It appears that in omap_dm_timer_request() there is a call to
spin_lock_irqsave() and then, as visible on the back trace,
clk_get_sys() takes a mutex lock while the spinlock is still held.
To me it appears commit 3392cdd33a0 might have introduced this problem,
but I'm not sure.
Any thoughts?
-Timo
--
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
Re: [PATCHv2 1/2] media: rc: Introduce RX51 IR transmitter driver
On 08.10 2012 14:06:38, Timo Kokkonen wrote:
I should have probably tried this before, but when I enabled lock
debugging with this driver, I got this:
[ 663.848083] BUG: sleeping function called from invalid context at
kernel/mutex.c:269
[ 663.856262] in_atomic(): 1, irqs_disabled(): 128, pid: 889, name: lircd
[ 663.863220] 1 lock held by lircd/889:
[ 663.867065] #0: (dm_timer_lock){..}, at: [c00343e4]
omap_dm_timer_request_specific+0x18/0xc4
[ 663.876739] irq event stamp: 1770
[ 663.880218] hardirqs last enabled at (1769): [c050e4fc]
__mutex_unlock_slowpath+0xe0/0x15c
[ 663.889221] hardirqs last disabled at (1770): [c05102e8]
_raw_spin_lock_irqsave+0x1c/0x60
[ 663.898010] softirqs last enabled at (1674): [c04bcc14]
unix_create1+0x148/0x174
[ 663.906066] softirqs last disabled at (1672): [c04bcbfc]
unix_create1+0x130/0x174
[ 663.914154] [c0019a90] (unwind_backtrace+0x0/0xf8) from [c050e11c]
(mutex_lock_nested+0x28/0x328)
[ 663.923858] [c050e11c] (mutex_lock_nested+0x28/0x328) from
[c04196bc] (clk_get_sys+0x1c/0xfc)
[ 663.933197] [c04196bc] (clk_get_sys+0x1c/0xfc) from [c0034344]
(omap_dm_timer_prepare+0xe4/0x16c)
[ 663.942901] [c0034344] (omap_dm_timer_prepare+0xe4/0x16c) from
[c0034440] (omap_dm_timer_request_specific+0x74/0xc4)
[ 663.954345] [c0034440] (omap_dm_timer_request_specific+0x74/0xc4) from
[c03e7554] (lirc_rx51_open+0x50/0x154)
[ 663.965148] [c03e7554] (lirc_rx51_open+0x50/0x154) from [c0103cf0]
(chrdev_open+0x98/0x16c)
[ 663.974304] [c0103cf0] (chrdev_open+0x98/0x16c) from [c00fd608]
(do_dentry_open+0x1dc/0x264)
[ 663.983551] [c00fd608] (do_dentry_open+0x1dc/0x264) from [c00fd9ec]
(finish_open+0x44/0x5c)
[ 663.992706] [c00fd9ec] (finish_open+0x44/0x5c) from [c010e230]
(do_last.isra.21+0x598/0xba8)
[ 664.001953] [c010e230] (do_last.isra.21+0x598/0xba8) from [c010e8e8]
(path_openat+0xa8/0x47c)
[ 664.011291] [c010e8e8] (path_openat+0xa8/0x47c) from [c010efb4]
(do_filp_open+0x2c/0x80)
[ 664.020172] [c010efb4] (do_filp_open+0x2c/0x80) from [c00fe674]
(do_sys_open+0xe0/0x174)
[ 664.029052] [c00fe674] (do_sys_open+0xe0/0x174) from [c0013a60]
(ret_fast_syscall+0x0/0x3c)
It appears that in omap_dm_timer_request() there is a call to
spin_lock_irqsave() and then, as visible on the back trace,
clk_get_sys() takes a mutex lock while the spinlock is still held.
To me it appears commit 3392cdd33a0 might have introduced this problem,
but I'm not sure.
Any thoughts?
What is the actual reason for holding the omap_timer_list lock in
omap_dm_timer_request*() functions? Is it just protecting the list? I
would guess so if the name implies anything..
Thus, do we still need to hold the lock when we call the
omap_dm_timer_prepare() or can we drop the lock before calling the
prepare? We can't keep on holding a spinlock of we are about to call
clk_get().
So, how about something like this:
diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index 626ad8c..1f66cac 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -189,6 +189,7 @@ struct omap_dm_timer *omap_dm_timer_request(void)
timer-reserved = 1;
break;
}
+ spin_unlock_irqrestore(dm_timer_lock, flags);
if (timer) {
ret = omap_dm_timer_prepare(timer);
@@ -197,7 +198,6 @@ struct omap_dm_timer *omap_dm_timer_request(void)
timer = NULL;
}
}
- spin_unlock_irqrestore(dm_timer_lock, flags);
if (!timer)
pr_debug(%s: timer request failed!\n, __func__);
@@ -220,6 +220,7 @@ struct omap_dm_timer *omap_dm_timer_request_specific(int id)
break;
}
}
+ spin_unlock_irqrestore(dm_timer_lock, flags);
if (timer) {
ret = omap_dm_timer_prepare(timer);
@@ -228,7 +229,6 @@ struct omap_dm_timer *omap_dm_timer_request_specific(int id)
timer = NULL;
}
}
- spin_unlock_irqrestore(dm_timer_lock, flags);
if (!timer)
pr_debug(%s: timer%d request failed!\n, __func__, id);
--
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
[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-omap 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 timo.t.kokko...@iki.fi
---
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 plat/gpmc.h
#include plat/onenand.h
#include plat/gpmc-smc91x.h
+#include plat/omap-pm.h
#include mach/board-rx51.h
@@ -46,6 +47,10 @@
#include ../drivers/staging/iio/light/tsl2563.h
#include linux/lis3lv02d.h
+#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-omap 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 timo.t.kokko...@iki.fi
---
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 linux/module.h
+#include linux/interrupt.h
+#include linux/uaccess.h
+#include linux/platform_device.h
+#include linux/sched.h
+#include linux/wait.h
+
+#include plat/dmtimer.h
+#include plat/clock.h
+#include plat/omap-pm.h
+
+#include media/lirc.h
+#include media/lirc_dev.h
+#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
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 timo.t.kokko...@iki.fi
---
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 plat/gpmc.h
#include plat/onenand.h
#include plat/gpmc-smc91x.h
+#include plat/omap-pm.h
#include mach/board-rx51.h
@@ -46,6 +47,10 @@
#include ../drivers/staging/iio/light/tsl2563.h
#include linux/lis3lv02d.h
+#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-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] twl4030_wdt: Disable watchdog while probing
Hi Wim, On Thu, 2010-05-20 at 09:46 +0200, ext Wim Van Sebroeck wrote: = bottom line: this patch adds the correct default behaviour and should not be changed. Feel free to add the module-parameter to keep the watchog running at start. Thanks for the detailed explanation. I was worried about the kernel and the early user space booting being in the grey area where errors might occurr which can cause the device (I'm talking about embedded devices here) to go into failed state without watchdog being able to reboot the machinge. But as you said, this can be fixed by adding additional module parameters that override this default behavior, so I have no further objections for this patch. Acked-By: Timo Kokkonen timo.t.kokko...@nokia.com -- 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
Re: [PATCH] twl4030_wdt: Disable watchdog while probing
Hi,
On Mon, 2010-05-17 at 13:12 +0200, Palande Ameya (Nokia-D/Helsinki)
wrote:
If we are not able to register then it is better to have watchdog in disabled
state than noticing a system reboot.
Signed-off-by: Ameya Palande ameya.pala...@nokia.com
---
drivers/watchdog/twl4030_wdt.c |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/watchdog/twl4030_wdt.c b/drivers/watchdog/twl4030_wdt.c
index dcabe77..b5045ca 100644
--- a/drivers/watchdog/twl4030_wdt.c
+++ b/drivers/watchdog/twl4030_wdt.c
@@ -190,6 +190,8 @@ static int __devinit twl4030_wdt_probe(struct
platform_device *pdev)
twl4030_wdt_dev = pdev;
+ twl4030_wdt_disable(wdt);
+
ret = misc_register(wdt-miscdev);
if (ret) {
dev_err(wdt-miscdev.parent,
It seems that in this patch, you disable the watchdog regardless whether
the registration fails or not. I wonder if this is a good thing to do.
What if the bootloader have already enabled the watchdog and then user
space booting fails for some reason before any process is able to open
and re-enable the watchdog? We could end up with a hung system that does
not have the watchdog enabled. Maybe we should disable the watchdog only
in case the registration actually fails?
And even in that case, do we want to disable the watchdog? What are the
situations where watchdog registration can fail? Is there a possibility
that we would like to leave watchdog running in such case and cause a
reboot (say, something bad happened before the watchdog module was
loaded and the system ran out of memory, which causes watchdog
registration to fail)?
Maybe there is something I've missed, but this kind of questions came in
my mind..
-Timo
--
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
[PATCH 1/1] Export dmtimer functions
Make the dmtimer function symbols available so modules can take use of
them.
Signed-off-by: Timo Kokkonen timo.t.kokko...@nokia.com
---
arch/arm/plat-omap/dmtimer.c | 26 ++
1 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index ed397f0..a05205c 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -33,6 +33,7 @@
#include linux/clk.h
#include linux/delay.h
#include linux/io.h
+#include linux/module.h
#include mach/hardware.h
#include mach/dmtimer.h
#include mach/irqs.h
@@ -360,6 +361,7 @@ struct omap_dm_timer *omap_dm_timer_request(void)
return timer;
}
+EXPORT_SYMBOL_GPL(omap_dm_timer_request);
struct omap_dm_timer *omap_dm_timer_request_specific(int id)
{
@@ -383,6 +385,7 @@ struct omap_dm_timer *omap_dm_timer_request_specific(int id)
return timer;
}
+EXPORT_SYMBOL_GPL(omap_dm_timer_request_specific);
void omap_dm_timer_free(struct omap_dm_timer *timer)
{
@@ -393,6 +396,7 @@ void omap_dm_timer_free(struct omap_dm_timer *timer)
WARN_ON(!timer-reserved);
timer-reserved = 0;
}
+EXPORT_SYMBOL_GPL(omap_dm_timer_free);
void omap_dm_timer_enable(struct omap_dm_timer *timer)
{
@@ -404,6 +408,7 @@ void omap_dm_timer_enable(struct omap_dm_timer *timer)
timer-enabled = 1;
}
+EXPORT_SYMBOL_GPL(omap_dm_timer_enable);
void omap_dm_timer_disable(struct omap_dm_timer *timer)
{
@@ -415,11 +420,13 @@ void omap_dm_timer_disable(struct omap_dm_timer *timer)
timer-enabled = 0;
}
+EXPORT_SYMBOL_GPL(omap_dm_timer_disable);
int omap_dm_timer_get_irq(struct omap_dm_timer *timer)
{
return timer-irq;
}
+EXPORT_SYMBOL_GPL(omap_dm_timer_get_irq);
#if defined(CONFIG_ARCH_OMAP1)
@@ -450,6 +457,7 @@ __u32 omap_dm_timer_modify_idlect_mask(__u32 inputmask)
return inputmask;
}
+EXPORT_SYMBOL_GPL(omap_dm_timer_modify_idlect_mask);
#elif defined(CONFIG_ARCH_OMAP2) || defined (CONFIG_ARCH_OMAP3)
@@ -457,6 +465,7 @@ struct clk *omap_dm_timer_get_fclk(struct omap_dm_timer
*timer)
{
return timer-fclk;
}
+EXPORT_SYMBOL_GPL(omap_dm_timer_get_fclk);
__u32 omap_dm_timer_modify_idlect_mask(__u32 inputmask)
{
@@ -464,6 +473,7 @@ __u32 omap_dm_timer_modify_idlect_mask(__u32 inputmask)
return 0;
}
+EXPORT_SYMBOL_GPL(omap_dm_timer_modify_idlect_mask);
#endif
@@ -471,6 +481,7 @@ void omap_dm_timer_trigger(struct omap_dm_timer *timer)
{
omap_dm_timer_write_reg(timer, OMAP_TIMER_TRIGGER_REG, 0);
}
+EXPORT_SYMBOL_GPL(omap_dm_timer_trigger);
void omap_dm_timer_start(struct omap_dm_timer *timer)
{
@@ -482,6 +493,7 @@ void omap_dm_timer_start(struct omap_dm_timer *timer)
omap_dm_timer_write_reg(timer, OMAP_TIMER_CTRL_REG, l);
}
}
+EXPORT_SYMBOL_GPL(omap_dm_timer_start);
void omap_dm_timer_stop(struct omap_dm_timer *timer)
{
@@ -493,6 +505,7 @@ void omap_dm_timer_stop(struct omap_dm_timer *timer)
omap_dm_timer_write_reg(timer, OMAP_TIMER_CTRL_REG, l);
}
}
+EXPORT_SYMBOL_GPL(omap_dm_timer_stop);
#ifdef CONFIG_ARCH_OMAP1
@@ -505,6 +518,7 @@ void omap_dm_timer_set_source(struct omap_dm_timer *timer,
int source)
l |= source n;
omap_writel(l, MOD_CONF_CTRL_1);
}
+EXPORT_SYMBOL_GPL(omap_dm_timer_set_source);
#else
@@ -521,6 +535,7 @@ void omap_dm_timer_set_source(struct omap_dm_timer *timer,
int source)
* cause an abort. */
__delay(15);
}
+EXPORT_SYMBOL_GPL(omap_dm_timer_set_source);
#endif
@@ -539,6 +554,7 @@ void omap_dm_timer_set_load(struct omap_dm_timer *timer,
int autoreload,
omap_dm_timer_write_reg(timer, OMAP_TIMER_TRIGGER_REG, 0);
}
+EXPORT_SYMBOL_GPL(omap_dm_timer_set_load);
/* Optimized set_load which removes costly spin wait in timer_start */
void omap_dm_timer_set_load_start(struct omap_dm_timer *timer, int autoreload,
@@ -558,6 +574,7 @@ void omap_dm_timer_set_load_start(struct omap_dm_timer
*timer, int autoreload,
omap_dm_timer_write_reg(timer, OMAP_TIMER_COUNTER_REG, load);
omap_dm_timer_write_reg(timer, OMAP_TIMER_CTRL_REG, l);
}
+EXPORT_SYMBOL_GPL(omap_dm_timer_set_load_start);
void omap_dm_timer_set_match(struct omap_dm_timer *timer, int enable,
unsigned int match)
@@ -572,6 +589,7 @@ void omap_dm_timer_set_match(struct omap_dm_timer *timer,
int enable,
omap_dm_timer_write_reg(timer, OMAP_TIMER_CTRL_REG, l);
omap_dm_timer_write_reg(timer, OMAP_TIMER_MATCH_REG, match);
}
+EXPORT_SYMBOL_GPL(omap_dm_timer_set_match);
void omap_dm_timer_set_pwm(struct omap_dm_timer *timer, int def_on,
int toggle, int trigger)
@@ -588,6 +606,7 @@ void omap_dm_timer_set_pwm(struct omap_dm_timer *timer, int
def_on,
l |= trigger 10;
omap_dm_timer_write_reg(timer, OMAP_TIMER_CTRL_REG, l);
}
+EXPORT_SYMBOL_GPL(omap_dm_timer_set_pwm);
void
