On Thu, Dec 01, 2022 at 05:44:52PM +0200, Volodymyr Kharuk wrote:
> Hi Hans,
>
> On Fri, Nov 25, 2022 at 03:35:34PM +0100, Hans Verkuil wrote:
> > On 25/11/2022 14:34, Volodymyr Kharuk wrote:
> > > For V4L2_CID_TOF_TINT, which is dynamic array, it is required to use
> > > __v4l2_ctrl_modify_range. So the idea is to use type_ops instead of u64
> > > from union. It will allow to work with any type.
> > >
> > > Signed-off-by: Volodymyr Kharuk <vkh@xxxxxxxxxxx>
> > > ---
> > > drivers/media/v4l2-core/v4l2-ctrls-api.c | 15 +++------------
> > > 1 file changed, 3 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/media/v4l2-core/v4l2-ctrls-api.c b/drivers/media/v4l2-core/v4l2-ctrls-api.c
> > > index d0a3aa3806fb..09735644a2f1 100644
> > > --- a/drivers/media/v4l2-core/v4l2-ctrls-api.c
> > > +++ b/drivers/media/v4l2-core/v4l2-ctrls-api.c
> > > @@ -942,8 +942,6 @@ int __v4l2_ctrl_modify_range(struct v4l2_ctrl *ctrl,
> > > case V4L2_CTRL_TYPE_U8:
> > > case V4L2_CTRL_TYPE_U16:
> > > case V4L2_CTRL_TYPE_U32:
> > > - if (ctrl->is_array)
> > > - return -EINVAL;
> > > ret = check_range(ctrl->type, min, max, step, def);
> > > if (ret)
> > > return ret;
> > > @@ -960,17 +958,10 @@ int __v4l2_ctrl_modify_range(struct v4l2_ctrl *ctrl,
> > > ctrl->default_value = def;
> > > }
> > > cur_to_new(ctrl);
> > > - if (validate_new(ctrl, ctrl->p_new)) {
> > > - if (ctrl->type == V4L2_CTRL_TYPE_INTEGER64)
> > > - *ctrl->p_new.p_s64 = def;
> > > - else
> > > - *ctrl->p_new.p_s32 = def;
> > > - }
> > > + if (validate_new(ctrl, ctrl->p_new))
> > > + ctrl->type_ops->init(ctrl, 0, ctrl->p_new);
> >
> > Hmm, this sets *all* elements of the array to the default_value, not
> > just the elements whose value is out of range.
> >
> > Is that what you want? Should this perhaps depend on the type of
> > control? I.e. in some cases it might make sense to do this, in other
> > cases it makes more sense to only adjust the elements that are out
> > of range.
> >
> > How does that work for this TINT control?
> Actually for types like INTEGER/U8/U16/U32/BOOLEAN/BUTTON/BITMASK, the function
> validate_new will return zero always as well as they fix the range on the fly.
> So that is ok for mlx7502x sensors.
>
> For types like compound/string/menu indeed, it will sets all elements of array
> to default array. To fix it I propose to fix it by using the functions
> std_validate_elem to check per element and then set the default manually.
> But then it means, that __v4l2_ctrl_modify_range will work only for standart
> v4l2 types, and not if driver use their own implementation.
>
> Is it fine for you? What do you think?
I've another idea. If validate_new is fixing on the fly
and we can't modify range for copmound/string types, so we can forbidden
array for MENU types. Then validate_new will do the job for the rest types.
As for now there are no needs in arrays for MENU types.
What do you think?
--
--
BR,
Volodymyr Kharuk
