[PATCH v6 0/7] Clk improvements

[PATCH v6 0/7] Clk improvements

[Daniel Almeida]

This series contains a few improvements that simplify clock handling for
drivers.

Patch 1 converts clk.rs to the kernel-vertical style for imports.

Patch 2 replaces OptionalClk with Clk::get_optional(). There is no struct
optional_clk in C, and Clk already allows a NULL pointer.

Patch 3 implements the same typestate pattern that has been used
successfully for Regulators. This is needed because otherwise drivers
will be responsible for unpreparing and disabling clocks themselves and
ultimately handling the reference counts on their own. This is
undesirable. The patch automatically encodes this information using the
type system so that no misuse can occur.

Patch 4 adds guards that prepare and/or enable a clock for the lifetime
of the guard without consuming the Clk, so that a driver can keep a
long-lived Clk<Prepared> and enable it for scoped sections.

Patch 5 makes things more convenient by offering devres-managed APIs. This
lets drivers set clock parameters once and forget about lifetime
management.

Patch 6 adds ExclusiveClk<T>, which holds rate exclusivity over a Clk<T>
in any state until it is dropped.

Patch 7 uses it in pwm-th1520 to lock the clock rate for the lifetime of
the driver data.

---
Changes in v6:
- Rebased onto the latest clk-next.
- Moved the import style conversion to the front (Alexandre, Onur).
- Kept Alexandre's and Onur's Reviewed-by on the imports patch, and
  Alexandre's on the devres patch. Dropped Alexandre's Reviewed-by on the
  type-state patch, which changed too much since v5.
- Split the OptionalClk removal into its own patch (Gary).
- Replaced "rust: clk: implement Clone for Clk<T>" with EnabledGuard,
  PreparedGuard and PrepareEnabledGuard (Alexandre, Gary). Clones shared
  a single struct clk, so per-consumer state such as the rate
  exclusivity taken by clk_rate_exclusive_get() was shared between them.
- Dropped Clk::<Prepared>::with_enabled(), superseded by EnabledGuard.
- Named the ClkState constants ENABLED and PREPARED from the start
  (Alexandre).
- Implemented Debug for Error<State> (Alexandre).
- Documented that dropping a prepared or enabled Clk may sleep
  (Alexandre).
- Used a different connection id for the second clock in the example
  (Alexandre).
- Used `?` in the per-state get() and get_optional() shortcuts.
- Dropped the trailing `//` from doctest imports (Alexandre).
- Removed the intra-doc links from SAFETY comments (Onur) and reworded
  the devm_enable_optional() documentation (Alexandre).
- Expanded the devres commit message (Gary).
- Added ExclusiveClk and its pwm-th1520 user, based on Maurice
  Hieronymus's series [1], with ExclusiveClk made generic over the clock
  state, ExclusiveClk::new() in place of Clk::rate_exclusive_get(), and
  a new ExclusiveClk::into_inner().
- Link to v5: https://patch.msgid.link/20260706-clk-type-state-v5-0-67c5f326a16c@collabora.com

[1] https://lore.kernel.org/rust-for-linux/20260711-rate-exclusive-get-th1520-v4-0-8244a0666a71@mailbox.org/

Changes in v5:
- Rebased onto the latest clk-next.
- New patch: "rust: clk: implement Clone for Clk<T>". Each clone is an
  independent view of the same underlying clock, owning its own
  prepare/enable counts; the raw pointer is shared through an Arc so
  clk_get()/clk_put() stay balanced. Follows the v3 discussion with
  Boris Brezillon and Gary Guo.
- Moved rate() and set_rate() from Clk<Enabled> to all states; the C API
  does not restrict these to enabled clocks, and some clocks can only
  change rate before being prepared.
- Made Clk::<Enabled>::disable() infallible, matching unprepare() and
  the void C API (Onur).
- Fixed a typo in the "# Invariants" section, fixed SAFETY comments
  that referred to the non-existent `self.0` field, and added missing
  INVARIANT comments on the state transitions.
- Added the missing rustdoc link definitions for clk_unprepare and
  clk_put (Maurice).
- Dropped the redundant `Result` import and the `use
  kernel::error::Result;` lines from the doctests; the prelude already
  provides it (Miguel).
- Added #[inline] to the devm_* helpers.
- Link to v4: https://patch.msgid.link/20260618-clk-type-state-v4-0-8be082786080@collabora.com

Changes in v4:
- Rebased onto clk-next. Alice Ryhl's "rust: clk: implement Send and Sync"
  series is now merged upstream, so it is no longer carried as a dependency.
- Fixed the build with CONFIG_CPUFREQ_DT_RUST=y. The generic DT cpufreq
  driver only has the (unbound) per-CPU device, so it cannot hand a
  &Device<Bound> to Policy::set_clk(). Added a pub(crate) Clk::get_unbound()
  for the few in-tree abstractions that operate on a device outside a bind
  scope; set_clk() now takes &Device and uses it. Clk::get()/get_optional()
  and the devm_* helpers still require &Device<Bound>.
- Added impl From<Error<State>> for kernel::error::Error, so the fallible
  state transitions can be used with `?` (and chained) instead of
  .map_err(|e| e.error).
- Added Clk::<Prepared>::with_enabled(), which runs a closure with the clock
  temporarily enabled, scoping the Enabled state without giving up the
  prepared clock.
- Documented how to change a clock's state at runtime via an enum, for
  drivers that enable/disable across resume/suspend.
- Link to v3: https://lore.kernel.org/r/20260107-clk-type-state-v3-0-77d3e3ee59c2@collabora.com

Changes in v3:
- Rebased on top of 6.19-rc4
- Dropped patch 1 (from Alice), added her series as a dependency instead
- Fixed Tyr, PWM_TH1520 drivers
- Changed clk.rs imports to kernel-vertical style
- Added support get_optional shortcut for Prepared and Enabled (i.e.:
  Clk::<Enabled>::get_optional())
- Fixed misplaced #[inline] tag

Thanks, Danilo {
  - Moved the devres changes into its own patch
  - Require &Device<Bound> for all functions where a &Device is used
  - Account for con_in in SAFETY comments where applicable
  - Added backticks
}

- Link to v2: https://lore.kernel.org/r/20250910-clk-type-state-v2-0-1b97c11bb631@collabora.com

Changes in v2:
- Added Alice's patch as patch 1, since it is a dependency.
- Added devm helpers (like we did for Regulator<T>)
- Fixed missing clk_put() call in Drop (Danilo)
- Fixed missing parenthesis and wrong docs (Viresh)
- Removed extra "dev" parameter from "shutdown" example (Danilo)
- Removed useless type annotation from example (Danilo)
- Link to v1: https://lore.kernel.org/rust-for-linux/20250729-clk-type-state-v1-1-896b53816f7b@collabora.com/#r

---
Daniel Almeida (6):
      rust: clk: use 'kernel vertical style' for imports
      rust: clk: replace `OptionalClk` with `Clk::get_optional()`
      rust: clk: use the type-state pattern
      rust: clk: add guards for scoped prepare and enable
      rust: clk: add devres-managed clks
      rust: clk: add `ExclusiveClk`

Maurice Hieronymus (1):
      pwm: th1520: Lock clock rate with clk_rate_exclusive_get

 drivers/cpufreq/rcpufreq_dt.rs |   7 +-
 drivers/gpu/drm/tyr/driver.rs  |  32 +-
 drivers/pwm/pwm_th1520.rs      |  25 +-
 rust/kernel/clk.rs             | 834 +++++++++++++++++++++++++++++++++++------
 rust/kernel/cpufreq.rs         |   8 +-
 5 files changed, 748 insertions(+), 158 deletions(-)
---
base-commit: d3645b43ad60fe1ab3b4a869f6908414d5831a65
change-id: 20250909-clk-type-state-c01aa7dd551d

Best regards,
--  
Daniel Almeida <daniel.almeida@collabora.com>

[PATCH v6 1/7] rust: clk: use 'kernel vertical style' for imports

[Daniel Almeida]

Convert all imports to use the new import style. This will make it easier
to land new changes in the future. Drop the `Result` import, which the
prelude already provides.

No change of functionality implied.

Link: https://docs.kernel.org/rust/coding-guidelines.html#imports
Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Onur Özkan <work@onurozkan.dev>
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
 rust/kernel/clk.rs | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/rust/kernel/clk.rs b/rust/kernel/clk.rs
