[PATCH]v4l: list entries no need to check

2010-11-17 Thread Figo.zhang

list entries are not need to be inited, so it no need
for checking. 

Reported-by: Andrew Chew ac...@nvidia.com
Signed-off-by: Figo.zhang figo1...@gmail.com
---
 drivers/media/video/mx1_camera.c   |3 ---
 drivers/media/video/pxa_camera.c   |3 ---
 drivers/media/video/sh_mobile_ceu_camera.c |3 ---
 3 files changed, 0 insertions(+), 9 deletions(-)

diff --git a/drivers/media/video/mx1_camera.c b/drivers/media/video/mx1_camera.c
index 5c17f9e..cb2dd24 100644
--- a/drivers/media/video/mx1_camera.c
+++ b/drivers/media/video/mx1_camera.c
@@ -182,9 +182,6 @@ static int mx1_videobuf_prepare(struct videobuf_queue *vq,
dev_dbg(icd-dev.parent, %s (vb=0x%p) 0x%08lx %d\n, __func__,
vb, vb-baddr, vb-bsize);
 
-   /* Added list head initialization on alloc */
-   WARN_ON(!list_empty(vb-queue));
-
BUG_ON(NULL == icd-current_fmt);
 
/*
diff --git a/drivers/media/video/pxa_camera.c b/drivers/media/video/pxa_camera.c
index 9de7d59..421de10 100644
--- a/drivers/media/video/pxa_camera.c
+++ b/drivers/media/video/pxa_camera.c
@@ -444,9 +444,6 @@ static int pxa_videobuf_prepare(struct videobuf_queue *vq,
dev_dbg(dev, %s (vb=0x%p) 0x%08lx %d\n, __func__,
vb, vb-baddr, vb-bsize);
 
-   /* Added list head initialization on alloc */
-   WARN_ON(!list_empty(vb-queue));
-
 #ifdef DEBUG
/*
 * This can be useful if you want to see if we actually fill
diff --git a/drivers/media/video/sh_mobile_ceu_camera.c 
b/drivers/media/video/sh_mobile_ceu_camera.c
index 2b24bd0..b2bef3f 100644
--- a/drivers/media/video/sh_mobile_ceu_camera.c
+++ b/drivers/media/video/sh_mobile_ceu_camera.c
@@ -354,9 +354,6 @@ static int sh_mobile_ceu_videobuf_prepare(struct 
videobuf_queue *vq,
dev_dbg(icd-dev.parent, %s (vb=0x%p) 0x%08lx %zd\n, __func__,
vb, vb-baddr, vb-bsize);
 
-   /* Added list head initialization on alloc */
-   WARN_ON(!list_empty(vb-queue));
-
 #ifdef DEBUG
/*
 * This can be useful if you want to see if we actually fill


--
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


Re: Allocating videobuf_buffer, but lists not being initialized

2010-11-16 Thread Figo.zhang

于 11/16/2010 03:37 PM, Hans Verkuil 写道:

On Tuesday, November 16, 2010 02:10:39 Andrew Chew wrote:

I'm looking at drivers/media/video/videobuf-dma-contig.c's __videobuf_alloc() 
routine.  We call kzalloc() to allocate the videobuf_buffer.  However, I don't see 
where the two lists (vb-stream and vb-queue) that are a part of struct 
videobuf_buffer get initialized (with, say, INIT_LIST_HEAD).


Yuck. The videobuf framework doesn't initialize vb-stream at all. It relies on
list_add_tail to effectively initialize it for it. It works, but it is not
exactly clean programming :-(

The vb-queue list has to be initialized in the driver. Never understood the
reason for that either.

Marek, can you make sure that videobuf2 will initialize these lists correctly?
That is, vb2 should do this initialization instead of the driver.


vb2 have init the list :
INIT_LIST_HEAD(q-queued_list);
INIT_LIST_HEAD(q-done_list);

btw, queued_list re-name grabbing_list is better?




--
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


Re: Allocating videobuf_buffer, but lists not being initialized

2010-11-16 Thread Figo.zhang

于 11/16/2010 03:40 PM, Hans Verkuil 写道:

On Tuesday, November 16, 2010 06:29:53 Pawel Osciak wrote:

On Mon, Nov 15, 2010 at 17:10, Andrew Chewac...@nvidia.com  wrote:

I'm looking at drivers/media/video/videobuf-dma-contig.c's __videobuf_alloc() 
routine.  We call kzalloc() to allocate the videobuf_buffer.  However, I don't see 
where the two lists (vb-stream and vb-queue) that are a part of struct 
videobuf_buffer get initialized (with, say, INIT_LIST_HEAD).



Those are not lists, but list entries. Those members of
videobuf_buffer struct are used to put the buffer on one of the
following lists: stream is a list entry for stream list in
videobuf_queue, queue is used as list entry for driver's buffer queue.


So? They still should be initialized properly. It's bad form to leave
invalid pointers there.


it just a list-entry, it has initialized by kzalloc at 
__videobuf_alloc_vb(),




Regards,

Hans



--
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


Re: [PATCH 1/1] videobuf: Initialize lists in videobuf_buffer.

2010-11-16 Thread Figo.zhang

 
 diff --git a/drivers/media/video/videobuf-dma-contig.c 
 b/drivers/media/video/videobuf-dma-contig.c
 index c969111..f7e0f86 100644
 --- a/drivers/media/video/videobuf-dma-contig.c
 +++ b/drivers/media/video/videobuf-dma-contig.c
 @@ -193,6 +193,8 @@ static struct videobuf_buffer *__videobuf_alloc_vb(size_t 
 size)
   if (vb) {
   mem = vb-priv = ((char *)vb) + size;
   mem-magic = MAGIC_DC_MEM;
 + INIT_LIST_HEAD(vb-stream);
 + INIT_LIST_HEAD(vb-queue);

i think it no need to be init, it just a list-entry.
--
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


Re: [PATCH v4 2/6] drivers:staging: ti-st: fmdrv_v4l2 sources

2010-11-16 Thread Figo.zhang
于 11/16/2010 09:18 PM, manjunatha_ha...@ti.com 写道:
 From: Manjunatha Hallimanjunatha_ha...@ti.com
 
 This module interfaces V4L2 subsystem and FM common
 module. It registers itself with V4L2 as Radio module.
 
 Signed-off-by: Manjunatha Hallimanjunatha_ha...@ti.com
 ---
   drivers/staging/ti-st/fmdrv_v4l2.c |  757 
 
   drivers/staging/ti-st/fmdrv_v4l2.h |   32 ++
   2 files changed, 789 insertions(+), 0 deletions(-)
   create mode 100644 drivers/staging/ti-st/fmdrv_v4l2.c
   create mode 100644 drivers/staging/ti-st/fmdrv_v4l2.h
 
 diff --git a/drivers/staging/ti-st/fmdrv_v4l2.c 
 b/drivers/staging/ti-st/fmdrv_v4l2.c
 new file mode 100644
 index 000..687d10f
 --- /dev/null
 +++ b/drivers/staging/ti-st/fmdrv_v4l2.c
 @@ -0,0 +1,757 @@
 +/*
 + *  FM Driver for Connectivity chip of Texas Instruments.
 + *  This file provides interfaces to V4L2 subsystem.
 + *
 + *  This module registers with V4L2 subsystem as Radio
 + *  data system interface (/dev/radio). During the registration,
 + *  it will expose two set of function pointers.
 + *
 + *1) File operation related API (open, close, read, write, poll...etc).
 + *2) Set of V4L2 IOCTL complaint API.
 + *
 + *  Copyright (C) 2010 Texas Instruments
 + *  Author: Raja Maniraja_m...@ti.com
 + *
 + *  This program is free software; you can redistribute it and/or modify
 + *  it under the terms of the GNU General Public License version 2 as
 + *  published by the Free Software Foundation.
 + *
 + *  This program is distributed in the hope that it will be useful,
 + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
 + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + *  GNU General Public License for more details.
 + *
 + *  You should have received a copy of the GNU General Public License
 + *  along with this program; if not, write to the Free Software
 + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 + *
 + */
 +
 +#include fmdrv.h
 +#include fmdrv_v4l2.h
 +#include fmdrv_common.h
 +#include fmdrv_rx.h
 +#include fmdrv_tx.h
 +
 +static struct video_device *gradio_dev;

why are you using global variable here?

--
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


Re: [PATCH 1/1] videobuf: Initialize lists in videobuf_buffer.

2010-11-16 Thread Figo.zhang

于 11/17/2010 09:38 AM, Andrew Chew 写道:

diff --git a/drivers/media/video/videobuf-dma-contig.c

b/drivers/media/video/videobuf-dma-contig.c

index c969111..f7e0f86 100644
--- a/drivers/media/video/videobuf-dma-contig.c
+++ b/drivers/media/video/videobuf-dma-contig.c
@@ -193,6 +193,8 @@ static struct videobuf_buffer

*__videobuf_alloc_vb(size_t size)

if (vb) {
mem = vb-priv = ((char *)vb) + size;
mem-magic = MAGIC_DC_MEM;
+   INIT_LIST_HEAD(vb-stream);
+   INIT_LIST_HEAD(vb-queue);


i think it no need to be init, it just a list-entry.


Okay, if that's really the case, then sh_mobile_ceu_camera.c, pxa_camera.c, mx1_camera.c, 
mx2_camera.c, and omap1_camera.c needs to be fixed to remove that 
WARN_ON(!list_empty(vb-queue)); in their videobuf_prepare() methods, because 
those WARN_ON's are assuming that vb-queue is properly initialized as a list head.

Which will it be?


yes, i think those WARN_ONs are no need.
--
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


Re: [PATCH 1/1] videobuf: Initialize lists in videobuf_buffer.

2010-11-16 Thread Figo.zhang

On 11/17/2010 03:11 PM, Hans Verkuil wrote:

On Wednesday, November 17, 2010 02:38:09 Andrew Chew wrote:

diff --git a/drivers/media/video/videobuf-dma-contig.c

b/drivers/media/video/videobuf-dma-contig.c

index c969111..f7e0f86 100644
--- a/drivers/media/video/videobuf-dma-contig.c
+++ b/drivers/media/video/videobuf-dma-contig.c
@@ -193,6 +193,8 @@ static struct videobuf_buffer

*__videobuf_alloc_vb(size_t size)

if (vb) {
mem = vb-priv = ((char *)vb) + size;
mem-magic = MAGIC_DC_MEM;
+   INIT_LIST_HEAD(vb-stream);
+   INIT_LIST_HEAD(vb-queue);


i think it no need to be init, it just a list-entry.


Okay, if that's really the case, then sh_mobile_ceu_camera.c, pxa_camera.c, mx1_camera.c, 
mx2_camera.c, and omap1_camera.c needs to be fixed to remove that 
WARN_ON(!list_empty(vb-queue)); in their videobuf_prepare() methods, because 
those WARN_ON's are assuming that vb-queue is properly initialized as a list head.

Which will it be?



These list entries need to be inited. It is bad form to have uninitialized
list entries. It is not as if this is a big deal to initialize them properly.


in kernel source code, list entry are not often to be inited.

for example, see mm/vmscan.c register_shrinker(), no one init the 
shrinker-list.


another example: see mm/swapfile.c  add_swap_extent(), no one init the
new_se-list.





--
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


Re: [PATCH v2]Resend:videobuf_dma_sg: a new implementation for mmap

2010-08-02 Thread Figo.zhang
On Fri, 2010-07-30 at 19:39 +0800, Figo.zhang wrote:
 On Fri, 2010-07-30 at 11:31 +0200, Laurent Pinchart wrote:
  Hi,
  
  On Friday 30 July 2010 02:08:02 Figo.zhang wrote:
   a mmap issue for videobuf-dma-sg: it will alloc a new page for mmaping 
   when
   it encounter page fault at video_vm_ops-fault(). pls see
   http://www.spinics.net/lists/linux-media/msg21243.html
   
   a new implementation for mmap, it translate to vmalloc to page at
   video_vm_ops-fault().
   
   in v2, if mem-dma.vmalloc is NULL at video_vm_ops-fault(), it will alloc
   memory by vmlloc_32().
  
  You're replacing allocation in videobuf_vm_fault by allocationg in 
  videobuf_vm_fault. I don't see the point. videobuf_vm_fault needs to go 
  away 
  completely.
 in now videobuf code, the mmap alloc a new buf, the capture dma buffer
 using vmalloc() alloc buffer, how is
 relationship with them? in usrspace , the mmap region will not the
 actual capture dma data, how is work? 


hmm, I understand with some mistake before.for mmap in videobuf-sg, it
is not call vmalloc_32() to alloc memory, it call
videobuf_dma_init_user_locked()-get_user_pages(), and the end, it will
handle_mm_fault()- vm_ops-fault(). Because in mmap(), it have assigned
vaule for baddr variable.

it is too obscure for videobuf-sg, hope for videbuf2. ~~


--
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


Re: [PATCH v2]Resend:videobuf_dma_sg: a new implementation for mmap

2010-07-30 Thread Figo.zhang
On Fri, 2010-07-30 at 11:31 +0200, Laurent Pinchart wrote:
 Hi,
 
 On Friday 30 July 2010 02:08:02 Figo.zhang wrote:
  a mmap issue for videobuf-dma-sg: it will alloc a new page for mmaping when
  it encounter page fault at video_vm_ops-fault(). pls see
  http://www.spinics.net/lists/linux-media/msg21243.html
  
  a new implementation for mmap, it translate to vmalloc to page at
  video_vm_ops-fault().
  
  in v2, if mem-dma.vmalloc is NULL at video_vm_ops-fault(), it will alloc
  memory by vmlloc_32().
 
 You're replacing allocation in videobuf_vm_fault by allocationg in 
 videobuf_vm_fault. I don't see the point. videobuf_vm_fault needs to go away 
 completely.
in now videobuf code, the mmap alloc a new buf, the capture dma buffer
using vmalloc() alloc buffer, how is
relationship with them? in usrspace , the mmap region will not the
actual capture dma data, how is work? 

 
 This has been discussed previously: fixing videobuf is not really possible. A 
 videobuf2 implementation is needed and is (slowly) being worked on. 
hi Mauro, do you think this is a issue for videobuf-dma-sg?

 I wouldn't 
 bother with this patch, just drop it.
 
  Signed-off-by: Figo.zhang figo1...@gmail.com
  ---
  drivers/media/video/videobuf-dma-sg.c |   50
  +++-- 1 files changed, 41 insertions(+), 9
  deletions(-)
  
  diff --git a/drivers/media/video/videobuf-dma-sg.c
  b/drivers/media/video/videobuf-dma-sg.c index 8359e6b..f7295da 100644
  --- a/drivers/media/video/videobuf-dma-sg.c
  +++ b/drivers/media/video/videobuf-dma-sg.c
  @@ -201,10 +201,11 @@ int videobuf_dma_init_kernel(struct videobuf_dmabuf
  *dma, int direction, dprintk(1, init kernel [%d pages]\n, nr_pages);
  
  dma-direction = direction;
  -   dma-vmalloc = vmalloc_32(nr_pages  PAGE_SHIFT);
  -   if (NULL == dma-vmalloc) {
  -   dprintk(1, vmalloc_32(%d pages) failed\n, nr_pages);
  -   return -ENOMEM;
  +   if (!dma-vmalloc)
  +   dma-vmalloc = vmalloc_32(nr_pages  PAGE_SHIFT);
  +   if (NULL == dma-vmalloc) {
  +   dprintk(1, vmalloc_32(%d pages) failed\n, nr_pages);
  +   return -ENOMEM;
  }
  
  dprintk(1, vmalloc is at addr 0x%08lx, size=%d\n,
  @@ -397,16 +398,47 @@ static void videobuf_vm_close(struct vm_area_struct
  *vma) */
   static int videobuf_vm_fault(struct vm_area_struct *vma, struct vm_fault
  *vmf) {
  -   struct page *page;
  +   struct page *page = NULL;
  +   struct videobuf_mapping *map = vma-vm_private_data;
  +   struct videobuf_queue *q = map-q;
  +   struct videobuf_dma_sg_memory *mem = NULL;
  +
  +   unsigned long offset;
  +   unsigned long page_nr;
  +   int first;
  
  dprintk(3, fault: fault @ %08lx [vma %08lx-%08lx]\n,
  (unsigned long)vmf-virtual_address,
  vma-vm_start, vma-vm_end);
  
  -   page = alloc_page(GFP_USER | __GFP_DMA32);
  -   if (!page)
  -   return VM_FAULT_OOM;
  -   clear_user_highpage(page, (unsigned long)vmf-virtual_address);
  +   mutex_lock(q-vb_lock);
  +   offset = (unsigned long)vmf-virtual_address - vma-vm_start;
  +   page_nr = offset  PAGE_SHIFT;
  +
  +   for (first = 0; first  VIDEO_MAX_FRAME; first++) {
  +   if (NULL == q-bufs[first])
  +   continue;
  +
  +   MAGIC_CHECK(mem-magic, MAGIC_SG_MEM);
  +
  +   if (q-bufs[first]-map == map)
  +   break;
  +   }
  +
  +   mem = q-bufs[first]-priv;
  +   if (!mem)
  +   return VM_FAULT_SIGBUS;
  +   if (!mem-dma.vmalloc) {
  +   mem-dma.vmalloc = vmalloc_32(PAGE_ALIGN(q-bufs[first]-size));
  +   if (NULL == mem-dma.vmalloc) {
  +   dprintk(1, %s: vmalloc_32() failed\n, __func__);
  +   return VM_FAULT_OOM;
  +   }
  +   } else
  +   page = vmalloc_to_page(mem-dma.vmalloc+
  +   (offset  (~PAGE_MASK)));
  +   mutex_unlock(q-vb_lock);
  +
  vmf-page = page;
  
  return 0;
 


--
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


Re: [PATCH v2]Resend:videobuf_dma_sg: a new implementation for mmap

2010-07-30 Thread Figo.zhang
On Fri, 2010-07-30 at 11:31 +0200, Laurent Pinchart wrote:
 Hi,
 
 On Friday 30 July 2010 02:08:02 Figo.zhang wrote:
  a mmap issue for videobuf-dma-sg: it will alloc a new page for mmaping when
  it encounter page fault at video_vm_ops-fault(). pls see
  http://www.spinics.net/lists/linux-media/msg21243.html
  
  a new implementation for mmap, it translate to vmalloc to page at
  video_vm_ops-fault().
  
  in v2, if mem-dma.vmalloc is NULL at video_vm_ops-fault(), it will alloc
  memory by vmlloc_32().
 
 You're replacing allocation in videobuf_vm_fault by allocationg in 
 videobuf_vm_fault. I don't see the point. videobuf_vm_fault needs to go away 
 completely.
in now videobuf code, the mmap alloc a new buf, the capture dma buffer
using vmalloc() alloc buffer, how is
relationship with them? in usrspace , the mmap region will not the
actual capture dma data, how is work? 

 
 This has been discussed previously: fixing videobuf is not really possible. A 
 videobuf2 implementation is needed and is (slowly) being worked on. 
hi Mauro, do you think this is a issue for videobuf-dma-sg?

 I wouldn't 
 bother with this patch, just drop it.
 
  Signed-off-by: Figo.zhang figo1...@gmail.com
  ---
  drivers/media/video/videobuf-dma-sg.c |   50
  +++-- 1 files changed, 41 insertions(+), 9
  deletions(-)
  
  diff --git a/drivers/media/video/videobuf-dma-sg.c
  b/drivers/media/video/videobuf-dma-sg.c index 8359e6b..f7295da 100644
  --- a/drivers/media/video/videobuf-dma-sg.c
  +++ b/drivers/media/video/videobuf-dma-sg.c
  @@ -201,10 +201,11 @@ int videobuf_dma_init_kernel(struct videobuf_dmabuf
  *dma, int direction, dprintk(1, init kernel [%d pages]\n, nr_pages);
  
  dma-direction = direction;
  -   dma-vmalloc = vmalloc_32(nr_pages  PAGE_SHIFT);
  -   if (NULL == dma-vmalloc) {
  -   dprintk(1, vmalloc_32(%d pages) failed\n, nr_pages);
  -   return -ENOMEM;
  +   if (!dma-vmalloc)
  +   dma-vmalloc = vmalloc_32(nr_pages  PAGE_SHIFT);
  +   if (NULL == dma-vmalloc) {
  +   dprintk(1, vmalloc_32(%d pages) failed\n, nr_pages);
  +   return -ENOMEM;
  }
  
  dprintk(1, vmalloc is at addr 0x%08lx, size=%d\n,
  @@ -397,16 +398,47 @@ static void videobuf_vm_close(struct vm_area_struct
  *vma) */
   static int videobuf_vm_fault(struct vm_area_struct *vma, struct vm_fault
  *vmf) {
  -   struct page *page;
  +   struct page *page = NULL;
  +   struct videobuf_mapping *map = vma-vm_private_data;
  +   struct videobuf_queue *q = map-q;
  +   struct videobuf_dma_sg_memory *mem = NULL;
  +
  +   unsigned long offset;
  +   unsigned long page_nr;
  +   int first;
  
  dprintk(3, fault: fault @ %08lx [vma %08lx-%08lx]\n,
  (unsigned long)vmf-virtual_address,
  vma-vm_start, vma-vm_end);
  
  -   page = alloc_page(GFP_USER | __GFP_DMA32);
  -   if (!page)
  -   return VM_FAULT_OOM;
  -   clear_user_highpage(page, (unsigned long)vmf-virtual_address);
  +   mutex_lock(q-vb_lock);
  +   offset = (unsigned long)vmf-virtual_address - vma-vm_start;
  +   page_nr = offset  PAGE_SHIFT;
  +
  +   for (first = 0; first  VIDEO_MAX_FRAME; first++) {
  +   if (NULL == q-bufs[first])
  +   continue;
  +
  +   MAGIC_CHECK(mem-magic, MAGIC_SG_MEM);
  +
  +   if (q-bufs[first]-map == map)
  +   break;
  +   }
  +
  +   mem = q-bufs[first]-priv;
  +   if (!mem)
  +   return VM_FAULT_SIGBUS;
  +   if (!mem-dma.vmalloc) {
  +   mem-dma.vmalloc = vmalloc_32(PAGE_ALIGN(q-bufs[first]-size));
  +   if (NULL == mem-dma.vmalloc) {
  +   dprintk(1, %s: vmalloc_32() failed\n, __func__);
  +   return VM_FAULT_OOM;
  +   }
  +   } else
  +   page = vmalloc_to_page(mem-dma.vmalloc+
  +   (offset  (~PAGE_MASK)));
  +   mutex_unlock(q-vb_lock);
  +
  vmf-page = page;
  
  return 0;
 



--
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


Re: [PATCH v2]videobuf_dma_sg: a new implementation for mmap

2010-07-29 Thread Figo.zhang
hi Laurent,
would you like to test this patch?

btw, why i send the patch , patchwork websit display a part of my patch?
https://patchwork.kernel.org/patch/114760/

Best,
Figo.zhang

On Wed, 2010-07-28 at 21:08 +0800, Figo.zhang wrote:
 a mmap issue for videobuf-dma-sg: it will alloc a new page for mmaping
 when it encounter page fault at video_vm_ops-fault().
 
 a new implementation for mmap, it translate the vmalloc to page at
 video_vm_ops-fault().
 
 in v2, if mem-dma.vmalloc is NULL at video_vm_ops-fault(), it will
 alloc memory by vmalloc_32().
 
 Signed-off-by: Figo.zhang figo1...@gmail.com
 ---
 drivers/media/video/videobuf-dma-sg.c |   51
 +++--
  1 files changed, 42 insertions(+), 9 deletions(-)
 
 diff --git a/drivers/media/video/videobuf-dma-sg.c
 b/drivers/media/video/videobuf-dma-sg.c
 index 8359e6b..767483d 100644
 --- a/drivers/media/video/videobuf-dma-sg.c
 +++ b/drivers/media/video/videobuf-dma-sg.c
 @@ -201,10 +201,11 @@ int videobuf_dma_init_kernel(struct
 videobuf_dmabuf *dma, int direction,
   dprintk(1, init kernel [%d pages]\n, nr_pages);
  
   dma-direction = direction;
 - dma-vmalloc = vmalloc_32(nr_pages  PAGE_SHIFT);
 - if (NULL == dma-vmalloc) {
 - dprintk(1, vmalloc_32(%d pages) failed\n, nr_pages);
 - return -ENOMEM;
 + if (!dma-vmalloc)
 + dma-vmalloc = vmalloc_32(nr_pages  PAGE_SHIFT);
 + if (NULL == dma-vmalloc) {
 + dprintk(1, vmalloc_32(%d pages) failed\n, nr_pages);
 + return -ENOMEM;
   }
  
   dprintk(1, vmalloc is at addr 0x%08lx, size=%d\n,
 @@ -397,16 +398,48 @@ static void videobuf_vm_close(struct
 vm_area_struct *vma)
   */
  static int videobuf_vm_fault(struct vm_area_struct *vma, struct
 vm_fault *vmf)
  {
 - struct page *page;
 + struct page *page = NULL;
 + struct videobuf_mapping *map = vma-vm_private_data;
 + struct videobuf_queue *q = map-q;
 + struct videobuf_dma_sg_memory *mem = NULL;
 +
 + unsigned long offset;
 + unsigned long page_nr;
 + int first;
  
   dprintk(3, fault: fault @ %08lx [vma %08lx-%08lx]\n,
   (unsigned long)vmf-virtual_address,
   vma-vm_start, vma-vm_end);
  
 - page = alloc_page(GFP_USER | __GFP_DMA32);
 - if (!page)
 - return VM_FAULT_OOM;
 - clear_user_highpage(page, (unsigned long)vmf-virtual_address);
 + mutex_lock(q-vb_lock);
 +
 + offset = (unsigned long)vmf-virtual_address - vma-vm_start;
 + page_nr = offset  PAGE_SHIFT;
 +
 + for (first = 0; first  VIDEO_MAX_FRAME; first++) {
 + if (NULL == q-bufs[first])
 + continue;
 +
 + MAGIC_CHECK(mem-magic, MAGIC_SG_MEM);
 +
 + if (q-bufs[first]-map == map)
 + break;
 + }
 +
 + mem = q-bufs[first]-priv;
 + if (!mem)
 + return VM_FAULT_SIGBUS;
 + if (!mem-dma.vmalloc) {
 + mem-dma.vmalloc = vmalloc_32(PAGE_ALIGN(q-bufs[first]-size));
 + if (NULL == mem-dma.vmalloc) {
 + dprintk(1, %s: vmalloc_32() failed\n, __func__);
 + return VM_FAULT_OOM;
 + }
 + } else
 + page = vmalloc_to_page(mem-dma.vmalloc+
 + (offset  (~PAGE_MASK)));
 + mutex_unlock(q-vb_lock);
 +
   vmf-page = page;
  
   return 0;
 


--
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


[PATCH v2]Resend:videobuf_dma_sg: a new implementation for mmap

2010-07-29 Thread Figo.zhang
a mmap issue for videobuf-dma-sg: it will alloc a new page for mmaping when it 
encounter
page fault at video_vm_ops-fault(). pls see 
http://www.spinics.net/lists/linux-media/msg21243.html

a new implementation for mmap, it translate to vmalloc to page at 
video_vm_ops-fault().

in v2, if mem-dma.vmalloc is NULL at video_vm_ops-fault(), it will alloc 
memory by 
vmlloc_32().

Signed-off-by: Figo.zhang figo1...@gmail.com
---
drivers/media/video/videobuf-dma-sg.c |   50 +++--
 1 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/drivers/media/video/videobuf-dma-sg.c 
b/drivers/media/video/videobuf-dma-sg.c
index 8359e6b..f7295da 100644
--- a/drivers/media/video/videobuf-dma-sg.c
+++ b/drivers/media/video/videobuf-dma-sg.c
@@ -201,10 +201,11 @@ int videobuf_dma_init_kernel(struct videobuf_dmabuf *dma, 
int direction,
dprintk(1, init kernel [%d pages]\n, nr_pages);
 
dma-direction = direction;
-   dma-vmalloc = vmalloc_32(nr_pages  PAGE_SHIFT);
-   if (NULL == dma-vmalloc) {
-   dprintk(1, vmalloc_32(%d pages) failed\n, nr_pages);
-   return -ENOMEM;
+   if (!dma-vmalloc)
+   dma-vmalloc = vmalloc_32(nr_pages  PAGE_SHIFT);
+   if (NULL == dma-vmalloc) {
+   dprintk(1, vmalloc_32(%d pages) failed\n, nr_pages);
+   return -ENOMEM;
}
 
dprintk(1, vmalloc is at addr 0x%08lx, size=%d\n,
@@ -397,16 +398,47 @@ static void videobuf_vm_close(struct vm_area_struct *vma)
  */
 static int videobuf_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 {
-   struct page *page;
+   struct page *page = NULL;
+   struct videobuf_mapping *map = vma-vm_private_data;
+   struct videobuf_queue *q = map-q;
+   struct videobuf_dma_sg_memory *mem = NULL;
+
+   unsigned long offset;
+   unsigned long page_nr;
+   int first;
 
dprintk(3, fault: fault @ %08lx [vma %08lx-%08lx]\n,
(unsigned long)vmf-virtual_address,
vma-vm_start, vma-vm_end);
 
-   page = alloc_page(GFP_USER | __GFP_DMA32);
-   if (!page)
-   return VM_FAULT_OOM;
-   clear_user_highpage(page, (unsigned long)vmf-virtual_address);
+   mutex_lock(q-vb_lock);
+   offset = (unsigned long)vmf-virtual_address - vma-vm_start;
+   page_nr = offset  PAGE_SHIFT;
+
+   for (first = 0; first  VIDEO_MAX_FRAME; first++) {
+   if (NULL == q-bufs[first])
+   continue;
+
+   MAGIC_CHECK(mem-magic, MAGIC_SG_MEM);
+
+   if (q-bufs[first]-map == map)
+   break;
+   }
+
+   mem = q-bufs[first]-priv;
+   if (!mem)
+   return VM_FAULT_SIGBUS;
+   if (!mem-dma.vmalloc) {
+   mem-dma.vmalloc = vmalloc_32(PAGE_ALIGN(q-bufs[first]-size));
+   if (NULL == mem-dma.vmalloc) {
+   dprintk(1, %s: vmalloc_32() failed\n, __func__);
+   return VM_FAULT_OOM;
+   }
+   } else
+   page = vmalloc_to_page(mem-dma.vmalloc+
+   (offset  (~PAGE_MASK)));
+   mutex_unlock(q-vb_lock);
+
vmf-page = page;
 
return 0;


--
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


[PATCH v2]videobuf_dma_sg: a new implementation for mmap

2010-07-28 Thread Figo.zhang
a mmap issue for videobuf-dma-sg: it will alloc a new page for mmaping
when it encounter page fault at video_vm_ops-fault().

a new implementation for mmap, it translate the vmalloc to page at
video_vm_ops-fault().

in v2, if mem-dma.vmalloc is NULL at video_vm_ops-fault(), it will
alloc memory by vmalloc_32().

Signed-off-by: Figo.zhang figo1...@gmail.com
---
 drivers/media/video/videobuf-dma-sg.c |   51
+++--
 1 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/drivers/media/video/videobuf-dma-sg.c
b/drivers/media/video/videobuf-dma-sg.c
index 8359e6b..767483d 100644
--- a/drivers/media/video/videobuf-dma-sg.c
+++ b/drivers/media/video/videobuf-dma-sg.c
@@ -201,10 +201,11 @@ int videobuf_dma_init_kernel(struct
videobuf_dmabuf *dma, int direction,
dprintk(1, init kernel [%d pages]\n, nr_pages);
 
dma-direction = direction;
-   dma-vmalloc = vmalloc_32(nr_pages  PAGE_SHIFT);
-   if (NULL == dma-vmalloc) {
-   dprintk(1, vmalloc_32(%d pages) failed\n, nr_pages);
-   return -ENOMEM;
+   if (!dma-vmalloc)
+   dma-vmalloc = vmalloc_32(nr_pages  PAGE_SHIFT);
+   if (NULL == dma-vmalloc) {
+   dprintk(1, vmalloc_32(%d pages) failed\n, nr_pages);
+   return -ENOMEM;
}
 
dprintk(1, vmalloc is at addr 0x%08lx, size=%d\n,
@@ -397,16 +398,48 @@ static void videobuf_vm_close(struct
vm_area_struct *vma)
  */
 static int videobuf_vm_fault(struct vm_area_struct *vma, struct
vm_fault *vmf)
 {
-   struct page *page;
+   struct page *page = NULL;
+   struct videobuf_mapping *map = vma-vm_private_data;
+   struct videobuf_queue *q = map-q;
+   struct videobuf_dma_sg_memory *mem = NULL;
+
+   unsigned long offset;
+   unsigned long page_nr;
+   int first;
 
dprintk(3, fault: fault @ %08lx [vma %08lx-%08lx]\n,
(unsigned long)vmf-virtual_address,
vma-vm_start, vma-vm_end);
 
-   page = alloc_page(GFP_USER | __GFP_DMA32);
-   if (!page)
-   return VM_FAULT_OOM;
-   clear_user_highpage(page, (unsigned long)vmf-virtual_address);
+   mutex_lock(q-vb_lock);
+
+   offset = (unsigned long)vmf-virtual_address - vma-vm_start;
+   page_nr = offset  PAGE_SHIFT;
+
+   for (first = 0; first  VIDEO_MAX_FRAME; first++) {
+   if (NULL == q-bufs[first])
+   continue;
+
+   MAGIC_CHECK(mem-magic, MAGIC_SG_MEM);
+
+   if (q-bufs[first]-map == map)
+   break;
+   }
+
+   mem = q-bufs[first]-priv;
+   if (!mem)
+   return VM_FAULT_SIGBUS;
+   if (!mem-dma.vmalloc) {
+   mem-dma.vmalloc = vmalloc_32(PAGE_ALIGN(q-bufs[first]-size));
+   if (NULL == mem-dma.vmalloc) {
+   dprintk(1, %s: vmalloc_32() failed\n, __func__);
+   return VM_FAULT_OOM;
+   }
+   } else
+   page = vmalloc_to_page(mem-dma.vmalloc+
+   (offset  (~PAGE_MASK)));
+   mutex_unlock(q-vb_lock);
+
vmf-page = page;
 
return 0;



--
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


[PATCH v2]videobuf_dma_sg: a new implementation for mmap

2010-07-28 Thread Figo.zhang
a mmap issue for videobuf-dma-sg: it will alloc a new page for mmaping
when it encounter page fault at video_vm_ops-fault().

a new implementation for mmap, it translate the vmalloc to page at
video_vm_ops-fault().

in v2, if mem-dma.vmalloc is NULL at video_vm_ops-fault(), it will
alloc memory by vmalloc_32().

Signed-off-by: Figo.zhang figo1...@gmail.com
---
drivers/media/video/videobuf-dma-sg.c |   51
+++--
 1 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/drivers/media/video/videobuf-dma-sg.c
b/drivers/media/video/videobuf-dma-sg.c
index 8359e6b..767483d 100644
--- a/drivers/media/video/videobuf-dma-sg.c
+++ b/drivers/media/video/videobuf-dma-sg.c
@@ -201,10 +201,11 @@ int videobuf_dma_init_kernel(struct
videobuf_dmabuf *dma, int direction,
dprintk(1, init kernel [%d pages]\n, nr_pages);
 
dma-direction = direction;
-   dma-vmalloc = vmalloc_32(nr_pages  PAGE_SHIFT);
-   if (NULL == dma-vmalloc) {
-   dprintk(1, vmalloc_32(%d pages) failed\n, nr_pages);
-   return -ENOMEM;
+   if (!dma-vmalloc)
+   dma-vmalloc = vmalloc_32(nr_pages  PAGE_SHIFT);
+   if (NULL == dma-vmalloc) {
+   dprintk(1, vmalloc_32(%d pages) failed\n, nr_pages);
+   return -ENOMEM;
}
 
dprintk(1, vmalloc is at addr 0x%08lx, size=%d\n,
@@ -397,16 +398,48 @@ static void videobuf_vm_close(struct
vm_area_struct *vma)
  */
 static int videobuf_vm_fault(struct vm_area_struct *vma, struct
vm_fault *vmf)
 {
-   struct page *page;
+   struct page *page = NULL;
+   struct videobuf_mapping *map = vma-vm_private_data;
+   struct videobuf_queue *q = map-q;
+   struct videobuf_dma_sg_memory *mem = NULL;
+
+   unsigned long offset;
+   unsigned long page_nr;
+   int first;
 
dprintk(3, fault: fault @ %08lx [vma %08lx-%08lx]\n,
(unsigned long)vmf-virtual_address,
vma-vm_start, vma-vm_end);
 
-   page = alloc_page(GFP_USER | __GFP_DMA32);
-   if (!page)
-   return VM_FAULT_OOM;
-   clear_user_highpage(page, (unsigned long)vmf-virtual_address);
+   mutex_lock(q-vb_lock);
+
+   offset = (unsigned long)vmf-virtual_address - vma-vm_start;
+   page_nr = offset  PAGE_SHIFT;
+
+   for (first = 0; first  VIDEO_MAX_FRAME; first++) {
+   if (NULL == q-bufs[first])
+   continue;
+
+   MAGIC_CHECK(mem-magic, MAGIC_SG_MEM);
+
+   if (q-bufs[first]-map == map)
+   break;
+   }
+
+   mem = q-bufs[first]-priv;
+   if (!mem)
+   return VM_FAULT_SIGBUS;
+   if (!mem-dma.vmalloc) {
+   mem-dma.vmalloc = vmalloc_32(PAGE_ALIGN(q-bufs[first]-size));
+   if (NULL == mem-dma.vmalloc) {
+   dprintk(1, %s: vmalloc_32() failed\n, __func__);
+   return VM_FAULT_OOM;
+   }
+   } else
+   page = vmalloc_to_page(mem-dma.vmalloc+
+   (offset  (~PAGE_MASK)));
+   mutex_unlock(q-vb_lock);
+
vmf-page = page;
 
return 0;


--
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


[PATCH]videobuf_dma_sg: a new implementation for mmap

2010-07-27 Thread Figo.zhang

a mmap issue for videobuf-dma-sg: it will alloc a new page for mmaping
when it encounter page fault at video_vm_ops-fault().

a new implementation for mmap, it translate the vmalloc to page at
video_vm_ops-fault().

Signed-off-by: Figo.zhang figo1...@gmail.com
---
drivers/media/video/videobuf-dma-sg.c |   38

 1 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/drivers/media/video/videobuf-dma-sg.c
b/drivers/media/video/videobuf-dma-sg.c
index 8359e6b..c9a8817 100644
--- a/drivers/media/video/videobuf-dma-sg.c
+++ b/drivers/media/video/videobuf-dma-sg.c
@@ -397,16 +397,44 @@ static void videobuf_vm_close(struct
vm_area_struct *vma)
  */
 static int videobuf_vm_fault(struct vm_area_struct *vma, struct
vm_fault *vmf)
 {
-   struct page *page;
+   struct page *page = NULL;
+   struct videobuf_mapping *map = vma-vm_private_data;
+   struct videobuf_queue *q = map-q;
+   struct videobuf_dma_sg_memory *mem = NULL;
+
+   unsigned long offset;
+   unsigned long page_nr;
+   int first;
 
dprintk(3, fault: fault @ %08lx [vma %08lx-%08lx]\n,
(unsigned long)vmf-virtual_address,
vma-vm_start, vma-vm_end);
 
-   page = alloc_page(GFP_USER | __GFP_DMA32);
-   if (!page)
-   return VM_FAULT_OOM;
-   clear_user_highpage(page, (unsigned long)vmf-virtual_address);
+   mutex_lock(q-vb_lock);
+
+   offset = (unsigned long)vmf-virtual_address - vma-vm_start;
+   page_nr = offset  PAGE_SHIFT;
+
+   for (first = 0; first  VIDEO_MAX_FRAME; first++) {
+   if (NULL == q-bufs[first])
+   continue;
+
+   MAGIC_CHECK(mem-magic, MAGIC_SG_MEM);
+
+   if (q-bufs[first]-map == map)
+   break;
+   }
+
+   mem = q-bufs[first]-priv;
+   if (!mem)
+   return VM_FAULT_SIGBUS;
+   if (mem-dma.vmalloc)
+   page = vmalloc_to_page(mem-dma.vmalloc+
+   (offset  (~PAGE_MASK)));
+   if (mem-dma.pages)
+   page = mem-dma.pages[page_nr];
+   mutex_unlock(q-vb_lock);
+
vmf-page = page;
 
return 0;


--
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


Re: how to mmap in videobuf-dma-sg.c

2010-07-24 Thread Figo.zhang
On Thu, 2009-05-21 at 07:35 -0300, Mauro Carvalho Chehab wrote:
 Em Thu, 21 May 2009 12:46:04 +0800
 Figo.zhang figo1...@gmail.com escreveu:
 
  hi,all,
   I am puzzle that how to mmap ( V4L2_MEMORY_MMAP) in videobuf-dma-sg.c?
  
  In this file, it alloc the momery using vmalloc_32() , and put this
  momery into sglist table,and then use dma_map_sg() to create sg dma at
  __videobuf_iolock() function. but in __videobuf_mmap_mapper(), i canot
  understand how it do the mmap? 
  why it not use the remap_vmalloc_range() to do the mmap?
 
 The answer is simple: remap_vmalloc_range() is newer than videobuf code. This
 part of the code was written back to kernel 2.4, and nobody cared to update it
 to use those newer functions, and simplify its code.
 
 If you want, feel free to propose some cleanups on it
thanks, in __videobuf_mmap_mapper(), it define a videobuf_vm_ops-fault,
it
will alloc a new page for mmaping when it  encounter page fault
(do_page_fault),
so how the mmap() can mmap the vmalloc memory which had allocted before
using __videobuf_iolock()/vmalloc_
32() ?

Thanks,
Figo.zhang

 
 
 
 Cheers,
 Mauro


--
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


[PATCH v3]poll method lose race condition

2009-06-16 Thread Figo.zhang
 bttv-driver.c,cx23885-video.c,cx88-video.c: poll method lose race condition 
for capture video.
 
 In v2, use the clear logic.Thanks to Trent Piepho's help.

 In v3, fix a hold mutex locked issue.

Signed-off-by: Figo.zhang figo1...@gmail.com
--- 
 drivers/media/video/bt8xx/bttv-driver.c |   11 +++
 drivers/media/video/cx23885/cx23885-video.c |   14 ++
 drivers/media/video/cx88/cx88-video.c   |   14 ++
 3 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/drivers/media/video/bt8xx/bttv-driver.c 
b/drivers/media/video/bt8xx/bttv-driver.c
index 23b7499..3688b6e 100644
--- a/drivers/media/video/bt8xx/bttv-driver.c
+++ b/drivers/media/video/bt8xx/bttv-driver.c
@@ -3152,6 +3152,7 @@ static unsigned int bttv_poll(struct file *file, 
poll_table *wait)
struct bttv_fh *fh = file-private_data;
struct bttv_buffer *buf;
enum v4l2_field field;
+   unsigned int rc = POLLERR;
 
if (V4L2_BUF_TYPE_VBI_CAPTURE == fh-type) {
if (!check_alloc_btres(fh-btv,fh,RESOURCE_VBI))
@@ -3160,9 +3161,10 @@ static unsigned int bttv_poll(struct file *file, 
poll_table *wait)
}
 
if (check_btres(fh,RESOURCE_VIDEO_STREAM)) {
+   mutex_lock(fh-cap.vb_lock);
/* streaming capture */
if (list_empty(fh-cap.stream))
-   return POLLERR;
+   goto err;
buf = list_entry(fh-cap.stream.next,struct 
bttv_buffer,vb.stream);
} else {
/* read() capture */
@@ -3191,11 +3193,12 @@ static unsigned int bttv_poll(struct file *file, 
poll_table *wait)
poll_wait(file, buf-vb.done, wait);
if (buf-vb.state == VIDEOBUF_DONE ||
buf-vb.state == VIDEOBUF_ERROR)
-   return POLLIN|POLLRDNORM;
-   return 0;
+   rc =  POLLIN|POLLRDNORM;
+   else
+   rc = 0;
 err:
mutex_unlock(fh-cap.vb_lock);
-   return POLLERR;
+   return rc;
 }
 
 static int bttv_open(struct file *file)
diff --git a/drivers/media/video/cx23885/cx23885-video.c 
b/drivers/media/video/cx23885/cx23885-video.c
index 68068c6..66bbd2e 100644
--- a/drivers/media/video/cx23885/cx23885-video.c
+++ b/drivers/media/video/cx23885/cx23885-video.c
@@ -796,6 +796,7 @@ static unsigned int video_poll(struct file *file,
 {
struct cx23885_fh *fh = file-private_data;
struct cx23885_buffer *buf;
+   unsigned int rc = POLLERR;
 
if (V4L2_BUF_TYPE_VBI_CAPTURE == fh-type) {
if (!res_get(fh-dev, fh, RESOURCE_VBI))
@@ -803,23 +804,28 @@ static unsigned int video_poll(struct file *file,
return videobuf_poll_stream(file, fh-vbiq, wait);
}
 
+   mutex_lock(fh-vidq.vb_lock);
if (res_check(fh, RESOURCE_VIDEO)) {
/* streaming capture */
if (list_empty(fh-vidq.stream))
-   return POLLERR;
+   goto done;
buf = list_entry(fh-vidq.stream.next,
struct cx23885_buffer, vb.stream);
} else {
/* read() capture */
buf = (struct cx23885_buffer *)fh-vidq.read_buf;
if (NULL == buf)
-   return POLLERR;
+   goto done;
}
poll_wait(file, buf-vb.done, wait);
if (buf-vb.state == VIDEOBUF_DONE ||
buf-vb.state == VIDEOBUF_ERROR)
-   return POLLIN|POLLRDNORM;
-   return 0;
+   rc =  POLLIN|POLLRDNORM;
+   else
+   rc = 0;
+done:
+   mutex_unlock(fh-vidq.vb_lock);
+   return rc;
 }
 
 static int video_release(struct file *file)
diff --git a/drivers/media/video/cx88/cx88-video.c 
b/drivers/media/video/cx88/cx88-video.c
index b993d42..97545a0 100644
--- a/drivers/media/video/cx88/cx88-video.c
+++ b/drivers/media/video/cx88/cx88-video.c
@@ -869,6 +869,7 @@ video_poll(struct file *file, struct poll_table_struct 
*wait)
 {
struct cx8800_fh *fh = file-private_data;
struct cx88_buffer *buf;
+   unsigned int rc = POLLERR;
 
if (V4L2_BUF_TYPE_VBI_CAPTURE == fh-type) {
if (!res_get(fh-dev,fh,RESOURCE_VBI))
@@ -876,22 +877,27 @@ video_poll(struct file *file, struct poll_table_struct 
*wait)
return videobuf_poll_stream(file, fh-vbiq, wait);
}
 
+   mutex_lock(fh-vidq.vb_lock);
if (res_check(fh,RESOURCE_VIDEO)) {
/* streaming capture */
if (list_empty(fh-vidq.stream))
-   return POLLERR;
+   goto done;
buf = list_entry(fh-vidq.stream.next,struct 
cx88_buffer,vb.stream);
} else {
/* read() capture */
buf = (struct cx88_buffer*)fh-vidq.read_buf;
if (NULL == buf)
-   return POLLERR;
+   goto done

Re: [PATCH V2] poll method lose race condition

2009-06-15 Thread Figo.zhang

hi Mauro,

 is it ok for this v2?

Best Regards,
Figo.zhang

 2009/6/1 Figo.zhang figo1...@gmail.com
  bttv-driver.c,cx23885-video.c,cx88-video.c: poll method lose
 race condition for capture video.
 
  In v2, use the clear logic.Thanks to Trent Piepho's help.
 
 Signed-off-by: Figo.zhang figo1...@gmail.com
 ---
  drivers/media/video/bt8xx/bttv-driver.c |   20
 
  drivers/media/video/cx23885/cx23885-video.c |   15
 +++
  drivers/media/video/cx88/cx88-video.c   |   15
 +++
  3 files changed, 34 insertions(+), 16 deletions(-)
 
 diff --git a/drivers/media/video/bt8xx/bttv-driver.c
 b/drivers/media/video/bt8xx/bttv-driver.c
 index 23b7499..8d20528 100644
 --- a/drivers/media/video/bt8xx/bttv-driver.c
 +++ b/drivers/media/video/bt8xx/bttv-driver.c
 @@ -3152,6 +3152,7 @@ static unsigned int bttv_poll(struct
 file *file, poll_table *wait)
struct bttv_fh *fh = file-private_data;
struct bttv_buffer *buf;
enum v4l2_field field;
 +   unsigned int rc = POLLERR;
 
if (V4L2_BUF_TYPE_VBI_CAPTURE == fh-type) {
if (!
 check_alloc_btres(fh-btv,fh,RESOURCE_VBI))
 @@ -3160,9 +3161,10 @@ static unsigned int bttv_poll(struct
 file *file, poll_table *wait)
}
 
if (check_btres(fh,RESOURCE_VIDEO_STREAM)) {
 +   mutex_lock(fh-cap.vb_lock);
/* streaming capture */
if (list_empty(fh-cap.stream))
 -   return POLLERR;
 +   return done;
buf = list_entry(fh-cap.stream.next,struct
 bttv_buffer,vb.stream);
} else {
/* read() capture */
 @@ -3170,16 +3172,16 @@ static unsigned int bttv_poll(struct
 file *file, poll_table *wait)
if (NULL == fh-cap.read_buf) {
/* need to capture a new frame */
if
 (locked_btres(fh-btv,RESOURCE_VIDEO_STREAM))
 -   goto err;
 +   goto done;
fh-cap.read_buf =
 videobuf_sg_alloc(fh-cap.msize);
if (NULL == fh-cap.read_buf)
 -   goto err;
 +   goto done;
fh-cap.read_buf-memory =
 V4L2_MEMORY_USERPTR;
field = videobuf_next_field(fh-cap);
if (0 !=
 fh-cap.ops-buf_prepare(fh-cap,fh-cap.read_buf,field)) {
kfree (fh-cap.read_buf);
fh-cap.read_buf = NULL;
 -   goto err;
 +   goto done;
}
 
  fh-cap.ops-buf_queue(fh-cap,fh-cap.read_buf);
fh-cap.read_off = 0;
 @@ -3191,11 +3193,13 @@ static unsigned int bttv_poll(struct
 file *file, poll_table *wait)
poll_wait(file, buf-vb.done, wait);
if (buf-vb.state == VIDEOBUF_DONE ||
buf-vb.state == VIDEOBUF_ERROR)
 -   return POLLIN|POLLRDNORM;
 -   return 0;
 -err:
 +   rc =  POLLIN|POLLRDNORM;
 +   else
 +   rc = 0;
 +
 +done:
mutex_unlock(fh-cap.vb_lock);
 -   return POLLERR;
 +   return rc;
  }
 
  static int bttv_open(struct file *file)
 diff --git a/drivers/media/video/cx23885/cx23885-video.c
 b/drivers/media/video/cx23885/cx23885-video.c
 index 68068c6..f542493 100644
 --- a/drivers/media/video/cx23885/cx23885-video.c
 +++ b/drivers/media/video/cx23885/cx23885-video.c
 @@ -796,6 +796,7 @@ static unsigned int video_poll(struct file
 *file,
  {
struct cx23885_fh *fh = file-private_data;
struct cx23885_buffer *buf;
 +   unsigned int rc = POLLERR;
 
if (V4L2_BUF_TYPE_VBI_CAPTURE == fh-type) {
if (!res_get(fh-dev, fh, RESOURCE_VBI))
 @@ -803,23 +804,29 @@ static unsigned int video_poll(struct
 file *file,
return videobuf_poll_stream(file, fh-vbiq,
 wait);
}
 
 +   mutex_lock(fh-vidq.vb_lock);
if (res_check(fh, RESOURCE_VIDEO

Re: [PATCH] zr364xx.c: vfree does its own NULL check

2009-06-15 Thread Figo.zhang
hi Mauro,
is it ok for this patch?

Best Regards,

Figo.zhang


On Sat, 2009-06-06 at 17:16 +0800, Figo.zhang wrote:
 vfree() does it's own NULL checking, no need for explicit check before
 calling it.
 
 Signed-off-by: Figo.zhang figo1...@gmail.com
 --- 
  drivers/media/video/zr364xx.c |6 --
  1 files changed, 4 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/media/video/zr364xx.c b/drivers/media/video/zr364xx.c
 index ac169c9..fc976f4 100644
 --- a/drivers/media/video/zr364xx.c
 +++ b/drivers/media/video/zr364xx.c
 @@ -882,9 +882,11 @@ static void zr364xx_disconnect(struct usb_interface 
 *intf)
   video_unregister_device(cam-vdev);
   cam-vdev = NULL;
   kfree(cam-buffer);
 - if (cam-framebuf)
 - vfree(cam-framebuf);
 + cam-buffer = NULL;
 + vfree(cam-framebuf);
 + cam-framebuf = NULL;
   kfree(cam);
 + cam = NULL;
  }
  
 
 
 
 
 
 
 

--
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


Re: [PATCH] ov511.c: video_register_device() return zero on success

2009-06-11 Thread Figo.zhang
On Thu, 2009-06-11 at 01:40 -0300, Mauro Carvalho Chehab wrote:
 Em Wed, 10 Jun 2009 22:39:51 -0300
 Mauro Carvalho Chehab mche...@redhat.com escreveu:
 
  Em Sun, 31 May 2009 14:41:52 +0800
  Figo.zhang figo1...@gmail.com escreveu:
  
   video_register_device() return zero on success, it would not return a 
   positive integer.
   
   Signed-off-by: Figo.zhang figo1...@gmail.com
   --- 
drivers/media/video/ov511.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
   
   diff --git a/drivers/media/video/ov511.c b/drivers/media/video/ov511.c
   index 9af5532..816427e 100644
   --- a/drivers/media/video/ov511.c
   +++ b/drivers/media/video/ov511.c
   @@ -5851,7 +5851,7 @@ ov51x_probe(struct usb_interface *intf, const 
   struct usb_device_id *id)
 break;

 if (video_register_device(ov-vdev, VFL_TYPE_GRABBER,
   - unit_video[i]) = 0) {
   + unit_video[i]) == 0) {
 break;
 }
 }
  
  Nack.
  
  Errors are always negative. So, any zero or positive value indicates that 
  no error occurred.
  
  Yet, the logic for forcing ov51x to specific minor number seems broken: it 
  will
  end by registering the device twice, if used.
  
  So, that part of the function needs a rewrite. I'll fix it.
  
 
 Hmm... the issue seems more complex than I've imagined...
 
 When ov511 were written, it was assumed that video_register_device(dev, v, nr)
 would have the following behavior:
 
   if nr = -1, it would do dynamic minor allocation;
   if nr = 0, it would allocate to 'nr' minor or fail.
 
 With the original behavior.
 
 The ov511_probe registering loop is doing something like this (I did a cleanup
 to allow a better understand of the logic):
 
 snip
 #define OV511_MAX_UNIT_VIDEO 16
 ...
 static int unit_video[OV511_MAX_UNIT_VIDEO];
 ...
 module_param_array(unit_video, int, num_uv, 0);
 MODULE_PARM_DESC(unit_video,
   Force use of specific minor number(s). 0 is not allowed.);
 ...
 static int
 ov51x_probe(struct usb_interface *intf, const struct usb_device_id *id)
 {
 ...
 for (i = 0; i  OV511_MAX_UNIT_VIDEO; i++) {
 /* Minor 0 cannot be specified; assume user wants autodetect 
 */
 if (unit_video[i] == 0)
 break;
 
 if (video_register_device(ov-vdev, VFL_TYPE_GRABBER,
 unit_video[i]) = 0)
 goto register_succeded;
 }
 
 /* Use the next available one */
 if (video_register_device(ov-vdev, VFL_TYPE_GRABBER, -1)  0) {
 err(video_register_device failed);
 goto error;
 
 register_succeeded:
 dev_info(intf-dev, Device at %s registered to minor %d\n,
  ov-usb_path, ov-vdev-minor);
 /snip
 
 So, if you probe ov511 with a list of device numbers, for example:
 
 modprobe ov511 4,1,3
 

you means specify the minior video
device: /dev/vidoe4,/dev/video1, /dev/video3 ? 

it maybe : modprobe ov511 unit_video=4,1,3
  
 And assuming that you have 5 ov511 devices, and connect they one by one,
 they'll gain the following device numbers (at the connection order):
 /dev/video4
 /dev/video1
 /dev/video3
 /dev/video0
 /dev/video2
 
 However, changeset 9133:
 
 changeset:   9133:64aed7485a43
 parent:  9073:4db9722caf4f
 user:Hans Verkuil hverk...@xs4all.nl
 date:Sat Oct 04 13:36:54 2008 +0200
 summary: v4l: disconnect kernel number from minor
 
 Changed the behavior video_register_device(dev, v, nr):
 
 + nr = find_next_zero_bit(video_nums[type], minor_cnt, nr == -1 ? 0 : nr);
 
 So, instead of accepting just -1 or nr, it will now do:
   if nr = -1, it will do dynamic minor allocation (as before);
   if nr = 0, it will also do dynamic minor allocation, but starting from 
 nr.
 
 So, the new code won't fail if nr is already allocated, but it will return the
 next unallocated nr.
 
 With the ov511 code, this means that you'll have, instead:
 
 /dev/video5
 /dev/video6
 /dev/video7
 /dev/video8
 /dev/video9
 
 So, changeset 9133 actually broke the ov511 probe original behavior.
 
 In order to restore the original behavior, the probe logic should be replaced
 by something else (like the approach taken by em28xx).
 
 Also, changeset 9133 may also cause other side effects on other drivers that
 were expecting the original behavior.
 
 To make things worse, the function comment doesn't properly explain the 
 current
 behavior.
 
 ---
 
 Figo,
 
 Since we are in the middle of a merge window, it is unlikely that I'll have
 enough time to properly fix it right now (since my priority right now is to
 merge the existing patches).
 
 As you started proposing a patch for it, maybe you could try to fix it and
 check about similar troubles on other drivers.
 
 Cheers,
 Mauro


in v2, if insmod without specify 'unit_video', it use autodetect video device.
if specify the 'unit_video

Re: how to mmap in videobuf-dma-sg.c

2009-06-10 Thread Figo.zhang
On Thu, 2009-05-21 at 18:48 +0800, Figo.zhang wrote:
 On Thu, 2009-05-21 at 07:35 -0300, Mauro Carvalho Chehab wrote:
  Em Thu, 21 May 2009 12:46:04 +0800
  Figo.zhang figo1...@gmail.com escreveu:
  
   hi,all,
I am puzzle that how to mmap ( V4L2_MEMORY_MMAP) in videobuf-dma-sg.c?
   
   In this file, it alloc the momery using vmalloc_32() , and put this
   momery into sglist table,and then use dma_map_sg() to create sg dma at
   __videobuf_iolock() function. but in __videobuf_mmap_mapper(), i canot
   understand how it do the mmap? 
   why it not use the remap_vmalloc_range() to do the mmap?
  
  The answer is simple: remap_vmalloc_range() is newer than videobuf code. 
  This
  part of the code was written back to kernel 2.4, and nobody cared to update 
  it
  to use those newer functions, and simplify its code.
  
  If you want, feel free to propose some cleanups on it
  
  
  
  Cheers,
  Mauro
 
 hi mauro,
 Thank you! 
 But i canot found the similar function code of remap_vmalloc_range() in
 the videobuf-dma-contig.c file. So i want to know the how is work in
 __videobuf_mmap_mapper() function?
 
 

hi mauro:
Thank you. But i still have a puzzle question about mmap() in
videobuf-dma-sg.c. I canot find how to remap the dma buffer
(which alloc by vmalloc_32()) to the vma area in
__videobuf_mmap_mapper()? 

there is my test driver code about dma-sg,it work well. i use
remap_pfn_range() to remap the dma buffer to vma area in mmap method.

so would you like to give me some detail about it?

Best Regards,

Figo.zhang


static int mydev_alloc_dma_sg(struct mydev_device *dev, struct mydev_buf
*buf)
{
int nr_pages;

int i;
struct page *pg;
unsigned char * virt;

nr_pages = mydev_buffer_pages(buf-size);

buf-nr_pages = nr_pages;

dprintk(%s:: buf-nr_pages =%d\n, __func__, buf-nr_pages);

buf-sglist = kcalloc(buf-nr_pages, sizeof(struct scatterlist),
GFP_KERNEL);
if (NULL == buf-sglist)
return NULL;

sg_init_table(buf-sglist, buf-nr_pages);

buf-vmalloc = vmalloc_32(buf-nr_pages  PAGE_SHIFT);

memset(buf-vmalloc,0,buf-nr_pages  PAGE_SHIFT);

virt = buf-vmalloc;

for(i = 0; i buf-nr_pages; i++,virt += PAGE_SIZE){
pg = vmalloc_to_page(virt);
if (NULL == pg)
goto nopage;
BUG_ON(PageHighMem(pg));
sg_set_page(buf-sglist[i], pg, PAGE_SIZE, 0);

}

buf-sglen = dma_map_sg(dev-pci-dev, buf-sglist,
buf-nr_pages, DMA_FROM_DEVICE);

return 0;

 nopage:
dprintk(sgl: oops - no page\n);
kfree(buf-sglist);
return 0;
}

in mmap();

static int do_mmap_sg(struct mydev_device  *dev, struct vm_area_struct *
vma)
{
struct videobuf_mapping *map;
unsigned long pos,page;
unsigned long start = vma-vm_start;
unsigned long size = vma-vm_end - vma-vm_start;
unsigned long offset = vma-vm_pgoff  PAGE_SHIFT;
unsigned int first, last, end;

int retval = -EINVAL;
int i;

/* look for first buffer to map */
for (first = 0; first  VIDEO_MAX_FRAME; first++) {

if (dev-ktbuf[first].boff == (vma-vm_pgoff  PAGE_SHIFT))
break;
}
if (VIDEO_MAX_FRAME == first) {
dprintk(mmap app bug: offset invalid [offset=0x%lx]\n,
(vma-vm_pgoff  PAGE_SHIFT));
goto done;
}
/* create mapping + update buffer list */
retval = -ENOMEM;
map = kmalloc(sizeof(struct videobuf_mapping),GFP_KERNEL);
if (NULL == map)
goto done;

pos = (unsigned long)dev-ktbuf[first].vmalloc;

while (size  0) {
page = vmalloc_to_pfn((void *)pos);
if (remap_pfn_range(vma, start, page, PAGE_SIZE, PAGE_SHARED)) {
return -EAGAIN;
}
start += PAGE_SIZE;
pos += PAGE_SIZE;
if (size  PAGE_SIZE)
size -= PAGE_SIZE;
else
size = 0;
}

map-count= 1;
map-start= vma-vm_start;
map-end  = vma-vm_end;
vma-vm_ops   = mydev_vm_ops;
vma-vm_flags |=/* VM_DONTEXPAND |*/ VM_RESERVED;
vma-vm_flags = ~VM_IO; /* using shared anonymous pages */
vma-vm_private_data = map;

mydev_vm_open(vma);
retval = 0;

 done:
return retval;
}





--
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


[PATCH] zr364xx.c: vfree does its own NULL check

2009-06-06 Thread Figo.zhang
vfree() does it's own NULL checking, no need for explicit check before
calling it.

Signed-off-by: Figo.zhang figo1...@gmail.com
--- 
 drivers/media/video/zr364xx.c |6 --
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/zr364xx.c b/drivers/media/video/zr364xx.c
index ac169c9..fc976f4 100644
--- a/drivers/media/video/zr364xx.c
+++ b/drivers/media/video/zr364xx.c
@@ -882,9 +882,11 @@ static void zr364xx_disconnect(struct usb_interface *intf)
video_unregister_device(cam-vdev);
cam-vdev = NULL;
kfree(cam-buffer);
-   if (cam-framebuf)
-   vfree(cam-framebuf);
+   cam-buffer = NULL;
+   vfree(cam-framebuf);
+   cam-framebuf = NULL;
kfree(cam);
+   cam = NULL;
 }
 
 

 

 
 


--
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


[PATCH]usbvision-core.c: vfree does its own NULL check

2009-06-06 Thread Figo.zhang
vfree() does it's own NULL checking,so no need for check before
calling it.

Signed-off-by: Figo.zhang figo1...@gmail.com
---  
drivers/media/video/usbvision/usbvision-core.c |   14 ++
 1 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/media/video/usbvision/usbvision-core.c 
b/drivers/media/video/usbvision/usbvision-core.c
index 8bc03b9..1603b2b 100644
--- a/drivers/media/video/usbvision/usbvision-core.c
+++ b/drivers/media/video/usbvision/usbvision-core.c
@@ -390,10 +390,9 @@ int usbvision_scratch_alloc(struct usb_usbvision 
*usbvision)
 
 void usbvision_scratch_free(struct usb_usbvision *usbvision)
 {
-   if (usbvision-scratch != NULL) {
-   vfree(usbvision-scratch);
-   usbvision-scratch = NULL;
-   }
+   vfree(usbvision-scratch);
+   usbvision-scratch = NULL;
+   
 }
 
 /*
@@ -506,10 +505,9 @@ int usbvision_decompress_alloc(struct usb_usbvision 
*usbvision)
  */
 void usbvision_decompress_free(struct usb_usbvision *usbvision)
 {
-   if (usbvision-IntraFrameBuffer != NULL) {
-   vfree(usbvision-IntraFrameBuffer);
-   usbvision-IntraFrameBuffer = NULL;
-   }
+   vfree(usbvision-IntraFrameBuffer);
+   usbvision-IntraFrameBuffer = NULL;
+   
 }
 
 /


--
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


Re: [PATCH]videobuf-core.c: add pointer check

2009-06-06 Thread Figo.zhang
On Sun, 2009-06-07 at 09:16 +0800, Figo.zhang wrote:
 On Wed, 2009-06-03 at 10:01 +0800, Figo.zhang wrote:
  add poiter check for videobuf_queue_core_init().
  
  any guys who write a v4l driver, pass a NULL pointer or a non-inintial
  pointer to the first parameter such as videobuf_queue_sg_init() , it 
  would be crashed.
  
  Signed-off-by: Figo.zhang figo1...@x
  --- 
  drivers/media/video/videobuf-core.c |1 +
   1 files changed, 1 insertions(+), 0 deletions(-)
  
  diff --git a/drivers/media/video/videobuf-core.c 
  b/drivers/media/video/videobuf-core.c
  index b7b0584..5f41fd9 100644
  --- a/drivers/media/video/videobuf-core.c
  +++ b/drivers/media/video/videobuf-core.c
  @@ -118,6 +118,7 @@ void videobuf_queue_core_init(struct videobuf_queue *q,
   void *priv,
   struct videobuf_qtype_ops *int_ops)
   {
  +   BUG_ON(!q);
  memset(q, 0, sizeof(*q));
  q-irqlock   = irqlock;
  q-dev   = dev;
  
 
 i do a test driver for it, the main code like this.
 
 struct mydev_dev{
   struct video_device *video_dev;
   ...
   struct videobuf_queue  *cap;
 };
 
 
 
 static int video_open(struct inode *inode, struct file *file)
 {
   ...
   videobuf_queue_dma_contig_init(dev-cap, video_qops,
   dev-pci-dev, dev-slock,
   V4L2_BUF_TYPE_VIDEO_CAPTURE,
   V4L2_FIELD_ALTERNATE,
   sizeof(struct mydev_buf),
   dev);
 
   ...
 }
 
 i pass a non-initial pointer for the first argment, so it crashed.
 
 
 
 BUG: unable to handle kernel NULL pointer dereference at 
 IP: [f8d6bd67] :videobuf_core:videobuf_queue_core_init+0x1b/0xbf
 *pde = 7ed5a067 
 Oops: 0002 [#1] SMP 
 Modules linked in: mydev_drv tvp5160_vd videobuf_dma_contig videobuf_dma_sg
  videobuf_core videocodec videodev v4l2_int_device v4l2_common v4l1_compat
  compat_ioctl32 fuse i915 drm sco bridge stp bnep l2cap bluetooth sunrpc ipv6
 cpufreq_ondemand acpi_cpufreq dm_multipath uinput snd_hda_intel snd_seq_dummy 
 snd_seq_oss snd_seq_midi_event snd_seq snd_seq_device ppdev snd_pcm_oss 
 parport_pc
  snd_mixer_oss snd_pcm snd_timer snd_page_alloc snd_hwdep parport snd 
 i2c_i801 
 i2c_core pcspkr soundcore iTCO_wdt iTCO_vendor_support r8169 mii ftdi_sio 
 usbserial
  ata_generic pata_acpi [last unloaded: microcode]
 
 Pid: 4053, comm: capture Not tainted (2.6.27.5-117.fc10.i686 #1)
 EIP: 0060:[f8d6bd67] EFLAGS: 00210246 CPU: 1
 EIP is at videobuf_queue_core_init+0x1b/0xbf [videobuf_core]
 EAX:  EBX:  ECX: 0036 EDX: 0036
 ESI: f8e1e158 EDI:  EBP: f15f3e28 ESP: f15f3e18
  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
 Process capture (pid: 4053, ti=f15f3000 task=f155 task.ti=f15f3000)
 Stack: f790905c f5fd5224 f15d8840 f5811560 f15f3e48 f8e01163 f5fd5224 
 0001 
0007 006c f5fd5200 f8e0202c f15f3e70 f8e1b5a0 f5fd5224 
 0001 
0007 006c f5fd5200 f8e0faa4  f15d8840 f15f3e88 
 f8e0c195 
 Call Trace:
  [f8e01163] ? videobuf_queue_dma_contig_init+0x1c/0x21 [videobuf_dma_contig]
  [f8e1b5a0] ? video_open+0x8b/0xb1 [kt2741drv]
  [f8e0c195] ? video_open+0xc7/0x125 [videodev]
  [c0492767] ? chrdev_open+0x12b/0x142
  [c048edd2] ? __dentry_open+0x10e/0x1fc
  [c048ef47] ? nameidata_to_filp+0x1f/0x33
  [c049263c] ? chrdev_open+0x0/0x142
  [c0498a3c] ? do_filp_open+0x31c/0x611
  [c048a971] ? virt_to_head_page+0x22/0x2e
  [c041d8ba] ? need_resched+0x18/0x22
  [c048ebf0] ? do_sys_open+0x42/0xb7
  [c048eca7] ? sys_open+0x1e/0x26
  [c0403c76] ? syscall_call+0x7/0xb
  [c06a007b] ? init_intel_cacheinfo+0x0/0x421
  ===
 Code: d8 e8 69 ff ff ff 89 d8 e8 6d a9 93 c7 5b 5d c3 55 89 e5 57 89 c7 56 89
  d6 53 ba 36 00 00 00 83 ec 04 89 c3 89 4d f0 31 c0 89 d1 f3 ab 8b 45 08 8b
  4d f0 89 b3 b8 00 00 00 89 43 10 8b 45 0c 89 
 EIP: [f8d6bd67] videobuf_queue_core_init+0x1b/0xbf [videobuf_core] SS:ESP 
 0068:f15f3e18
 ---[ end trace 4bfe52d17b82af8c ]---
 
 so i think it need to add pointer checking for the first argment of 
 videobuf_queue_core_init(),
 the videobuf code would be more stronger and reliable.
 

hi, Mauro  Hans,

at this point, would you agree with me or would you like to give me some
advice?

Best Regards,

Figo.zhang

--
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


Re: [PATCH]V4L:some v4l drivers have error for video_register_device

2009-06-04 Thread figo.zhang
On Thu, 2009-06-04 at 11:18 +0200, Laurent Pinchart wrote:
 Hi,
 
 On Thursday 04 June 2009 06:20:07 figo.zhang wrote:
  The function video_register_device() will call the
  video_register_device_index(). In this function, firtly it will do some
  argments check , if failed,it will return a negative number such as
  -EINVAL, and then do cdev_alloc() and device_register(), if success return
  zero. so video_register_device_index() canot return a a positive number.
 
  for example, see the drivers/media/video/stk-webcam.c (line 1325):
 
  err = video_register_device(dev-vdev, VFL_TYPE_GRABBER, -1);
  if (err)
  STK_ERROR(v4l registration failed\n);
  else
  STK_INFO(Syntek USB2.0 Camera is now controlling video device
   /dev/video%d\n, dev-vdev.num);
 
  in my opinion, it will be cleaner to do something like this:
 
  err = video_register_device(dev-vdev, VFL_TYPE_GRABBER, -1);
  if (err != 0)
  STK_ERROR(v4l registration failed\n);
  else
  STK_INFO(Syntek USB2.0 Camera is now controlling video device
   /dev/video%d\n, dev-vdev.num);
 
 What's the difference ? (err != 0) and (err) are identical.
 
 Best regards,
 
 Laurent Pinchart

yes, it is the same, but it is easy for reading.

--
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


Re: [PATCH]V4L:some v4l drivers have error for video_register_device

2009-06-04 Thread figo.zhang
On Thu, 2009-06-04 at 11:18 +0200, Laurent Pinchart wrote:
 Hi,
 
 On Thursday 04 June 2009 06:20:07 figo.zhang wrote:
  The function video_register_device() will call the
  video_register_device_index(). In this function, firtly it will do some
  argments check , if failed,it will return a negative number such as
  -EINVAL, and then do cdev_alloc() and device_register(), if success return
  zero. so video_register_device_index() canot return a a positive number.
 
  for example, see the drivers/media/video/stk-webcam.c (line 1325):
 
  err = video_register_device(dev-vdev, VFL_TYPE_GRABBER, -1);
  if (err)
  STK_ERROR(v4l registration failed\n);
  else
  STK_INFO(Syntek USB2.0 Camera is now controlling video device
   /dev/video%d\n, dev-vdev.num);
 
  in my opinion, it will be cleaner to do something like this:
 
  err = video_register_device(dev-vdev, VFL_TYPE_GRABBER, -1);
  if (err != 0)
  STK_ERROR(v4l registration failed\n);
  else
  STK_INFO(Syntek USB2.0 Camera is now controlling video device
   /dev/video%d\n, dev-vdev.num);
 
 What's the difference ? (err != 0) and (err) are identical.
 
 Best regards,
 
 Laurent Pinchart

yes, it is the same, but it is easy for reading.

btw, see driver/media/video/ov511.c (line 5853):
if (video_register_device(ov-vdev, VFL_TYPE_GRABBER,
unit_video[i]) = 0) {
break;
}

it would not return a positive number,i think. pls see my other path:
http://www.spinics.net/lists/linux-media/msg06140.html



--
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


Re: [PATCH]videobuf-core.c: add pointer check

2009-06-04 Thread figo.zhang
On Wed, 2009-06-03 at 10:01 +0800, Figo.zhang wrote:
 add poiter check for videobuf_queue_core_init().
 
 any guys who write a v4l driver, pass a NULL pointer or a non-inintial
 pointer to the first parameter such as videobuf_queue_sg_init() , it 
 would be crashed.
 
 Signed-off-by: Figo.zhang figo1...@gmail.com
 --- 
 drivers/media/video/videobuf-core.c |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/media/video/videobuf-core.c 
 b/drivers/media/video/videobuf-core.c
 index b7b0584..5f41fd9 100644
 --- a/drivers/media/video/videobuf-core.c
 +++ b/drivers/media/video/videobuf-core.c
 @@ -118,6 +118,7 @@ void videobuf_queue_core_init(struct videobuf_queue *q,
void *priv,
struct videobuf_qtype_ops *int_ops)
  {
 + BUG_ON(!q);
   memset(q, 0, sizeof(*q));
   q-irqlock   = irqlock;
   q-dev   = dev;
 

i do a test driver for it, the main code like this.

struct mydev_dev{
struct video_device *video_dev;
...
struct videobuf_queue  *cap;
};



static int video_open(struct inode *inode, struct file *file)
{
...
videobuf_queue_dma_contig_init(dev-cap, video_qops,
dev-pci-dev, dev-slock,
V4L2_BUF_TYPE_VIDEO_CAPTURE,
V4L2_FIELD_ALTERNATE,
sizeof(struct mydev_buf),
dev);

...
}

i pass a non-initial pointer for the first argment, so it crashed.



BUG: unable to handle kernel NULL pointer dereference at 
IP: [f8d6bd67] :videobuf_core:videobuf_queue_core_init+0x1b/0xbf
*pde = 7ed5a067 
Oops: 0002 [#1] SMP 
Modules linked in: mydev_drv tvp5160_vd videobuf_dma_contig videobuf_dma_sg
 videobuf_core videocodec videodev v4l2_int_device v4l2_common v4l1_compat
 compat_ioctl32 fuse i915 drm sco bridge stp bnep l2cap bluetooth sunrpc ipv6
cpufreq_ondemand acpi_cpufreq dm_multipath uinput snd_hda_intel snd_seq_dummy 
snd_seq_oss snd_seq_midi_event snd_seq snd_seq_device ppdev snd_pcm_oss 
parport_pc
 snd_mixer_oss snd_pcm snd_timer snd_page_alloc snd_hwdep parport snd i2c_i801 
i2c_core pcspkr soundcore iTCO_wdt iTCO_vendor_support r8169 mii ftdi_sio 
usbserial
 ata_generic pata_acpi [last unloaded: microcode]

Pid: 4053, comm: capture Not tainted (2.6.27.5-117.fc10.i686 #1)
EIP: 0060:[f8d6bd67] EFLAGS: 00210246 CPU: 1
EIP is at videobuf_queue_core_init+0x1b/0xbf [videobuf_core]
EAX:  EBX:  ECX: 0036 EDX: 0036
ESI: f8e1e158 EDI:  EBP: f15f3e28 ESP: f15f3e18
 DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
Process capture (pid: 4053, ti=f15f3000 task=f155 task.ti=f15f3000)
Stack: f790905c f5fd5224 f15d8840 f5811560 f15f3e48 f8e01163 f5fd5224 0001 
   0007 006c f5fd5200 f8e0202c f15f3e70 f8e1b5a0 f5fd5224 0001 
   0007 006c f5fd5200 f8e0faa4  f15d8840 f15f3e88 f8e0c195 
Call Trace:
 [f8e01163] ? videobuf_queue_dma_contig_init+0x1c/0x21 [videobuf_dma_contig]
 [f8e1b5a0] ? video_open+0x8b/0xb1 [kt2741drv]
 [f8e0c195] ? video_open+0xc7/0x125 [videodev]
 [c0492767] ? chrdev_open+0x12b/0x142
 [c048edd2] ? __dentry_open+0x10e/0x1fc
 [c048ef47] ? nameidata_to_filp+0x1f/0x33
 [c049263c] ? chrdev_open+0x0/0x142
 [c0498a3c] ? do_filp_open+0x31c/0x611
 [c048a971] ? virt_to_head_page+0x22/0x2e
 [c041d8ba] ? need_resched+0x18/0x22
 [c048ebf0] ? do_sys_open+0x42/0xb7
 [c048eca7] ? sys_open+0x1e/0x26
 [c0403c76] ? syscall_call+0x7/0xb
 [c06a007b] ? init_intel_cacheinfo+0x0/0x421
 ===
Code: d8 e8 69 ff ff ff 89 d8 e8 6d a9 93 c7 5b 5d c3 55 89 e5 57 89 c7 56 89
 d6 53 ba 36 00 00 00 83 ec 04 89 c3 89 4d f0 31 c0 89 d1 f3 ab 8b 45 08 8b
 4d f0 89 b3 b8 00 00 00 89 43 10 8b 45 0c 89 
EIP: [f8d6bd67] videobuf_queue_core_init+0x1b/0xbf [videobuf_core] SS:ESP 
0068:f15f3e18
---[ end trace 4bfe52d17b82af8c ]---

so i think it need to add pointer checking for the first argment of 
videobuf_queue_core_init(),
the videobuf code would be more stronger and reliable.

--
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


Re: [PATCH]V4L:some v4l drivers have error for video_register_device

2009-06-04 Thread Figo.zhang
On Thu, 2009-06-04 at 11:27 +0200, Hans Verkuil wrote:
  On Thu, 2009-06-04 at 11:18 +0200, Laurent Pinchart wrote:
  Hi,
 
  On Thursday 04 June 2009 06:20:07 figo.zhang wrote:
   The function video_register_device() will call the
   video_register_device_index(). In this function, firtly it will do
  some
   argments check , if failed,it will return a negative number such as
   -EINVAL, and then do cdev_alloc() and device_register(), if success
  return
   zero. so video_register_device_index() canot return a a positive
  number.
  
   for example, see the drivers/media/video/stk-webcam.c (line 1325):
  
   err = video_register_device(dev-vdev, VFL_TYPE_GRABBER, -1);
if (err)
STK_ERROR(v4l registration failed\n);
else
STK_INFO(Syntek USB2.0 Camera is now controlling video device
 /dev/video%d\n, dev-vdev.num);
  
   in my opinion, it will be cleaner to do something like this:
  
   err = video_register_device(dev-vdev, VFL_TYPE_GRABBER, -1);
if (err != 0)
STK_ERROR(v4l registration failed\n);
else
STK_INFO(Syntek USB2.0 Camera is now controlling video device
 /dev/video%d\n, dev-vdev.num);
 
  What's the difference ? (err != 0) and (err) are identical.
 
  Best regards,
 
  Laurent Pinchart
 
  yes, it is the same, but it is easy for reading.
 
 To be honest, I think '(err)' is easier to read. Unless there is some new
 CodingStyle rule I'm not aware of I see no reason for applying these
 changes.
 
 Regards,
 
 Hans
 

yes, but i found the the kernel code using the '(err != 0) or (err == 0)' is
more popular,in v4l code for example:

v4l1-compat.c  line 507
v4l2-int-device.c  line 52
arv.c   line 333
arv.c   line 844,856

videobuf-core.c  line 529,766,984,1002,1053
videobuf-dma-sg.c  line 211,222,248,350,456,671,

.

so i dont know which style is recommended for kernel code?

Best Regards,

Figo.zhang


--
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


Re: [PATCH]V4L:some v4l drivers have error for video_register_device

2009-06-03 Thread figo.zhang
The function video_register_device() will call the 
video_register_device_index().
In this function, firtly it will do some argments check , if failed,it will 
return a 
negative number such as -EINVAL, and then do cdev_alloc() and 
device_register(), if
success return zero. so video_register_device_index() canot return a a positive 
number.

for example, see the drivers/media/video/stk-webcam.c (line 1325):

err = video_register_device(dev-vdev, VFL_TYPE_GRABBER, -1);
if (err)
STK_ERROR(v4l registration failed\n);
else
STK_INFO(Syntek USB2.0 Camera is now controlling video device
 /dev/video%d\n, dev-vdev.num);

in my opinion, it will be cleaner to do something like this:

err = video_register_device(dev-vdev, VFL_TYPE_GRABBER, -1);
if (err != 0)
STK_ERROR(v4l registration failed\n);
else
STK_INFO(Syntek USB2.0 Camera is now controlling video device
 /dev/video%d\n, dev-vdev.num);



Figo.zhang


On Wed, 2009-05-27 at 11:25 +0800, Figo.zhang wrote:
 For video_register_device(), it return zero means register success.It would 
 be return zero
 or a negative number (on failure),it would not return a positive number.so it 
 have better to
 use this to check err:
   int err = video_register_device(vdev, VFL_TYPE_GRABBER, -1);
   if (err != 0) {
   /*err code*/
   }
 
  
 Signed-off-by: Figo.zhang figo1...@gmail.com
 --- 
  Documentation/video4linux/v4l2-framework.txt |2 +-
  drivers/media/radio/radio-maestro.c  |2 +-
  drivers/media/radio/radio-si470x.c   |2 +-
  drivers/media/video/cafe_ccic.c  |2 +-
  drivers/media/video/cx231xx/cx231xx-video.c  |2 +-
  drivers/media/video/em28xx/em28xx-video.c|2 +-
  drivers/media/video/et61x251/et61x251_core.c |2 +-
  drivers/media/video/hdpvr/hdpvr-video.c  |2 +-
  drivers/media/video/ivtv/ivtv-streams.c  |2 +-
  drivers/media/video/sn9c102/sn9c102_core.c   |2 +-
  drivers/media/video/stk-webcam.c |2 +-
  drivers/media/video/w9968cf.c|2 +-
  drivers/media/video/zc0301/zc0301_core.c |2 +-
  drivers/media/video/zr364xx.c|2 +-
  sound/i2c/other/tea575x-tuner.c  |2 +-
  15 files changed, 15 insertions(+), 15 deletions(-)
 
 diff --git a/Documentation/video4linux/v4l2-framework.txt 
 b/Documentation/video4linux/v4l2-framework.txt
 index 854808b..250232a 100644
 --- a/Documentation/video4linux/v4l2-framework.txt
 +++ b/Documentation/video4linux/v4l2-framework.txt
 @@ -447,7 +447,7 @@ Next you register the video device: this will create the 
 character device
  for you.
  
   err = video_register_device(vdev, VFL_TYPE_GRABBER, -1);
 - if (err) {
 + if (err != 0) {
   video_device_release(vdev); /* or kfree(my_vdev); */
   return err;
   }
 diff --git a/drivers/media/radio/radio-maestro.c 
 b/drivers/media/radio/radio-maestro.c
 index 64d737c..b5e93c2 100644
 --- a/drivers/media/radio/radio-maestro.c
 +++ b/drivers/media/radio/radio-maestro.c
 @@ -379,7 +379,7 @@ static int __devinit maestro_probe(struct pci_dev *pdev,
   video_set_drvdata(dev-vdev, dev);
  
   retval = video_register_device(dev-vdev, VFL_TYPE_RADIO, radio_nr);
 - if (retval) {
 + if (retval != 0) {
   v4l2_err(v4l2_dev, can't register video device!\n);
   goto errfr1;
   }
 diff --git a/drivers/media/radio/radio-si470x.c 
 b/drivers/media/radio/radio-si470x.c
 index bd945d0..edb520a 100644
 --- a/drivers/media/radio/radio-si470x.c
 +++ b/drivers/media/radio/radio-si470x.c
 @@ -1740,7 +1740,7 @@ static int si470x_usb_driver_probe(struct usb_interface 
 *intf,
  
   /* register video device */
   retval = video_register_device(radio-videodev, VFL_TYPE_RADIO, 
 radio_nr);
 - if (retval) {
 + if (retval != 0) {
   printk(KERN_WARNING DRIVER_NAME
   : Could not register video device\n);
   goto err_all;
 diff --git a/drivers/media/video/cafe_ccic.c b/drivers/media/video/cafe_ccic.c
 index c4d181d..fd93698 100644
 --- a/drivers/media/video/cafe_ccic.c
 +++ b/drivers/media/video/cafe_ccic.c
 @@ -1974,7 +1974,7 @@ static int cafe_pci_probe(struct pci_dev *pdev,
  /*   cam-vdev.debug = V4L2_DEBUG_IOCTL_ARG;*/
   cam-vdev.v4l2_dev = cam-v4l2_dev;
   ret = video_register_device(cam-vdev, VFL_TYPE_GRABBER, -1);
 - if (ret)
 + if (ret != 0)
   goto out_smbus;
   video_set_drvdata(cam-vdev, cam);
  
 diff --git a/drivers/media/video/cx231xx/cx231xx-video.c 
 b/drivers/media/video/cx231xx/cx231xx-video.c
 index a23ae73..14e5008 100644
 --- a/drivers/media/video/cx231xx/cx231xx-video.c
 +++ b/drivers/media/video/cx231xx/cx231xx-video.c
 @@ -2382,7 +2382,7 @@ int cx231xx_register_analog_devices(struct cx231xx *dev

[PATCH]videobuf-core.c: add pointer check

2009-06-02 Thread Figo.zhang
add poiter check for videobuf_queue_core_init().

any guys who write a v4l driver, pass a NULL pointer or a non-inintial
pointer to the first parameter such as videobuf_queue_sg_init() , it 
would be crashed.

Signed-off-by: Figo.zhang figo1...@gmail.com
--- 
drivers/media/video/videobuf-core.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/videobuf-core.c 
b/drivers/media/video/videobuf-core.c
index b7b0584..5f41fd9 100644
--- a/drivers/media/video/videobuf-core.c
+++ b/drivers/media/video/videobuf-core.c
@@ -118,6 +118,7 @@ void videobuf_queue_core_init(struct videobuf_queue *q,
 void *priv,
 struct videobuf_qtype_ops *int_ops)
 {
+   BUG_ON(!q);
memset(q, 0, sizeof(*q));
q-irqlock   = irqlock;
q-dev   = dev;


--
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


[PATCH] bttv-driver.c :poll method lose race condition for capture video

2009-05-31 Thread Figo.zhang
bttv-driver.c,cx23885-video.c,cx88-video.c: poll method lose race condition for 
capture video.

Signed-off-by: Figo.zhang figo1...@gmail.com
--- 
 drivers/media/video/bt8xx/bttv-driver.c |7 +--
 drivers/media/video/cx23885/cx23885-video.c |   15 +++
 drivers/media/video/cx88/cx88-video.c   |   13 ++---
 3 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/media/video/bt8xx/bttv-driver.c 
b/drivers/media/video/bt8xx/bttv-driver.c
index 23b7499..9cadfcc 100644
--- a/drivers/media/video/bt8xx/bttv-driver.c
+++ b/drivers/media/video/bt8xx/bttv-driver.c
@@ -3152,6 +3152,7 @@ static unsigned int bttv_poll(struct file *file, 
poll_table *wait)
struct bttv_fh *fh = file-private_data;
struct bttv_buffer *buf;
enum v4l2_field field;
+   unsigned int rc = 0;
 
if (V4L2_BUF_TYPE_VBI_CAPTURE == fh-type) {
if (!check_alloc_btres(fh-btv,fh,RESOURCE_VBI))
@@ -3160,6 +3161,7 @@ static unsigned int bttv_poll(struct file *file, 
poll_table *wait)
}
 
if (check_btres(fh,RESOURCE_VIDEO_STREAM)) {
+   mutex_lock(fh-cap.vb_lock);
/* streaming capture */
if (list_empty(fh-cap.stream))
return POLLERR;
@@ -3191,8 +3193,9 @@ static unsigned int bttv_poll(struct file *file, 
poll_table *wait)
poll_wait(file, buf-vb.done, wait);
if (buf-vb.state == VIDEOBUF_DONE ||
buf-vb.state == VIDEOBUF_ERROR)
-   return POLLIN|POLLRDNORM;
-   return 0;
+   rc = POLLIN|POLLRDNORM;
+   mutex_unlock(fh-cap.vb_lock);
+   return rc;
 err:
mutex_unlock(fh-cap.vb_lock);
return POLLERR;
diff --git a/drivers/media/video/cx23885/cx23885-video.c 
b/drivers/media/video/cx23885/cx23885-video.c
index 68068c6..386ad86 100644
--- a/drivers/media/video/cx23885/cx23885-video.c
+++ b/drivers/media/video/cx23885/cx23885-video.c
@@ -796,6 +796,7 @@ static unsigned int video_poll(struct file *file,
 {
struct cx23885_fh *fh = file-private_data;
struct cx23885_buffer *buf;
+   unsigned int rc = 0;
 
if (V4L2_BUF_TYPE_VBI_CAPTURE == fh-type) {
if (!res_get(fh-dev, fh, RESOURCE_VBI))
@@ -803,23 +804,29 @@ static unsigned int video_poll(struct file *file,
return videobuf_poll_stream(file, fh-vbiq, wait);
}
 
+   mutex_lock(fh-vidq.vb_lock);
if (res_check(fh, RESOURCE_VIDEO)) {
/* streaming capture */
if (list_empty(fh-vidq.stream))
-   return POLLERR;
+   goto err;
buf = list_entry(fh-vidq.stream.next,
struct cx23885_buffer, vb.stream);
} else {
/* read() capture */
buf = (struct cx23885_buffer *)fh-vidq.read_buf;
if (NULL == buf)
-   return POLLERR;
+   goto err;
}
poll_wait(file, buf-vb.done, wait);
if (buf-vb.state == VIDEOBUF_DONE ||
buf-vb.state == VIDEOBUF_ERROR)
-   return POLLIN|POLLRDNORM;
-   return 0;
+   rc = POLLIN|POLLRDNORM;
+   mutex_unlock(fh-vidq.vb_lock);
+   return rc;
+
+err:
+   mutex_unlock(fh-vidq.vb_lock);
+   return POLLERR;
 }
 
 static int video_release(struct file *file)
diff --git a/drivers/media/video/cx88/cx88-video.c 
b/drivers/media/video/cx88/cx88-video.c
index b993d42..f778d15 100644
--- a/drivers/media/video/cx88/cx88-video.c
+++ b/drivers/media/video/cx88/cx88-video.c
@@ -869,6 +869,7 @@ video_poll(struct file *file, struct poll_table_struct 
*wait)
 {
struct cx8800_fh *fh = file-private_data;
struct cx88_buffer *buf;
+   unsigned int rc = 0;
 
if (V4L2_BUF_TYPE_VBI_CAPTURE == fh-type) {
if (!res_get(fh-dev,fh,RESOURCE_VBI))
@@ -876,22 +877,28 @@ video_poll(struct file *file, struct poll_table_struct 
*wait)
return videobuf_poll_stream(file, fh-vbiq, wait);
}
 
+   mutex_lock(fh-vidq.vb_lock);
if (res_check(fh,RESOURCE_VIDEO)) {
/* streaming capture */
if (list_empty(fh-vidq.stream))
-   return POLLERR;
+   goto err;
buf = list_entry(fh-vidq.stream.next,struct 
cx88_buffer,vb.stream);
} else {
/* read() capture */
buf = (struct cx88_buffer*)fh-vidq.read_buf;
if (NULL == buf)
-   return POLLERR;
+   goto err;
}
poll_wait(file, buf-vb.done, wait);
if (buf-vb.state == VIDEOBUF_DONE ||
buf-vb.state == VIDEOBUF_ERROR)
-   return POLLIN|POLLRDNORM;
+   rc = POLLIN|POLLRDNORM;
+   mutex_unlock(fh-vidq.vb_lock);
return 0;
+
+err:
+   mutex_unlock(fh-vidq.vb_lock);
+   return

[PATCH] ov511.c: video_register_device() return zero on success

2009-05-31 Thread Figo.zhang
video_register_device() return zero on success, it would not return a positive 
integer.

Signed-off-by: Figo.zhang figo1...@gmail.com
--- 
 drivers/media/video/ov511.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/ov511.c b/drivers/media/video/ov511.c
index 9af5532..816427e 100644
--- a/drivers/media/video/ov511.c
+++ b/drivers/media/video/ov511.c
@@ -5851,7 +5851,7 @@ ov51x_probe(struct usb_interface *intf, const struct 
usb_device_id *id)
break;
 
if (video_register_device(ov-vdev, VFL_TYPE_GRABBER,
-   unit_video[i]) = 0) {
+   unit_video[i]) == 0) {
break;
}
}


--
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


[PATCH V2] poll method lose race condition

2009-05-31 Thread Figo.zhang
 bttv-driver.c,cx23885-video.c,cx88-video.c: poll method lose race condition 
for capture video.

 In v2, use the clear logic.Thanks to Trent Piepho's help.

Signed-off-by: Figo.zhang figo1...@gmail.com
---
 drivers/media/video/bt8xx/bttv-driver.c |   20 
 drivers/media/video/cx23885/cx23885-video.c |   15 +++
 drivers/media/video/cx88/cx88-video.c   |   15 +++
 3 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/drivers/media/video/bt8xx/bttv-driver.c 
b/drivers/media/video/bt8xx/bttv-driver.c
index 23b7499..8d20528 100644
--- a/drivers/media/video/bt8xx/bttv-driver.c
+++ b/drivers/media/video/bt8xx/bttv-driver.c
@@ -3152,6 +3152,7 @@ static unsigned int bttv_poll(struct file *file, 
poll_table *wait)
struct bttv_fh *fh = file-private_data;
struct bttv_buffer *buf;
enum v4l2_field field;
+   unsigned int rc = POLLERR;
 
if (V4L2_BUF_TYPE_VBI_CAPTURE == fh-type) {
if (!check_alloc_btres(fh-btv,fh,RESOURCE_VBI))
@@ -3160,9 +3161,10 @@ static unsigned int bttv_poll(struct file *file, 
poll_table *wait)
}
 
if (check_btres(fh,RESOURCE_VIDEO_STREAM)) {
+   mutex_lock(fh-cap.vb_lock);
/* streaming capture */
if (list_empty(fh-cap.stream))
-   return POLLERR;
+   return done;
buf = list_entry(fh-cap.stream.next,struct 
bttv_buffer,vb.stream);
} else {
/* read() capture */
@@ -3170,16 +3172,16 @@ static unsigned int bttv_poll(struct file *file, 
poll_table *wait)
if (NULL == fh-cap.read_buf) {
/* need to capture a new frame */
if (locked_btres(fh-btv,RESOURCE_VIDEO_STREAM))
-   goto err;
+   goto done;
fh-cap.read_buf = videobuf_sg_alloc(fh-cap.msize);
if (NULL == fh-cap.read_buf)
-   goto err;
+   goto done;
fh-cap.read_buf-memory = V4L2_MEMORY_USERPTR;
field = videobuf_next_field(fh-cap);
if (0 != 
fh-cap.ops-buf_prepare(fh-cap,fh-cap.read_buf,field)) {
kfree (fh-cap.read_buf);
fh-cap.read_buf = NULL;
-   goto err;
+   goto done;
}
fh-cap.ops-buf_queue(fh-cap,fh-cap.read_buf);
fh-cap.read_off = 0;
@@ -3191,11 +3193,13 @@ static unsigned int bttv_poll(struct file *file, 
poll_table *wait)
poll_wait(file, buf-vb.done, wait);
if (buf-vb.state == VIDEOBUF_DONE ||
buf-vb.state == VIDEOBUF_ERROR)
-   return POLLIN|POLLRDNORM;
-   return 0;
-err:
+   rc =  POLLIN|POLLRDNORM;
+   else 
+   rc = 0;
+   
+done:
mutex_unlock(fh-cap.vb_lock);
-   return POLLERR;
+   return rc;
 }
 
 static int bttv_open(struct file *file)
diff --git a/drivers/media/video/cx23885/cx23885-video.c 
b/drivers/media/video/cx23885/cx23885-video.c
index 68068c6..f542493 100644
--- a/drivers/media/video/cx23885/cx23885-video.c
+++ b/drivers/media/video/cx23885/cx23885-video.c
@@ -796,6 +796,7 @@ static unsigned int video_poll(struct file *file,
 {
struct cx23885_fh *fh = file-private_data;
struct cx23885_buffer *buf;
+   unsigned int rc = POLLERR;
 
if (V4L2_BUF_TYPE_VBI_CAPTURE == fh-type) {
if (!res_get(fh-dev, fh, RESOURCE_VBI))
@@ -803,23 +804,29 @@ static unsigned int video_poll(struct file *file,
return videobuf_poll_stream(file, fh-vbiq, wait);
}
 
+   mutex_lock(fh-vidq.vb_lock);
if (res_check(fh, RESOURCE_VIDEO)) {
/* streaming capture */
if (list_empty(fh-vidq.stream))
-   return POLLERR;
+   goto done;
buf = list_entry(fh-vidq.stream.next,
struct cx23885_buffer, vb.stream);
} else {
/* read() capture */
buf = (struct cx23885_buffer *)fh-vidq.read_buf;
if (NULL == buf)
-   return POLLERR;
+   goto done;
}
poll_wait(file, buf-vb.done, wait);
if (buf-vb.state == VIDEOBUF_DONE ||
buf-vb.state == VIDEOBUF_ERROR)
-   return POLLIN|POLLRDNORM;
-   return 0;
+   rc =  POLLIN|POLLRDNORM;
+   else
+   rc = 0;
+
+done:
+   mutex_unlock(fh-vidq.vb_lock);
+   return rc;
 }
 
 static int video_release(struct file *file)
diff --git a/drivers/media/video/cx88/cx88-video.c 
b/drivers/media/video/cx88/cx88-video.c
index b993d42..b1ca54b 100644
--- a/drivers/media

[PATCH]V4L:some v4l drivers have error for video_register_device

2009-05-26 Thread Figo.zhang
For video_register_device(), it return zero means register success.It would be 
return zero
or a negative number (on failure),it would not return a positive number.so it 
have better to
use this to check err:
int err = video_register_device(vdev, VFL_TYPE_GRABBER, -1);
if (err != 0) {
/*err code*/
}

 
Signed-off-by: Figo.zhang figo1...@gmail.com
--- 
 Documentation/video4linux/v4l2-framework.txt |2 +-
 drivers/media/radio/radio-maestro.c  |2 +-
 drivers/media/radio/radio-si470x.c   |2 +-
 drivers/media/video/cafe_ccic.c  |2 +-
 drivers/media/video/cx231xx/cx231xx-video.c  |2 +-
 drivers/media/video/em28xx/em28xx-video.c|2 +-
 drivers/media/video/et61x251/et61x251_core.c |2 +-
 drivers/media/video/hdpvr/hdpvr-video.c  |2 +-
 drivers/media/video/ivtv/ivtv-streams.c  |2 +-
 drivers/media/video/sn9c102/sn9c102_core.c   |2 +-
 drivers/media/video/stk-webcam.c |2 +-
 drivers/media/video/w9968cf.c|2 +-
 drivers/media/video/zc0301/zc0301_core.c |2 +-
 drivers/media/video/zr364xx.c|2 +-
 sound/i2c/other/tea575x-tuner.c  |2 +-
 15 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/Documentation/video4linux/v4l2-framework.txt 
b/Documentation/video4linux/v4l2-framework.txt
index 854808b..250232a 100644
--- a/Documentation/video4linux/v4l2-framework.txt
+++ b/Documentation/video4linux/v4l2-framework.txt
@@ -447,7 +447,7 @@ Next you register the video device: this will create the 
character device
 for you.
 
err = video_register_device(vdev, VFL_TYPE_GRABBER, -1);
-   if (err) {
+   if (err != 0) {
video_device_release(vdev); /* or kfree(my_vdev); */
return err;
}
diff --git a/drivers/media/radio/radio-maestro.c 
b/drivers/media/radio/radio-maestro.c
index 64d737c..b5e93c2 100644
--- a/drivers/media/radio/radio-maestro.c
+++ b/drivers/media/radio/radio-maestro.c
@@ -379,7 +379,7 @@ static int __devinit maestro_probe(struct pci_dev *pdev,
video_set_drvdata(dev-vdev, dev);
 
retval = video_register_device(dev-vdev, VFL_TYPE_RADIO, radio_nr);
-   if (retval) {
+   if (retval != 0) {
v4l2_err(v4l2_dev, can't register video device!\n);
goto errfr1;
}
diff --git a/drivers/media/radio/radio-si470x.c 
b/drivers/media/radio/radio-si470x.c
index bd945d0..edb520a 100644
--- a/drivers/media/radio/radio-si470x.c
+++ b/drivers/media/radio/radio-si470x.c
@@ -1740,7 +1740,7 @@ static int si470x_usb_driver_probe(struct usb_interface 
*intf,
 
/* register video device */
retval = video_register_device(radio-videodev, VFL_TYPE_RADIO, 
radio_nr);
-   if (retval) {
+   if (retval != 0) {
printk(KERN_WARNING DRIVER_NAME
: Could not register video device\n);
goto err_all;
diff --git a/drivers/media/video/cafe_ccic.c b/drivers/media/video/cafe_ccic.c
index c4d181d..fd93698 100644
--- a/drivers/media/video/cafe_ccic.c
+++ b/drivers/media/video/cafe_ccic.c
@@ -1974,7 +1974,7 @@ static int cafe_pci_probe(struct pci_dev *pdev,
 /* cam-vdev.debug = V4L2_DEBUG_IOCTL_ARG;*/
cam-vdev.v4l2_dev = cam-v4l2_dev;
ret = video_register_device(cam-vdev, VFL_TYPE_GRABBER, -1);
-   if (ret)
+   if (ret != 0)
goto out_smbus;
video_set_drvdata(cam-vdev, cam);
 
diff --git a/drivers/media/video/cx231xx/cx231xx-video.c 
b/drivers/media/video/cx231xx/cx231xx-video.c
index a23ae73..14e5008 100644
--- a/drivers/media/video/cx231xx/cx231xx-video.c
+++ b/drivers/media/video/cx231xx/cx231xx-video.c
@@ -2382,7 +2382,7 @@ int cx231xx_register_analog_devices(struct cx231xx *dev)
/* register v4l2 video video_device */
ret = video_register_device(dev-vdev, VFL_TYPE_GRABBER,
video_nr[dev-devno]);
-   if (ret) {
+   if (ret != 0) {
cx231xx_errdev(unable to register video device (error=%i).\n,
   ret);
return ret;
diff --git a/drivers/media/video/em28xx/em28xx-video.c 
b/drivers/media/video/em28xx/em28xx-video.c
index 882796e..dcc3aca 100644
--- a/drivers/media/video/em28xx/em28xx-video.c
+++ b/drivers/media/video/em28xx/em28xx-video.c
@@ -2013,7 +2013,7 @@ int em28xx_register_analog_devices(struct em28xx *dev)
/* register v4l2 video video_device */
ret = video_register_device(dev-vdev, VFL_TYPE_GRABBER,
   video_nr[dev-devno]);
-   if (ret) {
+   if (ret != 0) {
em28xx_errdev(unable to register video device (error=%i).\n,
  ret);
return ret;
diff --git a/drivers/media/video/et61x251/et61x251_core.c 
b/drivers/media/video/et61x251/et61x251_core.c
index d1c1e45..8a767e1 100644

Re: how to mmap in videobuf-dma-sg.c

2009-05-21 Thread Figo.zhang
On Thu, 2009-05-21 at 07:35 -0300, Mauro Carvalho Chehab wrote:
 Em Thu, 21 May 2009 12:46:04 +0800
 Figo.zhang figo1...@gmail.com escreveu:
 
  hi,all,
   I am puzzle that how to mmap ( V4L2_MEMORY_MMAP) in videobuf-dma-sg.c?
  
  In this file, it alloc the momery using vmalloc_32() , and put this
  momery into sglist table,and then use dma_map_sg() to create sg dma at
  __videobuf_iolock() function. but in __videobuf_mmap_mapper(), i canot
  understand how it do the mmap? 
  why it not use the remap_vmalloc_range() to do the mmap?
 
 The answer is simple: remap_vmalloc_range() is newer than videobuf code. This
 part of the code was written back to kernel 2.4, and nobody cared to update it
 to use those newer functions, and simplify its code.
 
 If you want, feel free to propose some cleanups on it
 
 
 
 Cheers,
 Mauro

hi mauro,
Thank you! 
But i canot found the similar function code of remap_vmalloc_range() in
the videobuf-dma-contig.c file. So i want to know the how is work in
__videobuf_mmap_mapper() function?



--
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


how to mmap in videobuf-dma-sg.c

2009-05-20 Thread Figo.zhang
hi,all,
 I am puzzle that how to mmap ( V4L2_MEMORY_MMAP) in videobuf-dma-sg.c?

In this file, it alloc the momery using vmalloc_32() , and put this
momery into sglist table,and then use dma_map_sg() to create sg dma at
__videobuf_iolock() function. but in __videobuf_mmap_mapper(), i canot
understand how it do the mmap? 
why it not use the remap_vmalloc_range() to do the mmap?

Thanks ,

--
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


Re: [PATCH]saa7134-video.c: poll method lose race condition

2009-05-18 Thread figo.zhang
On Mon, 2009-05-18 at 08:28 +0200, Guennadi Liakhovetski wrote:
 Guennadi

hi,Guennadi,

I am sorry that it is my mistake.

Figo.zhang

--
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


[PATCH v2]saa7134-video.c: poll method lose race condition for capture video

2009-05-18 Thread figo.zhang
saa7134-video.c: poll method lose race condition for capture video.

In v2, when buf == NULL, it will goto err, return  PULLERR
immediately.

Signed-off-by: Figo.zhang figo.zh...@kolorific.com
---  
drivers/media/video/saa7134/saa7134-video.c |9 ++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/media/video/saa7134/saa7134-video.c 
b/drivers/media/video/saa7134/saa7134-video.c
index 493cad9..b1c2dbd 100644
--- a/drivers/media/video/saa7134/saa7134-video.c
+++ b/drivers/media/video/saa7134/saa7134-video.c
@@ -1423,11 +1423,13 @@ video_poll(struct file *file, struct poll_table_struct 
*wait)
 {
struct saa7134_fh *fh = file-private_data;
struct videobuf_buffer *buf = NULL;
+   unsigned int rc = 0;
 
if (V4L2_BUF_TYPE_VBI_CAPTURE == fh-type)
return videobuf_poll_stream(file, fh-vbi, wait);
 
if (res_check(fh,RESOURCE_VIDEO)) {
+   mutex_lock(fh-cap.vb_lock);
if (!list_empty(fh-cap.stream))
buf = list_entry(fh-cap.stream.next, struct 
videobuf_buffer, stream);
} else {
@@ -1446,13 +1448,14 @@ video_poll(struct file *file, struct poll_table_struct 
*wait)
}
 
if (!buf)
-   return POLLERR;
+   goto err;
 
poll_wait(file, buf-done, wait);
if (buf-state == VIDEOBUF_DONE ||
buf-state == VIDEOBUF_ERROR)
-   return POLLIN|POLLRDNORM;
-   return 0;
+   rc =  POLLIN|POLLRDNORM;
+   mutex_unlock(fh-cap.vb_lock);
+   return rc;
 
 err:
mutex_unlock(fh-cap.vb_lock);


--
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


[PATCH]saa7134-video.c: poll method lose race condition

2009-05-17 Thread figo.zhang
saa7134-video.c: poll method lose race condition


Signed-off-by: Figo.zhang figo.zh...@kolorific.com
--- 
drivers/media/video/saa7134/saa7134-video.c |9 ++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/media/video/saa7134/saa7134-video.c 
b/drivers/media/video/saa7134/saa7134-video.c
index 493cad9..95733df 100644
--- a/drivers/media/video/saa7134/saa7134-video.c
+++ b/drivers/media/video/saa7134/saa7134-video.c
@@ -1423,11 +1423,13 @@ video_poll(struct file *file, struct poll_table_struct 
*wait)
 {
struct saa7134_fh *fh = file-private_data;
struct videobuf_buffer *buf = NULL;
+   unsigned int rc = 0;
 
if (V4L2_BUF_TYPE_VBI_CAPTURE == fh-type)
return videobuf_poll_stream(file, fh-vbi, wait);
 
if (res_check(fh,RESOURCE_VIDEO)) {
+   mutex_lock(fh-cap.vb_lock);
if (!list_empty(fh-cap.stream))
buf = list_entry(fh-cap.stream.next, struct 
videobuf_buffer, stream);
} else {
@@ -1446,13 +1448,14 @@ video_poll(struct file *file, struct poll_table_struct 
*wait)
}
 
if (!buf)
-   return POLLERR;
+   rc = POLLERR;
 
poll_wait(file, buf-done, wait);
if (buf-state == VIDEOBUF_DONE ||
buf-state == VIDEOBUF_ERROR)
-   return POLLIN|POLLRDNORM;
-   return 0;
+   rc = POLLIN|POLLRDNORM;
+   mutex_unlock(fh-cap.vb_lock);
+   return rc;
 
 err:
mutex_unlock(fh-cap.vb_lock);


--
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


[PATCH]media/video: minor have assigned value twice

2009-05-17 Thread figo.zhang
The variable minor have assigned value twice, the first time is in the initial 
video_devicedata struct in
those drivers,pls see saa7134-video.c,line 2503.

Signed-off-by: Figo.zhang figo.zh...@kolorific.com
 ---
 drivers/media/video/bt8xx/bttv-driver.c|1 -
 drivers/media/video/cx23885/cx23885-417.c  |1 -
 drivers/media/video/cx88/cx88-core.c   |1 -
 drivers/media/video/saa7134/saa7134-core.c |1 -
 4 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/media/video/bt8xx/bttv-driver.c 
b/drivers/media/video/bt8xx/bttv-driver.c
index 23b7499..539ae45 100644
--- a/drivers/media/video/bt8xx/bttv-driver.c
+++ b/drivers/media/video/bt8xx/bttv-driver.c
@@ -4166,7 +4166,6 @@ static struct video_device *vdev_init(struct bttv *btv,
if (NULL == vfd)
return NULL;
*vfd = *template;
-   vfd-minor   = -1;
vfd-v4l2_dev = btv-c.v4l2_dev;
vfd-release = video_device_release;
vfd-debug   = bttv_debug;
diff --git a/drivers/media/video/cx23885/cx23885-417.c 
b/drivers/media/video/cx23885/cx23885-417.c
index 6f5df90..2943bfd 100644
--- a/drivers/media/video/cx23885/cx23885-417.c
+++ b/drivers/media/video/cx23885/cx23885-417.c
@@ -1742,7 +1742,6 @@ static struct video_device *cx23885_video_dev_alloc(
if (NULL == vfd)
return NULL;
*vfd = *template;
-   vfd-minor   = -1;
snprintf(vfd-name, sizeof(vfd-name), %s %s (%s), dev-name,
type, cx23885_boards[tsport-dev-board].name);
vfd-parent  = pci-dev;
diff --git a/drivers/media/video/cx88/cx88-core.c 
b/drivers/media/video/cx88/cx88-core.c
index 0e149b2..b4049de 100644
--- a/drivers/media/video/cx88/cx88-core.c
+++ b/drivers/media/video/cx88/cx88-core.c
@@ -1010,7 +1010,6 @@ struct video_device *cx88_vdev_init(struct cx88_core 
*core,
if (NULL == vfd)
return NULL;
*vfd = *template;
-   vfd-minor   = -1;
vfd-v4l2_dev = core-v4l2_dev;
vfd-parent = pci-dev;
vfd-release = video_device_release;
diff --git a/drivers/media/video/saa7134/saa7134-core.c 
b/drivers/media/video/saa7134/saa7134-core.c
index 2def6fe..37b1452 100644
--- a/drivers/media/video/saa7134/saa7134-core.c
+++ b/drivers/media/video/saa7134/saa7134-core.c
@@ -775,7 +775,6 @@ static struct video_device *vdev_init(struct saa7134_dev 
*dev,
if (NULL == vfd)
return NULL;
*vfd = *template;
-   vfd-minor   = -1;
vfd-v4l2_dev  = dev-v4l2_dev;
vfd-release = video_device_release;
vfd-debug   = video_debug;


--
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


Re: [PATCH]saa7134-video.c: poll method lose race condition

2009-05-17 Thread figo.zhang
On Mon, 2009-05-18 at 05:49 +0200, hermann pitton wrote:
 Am Montag, den 18.05.2009, 11:53 +0800 schrieb figo.zhang:
  On Mon, 2009-05-18 at 05:07 +0200, hermann pitton wrote:
   Am Montag, den 18.05.2009, 10:13 +0800 schrieb figo.zhang:
saa7134-video.c: poll method lose race condition


Signed-off-by: Figo.zhang figo.zh...@kolorific.com
--- 
drivers/media/video/saa7134/saa7134-video.c |9 ++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/media/video/saa7134/saa7134-video.c 
b/drivers/media/video/saa7134/saa7134-video.c
index 493cad9..95733df 100644
--- a/drivers/media/video/saa7134/saa7134-video.c
+++ b/drivers/media/video/saa7134/saa7134-video.c
@@ -1423,11 +1423,13 @@ video_poll(struct file *file, struct 
poll_table_struct *wait)
 {
struct saa7134_fh *fh = file-private_data;
struct videobuf_buffer *buf = NULL;
+   unsigned int rc = 0;
 
if (V4L2_BUF_TYPE_VBI_CAPTURE == fh-type)
return videobuf_poll_stream(file, fh-vbi, wait);
 
if (res_check(fh,RESOURCE_VIDEO)) {
+   mutex_lock(fh-cap.vb_lock);
if (!list_empty(fh-cap.stream))
buf = list_entry(fh-cap.stream.next, struct 
videobuf_buffer, stream);
} else {
@@ -1446,13 +1448,14 @@ video_poll(struct file *file, struct 
poll_table_struct *wait)
}
 
if (!buf)
-   return POLLERR;
+   rc = POLLERR;
 
poll_wait(file, buf-done, wait);
if (buf-state == VIDEOBUF_DONE ||
buf-state == VIDEOBUF_ERROR)
-   return POLLIN|POLLRDNORM;
-   return 0;
+   rc = POLLIN|POLLRDNORM;
+   mutex_unlock(fh-cap.vb_lock);
+   return rc;
 
 err:
mutex_unlock(fh-cap.vb_lock);


   
   Can you please give some description on what your patch might do
   something?
   
   Or are you a robot?
   
   Then, please give us your serial number, production year, and when we
   can expect you are out of duty and replaced ;)
   
   Cheers,
   Hermann
   
   
   
  
  hi, I just using the saa7134 chip to do a video capture card. The
  saa7134 driver in linux kernel, i found that the poll method have lose 
  race condition for RESOURCE_VIDEO. It have better to add a mutex lock.
  
 
 OK then, but better stay away from the core files.
 
 On what kernel this is?
 
 Give us some copy/paste dmesg output for your card with i2c_scan=1.
 Don't lose tuner stuff on this.
 
 Thanks,
 Hermann
 
 
 
hi, hermann pitton ,i am using the git-kernel, the card have some
hardware problems now.

--
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


[PATCH][RESEND]saa7134-video.c: fix the block bug

2009-05-07 Thread figo.zhang
when re-open or re-start (video_streamon), the q-curr would not be NULL in 
saa7134_buffer_queue(),
and all the qbuf will add to q-queue list,no one to do activate to start 
DMA,and then no interrupt 
would happened,so it will be block. 

In VIDEOBUF_NEEDS_INIT state , inital the curr pointer to be NULL int  the 
buffer_prepare().

Signed-off-by: Figo.zhang figo.zh...@kolorific.com
---
 drivers/media/video/saa7134/saa7134-video.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/saa7134/saa7134-video.c 
b/drivers/media/video/saa7134/saa7134-video.c
index 493cad9..550d6ce 100644
--- a/drivers/media/video/saa7134/saa7134-video.c
+++ b/drivers/media/video/saa7134/saa7134-video.c
@@ -1057,6 +1057,7 @@ static int buffer_prepare(struct videobuf_queue *q,
buf-vb.field  = field;
buf-fmt   = fh-fmt;
buf-pt= fh-pt_cap;
+   dev-video_q.curr = NULL;
 
err = videobuf_iolock(q,buf-vb,dev-ovbuf);
if (err)


--
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


[PATCH]saa7134-video.c: fix the block bug

2009-05-07 Thread figo.zhang
when re-open or re-start (video_streamon), the q-curr would not be NULL in 
saa7134_buffer_queue(),
and all the qbuf will add to q-queue list,no one to do activate to start 
DMA,and then no interrupt 
would happened,so it will be block. 

In VIDEOBUF_NEEDS_INIT state , inital the curr pointer to be NULL int  the 
buffer_prepare().

Signed-off-by: Figo.zhang figo.zh...@kolorific.com
---
 drivers/media/video/saa7134/saa7134-video.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/saa7134/saa7134-video.c 
b/drivers/media/video/saa7134/saa7134-video.c
index 493cad9..550d6ce 100644
--- a/drivers/media/video/saa7134/saa7134-video.c
+++ b/drivers/media/video/saa7134/saa7134-video.c
@@ -1057,6 +1057,7 @@ static int buffer_prepare(struct videobuf_queue *q,
buf-vb.field  = field;
buf-fmt   = fh-fmt;
buf-pt= fh-pt_cap;
+   dev-video_q.curr = NULL;
 
err = videobuf_iolock(q,buf-vb,dev-ovbuf);
if (err)


--
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


[PATCH][RESEND]saa7134-video.c: fix the block bug

2009-05-07 Thread figo.zhang
when re-open or re-start (video_streamon), the q-curr would not be NULL in 
saa7134_buffer_queue(),
and all the qbuf will add to q-queue list,no one to do activate to start 
DMA,and then no interrupt 
would happened,so it will be block. 

In VIDEOBUF_NEEDS_INIT state , initial the curr pointer to be NULL in  the 
buffer_prepare() function.

Signed-off-by: Figo.zhang figo.zh...@kolorific.com
---
 drivers/media/video/saa7134/saa7134-video.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/saa7134/saa7134-video.c 
b/drivers/media/video/saa7134/saa7134-video.c
index 493cad9..550d6ce 100644
--- a/drivers/media/video/saa7134/saa7134-video.c
+++ b/drivers/media/video/saa7134/saa7134-video.c
@@ -1057,6 +1057,7 @@ static int buffer_prepare(struct videobuf_queue *q,
buf-vb.field  = field;
buf-fmt   = fh-fmt;
buf-pt= fh-pt_cap;
+   dev-video_q.curr = NULL;
 
err = videobuf_iolock(q,buf-vb,dev-ovbuf);
if (err)


--
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