On Sun, Sep 13, 2009 at 10:58 AM, Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxx> wrote: > Em Fri, 11 Sep 2009 00:00:35 -0400 > Devin Heitmueller <dheitmueller@xxxxxxxxxxxxxx> escreveu: > >> Hello Mauro, >> >> Please PULL from http://kernellabs.com/hg/~dheitmueller/em28xx-vbi3 >> for the following >> >> em28xx: better describe vinctrl registers >> em28xx: make video isoc stream work when VBI is enabled >> em28xx: add raw VBI support for NTSC >> em28xx: fix mmap_mapper with vbi >> em28xx: restructure fh/dev locking to handle both video and vbi >> em28xx: remove unreferenced variable >> em28xx: do not create /dev/vbiX device if VBI not supported >> em28xx: only advertise VBI capability if supported >> em28xx: implement g_std v4l call >> em28xx: remove unneeded code that set VINCTRL register >> em28xx: fix unused variable warning >> >> Note: the support currently only works for NTSC. I will be getting >> PAL support working in the next couple of weeks as I get an >> environment setup for testing. >> >> Special thanks go out to EyeMagnet Limited for sponsoring this work. > > Applied, thanks. > > There are some CodingStyle fixes that need to be done. Please send a patch later. > > The first one is that drivers shouldn't contain emacs or other text editors > format tags. If you use such editors, use a local var instead, as stated at > Documentation/CodingStyle, chapter 18. > > The remaining ones are pointed by checkpatch.pl: > > /tmp/newpatches/hg_em28xx-vbi3_02.patch: > em28xx: make video isoc stream work when VBI is enabled > WARNING: printk() should include KERN_ facility level > #226: FILE: linux/drivers/media/video/em28xx/em28xx-video.c:510: > + printk("djh c should never happen\n"); > > WARNING: braces {} are not necessary for any arm of this statement > #238: FILE: linux/drivers/media/video/em28xx/em28xx-video.c:522: > + if (vbi_buf == NULL) > [...] > + else { > [...] > > WARNING: line over 80 characters > #241: FILE: linux/drivers/media/video/em28xx/em28xx-video.c:525: > + vbioutp = videobuf_to_vmalloc(&vbi_buf->vb); > > total: 0 errors, 3 warnings, 299 lines checked > > /tmp/newpatches/hg_em28xx-vbi3_03.patch: > em28xx: add raw VBI support for NTSC > WARNING: printk() should include KERN_ facility level > #116: FILE: linux/drivers/media/video/em28xx/em28xx-video.c:305: > + printk("dev is null\n"); > > WARNING: printk() should include KERN_ facility level > #121: FILE: linux/drivers/media/video/em28xx/em28xx-video.c:310: > + printk("dma_q is null\n"); > > WARNING: printk() should include KERN_ facility level > #127: FILE: linux/drivers/media/video/em28xx/em28xx-video.c:316: > + printk("buf is null\n"); > > WARNING: printk() should include KERN_ facility level > #132: FILE: linux/drivers/media/video/em28xx/em28xx-video.c:321: > + printk("p is null\n"); > > WARNING: printk() should include KERN_ facility level > #136: FILE: linux/drivers/media/video/em28xx/em28xx-video.c:325: > + printk("outp is null\n"); > > WARNING: braces {} are not necessary for any arm of this statement > #446: FILE: linux/drivers/media/video/em28xx/em28xx-video.c:1937: > + if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) > [...] > + else { > [...] > > ERROR: space required after that ',' (ctx:VxV) > #652: FILE: linux/drivers/media/video/em28xx/em28xx-vbi.c:32: > +module_param(vbibufs,int,0644); > ^ > > ERROR: space required after that ',' (ctx:VxV) > #652: FILE: linux/drivers/media/video/em28xx/em28xx-vbi.c:32: > +module_param(vbibufs,int,0644); > ^ > > ERROR: space required after that ',' (ctx:VxV) > #653: FILE: linux/drivers/media/video/em28xx/em28xx-vbi.c:33: > +MODULE_PARM_DESC(vbibufs,"number of vbi buffers, range 2-32"); > ^ > > ERROR: space required after that ',' (ctx:VxV) > #656: FILE: linux/drivers/media/video/em28xx/em28xx-vbi.c:36: > +module_param(vbi_debug,int,0644); > ^ > > ERROR: space required after that ',' (ctx:VxV) > #656: FILE: linux/drivers/media/video/em28xx/em28xx-vbi.c:36: > +module_param(vbi_debug,int,0644); > ^ > > ERROR: space required after that ',' (ctx:VxV) > #657: FILE: linux/drivers/media/video/em28xx/em28xx-vbi.c:37: > +MODULE_PARM_DESC(vbi_debug,"enable debug messages [vbi]"); > ^ > > ERROR: space required after that ',' (ctx:VxV) > #659: FILE: linux/drivers/media/video/em28xx/em28xx-vbi.c:39: > +#define dprintk(level,fmt, arg...) if (vbi_debug >= level) \ > ^ > > total: 7 errors, 6 warnings, 698 lines checked > > /tmp/newpatches/hg_em28xx-vbi3_05.patch: > em28xx: restructure fh/dev locking to handle both video and vbi > ERROR: return is not a function, parentheses are not required > #77: FILE: linux/drivers/media/video/em28xx/em28xx-video.c:891: > + return (fh->resources & bit); > > ERROR: return is not a function, parentheses are not required > #82: FILE: linux/drivers/media/video/em28xx/em28xx-video.c:896: > + return (dev->resources & bit); > > ERROR: space required after that ',' (ctx:VxV) > #150: FILE: linux/drivers/media/video/em28xx/em28xx-video.c:1742: > + if (unlikely(!res_get(fh,get_ressource(fh)))) > ^ > > ERROR: space required before the open parenthesis '(' > #233: FILE: linux/drivers/media/video/em28xx/em28xx-video.c:2249: > + if(dev->users == 1) { > > total: 4 errors, 0 warnings, 332 lines checked > > To cherry pick all files, you can do something like: > for i in /tmp/newpatches/*.patch; do ./mailimport $i; done > To merge it, the better is to run the merge script: > > > > > Cheers, > Mauro > Hello Mauro, Thanks for applying. I really need to figure out why checkpatch.pl didn't catch any of these on my system, since I *always* run "make checkpatch" before I do "make commit". This isn't the first time that this has happened (where I submit a patch series and you point out codingstyle issues). The emacs tags were actually because I used cx88-vbi.c as a starting point. I'll remove them. Anyway, I'll get a patch put together for the codingstyle issues you pointed out tonight. Thanks, Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- 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
