[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aKVLw_ouLWezuWJF@archiso>
Date: Wed, 20 Aug 2025 04:14:59 +0000
From: Elle Rhumsaa <elle@...thered-steel.dev>
To: Daniel Almeida <daniel.almeida@...labora.com>
Cc: Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>,
Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <lossin@...nel.org>,
Andreas Hindborg <a.hindborg@...nel.org>,
Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
Danilo Krummrich <dakr@...nel.org>,
Alexandre Courbot <acourbot@...dia.com>,
linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org,
kernel@...labora.com, linux-media@...r.kernel.org
Subject: Re: [PATCH 5/7] rust: v4l2: add device capabilities
On Mon, Aug 18, 2025 at 02:49:51AM -0300, Daniel Almeida wrote:
> All v4l2 devices must expose a given set of capabilities to the v4l2
> core and to userspace. Add support for that in v4l2::caps. This will be
> used by the next patch in order to add support for VIDIOC_QUERYCAP.
>
> Signed-off-by: Daniel Almeida <daniel.almeida@...labora.com>
> ---
> rust/kernel/media/v4l2/caps.rs | 193 ++++++++++++++++++++++++++++++++++++++++
> rust/kernel/media/v4l2/mod.rs | 2 +
> rust/kernel/media/v4l2/video.rs | 6 +-
> 3 files changed, 200 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/media/v4l2/caps.rs b/rust/kernel/media/v4l2/caps.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..4b0164c58d13e83e728091228fae025dbce59bc8
> --- /dev/null
> +++ b/rust/kernel/media/v4l2/caps.rs
> @@ -0,0 +1,193 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// SPDX-copyrightText: Copyright (C) 2025 Collabora Ltd.
> +
> +use crate::{prelude::*, str::CStr, types::Opaque};
> +use core::cmp::min;
> +
> +/// A wrapper over `struct v4l2_capability`.
> +///
> +/// # Invariants
> +///
> +/// - `self.0` is a valid instance of `struct v4l2_capability`.
> +/// - All strings in `struct v4l2_capability` are valid C strings.
> +///
> +/// TODO: This type would benefit from an #[derive(accessor)] macro to automate
> +/// the boilerplate below.
> +#[repr(transparent)]
> +pub struct Capabilities(Opaque<bindings::v4l2_capability>);
> +
> +impl Capabilities {
> + /// Returns the raw pointer to the `struct v4l2_capability`.
> + pub fn as_raw(&self) -> *const bindings::v4l2_capability {
> + self.0.get()
> + }
> +
> + /// Converts a raw pointer to a `Capabilities` reference.
> + ///
> + /// # Safety
> + ///
> + /// - `ptr` must be a valid pointer to a `struct v4l2_capability` that must
> + /// remain valid for the lifetime 'a.
> + /// - the returned reference must obey Rust's reference rules.
> + pub unsafe fn from_raw<'a>(ptr: *mut bindings::v4l2_capability) -> &'a mut Self {
> + // SAFETY: `ptr` is a valid pointer to a `struct v4l2_capability` as per the
> + // safety requirements of this function.
> + unsafe { &mut *(ptr.cast::<Self>()) }
> + }
> +
> + fn inner(&self) -> &bindings::v4l2_capability {
> + // SAFETY: safe as per the invariants of `Capabilities`
> + unsafe { &*self.0.get() }
> + }
> +
> + fn inner_mut(&mut self) -> &mut bindings::v4l2_capability {
> + // SAFETY: safe as per the invariants of `Capabilities`
> + unsafe { &mut *self.0.get() }
> + }
> +
> + /// Returns the `driver` field.
> + pub fn driver(&self) -> &CStr {
> + // SAFETY: safe as per the invariants of `Capabilities`
> + unsafe { CStr::from_bytes_with_nul_unchecked(&self.inner().driver) }
> + }
> +
> + /// Sets the `driver` field.
> + pub fn set_driver(&mut self, name: &CStr) -> Result {
> + if name.len_with_nul() > self.inner().driver.len() {
> + return Err(EINVAL);
> + }
> +
> + let cap = self.inner_mut();
> + let src = name.to_bytes_with_nul();
> + let n = min(src.len(), cap.driver.len());
> + cap.driver[..n].copy_from_slice(&src[..n]);
> +
> + Ok(())
> + }
> +
> + /// Returns the `card` field.
> + pub fn card(&self) -> &CStr {
> + // SAFETY: safe as per the invariants of `Capabilities`
> + unsafe { CStr::from_bytes_with_nul_unchecked(&self.inner().card) }
> + }
> +
> + /// Sets the `card` field.
> + pub fn set_card(&mut self, card: &CStr) -> Result {
> + if card.len_with_nul() > self.inner().card.len() {
> + return Err(EINVAL);
> + }
> +
> + let cap = self.inner_mut();
> + let src = card.to_bytes_with_nul();
> + let n = min(src.len(), cap.card.len());
> + cap.card[..n].copy_from_slice(&src[..n]);
> +
> + Ok(())
> + }
> +
> + /// Returns the `bus_info` field.
> + pub fn bus_info(&self) -> &CStr {
> + // SAFETY: safe as per the invariants of `Capabilities`
> + unsafe { CStr::from_bytes_with_nul_unchecked(&self.inner().bus_info) }
> + }
> +
> + /// Sets the `bus_info` field.
> + pub fn set_bus_info(&mut self, info: &CStr) -> Result {
> + if info.len_with_nul() > self.inner().bus_info.len() {
> + return Err(EINVAL);
> + }
> +
> + let cap = self.inner_mut();
> + let src = info.to_bytes_with_nul();
> + let n = min(src.len(), cap.bus_info.len());
> + cap.bus_info[..n].copy_from_slice(&src[..n]);
> +
> + Ok(())
> + }
> +
> + /// Returns the `version` field.
> + pub fn version(&self) -> u32 {
> + self.inner().version
> + }
> +
> + /// Sets the `version` field.
> + pub fn set_version(&mut self, v: u32) {
> + self.inner_mut().version = v;
> + }
> +
> + /// Returns the `capabilities` field.
> + pub fn capabilities(&self) -> u32 {
> + self.inner().capabilities
> + }
> +
> + /// Sets the `capabilities` field.
> + pub fn set_capabilities(&mut self, caps: u32) {
> + self.inner_mut().capabilities = caps;
> + }
> +
> + /// Returns the `device_caps` field.
> + pub fn device_caps(&self) -> Option<DeviceCaps> {
> + if self.inner().device_caps == 0 {
> + None
> + } else {
> + Some(DeviceCaps(self.inner().device_caps))
> + }
> + }
> +
> + /// Sets the `device_caps` field.
> + pub fn set_device_caps(&mut self, caps: DeviceCaps) {
> + self.inner_mut().device_caps = caps.as_raw();
> + }
> +}
> +
> +/// Device capabilities.
> +///
> +/// They can be combined with the operators `|`, `&`, and `!`.
> +///
> +/// Values can be used from the [`device_caps`] module.
> +#[derive(Clone, Copy, PartialEq)]
> +pub struct DeviceCaps(u32);
> +
> +impl DeviceCaps {
> + /// Get the raw representation of the device capabilties.
> + pub(crate) fn as_raw(self) -> u32 {
> + self.0
> + }
> +
> + /// Check whether `cap` is contained in `self`.
> + pub fn contains(self, cap: DeviceCaps) -> bool {
> + (self & cap) == cap
> + }
> +}
> +
> +impl core::ops::BitOr for DeviceCaps {
> + type Output = Self;
> + fn bitor(self, rhs: Self) -> Self::Output {
> + Self(self.0 | rhs.0)
> + }
> +}
> +
> +impl core::ops::BitAnd for DeviceCaps {
> + type Output = Self;
> + fn bitand(self, rhs: Self) -> Self::Output {
> + Self(self.0 & rhs.0)
> + }
> +}
> +
> +impl core::ops::Not for DeviceCaps {
> + type Output = Self;
> + fn not(self) -> Self::Output {
> + Self(!self.0)
> + }
> +}
> +
> +/// Device capabilities.
> +pub mod device_caps {
> + use super::DeviceCaps;
> +
> + /// The device is a video capture device.
> + pub const VIDEO_CAPTURE: DeviceCaps = DeviceCaps(bindings::V4L2_CAP_VIDEO_CAPTURE);
> +
> + /// The device is a video output device.
> + pub const VIDEO_OUTPUT: DeviceCaps = DeviceCaps(bindings::V4L2_CAP_VIDEO_OUTPUT);
> +}
These can probably be implemented as associated constants:
```rust
impl DeviceCaps {
pub const VIDEO_CAPTURE: Self = Self(bindings::V4L2_CAP_VIDEO_CAPTURE);
pub const VIDEO_OUTPUT: Self = Self(bindings::V4L2_CAP_VIDEO_OUTPUT);
```
That would allow for a slightly more ergonomic usage:
```rust
let _ = DeviceCaps::VIDEO_CAPTURE;
```
> diff --git a/rust/kernel/media/v4l2/mod.rs b/rust/kernel/media/v4l2/mod.rs
> index 1195c18f1336891c4b9b194d4e7e5cd40989ace9..1d8241f8a2230954371965bb91b20e726f144dce 100644
> --- a/rust/kernel/media/v4l2/mod.rs
> +++ b/rust/kernel/media/v4l2/mod.rs
> @@ -11,5 +11,7 @@
> /// Support for Video for Linux 2 (V4L2) video devices.
> pub mod video;
>
> +/// Support for Video for Linux 2 device capabilities.
> +pub mod caps;
> /// Support for Video for Linux 2 (V4L2) file handles.
> pub mod file;
> diff --git a/rust/kernel/media/v4l2/video.rs b/rust/kernel/media/v4l2/video.rs
> index 7ef2111c32ca55a2bced8325cd883b28204dc3ee..c0ac99a8234d2f7a8effd4701b9f7440236540c8 100644
> --- a/rust/kernel/media/v4l2/video.rs
> +++ b/rust/kernel/media/v4l2/video.rs
> @@ -20,7 +20,7 @@
> use crate::{
> alloc,
> error::to_result,
> - media::v4l2::{self, file::DriverFile, video},
> + media::v4l2::{self, caps::DeviceCaps, file::DriverFile, video},
> prelude::*,
> types::{ARef, AlwaysRefCounted, Opaque},
> };
> @@ -158,6 +158,9 @@ pub trait Driver: v4l2::device::Driver {
>
> /// The name to use when registering the device node.
> const NAME: &'static CStr;
> +
> + /// The capabilities offered by this device node.
> + const CAPS: DeviceCaps;
> }
>
> struct DeviceOptions<'a, T: Driver> {
> @@ -180,6 +183,7 @@ fn into_raw(self) -> bindings::video_device {
> vfl_dir: T::DIRECTION as c_uint,
> release: Some(Device::<T>::release_callback),
> fops: super::file::FileVtable::<T::File>::build(),
> + device_caps: T::CAPS.as_raw(),
> // SAFETY: All zeros is valid for the rest of the fields in this C
> // type.
> ..unsafe { MaybeUninit::zeroed().assume_init() }
>
> --
> 2.50.1
Reviewed-by: Elle Rhumsaa <elle@...thered-steel.dev>
Powered by blists - more mailing lists