Hello Alan,
> There are some oddments that I'd move into headers,
For example? I tried to keep the header as small as possible including
necessary defines and main structures only.
> you have a macro
> funny (don't use {;} for a null macro use do {;} while(0) [weird and
> wonderous C trick to get if/else right 8))
Bugwise:
You can't use floats in the kernel
Yes, this is a big ERROR. I am somehow embarassed :-)
cf_open doesn't handle O_NDELAY. If O_NDELAY is in the file flags you
should try the lock without sleeping and return -EWOULDBLOCK otherwise
(see audio drivers for an example)
Fixed.
I also don't understand what stops the following going wrong
open
fork
thread 1 - read image
thread 2 - change picture size
There's not a particular reason behind that. I used that procedure
to avoid the settings to be changed if the requested frames aren't still
captured. I can remove the incriminated lines of code (<5) if the policy
has to be different.
Other bits:
For things that really can't happen we normally use BUG().
I don't understand, sorry..
> DBG(2, "Device rejected: too many simoultaneous cameras.")
> (simultaneous -typo - no big deal just I noticed it)
> Overall looks basically sound. I think I'd dump the module param
> maze for ioctls and make that a per camera struct so you can alloc
> it dynamically but thats for later
Also, consider that the code you checked doesn't look like the code i am
preparing: the driver is cleaner now: software convertion and upscaling
and other post-processing routines has been removed.
Consequently lots of parameter as been taken away, so there's no need of
other ioctls.
What ahout the /proc interface? Can I keep it?
I will eventually update the driver in the future (as my purpose is to
mantain the current code while porting the driver to V4L2 (one of these
days.....))
I will send you the driver by email as soon as I totally "clean" it and
the ovsensor module is included in the kernel 2.5.
Luca Risolia
