* [PATCH v5 0/2] rust: leds: add led classdev abstractions
@ 2025-10-18 20:59 Markus Probst
2025-10-18 20:59 ` [PATCH v5 1/2] rust: Add trait to convert a device reference to a bus device reference Markus Probst
2025-10-18 20:59 ` [PATCH v5 2/2] rust: leds: add basic led classdev abstractions Markus Probst
0 siblings, 2 replies; 4+ messages in thread
From: Markus Probst @ 2025-10-18 20:59 UTC (permalink / raw)
To: Greg Kroah-Hartman, Miguel Ojeda, Alex Gaynor, Danilo Krummrich,
Rafael J. Wysocki, Igor Korotin, Lee Jones, Pavel Machek
Cc: Dave Ertman, Ira Weiny, Leon Romanovsky, Boqun Feng, Gary Guo,
bjorn3_gh, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Daniel Almeida, Bjorn Helgaas, kwilczynski,
linux-kernel, rust-for-linux, linux-pci, linux-leds,
Markus Probst
This patch currently depends on
https://lore.kernel.org/rust-for-linux/20251005102226.41876-1-igor.korotin.linux@gmail.com/.
This patch series has previously been contained in
https://lore.kernel.org/rust-for-linux/20251008181027.662616-1-markus.probst@posteo.de/T/#t
which added a rust written led driver for a microcontroller via i2c.
As the reading and writing to the i2c client via the register!
macro has not been implemented yet [1], the patch series will only
contain the additional abstractions required.
[1] https://lore.kernel.org/rust-for-linux/DDDS2V0V2NVJ.16ZKXCKUA1HUV@kernel.org/
The following changes were made:
* add abstraction to convert a device reference to a bus device
reference for use in class device callbacks
* add basic led classdev abstractions to register and unregister leds
Changes since v4:
* add abstraction to convert a device reference to a bus device
reference
* require the bus device as parent device and provide it in class device
callbacks
* remove Pin<Vec<_>> abstraction (as not relevant for the led
abstractions)
* fixed formatting in `led::Device::new`
* fixed `LedOps::BLOCKING` did the inverse effect
Changes since v3:
* fixed kunit tests failing because of example in documentation
Changes since v2:
* return `Devres` on `led::Device` creation
* replace KBox<T> with T in struct definition
* increment and decrement reference-count of fwnode
* make a device parent mandatory for led classdev creation
* rename `led::Handler` to `led::LedOps`
* add optional `brightness_get` function to `led::LedOps`
* use `#[vtable]` instead of `const BLINK: bool`
* use `Opaque::cast_from` instead of casting a pointer
* improve documentation
* improve support for older rust versions
* use `&Device<Bound>` for parent
Changes since v1:
* fixed typos noticed by Onur Özkan
Markus Probst (2):
rust: Add trait to convert a device reference to a bus device
reference
rust: leds: add basic led classdev abstractions
rust/kernel/auxiliary.rs | 7 +
rust/kernel/device.rs | 41 +++++
rust/kernel/i2c.rs | 7 +
rust/kernel/led.rs | 375 +++++++++++++++++++++++++++++++++++++++
rust/kernel/lib.rs | 1 +
rust/kernel/pci.rs | 7 +
rust/kernel/usb.rs | 6 +
7 files changed, 444 insertions(+)
create mode 100644 rust/kernel/led.rs
--
2.49.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v5 1/2] rust: Add trait to convert a device reference to a bus device reference
2025-10-18 20:59 [PATCH v5 0/2] rust: leds: add led classdev abstractions Markus Probst
@ 2025-10-18 20:59 ` Markus Probst
2025-10-19 14:28 ` Danilo Krummrich
2025-10-18 20:59 ` [PATCH v5 2/2] rust: leds: add basic led classdev abstractions Markus Probst
1 sibling, 1 reply; 4+ messages in thread
From: Markus Probst @ 2025-10-18 20:59 UTC (permalink / raw)
To: Greg Kroah-Hartman, Miguel Ojeda, Alex Gaynor, Danilo Krummrich,
Rafael J. Wysocki, Igor Korotin, Lee Jones, Pavel Machek
Cc: Dave Ertman, Ira Weiny, Leon Romanovsky, Boqun Feng, Gary Guo,
bjorn3_gh, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Daniel Almeida, Bjorn Helgaas, kwilczynski,
linux-kernel, rust-for-linux, linux-pci, linux-leds,
Markus Probst
Implement the `IntoBusDevice` trait for converting a `Device` reference to a
bus device reference for all bus devices. `Device` implements this trait as a
fallback.
The `IntoBusDevice` trait allows abstractions to provide the bus device in
class device callbacks.
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
rust/kernel/auxiliary.rs | 7 +++++++
rust/kernel/device.rs | 41 ++++++++++++++++++++++++++++++++++++++++
rust/kernel/i2c.rs | 7 +++++++
rust/kernel/pci.rs | 7 +++++++
rust/kernel/usb.rs | 6 ++++++
5 files changed, 68 insertions(+)
diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
index e11848bbf206..dea24265f549 100644
--- a/rust/kernel/auxiliary.rs
+++ b/rust/kernel/auxiliary.rs
@@ -15,6 +15,7 @@
};
use core::{
marker::PhantomData,
+ mem::offset_of,
ptr::{addr_of_mut, NonNull},
};
@@ -239,6 +240,12 @@ extern "C" fn release(dev: *mut bindings::device) {
}
}
+// SAFETY: `auxilary::Device` is a transparent wrapper of `struct auxiliary_device`.
+// The offset is guaranteed to point to a valid device field inside `auxilary::Device`.
+unsafe impl<Ctx: device::DeviceContext> device::IntoBusDevice<Ctx> for Device<Ctx> {
+ const OFFSET: usize = offset_of!(bindings::auxiliary_device, dev);
+}
+
// SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic
// argument.
kernel::impl_device_context_deref!(unsafe { Device });
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index 1321e6f0b53c..5527854a195f 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -511,6 +511,47 @@ impl DeviceContext for Core {}
impl DeviceContext for CoreInternal {}
impl DeviceContext for Normal {}
+/// Bus devices can implement this trait to allow abstractions to provide the bus device in
+/// class device callbacks.
+///
+/// # Safety
+///
+/// `IntoBusDevice::OFFSET` must be a offset to a device field in the implemented struct.
+pub(crate) unsafe trait IntoBusDevice<Ctx: DeviceContext>:
+ AsRef<Device<Ctx>>
+{
+ /// The relative offset to the device field.
+ ///
+ /// Use `offset_of!(bindings, field)` macro to avoid breakage.
+ const OFFSET: usize;
+
+ /// Convert a reference to [`Device`] into `Self`.
+ ///
+ /// # Safety
+ ///
+ /// `dev` must be contained in `Self`.
+ unsafe fn from_device(dev: &Device<Ctx>) -> &Self
+ where
+ Self: Sized,
+ {
+ let raw = dev.as_raw();
+ // SAFETY: `raw - Self::OFFSET` is guaranteed by the safety requirements
+ // to be a valid pointer to `Self`.
+ unsafe { &*raw.byte_sub(Self::OFFSET).cast::<Self>() }
+ }
+}
+
+// SAFETY: `Device` is a transparent wrapper of `device`.
+unsafe impl<Ctx: DeviceContext> IntoBusDevice<Ctx> for Device<Ctx> {
+ const OFFSET: usize = 0;
+}
+
+impl<Ctx: DeviceContext> AsRef<Device<Ctx>> for Device<Ctx> {
+ fn as_ref(&self) -> &Device<Ctx> {
+ self
+ }
+}
+
/// # Safety
///
/// The type given as `$device` must be a transparent wrapper of a type that doesn't depend on the
diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
index 7fccffebbd6c..e9a7e09b0116 100644
--- a/rust/kernel/i2c.rs
+++ b/rust/kernel/i2c.rs
@@ -15,6 +15,7 @@
use core::{
marker::PhantomData,
+ mem::offset_of,
ptr::{from_ref, NonNull},
};
@@ -465,6 +466,12 @@ fn as_raw(&self) -> *mut bindings::i2c_client {
}
}
+// SAFETY: `I2cClient` is a transparent wrapper of `struct i2c_client`.
+// The offset is guaranteed to point to a valid device field inside `I2cClient`.
+unsafe impl<Ctx: device::DeviceContext> device::IntoBusDevice<Ctx> for I2cClient<Ctx> {
+ const OFFSET: usize = offset_of!(bindings::i2c_client, dev);
+}
+
// SAFETY: `I2cClient` is a transparent wrapper of a type that doesn't depend on `I2cClient`'s generic
// argument.
kernel::impl_device_context_deref!(unsafe { I2cClient });
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 7fcc5f6022c1..ad00a5c1294a 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -19,6 +19,7 @@
};
use core::{
marker::PhantomData,
+ mem::offset_of,
ops::Deref,
ptr::{addr_of_mut, NonNull},
};
@@ -593,6 +594,12 @@ pub fn set_master(&self) {
}
}
+// SAFETY: `pci::Device` is a transparent wrapper of `struct pci_dev`.
+// The offset is guaranteed to point to a valid device field inside `pci::Device`.
+unsafe impl<Ctx: device::DeviceContext> device::IntoBusDevice<Ctx> for Device<Ctx> {
+ const OFFSET: usize = offset_of!(bindings::pci_dev, dev);
+}
+
// SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic
// argument.
kernel::impl_device_context_deref!(unsafe { Device });
diff --git a/rust/kernel/usb.rs b/rust/kernel/usb.rs
index 14ddb711bab3..8862004e54f9 100644
--- a/rust/kernel/usb.rs
+++ b/rust/kernel/usb.rs
@@ -324,6 +324,12 @@ fn as_raw(&self) -> *mut bindings::usb_interface {
}
}
+// SAFETY: `usb::Interface` is a transparent wrapper of `struct usb_interface`.
+// The offset is guaranteed to point to a valid device field inside `usb::Interface`.
+unsafe impl<Ctx: device::DeviceContext> device::IntoBusDevice<Ctx> for Interface<Ctx> {
+ const OFFSET: usize = offset_of!(bindings::usb_interface, dev);
+}
+
// SAFETY: `Interface` is a transparent wrapper of a type that doesn't depend on
// `Interface`'s generic argument.
kernel::impl_device_context_deref!(unsafe { Interface });
--
2.49.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v5 2/2] rust: leds: add basic led classdev abstractions
2025-10-18 20:59 [PATCH v5 0/2] rust: leds: add led classdev abstractions Markus Probst
2025-10-18 20:59 ` [PATCH v5 1/2] rust: Add trait to convert a device reference to a bus device reference Markus Probst
@ 2025-10-18 20:59 ` Markus Probst
1 sibling, 0 replies; 4+ messages in thread
From: Markus Probst @ 2025-10-18 20:59 UTC (permalink / raw)
To: Greg Kroah-Hartman, Miguel Ojeda, Alex Gaynor, Danilo Krummrich,
Rafael J. Wysocki, Igor Korotin, Lee Jones, Pavel Machek
Cc: Dave Ertman, Ira Weiny, Leon Romanovsky, Boqun Feng, Gary Guo,
bjorn3_gh, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Daniel Almeida, Bjorn Helgaas, kwilczynski,
linux-kernel, rust-for-linux, linux-pci, linux-leds,
Markus Probst
Implement the core abstractions needed for led class devices, including:
* `led::LedOps` - the trait for handling leds, including
`brightness_set`, `brightness_get` and `blink_set`
* `led::InitData` - data set for the led class device
* `led::Device` - a safe wrapper around `led_classdev`
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
rust/kernel/led.rs | 375 +++++++++++++++++++++++++++++++++++++++++++++
rust/kernel/lib.rs | 1 +
2 files changed, 376 insertions(+)
create mode 100644 rust/kernel/led.rs
diff --git a/rust/kernel/led.rs b/rust/kernel/led.rs
new file mode 100644
index 000000000000..980d6e512102
--- /dev/null
+++ b/rust/kernel/led.rs
@@ -0,0 +1,375 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Abstractions for the leds driver model.
+//!
+//! C header: [`include/linux/leds.h`](srctree/include/linux/leds.h)
+
+use core::{marker::PhantomData, pin::Pin, ptr::NonNull};
+
+use pin_init::{pin_data, pinned_drop, PinInit};
+
+use crate::{
+ build_error, container_of,
+ device::{self, property::FwNode, Bound, IntoBusDevice},
+ devres::Devres,
+ error::{from_result, to_result, Error, Result, VTABLE_DEFAULT_ERROR},
+ macros::vtable,
+ str::CStr,
+ try_pin_init,
+ types::{ARef, Opaque},
+};
+
+/// The led class device representation.
+///
+/// This structure represents the Rust abstraction for a C `struct led_classdev`.
+#[pin_data(PinnedDrop)]
+pub struct Device<T: LedOps> {
+ ops: T,
+ #[pin]
+ classdev: Opaque<bindings::led_classdev>,
+}
+
+/// The led init data representation.
+///
+/// This structure represents the Rust abstraction for a C `struct led_init_data`.
+#[derive(Default)]
+pub struct InitData<'a> {
+ fwnode: Option<&'a FwNode>,
+ default_label: Option<&'a CStr>,
+ devicename: Option<&'a CStr>,
+ devname_mandatory: bool,
+}
+
+impl InitData<'static> {
+ /// Creates a new [`InitData`]
+ pub fn new() -> Self {
+ Self::default()
+ }
+}
+
+impl<'a> InitData<'a> {
+ /// Sets the firmware node
+ pub fn fwnode<'b, 'c>(self, fwnode: &'b FwNode) -> InitData<'c>
+ where
+ 'a: 'c,
+ 'b: 'c,
+ {
+ InitData {
+ fwnode: Some(fwnode),
+ ..self
+ }
+ }
+
+ /// Sets a default label
+ pub fn default_label<'b, 'c>(self, label: &'b CStr) -> InitData<'c>
+ where
+ 'a: 'c,
+ 'b: 'c,
+ {
+ InitData {
+ default_label: Some(label),
+ ..self
+ }
+ }
+
+ /// Sets the device name
+ pub fn devicename<'b, 'c>(self, devicename: &'b CStr) -> InitData<'c>
+ where
+ 'a: 'c,
+ 'b: 'c,
+ {
+ InitData {
+ devicename: Some(devicename),
+ ..self
+ }
+ }
+
+ /// Sets if a device name is mandatory
+ pub fn devicename_mandatory(self, mandatory: bool) -> Self {
+ Self {
+ devname_mandatory: mandatory,
+
+ ..self
+ }
+ }
+}
+
+/// Trait defining the operations for a LED driver.
+///
+/// # Examples
+///
+///```
+/// # use kernel::{
+/// # c_str, device, devres::Devres,
+/// # error::Result, i2c::I2cClient, led,
+/// # macros::vtable, prelude::*,
+/// # };
+/// # use core::pin::Pin;
+///
+/// struct MyLedOps;
+///
+///
+/// #[vtable]
+/// impl led::LedOps for MyLedOps {
+/// type Bus = I2cClient<device::Bound>;
+/// const BLOCKING: bool = false;
+/// const MAX_BRIGHTNESS: u32 = 255;
+///
+/// fn brightness_set(
+/// &self,
+/// _dev: &I2cClient<device::Bound>,
+/// _brightness: u32
+/// ) -> Result<()> {
+/// // Set the brightness for the led here
+/// Ok(())
+/// }
+/// }
+///
+/// fn register_my_led(
+/// parent: &I2cClient<device::Bound>,
+/// ) -> Result<Pin<KBox<Devres<led::Device<MyLedOps>>>>> {
+/// KBox::pin_init(led::Device::new(
+/// parent,
+/// led::InitData::new()
+/// .default_label(c_str!("my_led")),
+/// MyLedOps,
+/// ), GFP_KERNEL)
+/// }
+///```
+/// Led drivers must implement this trait in order to register and handle a [`Device`].
+#[vtable]
+pub trait LedOps: Send + 'static + Sized {
+ /// The bus device required by the implementation.
+ #[allow(private_bounds)]
+ type Bus: IntoBusDevice<Bound>;
+ /// If set true, [`LedOps::brightness_set`] and [`LedOps::blink_set`] must not sleep
+ /// and perform the operation immediately.
+ const BLOCKING: bool;
+ /// The max brightness level
+ const MAX_BRIGHTNESS: u32;
+
+ /// Sets the brightness level.
+ ///
+ /// See also [`LedOps::BLOCKING`]
+ fn brightness_set(&self, dev: &Self::Bus, brightness: u32) -> Result<()>;
+
+ /// Gets the current brightness level.
+ fn brightness_get(&self, _dev: &Self::Bus) -> u32 {
+ build_error!(VTABLE_DEFAULT_ERROR)
+ }
+
+ /// Activates hardware accelerated blinking.
+ ///
+ /// delays are in milliseconds. If both are zero, a sensible default should be chosen.
+ /// The caller should adjust the timings in that case and if it can't match the values
+ /// specified exactly. Setting the brightness to 0 will disable the hardware accelerated
+ /// blinking.
+ ///
+ /// See also [`LedOps::BLOCKING`]
+ fn blink_set(
+ &self,
+ _dev: &Self::Bus,
+ _delay_on: &mut usize,
+ _delay_off: &mut usize,
+ ) -> Result<()> {
+ build_error!(VTABLE_DEFAULT_ERROR)
+ }
+}
+
+// SAFETY: A `led::Device` can be unregistered from any thread.
+unsafe impl<T: LedOps + Send> Send for Device<T> {}
+
+// SAFETY: `led::Device` can be shared among threads because all methods of `led::Device`
+// are thread safe.
+unsafe impl<T: LedOps + Sync> Sync for Device<T> {}
+
+impl<T: LedOps> Device<T> {
+ /// Registers a new led classdev.
+ ///
+ /// The [`Device`] will be unregistered on drop.
+ pub fn new<'a>(
+ parent: &'a T::Bus,
+ init_data: InitData<'a>,
+ ops: T,
+ ) -> impl PinInit<Devres<Self>, Error> + 'a {
+ Devres::new(
+ parent.as_ref(),
+ try_pin_init!(Self {
+ ops,
+ classdev <- Opaque::try_ffi_init(|ptr: *mut bindings::led_classdev| {
+ // SAFETY: `try_ffi_init` guarantees that `ptr` is valid for write.
+ // `led_classdev` gets fully initialized in-place by
+ // `led_classdev_register_ext` including `mutex` and `list_head`.
+ unsafe {
+ ptr.write(bindings::led_classdev {
+ max_brightness: T::MAX_BRIGHTNESS,
+ brightness_set: (!T::BLOCKING)
+ .then_some(Adapter::<T>::brightness_set_callback),
+ brightness_set_blocking: T::BLOCKING
+ .then_some(Adapter::<T>::brightness_set_blocking_callback),
+ brightness_get: T::HAS_BRIGHTNESS_GET
+ .then_some(Adapter::<T>::brightness_get_callback),
+ blink_set: T::HAS_BLINK_SET.then_some(Adapter::<T>::blink_set_callback),
+ ..bindings::led_classdev::default()
+ })
+ };
+
+ let fwnode = init_data.fwnode.map(ARef::from);
+
+ let mut init_data = bindings::led_init_data {
+ fwnode: fwnode
+ .as_ref()
+ .map_or(core::ptr::null_mut(), |fwnode| fwnode.as_raw()),
+ default_label: init_data
+ .default_label
+ .map_or(core::ptr::null(), CStr::as_char_ptr),
+ devicename: init_data
+ .devicename
+ .map_or(core::ptr::null(), CStr::as_char_ptr),
+ devname_mandatory: init_data.devname_mandatory,
+ };
+
+ // SAFETY:
+ // - `parent.as_raw()` is guaranteed to be a pointer to a valid `device`
+ // or a null pointer.
+ // - `ptr` is guaranteed to be a pointer to an initialized `led_classdev`.
+ to_result(unsafe {
+ bindings::led_classdev_register_ext(
+ parent.as_ref().as_raw(),
+ ptr,
+ &mut init_data,
+ )
+ })?;
+
+ core::mem::forget(fwnode); // keep the reference count incremented
+
+ Ok::<_, Error>(())
+ }),
+ }),
+ )
+ }
+
+ /// # Safety
+ /// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
+ /// `led::Device`.
+ unsafe fn from_raw<'a>(led_cdev: *mut bindings::led_classdev) -> &'a Self {
+ // SAFETY: The function's contract guarantees that `led_cdev` points to a `led_classdev`
+ // field embedded within a valid `led::Device`. `container_of!` can therefore
+ // safely calculate the address of the containing struct.
+ unsafe { &*container_of!(Opaque::cast_from(led_cdev), Self, classdev) }
+ }
+
+ fn parent(&self) -> &device::Device<Bound> {
+ // SAFETY:
+ // - `self.classdev.get()` is guaranteed to be a valid pointer to `led_classdev`.
+ unsafe { device::Device::from_raw((*(*self.classdev.get()).dev).parent) }
+ }
+}
+
+struct Adapter<T: LedOps> {
+ _p: PhantomData<T>,
+}
+
+impl<T: LedOps> Adapter<T> {
+ /// # Safety
+ /// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
+ /// `led::Device`.
+ /// This function is called on setting the brightness of a led.
+ unsafe extern "C" fn brightness_set_callback(
+ led_cdev: *mut bindings::led_classdev,
+ brightness: u32,
+ ) {
+ // SAFETY: The function's contract guarantees that `led_cdev` is a valid pointer to a
+ // `led_classdev` embedded within a `led::Device`.
+ let classdev = unsafe { Device::<T>::from_raw(led_cdev) };
+ // SAFETY: `classdev.parent()` is guaranteed to be contained in `T::Bus`.
+ let parent = unsafe { T::Bus::from_device(classdev.parent()) };
+
+ let _ = classdev.ops.brightness_set(parent, brightness);
+ }
+
+ /// # Safety
+ /// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
+ /// `led::Device`.
+ /// This function is called on setting the brightness of a led immediately.
+ unsafe extern "C" fn brightness_set_blocking_callback(
+ led_cdev: *mut bindings::led_classdev,
+ brightness: u32,
+ ) -> i32 {
+ from_result(|| {
+ // SAFETY: The function's contract guarantees that `led_cdev` is a valid pointer to a
+ // `led_classdev` embedded within a `led::Device`.
+ let classdev = unsafe { Device::<T>::from_raw(led_cdev) };
+ // SAFETY: `classdev.parent()` is guaranteed to be contained in `T::Bus`.
+ let parent = unsafe { T::Bus::from_device(classdev.parent()) };
+
+ classdev.ops.brightness_set(parent, brightness)?;
+ Ok(0)
+ })
+ }
+
+ /// # Safety
+ /// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
+ /// `led::Device`.
+ /// This function is called on getting the brightness of a led.
+ unsafe extern "C" fn brightness_get_callback(led_cdev: *mut bindings::led_classdev) -> u32 {
+ // SAFETY: The function's contract guarantees that `led_cdev` is a valid pointer to a
+ // `led_classdev` embedded within a `led::Device`.
+ let classdev = unsafe { Device::<T>::from_raw(led_cdev) };
+ // SAFETY: `classdev.parent()` is guaranteed to be contained in `T::Bus`.
+ let parent = unsafe { T::Bus::from_device(classdev.parent()) };
+
+ classdev.ops.brightness_get(parent)
+ }
+
+ /// # Safety
+ /// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
+ /// `led::Device`.
+ /// `delay_on` and `delay_off` must be valid pointers to `usize` and have
+ /// exclusive access for the period of this function.
+ /// This function is called on enabling hardware accelerated blinking.
+ unsafe extern "C" fn blink_set_callback(
+ led_cdev: *mut bindings::led_classdev,
+ delay_on: *mut usize,
+ delay_off: *mut usize,
+ ) -> i32 {
+ from_result(|| {
+ // SAFETY: The function's contract guarantees that `led_cdev` is a valid pointer to a
+ // `led_classdev` embedded within a `led::Device`.
+ let classdev = unsafe { Device::<T>::from_raw(led_cdev) };
+ // SAFETY: `classdev.parent()` is guaranteed to be contained in `T::Bus`.
+ let parent = unsafe { T::Bus::from_device(classdev.parent()) };
+
+ classdev.ops.blink_set(
+ parent,
+ // SAFETY: The function's contract guarantees that `delay_on` points to a `usize`
+ // and is exclusive for the period of this function.
+ unsafe { &mut *delay_on },
+ // SAFETY: The function's contract guarantees that `delay_off` points to a `usize`
+ // and is exclusive for the period of this function.
+ unsafe { &mut *delay_off },
+ )?;
+ Ok(0)
+ })
+ }
+}
+
+#[pinned_drop]
+impl<T: LedOps> PinnedDrop for Device<T> {
+ fn drop(self: Pin<&mut Self>) {
+ let raw = self.classdev.get();
+ // SAFETY: The existence of `self` guarantees that `self.classdev.get()` is a pointer to a
+ // valid `struct led_classdev`.
+ let dev: &device::Device = unsafe { device::Device::from_raw((*raw).dev) };
+
+ let _fwnode = dev
+ .fwnode()
+ // SAFETY: the reference count of `fwnode` has previously been
+ // incremented in `led::Device::new`.
+ .map(|fwnode| unsafe { ARef::from_raw(NonNull::from(fwnode)) });
+
+ // SAFETY: The existence of `self` guarantees that `self.classdev` has previously been
+ // successfully registered with `led_classdev_register_ext`.
+ unsafe { bindings::led_classdev_unregister(self.classdev.get()) };
+ }
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 8c0070a8029e..a8f49ce78f8d 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -105,6 +105,7 @@
pub mod jump_label;
#[cfg(CONFIG_KUNIT)]
pub mod kunit;
+pub mod led;
pub mod list;
pub mod maple_tree;
pub mod miscdevice;
--
2.49.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v5 1/2] rust: Add trait to convert a device reference to a bus device reference
2025-10-18 20:59 ` [PATCH v5 1/2] rust: Add trait to convert a device reference to a bus device reference Markus Probst
@ 2025-10-19 14:28 ` Danilo Krummrich
0 siblings, 0 replies; 4+ messages in thread
From: Danilo Krummrich @ 2025-10-19 14:28 UTC (permalink / raw)
To: Markus Probst
Cc: Greg Kroah-Hartman, Miguel Ojeda, Alex Gaynor, Rafael J. Wysocki,
Igor Korotin, Lee Jones, Pavel Machek, Dave Ertman, Ira Weiny,
Leon Romanovsky, Boqun Feng, Gary Guo, bjorn3_gh, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Daniel Almeida,
Bjorn Helgaas, kwilczynski, linux-kernel, rust-for-linux,
linux-pci, linux-leds
On Sat Oct 18, 2025 at 10:59 PM CEST, Markus Probst wrote:
> Implement the `IntoBusDevice` trait for converting a `Device` reference to a
> bus device reference for all bus devices. `Device` implements this trait as a
> fallback.
>
> The `IntoBusDevice` trait allows abstractions to provide the bus device in
> class device callbacks.
>
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
> rust/kernel/auxiliary.rs | 7 +++++++
> rust/kernel/device.rs | 41 ++++++++++++++++++++++++++++++++++++++++
> rust/kernel/i2c.rs | 7 +++++++
i2c is not upstream yet, hence it should not be part of this patch. Instead you
should include the platform bus though.
> rust/kernel/pci.rs | 7 +++++++
> rust/kernel/usb.rs | 6 ++++++
> 5 files changed, 68 insertions(+)
>
> diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
> index e11848bbf206..dea24265f549 100644
> --- a/rust/kernel/auxiliary.rs
> +++ b/rust/kernel/auxiliary.rs
> @@ -15,6 +15,7 @@
> };
> use core::{
> marker::PhantomData,
> + mem::offset_of,
> ptr::{addr_of_mut, NonNull},
> };
>
> @@ -239,6 +240,12 @@ extern "C" fn release(dev: *mut bindings::device) {
> }
> }
>
> +// SAFETY: `auxilary::Device` is a transparent wrapper of `struct auxiliary_device`.
> +// The offset is guaranteed to point to a valid device field inside `auxilary::Device`.
> +unsafe impl<Ctx: device::DeviceContext> device::IntoBusDevice<Ctx> for Device<Ctx> {
> + const OFFSET: usize = offset_of!(bindings::auxiliary_device, dev);
> +}
> +
> // SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic
> // argument.
> kernel::impl_device_context_deref!(unsafe { Device });
> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index 1321e6f0b53c..5527854a195f 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs
> @@ -511,6 +511,47 @@ impl DeviceContext for Core {}
> impl DeviceContext for CoreInternal {}
> impl DeviceContext for Normal {}
>
> +/// Bus devices can implement this trait to allow abstractions to provide the bus device in
> +/// class device callbacks.
> +///
> +/// # Safety
> +///
> +/// `IntoBusDevice::OFFSET` must be a offset to a device field in the implemented struct.
I think we should also require that this must only be implemented by bus device
types.
> +pub(crate) unsafe trait IntoBusDevice<Ctx: DeviceContext>:
> + AsRef<Device<Ctx>>
We should probably name this AsBusDevice.
> +{
> + /// The relative offset to the device field.
> + ///
> + /// Use `offset_of!(bindings, field)` macro to avoid breakage.
> + const OFFSET: usize;
> +
> + /// Convert a reference to [`Device`] into `Self`.
> + ///
> + /// # Safety
> + ///
> + /// `dev` must be contained in `Self`.
> + unsafe fn from_device(dev: &Device<Ctx>) -> &Self
As mentioned in the other thread, my concern remains that this could be abused
by drivers.
For now the trait is pub(crate), but with the new build system coming soon,
we're able to split things out of the kernel crate, and hence bus abstractions
and driver-core code may end up in different crates requiring this to become
public.
We should at least document that this must not be used by drivers and is
intended for bus and class device abstractions only.
> + where
> + Self: Sized,
> + {
> + let raw = dev.as_raw();
> + // SAFETY: `raw - Self::OFFSET` is guaranteed by the safety requirements
> + // to be a valid pointer to `Self`.
> + unsafe { &*raw.byte_sub(Self::OFFSET).cast::<Self>() }
> + }
> +}
> +
> +// SAFETY: `Device` is a transparent wrapper of `device`.
> +unsafe impl<Ctx: DeviceContext> IntoBusDevice<Ctx> for Device<Ctx> {
> + const OFFSET: usize = 0;
> +}
A generic device is not guaranteed to be a bus device. Also, I don't see a
reason why class device abstractions would want to work with a generic device
rather than the actual bus device parent. Hence, let's drop this impl.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-10-19 14:28 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-18 20:59 [PATCH v5 0/2] rust: leds: add led classdev abstractions Markus Probst
2025-10-18 20:59 ` [PATCH v5 1/2] rust: Add trait to convert a device reference to a bus device reference Markus Probst
2025-10-19 14:28 ` Danilo Krummrich
2025-10-18 20:59 ` [PATCH v5 2/2] rust: leds: add basic led classdev abstractions Markus Probst
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).