mbox series

[PATCHv3,0/2] media: v4l2-core: v4l2-ctrls: check for handler_new_ref misuse

Message ID cover.1731399278.git.hverkuil@xs4all.nl
Headers show
Series media: v4l2-core: v4l2-ctrls: check for handler_new_ref misuse | expand

Message

Hans Verkuil Nov. 12, 2024, 8:14 a.m. UTC
This supersedes (1). 

The first patch fixes the tegra-video driver: it now creates the new
controls for the control handler first, before calling v4l2_ctrl_add_handler().

This prevents a warning, which is introduced in the next patch.

The second patch checks for adding a duplicate control to the same handler,
and returns an error, but it also adds a warning if new controls are added
to a handler after v4l2_ctrl_add_handler() is called.

Eventually this sequence should result in an error instead of a warning,
but I feel it is better to keep the warning for a few kernel cycles
to make sure any broken driver is fixed.

Comments

Ricardo Ribalda Delgado Nov. 26, 2024, 6:24 p.m. UTC | #1
Hi Hans

On Tue, Nov 12, 2024 at 9:41 AM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> This supersedes (1).
>
> The first patch fixes the tegra-video driver: it now creates the new
> controls for the control handler first, before calling v4l2_ctrl_add_handler().
>
> This prevents a warning, which is introduced in the next patch.
>
> The second patch checks for adding a duplicate control to the same handler,
> and returns an error, but it also adds a warning if new controls are added
> to a handler after v4l2_ctrl_add_handler() is called.

In the uvc driver, users can create controls with the ioct UVCIOC_CTRL_MAP
https://docs.kernel.org/userspace-api/media/drivers/uvcvideo.html

I guess that would be the equivalent of adding controls after
v4l2_ctrl_add_handler()

Right now, uvc does not use the control framework, but I would like to
dream about a world where this is possible :)

Is there a chance that you consider that usecase?

Regards!

>
> Eventually this sequence should result in an error instead of a warning,
> but I feel it is better to keep the warning for a few kernel cycles
> to make sure any broken driver is fixed.
>
> From my analysis it is only the tegra-video driver that has this issue,
> but I may have missed a corner case somewhere.
>
> Regards,
>
>         Hans
>
> 1: https://patchwork.linuxtv.org/project/linux-media/patch/ddb6e006-7440-41c5-8aaa-685b058418b4@xs4all.nl/
>
> Hans Verkuil (2):
>   media: staging: tegra-video: postpone v4l2_ctrl_add_handler
>   media: v4l2-core: v4l2-ctrls: check for handler_new_ref misuse
>
>  drivers/media/v4l2-core/v4l2-ctrls-core.c | 46 ++++++++++++++++++++---
>  drivers/staging/media/tegra-video/vi.c    | 11 +++---
>  include/media/v4l2-ctrls.h                |  5 +++
>  3 files changed, 50 insertions(+), 12 deletions(-)
>
> --
> 2.45.2
>
>