[PATCH]v4l: list entries no need to check
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
于 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
于 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.
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
于 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.
于 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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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]videobuf-core.c: add pointer check
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
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
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
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
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
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
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
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
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
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
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
how to mmap in videobuf-dma-sg.c
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
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
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
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
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
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
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
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
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
