Re: [PATCH 3/3] usb: gadget: uvc: Replace plain printk() with dev_*()
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
Great - looks good to me.
Reviewed-by: Kieran Bingham
> ---
> 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 *)_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 *)_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(_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(>func)) < 0)
> - INFO(cdev, "UVC connect failed with %d\n", ret);
> + uvcg_info(>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(>func)) < 0)
> - INFO(cdev, "UVC disconnect failed with %d\n", ret);
> + uvcg_info(>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
[PATCH 3/3] usb: gadget: uvc: Replace plain printk() with dev_*()
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
---
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 *)_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 *)_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(_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(>func)) < 0)
- INFO(cdev, "UVC connect failed with %d\n", ret);
+ uvcg_info(>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(>func)) < 0)
- INFO(cdev, "UVC disconnect failed with %d\n", ret);
+ uvcg_info(>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
