[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250304085351.inrvjgixvxla4yn3@vireshk-i7>
Date: Tue, 4 Mar 2025 14:23:51 +0530
From: Viresh Kumar <viresh.kumar@...aro.org>
To: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
Cc: Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>, Miguel Ojeda <ojeda@...nel.org>,
Alex Gaynor <alex.gaynor@...il.com>,
Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <benno.lossin@...ton.me>,
Andreas Hindborg <a.hindborg@...nel.org>,
Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
Russell King <linux@...linux.org.uk>, linux-clk@...r.kernel.org,
linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org,
Vincent Guittot <vincent.guittot@...aro.org>,
Daniel Almeida <daniel.almeida@...labora.com>
Subject: Re: [PATCH V3 2/2] rust: Add initial clk abstractions
On 03-03-25, 11:16, Miguel Ojeda wrote:
> On Mon, Mar 3, 2025 at 11:00 AM Viresh Kumar <viresh.kumar@...aro.org> wrote:
> >
> > +/// Frequency unit.
> > +pub type Hertz = crate::ffi::c_ulong;
>
> Do we want this to be an alias or would it make sense to take the
> chance to make this a newtype?
I have tried some improvements based on your (and Alice's comments), please see
if it looks any better now.
--
viresh
-------------------------8<-------------------------
diff --git a/rust/kernel/clk.rs b/rust/kernel/clk.rs
new file mode 100644
index 000000000000..fc3cb0f5f332
--- /dev/null
+++ b/rust/kernel/clk.rs
@@ -0,0 +1,232 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Clock abstractions.
+//!
+//! C header: [`include/linux/clk.h`](srctree/include/linux/clk.h)
+//!
+//! Reference: <https://docs.kernel.org/driver-api/clk.html>
+
+use crate::{
+ bindings,
+ device::Device,
+ error::{from_err_ptr, to_result, Result},
+ ffi::c_ulong,
+ prelude::*,
+};
+
+use core::{ops::Deref, ptr};
+
+/// Frequency unit.
+#[derive(Copy, Clone, PartialEq, Eq, Debug)]
+pub struct Hertz(c_ulong);
+
+impl Hertz {
+ /// Creates a new `Hertz` value.
+ pub fn new(freq: c_ulong) -> Self {
+ Hertz(freq)
+ }
+
+ /// Returns the frequency in `Hertz`.
+ pub fn value(self) -> c_ulong {
+ self.0
+ }
+}
+
+/// This structure represents the Rust abstraction for a C [`struct clk`].
+///
+/// # Invariants
+///
+/// A [`Clk`] instance always corresponds to a valid [`struct clk`] created by the C portion of the
+/// kernel.
+///
+/// Instances of this type are reference-counted. Calling `get` ensures that the allocation remains
+/// valid for the lifetime of the [`Clk`].
+///
+/// ## Example
+///
+/// The following example demonstrates how to obtain and configure a clock for a device.
+///
+/// ```
+/// use kernel::clk::{Clk, Hertz};
+/// use kernel::device::Device;
+/// use kernel::error::Result;
+///
+/// fn configure_clk(dev: &Device) -> Result {
+/// let clk = Clk::get(dev, "apb_clk")?;
+///
+/// clk.prepare_enable()?;
+///
+/// let expected_rate = Hertz::new(1_000_000_000);
+///
+/// if clk.rate() != expected_rate {
+/// clk.set_rate(expected_rate)?;
+/// }
+///
+/// clk.disable_unprepare();
+/// Ok(())
+/// }
+/// ```
+///
+/// [`struct clk`]: https://docs.kernel.org/driver-api/clk.html
+#[repr(transparent)]
+pub struct Clk(*mut bindings::clk);
+
+impl Clk {
+ /// Gets `Clk` corresponding to a [`Device`] and a connection id.
+ pub fn get(dev: &Device, name: Option<&CStr>) -> Result<Self> {
+ let con_id = if let Some(name) = name {
+ name.as_ptr() as *const _
+ } else {
+ ptr::null()
+ };
+
+ // SAFETY: It is safe to call `clk_get()` for a valid device pointer.
+ Ok(Self(from_err_ptr(unsafe {
+ bindings::clk_get(dev.as_raw(), con_id)
+ })?))
+ }
+
+ /// Obtain the raw `struct clk *`.
+ #[inline]
+ pub fn as_raw(&self) -> *mut bindings::clk {
+ self.0
+ }
+
+ /// Enable the clock.
+ #[inline]
+ pub fn enable(&self) -> Result {
+ // SAFETY: It is safe to call clk APIs of the C code for a clock pointer earlier returned
+ // by `clk_get()`.
+ to_result(unsafe { bindings::clk_enable(self.as_raw()) })
+ }
+
+ /// Disable the clock.
+ #[inline]
+ pub fn disable(&self) {
+ // SAFETY: It is safe to call clk APIs of the C code for a clock pointer earlier returned
+ // by `clk_get()`.
+ unsafe { bindings::clk_disable(self.as_raw()) };
+ }
+
+ /// Prepare the clock.
+ #[inline]
+ pub fn prepare(&self) -> Result {
+ // SAFETY: It is safe to call clk APIs of the C code for a clock pointer earlier returned
+ // by `clk_get()`.
+ to_result(unsafe { bindings::clk_prepare(self.as_raw()) })
+ }
+
+ /// Unprepare the clock.
+ #[inline]
+ pub fn unprepare(&self) {
+ // SAFETY: It is safe to call clk APIs of the C code for a clock pointer earlier returned
+ // by `clk_get()`.
+ unsafe { bindings::clk_unprepare(self.as_raw()) };
+ }
+
+ /// Prepare and enable the clock.
+ #[inline]
+ pub fn prepare_enable(&self) -> Result {
+ // SAFETY: It is safe to call clk APIs of the C code for a clock pointer earlier returned
+ // by `clk_get()`.
+ to_result(unsafe { bindings::clk_prepare_enable(self.as_raw()) })
+ }
+
+ /// Disable and unprepare the clock.
+ #[inline]
+ pub fn disable_unprepare(&self) {
+ // SAFETY: It is safe to call clk APIs of the C code for a clock pointer earlier returned
+ // by `clk_get()`.
+ unsafe { bindings::clk_disable_unprepare(self.as_raw()) };
+ }
+
+ /// Get clock's rate.
+ #[inline]
+ pub fn rate(&self) -> Hertz {
+ // SAFETY: It is safe to call clk APIs of the C code for a clock pointer earlier returned
+ // by `clk_get()`.
+ Hertz::new(unsafe { bindings::clk_get_rate(self.as_raw()) })
+ }
+
+ /// Set clock's rate.
+ #[inline]
+ pub fn set_rate(&self, rate: Hertz) -> Result {
+ // SAFETY: It is safe to call clk APIs of the C code for a clock pointer earlier returned
+ // by `clk_get()`.
+ to_result(unsafe { bindings::clk_set_rate(self.as_raw(), rate.value()) })
+ }
+}
+
+impl Drop for Clk {
+ fn drop(&mut self) {
+ // SAFETY: It is safe to call clk APIs of the C code for a clock pointer earlier returned
+ // by `clk_get()`.
+ unsafe { bindings::clk_put(self.as_raw()) };
+ }
+}
+
+/// 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 or `NULL` [`struct clk`] pointer.
+///
+/// Instances of this type are reference-counted. Calling `get` ensures that the allocation remains
+/// valid for the lifetime of the `OptionalClk`.
+///
+/// ## Example
+///
+/// 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::{OptionalClk, Hertz};
+/// use kernel::device::Device;
+/// use kernel::error::Result;
+///
+/// fn configure_clk(dev: &Device) -> Result {
+/// let clk = OptionalClk::get(dev, "apb_clk")?;
+///
+/// clk.prepare_enable()?;
+///
+/// let expected_rate = Hertz::new(1_000_000_000);
+///
+/// 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.
+ pub fn get(dev: &Device, name: Option<&CStr>) -> Result<Self> {
+ let con_id = if let Some(name) = name {
+ name.as_ptr() as *const _
+ } else {
+ ptr::null()
+ };
+
+ // SAFETY: It is safe to call `clk_get_optional()` for a valid device pointer.
+ 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
+ }
+}
Powered by blists - more mailing lists