* [PATCH v7 0/8] uvcvideo: Fix handling of power_line_frequency
@ 2022-06-17 10:36 Ricardo Ribalda
2022-06-17 10:36 ` [PATCH v7 1/8] media: uvcvideo: Add missing value for power_line_frequency Ricardo Ribalda
` (7 more replies)
0 siblings, 8 replies; 22+ messages in thread
From: Ricardo Ribalda @ 2022-06-17 10:36 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, linux-media,
linux-kernel, tfiga, senozhatsky, yunkec
Cc: Ricardo Ribalda
Hello,
This series is a new version of "[PATCH v3 0/7] uvcvideo: Fix handling
of power_line_frequency", with an attempt to generalize the
UVC_QUIRK_LIMITED_POWERLINE quirk that it introduced and turn it into a
control mappings override mechanism.
The goal is still to support the UVC 1.5 power line frequency control
extra option (patch 1/7), and work around an issue with devices that do
not implement support for disabling the power line frequency (patches
2/7 to 7/7).
Changelog v7:
- Support minimum for V4L2_CTRL_TYPE_MENU
Fix uvc_query_v4l2_menu
Changelog v6:
- Add support for per-device control mapping overrides
Fix invalid memory access
- Support minimum for V4L2_CTRL_TYPE_MENU
New patch
- Limit power line control for Quanta UVC Webcam
Fix id
Ricardo Ribalda (8):
media: uvcvideo: Add missing value for power_line_frequency
media: uvcvideo: Add support for per-device control mapping overrides
media: uvcvideo: Support minimum for V4L2_CTRL_TYPE_MENU
media: uvcvideo: Limit power line control for Quanta UVC Webcam
media: uvcvideo: Limit power line control for Chicony Easycamera
media: uvcvideo: Limit power line control for Chicony Easycamera
media: uvcvideo: Limit power line control for Quanta cameras
media: uvcvideo: Limit power line control for Acer EasyCamera
drivers/media/usb/uvc/uvc_ctrl.c | 90 ++++++++++++++++++++++++------
drivers/media/usb/uvc/uvc_driver.c | 89 +++++++++++++++++++++++++++++
drivers/media/usb/uvc/uvcvideo.h | 2 +
3 files changed, 165 insertions(+), 16 deletions(-)
--
2.36.1.476.g0c4daa206d-goog
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v7 1/8] media: uvcvideo: Add missing value for power_line_frequency
2022-06-17 10:36 [PATCH v7 0/8] uvcvideo: Fix handling of power_line_frequency Ricardo Ribalda
@ 2022-06-17 10:36 ` Ricardo Ribalda
2022-06-17 10:36 ` [PATCH v7 2/8] media: uvcvideo: Add support for per-device control mapping overrides Ricardo Ribalda
` (6 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Ricardo Ribalda @ 2022-06-17 10:36 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, linux-media,
linux-kernel, tfiga, senozhatsky, yunkec
Cc: Ricardo Ribalda
UVC 1.5 class defines 4 values for this control on:
4.2.2.3.6 Power Line Frequency Control
Add the missing value when the UVC version is 1.5.
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/usb/uvc/uvc_ctrl.c | 56 +++++++++++++++++++++++++-------
1 file changed, 45 insertions(+), 11 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 0e78233fc8a0..a709ebbb4d69 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -366,6 +366,7 @@ static const struct uvc_menu_info power_line_frequency_controls[] = {
{ 0, "Disabled" },
{ 1, "50 Hz" },
{ 2, "60 Hz" },
+ { 3, "Auto" },
};
static const struct uvc_menu_info exposure_auto_controls[] = {
@@ -504,17 +505,6 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
.v4l2_type = V4L2_CTRL_TYPE_INTEGER,
.data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
},
- {
- .id = V4L2_CID_POWER_LINE_FREQUENCY,
- .entity = UVC_GUID_UVC_PROCESSING,
- .selector = UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
- .size = 2,
- .offset = 0,
- .v4l2_type = V4L2_CTRL_TYPE_MENU,
- .data_type = UVC_CTRL_DATA_TYPE_ENUM,
- .menu_info = power_line_frequency_controls,
- .menu_count = ARRAY_SIZE(power_line_frequency_controls),
- },
{
.id = V4L2_CID_HUE_AUTO,
.entity = UVC_GUID_UVC_PROCESSING,
@@ -730,6 +720,34 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
},
};
+static const struct uvc_control_mapping uvc_ctrl_mappings_uvc11[] = {
+ {
+ .id = V4L2_CID_POWER_LINE_FREQUENCY,
+ .entity = UVC_GUID_UVC_PROCESSING,
+ .selector = UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
+ .size = 2,
+ .offset = 0,
+ .v4l2_type = V4L2_CTRL_TYPE_MENU,
+ .data_type = UVC_CTRL_DATA_TYPE_ENUM,
+ .menu_info = power_line_frequency_controls,
+ .menu_count = ARRAY_SIZE(power_line_frequency_controls) - 1,
+ },
+};
+
+static const struct uvc_control_mapping uvc_ctrl_mappings_uvc15[] = {
+ {
+ .id = V4L2_CID_POWER_LINE_FREQUENCY,
+ .entity = UVC_GUID_UVC_PROCESSING,
+ .selector = UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
+ .size = 2,
+ .offset = 0,
+ .v4l2_type = V4L2_CTRL_TYPE_MENU,
+ .data_type = UVC_CTRL_DATA_TYPE_ENUM,
+ .menu_info = power_line_frequency_controls,
+ .menu_count = ARRAY_SIZE(power_line_frequency_controls),
+ },
+};
+
/* ------------------------------------------------------------------------
* Utility functions
*/
@@ -2415,6 +2433,22 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
if (!ctrl->initialized)
return;
+ /* Process common mappings first. */
+ for (; mapping < mend; ++mapping) {
+ if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
+ ctrl->info.selector == mapping->selector)
+ __uvc_ctrl_add_mapping(chain, ctrl, mapping);
+ }
+
+ /* And then version-specific mappings. */
+ if (chain->dev->uvc_version < 0x0150) {
+ mapping = uvc_ctrl_mappings_uvc11;
+ mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc11);
+ } else {
+ mapping = uvc_ctrl_mappings_uvc15;
+ mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc15);
+ }
+
for (; mapping < mend; ++mapping) {
if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
ctrl->info.selector == mapping->selector)
--
2.36.1.476.g0c4daa206d-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v7 2/8] media: uvcvideo: Add support for per-device control mapping overrides
2022-06-17 10:36 [PATCH v7 0/8] uvcvideo: Fix handling of power_line_frequency Ricardo Ribalda
2022-06-17 10:36 ` [PATCH v7 1/8] media: uvcvideo: Add missing value for power_line_frequency Ricardo Ribalda
@ 2022-06-17 10:36 ` Ricardo Ribalda
2022-06-17 13:44 ` Laurent Pinchart
` (2 more replies)
2022-06-17 10:36 ` [PATCH v7 3/8] media: uvcvideo: Support minimum for V4L2_CTRL_TYPE_MENU Ricardo Ribalda
` (5 subsequent siblings)
7 siblings, 3 replies; 22+ messages in thread
From: Ricardo Ribalda @ 2022-06-17 10:36 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, linux-media,
linux-kernel, tfiga, senozhatsky, yunkec
Cc: Ricardo Ribalda
Some devices do not implement all their controls in a way that complies
with the UVC specification. This is for instance the case for several
devices that do not support the disabled mode for the power line
frequency control. Add a mechanism to allow per-device control mapping
overrides to avoid errors when accessing non-compliant controls.
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/usb/uvc/uvc_ctrl.c | 35 ++++++++++++++++++++++++++------
drivers/media/usb/uvc/uvcvideo.h | 1 +
2 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index a709ebbb4d69..092decfdaa62 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -2403,9 +2403,8 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
{
const struct uvc_control_info *info = uvc_ctrls;
const struct uvc_control_info *iend = info + ARRAY_SIZE(uvc_ctrls);
- const struct uvc_control_mapping *mapping = uvc_ctrl_mappings;
- const struct uvc_control_mapping *mend =
- mapping + ARRAY_SIZE(uvc_ctrl_mappings);
+ const struct uvc_control_mapping *mapping;
+ const struct uvc_control_mapping *mend;
/* XU controls initialization requires querying the device for control
* information. As some buggy UVC devices will crash when queried
@@ -2433,14 +2432,38 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
if (!ctrl->initialized)
return;
- /* Process common mappings first. */
- for (; mapping < mend; ++mapping) {
+ /*
+ * First check if the device provides a custom mapping for this control,
+ * used to override standard mappings for non-conformant devices. Don't
+ * process standard mappings if a custom mapping is found. This
+ * mechanism doesn't support combining standard and custom mappings for
+ * a single control.
+ */
+ if (chain->dev->info->mappings) {
+ bool custom = false;
+ unsigned int i;
+
+ for (i = 0; (mapping = chain->dev->info->mappings[i]); ++i) {
+ if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
+ ctrl->info.selector == mapping->selector) {
+ __uvc_ctrl_add_mapping(chain, ctrl, mapping);
+ custom = true;
+ }
+ }
+
+ if (custom)
+ return;
+ }
+
+ /* Process common mappings next. */
+ mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings);
+ for (mapping = uvc_ctrl_mappings; mapping < mend; ++mapping) {
if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
ctrl->info.selector == mapping->selector)
__uvc_ctrl_add_mapping(chain, ctrl, mapping);
}
- /* And then version-specific mappings. */
+ /* Finally process version-specific mappings. */
if (chain->dev->uvc_version < 0x0150) {
mapping = uvc_ctrl_mappings_uvc11;
mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc11);
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index c5b4febd2d94..fff5c5c99a3d 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -667,6 +667,7 @@ struct uvc_device_info {
u32 quirks;
u32 meta_format;
u16 uvc_version;
+ const struct uvc_control_mapping **mappings;
};
struct uvc_device {
--
2.36.1.476.g0c4daa206d-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v7 3/8] media: uvcvideo: Support minimum for V4L2_CTRL_TYPE_MENU
2022-06-17 10:36 [PATCH v7 0/8] uvcvideo: Fix handling of power_line_frequency Ricardo Ribalda
2022-06-17 10:36 ` [PATCH v7 1/8] media: uvcvideo: Add missing value for power_line_frequency Ricardo Ribalda
2022-06-17 10:36 ` [PATCH v7 2/8] media: uvcvideo: Add support for per-device control mapping overrides Ricardo Ribalda
@ 2022-06-17 10:36 ` Ricardo Ribalda
2022-06-17 13:49 ` Laurent Pinchart
2022-06-17 10:36 ` [PATCH v7 4/8] media: uvcvideo: Limit power line control for Quanta UVC Webcam Ricardo Ribalda
` (4 subsequent siblings)
7 siblings, 1 reply; 22+ messages in thread
From: Ricardo Ribalda @ 2022-06-17 10:36 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, linux-media,
linux-kernel, tfiga, senozhatsky, yunkec
Cc: Ricardo Ribalda
Currently all mappings of type V4L2_CTRL_TYPE_MENU, have a minimum of 0,
but there are some controls (limited powerline), that start with a value
different than 0.
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
drivers/media/usb/uvc/uvc_ctrl.c | 5 +++--
drivers/media/usb/uvc/uvcvideo.h | 1 +
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 092decfdaa62..3b20b23abd1e 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -1144,7 +1144,7 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
switch (mapping->v4l2_type) {
case V4L2_CTRL_TYPE_MENU:
- v4l2_ctrl->minimum = 0;
+ v4l2_ctrl->minimum = mapping->menu_min;
v4l2_ctrl->maximum = mapping->menu_count - 1;
v4l2_ctrl->step = 1;
@@ -1264,7 +1264,8 @@ int uvc_query_v4l2_menu(struct uvc_video_chain *chain,
goto done;
}
- if (query_menu->index >= mapping->menu_count) {
+ if (query_menu->index < mapping->menu_min ||
+ query_menu->index >= mapping->menu_count) {
ret = -EINVAL;
goto done;
}
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index fff5c5c99a3d..6ceb7f7b964d 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -254,6 +254,7 @@ struct uvc_control_mapping {
u32 data_type;
const struct uvc_menu_info *menu_info;
+ u32 menu_min;
u32 menu_count;
u32 master_id;
--
2.36.1.476.g0c4daa206d-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v7 4/8] media: uvcvideo: Limit power line control for Quanta UVC Webcam
2022-06-17 10:36 [PATCH v7 0/8] uvcvideo: Fix handling of power_line_frequency Ricardo Ribalda
` (2 preceding siblings ...)
2022-06-17 10:36 ` [PATCH v7 3/8] media: uvcvideo: Support minimum for V4L2_CTRL_TYPE_MENU Ricardo Ribalda
@ 2022-06-17 10:36 ` Ricardo Ribalda
2022-06-17 14:01 ` Laurent Pinchart
2022-06-17 10:36 ` [PATCH v7 5/8] media: uvcvideo: Limit power line control for Chicony Easycamera Ricardo Ribalda
` (3 subsequent siblings)
7 siblings, 1 reply; 22+ messages in thread
From: Ricardo Ribalda @ 2022-06-17 10:36 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, linux-media,
linux-kernel, tfiga, senozhatsky, yunkec
Cc: Ricardo Ribalda
The device does not implement the power line control correctly. Add a
corresponding control mapping override.
Bus 001 Device 003: ID 0408:3090 Quanta Computer, Inc. USB2.0 HD UVC WebCam
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass 239 Miscellaneous Device
bDeviceSubClass 2
bDeviceProtocol 1 Interface Association
bMaxPacketSize0 64
idVendor 0x0408 Quanta Computer, Inc.
idProduct 0x3090
bcdDevice 0.04
iManufacturer 3 Quanta
iProduct 1 USB2.0 HD UVC WebCam
iSerial 2 0x0001
bNumConfigurations 1
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/usb/uvc/uvc_driver.c | 35 ++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 6c86faecbea2..4fb07084f1c0 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -2643,6 +2643,32 @@ MODULE_PARM_DESC(timeout, "Streaming control requests timeout");
* Driver initialization and cleanup
*/
+static const struct uvc_menu_info power_line_frequency_controls_limited[] = {
+ { 0, "Invalid" },
+ { 1, "50 Hz" },
+ { 2, "60 Hz" },
+};
+
+static const struct uvc_control_mapping uvc_ctrl_power_line_mapping_limited = {
+ .id = V4L2_CID_POWER_LINE_FREQUENCY,
+ .entity = UVC_GUID_UVC_PROCESSING,
+ .selector = UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
+ .size = 2,
+ .offset = 0,
+ .v4l2_type = V4L2_CTRL_TYPE_MENU,
+ .data_type = UVC_CTRL_DATA_TYPE_ENUM,
+ .menu_info = power_line_frequency_controls_limited,
+ .menu_min = 1,
+ .menu_count = ARRAY_SIZE(power_line_frequency_controls_limited),
+};
+
+static const struct uvc_device_info uvc_ctrl_power_line_limited = {
+ .mappings = (const struct uvc_control_mapping *[]) {
+ &uvc_ctrl_power_line_mapping_limited,
+ NULL, /* Sentinel */
+ },
+};
+
static const struct uvc_device_info uvc_quirk_probe_minmax = {
.quirks = UVC_QUIRK_PROBE_MINMAX,
};
@@ -2673,6 +2699,15 @@ static const struct uvc_device_info uvc_quirk_force_y8 = {
* though they are compliant.
*/
static const struct usb_device_id uvc_ids[] = {
+ /* Quanta USB2.0 HD UVC Webcam */
+ { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
+ | USB_DEVICE_ID_MATCH_INT_INFO,
+ .idVendor = 0x0408,
+ .idProduct = 0x3090,
+ .bInterfaceClass = USB_CLASS_VIDEO,
+ .bInterfaceSubClass = 1,
+ .bInterfaceProtocol = 0,
+ .driver_info = (kernel_ulong_t)&uvc_ctrl_power_line_limited },
/* LogiLink Wireless Webcam */
{ .match_flags = USB_DEVICE_ID_MATCH_DEVICE
| USB_DEVICE_ID_MATCH_INT_INFO,
--
2.36.1.476.g0c4daa206d-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v7 5/8] media: uvcvideo: Limit power line control for Chicony Easycamera
2022-06-17 10:36 [PATCH v7 0/8] uvcvideo: Fix handling of power_line_frequency Ricardo Ribalda
` (3 preceding siblings ...)
2022-06-17 10:36 ` [PATCH v7 4/8] media: uvcvideo: Limit power line control for Quanta UVC Webcam Ricardo Ribalda
@ 2022-06-17 10:36 ` Ricardo Ribalda
2022-06-17 10:36 ` [PATCH v7 6/8] " Ricardo Ribalda
` (2 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Ricardo Ribalda @ 2022-06-17 10:36 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, linux-media,
linux-kernel, tfiga, senozhatsky, yunkec
Cc: Ricardo Ribalda
Two different Easycamera devices do not implement the power line control
correctly. Add a corresponding control mapping override.
Bus 001 Device 003: ID 04f2:b6ba Chicony Electronics Co., Ltd EasyCamera
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass 239 Miscellaneous Device
bDeviceSubClass 2
bDeviceProtocol 1 Interface Association
bMaxPacketSize0 64
idVendor 0x04f2 Chicony Electronics Co., Ltd
idProduct 0xb6ba
bcdDevice 10.70
iManufacturer 3 Chicony Electronics Co.,Ltd.
iProduct 1 EasyCamera
iSerial 2 0001
bNumConfigurations 1
Bus 001 Device 003: ID 04f2:b746 Chicony Electronics Co., Ltd EasyCamera
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass 239 Miscellaneous Device
bDeviceSubClass 2
bDeviceProtocol 1 Interface Association
bMaxPacketSize0 64
idVendor 0x04f2 Chicony Electronics Co., Ltd
idProduct 0xb746
bcdDevice 97.57
iManufacturer 3 Chicony Electronics Co.,Ltd.
iProduct 1 EasyCamera
iSerial 2 0001
bNumConfigurations 1
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/usb/uvc/uvc_driver.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 4fb07084f1c0..5b8a71a9edfb 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -2819,6 +2819,24 @@ static const struct usb_device_id uvc_ids[] = {
.bInterfaceSubClass = 1,
.bInterfaceProtocol = 0,
.driver_info = UVC_INFO_QUIRK(UVC_QUIRK_RESTRICT_FRAME_RATE) },
+ /* Chicony EasyCamera */
+ { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
+ | USB_DEVICE_ID_MATCH_INT_INFO,
+ .idVendor = 0x04f2,
+ .idProduct = 0xb6ba,
+ .bInterfaceClass = USB_CLASS_VIDEO,
+ .bInterfaceSubClass = 1,
+ .bInterfaceProtocol = 0,
+ .driver_info = (kernel_ulong_t)&uvc_ctrl_power_line_limited },
+ /* Chicony EasyCamera */
+ { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
+ | USB_DEVICE_ID_MATCH_INT_INFO,
+ .idVendor = 0x04f2,
+ .idProduct = 0xb746,
+ .bInterfaceClass = USB_CLASS_VIDEO,
+ .bInterfaceSubClass = 1,
+ .bInterfaceProtocol = 0,
+ .driver_info = (kernel_ulong_t)&uvc_ctrl_power_line_limited },
/* Alcor Micro AU3820 (Future Boy PC USB Webcam) */
{ .match_flags = USB_DEVICE_ID_MATCH_DEVICE
| USB_DEVICE_ID_MATCH_INT_INFO,
--
2.36.1.476.g0c4daa206d-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v7 6/8] media: uvcvideo: Limit power line control for Chicony Easycamera
2022-06-17 10:36 [PATCH v7 0/8] uvcvideo: Fix handling of power_line_frequency Ricardo Ribalda
` (4 preceding siblings ...)
2022-06-17 10:36 ` [PATCH v7 5/8] media: uvcvideo: Limit power line control for Chicony Easycamera Ricardo Ribalda
@ 2022-06-17 10:36 ` Ricardo Ribalda
2022-06-17 10:36 ` [PATCH v7 7/8] media: uvcvideo: Limit power line control for Quanta cameras Ricardo Ribalda
2022-06-17 10:36 ` [PATCH v7 8/8] media: uvcvideo: Limit power line control for Acer EasyCamera Ricardo Ribalda
7 siblings, 0 replies; 22+ messages in thread
From: Ricardo Ribalda @ 2022-06-17 10:36 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, linux-media,
linux-kernel, tfiga, senozhatsky, yunkec
Cc: Ricardo Ribalda
Another Chicony camera device does not implement the power line control
correctly. Add a corresponding control mapping override.
Bus 001 Device 003: ID 04f2:b5eb Chicony Electronics Co., Ltd EasyCamera
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass 239 Miscellaneous Device
bDeviceSubClass 2
bDeviceProtocol 1 Interface Association
bMaxPacketSize0 64
idVendor 0x04f2 Chicony Electronics Co., Ltd
idProduct 0xb5eb
bcdDevice 90.45
iManufacturer 3 Chicony Electronics Co.,Ltd.
iProduct 1 EasyCamera
iSerial 2 0001
bNumConfigurations 1
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/usb/uvc/uvc_driver.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 5b8a71a9edfb..d21de83021f5 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -2820,6 +2820,15 @@ static const struct usb_device_id uvc_ids[] = {
.bInterfaceProtocol = 0,
.driver_info = UVC_INFO_QUIRK(UVC_QUIRK_RESTRICT_FRAME_RATE) },
/* Chicony EasyCamera */
+ { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
+ | USB_DEVICE_ID_MATCH_INT_INFO,
+ .idVendor = 0x04f2,
+ .idProduct = 0xb5eb,
+ .bInterfaceClass = USB_CLASS_VIDEO,
+ .bInterfaceSubClass = 1,
+ .bInterfaceProtocol = 0,
+ .driver_info = (kernel_ulong_t)&uvc_ctrl_power_line_limited },
+ /* Chicony EasyCamera */
{ .match_flags = USB_DEVICE_ID_MATCH_DEVICE
| USB_DEVICE_ID_MATCH_INT_INFO,
.idVendor = 0x04f2,
--
2.36.1.476.g0c4daa206d-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v7 7/8] media: uvcvideo: Limit power line control for Quanta cameras
2022-06-17 10:36 [PATCH v7 0/8] uvcvideo: Fix handling of power_line_frequency Ricardo Ribalda
` (5 preceding siblings ...)
2022-06-17 10:36 ` [PATCH v7 6/8] " Ricardo Ribalda
@ 2022-06-17 10:36 ` Ricardo Ribalda
2022-06-17 10:36 ` [PATCH v7 8/8] media: uvcvideo: Limit power line control for Acer EasyCamera Ricardo Ribalda
7 siblings, 0 replies; 22+ messages in thread
From: Ricardo Ribalda @ 2022-06-17 10:36 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, linux-media,
linux-kernel, tfiga, senozhatsky, yunkec
Cc: Ricardo Ribalda
Two more cameras do not implement the power line control correctly. Add
a corresponding control mapping override.
Bus 001 Device 003: ID 0408:4034 Quanta Computer, Inc. ACER HD User Facing
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 2.01
bDeviceClass 239 Miscellaneous Device
bDeviceSubClass 2
bDeviceProtocol 1 Interface Association
bMaxPacketSize0 64
idVendor 0x0408 Quanta Computer, Inc.
idProduct 0x4034
bcdDevice 0.01
iManufacturer 1 Quanta
iProduct 2 ACER HD User Facing
iSerial 3 01.00.00
bNumConfigurations 1
Bus 001 Device 003: ID 0408:4030 Quanta Computer, Inc. HD User Facing
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 2.01
bDeviceClass 239 Miscellaneous Device
bDeviceSubClass 2
bDeviceProtocol 1 Interface Association
bMaxPacketSize0 64
idVendor 0x0408 Quanta Computer, Inc.
idProduct 0x4030
bcdDevice 0.02
iManufacturer 1 Quanta
iProduct 2 HD User Facing
iSerial 3 01.00.00
bNumConfigurations 1
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/usb/uvc/uvc_driver.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index d21de83021f5..a862a9d6a2fd 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -2708,6 +2708,24 @@ static const struct usb_device_id uvc_ids[] = {
.bInterfaceSubClass = 1,
.bInterfaceProtocol = 0,
.driver_info = (kernel_ulong_t)&uvc_ctrl_power_line_limited },
+ /* Quanta USB2.0 HD UVC Webcam */
+ { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
+ | USB_DEVICE_ID_MATCH_INT_INFO,
+ .idVendor = 0x0408,
+ .idProduct = 0x4030,
+ .bInterfaceClass = USB_CLASS_VIDEO,
+ .bInterfaceSubClass = 1,
+ .bInterfaceProtocol = 1,
+ .driver_info = (kernel_ulong_t)&uvc_ctrl_power_line_limited },
+ /* Quanta USB2.0 HD UVC Webcam */
+ { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
+ | USB_DEVICE_ID_MATCH_INT_INFO,
+ .idVendor = 0x0408,
+ .idProduct = 0x4034,
+ .bInterfaceClass = USB_CLASS_VIDEO,
+ .bInterfaceSubClass = 1,
+ .bInterfaceProtocol = 0,
+ .driver_info = (kernel_ulong_t)&uvc_ctrl_power_line_limited },
/* LogiLink Wireless Webcam */
{ .match_flags = USB_DEVICE_ID_MATCH_DEVICE
| USB_DEVICE_ID_MATCH_INT_INFO,
--
2.36.1.476.g0c4daa206d-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v7 8/8] media: uvcvideo: Limit power line control for Acer EasyCamera
2022-06-17 10:36 [PATCH v7 0/8] uvcvideo: Fix handling of power_line_frequency Ricardo Ribalda
` (6 preceding siblings ...)
2022-06-17 10:36 ` [PATCH v7 7/8] media: uvcvideo: Limit power line control for Quanta cameras Ricardo Ribalda
@ 2022-06-17 10:36 ` Ricardo Ribalda
7 siblings, 0 replies; 22+ messages in thread
From: Ricardo Ribalda @ 2022-06-17 10:36 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, linux-media,
linux-kernel, tfiga, senozhatsky, yunkec
Cc: Ricardo Ribalda
The device does not implement the power line control correctly. Add a
corresponding control mapping override.
Bus 001 Device 003: ID 5986:1172 Acer, Inc EasyCamera
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass 239 Miscellaneous Device
bDeviceSubClass 2
bDeviceProtocol 1 Interface Association
bMaxPacketSize0 64
idVendor 0x5986 Acer, Inc
idProduct 0x1172
bcdDevice 56.04
iManufacturer 3 Bison
iProduct 1 EasyCamera
iSerial 2
bNumConfigurations 1
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/usb/uvc/uvc_driver.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index a862a9d6a2fd..6d34992032e6 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -3248,6 +3248,15 @@ static const struct usb_device_id uvc_ids[] = {
.bInterfaceSubClass = 1,
.bInterfaceProtocol = 0,
.driver_info = UVC_INFO_QUIRK(UVC_QUIRK_FORCE_BPP) },
+ /* Acer EasyCamera */
+ { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
+ | USB_DEVICE_ID_MATCH_INT_INFO,
+ .idVendor = 0x5986,
+ .idProduct = 0x1172,
+ .bInterfaceClass = USB_CLASS_VIDEO,
+ .bInterfaceSubClass = 1,
+ .bInterfaceProtocol = 0,
+ .driver_info = (kernel_ulong_t)&uvc_ctrl_power_line_limited },
/* Intel RealSense D4M */
{ .match_flags = USB_DEVICE_ID_MATCH_DEVICE
| USB_DEVICE_ID_MATCH_INT_INFO,
--
2.36.1.476.g0c4daa206d-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v7 2/8] media: uvcvideo: Add support for per-device control mapping overrides
2022-06-17 10:36 ` [PATCH v7 2/8] media: uvcvideo: Add support for per-device control mapping overrides Ricardo Ribalda
@ 2022-06-17 13:44 ` Laurent Pinchart
2022-06-17 14:07 ` Ricardo Ribalda
2022-06-17 14:40 ` kernel test robot
2022-06-30 11:07 ` Dan Carpenter
2 siblings, 1 reply; 22+ messages in thread
From: Laurent Pinchart @ 2022-06-17 13:44 UTC (permalink / raw)
To: Ricardo Ribalda
Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, tfiga,
senozhatsky, yunkec
Hi Ricardo,
Thank you for the patch.
On Fri, Jun 17, 2022 at 12:36:39PM +0200, Ricardo Ribalda wrote:
> Some devices do not implement all their controls in a way that complies
> with the UVC specification. This is for instance the case for several
> devices that do not support the disabled mode for the power line
> frequency control. Add a mechanism to allow per-device control mapping
> overrides to avoid errors when accessing non-compliant controls.
>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 35 ++++++++++++++++++++++++++------
> drivers/media/usb/uvc/uvcvideo.h | 1 +
> 2 files changed, 30 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index a709ebbb4d69..092decfdaa62 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -2403,9 +2403,8 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
> {
> const struct uvc_control_info *info = uvc_ctrls;
> const struct uvc_control_info *iend = info + ARRAY_SIZE(uvc_ctrls);
> - const struct uvc_control_mapping *mapping = uvc_ctrl_mappings;
> - const struct uvc_control_mapping *mend =
> - mapping + ARRAY_SIZE(uvc_ctrl_mappings);
> + const struct uvc_control_mapping *mapping;
> + const struct uvc_control_mapping *mend;
>
> /* XU controls initialization requires querying the device for control
> * information. As some buggy UVC devices will crash when queried
> @@ -2433,14 +2432,38 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
> if (!ctrl->initialized)
> return;
>
> - /* Process common mappings first. */
> - for (; mapping < mend; ++mapping) {
> + /*
> + * First check if the device provides a custom mapping for this control,
> + * used to override standard mappings for non-conformant devices. Don't
> + * process standard mappings if a custom mapping is found. This
> + * mechanism doesn't support combining standard and custom mappings for
> + * a single control.
> + */
> + if (chain->dev->info->mappings) {
> + bool custom = false;
> + unsigned int i;
> +
> + for (i = 0; (mapping = chain->dev->info->mappings[i]); ++i) {
> + if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
> + ctrl->info.selector == mapping->selector) {
> + __uvc_ctrl_add_mapping(chain, ctrl, mapping);
> + custom = true;
> + }
> + }
> +
> + if (custom)
> + return;
> + }
> +
> + /* Process common mappings next. */
> + mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings);
I don't think mapping has the right value here, it could even be
uninitialized. Let's make this
mapping = uvc_ctrl_mappings;
mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings);
for (; mapping < mend; ++mapping) {
to match the code below.
> + for (mapping = uvc_ctrl_mappings; mapping < mend; ++mapping) {
> if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
> ctrl->info.selector == mapping->selector)
> __uvc_ctrl_add_mapping(chain, ctrl, mapping);
> }
>
> - /* And then version-specific mappings. */
> + /* Finally process version-specific mappings. */
> if (chain->dev->uvc_version < 0x0150) {
> mapping = uvc_ctrl_mappings_uvc11;
> mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc11);
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index c5b4febd2d94..fff5c5c99a3d 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -667,6 +667,7 @@ struct uvc_device_info {
> u32 quirks;
> u32 meta_format;
> u16 uvc_version;
> + const struct uvc_control_mapping **mappings;
> };
>
> struct uvc_device {
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v7 3/8] media: uvcvideo: Support minimum for V4L2_CTRL_TYPE_MENU
2022-06-17 10:36 ` [PATCH v7 3/8] media: uvcvideo: Support minimum for V4L2_CTRL_TYPE_MENU Ricardo Ribalda
@ 2022-06-17 13:49 ` Laurent Pinchart
2022-06-17 13:55 ` Ricardo Ribalda
0 siblings, 1 reply; 22+ messages in thread
From: Laurent Pinchart @ 2022-06-17 13:49 UTC (permalink / raw)
To: Ricardo Ribalda
Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, tfiga,
senozhatsky, yunkec
Hi Ricardo,
Thank you for the patch.
On Fri, Jun 17, 2022 at 12:36:40PM +0200, Ricardo Ribalda wrote:
> Currently all mappings of type V4L2_CTRL_TYPE_MENU, have a minimum of 0,
> but there are some controls (limited powerline), that start with a value
> different than 0.
>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 5 +++--
> drivers/media/usb/uvc/uvcvideo.h | 1 +
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 092decfdaa62..3b20b23abd1e 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -1144,7 +1144,7 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
>
> switch (mapping->v4l2_type) {
> case V4L2_CTRL_TYPE_MENU:
> - v4l2_ctrl->minimum = 0;
> + v4l2_ctrl->minimum = mapping->menu_min;
> v4l2_ctrl->maximum = mapping->menu_count - 1;
> v4l2_ctrl->step = 1;
>
> @@ -1264,7 +1264,8 @@ int uvc_query_v4l2_menu(struct uvc_video_chain *chain,
> goto done;
> }
>
> - if (query_menu->index >= mapping->menu_count) {
> + if (query_menu->index < mapping->menu_min ||
> + query_menu->index >= mapping->menu_count) {
> ret = -EINVAL;
> goto done;
> }
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index fff5c5c99a3d..6ceb7f7b964d 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -254,6 +254,7 @@ struct uvc_control_mapping {
> u32 data_type;
>
> const struct uvc_menu_info *menu_info;
> + u32 menu_min;
> u32 menu_count;
That's a bit of a stop-gap measure, could we turn it into a bitmask
instead ?
>
> u32 master_id;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v7 3/8] media: uvcvideo: Support minimum for V4L2_CTRL_TYPE_MENU
2022-06-17 13:49 ` Laurent Pinchart
@ 2022-06-17 13:55 ` Ricardo Ribalda
2022-06-17 14:11 ` Laurent Pinchart
0 siblings, 1 reply; 22+ messages in thread
From: Ricardo Ribalda @ 2022-06-17 13:55 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, tfiga,
senozhatsky, yunkec
Hi Laurent
On Fri, 17 Jun 2022 at 15:50, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Ricardo,
>
> Thank you for the patch.
>
> On Fri, Jun 17, 2022 at 12:36:40PM +0200, Ricardo Ribalda wrote:
> > Currently all mappings of type V4L2_CTRL_TYPE_MENU, have a minimum of 0,
> > but there are some controls (limited powerline), that start with a value
> > different than 0.
> >
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> > drivers/media/usb/uvc/uvc_ctrl.c | 5 +++--
> > drivers/media/usb/uvc/uvcvideo.h | 1 +
> > 2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> > index 092decfdaa62..3b20b23abd1e 100644
> > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > @@ -1144,7 +1144,7 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
> >
> > switch (mapping->v4l2_type) {
> > case V4L2_CTRL_TYPE_MENU:
> > - v4l2_ctrl->minimum = 0;
> > + v4l2_ctrl->minimum = mapping->menu_min;
> > v4l2_ctrl->maximum = mapping->menu_count - 1;
> > v4l2_ctrl->step = 1;
> >
> > @@ -1264,7 +1264,8 @@ int uvc_query_v4l2_menu(struct uvc_video_chain *chain,
> > goto done;
> > }
> >
> > - if (query_menu->index >= mapping->menu_count) {
> > + if (query_menu->index < mapping->menu_min ||
> > + query_menu->index >= mapping->menu_count) {
> > ret = -EINVAL;
> > goto done;
> > }
> > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > index fff5c5c99a3d..6ceb7f7b964d 100644
> > --- a/drivers/media/usb/uvc/uvcvideo.h
> > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > @@ -254,6 +254,7 @@ struct uvc_control_mapping {
> > u32 data_type;
> >
> > const struct uvc_menu_info *menu_info;
> > + u32 menu_min;
> > u32 menu_count;
>
> That's a bit of a stop-gap measure, could we turn it into a bitmask
> instead ?
Unfortunately that is uAPI :(
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/v4l2-controls.h#n101
We have to keep the control type and its values.
Regards!
>
> >
> > u32 master_id;
>
> --
> Regards,
>
> Laurent Pinchart
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v7 4/8] media: uvcvideo: Limit power line control for Quanta UVC Webcam
2022-06-17 10:36 ` [PATCH v7 4/8] media: uvcvideo: Limit power line control for Quanta UVC Webcam Ricardo Ribalda
@ 2022-06-17 14:01 ` Laurent Pinchart
2022-06-17 14:18 ` Ricardo Ribalda
0 siblings, 1 reply; 22+ messages in thread
From: Laurent Pinchart @ 2022-06-17 14:01 UTC (permalink / raw)
To: Ricardo Ribalda
Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, tfiga,
senozhatsky, yunkec
Hi Ricardo,
Thank you for the patch.
On Fri, Jun 17, 2022 at 12:36:41PM +0200, Ricardo Ribalda wrote:
> The device does not implement the power line control correctly. Add a
> corresponding control mapping override.
>
> Bus 001 Device 003: ID 0408:3090 Quanta Computer, Inc. USB2.0 HD UVC WebCam
> Device Descriptor:
> bLength 18
> bDescriptorType 1
> bcdUSB 2.00
> bDeviceClass 239 Miscellaneous Device
> bDeviceSubClass 2
> bDeviceProtocol 1 Interface Association
> bMaxPacketSize0 64
> idVendor 0x0408 Quanta Computer, Inc.
> idProduct 0x3090
> bcdDevice 0.04
> iManufacturer 3 Quanta
> iProduct 1 USB2.0 HD UVC WebCam
> iSerial 2 0x0001
> bNumConfigurations 1
>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/usb/uvc/uvc_driver.c | 35 ++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index 6c86faecbea2..4fb07084f1c0 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -2643,6 +2643,32 @@ MODULE_PARM_DESC(timeout, "Streaming control requests timeout");
> * Driver initialization and cleanup
> */
>
> +static const struct uvc_menu_info power_line_frequency_controls_limited[] = {
> + { 0, "Invalid" },
> + { 1, "50 Hz" },
> + { 2, "60 Hz" },
> +};
It's not nice to have to include the first item in the array, but we
can't fix that without modifying uvc_menu_info, which we can't do as
it's part of the UAPI. Let's keep it as-is, but I would then expose the
uvc_menu_info array from uvc_ctrl.c instead of duplicating it here.
> +
> +static const struct uvc_control_mapping uvc_ctrl_power_line_mapping_limited = {
> + .id = V4L2_CID_POWER_LINE_FREQUENCY,
> + .entity = UVC_GUID_UVC_PROCESSING,
> + .selector = UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
> + .size = 2,
> + .offset = 0,
> + .v4l2_type = V4L2_CTRL_TYPE_MENU,
> + .data_type = UVC_CTRL_DATA_TYPE_ENUM,
> + .menu_info = power_line_frequency_controls_limited,
> + .menu_min = 1,
> + .menu_count = ARRAY_SIZE(power_line_frequency_controls_limited),
> +};
> +
> +static const struct uvc_device_info uvc_ctrl_power_line_limited = {
> + .mappings = (const struct uvc_control_mapping *[]) {
> + &uvc_ctrl_power_line_mapping_limited,
> + NULL, /* Sentinel */
> + },
> +};
> +
> static const struct uvc_device_info uvc_quirk_probe_minmax = {
> .quirks = UVC_QUIRK_PROBE_MINMAX,
> };
> @@ -2673,6 +2699,15 @@ static const struct uvc_device_info uvc_quirk_force_y8 = {
> * though they are compliant.
> */
> static const struct usb_device_id uvc_ids[] = {
> + /* Quanta USB2.0 HD UVC Webcam */
> + { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
> + | USB_DEVICE_ID_MATCH_INT_INFO,
> + .idVendor = 0x0408,
> + .idProduct = 0x3090,
> + .bInterfaceClass = USB_CLASS_VIDEO,
> + .bInterfaceSubClass = 1,
> + .bInterfaceProtocol = 0,
> + .driver_info = (kernel_ulong_t)&uvc_ctrl_power_line_limited },
> /* LogiLink Wireless Webcam */
> { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
> | USB_DEVICE_ID_MATCH_INT_INFO,
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v7 2/8] media: uvcvideo: Add support for per-device control mapping overrides
2022-06-17 13:44 ` Laurent Pinchart
@ 2022-06-17 14:07 ` Ricardo Ribalda
0 siblings, 0 replies; 22+ messages in thread
From: Ricardo Ribalda @ 2022-06-17 14:07 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, tfiga,
senozhatsky, yunkec
Hi Laurent
On Fri, 17 Jun 2022 at 15:44, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Ricardo,
>
> Thank you for the patch.
>
> On Fri, Jun 17, 2022 at 12:36:39PM +0200, Ricardo Ribalda wrote:
> > Some devices do not implement all their controls in a way that complies
> > with the UVC specification. This is for instance the case for several
> > devices that do not support the disabled mode for the power line
> > frequency control. Add a mechanism to allow per-device control mapping
> > overrides to avoid errors when accessing non-compliant controls.
> >
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > drivers/media/usb/uvc/uvc_ctrl.c | 35 ++++++++++++++++++++++++++------
> > drivers/media/usb/uvc/uvcvideo.h | 1 +
> > 2 files changed, 30 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> > index a709ebbb4d69..092decfdaa62 100644
> > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > @@ -2403,9 +2403,8 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
> > {
> > const struct uvc_control_info *info = uvc_ctrls;
> > const struct uvc_control_info *iend = info + ARRAY_SIZE(uvc_ctrls);
> > - const struct uvc_control_mapping *mapping = uvc_ctrl_mappings;
> > - const struct uvc_control_mapping *mend =
> > - mapping + ARRAY_SIZE(uvc_ctrl_mappings);
> > + const struct uvc_control_mapping *mapping;
> > + const struct uvc_control_mapping *mend;
> >
> > /* XU controls initialization requires querying the device for control
> > * information. As some buggy UVC devices will crash when queried
> > @@ -2433,14 +2432,38 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
> > if (!ctrl->initialized)
> > return;
> >
> > - /* Process common mappings first. */
> > - for (; mapping < mend; ++mapping) {
> > + /*
> > + * First check if the device provides a custom mapping for this control,
> > + * used to override standard mappings for non-conformant devices. Don't
> > + * process standard mappings if a custom mapping is found. This
> > + * mechanism doesn't support combining standard and custom mappings for
> > + * a single control.
> > + */
> > + if (chain->dev->info->mappings) {
> > + bool custom = false;
> > + unsigned int i;
> > +
> > + for (i = 0; (mapping = chain->dev->info->mappings[i]); ++i) {
> > + if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
> > + ctrl->info.selector == mapping->selector) {
> > + __uvc_ctrl_add_mapping(chain, ctrl, mapping);
> > + custom = true;
> > + }
> > + }
> > +
> > + if (custom)
> > + return;
> > + }
> > +
> > + /* Process common mappings next. */
> > + mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings);
>
> I don't think mapping has the right value here, it could even be
> uninitialized. Let's make this
>
> mapping = uvc_ctrl_mappings;
> mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings);
>
> for (; mapping < mend; ++mapping) {
>
> to match the code below.
ups, sorry about that :(
. Will send as a new version
>
> > + for (mapping = uvc_ctrl_mappings; mapping < mend; ++mapping) {
> > if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
> > ctrl->info.selector == mapping->selector)
> > __uvc_ctrl_add_mapping(chain, ctrl, mapping);
> > }
> >
> > - /* And then version-specific mappings. */
> > + /* Finally process version-specific mappings. */
> > if (chain->dev->uvc_version < 0x0150) {
> > mapping = uvc_ctrl_mappings_uvc11;
> > mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc11);
> > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > index c5b4febd2d94..fff5c5c99a3d 100644
> > --- a/drivers/media/usb/uvc/uvcvideo.h
> > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > @@ -667,6 +667,7 @@ struct uvc_device_info {
> > u32 quirks;
> > u32 meta_format;
> > u16 uvc_version;
> > + const struct uvc_control_mapping **mappings;
> > };
> >
> > struct uvc_device {
>
> --
> Regards,
>
> Laurent Pinchart
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v7 3/8] media: uvcvideo: Support minimum for V4L2_CTRL_TYPE_MENU
2022-06-17 13:55 ` Ricardo Ribalda
@ 2022-06-17 14:11 ` Laurent Pinchart
2022-06-17 14:15 ` Ricardo Ribalda
0 siblings, 1 reply; 22+ messages in thread
From: Laurent Pinchart @ 2022-06-17 14:11 UTC (permalink / raw)
To: Ricardo Ribalda
Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, tfiga,
senozhatsky, yunkec
Hi Ricardo,
On Fri, Jun 17, 2022 at 03:55:52PM +0200, Ricardo Ribalda wrote:
> On Fri, 17 Jun 2022 at 15:50, Laurent Pinchart wrote:
> > On Fri, Jun 17, 2022 at 12:36:40PM +0200, Ricardo Ribalda wrote:
> > > Currently all mappings of type V4L2_CTRL_TYPE_MENU, have a minimum of 0,
> > > but there are some controls (limited powerline), that start with a value
> > > different than 0.
> > >
> > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > ---
> > > drivers/media/usb/uvc/uvc_ctrl.c | 5 +++--
> > > drivers/media/usb/uvc/uvcvideo.h | 1 +
> > > 2 files changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> > > index 092decfdaa62..3b20b23abd1e 100644
> > > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > > @@ -1144,7 +1144,7 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
> > >
> > > switch (mapping->v4l2_type) {
> > > case V4L2_CTRL_TYPE_MENU:
> > > - v4l2_ctrl->minimum = 0;
> > > + v4l2_ctrl->minimum = mapping->menu_min;
> > > v4l2_ctrl->maximum = mapping->menu_count - 1;
> > > v4l2_ctrl->step = 1;
> > >
> > > @@ -1264,7 +1264,8 @@ int uvc_query_v4l2_menu(struct uvc_video_chain *chain,
> > > goto done;
> > > }
> > >
> > > - if (query_menu->index >= mapping->menu_count) {
> > > + if (query_menu->index < mapping->menu_min ||
> > > + query_menu->index >= mapping->menu_count) {
> > > ret = -EINVAL;
> > > goto done;
> > > }
> > > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > > index fff5c5c99a3d..6ceb7f7b964d 100644
> > > --- a/drivers/media/usb/uvc/uvcvideo.h
> > > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > > @@ -254,6 +254,7 @@ struct uvc_control_mapping {
> > > u32 data_type;
> > >
> > > const struct uvc_menu_info *menu_info;
> > > + u32 menu_min;
> > > u32 menu_count;
> >
> > That's a bit of a stop-gap measure, could we turn it into a bitmask
> > instead ?
>
> Unfortunately that is uAPI :(
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/v4l2-controls.h#n101
>
> We have to keep the control type and its values.
Sure, I didn't mean changing that, but replacing menu_min and menu_count
in uvc_control_mapping with a menu_mask that stores a bitmask of all
supported values. This will allow skipping the first value in the power
line frequency control case, but will also support skipping other menu
entries for other controls in the future.
> > >
> > > u32 master_id;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v7 3/8] media: uvcvideo: Support minimum for V4L2_CTRL_TYPE_MENU
2022-06-17 14:11 ` Laurent Pinchart
@ 2022-06-17 14:15 ` Ricardo Ribalda
0 siblings, 0 replies; 22+ messages in thread
From: Ricardo Ribalda @ 2022-06-17 14:15 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, tfiga,
senozhatsky, yunkec
Hi Laurent
On Fri, 17 Jun 2022 at 16:11, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Ricardo,
>
> On Fri, Jun 17, 2022 at 03:55:52PM +0200, Ricardo Ribalda wrote:
> > On Fri, 17 Jun 2022 at 15:50, Laurent Pinchart wrote:
> > > On Fri, Jun 17, 2022 at 12:36:40PM +0200, Ricardo Ribalda wrote:
> > > > Currently all mappings of type V4L2_CTRL_TYPE_MENU, have a minimum of 0,
> > > > but there are some controls (limited powerline), that start with a value
> > > > different than 0.
> > > >
> > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > > ---
> > > > drivers/media/usb/uvc/uvc_ctrl.c | 5 +++--
> > > > drivers/media/usb/uvc/uvcvideo.h | 1 +
> > > > 2 files changed, 4 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> > > > index 092decfdaa62..3b20b23abd1e 100644
> > > > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > > > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > > > @@ -1144,7 +1144,7 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
> > > >
> > > > switch (mapping->v4l2_type) {
> > > > case V4L2_CTRL_TYPE_MENU:
> > > > - v4l2_ctrl->minimum = 0;
> > > > + v4l2_ctrl->minimum = mapping->menu_min;
> > > > v4l2_ctrl->maximum = mapping->menu_count - 1;
> > > > v4l2_ctrl->step = 1;
> > > >
> > > > @@ -1264,7 +1264,8 @@ int uvc_query_v4l2_menu(struct uvc_video_chain *chain,
> > > > goto done;
> > > > }
> > > >
> > > > - if (query_menu->index >= mapping->menu_count) {
> > > > + if (query_menu->index < mapping->menu_min ||
> > > > + query_menu->index >= mapping->menu_count) {
> > > > ret = -EINVAL;
> > > > goto done;
> > > > }
> > > > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > > > index fff5c5c99a3d..6ceb7f7b964d 100644
> > > > --- a/drivers/media/usb/uvc/uvcvideo.h
> > > > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > > > @@ -254,6 +254,7 @@ struct uvc_control_mapping {
> > > > u32 data_type;
> > > >
> > > > const struct uvc_menu_info *menu_info;
> > > > + u32 menu_min;
> > > > u32 menu_count;
> > >
> > > That's a bit of a stop-gap measure, could we turn it into a bitmask
> > > instead ?
> >
> > Unfortunately that is uAPI :(
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/v4l2-controls.h#n101
> >
> > We have to keep the control type and its values.
>
> Sure, I didn't mean changing that, but replacing menu_min and menu_count
> in uvc_control_mapping with a menu_mask that stores a bitmask of all
> supported values. This will allow skipping the first value in the power
> line frequency control case, but will also support skipping other menu
> entries for other controls in the future.
Ahh gotcha. Will implement that in the next version.
Thanks!
>
> > > >
> > > > u32 master_id;
>
> --
> Regards,
>
> Laurent Pinchart
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v7 4/8] media: uvcvideo: Limit power line control for Quanta UVC Webcam
2022-06-17 14:01 ` Laurent Pinchart
@ 2022-06-17 14:18 ` Ricardo Ribalda
0 siblings, 0 replies; 22+ messages in thread
From: Ricardo Ribalda @ 2022-06-17 14:18 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, tfiga,
senozhatsky, yunkec
Hi Laurent
On Fri, 17 Jun 2022 at 16:02, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Ricardo,
>
> Thank you for the patch.
Thank you :)
>
> On Fri, Jun 17, 2022 at 12:36:41PM +0200, Ricardo Ribalda wrote:
> > The device does not implement the power line control correctly. Add a
> > corresponding control mapping override.
> >
> > Bus 001 Device 003: ID 0408:3090 Quanta Computer, Inc. USB2.0 HD UVC WebCam
> > Device Descriptor:
> > bLength 18
> > bDescriptorType 1
> > bcdUSB 2.00
> > bDeviceClass 239 Miscellaneous Device
> > bDeviceSubClass 2
> > bDeviceProtocol 1 Interface Association
> > bMaxPacketSize0 64
> > idVendor 0x0408 Quanta Computer, Inc.
> > idProduct 0x3090
> > bcdDevice 0.04
> > iManufacturer 3 Quanta
> > iProduct 1 USB2.0 HD UVC WebCam
> > iSerial 2 0x0001
> > bNumConfigurations 1
> >
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > drivers/media/usb/uvc/uvc_driver.c | 35 ++++++++++++++++++++++++++++++
> > 1 file changed, 35 insertions(+)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > index 6c86faecbea2..4fb07084f1c0 100644
> > --- a/drivers/media/usb/uvc/uvc_driver.c
> > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > @@ -2643,6 +2643,32 @@ MODULE_PARM_DESC(timeout, "Streaming control requests timeout");
> > * Driver initialization and cleanup
> > */
> >
> > +static const struct uvc_menu_info power_line_frequency_controls_limited[] = {
> > + { 0, "Invalid" },
> > + { 1, "50 Hz" },
> > + { 2, "60 Hz" },
> > +};
>
> It's not nice to have to include the first item in the array, but we
> can't fix that without modifying uvc_menu_info, which we can't do as
> it's part of the UAPI. Let's keep it as-is, but I would then expose the
> uvc_menu_info array from uvc_ctrl.c instead of duplicating it here.
Instead of that, what about using the values from v4l2_ctrl_get_menu() ?
We will still provide a fallback in case the control is not standard.
Let me implement that in the next version and see what you think about it.
Regards!
>
> > +
> > +static const struct uvc_control_mapping uvc_ctrl_power_line_mapping_limited = {
> > + .id = V4L2_CID_POWER_LINE_FREQUENCY,
> > + .entity = UVC_GUID_UVC_PROCESSING,
> > + .selector = UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
> > + .size = 2,
> > + .offset = 0,
> > + .v4l2_type = V4L2_CTRL_TYPE_MENU,
> > + .data_type = UVC_CTRL_DATA_TYPE_ENUM,
> > + .menu_info = power_line_frequency_controls_limited,
> > + .menu_min = 1,
> > + .menu_count = ARRAY_SIZE(power_line_frequency_controls_limited),
> > +};
> > +
> > +static const struct uvc_device_info uvc_ctrl_power_line_limited = {
> > + .mappings = (const struct uvc_control_mapping *[]) {
> > + &uvc_ctrl_power_line_mapping_limited,
> > + NULL, /* Sentinel */
> > + },
> > +};
> > +
> > static const struct uvc_device_info uvc_quirk_probe_minmax = {
> > .quirks = UVC_QUIRK_PROBE_MINMAX,
> > };
> > @@ -2673,6 +2699,15 @@ static const struct uvc_device_info uvc_quirk_force_y8 = {
> > * though they are compliant.
> > */
> > static const struct usb_device_id uvc_ids[] = {
> > + /* Quanta USB2.0 HD UVC Webcam */
> > + { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
> > + | USB_DEVICE_ID_MATCH_INT_INFO,
> > + .idVendor = 0x0408,
> > + .idProduct = 0x3090,
> > + .bInterfaceClass = USB_CLASS_VIDEO,
> > + .bInterfaceSubClass = 1,
> > + .bInterfaceProtocol = 0,
> > + .driver_info = (kernel_ulong_t)&uvc_ctrl_power_line_limited },
> > /* LogiLink Wireless Webcam */
> > { .match_flags = USB_DEVICE_ID_MATCH_DEVICE
> > | USB_DEVICE_ID_MATCH_INT_INFO,
>
> --
> Regards,
>
> Laurent Pinchart
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v7 2/8] media: uvcvideo: Add support for per-device control mapping overrides
2022-06-17 10:36 ` [PATCH v7 2/8] media: uvcvideo: Add support for per-device control mapping overrides Ricardo Ribalda
2022-06-17 13:44 ` Laurent Pinchart
@ 2022-06-17 14:40 ` kernel test robot
2022-06-30 11:07 ` Dan Carpenter
2 siblings, 0 replies; 22+ messages in thread
From: kernel test robot @ 2022-06-17 14:40 UTC (permalink / raw)
To: Ricardo Ribalda, Laurent Pinchart, Mauro Carvalho Chehab,
linux-kernel, tfiga, senozhatsky, yunkec
Cc: llvm, kbuild-all, linux-media, Ricardo Ribalda
Hi Ricardo,
I love your patch! Perhaps something to improve:
[auto build test WARNING on media-tree/master]
[also build test WARNING on linus/master v5.19-rc2 next-20220617]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Ricardo-Ribalda/uvcvideo-Fix-handling-of-power_line_frequency/20220617-185644
base: git://linuxtv.org/media_tree.git master
config: hexagon-randconfig-r045-20220617 (https://download.01.org/0day-ci/archive/20220617/202206172239.M4dQBUhz-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project d764aa7fc6b9cc3fbe960019018f5f9e941eb0a6)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/10bdca4191d7a8be97c77dbe4ba89c05713ee0e2
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Ricardo-Ribalda/uvcvideo-Fix-handling-of-power_line_frequency/20220617-185644
git checkout 10bdca4191d7a8be97c77dbe4ba89c05713ee0e2
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/media/usb/uvc/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/media/usb/uvc/uvc_ctrl.c:2442:6: warning: variable 'mapping' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
if (chain->dev->info->mappings) {
^~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/media/usb/uvc/uvc_ctrl.c:2459:9: note: uninitialized use occurs here
mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings);
^~~~~~~
drivers/media/usb/uvc/uvc_ctrl.c:2442:2: note: remove the 'if' if its condition is always true
if (chain->dev->info->mappings) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/media/usb/uvc/uvc_ctrl.c:2406:43: note: initialize the variable 'mapping' to silence this warning
const struct uvc_control_mapping *mapping;
^
= NULL
1 warning generated.
vim +2442 drivers/media/usb/uvc/uvc_ctrl.c
2396
2397 /*
2398 * Add control information and hardcoded stock control mappings to the given
2399 * device.
2400 */
2401 static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
2402 struct uvc_control *ctrl)
2403 {
2404 const struct uvc_control_info *info = uvc_ctrls;
2405 const struct uvc_control_info *iend = info + ARRAY_SIZE(uvc_ctrls);
2406 const struct uvc_control_mapping *mapping;
2407 const struct uvc_control_mapping *mend;
2408
2409 /* XU controls initialization requires querying the device for control
2410 * information. As some buggy UVC devices will crash when queried
2411 * repeatedly in a tight loop, delay XU controls initialization until
2412 * first use.
2413 */
2414 if (UVC_ENTITY_TYPE(ctrl->entity) == UVC_VC_EXTENSION_UNIT)
2415 return;
2416
2417 for (; info < iend; ++info) {
2418 if (uvc_entity_match_guid(ctrl->entity, info->entity) &&
2419 ctrl->index == info->index) {
2420 uvc_ctrl_add_info(chain->dev, ctrl, info);
2421 /*
2422 * Retrieve control flags from the device. Ignore errors
2423 * and work with default flag values from the uvc_ctrl
2424 * array when the device doesn't properly implement
2425 * GET_INFO on standard controls.
2426 */
2427 uvc_ctrl_get_flags(chain->dev, ctrl, &ctrl->info);
2428 break;
2429 }
2430 }
2431
2432 if (!ctrl->initialized)
2433 return;
2434
2435 /*
2436 * First check if the device provides a custom mapping for this control,
2437 * used to override standard mappings for non-conformant devices. Don't
2438 * process standard mappings if a custom mapping is found. This
2439 * mechanism doesn't support combining standard and custom mappings for
2440 * a single control.
2441 */
> 2442 if (chain->dev->info->mappings) {
2443 bool custom = false;
2444 unsigned int i;
2445
2446 for (i = 0; (mapping = chain->dev->info->mappings[i]); ++i) {
2447 if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
2448 ctrl->info.selector == mapping->selector) {
2449 __uvc_ctrl_add_mapping(chain, ctrl, mapping);
2450 custom = true;
2451 }
2452 }
2453
2454 if (custom)
2455 return;
2456 }
2457
2458 /* Process common mappings next. */
2459 mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings);
2460 for (mapping = uvc_ctrl_mappings; mapping < mend; ++mapping) {
2461 if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
2462 ctrl->info.selector == mapping->selector)
2463 __uvc_ctrl_add_mapping(chain, ctrl, mapping);
2464 }
2465
2466 /* Finally process version-specific mappings. */
2467 if (chain->dev->uvc_version < 0x0150) {
2468 mapping = uvc_ctrl_mappings_uvc11;
2469 mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc11);
2470 } else {
2471 mapping = uvc_ctrl_mappings_uvc15;
2472 mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc15);
2473 }
2474
2475 for (; mapping < mend; ++mapping) {
2476 if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
2477 ctrl->info.selector == mapping->selector)
2478 __uvc_ctrl_add_mapping(chain, ctrl, mapping);
2479 }
2480 }
2481
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v7 2/8] media: uvcvideo: Add support for per-device control mapping overrides
2022-06-17 10:36 ` [PATCH v7 2/8] media: uvcvideo: Add support for per-device control mapping overrides Ricardo Ribalda
2022-06-17 13:44 ` Laurent Pinchart
2022-06-17 14:40 ` kernel test robot
@ 2022-06-30 11:07 ` Dan Carpenter
2022-06-30 23:16 ` Ricardo Ribalda
2 siblings, 1 reply; 22+ messages in thread
From: Dan Carpenter @ 2022-06-30 11:07 UTC (permalink / raw)
To: kbuild, Ricardo Ribalda, Laurent Pinchart, Mauro Carvalho Chehab,
linux-kernel, tfiga, senozhatsky, yunkec
Cc: lkp, kbuild-all, linux-media, Ricardo Ribalda
Hi Ricardo,
url: https://github.com/intel-lab-lkp/linux/commits/Ricardo-Ribalda/uvcvideo-Fix-handling-of-power_line_frequency/20220617-185644
base: git://linuxtv.org/media_tree.git master
config: microblaze-randconfig-m031-20220629 (https://download.01.org/0day-ci/archive/20220630/202206301144.r7yv0yk2-lkp@intel.com/config)
compiler: microblaze-linux-gcc (GCC) 11.3.0
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/media/usb/uvc/uvc_ctrl.c:2459 uvc_ctrl_init_ctrl() error: uninitialized symbol 'mapping'.
vim +/mapping +2459 drivers/media/usb/uvc/uvc_ctrl.c
866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2401 static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2402 struct uvc_control *ctrl)
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2403 {
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2404 const struct uvc_control_info *info = uvc_ctrls;
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2405 const struct uvc_control_info *iend = info + ARRAY_SIZE(uvc_ctrls);
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2406 const struct uvc_control_mapping *mapping;
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2407 const struct uvc_control_mapping *mend;
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2408
52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2409 /* XU controls initialization requires querying the device for control
52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2410 * information. As some buggy UVC devices will crash when queried
52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2411 * repeatedly in a tight loop, delay XU controls initialization until
52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2412 * first use.
52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2413 */
52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2414 if (UVC_ENTITY_TYPE(ctrl->entity) == UVC_VC_EXTENSION_UNIT)
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2415 return;
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2416
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2417 for (; info < iend; ++info) {
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2418 if (uvc_entity_match_guid(ctrl->entity, info->entity) &&
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2419 ctrl->index == info->index) {
866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2420 uvc_ctrl_add_info(chain->dev, ctrl, info);
93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2421 /*
93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2422 * Retrieve control flags from the device. Ignore errors
93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2423 * and work with default flag values from the uvc_ctrl
93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2424 * array when the device doesn't properly implement
93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2425 * GET_INFO on standard controls.
93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2426 */
866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2427 uvc_ctrl_get_flags(chain->dev, ctrl, &ctrl->info);
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2428 break;
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2429 }
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2430 }
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2431
071c8bb827c80a drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2432 if (!ctrl->initialized)
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2433 return;
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2434
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2435 /*
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2436 * First check if the device provides a custom mapping for this control,
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2437 * used to override standard mappings for non-conformant devices. Don't
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2438 * process standard mappings if a custom mapping is found. This
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2439 * mechanism doesn't support combining standard and custom mappings for
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2440 * a single control.
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2441 */
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2442 if (chain->dev->info->mappings) {
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2443 bool custom = false;
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2444 unsigned int i;
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2445
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2446 for (i = 0; (mapping = chain->dev->info->mappings[i]); ++i) {
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2447 if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2448 ctrl->info.selector == mapping->selector) {
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2449 __uvc_ctrl_add_mapping(chain, ctrl, mapping);
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2450 custom = true;
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2451 }
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2452 }
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2453
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2454 if (custom)
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2455 return;
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2456 }
"mapping" uninitialized if chain->dev->info->mappings is NULL.
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2457
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2458 /* Process common mappings next. */
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 @2459 mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings);
^^^^^^^
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2460 for (mapping = uvc_ctrl_mappings; mapping < mend; ++mapping) {
I'm surprised this made it through testing... Anyway, the easier way to
to iterate through an array of structs is just say:
for (i = 0; i < ARRAY_SIZE(uvc_ctrl_mappings); i++) {
mapping = &uvc_ctrl_mappings[i];
87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2461 if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2462 ctrl->info.selector == mapping->selector)
87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2463 __uvc_ctrl_add_mapping(chain, ctrl, mapping);
87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2464 }
87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2465
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2466 /* Finally process version-specific mappings. */
87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2467 if (chain->dev->uvc_version < 0x0150) {
87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2468 mapping = uvc_ctrl_mappings_uvc11;
87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2469 mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc11);
87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2470 } else {
87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2471 mapping = uvc_ctrl_mappings_uvc15;
87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2472 mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc15);
87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2473 }
87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2474
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2475 for (; mapping < mend; ++mapping) {
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2476 if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
071c8bb827c80a drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2477 ctrl->info.selector == mapping->selector)
866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2478 __uvc_ctrl_add_mapping(chain, ctrl, mapping);
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2479 }
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2480 }
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v7 2/8] media: uvcvideo: Add support for per-device control mapping overrides
2022-06-30 11:07 ` Dan Carpenter
@ 2022-06-30 23:16 ` Ricardo Ribalda
2022-07-01 2:31 ` [kbuild-all] " Philip Li
2022-07-01 7:26 ` Dan Carpenter
0 siblings, 2 replies; 22+ messages in thread
From: Ricardo Ribalda @ 2022-06-30 23:16 UTC (permalink / raw)
To: Dan Carpenter
Cc: kbuild, Laurent Pinchart, Mauro Carvalho Chehab, linux-kernel,
tfiga, senozhatsky, yunkec, lkp, kbuild-all, linux-media
Hi Dan
Thanks for your mail
On Thu, 30 Jun 2022 at 13:08, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> Hi Ricardo,
>
> url: https://github.com/intel-lab-lkp/linux/commits/Ricardo-Ribalda/uvcvideo-Fix-handling-of-power_line_frequency/20220617-185644
> base: git://linuxtv.org/media_tree.git master
> config: microblaze-randconfig-m031-20220629 (https://download.01.org/0day-ci/archive/20220630/202206301144.r7yv0yk2-lkp@intel.com/config)
> compiler: microblaze-linux-gcc (GCC) 11.3.0
>
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> smatch warnings:
> drivers/media/usb/uvc/uvc_ctrl.c:2459 uvc_ctrl_init_ctrl() error: uninitialized symbol 'mapping'.
>
> vim +/mapping +2459 drivers/media/usb/uvc/uvc_ctrl.c
>
> 866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2401 static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
> 866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2402 struct uvc_control *ctrl)
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2403 {
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2404 const struct uvc_control_info *info = uvc_ctrls;
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2405 const struct uvc_control_info *iend = info + ARRAY_SIZE(uvc_ctrls);
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2406 const struct uvc_control_mapping *mapping;
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2407 const struct uvc_control_mapping *mend;
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2408
> 52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2409 /* XU controls initialization requires querying the device for control
> 52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2410 * information. As some buggy UVC devices will crash when queried
> 52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2411 * repeatedly in a tight loop, delay XU controls initialization until
> 52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2412 * first use.
> 52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2413 */
> 52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2414 if (UVC_ENTITY_TYPE(ctrl->entity) == UVC_VC_EXTENSION_UNIT)
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2415 return;
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2416
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2417 for (; info < iend; ++info) {
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2418 if (uvc_entity_match_guid(ctrl->entity, info->entity) &&
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2419 ctrl->index == info->index) {
> 866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2420 uvc_ctrl_add_info(chain->dev, ctrl, info);
> 93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2421 /*
> 93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2422 * Retrieve control flags from the device. Ignore errors
> 93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2423 * and work with default flag values from the uvc_ctrl
> 93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2424 * array when the device doesn't properly implement
> 93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2425 * GET_INFO on standard controls.
> 93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2426 */
> 866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2427 uvc_ctrl_get_flags(chain->dev, ctrl, &ctrl->info);
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2428 break;
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2429 }
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2430 }
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2431
> 071c8bb827c80a drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2432 if (!ctrl->initialized)
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2433 return;
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2434
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2435 /*
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2436 * First check if the device provides a custom mapping for this control,
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2437 * used to override standard mappings for non-conformant devices. Don't
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2438 * process standard mappings if a custom mapping is found. This
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2439 * mechanism doesn't support combining standard and custom mappings for
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2440 * a single control.
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2441 */
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2442 if (chain->dev->info->mappings) {
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2443 bool custom = false;
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2444 unsigned int i;
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2445
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2446 for (i = 0; (mapping = chain->dev->info->mappings[i]); ++i) {
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2447 if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2448 ctrl->info.selector == mapping->selector) {
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2449 __uvc_ctrl_add_mapping(chain, ctrl, mapping);
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2450 custom = true;
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2451 }
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2452 }
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2453
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2454 if (custom)
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2455 return;
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2456 }
>
> "mapping" uninitialized if chain->dev->info->mappings is NULL.
>
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2457
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2458 /* Process common mappings next. */
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 @2459 mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings);
> ^^^^^^^
>
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2460 for (mapping = uvc_ctrl_mappings; mapping < mend; ++mapping) {
>
> I'm surprised this made it through testing... Anyway, the easier way to
> to iterate through an array of structs is just say:
There is already a new version under review that fixes this:
https://patchwork.linuxtv.org/project/linux-media/patch/20220617235610.321917-3-ribalda@chromium.org/
>
> for (i = 0; i < ARRAY_SIZE(uvc_ctrl_mappings); i++) {
> mapping = &uvc_ctrl_mappings[i];
>
I agree, and that is what I normally used, but that driver iterate this way...
Thanks again!
PS: Any idea why the test was triggered 14+ days after the patch was sent?
> 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2461 if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
> 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2462 ctrl->info.selector == mapping->selector)
> 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2463 __uvc_ctrl_add_mapping(chain, ctrl, mapping);
> 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2464 }
> 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2465
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2466 /* Finally process version-specific mappings. */
> 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2467 if (chain->dev->uvc_version < 0x0150) {
> 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2468 mapping = uvc_ctrl_mappings_uvc11;
> 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2469 mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc11);
> 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2470 } else {
> 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2471 mapping = uvc_ctrl_mappings_uvc15;
> 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2472 mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc15);
> 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2473 }
> 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2474
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2475 for (; mapping < mend; ++mapping) {
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2476 if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
> 071c8bb827c80a drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2477 ctrl->info.selector == mapping->selector)
> 866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2478 __uvc_ctrl_add_mapping(chain, ctrl, mapping);
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2479 }
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2480 }
>
> --
> 0-DAY CI Kernel Test Service
> https://01.org/lkp
>
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [kbuild-all] Re: [PATCH v7 2/8] media: uvcvideo: Add support for per-device control mapping overrides
2022-06-30 23:16 ` Ricardo Ribalda
@ 2022-07-01 2:31 ` Philip Li
2022-07-01 7:26 ` Dan Carpenter
1 sibling, 0 replies; 22+ messages in thread
From: Philip Li @ 2022-07-01 2:31 UTC (permalink / raw)
To: Ricardo Ribalda
Cc: Dan Carpenter, kbuild, Laurent Pinchart, Mauro Carvalho Chehab,
linux-kernel, tfiga, senozhatsky, yunkec, lkp, kbuild-all,
linux-media
On Fri, Jul 01, 2022 at 01:16:23AM +0200, Ricardo Ribalda wrote:
> Hi Dan
>
> Thanks for your mail
>
> On Thu, 30 Jun 2022 at 13:08, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> >
> > Hi Ricardo,
> >
> > url: https://github.com/intel-lab-lkp/linux/commits/Ricardo-Ribalda/uvcvideo-Fix-handling-of-power_line_frequency/20220617-185644
> > base: git://linuxtv.org/media_tree.git master
> > config: microblaze-randconfig-m031-20220629 (https://download.01.org/0day-ci/archive/20220630/202206301144.r7yv0yk2-lkp@intel.com/config)
> > compiler: microblaze-linux-gcc (GCC) 11.3.0
> >
> > If you fix the issue, kindly add following tag where applicable
> > Reported-by: kernel test robot <lkp@intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> >
> > smatch warnings:
> > drivers/media/usb/uvc/uvc_ctrl.c:2459 uvc_ctrl_init_ctrl() error: uninitialized symbol 'mapping'.
> >
> > vim +/mapping +2459 drivers/media/usb/uvc/uvc_ctrl.c
> >
> > 866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2401 static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
> > 866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2402 struct uvc_control *ctrl)
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2403 {
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2404 const struct uvc_control_info *info = uvc_ctrls;
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2405 const struct uvc_control_info *iend = info + ARRAY_SIZE(uvc_ctrls);
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2406 const struct uvc_control_mapping *mapping;
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2407 const struct uvc_control_mapping *mend;
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2408
> > 52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2409 /* XU controls initialization requires querying the device for control
> > 52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2410 * information. As some buggy UVC devices will crash when queried
> > 52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2411 * repeatedly in a tight loop, delay XU controls initialization until
> > 52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2412 * first use.
> > 52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2413 */
> > 52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2414 if (UVC_ENTITY_TYPE(ctrl->entity) == UVC_VC_EXTENSION_UNIT)
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2415 return;
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2416
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2417 for (; info < iend; ++info) {
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2418 if (uvc_entity_match_guid(ctrl->entity, info->entity) &&
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2419 ctrl->index == info->index) {
> > 866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2420 uvc_ctrl_add_info(chain->dev, ctrl, info);
> > 93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2421 /*
> > 93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2422 * Retrieve control flags from the device. Ignore errors
> > 93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2423 * and work with default flag values from the uvc_ctrl
> > 93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2424 * array when the device doesn't properly implement
> > 93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2425 * GET_INFO on standard controls.
> > 93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2426 */
> > 866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2427 uvc_ctrl_get_flags(chain->dev, ctrl, &ctrl->info);
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2428 break;
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2429 }
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2430 }
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2431
> > 071c8bb827c80a drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2432 if (!ctrl->initialized)
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2433 return;
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2434
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2435 /*
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2436 * First check if the device provides a custom mapping for this control,
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2437 * used to override standard mappings for non-conformant devices. Don't
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2438 * process standard mappings if a custom mapping is found. This
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2439 * mechanism doesn't support combining standard and custom mappings for
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2440 * a single control.
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2441 */
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2442 if (chain->dev->info->mappings) {
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2443 bool custom = false;
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2444 unsigned int i;
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2445
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2446 for (i = 0; (mapping = chain->dev->info->mappings[i]); ++i) {
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2447 if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2448 ctrl->info.selector == mapping->selector) {
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2449 __uvc_ctrl_add_mapping(chain, ctrl, mapping);
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2450 custom = true;
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2451 }
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2452 }
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2453
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2454 if (custom)
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2455 return;
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2456 }
> >
> > "mapping" uninitialized if chain->dev->info->mappings is NULL.
> >
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2457
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2458 /* Process common mappings next. */
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 @2459 mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings);
> > ^^^^^^^
> >
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2460 for (mapping = uvc_ctrl_mappings; mapping < mend; ++mapping) {
> >
> > I'm surprised this made it through testing... Anyway, the easier way to
> > to iterate through an array of structs is just say:
>
> There is already a new version under review that fixes this:
>
> https://patchwork.linuxtv.org/project/linux-media/patch/20220617235610.321917-3-ribalda@chromium.org/
>
>
> >
> > for (i = 0; i < ARRAY_SIZE(uvc_ctrl_mappings); i++) {
> > mapping = &uvc_ctrl_mappings[i];
> >
>
> I agree, and that is what I normally used, but that driver iterate this way...
>
> Thanks again!
>
> PS: Any idea why the test was triggered 14+ days after the patch was sent?
Thanks, the config is a random one, which is generated on 6/29 "microblaze-randconfig-m031-20220629",
which is probably the case that only this randconfig has chance to trigger the issue
and bisect successfully.
>
> > 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2461 if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
> > 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2462 ctrl->info.selector == mapping->selector)
> > 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2463 __uvc_ctrl_add_mapping(chain, ctrl, mapping);
> > 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2464 }
> > 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2465
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2466 /* Finally process version-specific mappings. */
> > 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2467 if (chain->dev->uvc_version < 0x0150) {
> > 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2468 mapping = uvc_ctrl_mappings_uvc11;
> > 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2469 mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc11);
> > 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2470 } else {
> > 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2471 mapping = uvc_ctrl_mappings_uvc15;
> > 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2472 mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc15);
> > 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2473 }
> > 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2474
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2475 for (; mapping < mend; ++mapping) {
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2476 if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
> > 071c8bb827c80a drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2477 ctrl->info.selector == mapping->selector)
> > 866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2478 __uvc_ctrl_add_mapping(chain, ctrl, mapping);
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2479 }
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2480 }
> >
> > --
> > 0-DAY CI Kernel Test Service
> > https://01.org/lkp
> >
>
>
> --
> Ricardo Ribalda
> _______________________________________________
> kbuild-all mailing list -- kbuild-all@lists.01.org
> To unsubscribe send an email to kbuild-all-leave@lists.01.org
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v7 2/8] media: uvcvideo: Add support for per-device control mapping overrides
2022-06-30 23:16 ` Ricardo Ribalda
2022-07-01 2:31 ` [kbuild-all] " Philip Li
@ 2022-07-01 7:26 ` Dan Carpenter
1 sibling, 0 replies; 22+ messages in thread
From: Dan Carpenter @ 2022-07-01 7:26 UTC (permalink / raw)
To: Ricardo Ribalda
Cc: kbuild, Laurent Pinchart, Mauro Carvalho Chehab, linux-kernel,
tfiga, senozhatsky, yunkec, lkp, kbuild-all, linux-media
On Fri, Jul 01, 2022 at 01:16:23AM +0200, Ricardo Ribalda wrote:
> >
> > "mapping" uninitialized if chain->dev->info->mappings is NULL.
> >
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2457
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2458 /* Process common mappings next. */
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 @2459 mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings);
> > ^^^^^^^
> >
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2460 for (mapping = uvc_ctrl_mappings; mapping < mend; ++mapping) {
> >
> > I'm surprised this made it through testing... Anyway, the easier way to
> > to iterate through an array of structs is just say:
>
> There is already a new version under review that fixes this:
>
> https://patchwork.linuxtv.org/project/linux-media/patch/20220617235610.321917-3-ribalda@chromium.org/
>
>
> >
> > for (i = 0; i < ARRAY_SIZE(uvc_ctrl_mappings); i++) {
> > mapping = &uvc_ctrl_mappings[i];
> >
>
> I agree, and that is what I normally used, but that driver iterate this way...
That's not really an excuse. Imagine you were a lemming and Disney
employees were trying to chase you off a cliff!
>
> Thanks again!
>
> PS: Any idea why the test was triggered 14+ days after the patch was sent?
I don't know how kbuild works. I run Smatch on linux-next so I would
have caught it eventually. Plus the clang people would have caught it.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2022-07-01 7:27 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-17 10:36 [PATCH v7 0/8] uvcvideo: Fix handling of power_line_frequency Ricardo Ribalda
2022-06-17 10:36 ` [PATCH v7 1/8] media: uvcvideo: Add missing value for power_line_frequency Ricardo Ribalda
2022-06-17 10:36 ` [PATCH v7 2/8] media: uvcvideo: Add support for per-device control mapping overrides Ricardo Ribalda
2022-06-17 13:44 ` Laurent Pinchart
2022-06-17 14:07 ` Ricardo Ribalda
2022-06-17 14:40 ` kernel test robot
2022-06-30 11:07 ` Dan Carpenter
2022-06-30 23:16 ` Ricardo Ribalda
2022-07-01 2:31 ` [kbuild-all] " Philip Li
2022-07-01 7:26 ` Dan Carpenter
2022-06-17 10:36 ` [PATCH v7 3/8] media: uvcvideo: Support minimum for V4L2_CTRL_TYPE_MENU Ricardo Ribalda
2022-06-17 13:49 ` Laurent Pinchart
2022-06-17 13:55 ` Ricardo Ribalda
2022-06-17 14:11 ` Laurent Pinchart
2022-06-17 14:15 ` Ricardo Ribalda
2022-06-17 10:36 ` [PATCH v7 4/8] media: uvcvideo: Limit power line control for Quanta UVC Webcam Ricardo Ribalda
2022-06-17 14:01 ` Laurent Pinchart
2022-06-17 14:18 ` Ricardo Ribalda
2022-06-17 10:36 ` [PATCH v7 5/8] media: uvcvideo: Limit power line control for Chicony Easycamera Ricardo Ribalda
2022-06-17 10:36 ` [PATCH v7 6/8] " Ricardo Ribalda
2022-06-17 10:36 ` [PATCH v7 7/8] media: uvcvideo: Limit power line control for Quanta cameras Ricardo Ribalda
2022-06-17 10:36 ` [PATCH v7 8/8] media: uvcvideo: Limit power line control for Acer EasyCamera Ricardo Ribalda
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).