* [PATCH v16 1/7] pwm: Export `pwmchip_release` for external use
2025-10-16 13:38 ` [PATCH v16 0/7] Rust Abstractions for PWM subsystem with TH1520 PWM driver Michal Wilczynski
@ 2025-10-16 13:38 ` Michal Wilczynski
2025-10-16 13:38 ` [PATCH v16 2/7] rust: pwm: Add Kconfig and basic data structures Michal Wilczynski
` (7 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Michal Wilczynski @ 2025-10-16 13:38 UTC (permalink / raw)
To: Uwe Kleine-König, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Michal Wilczynski, Guo Ren,
Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Marek Szyprowski, Benno Lossin, Michael Turquette, Drew Fustini,
Daniel Almeida, Benno Lossin, Drew Fustini
Cc: linux-kernel, linux-pwm, rust-for-linux, linux-riscv, devicetree,
Elle Rhumsaa
The upcoming Rust abstraction layer for the PWM subsystem uses a custom
`dev->release` handler to safely manage the lifetime of its driver
data.
To prevent leaking the memory of the `struct pwm_chip` (allocated by
`pwmchip_alloc`), this custom handler must also call the original
`pwmchip_release` function to complete the cleanup.
Make `pwmchip_release` a global, exported function so that it can be
called from the Rust FFI bridge. This involves removing the `static`
keyword, adding a prototype to the public header, and exporting the
symbol.
Reviewed-by: Elle Rhumsaa <elle@weathered-steel.dev>
Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
drivers/pwm/core.c | 3 ++-
include/linux/pwm.h | 6 ++++++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index ea2ccf42e81441d00a349f6d8641c92143d94797..47c9333baaf6c7d752a71c26bb1957ea2fd17a50 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -1608,12 +1608,13 @@ void pwmchip_put(struct pwm_chip *chip)
}
EXPORT_SYMBOL_GPL(pwmchip_put);
-static void pwmchip_release(struct device *pwmchip_dev)
+void pwmchip_release(struct device *pwmchip_dev)
{
struct pwm_chip *chip = pwmchip_from_dev(pwmchip_dev);
kfree(chip);
}
+EXPORT_SYMBOL_GPL(pwmchip_release);
struct pwm_chip *pwmchip_alloc(struct device *parent, unsigned int npwm, size_t sizeof_priv)
{
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 549ac4aaad59ba6d7ba58d818b46a58a8759e09c..148f056f336bbe17ffc3df22aa6b6bc3ad854e40 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -488,6 +488,12 @@ int __pwmchip_add(struct pwm_chip *chip, struct module *owner);
#define pwmchip_add(chip) __pwmchip_add(chip, THIS_MODULE)
void pwmchip_remove(struct pwm_chip *chip);
+/*
+ * For FFI wrapper use only:
+ * The Rust PWM abstraction needs this to properly free the pwm_chip.
+ */
+void pwmchip_release(struct device *dev);
+
int __devm_pwmchip_add(struct device *dev, struct pwm_chip *chip, struct module *owner);
#define devm_pwmchip_add(dev, chip) __devm_pwmchip_add(dev, chip, THIS_MODULE)
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v16 2/7] rust: pwm: Add Kconfig and basic data structures
2025-10-16 13:38 ` [PATCH v16 0/7] Rust Abstractions for PWM subsystem with TH1520 PWM driver Michal Wilczynski
2025-10-16 13:38 ` [PATCH v16 1/7] pwm: Export `pwmchip_release` for external use Michal Wilczynski
@ 2025-10-16 13:38 ` Michal Wilczynski
2025-10-16 13:38 ` [PATCH v16 3/7] rust: pwm: Add complete abstraction layer Michal Wilczynski
` (6 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Michal Wilczynski @ 2025-10-16 13:38 UTC (permalink / raw)
To: Uwe Kleine-König, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Michal Wilczynski, Guo Ren,
Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Marek Szyprowski, Benno Lossin, Michael Turquette, Drew Fustini,
Daniel Almeida, Benno Lossin, Drew Fustini
Cc: linux-kernel, linux-pwm, rust-for-linux, linux-riscv, devicetree,
Elle Rhumsaa
Introduce the foundational support for PWM abstractions in Rust.
This commit adds the `RUST_PWM_ABSTRACTIONS` Kconfig option to enable
the feature, along with the necessary build-system support and C
helpers.
It also introduces the first set of safe wrappers for the PWM
subsystem, covering the basic data carrying C structs and enums:
- `Polarity`: A safe wrapper for `enum pwm_polarity`.
- `Waveform`: A wrapper for `struct pwm_waveform`.
- `State`: A wrapper for `struct pwm_state`.
These types provide memory safe, idiomatic Rust representations of the
core PWM data structures and form the building blocks for the
abstractions that will follow.
Tested-by: Drew Fustini <fustini@kernel.org>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Elle Rhumsaa <elle@weathered-steel.dev>
Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
MAINTAINERS | 8 ++++
drivers/pwm/Kconfig | 12 +++++
rust/bindings/bindings_helper.h | 1 +
rust/helpers/helpers.c | 1 +
rust/helpers/pwm.c | 20 ++++++++
rust/kernel/lib.rs | 2 +
rust/kernel/pwm.rs | 102 ++++++++++++++++++++++++++++++++++++++++
7 files changed, 146 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 46126ce2f968e4f9260263f1574ee29f5ff0de1c..b01a016373c85fb1879e8948cedfc4e2ebc3915f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20763,6 +20763,14 @@ F: include/linux/pwm.h
F: include/linux/pwm_backlight.h
K: pwm_(config|apply_might_sleep|apply_atomic|ops)
+PWM SUBSYSTEM BINDINGS [RUST]
+M: Michal Wilczynski <m.wilczynski@samsung.com>
+L: linux-pwm@vger.kernel.org
+L: rust-for-linux@vger.kernel.org
+S: Maintained
+F: rust/helpers/pwm.c
+F: rust/kernel/pwm.rs
+
PXA GPIO DRIVER
M: Robert Jarzmik <robert.jarzmik@free.fr>
L: linux-gpio@vger.kernel.org
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index c2fd3f4b62d9ea422a51a73fa87dc7a73703ebaf..d87c4521268c18e1d11ced8c8c72ec79abc67b22 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -819,4 +819,16 @@ config PWM_XILINX
To compile this driver as a module, choose M here: the module
will be called pwm-xilinx.
+ config RUST_PWM_ABSTRACTIONS
+ bool
+ depends on RUST
+ help
+ This option enables the safe Rust abstraction layer for the PWM
+ subsystem. It provides idiomatic wrappers and traits necessary for
+ writing PWM controller drivers in Rust.
+
+ The abstractions handle resource management (like memory and reference
+ counting) and provide safe interfaces to the underlying C core,
+ allowing driver logic to be written in safe Rust.
+
endif
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 2e43c66635a2c9f31bd99b9817bd2d6ab89fbcf2..70b11fc6338c4f38b854419ab5bc44afe4905678 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -72,6 +72,7 @@
#include <linux/pm_opp.h>
#include <linux/poll.h>
#include <linux/property.h>
+#include <linux/pwm.h>
#include <linux/random.h>
#include <linux/refcount.h>
#include <linux/regulator/consumer.h>
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 551da6c9b5064c324d6f62bafcec672c6c6f5bee..014f20df9148220809ea2b8dcfe9776b0fe214d3 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -43,6 +43,7 @@
#include "poll.c"
#include "processor.c"
#include "property.c"
+#include "pwm.c"
#include "rbtree.c"
#include "rcu.c"
#include "refcount.c"
diff --git a/rust/helpers/pwm.c b/rust/helpers/pwm.c
new file mode 100644
index 0000000000000000000000000000000000000000..d75c588863685d3990b525bb1b84aa4bc35ac397
--- /dev/null
+++ b/rust/helpers/pwm.c
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2025 Samsung Electronics Co., Ltd.
+// Author: Michal Wilczynski <m.wilczynski@samsung.com>
+
+#include <linux/pwm.h>
+
+struct device *rust_helper_pwmchip_parent(const struct pwm_chip *chip)
+{
+ return pwmchip_parent(chip);
+}
+
+void *rust_helper_pwmchip_get_drvdata(struct pwm_chip *chip)
+{
+ return pwmchip_get_drvdata(chip);
+}
+
+void rust_helper_pwmchip_set_drvdata(struct pwm_chip *chip, void *data)
+{
+ pwmchip_set_drvdata(chip, data);
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 3dd7bebe78882a932f54f305864d5f372a7cd695..68c71d888fdb84183569120f0673c0f983d0ec9b 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -129,6 +129,8 @@
pub mod seq_file;
pub mod sizes;
mod static_assert;
+#[cfg(CONFIG_RUST_PWM_ABSTRACTIONS)]
+pub mod pwm;
#[doc(hidden)]
pub mod std_vendor;
pub mod str;
diff --git a/rust/kernel/pwm.rs b/rust/kernel/pwm.rs
new file mode 100644
index 0000000000000000000000000000000000000000..beabf0086a2f1beea01e0b0a9f6540c601f77a49
--- /dev/null
+++ b/rust/kernel/pwm.rs
@@ -0,0 +1,102 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2025 Samsung Electronics Co., Ltd.
+// Author: Michal Wilczynski <m.wilczynski@samsung.com>
+
+//! PWM subsystem abstractions.
+//!
+//! C header: [`include/linux/pwm.h`](srctree/include/linux/pwm.h).
+
+use crate::{
+ bindings,
+ prelude::*,
+ types::Opaque,
+};
+use core::convert::TryFrom;
+
+/// PWM polarity. Mirrors [`enum pwm_polarity`](srctree/include/linux/pwm.h).
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+pub enum Polarity {
+ /// Normal polarity (duty cycle defines the high period of the signal).
+ Normal,
+
+ /// Inversed polarity (duty cycle defines the low period of the signal).
+ Inversed,
+}
+
+impl TryFrom<bindings::pwm_polarity> for Polarity {
+ type Error = Error;
+
+ fn try_from(polarity: bindings::pwm_polarity) -> Result<Self, Error> {
+ match polarity {
+ bindings::pwm_polarity_PWM_POLARITY_NORMAL => Ok(Polarity::Normal),
+ bindings::pwm_polarity_PWM_POLARITY_INVERSED => Ok(Polarity::Inversed),
+ _ => Err(EINVAL),
+ }
+ }
+}
+
+impl From<Polarity> for bindings::pwm_polarity {
+ fn from(polarity: Polarity) -> Self {
+ match polarity {
+ Polarity::Normal => bindings::pwm_polarity_PWM_POLARITY_NORMAL,
+ Polarity::Inversed => bindings::pwm_polarity_PWM_POLARITY_INVERSED,
+ }
+ }
+}
+
+/// Represents a PWM waveform configuration.
+/// Mirrors struct [`struct pwm_waveform`](srctree/include/linux/pwm.h).
+#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)]
+pub struct Waveform {
+ /// Total duration of one complete PWM cycle, in nanoseconds.
+ pub period_length_ns: u64,
+
+ /// Duty-cycle active time, in nanoseconds.
+ ///
+ /// For a typical normal polarity configuration (active-high) this is the
+ /// high time of the signal.
+ pub duty_length_ns: u64,
+
+ /// Duty-cycle start offset, in nanoseconds.
+ ///
+ /// Delay from the beginning of the period to the first active edge.
+ /// In most simple PWM setups this is `0`, so the duty cycle starts
+ /// immediately at each period’s start.
+ pub duty_offset_ns: u64,
+}
+
+impl From<bindings::pwm_waveform> for Waveform {
+ fn from(wf: bindings::pwm_waveform) -> Self {
+ Waveform {
+ period_length_ns: wf.period_length_ns,
+ duty_length_ns: wf.duty_length_ns,
+ duty_offset_ns: wf.duty_offset_ns,
+ }
+ }
+}
+
+impl From<Waveform> for bindings::pwm_waveform {
+ fn from(wf: Waveform) -> Self {
+ bindings::pwm_waveform {
+ period_length_ns: wf.period_length_ns,
+ duty_length_ns: wf.duty_length_ns,
+ duty_offset_ns: wf.duty_offset_ns,
+ }
+ }
+}
+
+/// Wrapper for PWM state [`struct pwm_state`](srctree/include/linux/pwm.h).
+#[repr(transparent)]
+pub struct State(bindings::pwm_state);
+
+impl State {
+ /// Creates a `State` wrapper by taking ownership of a C `pwm_state` value.
+ pub(crate) fn from_c(c_state: bindings::pwm_state) -> Self {
+ State(c_state)
+ }
+
+ /// Returns `true` if the PWM signal is enabled.
+ pub fn enabled(&self) -> bool {
+ self.0.enabled
+ }
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v16 3/7] rust: pwm: Add complete abstraction layer
2025-10-16 13:38 ` [PATCH v16 0/7] Rust Abstractions for PWM subsystem with TH1520 PWM driver Michal Wilczynski
2025-10-16 13:38 ` [PATCH v16 1/7] pwm: Export `pwmchip_release` for external use Michal Wilczynski
2025-10-16 13:38 ` [PATCH v16 2/7] rust: pwm: Add Kconfig and basic data structures Michal Wilczynski
@ 2025-10-16 13:38 ` Michal Wilczynski
2025-10-16 13:38 ` [PATCH v16 4/7] pwm: Add Rust driver for T-HEAD TH1520 SoC Michal Wilczynski
` (5 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Michal Wilczynski @ 2025-10-16 13:38 UTC (permalink / raw)
To: Uwe Kleine-König, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Michal Wilczynski, Guo Ren,
Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Marek Szyprowski, Benno Lossin, Michael Turquette, Drew Fustini,
Daniel Almeida, Benno Lossin, Drew Fustini
Cc: linux-kernel, linux-pwm, rust-for-linux, linux-riscv, devicetree,
Elle Rhumsaa
Introduce a comprehensive abstraction layer for the PWM subsystem to
enable writing drivers in Rust.
Because `Device`, `Chip`, and `PwmOps` all refer to each other, they
form a single, indivisible unit with circular dependencies. They are
introduced together in this single commit to create a complete,
compilable abstraction layer.
The main components are:
- Data Wrappers: Safe, idiomatic wrappers for core C types like
`pwm_device`, and `pwm_chip`.
- PwmOps Trait: An interface that drivers can implement to provide
their hardware-specific logic, mirroring the C `pwm_ops` interface.
- FFI VTable and Adapter: A bridge to connect the high-level PwmOps trait
to the C kernel's pwm_ops vtable.
- Allocation and Lifetime Management: A high-level `Chip::new()`
API to safely allocate a chip and a `Registration` guard that integrates
with `devres` to manage the chip's registration with the PWM core.
An `AlwaysRefCounted` implementation and a custom release handler
prevent memory leaks by managing the chip's lifetime and freeing
driver data correctly.
Reviewed-by: Danilo Krummrich <dakr@kernel.org>
Reviewed-by: Elle Rhumsaa <elle@weathered-steel.dev>
Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
rust/kernel/pwm.rs | 664 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 662 insertions(+), 2 deletions(-)
diff --git a/rust/kernel/pwm.rs b/rust/kernel/pwm.rs
index beabf0086a2f1beea01e0b0a9f6540c601f77a49..79fbb13cd47f75681283648ddc4fffb7889be930 100644
--- a/rust/kernel/pwm.rs
+++ b/rust/kernel/pwm.rs
@@ -8,10 +8,14 @@
use crate::{
bindings,
+ container_of,
+ device::{self, Bound},
+ devres,
+ error::{self, to_result},
prelude::*,
- types::Opaque,
+ types::{ARef, AlwaysRefCounted, Opaque},
};
-use core::convert::TryFrom;
+use core::{convert::TryFrom, marker::PhantomData, ptr::NonNull};
/// PWM polarity. Mirrors [`enum pwm_polarity`](srctree/include/linux/pwm.h).
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
@@ -100,3 +104,659 @@ pub fn enabled(&self) -> bool {
self.0.enabled
}
}
+
+/// Describes the outcome of a `round_waveform` operation.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum RoundingOutcome {
+ /// The requested waveform was achievable exactly or by rounding values down.
+ ExactOrRoundedDown,
+
+ /// The requested waveform could only be achieved by rounding up.
+ RoundedUp,
+}
+
+/// Wrapper for a PWM device [`struct pwm_device`](srctree/include/linux/pwm.h).
+#[repr(transparent)]
+pub struct Device(Opaque<bindings::pwm_device>);
+
+impl Device {
+ /// Creates a reference to a [`Device`] from a valid C pointer.
+ ///
+ /// # Safety
+ ///
+ /// The caller must ensure that `ptr` is valid and remains valid for the lifetime of the
+ /// returned [`Device`] reference.
+ pub(crate) unsafe fn from_raw<'a>(ptr: *mut bindings::pwm_device) -> &'a Self {
+ // SAFETY: The safety requirements guarantee the validity of the dereference, while the
+ // `Device` type being transparent makes the cast ok.
+ unsafe { &*ptr.cast::<Self>() }
+ }
+
+ /// Returns a raw pointer to the underlying `pwm_device`.
+ fn as_raw(&self) -> *mut bindings::pwm_device {
+ self.0.get()
+ }
+
+ /// Gets the hardware PWM index for this device within its chip.
+ pub fn hwpwm(&self) -> u32 {
+ // SAFETY: `self.as_raw()` provides a valid pointer for `self`'s lifetime.
+ unsafe { (*self.as_raw()).hwpwm }
+ }
+
+ /// Gets a reference to the parent `Chip` that this device belongs to.
+ pub fn chip<T: PwmOps>(&self) -> &Chip<T> {
+ // SAFETY: `self.as_raw()` provides a valid pointer. (*self.as_raw()).chip
+ // is assumed to be a valid pointer to `pwm_chip` managed by the kernel.
+ // Chip::from_raw's safety conditions must be met.
+ unsafe { Chip::<T>::from_raw((*self.as_raw()).chip) }
+ }
+
+ /// Gets the label for this PWM device, if any.
+ pub fn label(&self) -> Option<&CStr> {
+ // SAFETY: self.as_raw() provides a valid pointer.
+ let label_ptr = unsafe { (*self.as_raw()).label };
+ if label_ptr.is_null() {
+ return None
+ }
+
+ // SAFETY: label_ptr is non-null and points to a C string
+ // managed by the kernel, valid for the lifetime of the PWM device.
+ Some(unsafe { CStr::from_char_ptr(label_ptr) })
+ }
+
+ /// Gets a copy of the current state of this PWM device.
+ pub fn state(&self) -> State {
+ // SAFETY: `self.as_raw()` gives a valid pointer. `(*self.as_raw()).state`
+ // is a valid `pwm_state` struct. `State::from_c` copies this data.
+ State::from_c(unsafe { (*self.as_raw()).state })
+ }
+
+ /// Sets the PWM waveform configuration and enables the PWM signal.
+ pub fn set_waveform(&self, wf: &Waveform, exact: bool) -> Result {
+ let c_wf = bindings::pwm_waveform::from(*wf);
+
+ // SAFETY: `self.as_raw()` provides a valid `*mut pwm_device` pointer.
+ // `&c_wf` is a valid pointer to a `pwm_waveform` struct. The C function
+ // handles all necessary internal locking.
+ let ret = unsafe { bindings::pwm_set_waveform_might_sleep(self.as_raw(), &c_wf, exact) };
+ to_result(ret)
+ }
+
+ /// Queries the hardware for the configuration it would apply for a given
+ /// request.
+ pub fn round_waveform(&self, wf: &mut Waveform) -> Result<RoundingOutcome> {
+ let mut c_wf = bindings::pwm_waveform::from(*wf);
+
+ // SAFETY: `self.as_raw()` provides a valid `*mut pwm_device` pointer.
+ // `&mut c_wf` is a valid pointer to a mutable `pwm_waveform` struct that
+ // the C function will update.
+ let ret = unsafe { bindings::pwm_round_waveform_might_sleep(self.as_raw(), &mut c_wf) };
+
+ to_result(ret)?;
+
+ *wf = Waveform::from(c_wf);
+
+ if ret == 1 {
+ Ok(RoundingOutcome::RoundedUp)
+ } else {
+ Ok(RoundingOutcome::ExactOrRoundedDown)
+ }
+ }
+
+ /// Reads the current waveform configuration directly from the hardware.
+ pub fn get_waveform(&self) -> Result<Waveform> {
+ let mut c_wf = bindings::pwm_waveform::default();
+
+ // SAFETY: `self.as_raw()` is a valid pointer. We provide a valid pointer
+ // to a stack-allocated `pwm_waveform` struct for the kernel to fill.
+ let ret = unsafe { bindings::pwm_get_waveform_might_sleep(self.as_raw(), &mut c_wf) };
+
+ to_result(ret)?;
+
+ Ok(Waveform::from(c_wf))
+ }
+}
+
+/// The result of a `round_waveform_tohw` operation.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub struct RoundedWaveform<WfHw> {
+ /// A status code, 0 for success or 1 if values were rounded up.
+ pub status: c_int,
+ /// The driver-specific hardware representation of the waveform.
+ pub hardware_waveform: WfHw,
+}
+
+/// Trait defining the operations for a PWM driver.
+pub trait PwmOps: 'static + Sized {
+ /// The driver-specific hardware representation of a waveform.
+ ///
+ /// This type must be [`Copy`], [`Default`], and fit within `PWM_WFHWSIZE`.
+ type WfHw: Copy + Default;
+
+ /// Optional hook for when a PWM device is requested.
+ fn request(
+ _chip: &Chip<Self>,
+ _pwm: &Device,
+ _parent_dev: &device::Device<Bound>,
+ ) -> Result {
+ Ok(())
+ }
+
+ /// Optional hook for capturing a PWM signal.
+ fn capture(
+ _chip: &Chip<Self>,
+ _pwm: &Device,
+ _result: &mut bindings::pwm_capture,
+ _timeout: usize,
+ _parent_dev: &device::Device<Bound>,
+ ) -> Result {
+ Err(ENOTSUPP)
+ }
+
+ /// Convert a generic waveform to the hardware-specific representation.
+ /// This is typically a pure calculation and does not perform I/O.
+ fn round_waveform_tohw(
+ _chip: &Chip<Self>,
+ _pwm: &Device,
+ _wf: &Waveform,
+ ) -> Result<RoundedWaveform<Self::WfHw>> {
+ Err(ENOTSUPP)
+ }
+
+ /// Convert a hardware-specific representation back to a generic waveform.
+ /// This is typically a pure calculation and does not perform I/O.
+ fn round_waveform_fromhw(
+ _chip: &Chip<Self>,
+ _pwm: &Device,
+ _wfhw: &Self::WfHw,
+ _wf: &mut Waveform,
+ ) -> Result {
+ Err(ENOTSUPP)
+ }
+
+ /// Read the current hardware configuration into the hardware-specific representation.
+ fn read_waveform(
+ _chip: &Chip<Self>,
+ _pwm: &Device,
+ _parent_dev: &device::Device<Bound>,
+ ) -> Result<Self::WfHw> {
+ Err(ENOTSUPP)
+ }
+
+ /// Write a hardware-specific waveform configuration to the hardware.
+ fn write_waveform(
+ _chip: &Chip<Self>,
+ _pwm: &Device,
+ _wfhw: &Self::WfHw,
+ _parent_dev: &device::Device<Bound>,
+ ) -> Result {
+ Err(ENOTSUPP)
+ }
+}
+
+/// Bridges Rust `PwmOps` to the C `pwm_ops` vtable.
+struct Adapter<T: PwmOps> {
+ _p: PhantomData<T>,
+}
+
+impl<T: PwmOps> Adapter<T> {
+ const VTABLE: PwmOpsVTable = create_pwm_ops::<T>();
+
+ /// # Safety
+ ///
+ /// `wfhw_ptr` must be valid for writes of `size_of::<T::WfHw>()` bytes.
+ unsafe fn serialize_wfhw(wfhw: &T::WfHw, wfhw_ptr: *mut c_void) -> Result {
+ let size = core::mem::size_of::<T::WfHw>();
+
+ build_assert!(size <= bindings::PWM_WFHWSIZE as usize);
+
+ // SAFETY: The caller ensures `wfhw_ptr` is valid for `size` bytes.
+ unsafe {
+ core::ptr::copy_nonoverlapping(
+ core::ptr::from_ref::<T::WfHw>(wfhw).cast::<u8>(),
+ wfhw_ptr.cast::<u8>(),
+ size,
+ );
+ }
+
+ Ok(())
+ }
+
+ /// # Safety
+ ///
+ /// `wfhw_ptr` must be valid for reads of `size_of::<T::WfHw>()` bytes.
+ unsafe fn deserialize_wfhw(wfhw_ptr: *const c_void) -> Result<T::WfHw> {
+ let size = core::mem::size_of::<T::WfHw>();
+
+ build_assert!(size <= bindings::PWM_WFHWSIZE as usize);
+
+ let mut wfhw = T::WfHw::default();
+ // SAFETY: The caller ensures `wfhw_ptr` is valid for `size` bytes.
+ unsafe {
+ core::ptr::copy_nonoverlapping(
+ wfhw_ptr.cast::<u8>(),
+ core::ptr::from_mut::<T::WfHw>(&mut wfhw).cast::<u8>(),
+ size,
+ );
+ }
+
+ Ok(wfhw)
+ }
+
+ /// # Safety
+ ///
+ /// `dev` must be a valid pointer to a `bindings::device` embedded within a
+ /// `bindings::pwm_chip`. This function is called by the device core when the
+ /// last reference to the device is dropped.
+ unsafe extern "C" fn release_callback(dev: *mut bindings::device) {
+ // SAFETY: The function's contract guarantees that `dev` points to a `device`
+ // field embedded within a valid `pwm_chip`. `container_of!` can therefore
+ // safely calculate the address of the containing struct.
+ let c_chip_ptr = unsafe { container_of!(dev, bindings::pwm_chip, dev) };
+
+ // SAFETY: `c_chip_ptr` is a valid pointer to a `pwm_chip` as established
+ // above. Calling this FFI function is safe.
+ let drvdata_ptr = unsafe { bindings::pwmchip_get_drvdata(c_chip_ptr) };
+
+ // SAFETY: The driver data was initialized in `new`. We run its destructor here.
+ unsafe { core::ptr::drop_in_place(drvdata_ptr.cast::<T>()) };
+
+ // Now, call the original release function to free the `pwm_chip` itself.
+ // SAFETY: `dev` is the valid pointer passed into this callback, which is
+ // the expected argument for `pwmchip_release`.
+ unsafe { bindings::pwmchip_release(dev); }
+ }
+
+ /// # Safety
+ ///
+ /// Pointers from C must be valid.
+ unsafe extern "C" fn request_callback(
+ chip_ptr: *mut bindings::pwm_chip,
+ pwm_ptr: *mut bindings::pwm_device,
+ ) -> c_int {
+ // SAFETY: PWM core guarentees `chip_ptr` and `pwm_ptr` are valid pointers.
+ let (chip, pwm) = unsafe { (Chip::<T>::from_raw(chip_ptr), Device::from_raw(pwm_ptr)) };
+
+ // SAFETY: The PWM core guarantees the parent device exists and is bound during callbacks.
+ let bound_parent = unsafe { chip.bound_parent_device() };
+ match T::request(chip, pwm, bound_parent) {
+ Ok(()) => 0,
+ Err(e) => e.to_errno(),
+ }
+ }
+
+ /// # Safety
+ ///
+ /// Pointers from C must be valid.
+ unsafe extern "C" fn capture_callback(
+ chip_ptr: *mut bindings::pwm_chip,
+ pwm_ptr: *mut bindings::pwm_device,
+ res: *mut bindings::pwm_capture,
+ timeout: usize,
+ ) -> c_int {
+ // SAFETY: Relies on the function's contract that `chip_ptr` and `pwm_ptr` are valid
+ // pointers.
+ let (chip, pwm, result) = unsafe {
+ (
+ Chip::<T>::from_raw(chip_ptr),
+ Device::from_raw(pwm_ptr),
+ &mut *res,
+ )
+ };
+
+ // SAFETY: The PWM core guarantees the parent device exists and is bound during callbacks.
+ let bound_parent = unsafe { chip.bound_parent_device() };
+ match T::capture(chip, pwm, result, timeout, bound_parent) {
+ Ok(()) => 0,
+ Err(e) => e.to_errno(),
+ }
+ }
+
+ /// # Safety
+ ///
+ /// Pointers from C must be valid.
+ unsafe extern "C" fn round_waveform_tohw_callback(
+ chip_ptr: *mut bindings::pwm_chip,
+ pwm_ptr: *mut bindings::pwm_device,
+ wf_ptr: *const bindings::pwm_waveform,
+ wfhw_ptr: *mut c_void,
+ ) -> c_int {
+ // SAFETY: Relies on the function's contract that `chip_ptr` and `pwm_ptr` are valid
+ // pointers.
+ let (chip, pwm, wf) = unsafe {
+ (
+ Chip::<T>::from_raw(chip_ptr),
+ Device::from_raw(pwm_ptr),
+ Waveform::from(*wf_ptr),
+ )
+ };
+ match T::round_waveform_tohw(chip, pwm, &wf) {
+ Ok(rounded) => {
+ // SAFETY: `wfhw_ptr` is valid per this function's safety contract.
+ if unsafe { Self::serialize_wfhw(&rounded.hardware_waveform, wfhw_ptr) }.is_err() {
+ return EINVAL.to_errno();
+ }
+ rounded.status
+ }
+ Err(e) => e.to_errno(),
+ }
+ }
+
+ /// # Safety
+ ///
+ /// Pointers from C must be valid.
+ unsafe extern "C" fn round_waveform_fromhw_callback(
+ chip_ptr: *mut bindings::pwm_chip,
+ pwm_ptr: *mut bindings::pwm_device,
+ wfhw_ptr: *const c_void,
+ wf_ptr: *mut bindings::pwm_waveform,
+ ) -> c_int {
+ // SAFETY: Relies on the function's contract that `chip_ptr` and `pwm_ptr` are valid
+ // pointers.
+ let (chip, pwm) = unsafe { (Chip::<T>::from_raw(chip_ptr), Device::from_raw(pwm_ptr)) };
+ // SAFETY: `deserialize_wfhw`'s safety contract is met by this function's contract.
+ let wfhw = match unsafe { Self::deserialize_wfhw(wfhw_ptr) } {
+ Ok(v) => v,
+ Err(e) => return e.to_errno(),
+ };
+
+ let mut rust_wf = Waveform::default();
+ match T::round_waveform_fromhw(chip, pwm, &wfhw, &mut rust_wf) {
+ Ok(()) => {
+ // SAFETY: `wf_ptr` is guaranteed valid by the C caller.
+ unsafe {
+ *wf_ptr = rust_wf.into();
+ };
+ 0
+ }
+ Err(e) => e.to_errno(),
+ }
+ }
+
+ /// # Safety
+ ///
+ /// Pointers from C must be valid.
+ unsafe extern "C" fn read_waveform_callback(
+ chip_ptr: *mut bindings::pwm_chip,
+ pwm_ptr: *mut bindings::pwm_device,
+ wfhw_ptr: *mut c_void,
+ ) -> c_int {
+ // SAFETY: Relies on the function's contract that `chip_ptr` and `pwm_ptr` are valid
+ // pointers.
+ let (chip, pwm) = unsafe { (Chip::<T>::from_raw(chip_ptr), Device::from_raw(pwm_ptr)) };
+
+ // SAFETY: The PWM core guarantees the parent device exists and is bound during callbacks.
+ let bound_parent = unsafe { chip.bound_parent_device() };
+ match T::read_waveform(chip, pwm, bound_parent) {
+ // SAFETY: `wfhw_ptr` is valid per this function's safety contract.
+ Ok(wfhw) => match unsafe { Self::serialize_wfhw(&wfhw, wfhw_ptr) } {
+ Ok(()) => 0,
+ Err(e) => e.to_errno(),
+ },
+ Err(e) => e.to_errno(),
+ }
+ }
+
+ /// # Safety
+ ///
+ /// Pointers from C must be valid.
+ unsafe extern "C" fn write_waveform_callback(
+ chip_ptr: *mut bindings::pwm_chip,
+ pwm_ptr: *mut bindings::pwm_device,
+ wfhw_ptr: *const c_void,
+ ) -> c_int {
+ // SAFETY: Relies on the function's contract that `chip_ptr` and `pwm_ptr` are valid
+ // pointers.
+ let (chip, pwm) = unsafe { (Chip::<T>::from_raw(chip_ptr), Device::from_raw(pwm_ptr)) };
+
+ // SAFETY: The PWM core guarantees the parent device exists and is bound during callbacks.
+ let bound_parent = unsafe { chip.bound_parent_device() };
+
+ // SAFETY: `wfhw_ptr` is valid per this function's safety contract.
+ let wfhw = match unsafe { Self::deserialize_wfhw(wfhw_ptr) } {
+ Ok(v) => v,
+ Err(e) => return e.to_errno(),
+ };
+ match T::write_waveform(chip, pwm, &wfhw, bound_parent) {
+ Ok(()) => 0,
+ Err(e) => e.to_errno(),
+ }
+ }
+}
+
+/// VTable structure wrapper for PWM operations.
+/// Mirrors [`struct pwm_ops`](srctree/include/linux/pwm.h).
+#[repr(transparent)]
+pub struct PwmOpsVTable(bindings::pwm_ops);
+
+// SAFETY: PwmOpsVTable is Send. The vtable contains only function pointers
+// and a size, which are simple data types that can be safely moved across
+// threads. The thread-safety of calling these functions is handled by the
+// kernel's locking mechanisms.
+unsafe impl Send for PwmOpsVTable {}
+
+// SAFETY: PwmOpsVTable is Sync. The vtable is immutable after it is created,
+// so it can be safely referenced and accessed concurrently by multiple threads
+// e.g. to read the function pointers.
+unsafe impl Sync for PwmOpsVTable {}
+
+impl PwmOpsVTable {
+ /// Returns a raw pointer to the underlying `pwm_ops` struct.
+ pub(crate) fn as_raw(&self) -> *const bindings::pwm_ops {
+ &self.0
+ }
+}
+
+/// Creates a PWM operations vtable for a type `T` that implements `PwmOps`.
+///
+/// This is used to bridge Rust trait implementations to the C `struct pwm_ops`
+/// expected by the kernel.
+pub const fn create_pwm_ops<T: PwmOps>() -> PwmOpsVTable {
+ // SAFETY: `core::mem::zeroed()` is unsafe. For `pwm_ops`, all fields are
+ // `Option<extern "C" fn(...)>` or data, so a zeroed pattern (None/0) is valid initially.
+ let mut ops: bindings::pwm_ops = unsafe { core::mem::zeroed() };
+
+ ops.request = Some(Adapter::<T>::request_callback);
+ ops.capture = Some(Adapter::<T>::capture_callback);
+
+ ops.round_waveform_tohw = Some(Adapter::<T>::round_waveform_tohw_callback);
+ ops.round_waveform_fromhw = Some(Adapter::<T>::round_waveform_fromhw_callback);
+ ops.read_waveform = Some(Adapter::<T>::read_waveform_callback);
+ ops.write_waveform = Some(Adapter::<T>::write_waveform_callback);
+ ops.sizeof_wfhw = core::mem::size_of::<T::WfHw>();
+
+ PwmOpsVTable(ops)
+}
+
+/// Wrapper for a PWM chip/controller ([`struct pwm_chip`](srctree/include/linux/pwm.h)).
+#[repr(transparent)]
+pub struct Chip<T: PwmOps>(Opaque<bindings::pwm_chip>, PhantomData<T>);
+
+impl<T: PwmOps> Chip<T> {
+ /// Creates a reference to a [`Chip`] from a valid pointer.
+ ///
+ /// # Safety
+ ///
+ /// The caller must ensure that `ptr` is valid and remains valid for the lifetime of the
+ /// returned [`Chip`] reference.
+ pub(crate) unsafe fn from_raw<'a>(ptr: *mut bindings::pwm_chip) -> &'a Self {
+ // SAFETY: The safety requirements guarantee the validity of the dereference, while the
+ // `Chip` type being transparent makes the cast ok.
+ unsafe { &*ptr.cast::<Self>() }
+ }
+
+ /// Returns a raw pointer to the underlying `pwm_chip`.
+ pub(crate) fn as_raw(&self) -> *mut bindings::pwm_chip {
+ self.0.get()
+ }
+
+ /// Gets the number of PWM channels (hardware PWMs) on this chip.
+ pub fn num_channels(&self) -> u32 {
+ // SAFETY: `self.as_raw()` provides a valid pointer for `self`'s lifetime.
+ unsafe { (*self.as_raw()).npwm }
+ }
+
+ /// Returns `true` if the chip supports atomic operations for configuration.
+ pub fn is_atomic(&self) -> bool {
+ // SAFETY: `self.as_raw()` provides a valid pointer for `self`'s lifetime.
+ unsafe { (*self.as_raw()).atomic }
+ }
+
+ /// Returns a reference to the embedded `struct device` abstraction.
+ pub fn device(&self) -> &device::Device {
+ // SAFETY:
+ // - `self.as_raw()` provides a valid pointer to `bindings::pwm_chip`.
+ // - The `dev` field is an instance of `bindings::device` embedded
+ // within `pwm_chip`.
+ // - Taking a pointer to this embedded field is valid.
+ // - `device::Device` is `#[repr(transparent)]`.
+ // - The lifetime of the returned reference is tied to `self`.
+ unsafe { device::Device::from_raw(&raw mut (*self.as_raw()).dev) }
+ }
+
+ /// Gets the typed driver specific data associated with this chip's embedded device.
+ pub fn drvdata(&self) -> &T {
+ // SAFETY: `pwmchip_get_drvdata` returns the pointer to the private data area,
+ // which we know holds our `T`. The pointer is valid for the lifetime of `self`.
+ unsafe { &*bindings::pwmchip_get_drvdata(self.as_raw()).cast::<T>() }
+ }
+
+ /// Returns a reference to the parent device of this PWM chip's device.
+ ///
+ /// # Safety
+ ///
+ /// The caller must guarantee that the parent device exists and is bound.
+ /// This is guaranteed by the PWM core during `PwmOps` callbacks.
+ unsafe fn bound_parent_device(&self) -> &device::Device<Bound> {
+ // SAFETY: Per the function's safety contract, the parent device exists.
+ let parent = unsafe { self.device().parent().unwrap_unchecked() };
+
+ // SAFETY: Per the function's safety contract, the parent device is bound.
+ // This is guaranteed by the PWM core during `PwmOps` callbacks.
+ unsafe { parent.as_bound() }
+ }
+
+ /// Allocates and wraps a PWM chip using `bindings::pwmchip_alloc`.
+ ///
+ /// Returns an [`ARef<Chip>`] managing the chip's lifetime via refcounting
+ /// on its embedded `struct device`.
+ pub fn new(
+ parent_dev: &device::Device,
+ num_channels: u32,
+ data: impl pin_init::PinInit<T, Error>,
+ ) -> Result<ARef<Self>> {
+ let sizeof_priv = core::mem::size_of::<T>();
+ // SAFETY: `pwmchip_alloc` allocates memory for the C struct and our private data.
+ let c_chip_ptr_raw = unsafe {
+ bindings::pwmchip_alloc(parent_dev.as_raw(), num_channels, sizeof_priv)
+ };
+
+ let c_chip_ptr: *mut bindings::pwm_chip = error::from_err_ptr(c_chip_ptr_raw)?;
+
+ // SAFETY: The `drvdata` pointer is the start of the private area, which is where
+ // we will construct our `T` object.
+ let drvdata_ptr = unsafe { bindings::pwmchip_get_drvdata(c_chip_ptr) };
+
+ // SAFETY: We construct the `T` object in-place in the allocated private memory.
+ unsafe { data.__pinned_init(drvdata_ptr.cast())? };
+
+ // SAFETY: `c_chip_ptr` points to a valid chip.
+ unsafe { (*c_chip_ptr).dev.release = Some(Adapter::<T>::release_callback); }
+
+ // SAFETY: `c_chip_ptr` points to a valid chip.
+ // The `Adapter`'s `VTABLE` has a 'static lifetime, so the pointer
+ // returned by `as_raw()` is always valid.
+ unsafe { (*c_chip_ptr).ops = Adapter::<T>::VTABLE.as_raw(); }
+
+ // Cast the `*mut bindings::pwm_chip` to `*mut Chip`. This is valid because
+ // `Chip` is `repr(transparent)` over `Opaque<bindings::pwm_chip>`, and
+ // `Opaque<T>` is `repr(transparent)` over `T`.
+ let chip_ptr_as_self = c_chip_ptr.cast::<Self>();
+
+ // 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)) })
+ }
+}
+
+// SAFETY: Implements refcounting for `Chip` using the embedded `struct device`.
+unsafe impl<T: PwmOps> AlwaysRefCounted for Chip<T> {
+ #[inline]
+ fn inc_ref(&self) {
+ // SAFETY: `self.0.get()` points to a valid `pwm_chip` because `self` exists.
+ // The embedded `dev` is valid. `get_device` increments its refcount.
+ unsafe { bindings::get_device(&raw mut (*self.0.get()).dev); }
+ }
+
+ #[inline]
+ unsafe fn dec_ref(obj: NonNull<Chip<T>>) {
+ let c_chip_ptr = obj.cast::<bindings::pwm_chip>().as_ptr();
+
+ // SAFETY: `obj` is a valid pointer to a `Chip` (and thus `bindings::pwm_chip`)
+ // with a non-zero refcount. `put_device` handles decrement and final release.
+ unsafe { bindings::put_device(&raw mut (*c_chip_ptr).dev); }
+ }
+}
+
+// SAFETY: `Chip` is a wrapper around `*mut bindings::pwm_chip`. The underlying C
+// 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> {}
+
+// 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> {}
+
+/// 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> {
+ 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();
+
+ // SAFETY: `chip_raw` points to a chip that was successfully registered.
+ // `bindings::pwmchip_remove` is the correct C function to unregister it.
+ // This `drop` implementation is called automatically by `devres` on driver unbind.
+ unsafe { bindings::pwmchip_remove(chip_raw); }
+ }
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v16 4/7] pwm: Add Rust driver for T-HEAD TH1520 SoC
2025-10-16 13:38 ` [PATCH v16 0/7] Rust Abstractions for PWM subsystem with TH1520 PWM driver Michal Wilczynski
` (2 preceding siblings ...)
2025-10-16 13:38 ` [PATCH v16 3/7] rust: pwm: Add complete abstraction layer Michal Wilczynski
@ 2025-10-16 13:38 ` Michal Wilczynski
2025-10-16 13:38 ` [PATCH v16 5/7] dt-bindings: pwm: thead: Add T-HEAD TH1520 PWM controller Michal Wilczynski
` (4 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Michal Wilczynski @ 2025-10-16 13:38 UTC (permalink / raw)
To: Uwe Kleine-König, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Michal Wilczynski, Guo Ren,
Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Marek Szyprowski, Benno Lossin, Michael Turquette, Drew Fustini,
Daniel Almeida, Benno Lossin, Drew Fustini
Cc: linux-kernel, linux-pwm, rust-for-linux, linux-riscv, devicetree,
Elle Rhumsaa
Introduce a PWM driver for the T-HEAD TH1520 SoC, written in Rust and
utilizing the safe PWM abstractions from the preceding commit.
The driver implements the pwm::PwmOps trait using the modern waveform
API (round_waveform_tohw, write_waveform, etc.) to support configuration
of period, duty cycle, and polarity for the TH1520's PWM channels.
Resource management is handled using idiomatic Rust patterns. The PWM
chip object is allocated via pwm::Chip::new and its registration with
the PWM core is managed by the pwm::Registration RAII guard. This
ensures pwmchip_remove is always called when the driver unbinds,
preventing resource leaks. Device managed resources are used for the
MMIO region, and the clock lifecycle is correctly managed in the
driver's private data Drop implementation.
The driver's core logic is written entirely in safe Rust, with no unsafe
blocks, except for the Send and Sync implementations for the driver
data, which are explained in the comments.
Reviewed-by: Elle Rhumsaa <elle@weathered-steel.dev>
Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
MAINTAINERS | 1 +
drivers/pwm/Kconfig | 11 ++
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm_th1520.rs | 378 ++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 391 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index b01a016373c85fb1879e8948cedfc4e2ebc3915f..b4ca6192091bd31573c391b9b5dc29b74c9a684c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22189,6 +22189,7 @@ F: drivers/pinctrl/pinctrl-th1520.c
F: drivers/pmdomain/thead/
F: drivers/power/reset/th1520-aon-reboot.c
F: drivers/power/sequencing/pwrseq-thead-gpu.c
+F: drivers/pwm/pwm_th1520.rs
F: drivers/reset/reset-th1520.c
F: include/dt-bindings/clock/thead,th1520-clk-ap.h
F: include/dt-bindings/power/thead,th1520-power.h
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index d87c4521268c18e1d11ced8c8c72ec79abc67b22..0b47456e2d57bbdda54b1318911994812e315612 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -748,6 +748,17 @@ config PWM_TEGRA
To compile this driver as a module, choose M here: the module
will be called pwm-tegra.
+config PWM_TH1520
+ tristate "TH1520 PWM support"
+ depends on RUST
+ select RUST_PWM_ABSTRACTIONS
+ help
+ This option enables the driver for the PWM controller found on the
+ T-HEAD TH1520 SoC.
+
+ To compile this driver as a module, choose M here; the module
+ will be called pwm-th1520. If you are unsure, say N.
+
config PWM_TIECAP
tristate "ECAP PWM support"
depends on ARCH_OMAP2PLUS || ARCH_DAVINCI_DA8XX || ARCH_KEYSTONE || ARCH_K3 || COMPILE_TEST
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index dfa8b4966ee19af18ea47080db4adf96c326f3d7..aed403f0a42b339778c37150007635d7efccfd51 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -68,6 +68,7 @@ obj-$(CONFIG_PWM_STMPE) += pwm-stmpe.o
obj-$(CONFIG_PWM_SUN4I) += pwm-sun4i.o
obj-$(CONFIG_PWM_SUNPLUS) += pwm-sunplus.o
obj-$(CONFIG_PWM_TEGRA) += pwm-tegra.o
+obj-$(CONFIG_PWM_TH1520) += pwm_th1520.o
obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.o
obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o
obj-$(CONFIG_PWM_TWL) += pwm-twl.o
diff --git a/drivers/pwm/pwm_th1520.rs b/drivers/pwm/pwm_th1520.rs
new file mode 100644
index 0000000000000000000000000000000000000000..0ad38b78be854ab3c10268fb20763d9962f59c0f
--- /dev/null
+++ b/drivers/pwm/pwm_th1520.rs
@@ -0,0 +1,378 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2025 Samsung Electronics Co., Ltd.
+// Author: Michal Wilczynski <m.wilczynski@samsung.com>
+
+//! Rust T-HEAD TH1520 PWM driver
+//!
+//! Limitations:
+//! - The period and duty cycle are controlled by 32-bit hardware registers,
+//! limiting the maximum resolution.
+//! - The driver supports continuous output mode only; one-shot mode is not
+//! implemented.
+//! - The controller hardware provides up to 6 PWM channels.
+//! - Reconfiguration is glitch free - new period and duty cycle values are
+//! latched and take effect at the start of the next period.
+//! - Polarity is handled via a simple hardware inversion bit; arbitrary
+//! duty cycle offsets are not supported.
+//! - Disabling a channel is achieved by configuring its duty cycle to zero to
+//! produce a static low output. Clearing the `start` does not reliably
+//! force the static inactive level defined by the `INACTOUT` bit. Hence
+//! this method is not used in this driver.
+//!
+
+use core::ops::Deref;
+use kernel::{
+ c_str,
+ clk::Clk,
+ device::{Bound, Core, Device},
+ devres,
+ io::mem::IoMem,
+ of, platform,
+ prelude::*,
+ pwm, time,
+};
+
+const TH1520_MAX_PWM_NUM: u32 = 6;
+
+// Register offsets
+const fn th1520_pwm_chn_base(n: u32) -> usize {
+ (n * 0x20) as usize
+}
+
+const fn th1520_pwm_ctrl(n: u32) -> usize {
+ th1520_pwm_chn_base(n)
+}
+
+const fn th1520_pwm_per(n: u32) -> usize {
+ th1520_pwm_chn_base(n) + 0x08
+}
+
+const fn th1520_pwm_fp(n: u32) -> usize {
+ th1520_pwm_chn_base(n) + 0x0c
+}
+
+// Control register bits
+const TH1520_PWM_START: u32 = 1 << 0;
+const TH1520_PWM_CFG_UPDATE: u32 = 1 << 2;
+const TH1520_PWM_CONTINUOUS_MODE: u32 = 1 << 5;
+const TH1520_PWM_FPOUT: u32 = 1 << 8;
+
+const TH1520_PWM_REG_SIZE: usize = 0xB0;
+
+fn ns_to_cycles(ns: u64, rate_hz: u64) -> u64 {
+ const NSEC_PER_SEC_U64: u64 = time::NSEC_PER_SEC as u64;
+
+ (match ns.checked_mul(rate_hz) {
+ Some(product) => product,
+ None => u64::MAX,
+ }) / NSEC_PER_SEC_U64
+}
+
+fn cycles_to_ns(cycles: u64, rate_hz: u64) -> u64 {
+ const NSEC_PER_SEC_U64: u64 = time::NSEC_PER_SEC as u64;
+
+ // TODO: Replace with a kernel helper like `mul_u64_u64_div_u64_roundup`
+ // once available in Rust.
+ let numerator = cycles
+ .saturating_mul(NSEC_PER_SEC_U64)
+ .saturating_add(rate_hz - 1);
+
+ numerator / rate_hz
+}
+
+/// Hardware-specific waveform representation for TH1520.
+#[derive(Copy, Clone, Debug, Default)]
+struct Th1520WfHw {
+ period_cycles: u32,
+ duty_cycles: u32,
+ ctrl_val: u32,
+ enabled: bool,
+}
+
+/// The driver's private data struct. It holds all necessary devres managed resources.
+#[pin_data(PinnedDrop)]
+struct Th1520PwmDriverData {
+ #[pin]
+ iomem: devres::Devres<IoMem<TH1520_PWM_REG_SIZE>>,
+ clk: Clk,
+}
+
+// This `unsafe` implementation is a temporary necessity because the underlying `kernel::clk::Clk`
+// type does not yet expose `Send` and `Sync` implementations. This block should be removed
+// as soon as the clock abstraction provides these guarantees directly.
+// TODO: Remove those unsafe impl's when Clk will support them itself.
+
+// SAFETY: The `devres` framework requires the driver's private data to be `Send` and `Sync`.
+// We can guarantee this because the PWM core synchronizes all callbacks, preventing concurrent
+// access to the contained `iomem` and `clk` resources.
+unsafe impl Send for Th1520PwmDriverData {}
+
+// SAFETY: The same reasoning applies as for `Send`. The PWM core's synchronization
+// guarantees that it is safe for multiple threads to have shared access (`&self`)
+// to the driver data during callbacks.
+unsafe impl Sync for Th1520PwmDriverData {}
+
+impl pwm::PwmOps for Th1520PwmDriverData {
+ type WfHw = Th1520WfHw;
+
+ fn round_waveform_tohw(
+ chip: &pwm::Chip<Self>,
+ _pwm: &pwm::Device,
+ wf: &pwm::Waveform,
+ ) -> Result<pwm::RoundedWaveform<Self::WfHw>> {
+ let data = chip.drvdata();
+ let mut status = 0;
+
+ if wf.period_length_ns == 0 {
+ dev_dbg!(chip.device(), "Requested period is 0, disabling PWM.\n");
+
+ return Ok(pwm::RoundedWaveform {
+ status: 0,
+ hardware_waveform: Th1520WfHw {
+ enabled: false,
+ ..Default::default()
+ },
+ });
+ }
+
+ let rate_hz = data.clk.rate().as_hz() as u64;
+
+ let mut period_cycles = ns_to_cycles(wf.period_length_ns, rate_hz).min(u64::from(u32::MAX));
+
+ if period_cycles == 0 {
+ dev_dbg!(
+ chip.device(),
+ "Requested period {} ns is too small for clock rate {} Hz, rounding up.\n",
+ wf.period_length_ns,
+ rate_hz
+ );
+
+ period_cycles = 1;
+ status = 1;
+ }
+
+ let mut duty_cycles = ns_to_cycles(wf.duty_length_ns, rate_hz).min(u64::from(u32::MAX));
+
+ let mut ctrl_val = TH1520_PWM_CONTINUOUS_MODE;
+
+ let is_inversed = wf.duty_length_ns > 0
+ && wf.duty_offset_ns > 0
+ && wf.duty_offset_ns >= wf.period_length_ns.saturating_sub(wf.duty_length_ns);
+ if is_inversed {
+ duty_cycles = period_cycles - duty_cycles;
+ } else {
+ ctrl_val |= TH1520_PWM_FPOUT;
+ }
+
+ let wfhw = Th1520WfHw {
+ // The cast is safe because the value was clamped with `.min(u64::from(u32::MAX))`.
+ period_cycles: period_cycles as u32,
+ duty_cycles: duty_cycles as u32,
+ ctrl_val,
+ enabled: true,
+ };
+
+ dev_dbg!(
+ chip.device(),
+ "Requested: {}/{} ns [+{} ns] -> HW: {}/{} cycles, ctrl 0x{:x}, rate {} Hz\n",
+ wf.duty_length_ns,
+ wf.period_length_ns,
+ wf.duty_offset_ns,
+ wfhw.duty_cycles,
+ wfhw.period_cycles,
+ wfhw.ctrl_val,
+ rate_hz
+ );
+
+ Ok(pwm::RoundedWaveform {
+ status: status,
+ hardware_waveform: wfhw,
+ })
+ }
+
+ fn round_waveform_fromhw(
+ chip: &pwm::Chip<Self>,
+ _pwm: &pwm::Device,
+ wfhw: &Self::WfHw,
+ wf: &mut pwm::Waveform,
+ ) -> Result {
+ let data = chip.drvdata();
+ let rate_hz = data.clk.rate().as_hz() as u64;
+
+ if wfhw.period_cycles == 0 {
+ dev_dbg!(chip.device(), "HW state has zero period, reporting as disabled.\n");
+ *wf = pwm::Waveform::default();
+ return Ok(());
+ }
+
+ wf.period_length_ns = cycles_to_ns(u64::from(wfhw.period_cycles), rate_hz);
+
+ let duty_cycles = u64::from(wfhw.duty_cycles);
+
+ if (wfhw.ctrl_val & TH1520_PWM_FPOUT) != 0 {
+ wf.duty_length_ns = cycles_to_ns(duty_cycles, rate_hz);
+ wf.duty_offset_ns = 0;
+ } else {
+ let period_cycles = u64::from(wfhw.period_cycles);
+ let original_duty_cycles = period_cycles.saturating_sub(duty_cycles);
+
+ // For an inverted signal, `duty_length_ns` is the high time (period - low_time).
+ wf.duty_length_ns = cycles_to_ns(original_duty_cycles, rate_hz);
+ // The offset is the initial low time, which is what the hardware register provides.
+ wf.duty_offset_ns = cycles_to_ns(duty_cycles, rate_hz);
+ }
+
+ Ok(())
+ }
+
+ fn read_waveform(
+ chip: &pwm::Chip<Self>,
+ pwm: &pwm::Device,
+ parent_dev: &Device<Bound>,
+ ) -> Result<Self::WfHw> {
+ let data = chip.drvdata();
+ let hwpwm = pwm.hwpwm();
+ let iomem_accessor = data.iomem.access(parent_dev)?;
+ let iomap = iomem_accessor.deref();
+
+ let ctrl = iomap.try_read32(th1520_pwm_ctrl(hwpwm))?;
+ let period_cycles = iomap.try_read32(th1520_pwm_per(hwpwm))?;
+ let duty_cycles = iomap.try_read32(th1520_pwm_fp(hwpwm))?;
+
+ let wfhw = Th1520WfHw {
+ period_cycles,
+ duty_cycles,
+ ctrl_val: ctrl,
+ enabled: duty_cycles != 0,
+ };
+
+ dev_dbg!(
+ chip.device(),
+ "PWM-{}: read_waveform: Read hw state - period: {}, duty: {}, ctrl: 0x{:x}, enabled: {}",
+ hwpwm,
+ wfhw.period_cycles,
+ wfhw.duty_cycles,
+ wfhw.ctrl_val,
+ wfhw.enabled
+ );
+
+ Ok(wfhw)
+ }
+
+ fn write_waveform(
+ chip: &pwm::Chip<Self>,
+ pwm: &pwm::Device,
+ wfhw: &Self::WfHw,
+ parent_dev: &Device<Bound>,
+ ) -> Result {
+ let data = chip.drvdata();
+ let hwpwm = pwm.hwpwm();
+ let iomem_accessor = data.iomem.access(parent_dev)?;
+ let iomap = iomem_accessor.deref();
+ let duty_cycles = iomap.try_read32(th1520_pwm_fp(hwpwm))?;
+ let was_enabled = duty_cycles != 0;
+
+ if !wfhw.enabled {
+ dev_dbg!(chip.device(), "PWM-{}: Disabling channel.\n", hwpwm);
+ if was_enabled {
+ iomap.try_write32(wfhw.ctrl_val, th1520_pwm_ctrl(hwpwm))?;
+ iomap.try_write32(0, th1520_pwm_fp(hwpwm))?;
+ iomap.try_write32(wfhw.ctrl_val | TH1520_PWM_CFG_UPDATE, th1520_pwm_ctrl(hwpwm))?;
+ }
+ return Ok(());
+ }
+
+ iomap.try_write32(wfhw.ctrl_val, th1520_pwm_ctrl(hwpwm))?;
+ iomap.try_write32(wfhw.period_cycles, th1520_pwm_per(hwpwm))?;
+ iomap.try_write32(wfhw.duty_cycles, th1520_pwm_fp(hwpwm))?;
+ iomap.try_write32(wfhw.ctrl_val | TH1520_PWM_CFG_UPDATE, th1520_pwm_ctrl(hwpwm))?;
+
+ // The `TH1520_PWM_START` bit must be written in a separate, final transaction, and
+ // only when enabling the channel from a disabled state.
+ if !was_enabled {
+ iomap.try_write32(wfhw.ctrl_val | TH1520_PWM_START, th1520_pwm_ctrl(hwpwm))?;
+ }
+
+ dev_dbg!(
+ chip.device(),
+ "PWM-{}: Wrote {}/{} cycles",
+ hwpwm,
+ wfhw.duty_cycles,
+ wfhw.period_cycles,
+ );
+
+ Ok(())
+ }
+}
+
+#[pinned_drop]
+impl PinnedDrop for Th1520PwmDriverData {
+ fn drop(self: Pin<&mut Self>) {
+ self.clk.disable_unprepare();
+ }
+}
+
+struct Th1520PwmPlatformDriver;
+
+kernel::of_device_table!(
+ OF_TABLE,
+ MODULE_OF_TABLE,
+ <Th1520PwmPlatformDriver as platform::Driver>::IdInfo,
+ [(of::DeviceId::new(c_str!("thead,th1520-pwm")), ())]
+);
+
+impl platform::Driver for Th1520PwmPlatformDriver {
+ type IdInfo = ();
+ const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
+
+ fn probe(
+ pdev: &platform::Device<Core>,
+ _id_info: Option<&Self::IdInfo>,
+ ) -> Result<Pin<KBox<Self>>> {
+ let dev = pdev.as_ref();
+ let request = pdev.io_request_by_index(0).ok_or(ENODEV)?;
+
+ let clk = Clk::get(dev, None)?;
+
+ clk.prepare_enable()?;
+
+ // TODO: Get exclusive ownership of the clock to prevent rate changes.
+ // The Rust equivalent of `clk_rate_exclusive_get()` is not yet available.
+ // This should be updated once it is implemented.
+ let rate_hz = clk.rate().as_hz();
+ if rate_hz == 0 {
+ dev_err!(dev, "Clock rate is zero\n");
+ return Err(EINVAL);
+ }
+
+ if rate_hz > time::NSEC_PER_SEC as usize {
+ dev_err!(
+ dev,
+ "Clock rate {} Hz is too high, not supported.\n",
+ rate_hz
+ );
+ return Err(EINVAL);
+ }
+
+ let chip = pwm::Chip::new(
+ dev,
+ TH1520_MAX_PWM_NUM,
+ try_pin_init!(Th1520PwmDriverData {
+ iomem <- request.iomap_sized::<TH1520_PWM_REG_SIZE>(),
+ clk <- clk,
+ }),
+ )?;
+
+ pwm::Registration::register(dev, chip)?;
+
+ Ok(KBox::new(Th1520PwmPlatformDriver, GFP_KERNEL)?.into())
+ }
+}
+
+kernel::module_platform_driver! {
+ type: Th1520PwmPlatformDriver,
+ name: "pwm-th1520",
+ authors: ["Michal Wilczynski <m.wilczynski@samsung.com>"],
+ description: "T-HEAD TH1520 PWM driver",
+ license: "GPL v2",
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v16 5/7] dt-bindings: pwm: thead: Add T-HEAD TH1520 PWM controller
2025-10-16 13:38 ` [PATCH v16 0/7] Rust Abstractions for PWM subsystem with TH1520 PWM driver Michal Wilczynski
` (3 preceding siblings ...)
2025-10-16 13:38 ` [PATCH v16 4/7] pwm: Add Rust driver for T-HEAD TH1520 SoC Michal Wilczynski
@ 2025-10-16 13:38 ` Michal Wilczynski
2025-10-16 13:38 ` [PATCH v16 6/7] riscv: dts: thead: Add PWM controller node Michal Wilczynski
` (3 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Michal Wilczynski @ 2025-10-16 13:38 UTC (permalink / raw)
To: Uwe Kleine-König, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Michal Wilczynski, Guo Ren,
Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Marek Szyprowski, Benno Lossin, Michael Turquette, Drew Fustini,
Daniel Almeida, Benno Lossin, Drew Fustini
Cc: linux-kernel, linux-pwm, rust-for-linux, linux-riscv, devicetree,
Krzysztof Kozlowski, Elle Rhumsaa
Add the Device Tree binding documentation for the T-HEAD
TH1520 SoC PWM controller.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Acked-by: Drew Fustini <fustini@kernel.org>
Reviewed-by: Elle Rhumsaa <elle@weathered-steel.dev>
Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
.../devicetree/bindings/pwm/thead,th1520-pwm.yaml | 48 ++++++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 49 insertions(+)
diff --git a/Documentation/devicetree/bindings/pwm/thead,th1520-pwm.yaml b/Documentation/devicetree/bindings/pwm/thead,th1520-pwm.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..855aec59ac53c430adc849271235686e87b10e6c
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/thead,th1520-pwm.yaml
@@ -0,0 +1,48 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/thead,th1520-pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: T-HEAD TH1520 PWM controller
+
+maintainers:
+ - Michal Wilczynski <m.wilczynski@samsung.com>
+
+allOf:
+ - $ref: pwm.yaml#
+
+properties:
+ compatible:
+ const: thead,th1520-pwm
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ items:
+ - description: SoC PWM clock
+
+ "#pwm-cells":
+ const: 3
+
+required:
+ - compatible
+ - reg
+ - clocks
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/thead,th1520-clk-ap.h>
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ pwm@ffec01c000 {
+ compatible = "thead,th1520-pwm";
+ reg = <0xff 0xec01c000 0x0 0x4000>;
+ clocks = <&clk CLK_PWM>;
+ #pwm-cells = <3>;
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index b4ca6192091bd31573c391b9b5dc29b74c9a684c..6aa7a2588c22301ff5ab6a3d6849173534de7fb4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22179,6 +22179,7 @@ F: Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml
F: Documentation/devicetree/bindings/mailbox/thead,th1520-mbox.yaml
F: Documentation/devicetree/bindings/net/thead,th1520-gmac.yaml
F: Documentation/devicetree/bindings/pinctrl/thead,th1520-pinctrl.yaml
+F: Documentation/devicetree/bindings/pwm/thead,th1520-pwm.yaml
F: Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
F: arch/riscv/boot/dts/thead/
F: drivers/clk/thead/clk-th1520-ap.c
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v16 6/7] riscv: dts: thead: Add PWM controller node
2025-10-16 13:38 ` [PATCH v16 0/7] Rust Abstractions for PWM subsystem with TH1520 PWM driver Michal Wilczynski
` (4 preceding siblings ...)
2025-10-16 13:38 ` [PATCH v16 5/7] dt-bindings: pwm: thead: Add T-HEAD TH1520 PWM controller Michal Wilczynski
@ 2025-10-16 13:38 ` Michal Wilczynski
2025-10-28 10:08 ` Drew Fustini
2025-10-16 13:38 ` [PATCH v16 7/7] riscv: dts: thead: Add PWM fan and thermal control Michal Wilczynski
` (2 subsequent siblings)
8 siblings, 1 reply; 15+ messages in thread
From: Michal Wilczynski @ 2025-10-16 13:38 UTC (permalink / raw)
To: Uwe Kleine-König, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Michal Wilczynski, Guo Ren,
Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Marek Szyprowski, Benno Lossin, Michael Turquette, Drew Fustini,
Daniel Almeida, Benno Lossin, Drew Fustini
Cc: linux-kernel, linux-pwm, rust-for-linux, linux-riscv, devicetree,
Elle Rhumsaa
Add the Device Tree node for the T-HEAD TH1520 SoC's PWM controller.
Reviewed-by: Drew Fustini <fustini@kernel.org>
Tested-by: Drew Fustini <fustini@kernel.org>
Reviewed-by: Elle Rhumsaa <elle@weathered-steel.dev>
Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
arch/riscv/boot/dts/thead/th1520.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
index e680d1a7c821f381d116748ed063fe2649aedaca..9ed0465cb527ccbc5b2b1ae3d9024091c6859754 100644
--- a/arch/riscv/boot/dts/thead/th1520.dtsi
+++ b/arch/riscv/boot/dts/thead/th1520.dtsi
@@ -502,6 +502,13 @@ uart2: serial@ffec010000 {
status = "disabled";
};
+ pwm: pwm@ffec01c000 {
+ compatible = "thead,th1520-pwm";
+ reg = <0xff 0xec01c000 0x0 0x4000>;
+ clocks = <&clk CLK_PWM>;
+ #pwm-cells = <3>;
+ };
+
clk: clock-controller@ffef010000 {
compatible = "thead,th1520-clk-ap";
reg = <0xff 0xef010000 0x0 0x1000>;
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v16 6/7] riscv: dts: thead: Add PWM controller node
2025-10-16 13:38 ` [PATCH v16 6/7] riscv: dts: thead: Add PWM controller node Michal Wilczynski
@ 2025-10-28 10:08 ` Drew Fustini
0 siblings, 0 replies; 15+ messages in thread
From: Drew Fustini @ 2025-10-28 10:08 UTC (permalink / raw)
To: Michal Wilczynski
Cc: Uwe Kleine-König, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Guo Ren, Fu Wei, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, Marek Szyprowski, Benno Lossin,
Michael Turquette, Daniel Almeida, linux-kernel, linux-pwm,
rust-for-linux, linux-riscv, devicetree, Elle Rhumsaa
On Thu, Oct 16, 2025 at 03:38:06PM +0200, Michal Wilczynski wrote:
> Add the Device Tree node for the T-HEAD TH1520 SoC's PWM controller.
>
> Reviewed-by: Drew Fustini <fustini@kernel.org>
> Tested-by: Drew Fustini <fustini@kernel.org>
> Reviewed-by: Elle Rhumsaa <elle@weathered-steel.dev>
> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> ---
> arch/riscv/boot/dts/thead/th1520.dtsi | 7 +++++++
> 1 file changed, 7 insertions(+)
Applied to thead-dt-for-next, thanks!
-Drew
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v16 7/7] riscv: dts: thead: Add PWM fan and thermal control
2025-10-16 13:38 ` [PATCH v16 0/7] Rust Abstractions for PWM subsystem with TH1520 PWM driver Michal Wilczynski
` (5 preceding siblings ...)
2025-10-16 13:38 ` [PATCH v16 6/7] riscv: dts: thead: Add PWM controller node Michal Wilczynski
@ 2025-10-16 13:38 ` Michal Wilczynski
2025-10-28 10:09 ` Drew Fustini
2025-10-22 8:34 ` [PATCH v16 0/7] Rust Abstractions for PWM subsystem with TH1520 PWM driver Michal Wilczynski
2025-10-25 12:26 ` Uwe Kleine-König
8 siblings, 1 reply; 15+ messages in thread
From: Michal Wilczynski @ 2025-10-16 13:38 UTC (permalink / raw)
To: Uwe Kleine-König, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Michal Wilczynski, Guo Ren,
Fu Wei, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Marek Szyprowski, Benno Lossin, Michael Turquette, Drew Fustini,
Daniel Almeida, Benno Lossin, Drew Fustini
Cc: linux-kernel, linux-pwm, rust-for-linux, linux-riscv, devicetree,
Elle Rhumsaa
Add Device Tree nodes to enable a PWM controlled fan and it's associated
thermal management for the Lichee Pi 4A board.
This enables temperature-controlled active cooling for the Lichee Pi 4A
board based on SoC temperature.
Reviewed-by: Drew Fustini <fustini@kernel.org>
Tested-by: Drew Fustini <fustini@kernel.org>
Reviewed-by: Elle Rhumsaa <elle@weathered-steel.dev>
Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
arch/riscv/boot/dts/thead/th1520-lichee-pi-4a.dts | 67 +++++++++++++++++++++++
1 file changed, 67 insertions(+)
diff --git a/arch/riscv/boot/dts/thead/th1520-lichee-pi-4a.dts b/arch/riscv/boot/dts/thead/th1520-lichee-pi-4a.dts
index 4020c727f09e8e2286fdc7fecd79dbd8eba69556..c58c2085ca92a3234f1350500cedae4157f0c35f 100644
--- a/arch/riscv/boot/dts/thead/th1520-lichee-pi-4a.dts
+++ b/arch/riscv/boot/dts/thead/th1520-lichee-pi-4a.dts
@@ -28,9 +28,76 @@ aliases {
chosen {
stdout-path = "serial0:115200n8";
};
+
+ thermal-zones {
+ cpu-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <1000>;
+ thermal-sensors = <&pvt 0>;
+
+ trips {
+ fan_config0: fan-trip0 {
+ temperature = <39000>;
+ hysteresis = <5000>;
+ type = "active";
+ };
+
+ fan_config1: fan-trip1 {
+ temperature = <50000>;
+ hysteresis = <5000>;
+ type = "active";
+ };
+
+ fan_config2: fan-trip2 {
+ temperature = <60000>;
+ hysteresis = <5000>;
+ type = "active";
+ };
+ };
+
+ cooling-maps {
+ map-active-0 {
+ cooling-device = <&fan 1 1>;
+ trip = <&fan_config0>;
+ };
+
+ map-active-1 {
+ cooling-device = <&fan 2 2>;
+ trip = <&fan_config1>;
+ };
+
+ map-active-2 {
+ cooling-device = <&fan 3 3>;
+ trip = <&fan_config2>;
+ };
+ };
+ };
+ };
+
+ fan: pwm-fan {
+ pinctrl-names = "default";
+ pinctrl-0 = <&fan_pins>;
+ compatible = "pwm-fan";
+ #cooling-cells = <2>;
+ pwms = <&pwm 1 10000000 0>;
+ cooling-levels = <0 66 196 255>;
+ };
+
};
&padctrl0_apsys {
+ fan_pins: fan-0 {
+ pwm1-pins {
+ pins = "GPIO3_3"; /* PWM1 */
+ function = "pwm";
+ bias-disable;
+ drive-strength = <25>;
+ input-disable;
+ input-schmitt-disable;
+ slew-rate = <0>;
+ };
+ };
+
uart0_pins: uart0-0 {
tx-pins {
pins = "UART0_TXD";
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v16 7/7] riscv: dts: thead: Add PWM fan and thermal control
2025-10-16 13:38 ` [PATCH v16 7/7] riscv: dts: thead: Add PWM fan and thermal control Michal Wilczynski
@ 2025-10-28 10:09 ` Drew Fustini
0 siblings, 0 replies; 15+ messages in thread
From: Drew Fustini @ 2025-10-28 10:09 UTC (permalink / raw)
To: Michal Wilczynski
Cc: Uwe Kleine-König, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Guo Ren, Fu Wei, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, Marek Szyprowski, Benno Lossin,
Michael Turquette, Daniel Almeida, linux-kernel, linux-pwm,
rust-for-linux, linux-riscv, devicetree, Elle Rhumsaa
On Thu, Oct 16, 2025 at 03:38:07PM +0200, Michal Wilczynski wrote:
> Add Device Tree nodes to enable a PWM controlled fan and it's associated
> thermal management for the Lichee Pi 4A board.
>
> This enables temperature-controlled active cooling for the Lichee Pi 4A
> board based on SoC temperature.
>
> Reviewed-by: Drew Fustini <fustini@kernel.org>
> Tested-by: Drew Fustini <fustini@kernel.org>
> Reviewed-by: Elle Rhumsaa <elle@weathered-steel.dev>
> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> ---
> arch/riscv/boot/dts/thead/th1520-lichee-pi-4a.dts | 67 +++++++++++++++++++++++
> 1 file changed, 67 insertions(+)
Applied to thead-dt-for-next, thanks!
-Drew
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v16 0/7] Rust Abstractions for PWM subsystem with TH1520 PWM driver
2025-10-16 13:38 ` [PATCH v16 0/7] Rust Abstractions for PWM subsystem with TH1520 PWM driver Michal Wilczynski
` (6 preceding siblings ...)
2025-10-16 13:38 ` [PATCH v16 7/7] riscv: dts: thead: Add PWM fan and thermal control Michal Wilczynski
@ 2025-10-22 8:34 ` Michal Wilczynski
2025-10-24 14:09 ` Uwe Kleine-König
2025-10-25 12:26 ` Uwe Kleine-König
8 siblings, 1 reply; 15+ messages in thread
From: Michal Wilczynski @ 2025-10-22 8:34 UTC (permalink / raw)
To: Uwe Kleine-König, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Guo Ren, Fu Wei, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, Marek Szyprowski, Benno Lossin,
Michael Turquette, Drew Fustini, Daniel Almeida
Cc: linux-kernel, linux-pwm, rust-for-linux, linux-riscv, devicetree,
Elle Rhumsaa, Krzysztof Kozlowski
On 10/16/25 15:38, Michal Wilczynski wrote:
> This patch series introduces Rust support for the T-HEAD TH1520 PWM
> controller and demonstrates its use for fan control on the Sipeed Lichee
> Pi 4A board.
>
> The primary goal of this patch series is to introduce a basic set of
> Rust abstractions for the Linux PWM subsystem. As a first user and
> practical demonstration of these abstractions, the series also provides
> a functional PWM driver for the T-HEAD TH1520 SoC. This allows control
> of its PWM channels and ultimately enables temperature controlled fan
> support for the Lichee Pi 4A board. This work aims to explore the use of
> Rust for PWM drivers and lay a foundation for potential future Rust
> based PWM drivers.
>
> The core of this series is a new rust/kernel/pwm.rs module that provides
> abstractions for writing PWM chip provider drivers in Rust. This has
> been significantly reworked from v1 based on extensive feedback. The key
> features of the new abstraction layer include:
>
> - Ownership and Lifetime Management: The pwm::Chip wrapper is managed
> by ARef, correctly tying its lifetime to its embedded struct device
> reference counter. Chip registration is handled by a pwm::Registration
> RAII guard, which guarantees that pwmchip_add is always paired with
> pwmchip_remove, preventing resource leaks.
>
> - Modern and Safe API: The PwmOps trait is now based on the modern
> waveform API (round_waveform_tohw, write_waveform, etc.) as recommended
> by the subsystem maintainer. It is generic over a driver's
> hardware specific data structure, moving all unsafe serialization logic
> into the abstraction layer and allowing drivers to be written in 100%
> safe Rust.
>
> - Ergonomics: The API provides safe, idiomatic wrappers for other PWM
> types (State, Device, etc.) and uses standard kernel error
> handling patterns.
>
> The series is structured as follows:
> - Expose static function pwmchip_release.
> - Rust PWM Abstractions: The new safe abstraction layer.
> - TH1520 PWM Driver: A new Rust driver for the TH1520 SoC, built on
> top of the new abstractions.
> - Device Tree Bindings & Nodes: The remaining patches add the necessary
> DT bindings and nodes for the TH1520 PWM controller, and the PWM fan
> configuration for the Lichee Pi 4A board.
>
> Testing:
> Tested on the TH1520 SoC. The fan works correctly. The duty/period
> calculations are correct. Fan starts slow when the chip is not hot and
> gradually increases the speed when PVT reports higher temperatures.
>
> The patches doesn't contain any dependencies that are not currently in
> the mainline kernel anymore.
>
> ---
> Changes in v16:
> - Re-base on top of 6.18-rc1.
> - Make RUST_PWM_ABSTRACTIONS an invisible Kconfig option and remove the
> redundant depends on PWM=y.
> - Handle period requests that are too small by rounding up to 1 cycle,
> rather than disabling the PWM.
> - Correctly report a status of 1 to indicate when the period has been
> rounded up.
> - Change the error code for an unsupported high clock rate from ERANGE
> to EINVAL for consistency.
> - Link to v15: https://lore.kernel.org/r/20250930-rust-next-pwm-working-fan-for-sending-v15-0-5661c3090877@samsung.com
>
Hi Uwe,
Since you mentioned last time that you were happy with the code, would
you please consider picking up this series for linux-next? It would be
great to get it in for wider testing to ensure everything is solid.
On a related note, it looks like Clk is getting Send and Sync traits
[1], which is excellent news! This will allow the TH1520 PWM driver to
be 100% safe Rust :-).
I recall you prefer to base your pull requests on -rc1. With that in
mind, I plan to send a follow up patch to remove the unsafe block for
the Clk handling after the next merge window closes.
[1] - https://lore.kernel.org/all/20251020-clk-send-sync-v2-0-44ab533ae084@google.com/T/#mdfdfa9947b4d51b9ebc6d667911bf19907761655
Best regards,
--
Michal Wilczynski <m.wilczynski@samsung.com>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v16 0/7] Rust Abstractions for PWM subsystem with TH1520 PWM driver
2025-10-22 8:34 ` [PATCH v16 0/7] Rust Abstractions for PWM subsystem with TH1520 PWM driver Michal Wilczynski
@ 2025-10-24 14:09 ` Uwe Kleine-König
2025-10-24 16:17 ` Michal Wilczynski
0 siblings, 1 reply; 15+ messages in thread
From: Uwe Kleine-König @ 2025-10-24 14:09 UTC (permalink / raw)
To: Michal Wilczynski
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Guo Ren, Fu Wei, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, Marek Szyprowski, Benno Lossin,
Michael Turquette, Drew Fustini, Daniel Almeida, linux-kernel,
linux-pwm, rust-for-linux, linux-riscv, devicetree, Elle Rhumsaa,
Krzysztof Kozlowski
[-- Attachment #1: Type: text/plain, Size: 3459 bytes --]
Hello Michael,
On Wed, Oct 22, 2025 at 10:34:42AM +0200, Michal Wilczynski wrote:
> Since you mentioned last time that you were happy with the code, would
> you please consider picking up this series for linux-next? It would be
> great to get it in for wider testing to ensure everything is solid.
I took another look, and just being able to compile and understanding
very little Rust, I came up with:
diff --git a/rust/kernel/pwm.rs b/rust/kernel/pwm.rs
index 79fbb13cd47f..c8f9f5b61bfa 100644
--- a/rust/kernel/pwm.rs
+++ b/rust/kernel/pwm.rs
@@ -15,38 +15,7 @@
prelude::*,
types::{ARef, AlwaysRefCounted, Opaque},
};
-use core::{convert::TryFrom, marker::PhantomData, ptr::NonNull};
+use core::{marker::PhantomData, ptr::NonNull};
-
-/// PWM polarity. Mirrors [`enum pwm_polarity`](srctree/include/linux/pwm.h).
-#[derive(Copy, Clone, Debug, PartialEq, Eq)]
-pub enum Polarity {
- /// Normal polarity (duty cycle defines the high period of the signal).
- Normal,
-
- /// Inversed polarity (duty cycle defines the low period of the signal).
- Inversed,
-}
-
-impl TryFrom<bindings::pwm_polarity> for Polarity {
- type Error = Error;
-
- fn try_from(polarity: bindings::pwm_polarity) -> Result<Self, Error> {
- match polarity {
- bindings::pwm_polarity_PWM_POLARITY_NORMAL => Ok(Polarity::Normal),
- bindings::pwm_polarity_PWM_POLARITY_INVERSED => Ok(Polarity::Inversed),
- _ => Err(EINVAL),
- }
- }
-}
-
-impl From<Polarity> for bindings::pwm_polarity {
- fn from(polarity: Polarity) -> Self {
- match polarity {
- Polarity::Normal => bindings::pwm_polarity_PWM_POLARITY_NORMAL,
- Polarity::Inversed => bindings::pwm_polarity_PWM_POLARITY_INVERSED,
- }
- }
-}
/// Represents a PWM waveform configuration.
/// Mirrors struct [`struct pwm_waveform`](srctree/include/linux/pwm.h).
@@ -89,22 +58,6 @@ fn from(wf: Waveform) -> Self {
}
}
-/// Wrapper for PWM state [`struct pwm_state`](srctree/include/linux/pwm.h).
-#[repr(transparent)]
-pub struct State(bindings::pwm_state);
-
-impl State {
- /// Creates a `State` wrapper by taking ownership of a C `pwm_state` value.
- pub(crate) fn from_c(c_state: bindings::pwm_state) -> Self {
- State(c_state)
- }
-
- /// Returns `true` if the PWM signal is enabled.
- pub fn enabled(&self) -> bool {
- self.0.enabled
- }
-}
-
/// Describes the outcome of a `round_waveform` operation.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum RoundingOutcome {
@@ -164,13 +117,6 @@ pub fn label(&self) -> Option<&CStr> {
Some(unsafe { CStr::from_char_ptr(label_ptr) })
}
- /// Gets a copy of the current state of this PWM device.
- pub fn state(&self) -> State {
- // SAFETY: `self.as_raw()` gives a valid pointer. `(*self.as_raw()).state`
- // is a valid `pwm_state` struct. `State::from_c` copies this data.
- State::from_c(unsafe { (*self.as_raw()).state })
- }
-
/// Sets the PWM waveform configuration and enables the PWM signal.
pub fn set_waveform(&self, wf: &Waveform, exact: bool) -> Result {
let c_wf = bindings::pwm_waveform::from(*wf);
Does that make sense?
I consider applying your series and put the above space on top, to
create my first Rust patch :-)
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v16 0/7] Rust Abstractions for PWM subsystem with TH1520 PWM driver
2025-10-24 14:09 ` Uwe Kleine-König
@ 2025-10-24 16:17 ` Michal Wilczynski
0 siblings, 0 replies; 15+ messages in thread
From: Michal Wilczynski @ 2025-10-24 16:17 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Guo Ren, Fu Wei, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, Marek Szyprowski, Benno Lossin,
Michael Turquette, Drew Fustini, Daniel Almeida, linux-kernel,
linux-pwm, rust-for-linux, linux-riscv, devicetree, Elle Rhumsaa,
Krzysztof Kozlowski
On 10/24/25 16:09, Uwe Kleine-König wrote:
> Hello Michael,
>
> On Wed, Oct 22, 2025 at 10:34:42AM +0200, Michal Wilczynski wrote:
>> Since you mentioned last time that you were happy with the code, would
>> you please consider picking up this series for linux-next? It would be
>> great to get it in for wider testing to ensure everything is solid.
>
> I took another look, and just being able to compile and understanding
> very little Rust, I came up with:
>
> diff --git a/rust/kernel/pwm.rs b/rust/kernel/pwm.rs
> index 79fbb13cd47f..c8f9f5b61bfa 100644
> --- a/rust/kernel/pwm.rs
> +++ b/rust/kernel/pwm.rs
> @@ -15,38 +15,7 @@
> prelude::*,
> types::{ARef, AlwaysRefCounted, Opaque},
> };
> -use core::{convert::TryFrom, marker::PhantomData, ptr::NonNull};
> +use core::{marker::PhantomData, ptr::NonNull};
> -
> -/// PWM polarity. Mirrors [`enum pwm_polarity`](srctree/include/linux/pwm.h).
> -#[derive(Copy, Clone, Debug, PartialEq, Eq)]
> -pub enum Polarity {
> - /// Normal polarity (duty cycle defines the high period of the signal).
> - Normal,
> -
> - /// Inversed polarity (duty cycle defines the low period of the signal).
> - Inversed,
> -}
> -
> -impl TryFrom<bindings::pwm_polarity> for Polarity {
> - type Error = Error;
> -
> - fn try_from(polarity: bindings::pwm_polarity) -> Result<Self, Error> {
> - match polarity {
> - bindings::pwm_polarity_PWM_POLARITY_NORMAL => Ok(Polarity::Normal),
> - bindings::pwm_polarity_PWM_POLARITY_INVERSED => Ok(Polarity::Inversed),
> - _ => Err(EINVAL),
> - }
> - }
> -}
> -
> -impl From<Polarity> for bindings::pwm_polarity {
> - fn from(polarity: Polarity) -> Self {
> - match polarity {
> - Polarity::Normal => bindings::pwm_polarity_PWM_POLARITY_NORMAL,
> - Polarity::Inversed => bindings::pwm_polarity_PWM_POLARITY_INVERSED,
> - }
> - }
> -}
>
> /// Represents a PWM waveform configuration.
> /// Mirrors struct [`struct pwm_waveform`](srctree/include/linux/pwm.h).
> @@ -89,22 +58,6 @@ fn from(wf: Waveform) -> Self {
> }
> }
>
> -/// Wrapper for PWM state [`struct pwm_state`](srctree/include/linux/pwm.h).
> -#[repr(transparent)]
> -pub struct State(bindings::pwm_state);
> -
> -impl State {
> - /// Creates a `State` wrapper by taking ownership of a C `pwm_state` value.
> - pub(crate) fn from_c(c_state: bindings::pwm_state) -> Self {
> - State(c_state)
> - }
> -
> - /// Returns `true` if the PWM signal is enabled.
> - pub fn enabled(&self) -> bool {
> - self.0.enabled
> - }
> -}
> -
> /// Describes the outcome of a `round_waveform` operation.
> #[derive(Debug, Clone, Copy, PartialEq, Eq)]
> pub enum RoundingOutcome {
> @@ -164,13 +117,6 @@ pub fn label(&self) -> Option<&CStr> {
> Some(unsafe { CStr::from_char_ptr(label_ptr) })
> }
>
> - /// Gets a copy of the current state of this PWM device.
> - pub fn state(&self) -> State {
> - // SAFETY: `self.as_raw()` gives a valid pointer. `(*self.as_raw()).state`
> - // is a valid `pwm_state` struct. `State::from_c` copies this data.
> - State::from_c(unsafe { (*self.as_raw()).state })
> - }
> -
> /// Sets the PWM waveform configuration and enables the PWM signal.
> pub fn set_waveform(&self, wf: &Waveform, exact: bool) -> Result {
> let c_wf = bindings::pwm_waveform::from(*wf);
>
> Does that make sense?
Hi Uwe,
Yes, that makes perfect sense.
I agree with your patch. I initially thought the State wrapper might be
necessary for the TH1520 driver, but I was able to refactor it to work
without it.
On reflection, both Polarity and State are really remnants of the older,
legacy API. Removing them is a good cleanup; it ensures future Rust
drivers will use the modern waveform API and avoids potential confusion.
>
> I consider applying your series and put the above space on top, to
> create my first Rust patch :-)
Great ! Congratulations on your first Rust patch :-)
On a related note there is this diff necessary to make CLIPPY happy:
diff --git a/drivers/pwm/pwm_th1520.rs b/drivers/pwm/pwm_th1520.rs
index 0ad38b78be85..a6c3b6025152 100644
--- a/drivers/pwm/pwm_th1520.rs
+++ b/drivers/pwm/pwm_th1520.rs
@@ -185,7 +185,7 @@ fn round_waveform_tohw(
);
Ok(pwm::RoundedWaveform {
- status: status,
+ status,
hardware_waveform: wfhw,
})
}
I could re-send the patchset, but since you're making a patch anyway
maybe you would also like to pick this ?
>
> Best regards
> Uwe
Best regards,
--
Michal Wilczynski <m.wilczynski@samsung.com>
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v16 0/7] Rust Abstractions for PWM subsystem with TH1520 PWM driver
2025-10-16 13:38 ` [PATCH v16 0/7] Rust Abstractions for PWM subsystem with TH1520 PWM driver Michal Wilczynski
` (7 preceding siblings ...)
2025-10-22 8:34 ` [PATCH v16 0/7] Rust Abstractions for PWM subsystem with TH1520 PWM driver Michal Wilczynski
@ 2025-10-25 12:26 ` Uwe Kleine-König
2025-10-26 11:14 ` Drew Fustini
8 siblings, 1 reply; 15+ messages in thread
From: Uwe Kleine-König @ 2025-10-25 12:26 UTC (permalink / raw)
To: Michal Wilczynski
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Guo Ren, Fu Wei, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, Marek Szyprowski, Benno Lossin,
Michael Turquette, Drew Fustini, Daniel Almeida, linux-kernel,
linux-pwm, rust-for-linux, linux-riscv, devicetree, Elle Rhumsaa,
Krzysztof Kozlowski
[-- Attachment #1: Type: text/plain, Size: 681 bytes --]
Hello,
On Thu, Oct 16, 2025 at 03:38:00PM +0200, Michal Wilczynski wrote:
> Michal Wilczynski (7):
> pwm: Export `pwmchip_release` for external use
> rust: pwm: Add Kconfig and basic data structures
> rust: pwm: Add complete abstraction layer
> pwm: Add Rust driver for T-HEAD TH1520 SoC
> dt-bindings: pwm: thead: Add T-HEAD TH1520 PWM controller
> riscv: dts: thead: Add PWM controller node
> riscv: dts: thead: Add PWM fan and thermal control
I applied patches #1-#5 to
https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git pwm/for-next
. If I should take the riscv dts changes, too, please tell me.
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v16 0/7] Rust Abstractions for PWM subsystem with TH1520 PWM driver
2025-10-25 12:26 ` Uwe Kleine-König
@ 2025-10-26 11:14 ` Drew Fustini
0 siblings, 0 replies; 15+ messages in thread
From: Drew Fustini @ 2025-10-26 11:14 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Michal Wilczynski, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Guo Ren, Fu Wei, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, Marek Szyprowski, Benno Lossin,
Michael Turquette, Daniel Almeida, linux-kernel, linux-pwm,
rust-for-linux, linux-riscv, devicetree, Elle Rhumsaa,
Krzysztof Kozlowski
[-- Attachment #1: Type: text/plain, Size: 854 bytes --]
On Sat, Oct 25, 2025 at 02:26:31PM +0200, Uwe Kleine-König wrote:
> Hello,
>
> On Thu, Oct 16, 2025 at 03:38:00PM +0200, Michal Wilczynski wrote:
> > Michal Wilczynski (7):
> > pwm: Export `pwmchip_release` for external use
> > rust: pwm: Add Kconfig and basic data structures
> > rust: pwm: Add complete abstraction layer
> > pwm: Add Rust driver for T-HEAD TH1520 SoC
> > dt-bindings: pwm: thead: Add T-HEAD TH1520 PWM controller
> > riscv: dts: thead: Add PWM controller node
> > riscv: dts: thead: Add PWM fan and thermal control
>
> I applied patches #1-#5 to
>
> https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git pwm/for-next
>
> . If I should take the riscv dts changes, too, please tell me.
I can take them through my thead-dt-for-next branch.
Thanks,
Drew
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread