| Message ID | 20230327091051.404184-1-hpa@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: v4l2-core: v4l2-ioctl: Printing log with dev_warn() when the pixelformat is unknown | expand |
Hi, On 3/27/23 11:10, Kate Hsuan wrote: > The original implementation used WARN() to notify the user of the "unknown > pixelformat" error. If fmt->description can be found, the function will > simply return without warnings. Recently, the description had removed from > uvc since v4l_fill_fmtdesc() can provide such kind of information. However, > for some of the USB cameras, such as Intel Realsense F200 returns unknown > format when probe(). Thus, the WARN() message shows and confuses the user. > This work changed WARN() to dev_warn() to reduce the severity of the > message. > > The returned video format of Intel RealSense F200 is shown as follows. > kernel: uvcvideo 2-7:1.3: Unknown video format 52564e49-2d90-4a58-920b-773f1f2c556b > kernel: uvcvideo 2-7:1.3: Unknown video format 49524e49-2d90-4a58-920b-773f1f2c556b > kernel: uvcvideo 2-7:1.3: Unknown video format 494c4552-1314-f943-a75a-ee6bbf012e23 > kernel: usb 2-7: Found UVC 1.10 device Intel(R) RealSense(TM) 3D Camera (Front F200) (8086:0a66) > > Link: https://bugzilla.redhat.com/show_bug.cgi?id=2180107 > Signed-off-by: Kate Hsuan <hpa@redhat.com> Thanks, patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > --- > drivers/media/v4l2-core/v4l2-ioctl.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c > index 87f163a89c80..aae987243a5b 100644 > --- a/drivers/media/v4l2-core/v4l2-ioctl.c > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c > @@ -1246,7 +1246,7 @@ static int v4l_enumoutput(const struct v4l2_ioctl_ops *ops, > return ops->vidioc_enum_output(file, fh, p); > } > > -static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) > +static void v4l_fill_fmtdesc(struct video_device *vdev, struct v4l2_fmtdesc *fmt) > { > const unsigned sz = sizeof(fmt->description); > const char *descr = NULL; > @@ -1504,7 +1504,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) > default: > if (fmt->description[0]) > return; > - WARN(1, "Unknown pixelformat 0x%08x\n", fmt->pixelformat); > + dev_warn(&vdev->dev, "Unknown pixelformat 0x%08x\n", fmt->pixelformat); > flags = 0; > snprintf(fmt->description, sz, "%p4cc", > &fmt->pixelformat); > @@ -1589,7 +1589,7 @@ static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops, > break; > } > if (ret == 0) > - v4l_fill_fmtdesc(p); > + v4l_fill_fmtdesc(vdev, p); > return ret; > } >
On 27/03/2023 13:05, Hans de Goede wrote: > Hi, > > On 3/27/23 11:10, Kate Hsuan wrote: >> The original implementation used WARN() to notify the user of the "unknown >> pixelformat" error. If fmt->description can be found, the function will >> simply return without warnings. Recently, the description had removed from >> uvc since v4l_fill_fmtdesc() can provide such kind of information. However, >> for some of the USB cameras, such as Intel Realsense F200 returns unknown >> format when probe(). Thus, the WARN() message shows and confuses the user. >> This work changed WARN() to dev_warn() to reduce the severity of the >> message. >> >> The returned video format of Intel RealSense F200 is shown as follows. >> kernel: uvcvideo 2-7:1.3: Unknown video format 52564e49-2d90-4a58-920b-773f1f2c556b >> kernel: uvcvideo 2-7:1.3: Unknown video format 49524e49-2d90-4a58-920b-773f1f2c556b >> kernel: uvcvideo 2-7:1.3: Unknown video format 494c4552-1314-f943-a75a-ee6bbf012e23 >> kernel: usb 2-7: Found UVC 1.10 device Intel(R) RealSense(TM) 3D Camera (Front F200) (8086:0a66) >> >> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2180107 >> Signed-off-by: Kate Hsuan <hpa@redhat.com> > > Thanks, patch looks good to me: > > Reviewed-by: Hans de Goede <hdegoede@redhat.com> > > Regards, > > Hans > > >> --- >> drivers/media/v4l2-core/v4l2-ioctl.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c >> index 87f163a89c80..aae987243a5b 100644 >> --- a/drivers/media/v4l2-core/v4l2-ioctl.c >> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c >> @@ -1246,7 +1246,7 @@ static int v4l_enumoutput(const struct v4l2_ioctl_ops *ops, >> return ops->vidioc_enum_output(file, fh, p); >> } >> >> -static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) >> +static void v4l_fill_fmtdesc(struct video_device *vdev, struct v4l2_fmtdesc *fmt) >> { >> const unsigned sz = sizeof(fmt->description); >> const char *descr = NULL; >> @@ -1504,7 +1504,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) >> default: >> if (fmt->description[0]) >> return; >> - WARN(1, "Unknown pixelformat 0x%08x\n", fmt->pixelformat); >> + dev_warn(&vdev->dev, "Unknown pixelformat 0x%08x\n", fmt->pixelformat); I don't really like fixing this here. The real cause is in the uvc driver where it doesn't fill in the description for these non-standard formats. A typical driver really should WARN here. >> flags = 0; >> snprintf(fmt->description, sz, "%p4cc", >> &fmt->pixelformat); >> @@ -1589,7 +1589,7 @@ static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops, >> break; >> } >> if (ret == 0) >> - v4l_fill_fmtdesc(p); >> + v4l_fill_fmtdesc(vdev, p); >> return ret; >> } >> > I'll post a new patch fixing this just in uvc. Regards, Hans
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index 87f163a89c80..aae987243a5b 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -1246,7 +1246,7 @@ static int v4l_enumoutput(const struct v4l2_ioctl_ops *ops, return ops->vidioc_enum_output(file, fh, p); } -static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) +static void v4l_fill_fmtdesc(struct video_device *vdev, struct v4l2_fmtdesc *fmt) { const unsigned sz = sizeof(fmt->description); const char *descr = NULL; @@ -1504,7 +1504,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) default: if (fmt->description[0]) return; - WARN(1, "Unknown pixelformat 0x%08x\n", fmt->pixelformat); + dev_warn(&vdev->dev, "Unknown pixelformat 0x%08x\n", fmt->pixelformat); flags = 0; snprintf(fmt->description, sz, "%p4cc", &fmt->pixelformat); @@ -1589,7 +1589,7 @@ static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops, break; } if (ret == 0) - v4l_fill_fmtdesc(p); + v4l_fill_fmtdesc(vdev, p); return ret; }
The original implementation used WARN() to notify the user of the "unknown pixelformat" error. If fmt->description can be found, the function will simply return without warnings. Recently, the description had removed from uvc since v4l_fill_fmtdesc() can provide such kind of information. However, for some of the USB cameras, such as Intel Realsense F200 returns unknown format when probe(). Thus, the WARN() message shows and confuses the user. This work changed WARN() to dev_warn() to reduce the severity of the message. The returned video format of Intel RealSense F200 is shown as follows. kernel: uvcvideo 2-7:1.3: Unknown video format 52564e49-2d90-4a58-920b-773f1f2c556b kernel: uvcvideo 2-7:1.3: Unknown video format 49524e49-2d90-4a58-920b-773f1f2c556b kernel: uvcvideo 2-7:1.3: Unknown video format 494c4552-1314-f943-a75a-ee6bbf012e23 kernel: usb 2-7: Found UVC 1.10 device Intel(R) RealSense(TM) 3D Camera (Front F200) (8086:0a66) Link: https://bugzilla.redhat.com/show_bug.cgi?id=2180107 Signed-off-by: Kate Hsuan <hpa@redhat.com> --- drivers/media/v4l2-core/v4l2-ioctl.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)