Hi Andrzej,
Thank you for the patch.
On Friday 28 February 2014 10:32:24 Andrzej Pietrasiewicz wrote:
> When configfs support is integrated the future uvc function
> module must not take any parameters. Move parameters to
> webcam.
This looks good to me, although the uvc_bind_config() function now takes quite
a lot of parameters. As you remove it later in the series that's not a big
issue. Please see below for a couple of small comments.
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxx>
> ---
> drivers/usb/gadget/f_uvc.c | 30 ++++++++++++------------------
> drivers/usb/gadget/f_uvc.h | 6 +++++-
> drivers/usb/gadget/webcam.c | 30 +++++++++++++++++++++++++++---
> 3 files changed, 44 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/usb/gadget/f_uvc.c b/drivers/usb/gadget/f_uvc.c
> index ff4340a..6eb5f06 100644
> --- a/drivers/usb/gadget/f_uvc.c
> +++ b/drivers/usb/gadget/f_uvc.c
> @@ -29,21 +29,9 @@
> #include "uvc.h"
>
> unsigned int uvc_gadget_trace_param;
> -
> -/*-------------------------------------------------------------------------
> */ -
> -/* module parameters specific to the Video streaming endpoint */
> -static unsigned int streaming_interval = 1;
> -module_param(streaming_interval, uint, S_IRUGO|S_IWUSR);
> -MODULE_PARM_DESC(streaming_interval, "1 - 16");
> -
> -static unsigned int streaming_maxpacket = 1024;
> -module_param(streaming_maxpacket, uint, S_IRUGO|S_IWUSR);
> -MODULE_PARM_DESC(streaming_maxpacket, "1 - 1023 (FS), 1 - 3072 (hs/ss)");
> -
> -static unsigned int streaming_maxburst;
> -module_param(streaming_maxburst, uint, S_IRUGO|S_IWUSR);
> -MODULE_PARM_DESC(streaming_maxburst, "0 - 15 (ss only)");
> +unsigned int streaming_interval;
> +unsigned int streaming_maxpacket;
> +unsigned int streaming_maxburst;
>
> /*
> --------------------------------------------------------------------------
> * Function descriptors
> @@ -748,7 +736,11 @@ uvc_bind_config(struct usb_configuration *c,
> const struct uvc_descriptor_header * const *ss_control,
> const struct uvc_descriptor_header * const *fs_streaming,
> const struct uvc_descriptor_header * const *hs_streaming,
> - const struct uvc_descriptor_header * const *ss_streaming)
> + const struct uvc_descriptor_header * const *ss_streaming,
> + unsigned int streaming_interval_webcam,
> + unsigned int streaming_maxpacket_webcam,
> + unsigned int streaming_maxburst_webcam,
> + unsigned int uvc_gadget_trace_webcam)
Although this gets removed later in the series, could you shorten the
parameter names by removing the _webcam suffix ?
> {
> struct uvc_device *uvc;
> int ret = 0;
> @@ -786,6 +778,10 @@ uvc_bind_config(struct usb_configuration *c,
> ss_streaming[0]->bDescriptorSubType != UVC_VS_INPUT_HEADER)
> goto error;
>
> + streaming_interval = streaming_interval_webcam;
> + streaming_maxpacket = streaming_maxpacket_webcam;
> + streaming_maxburst = streaming_maxburst_webcam;
> + uvc_gadget_trace_param = uvc_gadget_trace_webcam;
> uvc->desc.fs_control = fs_control;
> uvc->desc.ss_control = ss_control;
> uvc->desc.fs_streaming = fs_streaming;
> @@ -830,6 +826,4 @@ error:
> return ret;
> }
>
> -module_param_named(trace, uvc_gadget_trace_param, uint, S_IRUGO|S_IWUSR);
> -MODULE_PARM_DESC(trace, "Trace level bitmask");
>
> diff --git a/drivers/usb/gadget/f_uvc.h b/drivers/usb/gadget/f_uvc.h
> index ec52752..74b9602 100644
> --- a/drivers/usb/gadget/f_uvc.h
> +++ b/drivers/usb/gadget/f_uvc.h
> @@ -21,7 +21,11 @@ int uvc_bind_config(struct usb_configuration *c,
> const struct uvc_descriptor_header * const *hs_control,
> const struct uvc_descriptor_header * const *fs_streaming,
> const struct uvc_descriptor_header * const *hs_streaming,
> - const struct uvc_descriptor_header * const *ss_streaming);
> + const struct uvc_descriptor_header * const *ss_streaming,
> + unsigned int streaming_interval_webcam,
> + unsigned int streaming_maxpacket_webcam,
> + unsigned int streaming_maxburst_webcam,
> + unsigned int uvc_gadget_trace_webcam);
>
> #endif /* _F_UVC_H_ */
>
> diff --git a/drivers/usb/gadget/webcam.c b/drivers/usb/gadget/webcam.c
> index 8cef1e6..52789de 100644
> --- a/drivers/usb/gadget/webcam.c
> +++ b/drivers/usb/gadget/webcam.c
> @@ -29,6 +29,28 @@
> #include "f_uvc.c"
>
> USB_GADGET_COMPOSITE_OPTIONS();
> +
> +/*-------------------------------------------------------------------------
> */ +
> +/* module parameters specific to the Video streaming endpoint */
> +static unsigned int streaming_interval_webcam = 1;
> +module_param_named(streaming_interval, streaming_interval_webcam, uint,
> + S_IRUGO|S_IWUSR);
> +MODULE_PARM_DESC(streaming_interval, "1 - 16");
> +
> +static unsigned int streaming_maxpacket_webcam = 1024;
> +module_param_named(streaming_maxpacket, streaming_maxpacket_webcam, uint,
> + S_IRUGO|S_IWUSR);
> +MODULE_PARM_DESC(streaming_maxpacket, "1 - 1023 (FS), 1 - 3072 (hs/ss)");
> +
> +static unsigned int streaming_maxburst_webcam;
> +module_param_named(streaming_maxburst, streaming_maxburst_webcam, uint,
> + S_IRUGO|S_IWUSR);
> +MODULE_PARM_DESC(streaming_maxburst, "0 - 15 (ss only)");
> +
> +unsigned int uvc_gadget_trace_param_webcam;
> +module_param_named(trace, uvc_gadget_trace_param_webcam, uint,
> S_IRUGO|S_IWUSR); +MODULE_PARM_DESC(trace, "Trace level bitmask");
Could you remove the _webcam suffixes here as well ?
> /*
> --------------------------------------------------------------------------
> * Device descriptor
> */
> @@ -326,9 +348,11 @@ static const struct uvc_descriptor_header * const
> uvc_ss_streaming_cls[] = { static int __init
> webcam_config_bind(struct usb_configuration *c)
> {
> - return uvc_bind_config(c, uvc_fs_control_cls, uvc_ss_control_cls,
> - uvc_fs_streaming_cls, uvc_hs_streaming_cls,
> - uvc_ss_streaming_cls);
> + return uvc_bind_config(c, uvc_fs_control_cls,
> + uvc_ss_control_cls, uvc_fs_streaming_cls,
> + uvc_hs_streaming_cls, uvc_ss_streaming_cls,
> + streaming_interval_webcam, streaming_maxpacket_webcam,
> + streaming_maxburst_webcam, uvc_gadget_trace_param_webcam);
> }
>
> static struct usb_configuration webcam_config_driver = {
--
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
