Am 17.03.2012 14:11, schrieb Jean-Francois Moine:
> On Fri, 16 Mar 2012 23:15:45 +0100
> Frank Schäfer <fschaefer.oss@xxxxxxxxxxxxxx> wrote:
>
>> Anyway, I would be glad to get some feedback concerning form and content of the code, becausse I'm still a newbie to kernel programming.
> Hi Frank,
>
> I agree that writing a new gspca subdriver is easier than extending some
> other video driver...
>
> Here are some remarks:
>
> - you should not start the workqueue button_check immediately because
> the webcam hardware is not fully initialized in sd_config().
> Instead, you may check gspca_dev->present or have a greater delay
> (1s) in sd_config(). This also avoids to patch the gspca main.
>
> - usually, the sensor reset ('12 80' for Omnivision - 2nd item in
> ov2640_init) asks for some time. So, a little delay (200 ms) is
> welcome.
>
> - I am not sure that looping on usb_control_msg() on read or write error
> does help.
>
> - the debug flags D_USBI and D_USBO are no more available. It is easier
> to use usbmon to trace the exchanges.
>
> - using the new control mechanism removes a lot of code (see stk014.c).
>
> - using the gspca variable 'usb_err' avoids testing each USB write
> (during probe, you may use a flag to skip printing error messages).
>
> - in the form:
>
> - you must use
>
> module_usb_driver(sd_driver);
>
> to replace the module stuff.
>
> - don't use C++ comments
>
> - the code may be optimized, as replacing the sequences of
> write_prop_single() calls by a loop and a table.
>
> Best regards.
>
Hi Jean-Francois,
thanks for your remarks !
I will incoorporate them in case that a decision for a gspca subdriver
is made.
Regards,
Frank
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
