Re: [PATCHv2 1/2] media: rc: Introduce RX51 IR transmitter driver
On Fri, Aug 10, 2012 at 01:16:36PM +0300, Timo Kokkonen wrote:
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,
+
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
[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)
+{
+
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