index f04f5c4a03d6..c73fe7c57251 100644
--- a/rust/kernel/clk.rs
+++ b/rust/kernel/clk.rs
@@ -96,11 +96,17 @@ mod common_clk {
     use super::Hertz;
     use crate::{
         device::Device,
-        error::{from_err_ptr, to_result, Result},
-        prelude::*,
+        error::{
+            from_err_ptr,
+            to_result, //
+        },
+        prelude::*, //
     };
 
-    use core::{ops::Deref, ptr};
+    use core::{
+        ops::Deref,
+        ptr, //
+    };
 
     /// A reference-counted clock.
     ///
@@ -119,9 +125,11 @@ mod common_clk {
     /// The following example demonstrates how to obtain and configure a clock for a device.
     ///
     /// ```
-    /// use kernel::clk::{Clk, Hertz};
+    /// use kernel::clk::{
+    ///     Clk,
+    ///     Hertz,
+    /// };
     /// use kernel::device::Device;
-    /// use kernel::error::Result;
     ///
     /// fn configure_clk(dev: &Device) -> Result {
     ///     let clk = Clk::get(dev, Some(c"apb_clk"))?;
@@ -293,9 +301,11 @@ fn drop(&mut self) {
     /// device. The code functions correctly whether or not the clock is available.
     ///
     /// ```
-    /// use kernel::clk::{OptionalClk, Hertz};
+    /// use kernel::clk::{
+    ///     Hertz,
+    ///     OptionalClk,
+    /// };
     /// use kernel::device::Device;
-    /// use kernel::error::Result;
     ///
     /// fn configure_clk(dev: &Device) -> Result {
     ///     let clk = OptionalClk::get(dev, Some(c"apb_clk"))?;

-- 
2.56.0

[PATCH v6 2/7] rust: clk: replace `OptionalClk` with `Clk::get_optional()`

[Daniel Almeida]

OptionalClk wraps a Clk whose pointer may be NULL and derefs to it.
There is no struct optional_clk in C. clk_get_optional() returns a
struct clk pointer that is NULL when the clock has no producer, and the
clk functions that Clk wraps accept NULL. The type invariant of Clk
already allows a NULL pointer, so the wrapper is not needed.

Add Clk::get_optional() and remove OptionalClk. Convert tyr, its only
user.

Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
 drivers/gpu/drm/tyr/driver.rs | 13 +++---
 rust/kernel/clk.rs            | 98 +++++++++----------------------------------
 2 files changed, 25 insertions(+), 86 deletions(-)

diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index bfb0ba19caff..c382a26904ee 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -1,10 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0 or MIT
 
 use kernel::{
-    clk::{
-        Clk,
-        OptionalClk, //
-    },
+    clk::Clk,
     device::{
         Bound,
         Core,
@@ -117,8 +114,8 @@ fn probe<'bound>(
         _info: Option<&'bound Self::IdInfo>,
     ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
         let core_clk = Clk::get(pdev.as_ref(), Some(c"core"))?;
-        let stacks_clk = OptionalClk::get(pdev.as_ref(), Some(c"stacks"))?;
-        let coregroup_clk = OptionalClk::get(pdev.as_ref(), Some(c"coregroup"))?;
+        let stacks_clk = Clk::get_optional(pdev.as_ref(), Some(c"stacks"))?;
+        let coregroup_clk = Clk::get_optional(pdev.as_ref(), Some(c"coregroup"))?;
 
         core_clk.prepare_enable()?;
         stacks_clk.prepare_enable()?;
@@ -219,8 +216,8 @@ impl drm::Driver for TyrDrmDriver {
 
 struct Clocks {
     core: Clk,
-    stacks: OptionalClk,
-    coregroup: OptionalClk,
+    stacks: Clk,
+    coregroup: Clk,
 }
 
 impl Drop for Clocks {
diff --git a/rust/kernel/clk.rs b/rust/kernel/clk.rs
index c73fe7c57251..1de2484df8cf 100644
--- a/rust/kernel/clk.rs
+++ b/rust/kernel/clk.rs
@@ -103,10 +103,7 @@ mod common_clk {
         prelude::*, //
     };
 
-    use core::{
-        ops::Deref,
-        ptr, //
-    };
+    use core::ptr;
 
     /// A reference-counted clock.
     ///
@@ -175,6 +172,25 @@ pub fn get(dev: &Device, name: Option<&CStr>) -> Result<Self> {
             })?))
         }
 
+        /// Behaves the same as [`Self::get`], except when there is no clock
+        /// producer. In this case, instead of returning [`ENOENT`], it returns
+        /// a dummy [`Clk`].
+        ///
+        /// Equivalent to the kernel's [`clk_get_optional`] API.
+        ///
+        /// [`clk_get_optional`]:
+        /// https://docs.kernel.org/core-api/kernel-api.html#c.clk_get_optional
+        pub fn get_optional(dev: &Device, name: Option<&CStr>) -> Result<Self> {
+            let con_id = name.map_or(ptr::null(), |n| n.as_char_ptr());
+
+            // SAFETY: It is safe to call [`clk_get_optional`] for a valid device pointer.
+            //
+            // INVARIANT: The reference-count is decremented when [`Clk`] goes out of scope.
+            Ok(Self(from_err_ptr(unsafe {
+                bindings::clk_get_optional(dev.as_raw(), con_id)
+            })?))
+        }
+
         /// Obtain the raw [`struct clk`] pointer.
         #[inline]
         pub fn as_raw(&self) -> *mut bindings::clk {
@@ -280,80 +296,6 @@ fn drop(&mut self) {
             unsafe { bindings::clk_put(self.as_raw()) };
         }
     }
-
-    /// A reference-counted optional clock.
-    ///
-    /// A lightweight wrapper around an optional [`Clk`]. An [`OptionalClk`] represents a [`Clk`]
-    /// that a driver can function without but may improve performance or enable additional
-    /// features when available.
-    ///
-    /// # Invariants
-    ///
-    /// An [`OptionalClk`] instance encapsulates a [`Clk`] with either a valid [`struct clk`] or
-    /// `NULL` pointer.
-    ///
-    /// Instances of this type are reference-counted. Calling [`OptionalClk::get`] ensures that the
-    /// allocation remains valid for the lifetime of the [`OptionalClk`].
-    ///
-    /// # Examples
-    ///
-    /// The following example demonstrates how to obtain and configure an optional clock for a
-    /// device. The code functions correctly whether or not the clock is available.
-    ///
-    /// ```
-    /// use kernel::clk::{
-    ///     Hertz,
-    ///     OptionalClk,
-    /// };
-    /// use kernel::device::Device;
-    ///
-    /// fn configure_clk(dev: &Device) -> Result {
-    ///     let clk = OptionalClk::get(dev, Some(c"apb_clk"))?;
-    ///
-    ///     clk.prepare_enable()?;
-    ///
-    ///     let expected_rate = Hertz::from_ghz(1);
-    ///
-    ///     if clk.rate() != expected_rate {
-    ///         clk.set_rate(expected_rate)?;
-    ///     }
-    ///
-    ///     clk.disable_unprepare();
-    ///     Ok(())
-    /// }
-    /// ```
-    ///
-    /// [`struct clk`]: https://docs.kernel.org/driver-api/clk.html
-    pub struct OptionalClk(Clk);
-
-    impl OptionalClk {
-        /// Gets [`OptionalClk`] corresponding to a [`Device`] and a connection id.
-        ///
-        /// Equivalent to the kernel's [`clk_get_optional`] API.
-        ///
-        /// [`clk_get_optional`]:
-        /// https://docs.kernel.org/core-api/kernel-api.html#c.clk_get_optional
-        pub fn get(dev: &Device, name: Option<&CStr>) -> Result<Self> {
-            let con_id = name.map_or(ptr::null(), |n| n.as_char_ptr());
-
-            // SAFETY: It is safe to call [`clk_get_optional`] for a valid device pointer.
-            //
-            // INVARIANT: The reference-count is decremented when [`OptionalClk`] goes out of
-            // scope.
-            Ok(Self(Clk(from_err_ptr(unsafe {
-                bindings::clk_get_optional(dev.as_raw(), con_id)
-            })?)))
-        }
-    }
-
-    // Make [`OptionalClk`] behave like [`Clk`].
-    impl Deref for OptionalClk {
-        type Target = Clk;
-
-        fn deref(&self) -> &Clk {
-            &self.0
-        }
-    }
 }
 
 #[cfg(CONFIG_COMMON_CLK)]

-- 
2.56.0

[PATCH v6 3/7] rust: clk: use the type-state pattern

[Daniel Almeida]

The current Clk abstraction can still be improved on the following issues:

a) It only keeps track of a count to clk_get(), which means that users have
to manually call disable() and unprepare(), or a variation of those, like
disable_unprepare().

b) It allows repeated calls to prepare() or enable(), but it keeps no track
of how often these were called, i.e., it's currently legal to write the
following:

clk.prepare();
clk.prepare();
clk.enable();
clk.enable();

And nothing gets undone on drop().

c) It does not let a user express the state of the clk through the
type system. For example, there is currently no way to encode that a Clk is
enabled via the type system alone.

Following the Regulator abstraction, switch this abstraction to use the
type-state pattern instead. It solves both a) and b) by establishing a
number of states and the valid ways to transition between them. It also
automatically undoes any call to clk_get(), clk_prepare() and clk_enable()
as applicable on drop(), so users do not have to do anything special
before Clk goes out of scope.

It solves c) by directly encoding the state of the Clk into the type, e.g.:
Clk<Enabled> is now known to be a Clk that is enabled.

The INVARIANTS section for Clk is expanded to highlight the relationship
between the states and the respective reference counts that are owned by
each of them.

The examples are expanded to highlight how a user can transition between
states, as well as highlight some of the shortcuts built into the API.

The current implementation is also more flexible, in the sense that it
allows for more states to be added in the future. This lets us implement
different strategies for handling clocks, including one that mimics the
current API, allowing for multiple calls to prepare() and enable().

The users (cpufreq, rcpufreq_dt, tyr and pwm-th1520) are updated by this
patch (and not a separate one) to reflect the new changes. This is needed,
because otherwise this patch would break the build.

Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
 drivers/cpufreq/rcpufreq_dt.rs |   7 +-
 drivers/gpu/drm/tyr/driver.rs  |  35 ++--
 drivers/pwm/pwm_th1520.rs      |  20 +-
 rust/kernel/clk.rs             | 438 ++++++++++++++++++++++++++++++++++-------
 rust/kernel/cpufreq.rs         |   8 +-
 5 files changed, 393 insertions(+), 115 deletions(-)

