Re: [PATCH 02/18] lirc serial port receiver/transmitter device driver

Previous thread: Re: [PATCH 1/2] audit: fix NUL handling in untrusted strings by Miloslav on Thursday, September 11, 2008 - 1:03 pm. (1 message)

Next thread: blk: queue cleanup at device shutdown by Geoff Levand on Thursday, September 11, 2008 - 1:17 pm. (2 messages)
From: Stefan Bauer
Date: Thursday, September 11, 2008 - 12:49 pm

I just want to thank you very much for your work and give you my Tested-By. 
Todays git (b2e9c18a32423310a309f94ea5a659c4bb264378) works well here with 
lirc-0.8.3 userspace on a Pentium 3/i815-system.

But I've had a section mismatch in the lirc code, don't know if this is 
serious.

WARNING: drivers/input/lirc/lirc_serial.o(.init.text+0x11e): Section mismatch 
in reference from the function init_module() to the 
function .exit.text:lirc_serial_exit()
The function __init init_module() references
a function __exit lirc_serial_exit().
This is often seen when error handling in the init function
uses functionality in the exit path.
The fix is often to remove the __exit annotation of
lirc_serial_exit() so it may be used outside an exit section.


Regards,
Stefan
--

From: Jarod Wilson
Date: Friday, September 12, 2008 - 9:24 am

From what I've been able to ascertain, reducing the above to...

static u8 sinp(int offset)
{
        if (iommap != 0)
                /* the register is memory-mapped */
                offset <<= ioshift;

        return inb(io + offset);
}

...should be sane. inb() either calls ioport_map() and ioread8() as needed, or 
defines inb() as readb() (or __raw_readb()) on platforms where its 

I've borrowed the simple existence test from 
drivers/serial/8250.c::autoconfig(), which tries a few reads and writes from 
UART_IER. I think this is probably sufficient for verifying the port is legit.

Christoph B., you're definitely much more familiar with serial IR devices than 
I am... Is there any sort of test you can think of that we could use to try to 
verify the existence of an IR device hooked to the port? Or should we be happy 

Oh good! I haven't broken anything further w/my changes up to b2e9c18... ;) 
I've got another slew of updates to lirc_serial still pending though -- a few 
that I'm about to push, and at least one more chunk I still need to figure out 
(the hrtimers/rdtscl bits).

Continued testing would be much appreciated, since I still have no serial IR 
hardware myself (couldn't find the thingy that shipped with my Technisat card, 

Ah, I believe you're absolutely correct. Done.

-- 
Jarod Wilson
jarod@redhat.com

--

From: Janne Grunau
Date: Friday, September 12, 2008 - 5:26 pm

I hope I haven't broken anything with my lirc_dev changes. I doubt I'll 
have a change to test it before monday.

Janne
--

From: Stefan Bauer
Date: Saturday, September 13, 2008 - 1:41 am

Unfortunately, you did. Commit ea74897 (port lirc to dynamic device numbers) 
broke things.
This is what ea74897 and further (latest tested was dd13cc7) are telling me:

$ insmod drivers/input/lirc/lirc_dev.ko
$ insmod drivers/input/lirc/lirc_serial.ko
insmod: error inserting 'drivers/input/lirc/lirc_serial.ko': -1 Input/output 
error
$ dmesg | tail
lirc_dev: IR Remote Control driver registered, major 253 
lirc_serial: auto-detected active low receiver
lirc_dev: lirc_register_driver: sample_rate: 0
lirc_serial: register_chrdev failed!


There has also been a compile issue in the meanwhile, introduced with 95efa30 
(inb/outb and readb/writeb deal in u8 types), but this is gone with todays 
git pull :)


I'll keep on testing (unfortunately only at weekends),
regards, Stefan
--

From: Jarod Wilson
Date: Sunday, September 14, 2008 - 8:55 pm

I'd hope to get around to some testing myself much earlier in the weekend, but 
alas... Did just mix in a quick peed at lirc_i2c though:

...
lirc_dev: IR Remote Control driver registered, major 247 
bttv: driver version 0.9.17 loaded
bttv: using 8 buffers with 2080k (520 pages) each for capture
cx88/0: cx2388x v4l2 driver version 0.0.6 loaded
lirc_i2c: chip 0x10020 found @ 0x18 (Hauppauge IR)
lirc_dev: lirc_register_driver: sample_rate: 10

