Hi Laurent
On Thu, 9 Jun 2022 at 11:08, Laurent Pinchart
<laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
>
> From: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxx>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
> Changes since v3:
>
> - Turn the power line quirk into a control mapping overrides array
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 27 +++++++++++++++++++++++++--
> drivers/media/usb/uvc/uvcvideo.h | 1 +
> 2 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 95fdd41ab20b..e4826a846861 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -2444,14 +2444,37 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
> if (!ctrl->initialized)
> return;
>
> - /* Process common mappings first. */
> + /*
> + * 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;
> + }
> +
If there is a custom mapping you are overwriting the variable
mapping, the next loop will iterate with no limit.....
This is, you need:
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 362f84e280d6..c26b0b9fd36b 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -2493,9 +2493,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;
const u8 camera_entity[16] = UVC_GUID_UVC_CAMERA;
/* XU controls initialization requires querying the device for control
@@ -2551,6 +2550,8 @@ static void uvc_ctrl_init_ctrl(struct
uvc_video_chain *chain,
}
/* Process common mappings next. */
+ mapping = uvc_ctrl_mappings;
+ mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings);
for (; mapping < mend; ++mapping) {
if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
ctrl->info.selector == mapping->selector)
with that in place it works fine with a device with a quirk and a
device without a quirk
> + /* Process common mappings next. */
> 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. */
> + /* 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 d2eb107347ea..24c911aeebce 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -668,6 +668,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
