Hi Andrzej,
Thank you for the patch.
On Friday 28 February 2014 10:32:25 Andrzej Pietrasiewicz wrote:
> Compile uvc_queue, uvc_v4l2, uvc_video separately so that later they can
> be all combined in a separately compiled f_uvc.
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxx>
> ---
> drivers/usb/gadget/Makefile | 2 +-
> drivers/usb/gadget/f_uvc.c | 2 ++
> drivers/usb/gadget/f_uvc.h | 8 ++++++++
> drivers/usb/gadget/uvc.h | 1 +
> drivers/usb/gadget/uvc_queue.c | 42 ++++++++++++++++++---------------------
> drivers/usb/gadget/uvc_queue.h | 35 +++++++++++++++++++++++++++++++++++
> drivers/usb/gadget/uvc_v4l2.c | 4 +++-
> drivers/usb/gadget/uvc_v4l2.h | 21 +++++++++++++++++++++
> drivers/usb/gadget/uvc_video.c | 10 ++++------
> drivers/usb/gadget/uvc_video.h | 24 ++++++++++++++++++++++++
> drivers/usb/gadget/webcam.c | 6 +-----
> 11 files changed, 120 insertions(+), 35 deletions(-)
> create mode 100644 drivers/usb/gadget/uvc_v4l2.h
> create mode 100644 drivers/usb/gadget/uvc_video.h
>
> diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
> index 5f150bc..f73b48c 100644
> --- a/drivers/usb/gadget/Makefile
> +++ b/drivers/usb/gadget/Makefile
> @@ -81,7 +81,7 @@ g_multi-y := multi.o
> g_hid-y := hid.o
> g_dbgp-y := dbgp.o
> g_nokia-y := nokia.o
> -g_webcam-y := webcam.o
> +g_webcam-y := webcam.o uvc_queue.o uvc_v4l2.o uvc_video.o
> g_ncm-y := ncm.o
> g_acm_ms-y := acm_ms.o
> g_tcm_usb_gadget-y := tcm_usb_gadget.o
> diff --git a/drivers/usb/gadget/f_uvc.c b/drivers/usb/gadget/f_uvc.c
> index 6eb5f06..bd47af4 100644
> --- a/drivers/usb/gadget/f_uvc.c
> +++ b/drivers/usb/gadget/f_uvc.c
> @@ -27,6 +27,8 @@
> #include <media/v4l2-event.h>
>
> #include "uvc.h"
> +#include "uvc_video.h"
> +#include "uvc_v4l2.h"
Could you please move uvc_v4l2.h before uvc_video.h to keep the headers sorted
alphabetically ?
> unsigned int uvc_gadget_trace_param;
> unsigned int streaming_interval;
> diff --git a/drivers/usb/gadget/f_uvc.h b/drivers/usb/gadget/f_uvc.h
> index 74b9602..71b38dd 100644
> --- a/drivers/usb/gadget/f_uvc.h
> +++ b/drivers/usb/gadget/f_uvc.h
> @@ -16,6 +16,14 @@
> #include <linux/usb/composite.h>
> #include <linux/usb/video.h>
>
> +#include "uvc.h"
> +
> +void uvc_function_setup_continue(struct uvc_device *uvc);
> +
> +void uvc_function_connect(struct uvc_device *uvc);
> +
> +void uvc_function_disconnect(struct uvc_device *uvc);
> +
> int uvc_bind_config(struct usb_configuration *c,
> const struct uvc_descriptor_header * const *fs_control,
> const struct uvc_descriptor_header * const *hs_control,
> diff --git a/drivers/usb/gadget/uvc.h b/drivers/usb/gadget/uvc.h
> index 7a9111d..33e82b9 100644
> --- a/drivers/usb/gadget/uvc.h
> +++ b/drivers/usb/gadget/uvc.h
> @@ -58,6 +58,7 @@ struct uvc_event
> #include <linux/version.h>
> #include <media/v4l2-fh.h>
> #include <media/v4l2-device.h>
> +#include <linux/usb/composite.h>
Could you please move this line up to keep the headers sorted alphabetically ?
> #include "uvc_queue.h"
>
> diff --git a/drivers/usb/gadget/uvc_queue.c b/drivers/usb/gadget/uvc_queue.c
> index 0bb5d50..aa25186 100644
> --- a/drivers/usb/gadget/uvc_queue.c
> +++ b/drivers/usb/gadget/uvc_queue.c
> @@ -125,8 +125,7 @@ static struct vb2_ops uvc_queue_qops = {
> .wait_finish = uvc_wait_finish,
> };
>
> -static int uvc_queue_init(struct uvc_video_queue *queue,
> - enum v4l2_buf_type type)
> +int uvc_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type)
This will cause a conflict with the uvcvideo host driver which defines a
function using the same name. Several functions below will cause the same
conflict. Could you please add a patch before this one to rename all those
functions ? You could use a uvc_gadget_ prefix, although that might be a bit
long. uvcg_, uvc_g_ or g_uvc_ are options I would be fine with.
> {
> int ret;
>
[snip]
> diff --git a/drivers/usb/gadget/uvc_queue.h b/drivers/usb/gadget/uvc_queue.h
> index 8e76ce9..68e1f39 100644
> --- a/drivers/usb/gadget/uvc_queue.h
> +++ b/drivers/usb/gadget/uvc_queue.h
> @@ -57,6 +57,41 @@ static inline int uvc_queue_streaming(struct
> uvc_video_queue *queue) return vb2_is_streaming(&queue->queue);
> }
>
> +int uvc_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type);
> +
> +void uvc_free_buffers(struct uvc_video_queue *queue);
> +
> +int uvc_alloc_buffers(struct uvc_video_queue *queue,
> + struct v4l2_requestbuffers *rb);
> +
> +int uvc_query_buffer(struct uvc_video_queue *queue,
> + struct v4l2_buffer *buf);
> +
> +int uvc_queue_buffer(struct uvc_video_queue *queue,
> + struct v4l2_buffer *buf);
> +
> +int uvc_dequeue_buffer(struct uvc_video_queue *queue,
> + struct v4l2_buffer *buf, int nonblocking);
> +
> +unsigned int uvc_queue_poll(struct uvc_video_queue *queue,
> + struct file *file, poll_table *wait);
> +
> +int uvc_queue_mmap(struct uvc_video_queue *queue, struct vm_area_struct
> *vma); +
> +#ifndef CONFIG_MMU
> +unsigned long uvc_queue_get_unmapped_area(struct uvc_video_queue *queue,
> + unsigned long pgoff);
> +#endif /* CONFIG_MMU */
> +
> +void uvc_queue_cancel(struct uvc_video_queue *queue, int disconnect);
> +
> +int uvc_queue_enable(struct uvc_video_queue *queue, int enable);
> +
> +struct uvc_buffer *uvc_queue_next_buffer(struct uvc_video_queue *queue,
> + struct uvc_buffer *buf);
> +
> +struct uvc_buffer *uvc_queue_head(struct uvc_video_queue *queue);
> +
> #endif /* __KERNEL__ */
>
> #endif /* _UVC_QUEUE_H_ */
> diff --git a/drivers/usb/gadget/uvc_v4l2.c b/drivers/usb/gadget/uvc_v4l2.c
> index ad48e81..48774f3 100644
> --- a/drivers/usb/gadget/uvc_v4l2.c
> +++ b/drivers/usb/gadget/uvc_v4l2.c
> @@ -24,7 +24,9 @@
> #include <media/v4l2-ioctl.h>
>
> #include "uvc.h"
> +#include "f_uvc.h"
> #include "uvc_queue.h"
> +#include "uvc_video.h"
Here again, could you please sort the four new headers alphabetically ?
>
> /*
> --------------------------------------------------------------------------
> * Requests handling
> @@ -351,7 +353,7 @@ static unsigned long uvc_v4l2_get_unmapped_area(struct
> file *file, }
> #endif
>
> -static struct v4l2_file_operations uvc_v4l2_fops = {
> +struct v4l2_file_operations uvc_v4l2_fops = {
> .owner = THIS_MODULE,
> .open = uvc_v4l2_open,
> .release = uvc_v4l2_release,
> diff --git a/drivers/usb/gadget/uvc_v4l2.h b/drivers/usb/gadget/uvc_v4l2.h
> new file mode 100644
> index 0000000..ebcb1d1
> --- /dev/null
> +++ b/drivers/usb/gadget/uvc_v4l2.h
> @@ -0,0 +1,21 @@
> +/*
> + * uvc_v4l2.h -- USB Video Class Gadget driver
> + *
> + * Copyright (C) 2009-2010
> + * Laurent Pinchart (laurent.pinchart@xxxxxxxxxxxxxxxx)
> + *
> + * Copyright (c) 2013 Samsung Electronics Co., Ltd.
2014 ?
> + * http://www.samsung.com
> + * Author: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxx>
> + *
> + * 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.
> + */
> +
> +#ifndef __UVC__V4L2__H__
> +#define __UVC__V4L2__H__
Nitpicking, starting and ending the header guard macros with a double
underscore is pretty common, but it's the first time I see them being used
across the whole macro (same comment for uvc_video.h).
> +
> +extern struct v4l2_file_operations uvc_v4l2_fops;
> +
> +#endif /* __UVC__V4L2__H__ */
> diff --git a/drivers/usb/gadget/uvc_video.c b/drivers/usb/gadget/uvc_video.c
> index 71e896d..2f6e31e 100644
> --- a/drivers/usb/gadget/uvc_video.c
> +++ b/drivers/usb/gadget/uvc_video.c
> @@ -15,6 +15,7 @@
> #include <linux/errno.h>
> #include <linux/usb/ch9.h>
> #include <linux/usb/gadget.h>
> +#include <linux/usb/video.h>
>
> #include <media/v4l2-dev.h>
>
> @@ -278,8 +279,7 @@ error:
> * This function fills the available USB requests (listed in req_free) with
> * video data from the queued buffers.
> */
> -static int
> -uvc_video_pump(struct uvc_video *video)
> +int uvc_video_pump(struct uvc_video *video)
> {
> struct usb_request *req;
> struct uvc_buffer *buf;
> @@ -336,8 +336,7 @@ uvc_video_pump(struct uvc_video *video)
> /*
> * Enable or disable the video stream.
> */
> -static int
> -uvc_video_enable(struct uvc_video *video, int enable)
> +int uvc_video_enable(struct uvc_video *video, int enable)
> {
> unsigned int i;
> int ret;
> @@ -375,8 +374,7 @@ uvc_video_enable(struct uvc_video *video, int enable)
> /*
> * Initialize the UVC video stream.
> */
> -static int
> -uvc_video_init(struct uvc_video *video)
> +int uvc_video_init(struct uvc_video *video)
> {
> INIT_LIST_HEAD(&video->req_free);
> spin_lock_init(&video->req_lock);
> diff --git a/drivers/usb/gadget/uvc_video.h b/drivers/usb/gadget/uvc_video.h
> new file mode 100644
> index 0000000..2283780
> --- /dev/null
> +++ b/drivers/usb/gadget/uvc_video.h
> @@ -0,0 +1,24 @@
> +/*
> + * uvc_video.h -- USB Video Class Gadget driver
> + *
> + * Copyright (C) 2009-2010
> + * Laurent Pinchart (laurent.pinchart@xxxxxxxxxxxxxxxx)
> + *
> + * Copyright (c) 2013 Samsung Electronics Co., Ltd.
2014 as well ?
> + * http://www.samsung.com
> + * Author: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxx>
> + *
> + * 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.
> + */
> +#ifndef __UVC__VIDEO__H__
> +#define __UVC__VIDEO__H__
> +
> +int uvc_video_pump(struct uvc_video *video);
> +
> +int uvc_video_enable(struct uvc_video *video, int enable);
> +
> +int uvc_video_init(struct uvc_video *video);
> +
> +#endif /* __UVC__VIDEO__H__ */
> diff --git a/drivers/usb/gadget/webcam.c b/drivers/usb/gadget/webcam.c
> index 52789de..dfc9729 100644
> --- a/drivers/usb/gadget/webcam.c
> +++ b/drivers/usb/gadget/webcam.c
> @@ -12,10 +12,9 @@
>
> #include <linux/kernel.h>
> #include <linux/device.h>
> +#include <linux/module.h>
> #include <linux/usb/video.h>
>
> -#include "f_uvc.h"
> -
> /*
> * Kbuild is not very cooperative with respect to linking separately
> * compiled library objects into one module. So for now we won't use
> @@ -23,9 +22,6 @@
> * the runtime footprint, and giving us at least some parts of what
> * a "gcc --combine ... part1.c part2.c part3.c ... " build would.
> */
> -#include "uvc_queue.c"
> -#include "uvc_video.c"
> -#include "uvc_v4l2.c"
> #include "f_uvc.c"
>
> USB_GADGET_COMPOSITE_OPTIONS();
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