No register_chrdev failure reported, everything looks the same as prior to the 
dynamic dev num change (save the dev num, of course), and I've got a 
/dev/lirc0, but I'm unable to see any IR signals (start up lircd, run irw, 
press buttons on remote).

...and I just took a quick look at lirc_i2c... The result from 
lirc_register_driver() is never checked, whereas it is in the lirc_serial case 
(which is where the register_chrdev error msg came from). Narf. So its likely 
the same failure, just not noticed (will fix lirc_i2c in a sec).

-- 
Jarod Wilson
jarod@redhat.com

--

From: Jarod Wilson
Date: Monday, September 15, 2008 - 11:20 am

So I've done a bit of work to fix up a few drivers so they properly check to 
see that lirc_register_driver() actually succeeded, and then accidentally 
stumbled onto the fix for the failure when merging some coverity-inspired 
patches from Erik Hovland. lirc_dev.c's lirc_cdev_add() function had an 
inverted kmalloc failure check. With that fixed, lirc_i2c appears to be 
behaving for me now, although I can't actually check to be 100% certain until 
I get home tonight.

...
lirc_dev: IR Remote Control driver registered, major 240 
lirc_i2c: probe 0x1a @ ivtv i2c driver #0: no
lirc_i2c: probe 0x18 @ ivtv i2c driver #0: yes
lirc_i2c: chip 0x10020 found @ 0x18 (Hauppauge IR)
lirc_dev: lirc_register_driver: sample_rate: 10
lirc_dev: driver lirc_i2c registered at minor number = 0
lirc_dev (lirc_i2c[0]): poll thread started
lirc_i2c: ir_attach: success (minor 0)
...

Fixes are in the git tree now for anyone else who wants to or is willing to 
test. Will verify myself tonight w/lirc_i2c and lirc_mceusb2 at a minimum.

-- 
Jarod Wilson
jarod@redhat.com

--

From: Jarod Wilson
Date: Monday, September 15, 2008 - 9:08 pm

Current git tree works for receiving IR signals via lirc_serial (tested by 
Janne) and via lirc_mceusb2 (tested by me). Something is still slightly 
haywire with lirc_i2c though. It registers fine, but IR signals simply aren't 
making it to lircd. Probably unrelated to the dynamic device num changes, 
looking into it.

-- 
Jarod Wilson
jarod@redhat.com

--

From: Jarod Wilson
Date: Thursday, September 18, 2008 - 7:00 am

Despite compiling fine w/o #include'ing linux/i2c-algo-bit.h and coverity 
apparently thinking its not needed, its really quite necessary for things to 
function. Restored it, and lirc_i2c now works again.

-- 
Jarod Wilson
jarod@redhat.com

--

From: Stefan Bauer
Date: Friday, September 19, 2008 - 11:05 am

Thank you for keeping me up to date, here are my test results with todays 
(36e60bd457588b2a2fd37e0c1d0652a9fe214d85) git:

- Compiles fine, except some warnings:

drivers/input/lirc/lirc_dev.h:60:
  warning: 'lirc_buffer_available' defined but not used
drivers/input/lirc/lirc_dev.h:72:
  warning: 'lirc_buffer_write' defined but not used
drivers/input/lirc/lirc_dev.h:56:
  warning: 'lirc_buffer_empty' defined but not used
drivers/input/lirc/lirc_dev.h:60:
  warning: 'lirc_buffer_available' defined but not used
drivers/input/lirc/lirc_dev.h:66:
  warning: 'lirc_buffer_read' defined but not used

- Module loads fine, with this messages:

lirc_dev: IR Remote Control driver registered, major 252 
lirc_serial: auto-detected active low receiver
lirc_dev: lirc_register_driver: sample_rate: 0

- But lircd crashes immediately every time irexec/irw tries to connect:

