On 11/08/17 11:00, Mauro Carvalho Chehab wrote:
> Em Fri, 11 Aug 2017 08:05:03 +0200
> Hans Verkuil <hverk...@xs4all.nl> 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
My fault, I should have checked. The 'circular dependency' comment in the code
referred to the bad old days when v4l2_ctrl_fill was in v4l2-common.c and that
was a separate module. All that has long since changed.
> 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.
I have the patch ready, but I need to test it first with vivid.
Regards,
Hans
