On Sun, Sep 13, 2009 at 10:58 AM, Mauro Carvalho Chehab
<mche...@infradead.org> wrote:
> Em Fri, 11 Sep 2009 00:00:35 -0400
> Devin Heitmueller <dheitmuel...@kernellabs.com> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to