BUG: unable to handle kernel NULL pointer dereference at 0000000c
IP: [<cabc9006>] :lirc_dev:lirc_buffer_clear+0x6/0x17
*pde = 00000000 
Oops: 0002 [#1] 
Modules linked in: lirc_serial lirc_dev ves1820 arc4 ecb crypto_blkcipher 
cryptomgr crypto_algapi rt61pci rt2x00pci rt2x00lib led_class dvb_ttpci 
saa7146_vv saa7146 videobuf_dma_sg videobuf_core mac80211 videodev 
v4l1_compat ttpci_eeprom ehci_hcd uhci_hcd cfg80211 eeprom_93cx6

Pid: 2501, comm: lircd Not tainted (2.6.27-rc5 #7)
EIP: 0060:[<cabc9006>] EFLAGS: 00010046 CPU: 0
EIP is at lirc_buffer_clear+0x6/0x17 [lirc_dev]
EAX: 00000000 EBX: c7825cc0 ECX: cabc94f1 EDX: 00000246
ESI: fffffff0 EDI: c726537c EBP: c6bdf000 ESP: c6b73eac
 DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 0068
Process lircd (pid: 2501, ti=c6b72000 task=c72ef7a0 task.ti=c6b72000)
Stack: cabc958f 00000000 c7825d3c c014e15c 00000000 c6bdf000 c726537c 00000000 
       c014e09b c014b052 c78c2900 c759fd80 c6b73f14 c6bdf000 c6b73f14 00000003 
       c014b156 c6bdf000 00000000 00000000 00000000 c01544ae 00000002 00000026 
Call Trace:
 [<cabc958f>] irctl_open+0x9e/0x131 [lirc_dev]
 [<c014e15c>] ...
From: Janne Grunau
Date: Friday, September 19, 2008 - 11:26 am

not necessary, it is
commit bc2039245c6390bd414a61c9395604c4155a58c6
Author: Janne Grunau <j@jannau.net>
Date:   Thu Sep 18 20:49:19 2008 +0200

    reimplement lirc_buffer as kfifo wrapper

Janne
--

From: Stefan Bauer
Date: Friday, September 19, 2008 - 11:53 am

Thank you for pointing me, but bc2039245c6390bd414a61c9395604c4155a58c6~1 
(lirc_streamzap can use lirc_buffer_clear) also crashes lircd when irexec is 
started.

BUG: unable to handle kernel NULL pointer dereference at 00000000
IP: [<c01118ce>] try_to_wake_up+0x16/0x7c
*pde = 00000000 
Oops: 0000 [#1] 
Modules linked in: lirc_serial lirc_dev ves1820 arc4 ecb crypto_blkcipher 
cryptomgr crypto_algapi rt61pci rt2x00pci rt2x00lib led_class dvb_ttpci 
saa7146_vv saa7146 mac80211 videobuf_dma_sg ehci_hcd uhci_hcd videobuf_core 
videodev v4l1_compat ttpci_eeprom cfg80211 eeprom_93cx6

Pid: 2939, comm: lircd Not tainted (2.6.27-rc5 #7)
EIP: 0060:[<c01118ce>] EFLAGS: 00010046 CPU: 0
EIP is at try_to_wake_up+0x16/0x7c
EAX: c04091c0 EBX: 0000000f ECX: 00000000 EDX: c3e85e98
ESI: c04091c0 EDI: 00000000 EBP: 00000000 ESP: c3e85e98
 DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 0068
Process lircd (pid: 2939, ti=c3e84000 task=c3e866c0 task.ti=c3e84000)
Stack: 00000246 c6a16000 00000000 c691905c c6bed400 cadc05b3 00000000 c6a1607c 
       c014e15c 00000000 c6bed400 c691905c 00000000 c014e09b c014b052 c78c2a00 
       c774c180 c3e85f14 c6bed400 c3e85f14 00000003 c014b156 c6bed400 00000000 
Call Trace:
 [<cadc05b3>] irctl_open+0xdd/0x13c [lirc_dev]
 [<c014e15c>] chrdev_open+0xc1/0xf6
 [<c014e09b>] chrdev_open+0x0/0xf6
 [<c014b052>] __dentry_open+0xf2/0x1da
 [<c014b156>] nameidata_to_filp+0x1c/0x2c
 [<c01544ae>] do_filp_open+0x28b/0x545
 [<c015bc37>] alloc_fd+0x46/0xad
 [<c014ae8d>] do_sys_open+0x42/0xb6
 [<c014af45>] sys_open+0x1e/0x23
 [<c010297d>] sysenter_do_call+0x12/0x25
 [<c02f0000>] ioctl_standard_iw_point+0x12d/0x200
 =======================
Code: 00 00 00 00 5b 5e 5f c3 53 9c 5b fa e8 bb ff ff ff 53 9d 5b c3 55 57 89 
c7 56 53 89 d3 83 ec 04 89 e2 31 ed e8 3a fd ff ff 89 c6 <8b> 07 85 d8 74 41 
83 7f 48 00 75 25 31 c0 66 bd 01 00 e8 ba 40 
EIP: [<c01118ce>] try_to_wake_up+0x16/0x7c SS:ESP 0068:c3e85e98
---[ end trace 344ec27be5d90ccf ]---


Regards,
Stefan
--

From: Janne Grunau
Date: Friday, September 19, 2008 - 12:24 pm

ok, then it is not the obvious thing, I'll test in a couple of minutes

Janne
--

From: Janne Grunau
Date: Friday, September 19, 2008 - 5:10 pm

yes, a different bug. both fixed in 
git://git.jannau.net/linux-2.6-lirc.git

but lirc serial still doesn't work after 
bc2039245c6390bd414a61c9395604c4155a58c6. I'll look at it tomorrow.

I have also good news: irrecord + lirc_serial is working with bc203924^. 
Not sure if something changed or if it was PEBCAK.

Janne
--

From: Stefan Bauer
Date: Friday, September 26, 2008 - 12:42 pm

Hi,


just to tell you that I'm still there... :) Todays git 
(a842c8095aad7b71ffcd1e426185e53661d758c7) is also giving me this at first 
access to the device by lircd:

------------[ cut here ]------------
WARNING: at drivers/input/lirc/lirc_dev.h:37
lirc_buffer_clear+0x2c/0x30 [lirc_dev]()
calling lirc_buffer_clear on an uninitialized lirc_buffer
Modules linked in: lirc_serial lirc_dev ves1820 arc4 ecb crypto_blkcipher 
cryptomgr crypto_algapi rt61pci rt2x00pci rt2x00lib led_class dvb_ttpci 
saa7146_vv saa7146 videobuf_dma_sg mac80211 videobuf_core videodev ehci_hcd 
uhci_hcd ttpci_eeprom cfg80211 eeprom_93cx6 [last unloaded: lirc_dev]
Pid: 5144, comm: lircd Not tainted 2.6.27-rc5 #7
 [<c01151ce>] warn_slowpath+0x61/0x84
 [<c015971f>] __d_lookup+0x8b/0xbf
 [<c0151f26>] do_lookup+0x40/0x92
 [<c0158f6d>] dput+0x15/0x8d
 [<c0153533>] __link_path_walk+0x7ad/0x8b4
 [<c0153533>] __link_path_walk+0x7ad/0x8b4
 [<c01de0c1>] kobject_get+0xf/0x13
 [<c014dfbb>] cdev_get+0x1b/0x2d
 [<c014dfd4>] exact_lock+0x7/0xd
 [<c0242154>] kobj_lookup+0xda/0x104
 [<ca9fa15a>] lirc_buffer_clear+0x2c/0x30 [lirc_dev]
 [<ca9fa4f4>] lirc_dev_fop_open+0x9e/0x135 [lirc_dev]
 [<c014e15c>] chrdev_open+0xc1/0xf6
 [<c014e09b>] chrdev_open+0x0/0xf6
 [<c014b052>] __dentry_open+0xf2/0x1da
 [<c014b156>] nameidata_to_filp+0x1c/0x2c
 [<c01544ae>] do_filp_open+0x28b/0x545
 [<c015bc37>] alloc_fd+0x46/0xad
 [<c014ae8d>] do_sys_open+0x42/0xb6
 [<c014af45>] sys_open+0x1e/0x23
 [<c010297d>] sysenter_do_call+0x12/0x25
 [<c02f0000>] ioctl_standard_iw_point+0x12d/0x200
 =======================
---[ end trace 6c8eb5cbb580deaf ]---

Regards,
Stefan
--

From: Jarod Wilson
Date: Friday, September 19, 2008 - 11:54 am

Hm... Two things... First, I neglected to pull some of Janne's later changes 
in the same area until just a bit ago, so perhaps this has already been 
remedied, because second, lirc_i2c is working as expected w/current head 
(98242cbdca...)


-- 
Jarod Wilson
jarod@redhat.com

--

From: Stefan Bauer
Date: Friday, September 19, 2008 - 12:12 pm

98242cbdca behaves exactly as 36e60bd4575.

-Stefan
--

From: Christoph Bartelmus
Date: Saturday, September 13, 2008 - 12:09 am

Hi Jarod,


No, I can't think of anything else. OTOH there have never been real  
problems with this for the past 10 years...

Christoph
--

Previous thread: Re: [PATCH 1/2] audit: fix NUL handling in untrusted strings by Miloslav on Thursday, September 11, 2008 - 1:03 pm. (1 message)

Next thread: blk: queue cleanup at device shutdown by Geoff Levand on Thursday, September 11, 2008 - 1:17 pm. (2 messages)