On Thursday 30 July 2009 05:06:34 Mauro Carvalho Chehab wrote:
> Em Wed, 29 Jul 2009 17:08:31 +0200
> "Hans Verkuil" <hverkuil@xxxxxxxxx> escreveu:
>
> > > I did some tests here: if we replace -EINVAL with -ENOIOCTLCMD, we can
> > > properly
> > > make v4l2_device_call_until_err() to work, fixing the lack of a proper
> > > error
> > > report at the drivers. This error code seems also appropriate for this
> > > case.
> >
> > This is not sufficient: v4l2_device_call_until_err is not really suitable
> > for this.
>
> Agreed. Yet, the tests helped to see what changes were needed.
>
> >This would be better:
> >
> > #define __v4l2_device_call_subdevs_ctrls(v4l2_dev, cond, o, f, args...) \
> > ({ \
> > struct v4l2_subdev *sd; \
> > long err = 0; \
> > \
> > list_for_each_entry(sd, &(v4l2_dev)->subdevs, list) { \
> > if ((cond) && sd->ops->o && sd->ops->o->f) \
> > err = sd->ops->o->f(sd , ##args); \
> > if (err && err != -ENOIOCTLCMD) \
>
> No, this is not right. In the case of controls, it should be, instead:
>
> if (err != -ENOIOCTLCMD)
Indeed.
>
> Also, such routine is has nothing specific for ctrls, so, the naming doesn't
> look nice. I'll find a better name.
I hacked it together on short notice, so feel to hack it some more :-)
>
> > break; \
> > } \
> > (err == -ENOIOCTLCMD) ? -EINVAL : err; \
> > })
> >
> > This way -EINVAL is returned if the control isn't handled anywhere.
>
> Ok, but I'm in doubt if -EINVAL is the better return code on such case, but, in
> order to not break backward compatibility, better to return -EINVAL.
I'd love to be able to change the use of EINVAL for unknown controls. I think
there are some other cases as well in the spec where EINVAL is incorrectly
used. But that's almost impossible to change without breaking backwards
compatibility.
The only way I can think of doing this is by having the application explicitly
request such changed behavior. But that's a very slippery slope to go on.
Regards,
Hans
--
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
