RE: [PATCH/RFC v2.1 0/2] Mem-to-mem device framework

2009-12-28 Thread Pawel Osciak
Hello Hans,


On Wednesday 23 December 2009 16:06:18 Hans Verkuil wrote:
 Thank you for working on this! It's much appreciated. Now I've noticed that
 patches regarding memory-to-memory and memory pool tend to get very few 
 comments.
 I suspect that the main reason is that these are SoC-specific features that do
 not occur in consumer-type products. So most v4l developers do not have the
 interest and motivation (and time!) to look into this.

Thank you very much for your response. We were a bit surprised with the lack of
responses as there seemed to be a good number of people interested in this area.

I'm hoping that everybody interested would take a look at the test device posted
along with the patches. It's virtual, no specific hardware required, but it
demonstrates the concepts behind the framework, including transactions.

 One thing that I am missing is a high-level overview of what we want. 
 Currently
 there are patches/RFCs floating around for memory-to-memory support, 
 multiplanar
 support and memory-pool support.

 What I would like to see is a RFC that ties this all together from the point 
 of
 view of the public API. I.e. what are the requirements? Possibly solutions? 
 Open
 questions? Forget about how to implement it for the moment, that will follow
 from the chosen solutions.

Yes, that's true, sorry about that. We've been so into it after the memory pool
discussion and the V4L2 mini-summit that I neglected describing the big picture
behind this.

So to give a more high-level description, from the point of view of applications
and the V4L2 API:

---
Requirements:
---
(Some of the following were first posted by Laurent in:
http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/10204).

1. Support for devices that take input data in a source buffer, take a separate
destination buffer, process the source data and put it in the destination 
buffer.

2. Allow sharing buffers between devices, effectively chaining them to form
video pipelines. An example of this could be a video decoder, fed with video
stream which returns raw frames, which then have to be postprocessed by another
device and displayed. This is the main scenario we need to have for our S3C/S5P
series SoCs. Of course, we'd like zero-copy.

3. Allow using more than one buffer by the device at the same time. This is not
supported by videobuffer (e.g. we have to choose on which buffer we'd like
to sleep, and we do not always know that). This is not really a requirement
from the V4L2 API point of view, but has direct influence on how poll() and
blocking I/O works.

4. Multiplanar buffers. Our devices require them (see the RFC for more details:
http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/11212).

5. Solve problems with cache coherency on non-x86 architectures, especially in
videobuf for OUTPUT buffers. We need to flush the cache before starting the
transaction.

6. Reduce buffer queuing latency, e.g.: move operations such as locking, out
of qbuf.
Applications would like to queue a buffer and be able to fire up the device
as fast as possible.

7. Large buffer allocations, buffer preallocation, etc.


---
Solutions:
---
1. After a detailed discussion, we agreed in:
http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/10668,
that we'd like the application to be able to queue/dequeue both OUTPUT (as 
source)
and CAPTURE (as destination) buffers on one video node. Activating the device
(after streamon) would take effect only if there are both types of buffers 
available. The application would put source data into OUTPUT buffers and expect
to find it processed in dequeued CAPTURE buffers. Addressed by mem2mem 
framework.

2. I don't see anything to do here from the API's point of view. The application
would open two video nodes, e.g. video decoder and video postprocessor and queue
buffers dequeued from decoder on the postprocessor. To get the best performance,
this requires the buffers to be marked as non cached somehow to avoid unneeded
cache syncs.

3. Mem2mem addresses this partially by adding a transaction concept. It's
not bullet-proof though, as it assumes the buffers will be returned in the same
order as passed. Some videobuffer limitations will have to be addressed here.

4. See my RFC. Patches in progress. 

5. We have narrowed it down to an additional sync() before the operation
(i.e. in qbuf), but more issues may exist here. I have already added sync()
support for qbuf with minimal changes to videobuf and will be posting the
proposal soon. This also requires identifying the direction of the sync, but
we have found a way to do this without adding anything new (videobuf flags
are enough).

6. Later. We haven't done anything in this field.

