Em Fri, 11 Aug 2017 08:05:03 +0200 Hans Verkuil <hverkuil@xxxxxxxxx> escreveu: > On 11/08/17 02:16, Mauro Carvalho Chehab wrote: > > In the past, only string controls were pointers. That changed when compounded > > types got added, but the compat32 code was not updated. > > > > We could just add those controls there, but maintaining it is flaw, as we > > often forget about the compat code. So, instead, rely on the control type, > > as this is always updated when new controls are added. > > > > As both v4l2-ctrl and compat32 code are at videodev.ko module, we can > > move the ctrl_is_pointer() helper function to v4l2-ctrl.c. > > This series doesn't really solve anything: > > - it introduces a circular dependency between two modules What two modules? both v4l2-ctrl and compat32 belong to the *same* module. See the Makefile: videodev-objs := v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \ v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o \ v4l2-async.o ifeq ($(CONFIG_COMPAT),y) videodev-objs += v4l2-compat-ioctl32.o endif Both belong to videodev module. IMHO, the best is to move whatever control check logic it might need to v4l2-ctrls. > - it doesn't handle driver-custom controls (the old code didn't either). For > example vivid has custom pointer controls. True. > - it replaces a list of control IDs with a list of type IDs, which also has to > be kept up to date. True, but at least after the patch, the ancillary function is together with the code that handles the controls. Also, we don't introduce new types too often. > > I thought this over and I have a better and much simpler idea. I'll post a > patch for that. OK. > > Regards, > > Hans > > > > > --- > > > > Re-sending this patch series, as it was c/c to the linux-doc ML by mistake. > > > > Mauro Carvalho Chehab (3): > > media: v4l2-ctrls.h: better document the arguments for v4l2_ctrl_fill > > media: v4l2-ctrls: prepare the function to be used by compat32 code > > media: compat32: reimplement ctrl_is_pointer() > > > > drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 18 +--------- > > drivers/media/v4l2-core/v4l2-ctrls.c | 49 +++++++++++++++++++++++++-- > > include/media/v4l2-ctrls.h | 28 ++++++++++----- > > 3 files changed, 67 insertions(+), 28 deletions(-) > > > Thanks, Mauro
