The device structure is not available in many functions in sn9c20x- queue.c. Then there are the init functions in sn9c20x-ub.c and several functions in sn9c20x-sysfs.c (the problem can be fixed by getting the device from the *priv thingy. However I don't think this is really worth the effort and I think there will be performance loss.
GWater On 8 Jun., 08:15, Brian Johnson <[email protected]> wrote: > I think the main places i know of were you can't access the device > structure is before probe is called and early in the probe function > before we create the device structure. Where there other areas that > you found Josua? The other issue is of course that our current macros > besides just printing the message will check the log level allowing > the driver to dynamically adjust which messages get displayed. > > I'm attaching the current patch-set against kernel 2.6.30-rc8 for > testing purposes > > Changes: > * moved to header and renamed RX,RY,GX,GY,BX and BY arrays > * changed __u* to u* > * some error checking to create_sysfs function > * changed many ret = ... to return ... getting rid of unnecessary ret variable > * added current SXGA patches for ov965x sensors (forces bayer fmt) > * input device now returns KEY_CAMERA instead of BTN_0 > > On Sun, Jun 7, 2009 at 8:50 AM, GWater<[email protected]> wrote: > > > I tried to fix the UDIA problem but that seems impossible: > > > dev_*() requires "struct device *". In most functions this is > > available via &(dev->udev->dev). That's ugly but usable. Unfortunatly > > there are many functions that need info and debugging output which > > don't have this struct available at all. I tried very hard to get > > devices from usb_interfaces, v4l_devices, usb_devices but in the end I > > often had to fall back to using printk() directly. And that's not > > comfortable and not really an option. > > > My advice: Use dev_*() but redefine the macro like UDIA_*(). > > > Thoughts, other ideas? > > > GWater > > > If anyone wants my halfdone patch... Say so. > > >> Jun., 08:53, Daniel Ribeiro <[email protected]> wrote: > >> Em Sáb, 2009-06-06 às 01:35 -0400, Brian Johnson escreveu: > > >> > sn9c20x: 184k > >> > omnivision: 43k > >> > micron: 30k > >> > hv7131r: 4.4k > > >> > I could reduce the big one my about another 30k by splitting out the > >> > sysfs and debugfs as well which would break down as > > >> > sn9c20x: 154k > >> > sysfs: 20k > >> > debugfs: 10k > >> > omnivision: 43k > >> > micron: 30k > >> > hv7131r: 4.4k > > >> Thats a start. :) > > >> I have some comments that could help reduce review ping-pong, just small > >> stuff that I already know are not well seen on the kernel community... > > >> static const int RX[] > >> static const int RY[] > >> static const int GX[] > >> static const int GY[] > >> static const int BX[] > >> static const int BY[] > > >> Namespace pollution. Names like these are not good, because they may > >> clash with other stuff on the kernel, use a more meaningful name, and > >> keep it lower-case (upper case is generally for macro). > >> Also, it may be a good idea to move these into a header file. > > >> __u8, __u16, __u32 > > >> Just use u8, u16, u32... > > >> 545 ret = usb_sn9c20x_control_write(dev, 0x1006, led, 2); > >> 546 > >> 547 return ret; > > >> Just "return usb_sn9c20x...." No need for the ret variable. Also applies > >> to a lot of other functions. > > >> UDIA_INFO("Using yuv420 output format\n"); > > >> This is a show stopper. People dont like these. Use dev_err|warn|dbg() > >> or pr_debug(). > > >> int sn9c20x_create_sysfs_files(struct video_device *vdev) > > >> This function lacks error handling.. > > >> Other than these minor nitpicks the driver looks good. I lack the v4l > >> subsystem knowledge to comment on more technical stuff. > > >> Don't forget to run scripts/checkpatch.pl on the patches, and follow the > >> CodingStyle. > > >> Mauro is always at #v4l on irc.freenode.net, nick "mchehab", it may be a > >> good idea to come in and present the project/driver on IRC. > > >> It's already too late for .30, but I think you have chances to get it in > >> for .31 if you send it for review ASAP. > > >> Good Luck! > > >> -- > >> Daniel Ribeiro > > >> signature.asc > >> < 1 KBAnzeigenHerunterladen > > > > 0001-Add-sn9c20x-webcam-driver.patch > 203KAnzeigenHerunterladen > > 0002-sn9c20x-Add-sysfs-support.patch > 31KAnzeigenHerunterladen > > 0003-sn9c20x-Add-debugfs-support.patch > 21KAnzeigenHerunterladen > > 0004-sn9c20x-Add-support-for-omnivision-sensors.patch > 56KAnzeigenHerunterladen > > 0005-sn9c20x-Add-support-for-micron-sensors.patch > 35KAnzeigenHerunterladen > > 0006-sn9c20x-Add-hv7131r-sensor-support.patch > 6KAnzeigenHerunterladen --~--~---------~--~----~------------~-------~--~----~ Lets make microdia webcams plug'n play, (currently plug'n pray) To post to this group, send email to [email protected] Visit us online https://groups.google.com/group/microdia -~----------~----~----~----~------~----~------~--~---
