From 85a5ffbd56b236e96a7a22a631f9febf36d7ead5 Mon Sep 17 00:00:00 2001 From: Markus Probst Date: Tue, 2 Dec 2025 18:17:52 +0000 Subject: [PATCH] rust: pwm: Add UnregisteredChip wrapper around Chip MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The `pwm::Registration::register` function provides no guarantee that the function isn't called twice with the same pwm chip, which is considered unsafe. Add `pwm::UnregisteredChip` as wrapper around `pwm::Chip`. Implement `pwm::UnregisteredChip::register` for the registration. This function takes ownership of `pwm::UnregisteredChip` and therefore guarantees that the registration can't be called twice on the same pwm chip. Signed-off-by: Markus Probst Tested-by: Michal Wilczynski Acked-by: Michal Wilczynski Link: https://patch.msgid.link/20251202-pwm_safe_register-v2-1-7a2e0d1e287f@posteo.de [ukleinek: fixes a typo that Michal pointed out during review] Signed-off-by: Uwe Kleine-König --- drivers/pwm/pwm_th1520.rs | 2 +- rust/kernel/pwm.rs | 68 +++++++++++++++++++++++++-------------- 2 files changed, 45 insertions(+), 25 deletions(-) diff --git a/drivers/pwm/pwm_th1520.rs b/drivers/pwm/pwm_th1520.rs index e3b7e77356fc2..71b80761ec31f 100644 --- a/drivers/pwm/pwm_th1520.rs +++ b/drivers/pwm/pwm_th1520.rs @@ -372,7 +372,7 @@ impl platform::Driver for Th1520PwmPlatformDriver { }), )?; - pwm::Registration::register(dev, chip)?; + chip.register()?; Ok(Th1520PwmPlatformDriver) } diff --git a/rust/kernel/pwm.rs b/rust/kernel/pwm.rs index 1605d13d5d64a..2dd72a39acb5d 100644 --- a/rust/kernel/pwm.rs +++ b/rust/kernel/pwm.rs @@ -16,7 +16,11 @@ use crate::{ sync::aref::{ARef, AlwaysRefCounted}, types::Opaque, // }; -use core::{marker::PhantomData, ptr::NonNull}; +use core::{ + marker::PhantomData, + ops::Deref, + ptr::NonNull, // +}; /// Represents a PWM waveform configuration. /// Mirrors struct [`struct pwm_waveform`](srctree/include/linux/pwm.h). @@ -174,7 +178,7 @@ pub struct RoundedWaveform { } /// Trait defining the operations for a PWM driver. -pub trait PwmOps: 'static + Sized { +pub trait PwmOps: 'static + Send + Sync + Sized { /// The driver-specific hardware representation of a waveform. /// /// This type must be [`Copy`], [`Default`], and fit within `PWM_WFHWSIZE`. @@ -585,11 +589,12 @@ impl Chip { /// /// Returns an [`ARef`] managing the chip's lifetime via refcounting /// on its embedded `struct device`. - pub fn new( - parent_dev: &device::Device, + #[allow(clippy::new_ret_no_self)] + pub fn new<'a>( + parent_dev: &'a device::Device, num_channels: u32, data: impl pin_init::PinInit, - ) -> Result> { + ) -> Result> { let sizeof_priv = core::mem::size_of::(); // SAFETY: `pwmchip_alloc` allocates memory for the C struct and our private data. let c_chip_ptr_raw = @@ -624,7 +629,9 @@ impl Chip { // SAFETY: `chip_ptr_as_self` points to a valid `Chip` (layout-compatible with // `bindings::pwm_chip`) whose embedded device has refcount 1. // `ARef::from_raw` takes this pointer and manages it via `AlwaysRefCounted`. - Ok(unsafe { ARef::from_raw(NonNull::new_unchecked(chip_ptr_as_self)) }) + let chip = unsafe { ARef::from_raw(NonNull::new_unchecked(chip_ptr_as_self)) }; + + Ok(UnregisteredChip { chip, parent_dev }) } } @@ -655,37 +662,29 @@ unsafe impl AlwaysRefCounted for Chip { // structure's state is managed and synchronized by the kernel's device model // and PWM core locking mechanisms. Therefore, it is safe to move the `Chip` // wrapper (and the pointer it contains) across threads. -unsafe impl Send for Chip {} +unsafe impl Send for Chip {} // SAFETY: It is safe for multiple threads to have shared access (`&Chip`) because // the `Chip` data is immutable from the Rust side without holding the appropriate // kernel locks, which the C core is responsible for. Any interior mutability is // handled and synchronized by the C kernel code. -unsafe impl Sync for Chip {} +unsafe impl Sync for Chip {} -/// A resource guard that ensures `pwmchip_remove` is called on drop. -/// -/// This struct is intended to be managed by the `devres` framework by transferring its ownership -/// via [`devres::register`]. This ties the lifetime of the PWM chip registration -/// to the lifetime of the underlying device. -pub struct Registration { +/// A wrapper around `ARef>` that ensures that `register` can only be called once. +pub struct UnregisteredChip<'a, T: PwmOps> { chip: ARef>, + parent_dev: &'a device::Device, } -impl Registration { +impl UnregisteredChip<'_, T> { /// Registers a PWM chip with the PWM subsystem. /// /// Transfers its ownership to the `devres` framework, which ties its lifetime /// to the parent device. /// On unbind of the parent device, the `devres` entry will be dropped, automatically /// calling `pwmchip_remove`. This function should be called from the driver's `probe`. - pub fn register(dev: &device::Device, chip: ARef>) -> Result { - let chip_parent = chip.device().parent().ok_or(EINVAL)?; - if dev.as_raw() != chip_parent.as_raw() { - return Err(EINVAL); - } - - let c_chip_ptr = chip.as_raw(); + pub fn register(self) -> Result>> { + let c_chip_ptr = self.chip.as_raw(); // SAFETY: `c_chip_ptr` points to a valid chip with its ops initialized. // `__pwmchip_add` is the C function to register the chip with the PWM core. @@ -693,12 +692,33 @@ impl Registration { to_result(bindings::__pwmchip_add(c_chip_ptr, core::ptr::null_mut()))?; } - let registration = Registration { chip }; + let registration = Registration { + chip: ARef::clone(&self.chip), + }; + + devres::register(self.parent_dev, registration, GFP_KERNEL)?; - devres::register(dev, registration, GFP_KERNEL) + Ok(self.chip) } } +impl Deref for UnregisteredChip<'_, T> { + type Target = Chip; + + fn deref(&self) -> &Self::Target { + &self.chip + } +} + +/// A resource guard that ensures `pwmchip_remove` is called on drop. +/// +/// This struct is intended to be managed by the `devres` framework by transferring its ownership +/// via [`devres::register`]. This ties the lifetime of the PWM chip registration +/// to the lifetime of the underlying device. +struct Registration { + chip: ARef>, +} + impl Drop for Registration { fn drop(&mut self) { let chip_raw = self.chip.as_raw(); -- 2.47.3