diff --git a/drivers/cpufreq/rcpufreq_dt.rs b/drivers/cpufreq/rcpufreq_dt.rs
index d7ead60bf8c2..9a79f382fe3c 100644
--- a/drivers/cpufreq/rcpufreq_dt.rs
+++ b/drivers/cpufreq/rcpufreq_dt.rs
@@ -3,7 +3,10 @@
 //! Rust based implementation of the cpufreq-dt driver.
 
 use kernel::{
-    clk::Clk,
+    clk::{
+        Clk,
+        Unprepared, //
+    },
     cpu,
     cpufreq, //
     cpumask::CpumaskVar,
@@ -47,7 +50,7 @@ struct CPUFreqDTDevice {
     freq_table: opp::FreqTable,
     _mask: CpumaskVar,
     _token: Option<opp::ConfigToken>,
-    _clk: Clk,
+    _clk: Clk<Unprepared>,
 }
 
 #[derive(Default)]
diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index c382a26904ee..b387c4b9bff3 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -1,7 +1,10 @@
 // SPDX-License-Identifier: GPL-2.0 or MIT
 
 use kernel::{
-    clk::Clk,
+    clk::{
+        Clk,
+        Enabled, //
+    },
     device::{
         Bound,
         Core,
@@ -113,13 +116,9 @@ fn probe<'bound>(
         pdev: &'bound platform::Device<Core<'_>>,
         _info: Option<&'bound Self::IdInfo>,
     ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
-        let core_clk = Clk::get(pdev.as_ref(), Some(c"core"))?;
-        let stacks_clk = Clk::get_optional(pdev.as_ref(), Some(c"stacks"))?;
-        let coregroup_clk = Clk::get_optional(pdev.as_ref(), Some(c"coregroup"))?;
-
-        core_clk.prepare_enable()?;
-        stacks_clk.prepare_enable()?;
-        coregroup_clk.prepare_enable()?;
+        let core_clk = Clk::<Enabled>::get(pdev.as_ref(), Some(c"core"))?;
+        let stacks_clk = Clk::<Enabled>::get_optional(pdev.as_ref(), Some(c"stacks"))?;
+        let coregroup_clk = Clk::<Enabled>::get_optional(pdev.as_ref(), Some(c"coregroup"))?;
 
         let mali_regulator = Regulator::<regulator::Enabled>::get(pdev.as_ref(), c"mali")?;
         let sram_regulator = Regulator::<regulator::Enabled>::get(pdev.as_ref(), c"sram")?;
@@ -160,9 +159,9 @@ fn probe<'bound>(
                 pdev,
                 fw: firmware,
                 clks <- new_mutex!(Clocks {
-                    core: core_clk,
-                    stacks: stacks_clk,
-                    coregroup: coregroup_clk,
+                    _core: core_clk,
+                    _stacks: stacks_clk,
+                    _coregroup: coregroup_clk,
                 }),
                 regulators <- new_mutex!(Regulators {
                     _mali: mali_regulator,
@@ -215,17 +214,9 @@ impl drm::Driver for TyrDrmDriver {
 }
 
 struct Clocks {
-    core: Clk,
-    stacks: Clk,
-    coregroup: Clk,
-}
-
-impl Drop for Clocks {
-    fn drop(&mut self) {
-        self.core.disable_unprepare();
-        self.stacks.disable_unprepare();
-        self.coregroup.disable_unprepare();
-    }
+    _core: Clk<Enabled>,
+    _stacks: Clk<Enabled>,
+    _coregroup: Clk<Enabled>,
 }
 
 struct Regulators {
diff --git a/drivers/pwm/pwm_th1520.rs b/drivers/pwm/pwm_th1520.rs
index 728993998446..5180b912efad 100644
--- a/drivers/pwm/pwm_th1520.rs
+++ b/drivers/pwm/pwm_th1520.rs
@@ -21,7 +21,10 @@
 //!
 
 use kernel::{
-    clk::Clk,
+    clk::{
+        Clk,
+        Enabled, //
+    },
     device::{
         Bound,
         Core,
@@ -87,11 +90,11 @@ struct Th1520WfHw {
 }
 
 /// The driver's private data struct. It holds all necessary devres managed resources.
-#[pin_data(PinnedDrop)]
+#[pin_data]
 struct Th1520PwmDriverData {
     #[pin]
     iomem: DevresIoMem<TH1520_PWM_REG_SIZE>,
-    clk: Clk,
+    clk: Clk<Enabled>,
 }
 
 impl pwm::PwmOps for Th1520PwmDriverData {
@@ -294,13 +297,6 @@ fn write_waveform(
     }
 }
 
-#[pinned_drop]
-impl PinnedDrop for Th1520PwmDriverData {
-    fn drop(self: Pin<&mut Self>) {
-        self.clk.disable_unprepare();
-    }
-}
-
 struct Th1520PwmPlatformDriver;
 
 kernel::of_device_table!(
@@ -321,9 +317,7 @@ fn probe<'bound>(
         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()?;
+        let clk = Clk::<Enabled>::get(dev, None)?;
 
         // TODO: Get exclusive ownership of the clock to prevent rate changes.
         // The Rust equivalent of `clk_rate_exclusive_get()` is not yet available.
diff --git a/rust/kernel/clk.rs b/rust/kernel/clk.rs
index 1de2484df8cf..c0812331c490 100644
--- a/rust/kernel/clk.rs
+++ b/rust/kernel/clk.rs
@@ -95,7 +95,10 @@ fn from(freq: Hertz) -> Self {
 mod common_clk {
     use super::Hertz;
     use crate::{
-        device::Device,
+        device::{
+            Bound,
+            Device, //
+        },
         error::{
             from_err_ptr,
             to_result, //
@@ -103,12 +106,116 @@ mod common_clk {
         prelude::*, //
     };
 
-    use core::ptr;
+    use core::{
+        marker::PhantomData,
+        mem::ManuallyDrop,
+        ptr, //
+    };
+
+    mod private {
+        pub trait Sealed {}
+
+        impl Sealed for super::Unprepared {}
+        impl Sealed for super::Prepared {}
+        impl Sealed for super::Enabled {}
+    }
+
+    /// A trait representing the different states that a [`Clk`] can be in.
+    pub trait ClkState: private::Sealed {
+        /// Whether the clock is enabled in this state.
+        const ENABLED: bool;
+
+        /// Whether the clock is prepared in this state.
+        const PREPARED: bool;
+    }
+
+    /// A state where the [`Clk`] is not prepared and not enabled.
+    pub struct Unprepared;
+
+    /// A state where the [`Clk`] is prepared but not enabled.
+    pub struct Prepared;
+
+    /// A state where the [`Clk`] is both prepared and enabled.
+    pub struct Enabled;
+
+    impl ClkState for Unprepared {
+        const ENABLED: bool = false;
+        const PREPARED: bool = false;
+    }
+
+    impl ClkState for Prepared {
+        const ENABLED: bool = false;
+        const PREPARED: bool = true;
+    }
+
+    impl ClkState for Enabled {
+        const ENABLED: bool = true;
+        const PREPARED: bool = true;
+    }
+
+    /// An error that can occur when trying to convert a [`Clk`] between states.
+    pub struct Error<State: ClkState> {
+        /// The error that occurred.
+        pub error: kernel::error::Error,
+
+        /// The [`Clk`] that caused the error, so that the operation may be
+        /// retried.
+        pub clk: Clk<State>,
+    }
+
+    impl<State: ClkState> core::fmt::Debug for Error<State> {
+        fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+            self.error.fmt(f)
+        }
+    }
+
+    impl<State: ClkState> From<Error<State>> for kernel::error::Error {
+        /// Discards the [`Clk`] and keeps only the error code.
+        ///
+        /// This makes the fallible state transitions usable with the `?`
+        /// operator when the caller does not need to retry the operation on the
+        /// original [`Clk`], e.g.:
+        ///
+        /// ```
+        /// use kernel::clk::{
+        ///     Clk,
+        ///     Enabled,
+        ///     Unprepared,
+        /// };
+        /// use kernel::device::{
+        ///     Bound,
+        ///     Device,
+        /// };
+        ///
+        /// fn get_enabled(dev: &Device<Bound>) -> Result<Clk<Enabled>> {
+        ///     let clk = Clk::<Unprepared>::get(dev, Some(c"apb_clk"))?
+        ///         .prepare()?
+        ///         .enable()?;
+        ///     Ok(clk)
+        /// }
+        /// ```
+        #[inline]
+        fn from(err: Error<State>) -> Self {
+            err.error
+        }
+    }
 
     /// A reference-counted clock.
     ///
     /// Rust abstraction for the C [`struct clk`].
     ///
+    /// A [`Clk`] instance represents a clock that can be in one of several
+    /// states: [`Unprepared`], [`Prepared`], or [`Enabled`].
+    ///
+    /// No action needs to be taken when a [`Clk`] is dropped. The calls to
+    /// `clk_disable()` and `clk_unprepare()` will be made as applicable.
+    /// Since `clk_unprepare()` may sleep, a [`Clk<Prepared>`] or a
+    /// [`Clk<Enabled>`] must not be dropped in atomic context.
+    ///
+    /// An optional [`Clk`] is treated like a regular [`Clk`], but its
+    /// inner `struct clk` pointer is `NULL`. This interfaces correctly with the
+    /// C API and also exposes all the methods of a regular [`Clk`] to users.
+    ///
     /// # Invariants
     ///
     /// A [`Clk`] instance holds either a pointer to a valid [`struct clk`] created by the C
@@ -117,6 +224,12 @@ mod common_clk {
     /// Instances of this type are reference-counted. Calling [`Clk::get`] ensures that the
     /// allocation remains valid for the lifetime of the [`Clk`].
     ///
+    /// The [`Prepared`] state is associated with a single count of
+    /// `clk_prepare()`, and the [`Enabled`] state is associated with a single
+    /// count of both `clk_prepare()` and `clk_enable()`.
+    ///
+    /// All states are associated with a single count of `clk_get()`.
+    ///
     /// # Examples
     ///
     /// The following example demonstrates how to obtain and configure a clock for a device.
@@ -124,14 +237,30 @@ mod common_clk {
     /// ```
     /// use kernel::clk::{
     ///     Clk,
+    ///     Enabled,
     ///     Hertz,
+    ///     Prepared,
+    ///     Unprepared,
+    /// };
+    /// use kernel::device::{
+    ///     Bound,
+    ///     Device,
     /// };
-    /// use kernel::device::Device;
     ///
-    /// fn configure_clk(dev: &Device) -> Result {
-    ///     let clk = Clk::get(dev, Some(c"apb_clk"))?;
+    /// fn configure_clk(dev: &Device<Bound>) -> Result {
+    ///     // Each state has its own `Clk::get`, e.g.:
+    ///     let clk: Clk<Enabled> = Clk::<Enabled>::get(dev, Some(c"apb_clk"))?;
+    ///
+    ///     // Any other state is also possible, e.g.:
+    ///     let clk: Clk<Prepared> = Clk::<Prepared>::get(dev, Some(c"bus_clk"))?;
     ///
-    ///     clk.prepare_enable()?;
+    ///     // Later:
+    ///     //
+    ///     // `?` works directly thanks to `From<Error<State>>`. The failed
+    ///     // `Clk` is dropped on error. Match on the returned `Error<State>`
+    ///     // instead (its `clk` field is the original `Clk`) if you want to
+    ///     // retry the operation.
+    ///     let clk: Clk<Enabled> = clk.enable()?;
     ///
     ///     let expected_rate = Hertz::from_ghz(1);
     ///
@@ -139,37 +268,102 @@ mod common_clk {
     ///         clk.set_rate(expected_rate)?;
     ///     }
     ///
-    ///     clk.disable_unprepare();
+    ///     // Nothing is needed here. The drop implementation will undo any
+    ///     // operations as appropriate.
+    ///     Ok(())
+    /// }
+    ///
+    /// fn shutdown(clk: Clk<Enabled>) -> Result {
+    ///     // The states can also be traversed in reverse:
+    ///     let clk: Clk<Prepared> = clk.disable();
+    ///
+    ///     // This is of type `Clk<Unprepared>`.
+    ///     let clk = clk.unprepare();
+    ///
     ///     Ok(())
     /// }
     /// ```
     ///
+    /// Drivers that need to change a clock's state at runtime (for example to
+    /// enable it on resume and disable it on suspend) can keep it in an enum
+    /// and move between the variants:
+    ///
+    /// ```
+    /// use kernel::clk::{Clk, Enabled, Prepared};
+    /// use kernel::error::Result;
+    ///
+    /// enum DeviceClk {
+    ///     Suspended(Clk<Prepared>),
+    ///     Resumed(Clk<Enabled>),
+    /// }
+    ///
+    /// impl DeviceClk {
+    ///     fn resume(self) -> Result<Self> {
+    ///         Ok(match self {
+    ///             DeviceClk::Suspended(clk) => DeviceClk::Resumed(clk.enable()?),
+    ///             resumed => resumed,
+    ///         })
+    ///     }
+    ///
+    ///     fn suspend(self) -> Result<Self> {
+    ///         Ok(match self {
+    ///             DeviceClk::Resumed(clk) => DeviceClk::Suspended(clk.disable()),
+    ///             suspended => suspended,
+    ///         })
+    ///     }
+    /// }
+    /// ```
+    ///
     /// [`struct clk`]: https://docs.kernel.org/driver-api/clk.html
     #[repr(transparent)]
-    pub struct Clk(*mut bindings::clk);
+    pub struct Clk<T: ClkState> {
+        inner: *mut bindings::clk,
+        _phantom: core::marker::PhantomData<T>,
+    }
 
     // SAFETY: It is safe to call `clk_put` on another thread than where `clk_get` was called.
-    unsafe impl Send for Clk {}
+    unsafe impl<T: ClkState> Send for Clk<T> {}
 
     // SAFETY: It is safe to call any combination of the `&self` methods in parallel, as the
     // methods are synchronized internally.
-    unsafe impl Sync for Clk {}
+    unsafe impl<T: ClkState> Sync for Clk<T> {}
 
-    impl Clk {
-        /// Gets [`Clk`] corresponding to a [`Device`] and a connection id.
+    impl Clk<Unprepared> {
+        /// Gets [`Clk`] corresponding to a bound [`Device`] and a connection
+        /// id.
         ///
         /// Equivalent to the kernel's [`clk_get`] API.
         ///
         /// [`clk_get`]: https://docs.kernel.org/core-api/kernel-api.html#c.clk_get
-        pub fn get(dev: &Device, name: Option<&CStr>) -> Result<Self> {
+        #[inline]
+        pub fn get(dev: &Device<Bound>, name: Option<&CStr>) -> Result<Clk<Unprepared>> {
+            Self::get_unbound(dev, name)
+        }
+
+        /// Gets [`Clk`] corresponding to a [`Device`] and a connection id,
+        /// without requiring the device to be bound.
+        ///
+        /// This is sound because [`clk_get`] and [`clk_put`] do not depend on the
+        /// device being bound to a driver. It is `pub(crate)` because a driver
+        /// should obtain its clocks through a bound device (see [`Clk::get`]). It
+        /// is meant for the few in-tree abstractions that operate on a device
+        /// outside a bind scope, such as the generic DT cpufreq driver.
+        ///
+        /// [`clk_get`]: https://docs.kernel.org/core-api/kernel-api.html#c.clk_get
+        /// [`clk_put`]: https://docs.kernel.org/core-api/kernel-api.html#c.clk_put
+        #[inline]
+        pub(crate) fn get_unbound(dev: &Device, name: Option<&CStr>) -> Result<Clk<Unprepared>> {
             let con_id = name.map_or(ptr::null(), |n| n.as_char_ptr());
 
-            // SAFETY: It is safe to call [`clk_get`] for a valid device pointer.
-            //
+            // SAFETY: It is safe to call `clk_get()` for a valid device pointer
+            // and any `con_id`, including NULL.
+            let inner = from_err_ptr(unsafe { bindings::clk_get(dev.as_raw(), con_id) })?;
+
             // INVARIANT: The reference-count is decremented when [`Clk`] goes out of scope.
-            Ok(Self(from_err_ptr(unsafe {
-                bindings::clk_get(dev.as_raw(), con_id)
-            })?))
+            Ok(Self {
+                inner,
+                _phantom: PhantomData,
+            })
         }
 
         /// Behaves the same as [`Self::get`], except when there is no clock
@@ -180,100 +374,179 @@ pub fn get(dev: &Device, name: Option<&CStr>) -> Result<Self> {
         ///
         /// [`clk_get_optional`]:
         /// https://docs.kernel.org/core-api/kernel-api.html#c.clk_get_optional
-        pub fn get_optional(dev: &Device, name: Option<&CStr>) -> Result<Self> {
+        #[inline]
+        pub fn get_optional(dev: &Device<Bound>, name: Option<&CStr>) -> Result<Clk<Unprepared>> {
             let con_id = name.map_or(ptr::null(), |n| n.as_char_ptr());
 
-            // SAFETY: It is safe to call [`clk_get_optional`] for a valid device pointer.
-            //
+            // SAFETY: It is safe to call `clk_get_optional()` for a valid device
+            // pointer and any `con_id`, including NULL.
+            let inner = from_err_ptr(unsafe { bindings::clk_get_optional(dev.as_raw(), con_id) })?;
+
             // INVARIANT: The reference-count is decremented when [`Clk`] goes out of scope.
-            Ok(Self(from_err_ptr(unsafe {
-                bindings::clk_get_optional(dev.as_raw(), con_id)
-            })?))
+            Ok(Self {
+                inner,
+                _phantom: PhantomData,
+            })
         }
 
-        /// Obtain the raw [`struct clk`] pointer.
+        /// Attempts to convert the [`Clk`] to a [`Prepared`] state.
+        ///
+        /// Equivalent to the kernel's [`clk_prepare`] API.
+        ///
+        /// [`clk_prepare`]: https://docs.kernel.org/core-api/kernel-api.html#c.clk_prepare
         #[inline]
-        pub fn as_raw(&self) -> *mut bindings::clk {
-            self.0
+        pub fn prepare(self) -> Result<Clk<Prepared>, Error<Unprepared>> {
+            // The `clk_get()` count moves to the returned `Clk<Prepared>`.
+            let clk = ManuallyDrop::new(self);
+
+            // SAFETY: By the type invariants, `clk.as_raw()` is a valid argument
+            // for `clk_prepare()`.
+            to_result(unsafe { bindings::clk_prepare(clk.as_raw()) })
+                // INVARIANT: `clk_prepare()` succeeded, so the returned
+                // `Clk<Prepared>` owns a single count of it, which is released
+                // when it leaves the [`Prepared`] state.
+                .map(|()| Clk {
+                    inner: clk.inner,
+                    _phantom: PhantomData,
+                })
+                .map_err(|error| Error {
+                    error,
+                    clk: ManuallyDrop::into_inner(clk),
+                })
         }
+    }
 
-        /// Enable the clock.
+    impl Clk<Prepared> {
+        /// Obtains a [`Clk`] from a bound [`Device`] and a connection id and
+        /// prepares it.
         ///
-        /// Equivalent to the kernel's [`clk_enable`] API.
-        ///
-        /// [`clk_enable`]: https://docs.kernel.org/core-api/kernel-api.html#c.clk_enable
+        /// Equivalent to calling [`Clk::get`], followed by [`Clk::prepare`].
         #[inline]
-        pub fn enable(&self) -> Result {
-            // SAFETY: By the type invariants, self.as_raw() is a valid argument for
-            // [`clk_enable`].
-            to_result(unsafe { bindings::clk_enable(self.as_raw()) })
+        pub fn get(dev: &Device<Bound>, name: Option<&CStr>) -> Result<Clk<Prepared>> {
+            Ok(Clk::<Unprepared>::get(dev, name)?.prepare()?)
         }
 
-        /// Disable the clock.
-        ///
-        /// Equivalent to the kernel's [`clk_disable`] API.
-        ///
-        /// [`clk_disable`]: https://docs.kernel.org/core-api/kernel-api.html#c.clk_disable
+        /// Behaves the same as [`Self::get`], except when there is no clock
+        /// producer. In this case, instead of returning [`ENOENT`], it returns
+        /// a dummy [`Clk`].
         #[inline]
-        pub fn disable(&self) {
-            // SAFETY: By the type invariants, self.as_raw() is a valid argument for
-            // [`clk_disable`].
-            unsafe { bindings::clk_disable(self.as_raw()) };
+        pub fn get_optional(dev: &Device<Bound>, name: Option<&CStr>) -> Result<Clk<Prepared>> {
+            Ok(Clk::<Unprepared>::get_optional(dev, name)?.prepare()?)
         }
 
-        /// Prepare the clock.
+        /// Converts the [`Clk`] to an [`Unprepared`] state.
         ///
-        /// Equivalent to the kernel's [`clk_prepare`] API.
+        /// Equivalent to the kernel's [`clk_unprepare`] API.
         ///
-        /// [`clk_prepare`]: https://docs.kernel.org/core-api/kernel-api.html#c.clk_prepare
+        /// [`clk_unprepare`]: https://docs.kernel.org/core-api/kernel-api.html#c.clk_unprepare
         #[inline]
-        pub fn prepare(&self) -> Result {
-            // SAFETY: By the type invariants, self.as_raw() is a valid argument for
-            // [`clk_prepare`].
-            to_result(unsafe { bindings::clk_prepare(self.as_raw()) })
+        pub fn unprepare(self) -> Clk<Unprepared> {
+            // The `clk_get()` count moves to the returned `Clk<Unprepared>`.
+            let clk = ManuallyDrop::new(self);
+
+            // SAFETY: By the type invariants, `clk.as_raw()` is a valid argument
+            // for `clk_unprepare()`.
+            unsafe { bindings::clk_unprepare(clk.as_raw()) }
+
+            // INVARIANT: The `clk_prepare()` count was released above, so the
+            // returned `Clk<Unprepared>` owns only the `clk_get()` count.
+            Clk {
+                inner: clk.inner,
+                _phantom: PhantomData,
+            }
         }
 
-        /// Unprepare the clock.
+        /// Attempts to convert the [`Clk`] to an [`Enabled`] state.
         ///
-        /// Equivalent to the kernel's [`clk_unprepare`] API.
+        /// Equivalent to the kernel's [`clk_enable`] API.
         ///
-        /// [`clk_unprepare`]: https://docs.kernel.org/core-api/kernel-api.html#c.clk_unprepare
+        /// [`clk_enable`]: https://docs.kernel.org/core-api/kernel-api.html#c.clk_enable
         #[inline]
-        pub fn unprepare(&self) {
-            // SAFETY: By the type invariants, self.as_raw() is a valid argument for
-            // [`clk_unprepare`].
-            unsafe { bindings::clk_unprepare(self.as_raw()) };
+        pub fn enable(self) -> Result<Clk<Enabled>, Error<Prepared>> {
+            // The `clk_get()` and `clk_prepare()` counts move to the returned
+            // `Clk<Enabled>`.
+            let clk = ManuallyDrop::new(self);
+
+            // SAFETY: By the type invariants, `clk.as_raw()` is a valid argument
+            // for `clk_enable()`.
+            to_result(unsafe { bindings::clk_enable(clk.as_raw()) })
+                // INVARIANT: `clk_enable()` succeeded, so the returned
+                // `Clk<Enabled>` owns a single count of it, which is released
+                // when it leaves the [`Enabled`] state.
+                .map(|()| Clk {
+                    inner: clk.inner,
+                    _phantom: PhantomData,
+                })
+                .map_err(|error| Error {
+                    error,
+                    clk: ManuallyDrop::into_inner(clk),
+                })
         }
+    }
 
-        /// Prepare and enable the clock.
+    impl Clk<Enabled> {
+        /// Gets [`Clk`] corresponding to a bound [`Device`] and a connection id
+        /// and then prepares and enables it.
         ///
-        /// Equivalent to calling [`Clk::prepare`] followed by [`Clk::enable`].
+        /// Equivalent to calling [`Clk::get`], followed by [`Clk::prepare`],
+        /// followed by [`Clk::enable`].
+        #[inline]
+        pub fn get(dev: &Device<Bound>, name: Option<&CStr>) -> Result<Clk<Enabled>> {
+            Ok(Clk::<Prepared>::get(dev, name)?.enable()?)
+        }
+
+        /// Behaves the same as [`Self::get`], except when there is no clock
+        /// producer. In this case, instead of returning [`ENOENT`], it returns
+        /// a dummy [`Clk`].
         #[inline]
-        pub fn prepare_enable(&self) -> Result {
-            // SAFETY: By the type invariants, self.as_raw() is a valid argument for
-            // [`clk_prepare_enable`].
-            to_result(unsafe { bindings::clk_prepare_enable(self.as_raw()) })
+        pub fn get_optional(dev: &Device<Bound>, name: Option<&CStr>) -> Result<Clk<Enabled>> {
+            Ok(Clk::<Prepared>::get_optional(dev, name)?.enable()?)
         }
 
-        /// Disable and unprepare the clock.
+        /// Disables the [`Clk`] and converts it to the [`Prepared`] state.
         ///
-        /// Equivalent to calling [`Clk::disable`] followed by [`Clk::unprepare`].
+        /// Equivalent to the kernel's [`clk_disable`] API.
+        ///
+        /// [`clk_disable`]: https://docs.kernel.org/core-api/kernel-api.html#c.clk_disable
         #[inline]
-        pub fn disable_unprepare(&self) {
-            // SAFETY: By the type invariants, self.as_raw() is a valid argument for
-            // [`clk_disable_unprepare`].
-            unsafe { bindings::clk_disable_unprepare(self.as_raw()) };
+        pub fn disable(self) -> Clk<Prepared> {
+            // The `clk_get()` and `clk_prepare()` counts move to the returned
+            // `Clk<Prepared>`.
+            let clk = ManuallyDrop::new(self);
+
+            // SAFETY: By the type invariants, `clk.as_raw()` is a valid argument
+            // for `clk_disable()`.
+            unsafe { bindings::clk_disable(clk.as_raw()) };
+
+            // INVARIANT: The `clk_enable()` count was released above, so the
+            // returned `Clk<Prepared>` owns the `clk_get()` and `clk_prepare()`
+            // counts.
+            Clk {
+                inner: clk.inner,
+                _phantom: PhantomData,
+            }
+        }
+    }
+
+    impl<T: ClkState> Clk<T> {
+        /// Obtain the raw [`struct clk`] pointer.
+        #[inline]
+        pub fn as_raw(&self) -> *mut bindings::clk {
+            self.inner
         }
 
         /// Get clock's rate.
         ///
         /// Equivalent to the kernel's [`clk_get_rate`] API.
         ///
+        /// The returned rate is only guaranteed to reflect what the hardware is
+        /// doing once the clock is [`Enabled`].
+        ///
         /// [`clk_get_rate`]: https://docs.kernel.org/core-api/kernel-api.html#c.clk_get_rate
         #[inline]
         pub fn rate(&self) -> Hertz {
-            // SAFETY: By the type invariants, self.as_raw() is a valid argument for
-            // [`clk_get_rate`].
+            // SAFETY: By the type invariants, `self.as_raw()` is a valid argument
+            // for `clk_get_rate()`.
             Hertz(unsafe { bindings::clk_get_rate(self.as_raw()) })
         }
 
@@ -284,15 +557,28 @@ pub fn rate(&self) -> Hertz {
         /// [`clk_set_rate`]: https://docs.kernel.org/core-api/kernel-api.html#c.clk_set_rate
         #[inline]
         pub fn set_rate(&self, rate: Hertz) -> Result {
-            // SAFETY: By the type invariants, self.as_raw() is a valid argument for
-            // [`clk_set_rate`].
+            // SAFETY: By the type invariants, `self.as_raw()` is a valid argument
+            // for `clk_set_rate()`.
             to_result(unsafe { bindings::clk_set_rate(self.as_raw(), rate.as_hz()) })
         }
     }
 
-    impl Drop for Clk {
+    impl<T: ClkState> Drop for Clk<T> {
         fn drop(&mut self) {
-            // SAFETY: By the type invariants, self.as_raw() is a valid argument for [`clk_put`].
+            if T::ENABLED {
+                // SAFETY: By the type invariants, `self.as_raw()` is a valid
+                // argument for `clk_disable()`.
+                unsafe { bindings::clk_disable(self.as_raw()) };
+            }
+
+            if T::PREPARED {
+                // SAFETY: By the type invariants, `self.as_raw()` is a valid
+                // argument for `clk_unprepare()`.
+                unsafe { bindings::clk_unprepare(self.as_raw()) };
+            }
+
+            // SAFETY: By the type invariants, `self.as_raw()` is a valid
+            // argument for `clk_put()`.
             unsafe { bindings::clk_put(self.as_raw()) };
         }
     }
diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
index affa2b9490ef..4c4ec14b0568 100644
--- a/rust/kernel/cpufreq.rs
+++ b/rust/kernel/cpufreq.rs
@@ -558,8 +558,12 @@ pub fn cpus(&mut self) -> &mut cpumask::Cpumask {
     /// The caller must guarantee that the returned [`Clk`] is not dropped while it is getting used
     /// by the C code.
     #[cfg(CONFIG_COMMON_CLK)]
-    pub unsafe fn set_clk(&mut self, dev: &Device, name: Option<&CStr>) -> Result<Clk> {
-        let clk = Clk::get(dev, name)?;
+    pub unsafe fn set_clk(
+        &mut self,
+        dev: &Device,
+        name: Option<&CStr>,
+    ) -> Result<Clk<crate::clk::Unprepared>> {
+        let clk = Clk::<crate::clk::Unprepared>::get_unbound(dev, name)?;
         self.as_mut_ref().clk = clk.as_raw();
         Ok(clk)
     }

-- 
2.56.0

[PATCH v6 4/7] rust: clk: add guards for scoped prepare and enable

[Daniel Almeida]

The state transitions consume the Clk, so a driver that keeps a
Clk<Prepared> in its data cannot enable it for a short section without
moving it out and back in. A clock handle also holds at most one count
of clk_prepare() and one of clk_enable().

Add EnabledGuard, PreparedGuard and PrepareEnabledGuard, returned by
Clk::enable_guard(), Clk::prepare_guard() and
Clk::prepare_enable_guard(). Each guard borrows the Clk, takes its count
when it is created and releases it when it is dropped. It dereferences
to the clock in the higher state, so a function that takes a
&Clk<Enabled> also accepts an EnabledGuard. Several guards may borrow the
same Clk, and the borrow keeps the Clk from being dropped or moved to
another state while a guard is alive.

There are no users yet.

Suggested-by: Alexandre Courbot <acourbot@nvidia.com>
Suggested-by: Gary Guo <gary@garyguo.net>
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
 rust/kernel/clk.rs | 218 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 218 insertions(+)

diff --git a/rust/kernel/clk.rs b/rust/kernel/clk.rs
index c0812331c490..93b4dc17f0ad 100644
--- a/rust/kernel/clk.rs
+++ b/rust/kernel/clk.rs
@@ -109,6 +109,7 @@ mod common_clk {
     use core::{
         marker::PhantomData,
         mem::ManuallyDrop,
+        ops::Deref,
         ptr, //
     };
 
@@ -228,6 +229,11 @@ fn from(err: Error<State>) -> Self {
     /// `clk_prepare()`, and the [`Enabled`] state is associated with a single
     /// count of both `clk_prepare()` and `clk_enable()`.
     ///
+    /// A shared reference to a [`Clk<Prepared>`] or a [`Clk<Enabled>`] may also
+    /// be obtained from a [`PreparedGuard`], an [`EnabledGuard`] or a
+    /// [`PrepareEnabledGuard`]. In that case, the guard owns the counts of that
+    /// state for as long as the reference is alive.
+    ///
     /// All states are associated with a single count of `clk_get()`.
     ///
     /// # Examples
@@ -314,6 +320,43 @@ fn from(err: Error<State>) -> Self {
     /// }
     /// ```
     ///
+    /// Drivers that keep a clock around in a lower state can take a guard
+    /// instead of moving the clock through the states, e.g. to have it running
+    /// for a short section:
+    ///
+    /// ```
+    /// use kernel::clk::{
+    ///     Clk,
+    ///     Hertz,
+    ///     Prepared,
+    ///     Unprepared,
+    /// };
+    ///
+    /// fn read_rate(clk: &Clk<Prepared>) -> Result<Hertz> {
+    ///     let enabled = clk.enable_guard()?;
+    ///
+    ///     // The clock is enabled until `enabled` goes out of scope.
+    ///     Ok(enabled.rate())
+    /// }
+    ///
+    /// fn touch_registers(clk: &Clk<Unprepared>) -> Result {
+    ///     {
+    ///         let prepared = clk.prepare_guard()?;
+    ///         let _enabled = prepared.enable_guard()?;
+    ///
+    ///         // `_enabled` borrows `prepared`, so the clock is disabled before
+    ///         // it is unprepared.
+    ///     }
+    ///
+    ///     // Or, alternatively:
+    ///     {
+    ///         let _enabled = clk.prepare_enable_guard()?;
+    ///     }
+    ///
+    ///     Ok(())
+    /// }
+    /// ```
+    ///
     /// [`struct clk`]: https://docs.kernel.org/driver-api/clk.html
     #[repr(transparent)]
     pub struct Clk<T: ClkState> {
@@ -414,6 +457,43 @@ pub fn prepare(self) -> Result<Clk<Prepared>, Error<Unprepared>> {
                     clk: ManuallyDrop::into_inner(clk),
                 })
         }
+
+        /// Prepares the clock for the lifetime of the returned guard.
+        ///
+        /// Unlike [`Clk::prepare`], this does not consume `self`. The guard
+        /// dereferences to a [`Clk<Prepared>`].
+        ///
+        /// Equivalent to the kernel's [`clk_prepare`] API.
+        ///
+        /// [`clk_prepare`]: https://docs.kernel.org/core-api/kernel-api.html#c.clk_prepare
+        #[inline]
+        pub fn prepare_guard(&self) -> Result<PreparedGuard<'_>> {
+            // SAFETY: By the type invariants, `self.as_raw()` is a valid argument
+            // for `clk_prepare()`.
+            to_result(unsafe { bindings::clk_prepare(self.as_raw()) })?;
+
+            // INVARIANT: `clk_prepare()` succeeded, so the guard owns a single
+            // count of it.
+            Ok(PreparedGuard { clk: self })
+        }
+
+        /// Prepares and enables the clock for the lifetime of the returned guard.
+        ///
+        /// Unlike [`Clk::prepare`] followed by [`Clk::enable`], this does not
+        /// consume `self`. The guard dereferences to a [`Clk<Enabled>`].
+        ///
+        /// Equivalent to calling [`Clk::prepare_guard`] followed by
+        /// [`Clk::enable_guard`], with a single guard holding both counts.
+        #[inline]
+        pub fn prepare_enable_guard(&self) -> Result<PrepareEnabledGuard<'_>> {
+            // SAFETY: By the type invariants, `self.as_raw()` is a valid argument
+            // for `clk_prepare_enable()`.
+            to_result(unsafe { bindings::clk_prepare_enable(self.as_raw()) })?;
+
+            // INVARIANT: `clk_prepare_enable()` succeeded, so the guard owns a
+            // single count of both `clk_prepare()` and `clk_enable()`.
+            Ok(PrepareEnabledGuard { clk: self })
+        }
     }
 
     impl Clk<Prepared> {
@@ -482,6 +562,27 @@ pub fn enable(self) -> Result<Clk<Enabled>, Error<Prepared>> {
                     clk: ManuallyDrop::into_inner(clk),
                 })
         }
+
+        /// Enables the clock for the lifetime of the returned guard.
+        ///
+        /// Unlike [`Clk::enable`], this does not consume `self`, so it can be used
+        /// on a [`Clk<Prepared>`] that is shared or stored in driver data. The
+        /// clock stays [`Prepared`] through `self`, and the guard dereferences to
+        /// a [`Clk<Enabled>`].
+        ///
+        /// Equivalent to the kernel's [`clk_enable`] API.
+        ///
+        /// [`clk_enable`]: https://docs.kernel.org/core-api/kernel-api.html#c.clk_enable
+        #[inline]
+        pub fn enable_guard(&self) -> Result<EnabledGuard<'_>> {
+            // SAFETY: By the type invariants, `self.as_raw()` is a valid argument
+            // for `clk_enable()`.
+            to_result(unsafe { bindings::clk_enable(self.as_raw()) })?;
+
+            // INVARIANT: `clk_enable()` succeeded, so the guard owns a single
+            // count of it.
+            Ok(EnabledGuard { clk: self })
+        }
     }
 
     impl Clk<Enabled> {
@@ -582,6 +683,123 @@ fn drop(&mut self) {
             unsafe { bindings::clk_put(self.as_raw()) };
         }
     }
+
+    /// A guard that keeps a [`Clk`] enabled.
+    ///
+    /// Returned by [`Clk::enable_guard`]. Dropping it disables the clock again,
+    /// while the borrowed [`Clk<Prepared>`] keeps it prepared. It dereferences to
+    /// [`Clk<Enabled>`], so it can be passed wherever an enabled clock is
+    /// expected.
+    ///
+    /// # Invariants
+    ///
+    /// The guard owns a single count of `clk_enable()` on `clk`, taken by
+    /// [`Clk::enable_guard`].
+    #[must_use = "if unused, the clock will be disabled immediately"]
+    pub struct EnabledGuard<'a> {
+        clk: &'a Clk<Prepared>,
+    }
+
+    impl Deref for EnabledGuard<'_> {
+        type Target = Clk<Enabled>;
+
+        #[inline]
+        fn deref(&self) -> &Clk<Enabled> {
+            // SAFETY: `Clk<T>` is `#[repr(transparent)]` over the `struct clk`
+            // pointer for every `T`, so `Clk<Prepared>` and `Clk<Enabled>` have
+            // the same layout. By the type invariants, the guard owns an enable
+            // count for as long as the returned reference is alive, and a shared
+            // reference cannot be used to drop the `Clk`.
+            unsafe { &*ptr::from_ref(self.clk).cast::<Clk<Enabled>>() }
+        }
+    }
+
+    impl Drop for EnabledGuard<'_> {
+        #[inline]
+        fn drop(&mut self) {
+            // SAFETY: By the type invariants, the guard owns a `clk_enable()`
+            // count on `self.clk`, which this releases.
+            unsafe { bindings::clk_disable(self.clk.as_raw()) };
+        }
+    }
+
+    /// A guard that keeps a [`Clk`] prepared.
+    ///
+    /// Returned by [`Clk::prepare_guard`]. Dropping it unprepares the clock
+    /// again, which may sleep. It dereferences to [`Clk<Prepared>`], so it can be
+    /// passed wherever a prepared clock is expected.
+    ///
+    /// # Invariants
+    ///
+    /// The guard owns a single count of `clk_prepare()` on `clk`, taken by
+    /// [`Clk::prepare_guard`].
+    #[must_use = "if unused, the clock will be unprepared immediately"]
+    pub struct PreparedGuard<'a> {
+        clk: &'a Clk<Unprepared>,
+    }
+
+    impl Deref for PreparedGuard<'_> {
+        type Target = Clk<Prepared>;
+
+        #[inline]
+        fn deref(&self) -> &Clk<Prepared> {
+            // SAFETY: `Clk<T>` is `#[repr(transparent)]` over the `struct clk`
+            // pointer for every `T`, so `Clk<Unprepared>` and `Clk<Prepared>`
+            // have the same layout. By the type invariants, the guard owns a
+            // prepare count for as long as the returned reference is alive, and
+            // a shared reference cannot be used to drop the `Clk`.
+            unsafe { &*ptr::from_ref(self.clk).cast::<Clk<Prepared>>() }
+        }
+    }
+
+    impl Drop for PreparedGuard<'_> {
+        #[inline]
+        fn drop(&mut self) {
+            // SAFETY: By the type invariants, the guard owns a `clk_prepare()`
+            // count on `self.clk`, which this releases.
+            unsafe { bindings::clk_unprepare(self.clk.as_raw()) };
+        }
+    }
+
+    /// A guard that keeps a [`Clk`] prepared and enabled.
+    ///
+    /// Returned by [`Clk::prepare_enable_guard`]. Dropping it disables and
+    /// unprepares the clock again, which may sleep. It dereferences to
+    /// [`Clk<Enabled>`], so it can be passed wherever an enabled clock is
+    /// expected.
+    ///
+    /// # Invariants
+    ///
+    /// The guard owns a single count of both `clk_prepare()` and `clk_enable()`
+    /// on `clk`, taken by [`Clk::prepare_enable_guard`].
+    #[must_use = "if unused, the clock will be disabled and unprepared immediately"]
+    pub struct PrepareEnabledGuard<'a> {
+        clk: &'a Clk<Unprepared>,
+    }
+
+    impl Deref for PrepareEnabledGuard<'_> {
+        type Target = Clk<Enabled>;
+
+        #[inline]
+        fn deref(&self) -> &Clk<Enabled> {
+            // SAFETY: `Clk<T>` is `#[repr(transparent)]` over the `struct clk`
+            // pointer for every `T`, so `Clk<Unprepared>` and `Clk<Enabled>`
+            // have the same layout. By the type invariants, the guard owns a
+            // prepare and an enable count for as long as the returned reference
+            // is alive, and a shared reference cannot be used to drop the `Clk`.
+            unsafe { &*ptr::from_ref(self.clk).cast::<Clk<Enabled>>() }
+        }
+    }
+
+    impl Drop for PrepareEnabledGuard<'_> {
+        #[inline]
+        fn drop(&mut self) {
+            // SAFETY: By the type invariants, the guard owns a count of both
+            // `clk_prepare()` and `clk_enable()` on `self.clk`, which this
+            // releases.
+            unsafe { bindings::clk_disable_unprepare(self.clk.as_raw()) };
+        }
+    }
 }
 
 #[cfg(CONFIG_COMMON_CLK)]

-- 
2.56.0

[PATCH v6 5/7] rust: clk: add devres-managed clks

[Daniel Almeida]

Many drivers only need a clock running for as long as the device is
bound and never touch it afterwards. In C, devm_clk_get_enabled() and
devm_clk_get_optional_enabled() cover this case.

Add devm_enable(), devm_enable_optional() and
devm_enable_optional_with_rate(), which wrap the C helpers. They return
no handle, so the driver keeps nothing in its data and cannot change the
state of the clock. The clock is disabled, unprepared and put when the
device is unbound.

This mirrors regulator::devm_enable() and devm_enable_optional().

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
 rust/kernel/clk.rs | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/rust/kernel/clk.rs b/rust/kernel/clk.rs
index 93b4dc17f0ad..e2587c613182 100644
--- a/rust/kernel/clk.rs
+++ b/rust/kernel/clk.rs
@@ -121,6 +121,52 @@ impl Sealed for super::Prepared {}
         impl Sealed for super::Enabled {}
     }
 
+    /// Obtains and enables a [`devres`]-managed [`Clk`] for a bound device.
+    ///
+    /// [`devres`]: crate::devres::Devres
+    #[inline]
+    pub fn devm_enable(dev: &Device<Bound>, name: Option<&CStr>) -> Result {
+        let name = name.map_or(ptr::null(), |n| n.as_char_ptr());
+
+        // SAFETY: It is safe to call `devm_clk_get_enabled()` for a valid device
+        // pointer and any `name`, including NULL.
+        from_err_ptr(unsafe { bindings::devm_clk_get_enabled(dev.as_raw(), name) })?;
+        Ok(())
+    }
+
+    /// Obtains and enables an optional [`devres`]-managed [`Clk`] for a bound
+    /// device.
+    ///
+    /// If the clock does not exist, this succeeds and does nothing.
+    ///
+    /// [`devres`]: crate::devres::Devres
+    #[inline]
+    pub fn devm_enable_optional(dev: &Device<Bound>, name: Option<&CStr>) -> Result {
+        let name = name.map_or(ptr::null(), |n| n.as_char_ptr());
+
+        // SAFETY: It is safe to call `devm_clk_get_optional_enabled()` for a
+        // valid device pointer and any `name`, including NULL.
+        from_err_ptr(unsafe { bindings::devm_clk_get_optional_enabled(dev.as_raw(), name) })?;
+        Ok(())
+    }
+
+    /// Same as [`devm_enable_optional`], but also sets the rate.
+    #[inline]
+    pub fn devm_enable_optional_with_rate(
+        dev: &Device<Bound>,
+        name: Option<&CStr>,
+        rate: Hertz,
+    ) -> Result {
+        let name = name.map_or(ptr::null(), |n| n.as_char_ptr());
+
+        // SAFETY: It is safe to call `devm_clk_get_optional_enabled_with_rate()`
+        // for a valid device pointer and any `name`, including NULL.
+        from_err_ptr(unsafe {
+            bindings::devm_clk_get_optional_enabled_with_rate(dev.as_raw(), name, rate.as_hz())
+        })?;
+        Ok(())
+    }
+
     /// A trait representing the different states that a [`Clk`] can be in.
     pub trait ClkState: private::Sealed {
         /// Whether the clock is enabled in this state.

-- 
2.56.0

[PATCH v6 6/7] rust: clk: add `ExclusiveClk`

[Daniel Almeida]

Some drivers derive their configuration from a clock rate and cannot
tolerate the rate changing underneath them. In C, they claim rate
exclusivity with clk_rate_exclusive_get(), after which the clock
framework refuses rate changes from other consumers until
clk_rate_exclusive_put() is called.

Add ExclusiveClk<T>, which takes a Clk<T> in any state and holds rate
exclusivity until it is dropped. ExclusiveClk::new() calls
clk_rate_exclusive_get(), and dropping the ExclusiveClk or calling
ExclusiveClk::into_inner() calls clk_rate_exclusive_put().
ExclusiveClk dereferences to the Clk<T>, so the rate can still be read
and changed through it. Since a Clk cannot be cloned, the ExclusiveClk
is the only handle to the consumer that holds the exclusivity.

pwm-th1520 is the first user, in the next patch.

Co-developed-by: Maurice Hieronymus <mhi@mailbox.org>
Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
 rust/kernel/clk.rs | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 100 insertions(+)

diff --git a/rust/kernel/clk.rs b/rust/kernel/clk.rs
index e2587c613182..8d7ba0fae459 100644
--- a/rust/kernel/clk.rs
+++ b/rust/kernel/clk.rs
@@ -846,6 +846,106 @@ fn drop(&mut self) {
             unsafe { bindings::clk_disable_unprepare(self.clk.as_raw()) };
         }
     }
+
+    /// A [`Clk`] with exclusive control over its rate.
+    ///
+    /// While an [`ExclusiveClk`] exists, no other consumer of the same clock
+    /// may change its rate. The rate can still be changed through the
+    /// [`ExclusiveClk`] itself, which dereferences to the [`Clk`] in its
+    /// current state.
+    ///
+    /// The [`Clk`] is moved into the [`ExclusiveClk`], and a [`Clk`] cannot be
+    /// cloned, so only the [`ExclusiveClk`] refers to the consumer handle that
+    /// holds the exclusivity.
+    ///
+    /// # Invariants
+    ///
+    /// `clk_rate_exclusive_get()` has been called on the wrapped [`Clk`], and
+    /// the matching `clk_rate_exclusive_put()` has not been called yet.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::clk::{
+    ///     Clk,
+    ///     Enabled,
+    ///     ExclusiveClk,
+    ///     Hertz,
+    /// };
+    /// use kernel::device::{
+    ///     Bound,
+    ///     Device,
+    /// };
+    ///
+    /// fn lock_rate(dev: &Device<Bound>) -> Result<ExclusiveClk<Enabled>> {
+    ///     let clk = Clk::<Enabled>::get(dev, None)?;
+    ///     clk.set_rate(Hertz::from_mhz(24))?;
+    ///
+    ///     ExclusiveClk::new(clk)
+    /// }
+    /// ```
+    pub struct ExclusiveClk<T: ClkState>(Clk<T>);
+
+    impl<T: ClkState> ExclusiveClk<T> {
+        /// Takes exclusive control over the rate of `clk`.
+        ///
+        /// No other consumer of the clock may change its rate until the
+        /// returned [`ExclusiveClk`] is dropped or [`ExclusiveClk::into_inner`]
+        /// is called. This may sleep.
+        ///
+        /// Equivalent to the kernel's [`clk_rate_exclusive_get`] API.
+        ///
+        /// [`clk_rate_exclusive_get`]:
+        /// https://docs.kernel.org/core-api/kernel-api.html#c.clk_rate_exclusive_get
+        #[inline]
+        pub fn new(clk: Clk<T>) -> Result<Self> {
+            // SAFETY: By the type invariants of `Clk`, `clk.as_raw()` is a valid
+            // argument for `clk_rate_exclusive_get()`.
+            to_result(unsafe { bindings::clk_rate_exclusive_get(clk.as_raw()) })?;
+
+            // INVARIANT: `clk_rate_exclusive_get()` succeeded, and the matching
+            // `clk_rate_exclusive_put()` is called when the `ExclusiveClk` is
+            // dropped or consumed by `into_inner()`.
+            Ok(Self(clk))
+        }
+
+        /// Releases the exclusive control over the rate and returns the [`Clk`].
+        ///
+        /// Equivalent to the kernel's [`clk_rate_exclusive_put`] API.
+        ///
+        /// [`clk_rate_exclusive_put`]:
+        /// https://docs.kernel.org/core-api/kernel-api.html#c.clk_rate_exclusive_put
+        #[inline]
+        pub fn into_inner(self) -> Clk<T> {
+            let this = ManuallyDrop::new(self);
+
+            // SAFETY: By the type invariants, `clk_rate_exclusive_get()` was
+            // called on `this.0` and not yet balanced, so this balances it.
+            unsafe { bindings::clk_rate_exclusive_put(this.0.as_raw()) };
+
+            // SAFETY: `this` is wrapped in `ManuallyDrop`, so the `Clk` is moved
+            // out exactly once and the `Drop` of `ExclusiveClk` does not run.
+            unsafe { ptr::read(&this.0) }
+        }
+    }
+
+    impl<T: ClkState> Deref for ExclusiveClk<T> {
+        type Target = Clk<T>;
+
+        #[inline]
+        fn deref(&self) -> &Clk<T> {
+            &self.0
+        }
+    }
+
+    impl<T: ClkState> Drop for ExclusiveClk<T> {
+        #[inline]
+        fn drop(&mut self) {
+            // SAFETY: By the type invariants, `clk_rate_exclusive_get()` was
+            // called on `self.0` and not yet balanced, so this balances it.
+            unsafe { bindings::clk_rate_exclusive_put(self.0.as_raw()) };
+        }
+    }
 }
 
 #[cfg(CONFIG_COMMON_CLK)]

-- 
2.56.0

[PATCH v6 7/7] pwm: th1520: Lock clock rate with clk_rate_exclusive_get

[Daniel Almeida]

From: Maurice Hieronymus <mhi@mailbox.org>

The driver derives period and duty cycle from the clock rate read at
probe, so a later rate change would silently miscompute waveforms.
Switch to the new ExclusiveClk wrapper to hold the rate for the lifetime
of the driver data and drop the corresponding TODO.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Michal Wilczynski <m.wilczynski@samsung.com>
Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
[ Adapted to ExclusiveClk<Enabled> and ExclusiveClk::new(), and rebased
  onto the vertical imports and DevresIoMem conversion. - Daniel ]
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
 drivers/pwm/pwm_th1520.rs | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/pwm/pwm_th1520.rs b/drivers/pwm/pwm_th1520.rs
index 5180b912efad..9be0a7f210b9 100644
--- a/drivers/pwm/pwm_th1520.rs
+++ b/drivers/pwm/pwm_th1520.rs
@@ -23,7 +23,8 @@
 use kernel::{
     clk::{
         Clk,
-        Enabled, //
+        Enabled,
+        ExclusiveClk, //
     },
     device::{
         Bound,
@@ -94,7 +95,7 @@ struct Th1520WfHw {
 struct Th1520PwmDriverData {
     #[pin]
     iomem: DevresIoMem<TH1520_PWM_REG_SIZE>,
-    clk: Clk<Enabled>,
+    clk: ExclusiveClk<Enabled>,
 }
 
 impl pwm::PwmOps for Th1520PwmDriverData {
@@ -318,10 +319,8 @@ fn probe<'bound>(
         let request = pdev.io_request_by_index(0).ok_or(ENODEV)?;
 
         let clk = Clk::<Enabled>::get(dev, None)?;
+        let clk = ExclusiveClk::new(clk)?;
 
-        // 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");

-- 
2.56.0

Re: [PATCH v6 6/7] rust: clk: add `ExclusiveClk`

[Daniel Almeida]

> On 13 Oct 2026, at 14:07, Daniel Almeida <daniel.almeida@collabora.com> wrote:
> 
> Some drivers derive their configuration from a clock rate and cannot
> tolerate the rate changing underneath them. In C, they claim rate
> exclusivity with clk_rate_exclusive_get(), after which the clock
> framework refuses rate changes from other consumers until
> clk_rate_exclusive_put() is called.
> 
> Add ExclusiveClk<T>, which takes a Clk<T> in any state and holds rate
> exclusivity until it is dropped. ExclusiveClk::new() calls
> clk_rate_exclusive_get(), and dropping the ExclusiveClk or calling
> ExclusiveClk::into_inner() calls clk_rate_exclusive_put().
> ExclusiveClk dereferences to the Clk<T>, so the rate can still be read
> and changed through it. Since a Clk cannot be cloned, the ExclusiveClk
> is the only handle to the consumer that holds the exclusivity.
> 
> pwm-th1520 is the first user, in the next patch.
> 
> Co-developed-by: Maurice Hieronymus <mhi@mailbox.org>
> Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>

Maurice, I changed a few things in the design, so I took ownership with
your co-developed-by tag.

Let me know if that works for you, or what you would rather see instead.

— Daniel

Re: [PATCH v6 1/7] rust: clk: use 'kernel vertical style' for imports

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] rust: clk: missing trailing // in vertical imports in doctests
--

commit d1e8fb84f14ee05c8bc2b0c2603a25f2c8d23a0f
Author: Daniel Almeida <daniel.almeida@collabora.com>

rust: clk: use 'kernel vertical style' for imports

This commit converts all imports in the clk module to use the kernel 
vertical style. It also drops the Result import since it is provided 
by the prelude.

Link: https://docs.kernel.org/rust/coding-guidelines.html#imports

> diff --git a/rust/kernel/clk.rs b/rust/kernel/clk.rs
> index f04f5c4a03d64..c73fe7c572511 100644
> --- a/rust/kernel/clk.rs
> +++ b/rust/kernel/clk.rs

[ ... ]

> @@ -119,9 +125,11 @@ mod common_clk {
>      /// The following example demonstrates how to obtain and configure a clock for a device.
>      ///
>      /// ```
> -    /// use kernel::clk::{Clk, Hertz};
> +    /// use kernel::clk::{
> +    ///     Clk,
> +    ///     Hertz,
> +    /// };

[Severity: Low]
This isn't a bug, but should this vertical import have a trailing // empty
comment? According to Documentation/rust/coding-guidelines.rst, vertical
imports require a trailing // to prevent rustfmt from collapsing the lines.
It looks like the doc test for Clk::get is missing it here.

>      /// use kernel::device::Device;
> -    /// use kernel::error::Result;
>      ///
>      /// fn configure_clk(dev: &Device) -> Result {
>      ///     let clk = Clk::get(dev, Some(c"apb_clk"))?;

[ ... ]

> @@ -293,9 +301,11 @@ fn drop(&mut self) {
>      /// device. The code functions correctly whether or not the clock is available.
>      ///
>      /// ```
> -    /// use kernel::clk::{OptionalClk, Hertz};
> +    /// use kernel::clk::{
> +    ///     Hertz,
> +    ///     OptionalClk,
> +    /// };

[Severity: Low]
Similarly, should the vertical import in the OptionalClk::get doc test also
include a trailing // comment to match the kernel vertical style guidelines?

>      /// use kernel::device::Device;
> -    /// use kernel::error::Result;
>      ///
>      /// fn configure_clk(dev: &Device) -> Result {
>      ///     let clk = OptionalClk::get(dev, Some(c"apb_clk"))?;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013-clk-type-state-v6-0-a5e9977b3509@collabora.com?part=1

Re: [PATCH v6 2/7] rust: clk: replace `OptionalClk` with `Clk::get_optional()`

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] rust: clk: missing #[inline] on get_optional() abstraction wrapper
--

commit 796077cfd7119a6144686ab76ca00413f2a3071b
Author: Daniel Almeida <daniel.almeida@collabora.com>
rust: clk: replace OptionalClk with Clk::get_optional()

This commit replaces the OptionalClk wrapper with a get_optional() method
on the Clk struct, as clk_get_optional() already returns a valid struct clk
pointer. The tyr driver is converted to use this new API.

> diff --git a/rust/kernel/clk.rs b/rust/kernel/clk.rs
> index c73fe7c572511..1de2484df8cf5 100644
> --- a/rust/kernel/clk.rs
> +++ b/rust/kernel/clk.rs
[ ... ]
> @@ -175,6 +172,25 @@ pub fn get(dev: &Device, name: Option<&CStr>) -> Result<Self> {

[Severity: Low]
This problem wasn't introduced by this patch, but should this preexisting
function have an #[inline] attribute? According to the Rust subsystem
guidelines, small functions or functions forwarding to a C binding call in
abstraction crates should be annotated with #[inline].

>              })?))
>          }
>  
> +        /// Behaves the same as [`Self::get`], except when there is no clock
> +        /// producer. In this case, instead of returning [`ENOENT`], it returns
> +        /// a dummy [`Clk`].
> +        ///
> +        /// Equivalent to the kernel's [`clk_get_optional`] API.
> +        ///
> +        /// [`clk_get_optional`]:
> +        /// https://docs.kernel.org/core-api/kernel-api.html#c.clk_get_optional
> +        pub fn get_optional(dev: &Device, name: Option<&CStr>) -> Result<Self> {

[Severity: Low]
Should this new function also include the #[inline] attribute? Since it
primarily prepares arguments and forwards them to
bindings::clk_get_optional(), adding #[inline] would align it with the
subsystem guidelines for abstraction wrappers.

> +            let con_id = name.map_or(ptr::null(), |n| n.as_char_ptr());
> +
> +            // SAFETY: It is safe to call [`clk_get_optional`] for a valid device pointer.
> +            //
> +            // INVARIANT: The reference-count is decremented when [`Clk`] goes out of scope.
> +            Ok(Self(from_err_ptr(unsafe {
> +                bindings::clk_get_optional(dev.as_raw(), con_id)
> +            })?))
> +        }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013-clk-type-state-v6-0-a5e9977b3509@collabora.com?part=2