Re: [PATCH 1/2] v4l2 subdevs: replace get/set_crop by get/set_selection

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Sakari,

On Wednesday 03 December 2014 13:06:00 Sakari Ailus wrote:
> On Tue, Dec 02, 2014 at 01:21:40PM +0100, Hans Verkuil wrote:
> > From: Hans Verkuil <hans.verkuil@cisco.com>
> > 
> > The crop and selection pad ops are duplicates. Replace all uses of
> > get/set_crop by get/set_selection. This will make it possible to drop
> > get/set_crop altogether.
> > 
> > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> > Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Cc: Prabhakar Lad <prabhakar.csengg@gmail.com>
> > Cc: Philipp Zabel <p.zabel@pengutronix.de>
> 
> For both:
> 
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> 
> Another point I'd like to draw attention to are the reserved fields --- some
> drivers appear to zero them whereas some pay no attention. Shouldn't we
> check in the sub-device IOCTL handler that the user has zeroed them, or
> zero them for the user? I think this has probably been discussed before on
> V4L2. Both have their advantages, probably zeroing them in the framework
> would be the best option. What do you think?

I think we should at least be consistent across drivers. Duplicating checks 
across drivers being more error-prone it would likely be better to implement 
them in core code. The question that remains to be answered is whether we can 
consider that bridge drivers will correctly zero reserved fields when using 
the API internally. If not, we'll need wrapper functions around subdev 
operations to zero reserved fields, or possibly just to output a WARN_ON (as a 
bridge driver bug should be fixed instead of silently ignored).

> Definitely out of scope of this set.

-- 
Regards,

Laurent Pinchart