Hi Michel and Laurent,
> Hi Laurent,
>
> On Fri, Feb 08, 2013 at 12:32:44AM +0100, Laurent Pinchart wrote:
> > Hi Bhupesh,
> >
> > On Thursday 07 February 2013 23:39:07 Laurent Pinchart wrote:
> > > On Wednesday 30 January 2013 15:56:52 Bhupesh SHARMA wrote:
> > > > On Monday, January 21, 2013 6:16 PM Laurent Pinchart wrote:
> > > > > On Friday 18 January 2013 20:46:59 Felipe Balbi wrote:
> > > > > > On Thu, Jan 17, 2013 at 04:23:48PM +0530, Bhupesh Sharma wrote:
> > > > > > > This patchset tries to enhance the UVC webcam gadget driver
> > > > > > > and is based on Laurent's git tree available here (head uvc-
> gadget):
> > > > > > > git://linuxtv.org/pinchartl/uvcvideo.git
> > > > > > >
> > > > > > > Note that to ease review and integration of these patches, I
> > > > > > > have rebased them on Laurent's repo and all the relevant
> > > > > > > patches after review can be pushed in Felipe's repo in one go.
> > > > > > >
> > > > > > > The patches 3/5 and 4/5 in this patchset try to handle all
> > > > > > > the review comments received on the following UVC gadget
> > > > > > > related
> > > > > > > patches:
> > > > > > >
> > > > > > > [PATCH V2 1/2] usb: gadget/uvc: Port UVC webcam gadget to
> > > > > > > use
> > > > > > > videobuf2 framework [PATCH V2 2/2] usb: gadget/uvc: Add
> > > > > > > support for 'USB_GADGET_DELAYED_STATUS' response for a
> > > > > > > set_intf(alt-set 1) command
> > > > > > >
> > > > > > > which can be viewed here:
> > > > > > > [1] http://www.spinics.net/lists/linux-usb/msg68297.html
> > > > > > > [2] http://www.spinics.net/lists/linux-usb/msg68298.html
> > > > > > >
> > > > > > > I have tested this patchset on a super-speed compliant USB
> > > > > > > device controller (DWC3), with the VIVI capture device
> > > > > > > acting as a dummy source of video data and I also have modified
> the 'uvc-gadget'
> > > > > > > application written by Laurent (original application available
> > > > > > > here:
> > > > > > > http://git.ideasonboard.org/uvc-gadget.git) for testing the
> > > > > > > complete flow from V4L2 to UVC domain and vice versa.
> > > > > >
> > > > > > Laurent, do you wanna queue this yourself or should I take it ?
> > > > >
> > > > > I was away last week, please give me a couple of days to review
> > > > > the patches. I'll take them in my tree with the other UVC gadget
> > > > > patches I have and I'll send you a pull request.
> > > >
> > > > Did you get any time to review this patchset?
> > >
> > > Sorry for the delay. I'm reviewing the patches now.
> > >
> > > I've rebased my uvcvideo-gadget branch on top of v3.8-rc6 and
> > > resolved the conflicts.
> > >
> > > As your 1/5 "usb: gadget/uvc: Fix VS_INPUT_HEADER.bEndpointAddress
> > > and Video Streaming.bEndpointAddress values" patch fixes a bug
> introduced in "usb:
> > > gadget/uvc: Configure the streaming endpoint based on the speed", do
> > > you mind if I squash it with that commit to avoid breaking bisection ?
> >
> > I've reviewed patches 1/5, 2/5, 4/5 and 5/5. I will handle patch 3 tomorrow.
>
> Beside my two comments on patch 3 i have tested it with the contig allocator
> instead of vmalloc [¹]. With that changes applied i can give my Tested-By if
> desired.
@ Michael, sure I can apply your comments and also Laurent's and then probably
you can add your Tested-By.
@ Laurent, would that be fine?
Regards,
Bhupesh
> [¹]
> ---
> drivers/usb/gadget/f_uvc.c | 11 ++++++++++-
> drivers/usb/gadget/uvc.h | 1 +
> drivers/usb/gadget/uvc_queue.c | 7 ++++---
> 3 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/gadget/f_uvc.c b/drivers/usb/gadget/f_uvc.c index
> 22f7570..0de0399 100644
> --- a/drivers/usb/gadget/f_uvc.c
> +++ b/drivers/usb/gadget/f_uvc.c
> @@ -19,11 +19,11 @@
> #include <linux/usb/ch9.h>
> #include <linux/usb/gadget.h>
> #include <linux/usb/video.h>
> -#include <linux/vmalloc.h>
> #include <linux/wait.h>
>
> #include <media/v4l2-dev.h>
> #include <media/v4l2-event.h>
> +#include <media/videobuf2-dma-contig.h>
>
> #include "uvc.h"
>
> @@ -601,6 +601,8 @@ uvc_function_unbind(struct usb_configuration *c,
> struct usb_function *f)
> kfree(uvc->control_buf);
> }
>
> + vb2_dma_contig_cleanup_ctx(uvc->ctx);
> +
> kfree(f->descriptors);
> kfree(f->hs_descriptors);
> kfree(f->ss_descriptors);
> @@ -618,6 +620,13 @@ uvc_function_bind(struct usb_configuration *c,
> struct usb_function *f)
>
> INFO(cdev, "uvc_function_bind\n");
>
> + cdev->gadget->dev.coherent_dma_mask = 0xffffffff;
> + uvc->ctx = vb2_dma_contig_init_ctx(&cdev->gadget->dev);
> + if (IS_ERR(uvc->ctx)) {
> + printk(KERN_ERR "Failed to alloc vb2 context\n");
> + goto error;
> + }
> +
> /* sanity check the streaming endpoint module parameters */
> if (streaming_interval < 1)
> streaming_interval = 1;
> diff --git a/drivers/usb/gadget/uvc.h b/drivers/usb/gadget/uvc.h index
> 9391993..56a1550 100644
> --- a/drivers/usb/gadget/uvc.h
> +++ b/drivers/usb/gadget/uvc.h
> @@ -150,6 +150,7 @@ struct uvc_device
> enum uvc_state state;
> struct usb_function func;
> struct uvc_video video;
> + void *ctx;
>
> /* Descriptors */
> struct {
> diff --git a/drivers/usb/gadget/uvc_queue.c
> b/drivers/usb/gadget/uvc_queue.c index bd20fab..6b6240f 100644
> --- a/drivers/usb/gadget/uvc_queue.c
> +++ b/drivers/usb/gadget/uvc_queue.c
> @@ -17,10 +17,9 @@
> #include <linux/module.h>
> #include <linux/usb.h>
> #include <linux/videodev2.h>
> -#include <linux/vmalloc.h>
> #include <linux/wait.h>
>
> -#include <media/videobuf2-vmalloc.h>
> +#include <media/videobuf2-dma-contig.h>
>
> #include "uvc.h"
>
> @@ -46,6 +45,7 @@ static int uvc_queue_setup(struct vb2_queue *vq,
> const struct v4l2_format *fmt, {
> struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
> struct uvc_video *video = container_of(queue, struct uvc_video,
> queue);
> + struct uvc_device *uvc = container_of(video, struct uvc_device,
> +video);
>
> if (*nbuffers > UVC_MAX_VIDEO_BUFFERS)
> *nbuffers = UVC_MAX_VIDEO_BUFFERS;
> @@ -53,6 +53,7 @@ static int uvc_queue_setup(struct vb2_queue *vq,
> const struct v4l2_format *fmt,
> *nplanes = 1;
>
> sizes[0] = video->imagesize;
> + alloc_ctxs[0] = uvc->ctx;
>
> return 0;
> }
> @@ -119,7 +120,7 @@ static int uvc_queue_init(struct uvc_video_queue
> *queue,
> queue->queue.drv_priv = queue;
> queue->queue.buf_struct_size = sizeof(struct uvc_buffer);
> queue->queue.ops = &uvc_queue_qops;
> - queue->queue.mem_ops = &vb2_vmalloc_memops;
> + queue->queue.mem_ops = &vb2_dma_contig_memops;
> ret = vb2_queue_init(&queue->queue);
> if (ret)
> return ret;
>
>
> --
> Pengutronix e.K. | |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
