Hi Laurent,
On 18/09/18 11:35, Laurent Pinchart wrote:
> Adding device context to the kernel log messages make them more useful.
> Add new uvcg_* macros based on dev_*() that print both the gadget device
> name and the function name.
>
> While at it, remove a commented out printk statement and an unused
> printk-based macro.
>
> Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Great - looks good to me.
Reviewed-by: Kieran Bingham <kieran.bing...@ideasonboard.com>
> ---
> drivers/usb/gadget/function/f_uvc.c | 41
> ++++++++++++++-------------------
> drivers/usb/gadget/function/uvc.h | 16 ++++++-------
> drivers/usb/gadget/function/uvc_v4l2.c | 4 ++--
> drivers/usb/gadget/function/uvc_video.c | 18 +++++++++------
> drivers/usb/gadget/function/uvc_video.h | 2 +-
> 5 files changed, 39 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_uvc.c
> b/drivers/usb/gadget/function/f_uvc.c
> index 4ea987741e6e..0cc4a6220050 100644
> --- a/drivers/usb/gadget/function/f_uvc.c
> +++ b/drivers/usb/gadget/function/f_uvc.c
> @@ -232,13 +232,8 @@ uvc_function_setup(struct usb_function *f, const struct
> usb_ctrlrequest *ctrl)
> struct v4l2_event v4l2_event;
> struct uvc_event *uvc_event = (void *)&v4l2_event.u.data;
>
> - /* printk(KERN_INFO "setup request %02x %02x value %04x index %04x
> %04x\n",
> - * ctrl->bRequestType, ctrl->bRequest, le16_to_cpu(ctrl->wValue),
> - * le16_to_cpu(ctrl->wIndex), le16_to_cpu(ctrl->wLength));
> - */
> -
> if ((ctrl->bRequestType & USB_TYPE_MASK) != USB_TYPE_CLASS) {
> - INFO(f->config->cdev, "invalid request type\n");
> + uvcg_info(f, "invalid request type\n");
> return -EINVAL;
> }
>
> @@ -272,7 +267,7 @@ uvc_function_get_alt(struct usb_function *f, unsigned
> interface)
> {
> struct uvc_device *uvc = to_uvc(f);
>
> - INFO(f->config->cdev, "uvc_function_get_alt(%u)\n", interface);
> + uvcg_info(f, "%s(%u)\n", __func__, interface);
>
> if (interface == uvc->control_intf)
> return 0;
> @@ -291,13 +286,13 @@ uvc_function_set_alt(struct usb_function *f, unsigned
> interface, unsigned alt)
> struct uvc_event *uvc_event = (void *)&v4l2_event.u.data;
> int ret;
>
> - INFO(cdev, "uvc_function_set_alt(%u, %u)\n", interface, alt);
> + uvcg_info(f, "%s(%u, %u)\n", __func__, interface, alt);
>
> if (interface == uvc->control_intf) {
> if (alt)
> return -EINVAL;
>
> - INFO(cdev, "reset UVC Control\n");
> + uvcg_info(f, "reset UVC Control\n");
> usb_ep_disable(uvc->control_ep);
>
> if (!uvc->control_ep->desc)
> @@ -348,7 +343,7 @@ uvc_function_set_alt(struct usb_function *f, unsigned
> interface, unsigned alt)
> if (!uvc->video.ep)
> return -EINVAL;
>
> - INFO(cdev, "reset UVC\n");
> + uvcg_info(f, "reset UVC\n");
> usb_ep_disable(uvc->video.ep);
>
> ret = config_ep_by_speed(f->config->cdev->gadget,
> @@ -373,7 +368,7 @@ uvc_function_disable(struct usb_function *f)
> struct uvc_device *uvc = to_uvc(f);
> struct v4l2_event v4l2_event;
>
> - INFO(f->config->cdev, "uvc_function_disable\n");
> + uvcg_info(f, "%s()\n", __func__);
>
> memset(&v4l2_event, 0, sizeof(v4l2_event));
> v4l2_event.type = UVC_EVENT_DISCONNECT;
> @@ -392,21 +387,19 @@ uvc_function_disable(struct usb_function *f)
> void
> uvc_function_connect(struct uvc_device *uvc)
> {
> - struct usb_composite_dev *cdev = uvc->func.config->cdev;
> int ret;
>
> if ((ret = usb_function_activate(&uvc->func)) < 0)
> - INFO(cdev, "UVC connect failed with %d\n", ret);
> + uvcg_info(&uvc->func, "UVC connect failed with %d\n", ret);
> }
>
> void
> uvc_function_disconnect(struct uvc_device *uvc)
> {
> - struct usb_composite_dev *cdev = uvc->func.config->cdev;
> int ret;
>
> if ((ret = usb_function_deactivate(&uvc->func)) < 0)
> - INFO(cdev, "UVC disconnect failed with %d\n", ret);
> + uvcg_info(&uvc->func, "UVC disconnect failed with %d\n", ret);
> }
>
> /* --------------------------------------------------------------------------
> @@ -605,7 +598,7 @@ uvc_function_bind(struct usb_configuration *c, struct
> usb_function *f)
> struct f_uvc_opts *opts;
> int ret = -EINVAL;
>
> - INFO(cdev, "uvc_function_bind\n");
> + uvcg_info(f, "%s()\n", __func__);
>
> opts = fi_to_f_uvc_opts(f->fi);
> /* Sanity check the streaming endpoint module parameters.
> @@ -618,8 +611,8 @@ uvc_function_bind(struct usb_configuration *c, struct
> usb_function *f)
> if (opts->streaming_maxburst &&
> (opts->streaming_maxpacket % 1024) != 0) {
> opts->streaming_maxpacket = roundup(opts->streaming_maxpacket,
> 1024);
> - INFO(cdev, "overriding streaming_maxpacket to %d\n",
> - opts->streaming_maxpacket);
> + uvcg_info(f, "overriding streaming_maxpacket to %d\n",
> + opts->streaming_maxpacket);
> }
>
> /* Fill in the FS/HS/SS Video Streaming specific descriptors from the
> @@ -658,7 +651,7 @@ uvc_function_bind(struct usb_configuration *c, struct
> usb_function *f)
> /* Allocate endpoints. */
> ep = usb_ep_autoconfig(cdev->gadget, &uvc_control_ep);
> if (!ep) {
> - INFO(cdev, "Unable to allocate control EP\n");
> + uvcg_info(f, "Unable to allocate control EP\n");
> goto error;
> }
> uvc->control_ep = ep;
> @@ -672,7 +665,7 @@ uvc_function_bind(struct usb_configuration *c, struct
> usb_function *f)
> ep = usb_ep_autoconfig(cdev->gadget, &uvc_fs_streaming_ep);
>
> if (!ep) {
> - INFO(cdev, "Unable to allocate streaming EP\n");
> + uvcg_info(f, "Unable to allocate streaming EP\n");
> goto error;
> }
> uvc->video.ep = ep;
> @@ -745,19 +738,19 @@ uvc_function_bind(struct usb_configuration *c, struct
> usb_function *f)
> uvc->control_req->context = uvc;
>
> if (v4l2_device_register(&cdev->gadget->dev, &uvc->v4l2_dev)) {
> - printk(KERN_INFO "v4l2_device_register failed\n");
> + uvcg_err(f, "failed to register V4L2 device\n");
> goto error;
> }
>
> /* Initialise video. */
> - ret = uvcg_video_init(&uvc->video);
> + ret = uvcg_video_init(&uvc->video, uvc);
> if (ret < 0)
> goto error;
>
> /* Register a V4L2 device. */
> ret = uvc_register_video(uvc);
> if (ret < 0) {
> - printk(KERN_INFO "Unable to register video device\n");
> + uvcg_err(f, "failed to register video device\n");
> goto error;
> }
>
> @@ -894,7 +887,7 @@ static void uvc_unbind(struct usb_configuration *c,
> struct usb_function *f)
> struct usb_composite_dev *cdev = c->cdev;
> struct uvc_device *uvc = to_uvc(f);
>
> - INFO(cdev, "%s\n", __func__);
> + uvcg_info(f, "%s\n", __func__);
>
> device_remove_file(&uvc->vdev.dev, &dev_attr_function_name);
> video_unregister_device(&uvc->vdev);
> diff --git a/drivers/usb/gadget/function/uvc.h
> b/drivers/usb/gadget/function/uvc.h
> index 93cf78b420fe..099d650082e5 100644
> --- a/drivers/usb/gadget/function/uvc.h
> +++ b/drivers/usb/gadget/function/uvc.h
> @@ -24,6 +24,7 @@
> struct usb_ep;
> struct usb_request;
> struct uvc_descriptor_header;
> +struct uvc_device;
>
> /* ------------------------------------------------------------------------
> * Debugging, printing and logging
> @@ -51,14 +52,12 @@ extern unsigned int uvc_gadget_trace_param;
> printk(KERN_DEBUG "uvcvideo: " msg); \
> } while (0)
>
> -#define uvc_warn_once(dev, warn, msg...) \
> - do { \
> - if (!test_and_set_bit(warn, &dev->warnings)) \
> - printk(KERN_INFO "uvcvideo: " msg); \
> - } while (0)
> -
> -#define uvc_printk(level, msg...) \
> - printk(level "uvcvideo: " msg)
> +#define uvcg_dbg(f, fmt, args...) \
> + dev_dbg(&(f)->config->cdev->gadget->dev, "%s: " fmt, (f)->name, ##args)
> +#define uvcg_info(f, fmt, args...) \
> + dev_info(&(f)->config->cdev->gadget->dev, "%s: " fmt, (f)->name, ##args)
> +#define uvcg_err(f, fmt, args...) \
> + dev_err(&(f)->config->cdev->gadget->dev, "%s: " fmt, (f)->name, ##args)
>
> /* ------------------------------------------------------------------------
> * Driver specific constants
> @@ -73,6 +72,7 @@ extern unsigned int uvc_gadget_trace_param;
> */
>
> struct uvc_video {
> + struct uvc_device *uvc;
> struct usb_ep *ep;
>
> /* Frame parameters */
> diff --git a/drivers/usb/gadget/function/uvc_v4l2.c
> b/drivers/usb/gadget/function/uvc_v4l2.c
> index 7f1ca3b57823..a1183eccee22 100644
> --- a/drivers/usb/gadget/function/uvc_v4l2.c
> +++ b/drivers/usb/gadget/function/uvc_v4l2.c
> @@ -115,8 +115,8 @@ uvc_v4l2_set_format(struct file *file, void *fh, struct
> v4l2_format *fmt)
> }
>
> if (i == ARRAY_SIZE(uvc_formats)) {
> - printk(KERN_INFO "Unsupported format 0x%08x.\n",
> - fmt->fmt.pix.pixelformat);
> + uvcg_info(&uvc->func, "Unsupported format 0x%08x.\n",
> + fmt->fmt.pix.pixelformat);
> return -EINVAL;
> }
>
> diff --git a/drivers/usb/gadget/function/uvc_video.c
> b/drivers/usb/gadget/function/uvc_video.c
> index 2c9821ec836e..5c042f380708 100644
> --- a/drivers/usb/gadget/function/uvc_video.c
> +++ b/drivers/usb/gadget/function/uvc_video.c
> @@ -131,7 +131,9 @@ static int uvcg_video_ep_queue(struct uvc_video *video,
> struct usb_request *req)
>
> ret = usb_ep_queue(video->ep, req, GFP_ATOMIC);
> if (ret < 0) {
> - printk(KERN_INFO "Failed to queue request (%d).\n", ret);
> + uvcg_err(&video->uvc->func, "Failed to queue request (%d).\n",
> + ret);
> +
> /* Isochronous endpoints can't be halted. */
> if (usb_endpoint_xfer_bulk(video->ep->desc))
> usb_ep_set_halt(video->ep);
> @@ -184,13 +186,14 @@ uvc_video_complete(struct usb_ep *ep, struct
> usb_request *req)
> break;
>
> case -ESHUTDOWN: /* disconnect from host. */
> - printk(KERN_DEBUG "VS request cancelled.\n");
> + uvcg_dbg(&video->uvc->func, "VS request cancelled.\n");
> uvcg_queue_cancel(queue, 1);
> goto requeue;
>
> default:
> - printk(KERN_INFO "VS request completed with status %d.\n",
> - req->status);
> + uvcg_info(&video->uvc->func,
> + "VS request completed with status %d.\n",
> + req->status);
> uvcg_queue_cancel(queue, 0);
> goto requeue;
> }
> @@ -354,8 +357,8 @@ int uvcg_video_enable(struct uvc_video *video, int enable)
> int ret;
>
> if (video->ep == NULL) {
> - printk(KERN_INFO "Video enable failed, device is "
> - "uninitialized.\n");
> + uvcg_info(&video->uvc->func,
> + "Video enable failed, device is uninitialized.\n");
> return -ENODEV;
> }
>
> @@ -387,11 +390,12 @@ int uvcg_video_enable(struct uvc_video *video, int
> enable)
> /*
> * Initialize the UVC video stream.
> */
> -int uvcg_video_init(struct uvc_video *video)
> +int uvcg_video_init(struct uvc_video *video, struct uvc_device *uvc)
> {
> INIT_LIST_HEAD(&video->req_free);
> spin_lock_init(&video->req_lock);
>
> + video->uvc = uvc;
> video->fcc = V4L2_PIX_FMT_YUYV;
> video->bpp = 16;
> video->width = 320;
> diff --git a/drivers/usb/gadget/function/uvc_video.h
> b/drivers/usb/gadget/function/uvc_video.h
> index 7d77122b0ff9..278dc52c7604 100644
> --- a/drivers/usb/gadget/function/uvc_video.h
> +++ b/drivers/usb/gadget/function/uvc_video.h
> @@ -18,6 +18,6 @@ int uvcg_video_pump(struct uvc_video *video);
>
> int uvcg_video_enable(struct uvc_video *video, int enable);
>
> -int uvcg_video_init(struct uvc_video *video);
> +int uvcg_video_init(struct uvc_video *video, struct uvc_device *uvc);
>
> #endif /* __UVC_VIDEO_H__ */
>
--
Regards
--
Kieran