7. We use our own allocator (see
http://thread.gmane.org/gmane.linux.ports.arm.kernel/56879), but we have a new
concept for that which we'd like to discuss separately later. 


 Note 

[patch] cx25821: fix double unlock in medusa_video_init()

2009-12-28 Thread Dan Carpenter
medusa_set_videostandard() takes the lock but it always drops it before
returning.

This was found with a static checker and compile tested only.  :/

Signed-off-by: Dan Carpenter erro...@gmail.com

--- orig/drivers/staging/cx25821/cx25821-medusa-video.c 2009-12-28 
09:13:01.0 +0200
+++ devel/drivers/staging/cx25821/cx25821-medusa-video.c2009-12-28 
09:13:55.0 +0200
@@ -860,10 +860,8 @@ int medusa_video_init(struct cx25821_dev
 
ret_val = medusa_set_videostandard(dev);
 
-   if (ret_val  0) {
-   mutex_unlock(dev-lock);
+   if (ret_val  0)
return -EINVAL;
-   }
 
return 1;
 }
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[patch] fix weird array index in zl10036.c

2009-12-28 Thread Dan Carpenter
I was initially concerned about the weird array index (the 2 bumps
into the next row of the array).  Matthias Schwarzott look at the
datasheet and it turns out it should be zl10036_init_tab[1][0] |= 0x01;

Signed-off-by: Dan Carpenter erro...@gmail.com

--- orig/drivers/media/dvb/frontends/zl10036.c  2009-12-28 19:04:51.0 
+0200
+++ devel/drivers/media/dvb/frontends/zl10036.c 2009-12-28 19:07:18.0 
+0200
@@ -411,7 +411,7 @@ static int zl10036_init_regs(struct zl10
state-bf = 0xff;
 
if (!state-config-rf_loop_enable)
-   zl10036_init_tab[1][2] |= 0x01;
+   zl10036_init_tab[1][0] |= 0x01;
 
deb_info(%s\n, __func__);
 
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Nova-T 500 Dual DVB-T

2009-12-28 Thread Yves
So, I tried using kernel 2.6.32.2 from http://www.kernel.org. I compiled 
it OK.

I still get an error with v4l-dvb :

[myt...@localhost v4l-dvb]$ make -j 
5

make -C 
/home/mythtv/v4l-dvb/v4l 

make[1]: entrant dans le répertoire « /home/mythtv/v4l-dvb/v4l 
»
No version yet, using 
2.6.32.2   

make[1]: quittant le répertoire « /home/mythtv/v4l-dvb/v4l 
» 

make[1]: entrant dans le répertoire « /home/mythtv/v4l-dvb/v4l 
»
scripts/make_makefile.pl 

./scripts/make_kconfig.pl /lib/modules/2.6.32.2/build 
/lib/modules/2.6.32.2/source   

Updating/Creating 
.config

Preparing to compile for kernel version 
2.6.32   

Preparing to compile for kernel version 
2.6.32   

Created default (all yes) .config 
file   

./scripts/make_myconfig.pl   

make[1]: quittant le répertoire « /home/mythtv/v4l-dvb/v4l 
» 

make[1]: entrant dans le répertoire « /home/mythtv/v4l-dvb/v4l 
»
perl scripts/make_config_compat.pl /lib/modules/2.6.32.2/source 
./.myconfig ./config-compat.h   
ln -sf . 
oss 

creating symbolic 
links...   

make -C firmware 
prep

make[2]: Entering directory 
`/home/mythtv/v4l-dvb/v4l/firmware'  

make[2]: Leaving directory 
`/home/mythtv/v4l-dvb/v4l/firmware'   

make -C 
firmware 

make[2]: Entering directory 
`/home/mythtv/v4l-dvb/v4l/firmware'  

 CC  
ihex2fw

Generating 
vicam/firmware.fw 

Generating 
dabusb/firmware.fw

Generating 
ttusb-budget/dspbootcode.bin  

Generating 
dabusb/bitstream.bin  

Generating 
cpia2/stv0672_vp4.bin 

Generating 
av7110/bootcode.bin   

make[2]: Leaving directory 
`/home/mythtv/v4l-dvb/v4l/firmware'   

Kernel build directory is 
/lib/modules/2.6.32.2/build

make -C /lib/modules/2.6.32.2/build SUBDIRS=/home/mythtv/v4l-dvb/v4l  
modules   
make[2]: Entering directory 
`/home/mythtv/Téléchargements/linux-2.6.32.2'

 CC [M]  
/home/mythtv/v4l-dvb/v4l/tuner-xc2028.o

Re: Mantis driver on TechniSat CableStar HD 2

2009-12-28 Thread Wolfgang Denk
Dear Manu Abraham,

In message 1a297b360912271420g154e34a4gab4382de3a316...@mail.gmail.com you 
wrote:
 
 If you can successfully tune and get a valid TS from the DVR device,
 you can rule out issues with the card and the driver.
 You can verify the functionality of the hardware and driver with the
 command line applications from the dvb-apps repository.

Thanks, this got me started. The card and the driver is working fine.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
The first 90% of a project takes 90% of the time, the last 10%  takes
the other 90% of the time.
--
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


[cron job] v4l-dvb daily build 2.6.22 and up: ERRORS, 2.6.16-2.6.21: ERRORS

2009-12-28 Thread Hans Verkuil
This message is generated daily by a cron job that builds v4l-dvb for
the kernels and architectures in the list below.

Results of the daily build of v4l-dvb:

date:Mon Dec 28 19:00:03 CET 2009
path:http://www.linuxtv.org/hg/v4l-dvb
changeset:   13848:75c97b2d1a2a
gcc version: gcc (GCC) 4.3.1
hardware:x86_64
host os: 2.6.26

linux-2.6.30-armv5: OK
linux-2.6.31-armv5: OK
linux-2.6.32-armv5: OK
linux-2.6.32-armv5-davinci: OK
linux-2.6.30-armv5-ixp: OK
linux-2.6.31-armv5-ixp: OK
linux-2.6.32-armv5-ixp: OK
linux-2.6.30-armv5-omap2: OK
linux-2.6.31-armv5-omap2: OK
linux-2.6.32-armv5-omap2: OK
linux-2.6.22.19-i686: ERRORS
linux-2.6.23.12-i686: ERRORS
linux-2.6.24.7-i686: ERRORS
linux-2.6.25.11-i686: ERRORS
linux-2.6.26-i686: WARNINGS
linux-2.6.27-i686: ERRORS
linux-2.6.28-i686: ERRORS
linux-2.6.29.1-i686: ERRORS
linux-2.6.30-i686: ERRORS
linux-2.6.31-i686: ERRORS
linux-2.6.32-i686: ERRORS
linux-2.6.30-m32r: OK
linux-2.6.31-m32r: OK
linux-2.6.32-m32r: OK
linux-2.6.30-mips: WARNINGS
linux-2.6.31-mips: OK
linux-2.6.32-mips: OK
linux-2.6.30-powerpc64: WARNINGS
linux-2.6.31-powerpc64: OK
linux-2.6.32-powerpc64: WARNINGS
linux-2.6.22.19-x86_64: ERRORS
linux-2.6.23.12-x86_64: ERRORS
linux-2.6.24.7-x86_64: ERRORS
linux-2.6.25.11-x86_64: ERRORS
linux-2.6.26-x86_64: WARNINGS
linux-2.6.27-x86_64: OK
linux-2.6.28-x86_64: OK
linux-2.6.29.1-x86_64: WARNINGS
linux-2.6.30-x86_64: OK
linux-2.6.31-x86_64: WARNINGS
linux-2.6.32-x86_64: WARNINGS
spec: OK
sparse (linux-2.6.32): ERRORS
linux-2.6.16.61-i686: ERRORS
linux-2.6.17.14-i686: ERRORS
linux-2.6.18.8-i686: ERRORS
linux-2.6.19.5-i686: ERRORS
linux-2.6.20.21-i686: ERRORS
linux-2.6.21.7-i686: ERRORS
linux-2.6.16.61-x86_64: ERRORS
linux-2.6.17.14-x86_64: ERRORS
linux-2.6.18.8-x86_64: ERRORS
linux-2.6.19.5-x86_64: ERRORS
linux-2.6.20.21-x86_64: ERRORS
linux-2.6.21.7-x86_64: ERRORS

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Monday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Monday.tar.bz2

The V4L-DVB specification from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/media.html
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Which modules for the VP-2033? Where is the module mantis.ko?

2009-12-28 Thread Aljaž Prusnik
On pon, 2009-12-28 at 02:23 +0400, Manu Abraham wrote:
 Can you please do a lspci -vn for the Mantis card you have ? Also try
 loading the mantis.ko module with verbose=5 module parameter, to get
 more debug information.

$ lspci -vn

03:07.0 0480: 1822:4e35 (rev 01)
Subsystem: 1ae4:0002
Flags: bus master, medium devsel, latency 32, IRQ 21
Memory at fdcff000 (32-bit, prefetchable) [size=4K]
Kernel driver in use: Mantis


$ modprobe -v mantis  verbose=5

insmod /lib/modules/2.6.33-rc1/kernel/drivers/media/dvb/mantis/mantis_core.ko 
insmod /lib/modules/2.6.33-rc1/kernel/drivers/media/dvb/mantis/mantis.ko
verbose=5

$ dmesg

[ 2371.762511] Mantis :03:07.0: PCI INT A disabled
[ 2462.703393] Mantis :03:07.0: PCI INT A - GSI 21 (level, low) -
IRQ 21
[ 2462.704761] DVB: registering new adapter (Mantis DVB adapter)
[ 2463.563234] DVB: registering adapter 0 frontend 0 (Philips TDA10023
DVB-C)...
[ 2522.842548] Mantis :03:07.0: PCI INT A disabled
[ 2534.495759] found a VP-2040 PCI DVB-C device on (03:07.0),
[ 2534.495771] Mantis :03:07.0: PCI INT A - GSI 21 (level, low) -
IRQ 21
[ 2534.495796] Mantis Rev 1 [1ae4:0002], irq: 21, latency: 32
[ 2534.495799] memory: 0x0, mmio: 0xc900128cc000
[ 2534.495818] mantis_stream_control (0): Set stream to HIF
[ 2534.495841] mantis_i2c_init (0): Initializing I2C ..
[ 2534.495845] mantis_i2c_init (0): Disabling I2C interrupt
[ 2534.495849] mantis_i2c_xfer (0): Messages:2
[ 2534.495852] mantis_i2c_write: Address=[0x50] W[ 08 ]
[ 2534.496243] mantis_i2c_read:  Address=[0x50] R[ 00 08 c9 d0
02 09 ]
[ 2534.497049] MAC Address=[00:08:c9:d0:02:09]
[ 2534.497051] mantis_dma_init (0): Mantis DMA init
[ 2534.497071] mantis_alloc_buffers (0): DMA=0x3d67
cpu=0x88003d67 size=65536
[ 2534.497073] mantis_alloc_buffers (0): RISC=0x3b731000
cpu=0x88003b731000 size=1000
[ 2534.497076] mantis_calc_lines (0): Mantis RISC block bytes=[4096],
line bytes=[2048], line count=[32]
[ 2534.497077] mantis_dvb_init (0): dvb_register_adapter
[ 2534.497079] DVB: registering new adapter (Mantis DVB adapter)
[ 2534.497081] mantis_dvb_init (0): dvb_dmx_init
[ 2534.497166] mantis_dvb_init (0): dvb_dmxdev_init
[ 2534.497269] mantis_frontend_power (0): Power ON
[ 2534.497271] gpio_set_bits (0): Set Bit 12 to 1
[ 2534.497274] gpio_set_bits (0): GPIO Value 3000
[ 2534.598099] gpio_set_bits (0): Set Bit 12 to 1
[ 2534.598111] gpio_set_bits (0): GPIO Value 3000
[ 2534.699066] mantis_frontend_soft_reset (0): Frontend RESET
[ 2534.699077] gpio_set_bits (0): Set Bit 13 to 0
[ 2534.699085] gpio_set_bits (0): GPIO Value 1000
[ 2534.800077] gpio_set_bits (0): Set Bit 13 to 0
[ 2534.800087] gpio_set_bits (0): GPIO Value 1000
[ 2534.901079] gpio_set_bits (0): Set Bit 13 to 1
[ 2534.901089] gpio_set_bits (0): GPIO Value 3000
[ 2535.002089] gpio_set_bits (0): Set Bit 13 to 1
[ 2535.002099] gpio_set_bits (0): GPIO Value 3000
[ 2535.354084] vp2040_frontend_init (0): Probing for CU1216 (DVB-C)
[ 2535.354096] mantis_i2c_xfer (0): Messages:2
[ 2535.354103] mantis_i2c_write: Address=[0x50] W[ ff ]
[ 2535.354328] mantis_i2c_read:  Address=[0x50] R[ e4 ]
[ 2535.354555] mantis_i2c_xfer (0): Messages:2
[ 2535.354560] mantis_i2c_write: Address=[0x0c] W[ 1a ]
[ 2535.354781] mantis_i2c_read:  Address=[0x0c] R[ 7d ]
[ 2535.355034] mantis_i2c_xfer (0): Messages:2
[ 2535.355039] mantis_i2c_write: Address=[0x50] W[ ff ]
[ 2535.355257] mantis_i2c_read:  Address=[0x50] R[ e4 ]
[ 2535.355482] mantis_i2c_xfer (0): Messages:1
[ 2535.355486] mantis_i2c_write: Address=[0x0c] W[ 00 33 ]
[ 2535.355819] mantis_i2c_xfer (0): Messages:2
[ 2535.355823] mantis_i2c_write: Address=[0x0c] W[ 1a ]
[ 2535.356055] mantis_i2c_read:  Address=[0x0c] R[ 7d ]
[ 2535.356279] vp2040_frontend_init (0): found Philips CU1216 DVB-C
frontend (TDA10023) @ 0x0c
[ 2535.356285] vp2040_frontend_init (0): Mantis DVB-C Philips CU1216
frontend attach success
[ 2535.356291] vp2040_frontend_init (0): Done!
[ 2535.356298] DVB: registering adapter 0 frontend 0 (Philips TDA10023
DVB-C)...
[ 2535.356573] mantis_uart_init (0): Initializing UART @ 9600bps
parity:NONE
[ 2535.356602] mantis_uart_read (0): Reading ... 3f
[ 2535.356609] mantis_uart_work (0): UART BUF:0 3f 
[ 2535.356613] 
[ 2535.356619] mantis_uart_init (0): UART succesfully initialized




--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Which modules for the VP-2033? Where is the module mantis.ko?

2009-12-28 Thread Aljaž Prusnik
On pon, 2009-12-28 at 20:28 +0100, Aljaž Prusnik wrote:
 On pon, 2009-12-28 at 02:23 +0400, Manu Abraham wrote:
  Can you please do a lspci -vn for the Mantis card you have ? Also try
  loading the mantis.ko module with verbose=5 module parameter, to get
  more debug information.
 

To continue, it seems the module is registering the remote commands, but
dunno, why irw shows nothing:

dmesg of typing on remote:
[ 4854.805594] mantis_uart_read (0): Reading ... 3d
[ 4854.805605] mantis_uart_work (0): UART BUF:0 3d 
[ 4854.805609] 
[ 4854.805615] mantis_uart_read (0): Reading ... 3d
[ 4854.805621] mantis_uart_work (0): UART BUF:0 3d 
[ 4854.805624] 
[ 4895.266923] 
[ 4895.266926] -- Stat=4000800 Mask=800 --IRQ-1
[ 4895.266956] 
[ 4895.266958] -- Stat=4000800 Mask=800 --IRQ-1
[ 4895.266979] 
[ 4895.266981] -- Stat=4000800 Mask=800 --IRQ-1
[ 4895.266999] 
[ 4895.267000] -- Stat=4000800 Mask=800 --IRQ-1
[ 4895.267015] 
[ 4895.267016] -- Stat=4000800 Mask=800 --IRQ-1
[ 4895.267031] 
[ 4895.267032] -- Stat=4000800 Mask=800 --IRQ-1
[ 4895.267043] mantis_uart_read (0): Reading ... 3e
[ 4895.267054] mantis_uart_work (0): UART BUF:0 3e 
[ 4895.267058] 
[ 4895.267065] mantis_uart_read (0): Reading ... 3e
[ 4895.267070] mantis_uart_work (0): UART BUF:0 3e 


--
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


cx18: leadtec pvr 2100 card fix

2009-12-28 Thread Sergey Bolshakov

Hi.
Seems cx18 module has incorrect .xceive_pin value for card,
as i see lots of i2c errors in dmesg from xc2028.
i'm using 2.6.32.2, my hardware is:

# lspci -vnns 00:09.0
00:09.0 Multimedia video controller [0400]: Conexant Systems, Inc. CX23418 
Single-Chip MPEG-2 Encoder with Integrated Analog Video/Broadcast Audio Decoder 
[14f1:5b7a]
Subsystem: LeadTek Research Inc. Device [107d:6f27]
Flags: bus master, medium devsel, latency 64, IRQ 17
Memory at f000 (32-bit, non-prefetchable) [size=64M]
Capabilities: [44] Vital Product Data
Capabilities: [4c] Power Management version 2
Kernel driver in use: cx18
Kernel modules: cx18

Following fixes this problem for me, the rest seems working:

diff --git a/drivers/media/video/cx18/cx18-cards.c b/drivers/media/video/cx18/cx18-cards.c
index f11e47a..f808fb6 100644
--- a/drivers/media/video/cx18/cx18-cards.c
+++ b/drivers/media/video/cx18/cx18-cards.c
@@ -393,7 +393,7 @@ static const struct cx18_card cx18_card_leadtek_pvr2100 = {
 	.gpio_init.direction = 0x7,
 	.gpio_audio_input = { .mask   = 0x7,
 			  .tuner  = 0x6, .linein = 0x2, .radio  = 0x2 },
-	.xceive_pin = 15,
+	.xceive_pin = 1,
 	.pci_list = cx18_pci_leadtek_pvr2100,
 	.i2c = cx18_i2c_std,
 };

-- 


Re: [linux-dvb] Acoustical mode for femon

2009-12-28 Thread Mika Laitio

The monitoring application produces a sound indicating the signal quality. The
higher the beep the better the signal quality.
This is useful while mounting the antenna for finding the best position without
having to look at the monitor or without even having a monitor.


Thank you for this!  Very useful since it's hard to be outside aiming
and inside looking at a monitor at the same time.  :)


While I was trying to locate astra on last year by rotating the dish I 
used both the cheap (10 euro) beeper that was but in the satellite cable 
end to find out the abbroximately dish direction. Fine tuning was done by 
starring the laptop screen szap output to find out when it started to 
printout LOCKED output. (I used ssh from the laptop to dvb card computer 
where I had put the channel info for one astra channel and tried to szap 
for that one. That of course meant that I needed the correct channel info 
for one channel in satellite even before I could try to scan channels from 
that satellite...)


Mika
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] input: imon driver for SoundGraph iMON/Antec Veris IR devices

2009-12-28 Thread Jarod Wilson
Hey Dmitry,

Thanks much for the review, comments inline below...

On Dec 28, 2009, at 4:31 AM, Dmitry Torokhov wrote:

 Hi Jarod,
 
 On Mon, Dec 28, 2009 at 12:11:55AM -0500, Jarod Wilson wrote:
 This is an input layer driver for the SoundGraph iMON and Antec
 Veris IR and/or Display (LCD/VFD/VGA touchscreen) devices that
 do onboard signal decoding.
 
 This driver has been baking for quite a while in the lirc tree, as
 lirc_imon, which supported both all the current onboard decoding
 imon devices, as well as some older raw IR ones. I've split support
 into two different drivers now, this one, a pure input driver with
 no ties to lirc at all (device can be used with lircd via its
 userspace devinput driver though) and a pure lirc driver for the
 older devices that don't do onboard decoding. There's a bit of
 code duplication between the two, but not much anymore...
 
 I did start using *some* of the new sparse keymap code for this
 driver, but I quickly found I really needed more access to the
 raw data (as well as 64-bit hw codes), so I'm really only using
 sparse_keymap_{setup,free} right now, and I've not had cause to
 implement {s,g}etkeycode to date. Work for the future.
 
 Heavily tested with an Antec Veris Elite (IR + VFD), lightly tested
 with an Antec Veris Premiere (IR + LCD), works quite well on both,
 using both the stock Antec RM-200 remote and a Windows MCE remote
 (as well as with a Logitech Harmony 880 programmed to emulate the
 Antec remote).
 
 nb: checkpatch.pl has one warning on this patch:
  WARNING: struct file_operations should normally be const
  #200: FILE: drivers/input/misc/imon.c:146:
  +static struct file_operations display_fops = {
 
  total: 0 errors, 1 warnings, 2360 lines checked
 
 We don't make the struct const, because we swap in a new write fop
 if the device has an LCD character display instead of a VFD one.
 
 Why don't you set up separate fops for LCD and mark both const?

Was mainly to keep things simple, and that's just the way its been for some 
time... Creating separate fops didn't turn out to be too painful though, was 
only a 14-line increase, so I've gone ahead and done that.

 +/* Driver init/exit prototypes */
 +static int __init imon_init(void);
 +static void __exit imon_exit(void);
 
 Why are they needed?

Bah. Legacy crud from lirc_imon. Gone.

 +
 +/*** G L O B A L S ***/
 +
 +struct imon_context {
 +struct device *dev;
 +struct usb_device *usbdev_intf0;
 +/* Newer devices have two interfaces */
 +struct usb_device *usbdev_intf1;
 +int display_supported;  /* not all controllers do */
 +int display_isopen; /* display port has been opened */
 +int ir_isopen;  /* IR port open */
 +int ir_isassociating;   /* IR port open for association */
 
 bools for these?

Crap, yeah, been meaning to get around to changing those for some time, forgot 
about them. Done. (As well as for all other cases I could find where an int was 
being used where a bool would suffice).

 +/* to prevent races between open() and disconnect(), probing, etc */
 +static DEFINE_MUTEX(driver_lock);
 +
 +static int debug;
 +
 +/* lcd, vfd, vga or none? should be auto-detected, but can be overridden... 
 */
 +static int display_type;
 +
 +/* IR protocol: native iMON, Windows MCE (RC-6), or iMON w/o PAD stabilize 
 */
 +static int ir_protocol;
 +
 +/*
 + * In certain use cases, mouse mode isn't really helpful, and could actually
 + * cause confusion, so allow disabling it when the IR device is open.
 + */
 +static int nomouse;
 +
 +/* threshold at which a pad push registers as an arrow key in kbd mode */
 +static int pad_thresh;
 +
 +/***  M O D U L E   C O D E ***/
 +
 +MODULE_AUTHOR(MOD_AUTHOR);
 +MODULE_DESCRIPTION(MOD_DESC);
 +MODULE_VERSION(MOD_VERSION);
 +MODULE_LICENSE(GPL);
 +MODULE_DEVICE_TABLE(usb, imon_usb_id_table);
 +module_param(debug, int, S_IRUGO | S_IWUSR);
 +MODULE_PARM_DESC(debug, Debug messages: 0=no, 1=yes(default: no));
 
 Bool here. I also prefer keeping module_param() and MODULE_PARM_DESC()
 with the definition of the variable.

Done.

 +
 +static void free_imon_context(struct imon_context *context)
 +{
 +usb_free_urb(context-tx_urb);
 +usb_free_urb(context-rx_urb_intf0);
 +usb_free_urb(context-rx_urb_intf1);
 +kfree(context);
 +
 +dev_dbg(context-dev, %s: iMON context freed\n, __func__);
 +}

Eew. Just noticed the above has a nasty little use-after-free possibility 
with debugging enabled...

 +/**
 + * Process the incoming packet
 + */
 +static void imon_incoming_packet(struct imon_context *context,
 + struct urb *urb, int intf)
 +{
 +int len = urb-actual_length;
 +unsigned char *buf = urb-transfer_buffer;
 +struct device *dev = context-dev;
 +char rel_x = 0x00, rel_y = 0x00;
 +int ts_input = 0;
 +int dir = 0;
 +u16 timeout, threshold;
 +u16 kc;
 +u16 norelease = 0;
 +int i, k;
 +int offset =