Hi,

Another month, another mail. Are there still issues keeping this from being 
merged?

Regards,

Edgar

On 11/15/2017 12:54 PM, Kieran Bingham wrote:
> Hi Edgar,
> 
> Thanks for addressing my concerns in this updated patch.
> 
> On 12/10/17 08:54, Edgar Thier wrote:
>>
>> Use flags the device exposes for UVC controls.
>> This allows the device to define which property flags are set.
>>
>> Since some cameras offer auto-adjustments for properties (e.g. auto-gain),
>> the values of other properties (e.g. gain) can change in the camera.
>> Examining the flags ensures that the driver is aware of such properties.
>>
>> Signed-off-by: Edgar Thier <i...@edgarthier.net>
> 
> Reviewed-by: Kieran Bingham <kieran.bingham+rene...@ideasonboard.com>
> 
>> ---
>>  drivers/media/usb/uvc/uvc_ctrl.c | 64 
>> ++++++++++++++++++++++++++++++----------
>>  1 file changed, 49 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c 
>> b/drivers/media/usb/uvc/uvc_ctrl.c
>> index 20397aba..8f902a41 100644
>> --- a/drivers/media/usb/uvc/uvc_ctrl.c
>> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
>> @@ -1629,6 +1629,40 @@ static void uvc_ctrl_fixup_xu_info(struct uvc_device 
>> *dev,
>>      }
>>  }
>>
>> +/*
>> + * Retrieve flags for a given control
>> + */
>> +static int uvc_ctrl_get_flags(struct uvc_device *dev, const struct 
>> uvc_control *ctrl,
>> +    const struct uvc_control_info *info)
>> +{
>> +    u8 *data;
>> +    int ret = 0;
>> +    int flags = 0;
>> +
>> +    data = kmalloc(2, GFP_KERNEL);
>> +    if (data == NULL)
>> +            return -ENOMEM;
>> +
>> +    ret = uvc_query_ctrl(dev, UVC_GET_INFO, ctrl->entity->id, dev->intfnum,
>> +                                             info->selector, data, 1);
>> +    if (ret < 0) {
>> +            uvc_trace(UVC_TRACE_CONTROL,
>> +                              "GET_INFO failed on control %pUl/%u (%d).\n",
>> +                              info->entity, info->selector, ret);
>> +    } else {
>> +            flags = UVC_CTRL_FLAG_GET_MIN | UVC_CTRL_FLAG_GET_MAX
>> +                    | UVC_CTRL_FLAG_GET_RES | UVC_CTRL_FLAG_GET_DEF
>> +                    | (data[0] & UVC_CONTROL_CAP_GET ?
>> +                       UVC_CTRL_FLAG_GET_CUR : 0)
>> +                    | (data[0] & UVC_CONTROL_CAP_SET ?
>> +                       UVC_CTRL_FLAG_SET_CUR : 0)
>> +                    | (data[0] & UVC_CONTROL_CAP_AUTOUPDATE ?
>> +                       UVC_CTRL_FLAG_AUTO_UPDATE : 0);
>> +    }
>> +    kfree(data);
>> +    return flags;
>> +}
>> +
>>  /*
>>   * Query control information (size and flags) for XU controls.
>>   */
>> @@ -1636,6 +1670,7 @@ static int uvc_ctrl_fill_xu_info(struct uvc_device 
>> *dev,
>>      const struct uvc_control *ctrl, struct uvc_control_info *info)
>>  {
>>      u8 *data;
>> +    int flags;
>>      int ret;
>>
>>      data = kmalloc(2, GFP_KERNEL);
>> @@ -1659,24 +1694,15 @@ static int uvc_ctrl_fill_xu_info(struct uvc_device 
>> *dev,
>>
>>      info->size = le16_to_cpup((__le16 *)data);
>>
>> -    /* Query the control information (GET_INFO) */
>> -    ret = uvc_query_ctrl(dev, UVC_GET_INFO, ctrl->entity->id, dev->intfnum,
>> -                         info->selector, data, 1);
>> -    if (ret < 0) {
>> +    flags = uvc_ctrl_get_flags(dev, ctrl, info);
>> +
>> +    if (flags < 0) {
>>              uvc_trace(UVC_TRACE_CONTROL,
>> -                      "GET_INFO failed on control %pUl/%u (%d).\n",
>> -                      info->entity, info->selector, ret);
>> +                      "Failed to retrieve flags (%d).\n", ret);
>> +            ret = flags;
>>              goto done;
>>      }
>> -
>> -    info->flags = UVC_CTRL_FLAG_GET_MIN | UVC_CTRL_FLAG_GET_MAX
>> -                | UVC_CTRL_FLAG_GET_RES | UVC_CTRL_FLAG_GET_DEF
>> -                | (data[0] & UVC_CONTROL_CAP_GET ?
>> -                   UVC_CTRL_FLAG_GET_CUR : 0)
>> -                | (data[0] & UVC_CONTROL_CAP_SET ?
>> -                   UVC_CTRL_FLAG_SET_CUR : 0)
>> -                | (data[0] & UVC_CONTROL_CAP_AUTOUPDATE ?
>> -                   UVC_CTRL_FLAG_AUTO_UPDATE : 0);
>> +    info->flags = flags;
>>
>>      uvc_ctrl_fixup_xu_info(dev, ctrl, info);
>>
>> @@ -1890,6 +1916,7 @@ static int uvc_ctrl_add_info(struct uvc_device *dev, 
>> struct uvc_control *ctrl,
>>      const struct uvc_control_info *info)
>>  {
>>      int ret = 0;
>> +    int flags = 0;
>>
>>      ctrl->info = *info;
>>      INIT_LIST_HEAD(&ctrl->info.mappings);
>> @@ -1902,6 +1929,13 @@ static int uvc_ctrl_add_info(struct uvc_device *dev, 
>> struct uvc_control *ctrl,
>>              goto done;
>>      }
>>
>> +    flags = uvc_ctrl_get_flags(dev, ctrl, info);
>> +    if (flags < 0)
>> +            uvc_trace(UVC_TRACE_CONTROL,
>> +                      "Failed to retrieve flags (%d).\n", ret);
>> +    else
>> +            ctrl->info.flags = flags;
>> +
>>      ctrl->initialized = 1;
>>
>>      uvc_trace(UVC_TRACE_CONTROL, "Added control %pUl/%u to device %s "
>>

Reply via email to