On Mon, 19 Jan 2009 23:22:33 +0000
Adam Baker <li...@baker-net.org.uk> wrote:
> Add initial support for cameras based on the SQ Technologies SQ-905
> chipset (USB ID 2770:9120) to V4L2 using the gspca infrastructure.
[snip]
> ---
> Following all the comments when I posted this for review Theodore and
> I have produced 2 new versions. The most critical comment last time
> was that we were using the system work queue inappropriately so this
> version creates its own work queue. The alternative version that I
> will post shortly avoids a work queue altogether by using
> asynchronous USB commands but in order to do so has increased the
> code size.
>
> I'll leave it to the assembled list expertise to say which option is
> preferable.
[snip]
Hello Adam and Theodore,
I looked at your two versions, and I think the first one (work queue)
is the simplest. So, I am ready to put your driver in my repository for
inclusion in a next linux kernel.
I have just a few remarks and a request.
- There are still small CodingStyle errors.
- Why do you need the function name in the debug messages?
- In sd_init, you should better convert the 4 bytes to u32 and do a
switch.
- On disconnection, the function sd_stopN is not called, so the
workqueue may be still running.
- At streamon time (sd_start), you allocate the buffer and send a
command. This may be done in the workqueue function. This function may
also do the buffer free and send the stop command on exit.
Re-thinking the streaming part gives:
. streamon (sd_start)
. init_completion()
. start the workqueue
(dev->streaming is not useful)
. workqueue function
. allocate the transfer buffer (pointer in the stack)
. send 'start capture'
. read loop - don't forget:
- to test gspca_dev->streaming: it may be streamoff,
close or disconnect
- to protect to usb_control_msg by the
gspca_dev->usb_lock mutex: this will permit
to handle future webcam controls.
. on streamoff or USB error
. free the transfer buffer
. complete()
. streamoff
. sd_stopN: non useful
. sd_stop0:
. wait_for_completion
. dev->work_thread = NULL
Now, the request: some guys asked for support of their webcams based on
sq930x chips. A SANE backend driver exists, written by Gerard Klaver
(http://gkall.hobby.nl/sq930x.html).
May you have a look and say if handling these chips may be done in your
driver?
Regards.
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
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
