From 618882c92681de18e9bd99d2a88bb21c897283f3 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Sun, 10 Aug 2025 04:29:50 +0300 Subject: media: Wrap file->private_data access with a helper function Accessing file->private_data manually to retrieve the v4l2_fh pointer is error-prone, as the field is a void * and will happily convert implicitly to any pointer type. To avoid direct access to file->private_data, introduce a new inline function that retrieves the v4l2_fh pointer, and use it to replace common access patterns through the kernel. Changes to drivers have been generated with the following coccinelle semantic patch: @@ struct file *filp; identifier fh; @@ - struct v4l2_fh *fh = filp->private_data; + struct v4l2_fh *fh = file_to_v4l2_fh(filp); Manual changes have been applied to Documentation/ to update the usage patterns, and to include/media/v4l2-fh.h to add the new function. While at it, fix a typo in the title of v4l2-fh.rst: the file describes the "file handles" API, not "file handlers". No functional change is intended, this only paves the way to remove direct accesses to file->private_data and make V4L2 drivers safer. Other accesses to the field will be addressed separately. Signed-off-by: Laurent Pinchart Signed-off-by: Hans Verkuil --- Documentation/driver-api/media/v4l2-fh.rst | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) (limited to 'Documentation/driver-api') diff --git a/Documentation/driver-api/media/v4l2-fh.rst b/Documentation/driver-api/media/v4l2-fh.rst index 3eeaa8da0c9e..2c87b74578d9 100644 --- a/Documentation/driver-api/media/v4l2-fh.rst +++ b/Documentation/driver-api/media/v4l2-fh.rst @@ -1,7 +1,7 @@ .. SPDX-License-Identifier: GPL-2.0 -V4L2 File handlers ------------------- +V4L2 File handles +----------------- struct v4l2_fh provides a way to easily keep file handle specific data that is used by the V4L2 framework. @@ -18,7 +18,9 @@ This bit is set whenever :c:func:`v4l2_fh_init` is called. struct v4l2_fh is allocated as a part of the driver's own file handle structure and ``file->private_data`` is set to it in the driver's ``open()`` -function by the driver. +function by the driver. The :c:type:`v4l2_fh` file handle can be retrieved +from the :c:type:`file` using :c:func:`file_to_v4l2_fh`. Drivers must not +access ``file->private_data`` directly. In many cases the struct v4l2_fh will be embedded in a larger structure. In that case you should call: @@ -63,7 +65,7 @@ Example: int my_release(struct file *file) { - struct v4l2_fh *fh = file->private_data; + struct v4l2_fh *fh = file_to_v4l2_fh(file); struct my_fh *my_fh = container_of(fh, struct my_fh, fh); ... @@ -78,11 +80,9 @@ Below is a short description of the :c:type:`v4l2_fh` functions used: :c:func:`v4l2_fh_init ` (:c:type:`fh `, :c:type:`vdev `) - - Initialise the file handle. This **MUST** be performed in the driver's :c:type:`v4l2_file_operations`->open() handler. - :c:func:`v4l2_fh_add ` (:c:type:`fh `) @@ -101,6 +101,10 @@ Below is a short description of the :c:type:`v4l2_fh` functions used: - Uninitialise the file handle. After uninitialisation the :c:type:`v4l2_fh` memory can be freed. +:c:func:`file_to_v4l2_fh ` +(struct file \*filp) + +- Retrieve the :c:type:`v4l2_fh` instance associated with a :c:type:`file`. If struct v4l2_fh is not embedded, then you can use these helper functions: -- cgit v1.2.3 From 47f4b1acb4d505b1e7e81d8e0ebce774422b8c2e Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Sun, 10 Aug 2025 04:30:08 +0300 Subject: media: Set file->private_data in v4l2_fh_add() All the drivers that use v4l2_fh and call v4l2_fh_add() manually store a pointer to the v4l2_fh instance in file->private_data in their video device .open() file operation handler. Move the code to the v4l2_fh_add() function to avoid direct access to file->private_data in drivers. This requires adding a file pointer argument to the function. Changes to drivers have been generated with the following coccinelle semantic patch: @@ expression fh; identifier filp; identifier open; type ret; @@ ret open(..., struct file *filp, ...) { <... - filp->private_data = fh; ... - v4l2_fh_add(fh); + v4l2_fh_add(fh, filp); ...> } @@ expression fh; identifier filp; identifier open; type ret; @@ ret open(..., struct file *filp, ...) { <... - v4l2_fh_add(fh); + v4l2_fh_add(fh, filp); ... - filp->private_data = fh; ...> } Manual changes have been applied to Documentation/ to update the usage patterns, to drivers/media/v4l2-core/v4l2-fh.c to update the v4l2_fh_add() prototype set file->private_data, and to include/media/v4l2-fh.h to update the v4l2_fh_add() function prototype and its documentation. Additionally, white space issues have been fixed manually in drivers/media/platform/nvidia/tegra-vde/v4l2.c, drivers/media/platform/rockchip/rkvdec/rkvdec.c, drivers/media/v4l2-core/v4l2-fh.c and drivers/staging/most/video/video.c. Signed-off-by: Laurent Pinchart Signed-off-by: Hans Verkuil --- Documentation/driver-api/media/v4l2-fh.rst | 46 ++++++++++++++++-------------- 1 file changed, 24 insertions(+), 22 deletions(-) (limited to 'Documentation/driver-api') diff --git a/Documentation/driver-api/media/v4l2-fh.rst b/Documentation/driver-api/media/v4l2-fh.rst index 2c87b74578d9..a7393067f5db 100644 --- a/Documentation/driver-api/media/v4l2-fh.rst +++ b/Documentation/driver-api/media/v4l2-fh.rst @@ -11,25 +11,22 @@ data that is used by the V4L2 framework. since it is also used to implement priority handling (:ref:`VIDIOC_G_PRIORITY`). -The users of :c:type:`v4l2_fh` (in the V4L2 framework, not the driver) know -whether a driver uses :c:type:`v4l2_fh` as its ``file->private_data`` pointer -by testing the ``V4L2_FL_USES_V4L2_FH`` bit in :c:type:`video_device`->flags. -This bit is set whenever :c:func:`v4l2_fh_init` is called. - -struct v4l2_fh is allocated as a part of the driver's own file handle -structure and ``file->private_data`` is set to it in the driver's ``open()`` -function by the driver. The :c:type:`v4l2_fh` file handle can be retrieved -from the :c:type:`file` using :c:func:`file_to_v4l2_fh`. Drivers must not -access ``file->private_data`` directly. - -In many cases the struct v4l2_fh will be embedded in a larger -structure. In that case you should call: - -#) :c:func:`v4l2_fh_init` and :c:func:`v4l2_fh_add` in ``open()`` -#) :c:func:`v4l2_fh_del` and :c:func:`v4l2_fh_exit` in ``release()`` - -Drivers can extract their own file handle structure by using the container_of -macro. +struct v4l2_fh is allocated in the driver's ``open()`` file operation handler. +It is typically embedded in a larger driver-specific structure. The +:c:type:`v4l2_fh` must be initialized with a call to :c:func:`v4l2_fh_init`, +and added to the video device with :c:func:`v4l2_fh_add`. This associates the +:c:type:`v4l2_fh` with the :c:type:`file` by setting ``file->private_data`` to +point to the :c:type:`v4l2_fh`. + +Similarly, the struct v4l2_fh is freed in the driver's ``release()`` file +operation handler. It must be removed from the video device with +:c:func:`v4l2_fh_del` and cleaned up with :c:func:`v4l2_fh_exit` before being +freed. + +Drivers must not access ``file->private_data`` directly. They can retrieve the +:c:type:`v4l2_fh` associated with a :c:type:`file` by calling +:c:func:`file_to_v4l2_fh`. Drivers can extract their own file handle structure +by using the container_of macro. Example: @@ -58,8 +55,7 @@ Example: ... - file->private_data = &my_fh->fh; - v4l2_fh_add(&my_fh->fh); + v4l2_fh_add(&my_fh->fh, file); return 0; } @@ -84,7 +80,7 @@ Below is a short description of the :c:type:`v4l2_fh` functions used: :c:type:`v4l2_file_operations`->open() handler. :c:func:`v4l2_fh_add ` -(:c:type:`fh `) +(:c:type:`fh `, struct file \*filp) - Add a :c:type:`v4l2_fh` to :c:type:`video_device` file handle list. Must be called once the file handle is completely initialized. @@ -138,6 +134,12 @@ associated device node: - Same, but it calls v4l2_fh_is_singular with filp->private_data. +.. note:: + The V4L2 framework knows whether a driver uses :c:type:`v4l2_fh` as its + ``file->private_data`` pointer by testing the ``V4L2_FL_USES_V4L2_FH`` + bit in :c:type:`video_device`->flags. This bit is set whenever + :c:func:`v4l2_fh_init` is called. + V4L2 fh functions and data structures ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -- cgit v1.2.3 From 277966749f46bc6292c4052b4e66a554f193a78a Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Sun, 10 Aug 2025 04:30:09 +0300 Subject: media: Reset file->private_data to NULL in v4l2_fh_del() Multiple drivers that use v4l2_fh and call v4l2_fh_del() manually reset the file->private_data pointer to NULL in their video device .release() file operation handler. Move the code to the v4l2_fh_del() function to avoid direct access to file->private_data in drivers. This requires adding a file pointer argument to the function. Changes to drivers have been generated with the following coccinelle semantic patch: @@ expression fh; identifier filp; identifier release; type ret; @@ ret release(..., struct file *filp, ...) { <... - filp->private_data = NULL; ... - v4l2_fh_del(fh); + v4l2_fh_del(fh, filp); ...> } @@ expression fh; identifier filp; identifier release; type ret; @@ ret release(..., struct file *filp, ...) { <... - v4l2_fh_del(fh); + v4l2_fh_del(fh, filp); ... - filp->private_data = NULL; ...> } @@ expression fh; identifier filp; identifier release; type ret; @@ ret release(..., struct file *filp, ...) { <... - v4l2_fh_del(fh); + v4l2_fh_del(fh, filp); ...> } Manual changes have been applied to Documentation/ to update the usage patterns, to drivers/media/v4l2-core/v4l2-fh.c to update the v4l2_fh_del() prototype and reset file->private_data, and to include/media/v4l2-fh.h to update the v4l2_fh_del() function prototype and its documentation. Additionally, white space issues have been fixed manually in drivers/usb/gadget/function/uvc_v4l2.c Signed-off-by: Laurent Pinchart Signed-off-by: Hans Verkuil --- Documentation/driver-api/media/v4l2-fh.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'Documentation/driver-api') diff --git a/Documentation/driver-api/media/v4l2-fh.rst b/Documentation/driver-api/media/v4l2-fh.rst index a7393067f5db..afcad22ead7c 100644 --- a/Documentation/driver-api/media/v4l2-fh.rst +++ b/Documentation/driver-api/media/v4l2-fh.rst @@ -65,7 +65,7 @@ Example: struct my_fh *my_fh = container_of(fh, struct my_fh, fh); ... - v4l2_fh_del(&my_fh->fh); + v4l2_fh_del(&my_fh->fh, file); v4l2_fh_exit(&my_fh->fh); kfree(my_fh); return 0; @@ -86,7 +86,7 @@ Below is a short description of the :c:type:`v4l2_fh` functions used: Must be called once the file handle is completely initialized. :c:func:`v4l2_fh_del ` -(:c:type:`fh `) +(:c:type:`fh `, struct file \*filp) - Unassociate the file handle from :c:type:`video_device`. The file handle exit function may now be called. -- cgit v1.2.3 From bb4d6be205dae94aa2d3c3a1ad814dad90d4fd62 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Sun, 10 Aug 2025 04:30:14 +0300 Subject: media: Drop V4L2_FL_USES_V4L2_FH checks Now that all drivers use v4l2_fh, we can drop the V4L2_FL_USES_V4L2_FH checks through the V4L2 core. To ensure that all new drivers use v4l2_fh, keep setting the V4L2_FL_USES_V4L2_FH flag in v4l2_fh_init(), and verify it is set after the .open() file operation returns. Signed-off-by: Laurent Pinchart Signed-off-by: Hans Verkuil --- Documentation/driver-api/media/v4l2-fh.rst | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) (limited to 'Documentation/driver-api') diff --git a/Documentation/driver-api/media/v4l2-fh.rst b/Documentation/driver-api/media/v4l2-fh.rst index afcad22ead7c..a934caa483a4 100644 --- a/Documentation/driver-api/media/v4l2-fh.rst +++ b/Documentation/driver-api/media/v4l2-fh.rst @@ -3,13 +3,8 @@ V4L2 File handles ----------------- -struct v4l2_fh provides a way to easily keep file handle specific -data that is used by the V4L2 framework. - -.. attention:: - New drivers must use struct v4l2_fh - since it is also used to implement priority handling - (:ref:`VIDIOC_G_PRIORITY`). +struct v4l2_fh provides a way to easily keep file handle specific data that is +used by the V4L2 framework. Its usage is mandatory in all drivers. struct v4l2_fh is allocated in the driver's ``open()`` file operation handler. It is typically embedded in a larger driver-specific structure. The @@ -134,12 +129,6 @@ associated device node: - Same, but it calls v4l2_fh_is_singular with filp->private_data. -.. note:: - The V4L2 framework knows whether a driver uses :c:type:`v4l2_fh` as its - ``file->private_data`` pointer by testing the ``V4L2_FL_USES_V4L2_FH`` - bit in :c:type:`video_device`->flags. This bit is set whenever - :c:func:`v4l2_fh_init` is called. - V4L2 fh functions and data structures ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -- cgit v1.2.3 From e2b1ebd72c81ca7bcceb0ed2cea2200978b6e975 Mon Sep 17 00:00:00 2001 From: Mehdi Djait Date: Thu, 26 Jun 2025 15:33:53 +0200 Subject: Documentation: media: camera-sensor: Mention v4l2_devm_sensor_clk_get() for obtaining the clock Add the new v4l2 helper devm_v4l2_sensor_clk_get() to Documentation. the helper works on both DT- and ACPI-based platforms to retrieve a reference to the clock producer from firmware. Reviewed-by: Lad Prabhakar Reviewed-by: Laurent Pinchart Signed-off-by: Mehdi Djait Co-developed-by: Laurent Pinchart Signed-off-by: Laurent Pinchart Signed-off-by: Sakari Ailus Signed-off-by: Hans Verkuil --- Documentation/driver-api/media/camera-sensor.rst | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) (limited to 'Documentation/driver-api') diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst index c290833165e6..94bd1dae82d5 100644 --- a/Documentation/driver-api/media/camera-sensor.rst +++ b/Documentation/driver-api/media/camera-sensor.rst @@ -29,21 +29,31 @@ used in the system. Using another frequency may cause harmful effects elsewhere. Therefore only the pre-determined frequencies are configurable by the user. +The external clock frequency shall be retrieved by obtaining the external clock +using the ``devm_v4l2_sensor_clk_get()`` helper function, and then getting its +frequency with ``clk_get_rate()``. Usage of the helper function guarantees +correct behaviour regardless of whether the sensor is integrated in a DT-based +or ACPI-based system. + ACPI ~~~~ -Read the ``clock-frequency`` _DSD property to denote the frequency. The driver -can rely on this frequency being used. +ACPI-based systems typically don't register the sensor external clock with the +kernel, but specify the external clock frequency in the ``clock-frequency`` +_DSD property. The ``devm_v4l2_sensor_clk_get()`` helper creates and returns a +fixed clock set at that rate. Devicetree ~~~~~~~~~~ -The preferred way to achieve this is using ``assigned-clocks``, -``assigned-clock-parents`` and ``assigned-clock-rates`` properties. See the -`clock device tree bindings +Devicetree-based systems declare the sensor external clock in the device tree +and reference it from the sensor node. The preferred way to select the external +clock frequency is to use the ``assigned-clocks``, ``assigned-clock-parents`` +and ``assigned-clock-rates`` properties in the sensor node to set the clock +rate. See the `clock device tree bindings `_ -for more information. The driver then gets the frequency using -``clk_get_rate()``. +for more information. The ``devm_v4l2_sensor_clk_get()`` helper retrieves and +returns that clock. This approach has the drawback that there's no guarantee that the frequency hasn't been modified directly or indirectly by another driver, or supported by -- cgit v1.2.3 From 9bff72cb99b10ead19ab1e36d240cc54bc4954b1 Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Tue, 24 Jun 2025 08:39:35 +0200 Subject: Documentation: update Hans Verkuil's email address Replace hverkuil@xs4all.nl by hverkuil@kernel.org. Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- Documentation/driver-api/media/maintainer-entry-profile.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'Documentation/driver-api') diff --git a/Documentation/driver-api/media/maintainer-entry-profile.rst b/Documentation/driver-api/media/maintainer-entry-profile.rst index ad96a89ee916..2127e5b15e8f 100644 --- a/Documentation/driver-api/media/maintainer-entry-profile.rst +++ b/Documentation/driver-api/media/maintainer-entry-profile.rst @@ -75,7 +75,7 @@ The media maintainers that work on specific areas of the subsystem are: Sean Young - HDMI CEC: - Hans Verkuil + Hans Verkuil - Media controller drivers: Laurent Pinchart @@ -84,7 +84,7 @@ The media maintainers that work on specific areas of the subsystem are: Sakari Ailus - V4L2 drivers and core V4L2 frameworks: - Hans Verkuil + Hans Verkuil The subsystem maintainer is: Mauro Carvalho Chehab -- cgit v1.2.3