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
Description: Esta é uma parte de mensagem assinada digitalmente
