* [PATCH] Move pwm registration into pwm::Chip::new
@ 2025-11-27 17:15 ` Markus Probst
0 siblings, 0 replies; 14+ messages in thread
From: Markus Probst @ 2025-11-27 17:15 UTC (permalink / raw)
To: Drew Fustini, Guo Ren, Fu Wei, Uwe Kleine-König,
Michal Wilczynski, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Danilo Krummrich
Cc: linux-riscv, linux-pwm, linux-kernel, rust-for-linux,
Markus Probst
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 the code responsible for the registration into `pwm::Chip::new`. The
registration will happen before the driver gets access to the refcounted
pwm chip and can therefore guarantee that the registration isn't called
twice on the same pwm chip.
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
This patch provides the additional guarantee that the pwm chip doesn't
get registered twice.
The following changes were made:
- change the visibility of `pwm::Registration` to private
- remove the `pwm::Registration::register` function
- add code for registering the pwm chip in `pwm::Chip::new`
- add Send + Sync bounds to `PwmOps`
Note that I wasn't able to test this patch, due to the lack of hardware.
---
drivers/pwm/pwm_th1520.rs | 4 +---
rust/kernel/pwm.rs | 53 ++++++++++++++++++-----------------------------
2 files changed, 21 insertions(+), 36 deletions(-)
diff --git a/drivers/pwm/pwm_th1520.rs b/drivers/pwm/pwm_th1520.rs
index 955c359b07fb..1919b5a1f69e 100644
--- a/drivers/pwm/pwm_th1520.rs
+++ b/drivers/pwm/pwm_th1520.rs
@@ -363,7 +363,7 @@ fn probe(
return Err(EINVAL);
}
- let chip = pwm::Chip::new(
+ pwm::Chip::new(
dev,
TH1520_MAX_PWM_NUM,
try_pin_init!(Th1520PwmDriverData {
@@ -372,8 +372,6 @@ fn probe(
}),
)?;
- pwm::Registration::register(dev, chip)?;
-
Ok(KBox::new(Th1520PwmPlatformDriver, GFP_KERNEL)?.into())
}
}
diff --git a/rust/kernel/pwm.rs b/rust/kernel/pwm.rs
index cb00f8a8765c..c5d03ee8bc95 100644
--- a/rust/kernel/pwm.rs
+++ b/rust/kernel/pwm.rs
@@ -173,7 +173,7 @@ pub struct RoundedWaveform<WfHw> {
}
/// 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,7 +585,7 @@ unsafe fn bound_parent_device(&self) -> &device::Device<Bound> {
/// Returns an [`ARef<Chip>`] managing the chip's lifetime via refcounting
/// on its embedded `struct device`.
pub fn new(
- parent_dev: &device::Device,
+ parent_dev: &device::Device<Bound>,
num_channels: u32,
data: impl pin_init::PinInit<T, Error>,
) -> Result<ARef<Self>> {
@@ -623,7 +623,21 @@ pub fn new(
// 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)) };
+
+ // 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.
+ unsafe {
+ to_result(bindings::__pwmchip_add(c_chip_ptr, core::ptr::null_mut()))?;
+ }
+
+ let registration = Registration {
+ chip: ARef::clone(&chip),
+ };
+
+ devres::register(parent_dev, registration, GFP_KERNEL)?;
+
+ Ok(chip)
}
}
@@ -654,50 +668,23 @@ unsafe fn dec_ref(obj: NonNull<Chip<T>>) {
// 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<T: PwmOps + Send> Send for Chip<T> {}
+unsafe impl<T: PwmOps> Send for Chip<T> {}
// 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<T: PwmOps + Sync> Sync for Chip<T> {}
+unsafe impl<T: PwmOps> Sync for Chip<T> {}
/// 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<T: PwmOps> {
+struct Registration<T: PwmOps> {
chip: ARef<Chip<T>>,
}
-impl<T: 'static + PwmOps + Send + Sync> Registration<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<Bound>, chip: ARef<Chip<T>>) -> 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();
-
- // 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.
- unsafe {
- to_result(bindings::__pwmchip_add(c_chip_ptr, core::ptr::null_mut()))?;
- }
-
- let registration = Registration { chip };
-
- devres::register(dev, registration, GFP_KERNEL)
- }
-}
-
impl<T: PwmOps> Drop for Registration<T> {
fn drop(&mut self) {
let chip_raw = self.chip.as_raw();
---
base-commit: fae00ea9f00367771003ace78f29549dead58fc7
change-id: 20251127-pwm_safe_register-e49b0a261101
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH] Move pwm registration into pwm::Chip::new
2025-11-27 17:15 ` Markus Probst
@ 2025-11-27 17:16 ` Markus Probst
-1 siblings, 0 replies; 14+ messages in thread
From: Markus Probst @ 2025-11-27 17:16 UTC (permalink / raw)
To: Drew Fustini, Guo Ren, Fu Wei, Uwe Kleine-König,
Michal Wilczynski, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Danilo Krummrich
Cc: linux-riscv, linux-pwm, linux-kernel, rust-for-linux
[-- Attachment #1: Type: text/plain, Size: 6638 bytes --]
On Thu, 2025-11-27 at 17:15 +0000, Markus Probst wrote:
> 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 the code responsible for the registration into `pwm::Chip::new`. The
> registration will happen before the driver gets access to the refcounted
> pwm chip and can therefore guarantee that the registration isn't called
> twice on the same pwm chip.
>
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
> This patch provides the additional guarantee that the pwm chip doesn't
> get registered twice.
>
> The following changes were made:
> - change the visibility of `pwm::Registration` to private
> - remove the `pwm::Registration::register` function
> - add code for registering the pwm chip in `pwm::Chip::new`
> - add Send + Sync bounds to `PwmOps`
>
> Note that I wasn't able to test this patch, due to the lack of hardware.
> ---
> drivers/pwm/pwm_th1520.rs | 4 +---
> rust/kernel/pwm.rs | 53 ++++++++++++++++++-----------------------------
> 2 files changed, 21 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/pwm/pwm_th1520.rs b/drivers/pwm/pwm_th1520.rs
> index 955c359b07fb..1919b5a1f69e 100644
> --- a/drivers/pwm/pwm_th1520.rs
> +++ b/drivers/pwm/pwm_th1520.rs
> @@ -363,7 +363,7 @@ fn probe(
> return Err(EINVAL);
> }
>
> - let chip = pwm::Chip::new(
> + pwm::Chip::new(
> dev,
> TH1520_MAX_PWM_NUM,
> try_pin_init!(Th1520PwmDriverData {
> @@ -372,8 +372,6 @@ fn probe(
> }),
> )?;
>
> - pwm::Registration::register(dev, chip)?;
> -
> Ok(KBox::new(Th1520PwmPlatformDriver, GFP_KERNEL)?.into())
> }
> }
> diff --git a/rust/kernel/pwm.rs b/rust/kernel/pwm.rs
> index cb00f8a8765c..c5d03ee8bc95 100644
> --- a/rust/kernel/pwm.rs
> +++ b/rust/kernel/pwm.rs
> @@ -173,7 +173,7 @@ pub struct RoundedWaveform<WfHw> {
> }
>
> /// 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,7 +585,7 @@ unsafe fn bound_parent_device(&self) -> &device::Device<Bound> {
> /// Returns an [`ARef<Chip>`] managing the chip's lifetime via refcounting
> /// on its embedded `struct device`.
> pub fn new(
> - parent_dev: &device::Device,
> + parent_dev: &device::Device<Bound>,
> num_channels: u32,
> data: impl pin_init::PinInit<T, Error>,
> ) -> Result<ARef<Self>> {
> @@ -623,7 +623,21 @@ pub fn new(
> // 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)) };
> +
> + // 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.
> + unsafe {
> + to_result(bindings::__pwmchip_add(c_chip_ptr, core::ptr::null_mut()))?;
> + }
> +
> + let registration = Registration {
> + chip: ARef::clone(&chip),
> + };
> +
> + devres::register(parent_dev, registration, GFP_KERNEL)?;
> +
> + Ok(chip)
> }
> }
>
> @@ -654,50 +668,23 @@ unsafe fn dec_ref(obj: NonNull<Chip<T>>) {
> // 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<T: PwmOps + Send> Send for Chip<T> {}
> +unsafe impl<T: PwmOps> Send for Chip<T> {}
>
> // 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<T: PwmOps + Sync> Sync for Chip<T> {}
> +unsafe impl<T: PwmOps> Sync for Chip<T> {}
>
> /// 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<T: PwmOps> {
> +struct Registration<T: PwmOps> {
> chip: ARef<Chip<T>>,
> }
>
> -impl<T: 'static + PwmOps + Send + Sync> Registration<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<Bound>, chip: ARef<Chip<T>>) -> 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();
> -
> - // 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.
> - unsafe {
> - to_result(bindings::__pwmchip_add(c_chip_ptr, core::ptr::null_mut()))?;
> - }
> -
> - let registration = Registration { chip };
> -
> - devres::register(dev, registration, GFP_KERNEL)
> - }
> -}
> -
> impl<T: PwmOps> Drop for Registration<T> {
> fn drop(&mut self) {
> let chip_raw = self.chip.as_raw();
>
> ---
> base-commit: fae00ea9f00367771003ace78f29549dead58fc7
> change-id: 20251127-pwm_safe_register-e49b0a261101
Just noticed 5 sec too late, that I forgot the "rust: pwm: " commit
message prefix.
Thanks
- Markus Probst
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] Move pwm registration into pwm::Chip::new
@ 2025-11-27 17:16 ` Markus Probst
0 siblings, 0 replies; 14+ messages in thread
From: Markus Probst @ 2025-11-27 17:16 UTC (permalink / raw)
To: Drew Fustini, Guo Ren, Fu Wei, Uwe Kleine-König,
Michal Wilczynski, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Danilo Krummrich
Cc: linux-riscv, linux-pwm, linux-kernel, rust-for-linux
[-- Attachment #1.1: Type: text/plain, Size: 6638 bytes --]
On Thu, 2025-11-27 at 17:15 +0000, Markus Probst wrote:
> 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 the code responsible for the registration into `pwm::Chip::new`. The
> registration will happen before the driver gets access to the refcounted
> pwm chip and can therefore guarantee that the registration isn't called
> twice on the same pwm chip.
>
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
> This patch provides the additional guarantee that the pwm chip doesn't
> get registered twice.
>
> The following changes were made:
> - change the visibility of `pwm::Registration` to private
> - remove the `pwm::Registration::register` function
> - add code for registering the pwm chip in `pwm::Chip::new`
> - add Send + Sync bounds to `PwmOps`
>
> Note that I wasn't able to test this patch, due to the lack of hardware.
> ---
> drivers/pwm/pwm_th1520.rs | 4 +---
> rust/kernel/pwm.rs | 53 ++++++++++++++++++-----------------------------
> 2 files changed, 21 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/pwm/pwm_th1520.rs b/drivers/pwm/pwm_th1520.rs
> index 955c359b07fb..1919b5a1f69e 100644
> --- a/drivers/pwm/pwm_th1520.rs
> +++ b/drivers/pwm/pwm_th1520.rs
> @@ -363,7 +363,7 @@ fn probe(
> return Err(EINVAL);
> }
>
> - let chip = pwm::Chip::new(
> + pwm::Chip::new(
> dev,
> TH1520_MAX_PWM_NUM,
> try_pin_init!(Th1520PwmDriverData {
> @@ -372,8 +372,6 @@ fn probe(
> }),
> )?;
>
> - pwm::Registration::register(dev, chip)?;
> -
> Ok(KBox::new(Th1520PwmPlatformDriver, GFP_KERNEL)?.into())
> }
> }
> diff --git a/rust/kernel/pwm.rs b/rust/kernel/pwm.rs
> index cb00f8a8765c..c5d03ee8bc95 100644
> --- a/rust/kernel/pwm.rs
> +++ b/rust/kernel/pwm.rs
> @@ -173,7 +173,7 @@ pub struct RoundedWaveform<WfHw> {
> }
>
> /// 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,7 +585,7 @@ unsafe fn bound_parent_device(&self) -> &device::Device<Bound> {
> /// Returns an [`ARef<Chip>`] managing the chip's lifetime via refcounting
> /// on its embedded `struct device`.
> pub fn new(
> - parent_dev: &device::Device,
> + parent_dev: &device::Device<Bound>,
> num_channels: u32,
> data: impl pin_init::PinInit<T, Error>,
> ) -> Result<ARef<Self>> {
> @@ -623,7 +623,21 @@ pub fn new(
> // 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)) };
> +
> + // 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.
> + unsafe {
> + to_result(bindings::__pwmchip_add(c_chip_ptr, core::ptr::null_mut()))?;
> + }
> +
> + let registration = Registration {
> + chip: ARef::clone(&chip),
> + };
> +
> + devres::register(parent_dev, registration, GFP_KERNEL)?;
> +
> + Ok(chip)
> }
> }
>
> @@ -654,50 +668,23 @@ unsafe fn dec_ref(obj: NonNull<Chip<T>>) {
> // 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<T: PwmOps + Send> Send for Chip<T> {}
> +unsafe impl<T: PwmOps> Send for Chip<T> {}
>
> // 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<T: PwmOps + Sync> Sync for Chip<T> {}
> +unsafe impl<T: PwmOps> Sync for Chip<T> {}
>
> /// 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<T: PwmOps> {
> +struct Registration<T: PwmOps> {
> chip: ARef<Chip<T>>,
> }
>
> -impl<T: 'static + PwmOps + Send + Sync> Registration<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<Bound>, chip: ARef<Chip<T>>) -> 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();
> -
> - // 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.
> - unsafe {
> - to_result(bindings::__pwmchip_add(c_chip_ptr, core::ptr::null_mut()))?;
> - }
> -
> - let registration = Registration { chip };
> -
> - devres::register(dev, registration, GFP_KERNEL)
> - }
> -}
> -
> impl<T: PwmOps> Drop for Registration<T> {
> fn drop(&mut self) {
> let chip_raw = self.chip.as_raw();
>
> ---
> base-commit: fae00ea9f00367771003ace78f29549dead58fc7
> change-id: 20251127-pwm_safe_register-e49b0a261101
Just noticed 5 sec too late, that I forgot the "rust: pwm: " commit
message prefix.
Thanks
- Markus Probst
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Move pwm registration into pwm::Chip::new
2025-11-27 17:15 ` Markus Probst
@ 2025-11-28 9:28 ` Alice Ryhl
-1 siblings, 0 replies; 14+ messages in thread
From: Alice Ryhl @ 2025-11-28 9:28 UTC (permalink / raw)
To: Markus Probst
Cc: Drew Fustini, Guo Ren, Fu Wei, Uwe Kleine-König,
Michal Wilczynski, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, linux-riscv, linux-pwm,
linux-kernel, rust-for-linux
On Thu, Nov 27, 2025 at 05:15:06PM +0000, Markus Probst wrote:
> 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 the code responsible for the registration into `pwm::Chip::new`. The
> registration will happen before the driver gets access to the refcounted
> pwm chip and can therefore guarantee that the registration isn't called
> twice on the same pwm chip.
>
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
> This patch provides the additional guarantee that the pwm chip doesn't
> get registered twice.
>
> The following changes were made:
> - change the visibility of `pwm::Registration` to private
> - remove the `pwm::Registration::register` function
> - add code for registering the pwm chip in `pwm::Chip::new`
> - add Send + Sync bounds to `PwmOps`
>
> Note that I wasn't able to test this patch, due to the lack of hardware.
Overall looks reasonable, but I have one question:
> @@ -654,50 +668,23 @@ unsafe fn dec_ref(obj: NonNull<Chip<T>>) {
> // 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<T: PwmOps + Send> Send for Chip<T> {}
> +unsafe impl<T: PwmOps> Send for Chip<T> {}
>
> // 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<T: PwmOps + Sync> Sync for Chip<T> {}
> +unsafe impl<T: PwmOps> Sync for Chip<T> {}
Why was this changed?
Alice
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] Move pwm registration into pwm::Chip::new
@ 2025-11-28 9:28 ` Alice Ryhl
0 siblings, 0 replies; 14+ messages in thread
From: Alice Ryhl @ 2025-11-28 9:28 UTC (permalink / raw)
To: Markus Probst
Cc: Drew Fustini, Guo Ren, Fu Wei, Uwe Kleine-König,
Michal Wilczynski, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, linux-riscv, linux-pwm,
linux-kernel, rust-for-linux
On Thu, Nov 27, 2025 at 05:15:06PM +0000, Markus Probst wrote:
> 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 the code responsible for the registration into `pwm::Chip::new`. The
> registration will happen before the driver gets access to the refcounted
> pwm chip and can therefore guarantee that the registration isn't called
> twice on the same pwm chip.
>
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
> This patch provides the additional guarantee that the pwm chip doesn't
> get registered twice.
>
> The following changes were made:
> - change the visibility of `pwm::Registration` to private
> - remove the `pwm::Registration::register` function
> - add code for registering the pwm chip in `pwm::Chip::new`
> - add Send + Sync bounds to `PwmOps`
>
> Note that I wasn't able to test this patch, due to the lack of hardware.
Overall looks reasonable, but I have one question:
> @@ -654,50 +668,23 @@ unsafe fn dec_ref(obj: NonNull<Chip<T>>) {
> // 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<T: PwmOps + Send> Send for Chip<T> {}
> +unsafe impl<T: PwmOps> Send for Chip<T> {}
>
> // 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<T: PwmOps + Sync> Sync for Chip<T> {}
> +unsafe impl<T: PwmOps> Sync for Chip<T> {}
Why was this changed?
Alice
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] Move pwm registration into pwm::Chip::new
2025-11-28 9:28 ` Alice Ryhl
@ 2025-11-28 12:25 ` Markus Probst
-1 siblings, 0 replies; 14+ messages in thread
From: Markus Probst @ 2025-11-28 12:25 UTC (permalink / raw)
To: Alice Ryhl
Cc: Drew Fustini, Guo Ren, Fu Wei, Uwe Kleine-König,
Michal Wilczynski, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, linux-riscv, linux-pwm,
linux-kernel, rust-for-linux
[-- Attachment #1: Type: text/plain, Size: 2582 bytes --]
On Fri, 2025-11-28 at 09:28 +0000, Alice Ryhl wrote:
> On Thu, Nov 27, 2025 at 05:15:06PM +0000, Markus Probst wrote:
> > 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 the code responsible for the registration into `pwm::Chip::new`. The
> > registration will happen before the driver gets access to the refcounted
> > pwm chip and can therefore guarantee that the registration isn't called
> > twice on the same pwm chip.
> >
> > Signed-off-by: Markus Probst <markus.probst@posteo.de>
> > ---
> > This patch provides the additional guarantee that the pwm chip doesn't
> > get registered twice.
> >
> > The following changes were made:
> > - change the visibility of `pwm::Registration` to private
> > - remove the `pwm::Registration::register` function
> > - add code for registering the pwm chip in `pwm::Chip::new`
> > - add Send + Sync bounds to `PwmOps`
> >
> > Note that I wasn't able to test this patch, due to the lack of hardware.
>
> Overall looks reasonable, but I have one question:
>
> > @@ -654,50 +668,23 @@ unsafe fn dec_ref(obj: NonNull<Chip<T>>) {
> > // 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<T: PwmOps + Send> Send for Chip<T> {}
> > +unsafe impl<T: PwmOps> Send for Chip<T> {}
> >
> > // 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<T: PwmOps + Sync> Sync for Chip<T> {}
> > +unsafe impl<T: PwmOps> Sync for Chip<T> {}
>
> Why was this changed?
Registration::register required PwmOps to be Send + Sync.
Prior to this change, Chip::new didn't require it for PwmOps. Meaning
it was possible to allocate a new Chip with a PwmOps that is not Send +
Sync. As there was no use for it and it isn't possible anymore to
allocate a new Chip without registering it, I added Send + Sync as
trait dependency (see 1. hunk of rust/kernel/pwm.rs in the patch).
Because PwmOps now implied Send + Sync, it wasn't necessary anymore to
have the additional bounds there.
Thanks
- Markus Probst
>
> Alice
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] Move pwm registration into pwm::Chip::new
@ 2025-11-28 12:25 ` Markus Probst
0 siblings, 0 replies; 14+ messages in thread
From: Markus Probst @ 2025-11-28 12:25 UTC (permalink / raw)
To: Alice Ryhl
Cc: Drew Fustini, Guo Ren, Fu Wei, Uwe Kleine-König,
Michal Wilczynski, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, linux-riscv, linux-pwm,
linux-kernel, rust-for-linux
[-- Attachment #1.1: Type: text/plain, Size: 2582 bytes --]
On Fri, 2025-11-28 at 09:28 +0000, Alice Ryhl wrote:
> On Thu, Nov 27, 2025 at 05:15:06PM +0000, Markus Probst wrote:
> > 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 the code responsible for the registration into `pwm::Chip::new`. The
> > registration will happen before the driver gets access to the refcounted
> > pwm chip and can therefore guarantee that the registration isn't called
> > twice on the same pwm chip.
> >
> > Signed-off-by: Markus Probst <markus.probst@posteo.de>
> > ---
> > This patch provides the additional guarantee that the pwm chip doesn't
> > get registered twice.
> >
> > The following changes were made:
> > - change the visibility of `pwm::Registration` to private
> > - remove the `pwm::Registration::register` function
> > - add code for registering the pwm chip in `pwm::Chip::new`
> > - add Send + Sync bounds to `PwmOps`
> >
> > Note that I wasn't able to test this patch, due to the lack of hardware.
>
> Overall looks reasonable, but I have one question:
>
> > @@ -654,50 +668,23 @@ unsafe fn dec_ref(obj: NonNull<Chip<T>>) {
> > // 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<T: PwmOps + Send> Send for Chip<T> {}
> > +unsafe impl<T: PwmOps> Send for Chip<T> {}
> >
> > // 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<T: PwmOps + Sync> Sync for Chip<T> {}
> > +unsafe impl<T: PwmOps> Sync for Chip<T> {}
>
> Why was this changed?
Registration::register required PwmOps to be Send + Sync.
Prior to this change, Chip::new didn't require it for PwmOps. Meaning
it was possible to allocate a new Chip with a PwmOps that is not Send +
Sync. As there was no use for it and it isn't possible anymore to
allocate a new Chip without registering it, I added Send + Sync as
trait dependency (see 1. hunk of rust/kernel/pwm.rs in the patch).
Because PwmOps now implied Send + Sync, it wasn't necessary anymore to
have the additional bounds there.
Thanks
- Markus Probst
>
> Alice
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] Move pwm registration into pwm::Chip::new
2025-11-28 12:25 ` Markus Probst
@ 2025-12-01 10:06 ` Alice Ryhl
-1 siblings, 0 replies; 14+ messages in thread
From: Alice Ryhl @ 2025-12-01 10:06 UTC (permalink / raw)
To: Markus Probst
Cc: Drew Fustini, Guo Ren, Fu Wei, Uwe Kleine-König,
Michal Wilczynski, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, linux-riscv, linux-pwm,
linux-kernel, rust-for-linux
On Fri, Nov 28, 2025 at 12:25:04PM +0000, Markus Probst wrote:
> On Fri, 2025-11-28 at 09:28 +0000, Alice Ryhl wrote:
> > On Thu, Nov 27, 2025 at 05:15:06PM +0000, Markus Probst wrote:
> > > 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 the code responsible for the registration into `pwm::Chip::new`. The
> > > registration will happen before the driver gets access to the refcounted
> > > pwm chip and can therefore guarantee that the registration isn't called
> > > twice on the same pwm chip.
> > >
> > > Signed-off-by: Markus Probst <markus.probst@posteo.de>
> > > ---
> > > This patch provides the additional guarantee that the pwm chip doesn't
> > > get registered twice.
> > >
> > > The following changes were made:
> > > - change the visibility of `pwm::Registration` to private
> > > - remove the `pwm::Registration::register` function
> > > - add code for registering the pwm chip in `pwm::Chip::new`
> > > - add Send + Sync bounds to `PwmOps`
> > >
> > > Note that I wasn't able to test this patch, due to the lack of hardware.
> >
> > Overall looks reasonable, but I have one question:
> >
> > > @@ -654,50 +668,23 @@ unsafe fn dec_ref(obj: NonNull<Chip<T>>) {
> > > // 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<T: PwmOps + Send> Send for Chip<T> {}
> > > +unsafe impl<T: PwmOps> Send for Chip<T> {}
> > >
> > > // 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<T: PwmOps + Sync> Sync for Chip<T> {}
> > > +unsafe impl<T: PwmOps> Sync for Chip<T> {}
> >
> > Why was this changed?
>
> Registration::register required PwmOps to be Send + Sync.
> Prior to this change, Chip::new didn't require it for PwmOps. Meaning
> it was possible to allocate a new Chip with a PwmOps that is not Send +
> Sync. As there was no use for it and it isn't possible anymore to
> allocate a new Chip without registering it, I added Send + Sync as
> trait dependency (see 1. hunk of rust/kernel/pwm.rs in the patch).
>
> Because PwmOps now implied Send + Sync, it wasn't necessary anymore to
> have the additional bounds there.
Ah ok, I missed that. That's fine then.
Alice
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] Move pwm registration into pwm::Chip::new
@ 2025-12-01 10:06 ` Alice Ryhl
0 siblings, 0 replies; 14+ messages in thread
From: Alice Ryhl @ 2025-12-01 10:06 UTC (permalink / raw)
To: Markus Probst
Cc: Drew Fustini, Guo Ren, Fu Wei, Uwe Kleine-König,
Michal Wilczynski, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, linux-riscv, linux-pwm,
linux-kernel, rust-for-linux
On Fri, Nov 28, 2025 at 12:25:04PM +0000, Markus Probst wrote:
> On Fri, 2025-11-28 at 09:28 +0000, Alice Ryhl wrote:
> > On Thu, Nov 27, 2025 at 05:15:06PM +0000, Markus Probst wrote:
> > > 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 the code responsible for the registration into `pwm::Chip::new`. The
> > > registration will happen before the driver gets access to the refcounted
> > > pwm chip and can therefore guarantee that the registration isn't called
> > > twice on the same pwm chip.
> > >
> > > Signed-off-by: Markus Probst <markus.probst@posteo.de>
> > > ---
> > > This patch provides the additional guarantee that the pwm chip doesn't
> > > get registered twice.
> > >
> > > The following changes were made:
> > > - change the visibility of `pwm::Registration` to private
> > > - remove the `pwm::Registration::register` function
> > > - add code for registering the pwm chip in `pwm::Chip::new`
> > > - add Send + Sync bounds to `PwmOps`
> > >
> > > Note that I wasn't able to test this patch, due to the lack of hardware.
> >
> > Overall looks reasonable, but I have one question:
> >
> > > @@ -654,50 +668,23 @@ unsafe fn dec_ref(obj: NonNull<Chip<T>>) {
> > > // 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<T: PwmOps + Send> Send for Chip<T> {}
> > > +unsafe impl<T: PwmOps> Send for Chip<T> {}
> > >
> > > // 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<T: PwmOps + Sync> Sync for Chip<T> {}
> > > +unsafe impl<T: PwmOps> Sync for Chip<T> {}
> >
> > Why was this changed?
>
> Registration::register required PwmOps to be Send + Sync.
> Prior to this change, Chip::new didn't require it for PwmOps. Meaning
> it was possible to allocate a new Chip with a PwmOps that is not Send +
> Sync. As there was no use for it and it isn't possible anymore to
> allocate a new Chip without registering it, I added Send + Sync as
> trait dependency (see 1. hunk of rust/kernel/pwm.rs in the patch).
>
> Because PwmOps now implied Send + Sync, it wasn't necessary anymore to
> have the additional bounds there.
Ah ok, I missed that. That's fine then.
Alice
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Move pwm registration into pwm::Chip::new
2025-11-27 17:15 ` Markus Probst
@ 2025-11-28 13:53 ` Daniel Almeida
-1 siblings, 0 replies; 14+ messages in thread
From: Daniel Almeida @ 2025-11-28 13:53 UTC (permalink / raw)
To: Markus Probst
Cc: Drew Fustini, Guo Ren, Fu Wei, Uwe Kleine-König,
Michal Wilczynski, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Danilo Krummrich, linux-riscv,
linux-pwm, linux-kernel, rust-for-linux
> On 27 Nov 2025, at 14:15, Markus Probst <markus.probst@posteo.de> wrote:
>
> 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 the code responsible for the registration into `pwm::Chip::new`. The
> registration will happen before the driver gets access to the refcounted
> pwm chip and can therefore guarantee that the registration isn't called
> twice on the same pwm chip.
>
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
> This patch provides the additional guarantee that the pwm chip doesn't
> get registered twice.
>
> The following changes were made:
> - change the visibility of `pwm::Registration` to private
> - remove the `pwm::Registration::register` function
> - add code for registering the pwm chip in `pwm::Chip::new`
> - add Send + Sync bounds to `PwmOps`
>
> Note that I wasn't able to test this patch, due to the lack of hardware.
> ---
> drivers/pwm/pwm_th1520.rs | 4 +---
> rust/kernel/pwm.rs | 53 ++++++++++++++++++-----------------------------
> 2 files changed, 21 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/pwm/pwm_th1520.rs b/drivers/pwm/pwm_th1520.rs
> index 955c359b07fb..1919b5a1f69e 100644
> --- a/drivers/pwm/pwm_th1520.rs
> +++ b/drivers/pwm/pwm_th1520.rs
> @@ -363,7 +363,7 @@ fn probe(
> return Err(EINVAL);
> }
>
> - let chip = pwm::Chip::new(
> + pwm::Chip::new(
> dev,
> TH1520_MAX_PWM_NUM,
> try_pin_init!(Th1520PwmDriverData {
> @@ -372,8 +372,6 @@ fn probe(
> }),
> )?;
>
> - pwm::Registration::register(dev, chip)?;
> -
> Ok(KBox::new(Th1520PwmPlatformDriver, GFP_KERNEL)?.into())
> }
> }
> diff --git a/rust/kernel/pwm.rs b/rust/kernel/pwm.rs
> index cb00f8a8765c..c5d03ee8bc95 100644
> --- a/rust/kernel/pwm.rs
> +++ b/rust/kernel/pwm.rs
> @@ -173,7 +173,7 @@ pub struct RoundedWaveform<WfHw> {
> }
>
> /// 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,7 +585,7 @@ unsafe fn bound_parent_device(&self) -> &device::Device<Bound> {
> /// Returns an [`ARef<Chip>`] managing the chip's lifetime via refcounting
> /// on its embedded `struct device`.
> pub fn new(
> - parent_dev: &device::Device,
> + parent_dev: &device::Device<Bound>,
> num_channels: u32,
> data: impl pin_init::PinInit<T, Error>,
> ) -> Result<ARef<Self>> {
> @@ -623,7 +623,21 @@ pub fn new(
> // 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)) };
> +
> + // 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.
> + unsafe {
> + to_result(bindings::__pwmchip_add(c_chip_ptr, core::ptr::null_mut()))?;
> + }
> +
> + let registration = Registration {
> + chip: ARef::clone(&chip),
> + };
> +
> + devres::register(parent_dev, registration, GFP_KERNEL)?;
> +
> + Ok(chip)
> }
> }
>
> @@ -654,50 +668,23 @@ unsafe fn dec_ref(obj: NonNull<Chip<T>>) {
> // 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<T: PwmOps + Send> Send for Chip<T> {}
> +unsafe impl<T: PwmOps> Send for Chip<T> {}
>
> // 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<T: PwmOps + Sync> Sync for Chip<T> {}
> +unsafe impl<T: PwmOps> Sync for Chip<T> {}
>
> /// 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<T: PwmOps> {
> +struct Registration<T: PwmOps> {
> chip: ARef<Chip<T>>,
> }
>
> -impl<T: 'static + PwmOps + Send + Sync> Registration<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<Bound>, chip: ARef<Chip<T>>) -> 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();
> -
> - // 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.
> - unsafe {
> - to_result(bindings::__pwmchip_add(c_chip_ptr, core::ptr::null_mut()))?;
> - }
> -
> - let registration = Registration { chip };
> -
> - devres::register(dev, registration, GFP_KERNEL)
> - }
> -}
> -
> impl<T: PwmOps> Drop for Registration<T> {
> fn drop(&mut self) {
> let chip_raw = self.chip.as_raw();
>
> ---
> base-commit: fae00ea9f00367771003ace78f29549dead58fc7
> change-id: 20251127-pwm_safe_register-e49b0a261101
>
>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] Move pwm registration into pwm::Chip::new
@ 2025-11-28 13:53 ` Daniel Almeida
0 siblings, 0 replies; 14+ messages in thread
From: Daniel Almeida @ 2025-11-28 13:53 UTC (permalink / raw)
To: Markus Probst
Cc: Drew Fustini, Guo Ren, Fu Wei, Uwe Kleine-König,
Michal Wilczynski, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Danilo Krummrich, linux-riscv,
linux-pwm, linux-kernel, rust-for-linux
> On 27 Nov 2025, at 14:15, Markus Probst <markus.probst@posteo.de> wrote:
>
> 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 the code responsible for the registration into `pwm::Chip::new`. The
> registration will happen before the driver gets access to the refcounted
> pwm chip and can therefore guarantee that the registration isn't called
> twice on the same pwm chip.
>
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
> This patch provides the additional guarantee that the pwm chip doesn't
> get registered twice.
>
> The following changes were made:
> - change the visibility of `pwm::Registration` to private
> - remove the `pwm::Registration::register` function
> - add code for registering the pwm chip in `pwm::Chip::new`
> - add Send + Sync bounds to `PwmOps`
>
> Note that I wasn't able to test this patch, due to the lack of hardware.
> ---
> drivers/pwm/pwm_th1520.rs | 4 +---
> rust/kernel/pwm.rs | 53 ++++++++++++++++++-----------------------------
> 2 files changed, 21 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/pwm/pwm_th1520.rs b/drivers/pwm/pwm_th1520.rs
> index 955c359b07fb..1919b5a1f69e 100644
> --- a/drivers/pwm/pwm_th1520.rs
> +++ b/drivers/pwm/pwm_th1520.rs
> @@ -363,7 +363,7 @@ fn probe(
> return Err(EINVAL);
> }
>
> - let chip = pwm::Chip::new(
> + pwm::Chip::new(
> dev,
> TH1520_MAX_PWM_NUM,
> try_pin_init!(Th1520PwmDriverData {
> @@ -372,8 +372,6 @@ fn probe(
> }),
> )?;
>
> - pwm::Registration::register(dev, chip)?;
> -
> Ok(KBox::new(Th1520PwmPlatformDriver, GFP_KERNEL)?.into())
> }
> }
> diff --git a/rust/kernel/pwm.rs b/rust/kernel/pwm.rs
> index cb00f8a8765c..c5d03ee8bc95 100644
> --- a/rust/kernel/pwm.rs
> +++ b/rust/kernel/pwm.rs
> @@ -173,7 +173,7 @@ pub struct RoundedWaveform<WfHw> {
> }
>
> /// 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,7 +585,7 @@ unsafe fn bound_parent_device(&self) -> &device::Device<Bound> {
> /// Returns an [`ARef<Chip>`] managing the chip's lifetime via refcounting
> /// on its embedded `struct device`.
> pub fn new(
> - parent_dev: &device::Device,
> + parent_dev: &device::Device<Bound>,
> num_channels: u32,
> data: impl pin_init::PinInit<T, Error>,
> ) -> Result<ARef<Self>> {
> @@ -623,7 +623,21 @@ pub fn new(
> // 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)) };
> +
> + // 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.
> + unsafe {
> + to_result(bindings::__pwmchip_add(c_chip_ptr, core::ptr::null_mut()))?;
> + }
> +
> + let registration = Registration {
> + chip: ARef::clone(&chip),
> + };
> +
> + devres::register(parent_dev, registration, GFP_KERNEL)?;
> +
> + Ok(chip)
> }
> }
>
> @@ -654,50 +668,23 @@ unsafe fn dec_ref(obj: NonNull<Chip<T>>) {
> // 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<T: PwmOps + Send> Send for Chip<T> {}
> +unsafe impl<T: PwmOps> Send for Chip<T> {}
>
> // 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<T: PwmOps + Sync> Sync for Chip<T> {}
> +unsafe impl<T: PwmOps> Sync for Chip<T> {}
>
> /// 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<T: PwmOps> {
> +struct Registration<T: PwmOps> {
> chip: ARef<Chip<T>>,
> }
>
> -impl<T: 'static + PwmOps + Send + Sync> Registration<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<Bound>, chip: ARef<Chip<T>>) -> 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();
> -
> - // 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.
> - unsafe {
> - to_result(bindings::__pwmchip_add(c_chip_ptr, core::ptr::null_mut()))?;
> - }
> -
> - let registration = Registration { chip };
> -
> - devres::register(dev, registration, GFP_KERNEL)
> - }
> -}
> -
> impl<T: PwmOps> Drop for Registration<T> {
> fn drop(&mut self) {
> let chip_raw = self.chip.as_raw();
>
> ---
> base-commit: fae00ea9f00367771003ace78f29549dead58fc7
> change-id: 20251127-pwm_safe_register-e49b0a261101
>
>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Move pwm registration into pwm::Chip::new
2025-11-27 17:15 ` Markus Probst
@ 2025-11-30 22:13 ` Michal Wilczynski
-1 siblings, 0 replies; 14+ messages in thread
From: Michal Wilczynski @ 2025-11-30 22:13 UTC (permalink / raw)
To: Markus Probst
Cc: linux-riscv, linux-pwm, linux-kernel, rust-for-linux,
Drew Fustini, Guo Ren, Fu Wei, Uwe Kleine-König,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich
Hi Markus,
On 11/27/25 18:15, Markus Probst wrote:
> 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 the code responsible for the registration into `pwm::Chip::new`. The
> registration will happen before the driver gets access to the refcounted
> pwm chip and can therefore guarantee that the registration isn't called
> twice on the same pwm chip.
I agree that the double registration safety issue needs to be fixed.
However, merging the allocation and registration steps into Chip::new
restricts the API significantly.
There is a specific reason the C API separates pwmchip_alloc and
pwmchip_add. Drivers might need to configure the chip e.g. setting the
atomic flag, or acquire resources that require a pointer to the chip
(like requesting an IRQ) after allocation but before the device is
registered and visible to the subsystem.
I suggest we solve this using the Builder Pattern instead. This would
allow us to allocate, configure, and finally register the chip in
distinct safe steps.
Best regards,
--
Michal Wilczynski <m.wilczynski@samsung.com>
>
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
> This patch provides the additional guarantee that the pwm chip doesn't
> get registered twice.
>
> The following changes were made:
> - change the visibility of `pwm::Registration` to private
> - remove the `pwm::Registration::register` function
> - add code for registering the pwm chip in `pwm::Chip::new`
> - add Send + Sync bounds to `PwmOps`
>
> Note that I wasn't able to test this patch, due to the lack of hardware.
> ---
> drivers/pwm/pwm_th1520.rs | 4 +---
> rust/kernel/pwm.rs | 53 ++++++++++++++++++-----------------------------
> 2 files changed, 21 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/pwm/pwm_th1520.rs b/drivers/pwm/pwm_th1520.rs
> index 955c359b07fb..1919b5a1f69e 100644
> --- a/drivers/pwm/pwm_th1520.rs
> +++ b/drivers/pwm/pwm_th1520.rs
> @@ -363,7 +363,7 @@ fn probe(
> return Err(EINVAL);
> }
>
> - let chip = pwm::Chip::new(
> + pwm::Chip::new(
> dev,
> TH1520_MAX_PWM_NUM,
> try_pin_init!(Th1520PwmDriverData {
> @@ -372,8 +372,6 @@ fn probe(
> }),
> )?;
>
> - pwm::Registration::register(dev, chip)?;
> -
> Ok(KBox::new(Th1520PwmPlatformDriver, GFP_KERNEL)?.into())
> }
> }
> diff --git a/rust/kernel/pwm.rs b/rust/kernel/pwm.rs
> index cb00f8a8765c..c5d03ee8bc95 100644
> --- a/rust/kernel/pwm.rs
> +++ b/rust/kernel/pwm.rs
> @@ -173,7 +173,7 @@ pub struct RoundedWaveform<WfHw> {
> }
>
> /// 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,7 +585,7 @@ unsafe fn bound_parent_device(&self) -> &device::Device<Bound> {
> /// Returns an [`ARef<Chip>`] managing the chip's lifetime via refcounting
> /// on its embedded `struct device`.
> pub fn new(
> - parent_dev: &device::Device,
> + parent_dev: &device::Device<Bound>,
> num_channels: u32,
> data: impl pin_init::PinInit<T, Error>,
> ) -> Result<ARef<Self>> {
> @@ -623,7 +623,21 @@ pub fn new(
> // 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)) };
> +
> + // 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.
> + unsafe {
> + to_result(bindings::__pwmchip_add(c_chip_ptr, core::ptr::null_mut()))?;
> + }
> +
> + let registration = Registration {
> + chip: ARef::clone(&chip),
> + };
> +
> + devres::register(parent_dev, registration, GFP_KERNEL)?;
> +
> + Ok(chip)
> }
> }
>
> @@ -654,50 +668,23 @@ unsafe fn dec_ref(obj: NonNull<Chip<T>>) {
> // 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<T: PwmOps + Send> Send for Chip<T> {}
> +unsafe impl<T: PwmOps> Send for Chip<T> {}
>
> // 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<T: PwmOps + Sync> Sync for Chip<T> {}
> +unsafe impl<T: PwmOps> Sync for Chip<T> {}
>
> /// 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<T: PwmOps> {
> +struct Registration<T: PwmOps> {
> chip: ARef<Chip<T>>,
> }
>
> -impl<T: 'static + PwmOps + Send + Sync> Registration<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<Bound>, chip: ARef<Chip<T>>) -> 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();
> -
> - // 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.
> - unsafe {
> - to_result(bindings::__pwmchip_add(c_chip_ptr, core::ptr::null_mut()))?;
> - }
> -
> - let registration = Registration { chip };
> -
> - devres::register(dev, registration, GFP_KERNEL)
> - }
> -}
> -
> impl<T: PwmOps> Drop for Registration<T> {
> fn drop(&mut self) {
> let chip_raw = self.chip.as_raw();
>
> ---
> base-commit: fae00ea9f00367771003ace78f29549dead58fc7
> change-id: 20251127-pwm_safe_register-e49b0a261101
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] Move pwm registration into pwm::Chip::new
@ 2025-11-30 22:13 ` Michal Wilczynski
0 siblings, 0 replies; 14+ messages in thread
From: Michal Wilczynski @ 2025-11-30 22:13 UTC (permalink / raw)
To: Markus Probst
Cc: linux-riscv, linux-pwm, linux-kernel, rust-for-linux,
Drew Fustini, Guo Ren, Fu Wei, Uwe Kleine-König,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich
Hi Markus,
On 11/27/25 18:15, Markus Probst wrote:
> 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 the code responsible for the registration into `pwm::Chip::new`. The
> registration will happen before the driver gets access to the refcounted
> pwm chip and can therefore guarantee that the registration isn't called
> twice on the same pwm chip.
I agree that the double registration safety issue needs to be fixed.
However, merging the allocation and registration steps into Chip::new
restricts the API significantly.
There is a specific reason the C API separates pwmchip_alloc and
pwmchip_add. Drivers might need to configure the chip e.g. setting the
atomic flag, or acquire resources that require a pointer to the chip
(like requesting an IRQ) after allocation but before the device is
registered and visible to the subsystem.
I suggest we solve this using the Builder Pattern instead. This would
allow us to allocate, configure, and finally register the chip in
distinct safe steps.
Best regards,
--
Michal Wilczynski <m.wilczynski@samsung.com>
>
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
> This patch provides the additional guarantee that the pwm chip doesn't
> get registered twice.
>
> The following changes were made:
> - change the visibility of `pwm::Registration` to private
> - remove the `pwm::Registration::register` function
> - add code for registering the pwm chip in `pwm::Chip::new`
> - add Send + Sync bounds to `PwmOps`
>
> Note that I wasn't able to test this patch, due to the lack of hardware.
> ---
> drivers/pwm/pwm_th1520.rs | 4 +---
> rust/kernel/pwm.rs | 53 ++++++++++++++++++-----------------------------
> 2 files changed, 21 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/pwm/pwm_th1520.rs b/drivers/pwm/pwm_th1520.rs
> index 955c359b07fb..1919b5a1f69e 100644
> --- a/drivers/pwm/pwm_th1520.rs
> +++ b/drivers/pwm/pwm_th1520.rs
> @@ -363,7 +363,7 @@ fn probe(
> return Err(EINVAL);
> }
>
> - let chip = pwm::Chip::new(
> + pwm::Chip::new(
> dev,
> TH1520_MAX_PWM_NUM,
> try_pin_init!(Th1520PwmDriverData {
> @@ -372,8 +372,6 @@ fn probe(
> }),
> )?;
>
> - pwm::Registration::register(dev, chip)?;
> -
> Ok(KBox::new(Th1520PwmPlatformDriver, GFP_KERNEL)?.into())
> }
> }
> diff --git a/rust/kernel/pwm.rs b/rust/kernel/pwm.rs
> index cb00f8a8765c..c5d03ee8bc95 100644
> --- a/rust/kernel/pwm.rs
> +++ b/rust/kernel/pwm.rs
> @@ -173,7 +173,7 @@ pub struct RoundedWaveform<WfHw> {
> }
>
> /// 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,7 +585,7 @@ unsafe fn bound_parent_device(&self) -> &device::Device<Bound> {
> /// Returns an [`ARef<Chip>`] managing the chip's lifetime via refcounting
> /// on its embedded `struct device`.
> pub fn new(
> - parent_dev: &device::Device,
> + parent_dev: &device::Device<Bound>,
> num_channels: u32,
> data: impl pin_init::PinInit<T, Error>,
> ) -> Result<ARef<Self>> {
> @@ -623,7 +623,21 @@ pub fn new(
> // 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)) };
> +
> + // 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.
> + unsafe {
> + to_result(bindings::__pwmchip_add(c_chip_ptr, core::ptr::null_mut()))?;
> + }
> +
> + let registration = Registration {
> + chip: ARef::clone(&chip),
> + };
> +
> + devres::register(parent_dev, registration, GFP_KERNEL)?;
> +
> + Ok(chip)
> }
> }
>
> @@ -654,50 +668,23 @@ unsafe fn dec_ref(obj: NonNull<Chip<T>>) {
> // 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<T: PwmOps + Send> Send for Chip<T> {}
> +unsafe impl<T: PwmOps> Send for Chip<T> {}
>
> // 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<T: PwmOps + Sync> Sync for Chip<T> {}
> +unsafe impl<T: PwmOps> Sync for Chip<T> {}
>
> /// 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<T: PwmOps> {
> +struct Registration<T: PwmOps> {
> chip: ARef<Chip<T>>,
> }
>
> -impl<T: 'static + PwmOps + Send + Sync> Registration<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<Bound>, chip: ARef<Chip<T>>) -> 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();
> -
> - // 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.
> - unsafe {
> - to_result(bindings::__pwmchip_add(c_chip_ptr, core::ptr::null_mut()))?;
> - }
> -
> - let registration = Registration { chip };
> -
> - devres::register(dev, registration, GFP_KERNEL)
> - }
> -}
> -
> impl<T: PwmOps> Drop for Registration<T> {
> fn drop(&mut self) {
> let chip_raw = self.chip.as_raw();
>
> ---
> base-commit: fae00ea9f00367771003ace78f29549dead58fc7
> change-id: 20251127-pwm_safe_register-e49b0a261101
>
>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 14+ messages in thread