On 6/1/19 11:05 AM, Boris Brezillon wrote:
> On Thu, 30 May 2019 14:19:08 -0300
> Ezequiel Garcia <[email protected]> wrote:
>
>> These two control types don't really need a default value,
>> as they are not expected to carry any value.
>>
>> However, it's slightly clearer to initialize them explicitly
>> instead of falling back to the switch default.
>
> If they don't carry any value, why not having a case that does nothing?
> I find it disturbing to assign a default to something that does not
> have a value attached to it.
I agree to a point. It is still good to initialize these,
if only to make sure that there is a sane value in the control.
How about this:
case V4L2_CTRL_TYPE_BOOLEAN:
ptr.p_s32[idx] = ctrl->default_value;
break;
case V4L2_CTRL_TYPE_BUTTON:
case V4L2_CTRL_TYPE_CTRL_CLASS:
ptr.p_s32[idx] = 0;
break;
Regards,
Hans
>
>>
>> Signed-off-by: Ezequiel Garcia <[email protected]>
>> ---
>> drivers/media/v4l2-core/v4l2-ctrls.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
>> b/drivers/media/v4l2-core/v4l2-ctrls.c
>> index 1870cecad9ae..c7d5fdb8efb4 100644
>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>> @@ -1530,6 +1530,8 @@ static void std_init(const struct v4l2_ctrl *ctrl, u32
>> idx,
>> case V4L2_CTRL_TYPE_MENU:
>> case V4L2_CTRL_TYPE_BITMASK:
>> case V4L2_CTRL_TYPE_BOOLEAN:
>> + case V4L2_CTRL_TYPE_BUTTON:
>> + case V4L2_CTRL_TYPE_CTRL_CLASS:
>> ptr.p_s32[idx] = ctrl->default_value;
>> break;
>> case V4L2_CTRL_TYPE_U8:
>
