[RFC PATCH V3 0/8] Rust bindings for cpufreq and OPP core + sample driver

[RFC PATCH V3 0/8] Rust bindings for cpufreq and OPP core + sample driver

[Viresh Kumar]

Hello,

This RFC adds initial rust bindings for two subsystems, cpufreq and operating
performance points (OPP). The bindings are provided for most of the interface
these subsystems expose.

This series also provides a sample cpufreq driver rcpufreq-dt, which is a
duplicate of the merged cpufreq-dt driver (A generic platform agnostic device
tree based cpufreq driver) used on most of the ARM platforms.

This is tested with the help of QEMU for now and frequency transitions, various
configurations, driver binding/unbinding work as expected. No performance
measurement is done with this.

These patches (along with few other dependencies) are pushed here for anyone to
give them a try:

git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/linux.git rust/cpufreq-dt


This depends on basic bindings for few other modules: device/driver, platform
driver, OF, clk, and cpumask. I am not looking to upstream a full fledged
support for them yet.

Based on staging/rust-device from the Rust tree (which is based over v6.10-rc1).

V2->V3:
- Rebased on latest rust-device changes, which removed `Data` and so few changes
  were required to make it work.
- use srctree links (Alice Ryhl).
- Various changes the OPP creation APIs, new APIs: from_raw_opp() and
  from_raw_opp_owned() (Alice Ryhl).
- Inline as_raw() helpers (Alice Ryhl).
- Add new interface (`OPP::Token`) for dynamically created OPPs.
- Add Reviewed-by tag from Manos.
- Modified/simplified cpufreq registration structure / method a bit.

V1->V2:
- Create and use separate bindings for OF, clk, cpumask, etc (not included in
  this patchset but pushed to the above branch). This helped removing direct
  calls from the driver.
- Fix wrong usage of Pinning + Vec.
- Use Token for OPP Config.
- Use Opaque, transparent and Aref for few structures.
- Broken down into smaller patches to make it easy for reviewers.
- Based over staging/rust-device.

Thanks.


*** BLURB HERE ***

Viresh Kumar (8):
  rust: Add initial bindings for OPP framework
  rust: Extend OPP bindings for the OPP table
  rust: Extend OPP bindings for the configuration options
  rust: Add initial bindings for cpufreq framework
  rust: Extend cpufreq bindings for policy and driver ops
  rust: Extend cpufreq bindings for driver registration
  rust: Extend OPP bindings with CPU frequency table
  cpufreq: Add Rust based cpufreq-dt driver

 drivers/cpufreq/Kconfig         |   12 +
 drivers/cpufreq/Makefile        |    1 +
 drivers/cpufreq/rcpufreq_dt.rs  |  225 +++++++
 rust/bindings/bindings_helper.h |    2 +
 rust/helpers.c                  |   15 +
 rust/kernel/cpufreq.rs          | 1041 +++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |    4 +
 rust/kernel/opp.rs              |  918 +++++++++++++++++++++++++++
 8 files changed, 2218 insertions(+)
 create mode 100644 drivers/cpufreq/rcpufreq_dt.rs
 create mode 100644 rust/kernel/cpufreq.rs
 create mode 100644 rust/kernel/opp.rs

-- 
2.31.1.272.g89b43f80a514

[RFC PATCH V3 1/8] rust: Add initial bindings for OPP framework

[Viresh Kumar]

This commit adds initial Rust bindings for the Operating performance
points (OPP) core. This adds bindings for `struct dev_pm_opp` and
`struct dev_pm_opp_data` to begin with.

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/lib.rs              |   2 +
 rust/kernel/opp.rs              | 182 ++++++++++++++++++++++++++++++++
 3 files changed, 185 insertions(+)
 create mode 100644 rust/kernel/opp.rs

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index d8b54b9fa4d0..1bf8e053c8f4 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -15,6 +15,7 @@
 #include <linux/of.h>
 #include <linux/phy.h>
 #include <linux/platform_device.h>
+#include <linux/pm_opp.h>
 #include <linux/refcount.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 3bf1089b87a3..e309d7774cbd 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -45,6 +45,8 @@
 #[cfg(CONFIG_NET)]
 pub mod net;
 pub mod of;
+#[cfg(CONFIG_PM_OPP)]
+pub mod opp;
 pub mod platform;
 pub mod prelude;
 pub mod print;
diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
new file mode 100644
index 000000000000..b26e39a74635
--- /dev/null
+++ b/rust/kernel/opp.rs
@@ -0,0 +1,182 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Operating performance points.
+//!
+//! This module provides bindings for interacting with the OPP subsystem.
+//!
+//! C header: [`include/linux/pm_opp.h`](srctree/include/linux/pm_opp.h)
+
+use crate::{
+    bindings,
+    device::Device,
+    error::{code::*, to_result, Result},
+    types::{ARef, AlwaysRefCounted, Opaque},
+};
+
+use core::ptr;
+
+/// Dynamically created Operating performance point (OPP).
+pub struct Token {
+    dev: ARef<Device>,
+    freq: u64,
+}
+
+impl Token {
+    /// Adds an OPP dynamically.
+    pub fn new(dev: &ARef<Device>, mut data: Data) -> Result<Self> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        to_result(unsafe { bindings::dev_pm_opp_add_dynamic(dev.as_raw(), &mut data.0) })?;
+        Ok(Self {
+            dev: dev.clone(),
+            freq: data.freq(),
+        })
+    }
+}
+
+impl Drop for Token {
+    fn drop(&mut self) {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        unsafe { bindings::dev_pm_opp_remove(self.dev.as_raw(), self.freq) };
+    }
+}
+
+/// Equivalent to `struct dev_pm_opp_data` in the C Code.
+#[repr(transparent)]
+pub struct Data(bindings::dev_pm_opp_data);
+
+impl Data {
+    /// Creates new instance of [`Data`].
+    pub fn new(freq: u64, u_volt: u64, level: u32, turbo: bool) -> Self {
+        Self(bindings::dev_pm_opp_data {
+            turbo,
+            freq,
+            u_volt,
+            level,
+        })
+    }
+
+    /// Adds an OPP dynamically. The OPP is freed once the [`Token`] gets freed.
+    pub fn add_opp(self, dev: &ARef<Device>) -> Result<Token> {
+        Token::new(dev, self)
+    }
+
+    fn freq(&self) -> u64 {
+        self.0.freq
+    }
+}
+
+/// Operating performance point (OPP).
+///
+/// # Invariants
+///
+/// The pointer stored in `Self` is non-null and valid for the lifetime of the ARef instance. In
+/// particular, the ARef instance owns an increment on underlying object’s reference count.
+#[repr(transparent)]
+pub struct OPP(Opaque<bindings::dev_pm_opp>);
+
+// SAFETY: `OPP` only holds a pointer to a C OPP, which is safe to be used from any thread.
+unsafe impl Send for OPP {}
+
+// SAFETY: `OPP` only holds a pointer to a C OPP, references to which are safe to be used from any
+// thread.
+unsafe impl Sync for OPP {}
+
+// SAFETY: The type invariants guarantee that [`OPP`] is always refcounted.
+unsafe impl AlwaysRefCounted for OPP {
+    fn inc_ref(&self) {
+        // SAFETY: The existence of a shared reference means that the refcount is nonzero.
+        unsafe { bindings::dev_pm_opp_get(self.0.get()) };
+    }
+
+    unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
+        // SAFETY: The safety requirements guarantee that the refcount is nonzero.
+        unsafe { bindings::dev_pm_opp_put(obj.cast().as_ptr()) }
+    }
+}
+
+impl OPP {
+    /// Creates a reference to a [`OPP`] from a valid pointer.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that `ptr` is valid and remains valid for the lifetime of the
+    /// returned [`OPP`] reference.
+    pub unsafe fn from_raw_opp_owned<'a>(ptr: *mut bindings::dev_pm_opp) -> Result<&'a Self> {
+        // SAFETY: The caller guarantees that the pointer is not dangling
+        // and stays valid for the duration of 'a. The cast is okay because
+        // `OPP` is `repr(transparent)`.
+        Ok(unsafe { &*ptr.cast() })
+    }
+
+    /// Creates a reference to a [`OPP`] from a valid pointer.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that `ptr` is valid and remains valid for the lifetime of the
+    /// returned [`OPP`] reference.
+    pub unsafe fn from_raw_opp<'a>(ptr: *mut bindings::dev_pm_opp) -> Result<&'a Self> {
+        let opp = unsafe { Self::from_raw_opp_owned(ptr) }?;
+
+        // Take an extra reference to the OPP since the caller didn't take it.
+        opp.inc_ref();
+        Ok(opp)
+    }
+
+    #[inline]
+    fn as_raw(&self) -> *mut bindings::dev_pm_opp {
+        self.0.get()
+    }
+
+    /// Returns the frequency of an OPP.
+    pub fn freq(&self, index: Option<u32>) -> u64 {
+        let index = index.unwrap_or(0);
+
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // use it.
+        unsafe { bindings::dev_pm_opp_get_freq_indexed(self.as_raw(), index) }
+    }
+
+    /// Returns the voltage of an OPP.
+    pub fn voltage(&self) -> u64 {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // use it.
+        unsafe { bindings::dev_pm_opp_get_voltage(self.as_raw()) }
+    }
+
+    /// Returns the level of an OPP.
+    pub fn level(&self) -> u32 {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // use it.
+        unsafe { bindings::dev_pm_opp_get_level(self.as_raw()) }
+    }
+
+    /// Returns the power of an OPP.
+    pub fn power(&self) -> u64 {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // use it.
+        unsafe { bindings::dev_pm_opp_get_power(self.as_raw()) }
+    }
+
+    /// Returns the required pstate of an OPP.
+    pub fn required_pstate(&self, index: u32) -> u32 {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // use it.
+        unsafe { bindings::dev_pm_opp_get_required_pstate(self.as_raw(), index) }
+    }
+
+    /// Returns true if the OPP is turbo.
+    pub fn is_turbo(&self) -> bool {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // use it.
+        unsafe { bindings::dev_pm_opp_is_turbo(self.as_raw()) }
+    }
+}
+
+impl Drop for OPP {
+    fn drop(&mut self) {
+        // SAFETY: The safety requirements guarantee that the refcount is nonzero.
+        unsafe { bindings::dev_pm_opp_put(self.as_raw()) }
+    }
+}
-- 
2.31.1.272.g89b43f80a514

Re: [RFC PATCH V3 1/8] rust: Add initial bindings for OPP framework

[Boqun Feng]

Hi Viresh,

On Wed, Jul 03, 2024 at 12:44:26PM +0530, Viresh Kumar wrote:
> This commit adds initial Rust bindings for the Operating performance
> points (OPP) core. This adds bindings for `struct dev_pm_opp` and
> `struct dev_pm_opp_data` to begin with.
> 
> Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
[...]
> +
> +/// Operating performance point (OPP).
> +///
> +/// # Invariants
> +///
> +/// The pointer stored in `Self` is non-null and valid for the lifetime of the ARef instance. In
> +/// particular, the ARef instance owns an increment on underlying object´s reference count.

Since you use `ARef` pattern now, you may want to rewrite this
"invariants".

> +#[repr(transparent)]
> +pub struct OPP(Opaque<bindings::dev_pm_opp>);
> +
> +// SAFETY: `OPP` only holds a pointer to a C OPP, which is safe to be used from any thread.
> +unsafe impl Send for OPP {}
> +
> +// SAFETY: `OPP` only holds a pointer to a C OPP, references to which are safe to be used from any
> +// thread.
> +unsafe impl Sync for OPP {}
> +

Same for the above safety comments, as they are still based on the old
implementation.

> +// SAFETY: The type invariants guarantee that [`OPP`] is always refcounted.
> +unsafe impl AlwaysRefCounted for OPP {
> +    fn inc_ref(&self) {
> +        // SAFETY: The existence of a shared reference means that the refcount is nonzero.
> +        unsafe { bindings::dev_pm_opp_get(self.0.get()) };
> +    }
> +
> +    unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
> +        // SAFETY: The safety requirements guarantee that the refcount is nonzero.
> +        unsafe { bindings::dev_pm_opp_put(obj.cast().as_ptr()) }
> +    }
> +}
> +
> +impl OPP {
[...]
> +
> +impl Drop for OPP {

I don't think you need the `drop` implementation here, since it should
be already handled by `impl AlwaysRefCounted`, could you try to a doc
test for this? Something like:

	let opp: ARef<OPP> = <from a raw dev_pm_opp ponter whose refcount is 1>
	drop(opp);

IIUC, this will result double-free with the current implementation.

Overall, `OPP` is now representing to the actual device instead of the
pointer to the device, so the `drop` function won't need to handle the
refcounting.

Regards,
Boqun

> +    fn drop(&mut self) {
> +        // SAFETY: The safety requirements guarantee that the refcount is nonzero.
> +        unsafe { bindings::dev_pm_opp_put(self.as_raw()) }
> +    }
> +}
> -- 
> 2.31.1.272.g89b43f80a514
> 

Re: [RFC PATCH V3 1/8] rust: Add initial bindings for OPP framework

[Viresh Kumar]

Hi Boqun,

On 03-07-24, 08:34, Boqun Feng wrote:
> On Wed, Jul 03, 2024 at 12:44:26PM +0530, Viresh Kumar wrote:
> > +/// Operating performance point (OPP).
> > +///
> > +/// # Invariants
> > +///
> > +/// The pointer stored in `Self` is non-null and valid for the lifetime of the ARef instance. In
> > +/// particular, the ARef instance owns an increment on underlying object´s reference count.
> 
> Since you use `ARef` pattern now, you may want to rewrite this
> "invariants".

I copied it from the device's documentation. What all details should I
be writing here ? A link to some other implementation would be useful.

> > +impl Drop for OPP {
> 
> I don't think you need the `drop` implementation here, since it should
> be already handled by `impl AlwaysRefCounted`,

Right.

> could you try to a doc
> test for this? Something like:
> 
> 	let opp: ARef<OPP> = <from a raw dev_pm_opp ponter whose refcount is 1>
> 	drop(opp);

I now tested it with a kernel test to see what's going on internally

> IIUC, this will result double-free with the current implementation.

Quite the opposite actually. I am getting double get and a single put :)

Thanks a lot for pointing me to this direction as I have found that my
implementation was incorrect. This is how I understand it, I can be
wrong since I am okayish with Rust:

- What's getting returned from `from_raw_opp/from_raw_opp_owned` is a
  reference: `<&'a Self>`.

- Since this is a reference, when it gets out of scope, nothing
  happens. i.e. the `drop()` fn of `struct OPP` never gets called for
  the OPP object, as there is no real OPP object, but just a
  reference.

- When this gets converted to an `ARef` object (implicit typecasting),
  we increment the count. And when that gets dropped, we decrement it.
  But Apart from an `ARef` object, only the reference to the OPP gets
  dropped and hence again, drop() doesn't get called.

- The important part here is that `from_raw_opp()` shouldn't be
  incrementing the refcount, as drop() will never get called. And
  since we reach here from the C implementation, the OPP will remain
  valid for the function call.

- On the other hand, I can't return <&'a Self> from
  from_raw_opp_owned() anymore. In this case the OPP core has already
  incremented the refcount of the OPP (while it searched the OPP on
  behalf of the Rust code). Whatever is returned here, must drop the
  refcount when it goes out of scope. Also the returned OPP reference
  can live for a longer period of time in this case, since the call
  originates from the Rust side. So, it needs to be an explicit
  conversion to ARef which won't increment the refcount, but just
  decrement when the ARef gets out of scope.

Here is the diff that I need:

diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
index aaf220e6aeac..a99950b4d835 100644
--- a/rust/kernel/opp.rs
+++ b/rust/kernel/opp.rs
@@ -692,7 +692,7 @@ pub fn opp_from_freq(
         })?;
 
         // SAFETY: The `ptr` is guaranteed by the C code to be valid.
-        Ok(unsafe { OPP::from_raw_opp_owned(ptr)?.into() })
+        unsafe { OPP::from_raw_opp_owned(ptr) }
     }
 
     /// Finds OPP based on level.
@@ -718,7 +718,7 @@ pub fn opp_from_level(&self, mut level: u32, stype: SearchType) -> Result<ARef<O
         })?;
 
         // SAFETY: The `ptr` is guaranteed by the C code to be valid.
-        Ok(unsafe { OPP::from_raw_opp_owned(ptr)?.into() })
+        unsafe { OPP::from_raw_opp_owned(ptr) }
     }
 
     /// Finds OPP based on bandwidth.
@@ -743,7 +743,7 @@ pub fn opp_from_bw(&self, mut bw: u32, index: i32, stype: SearchType) -> Result<
         })?;
 
         // SAFETY: The `ptr` is guaranteed by the C code to be valid.
-        Ok(unsafe { OPP::from_raw_opp_owned(ptr)?.into() })
+        unsafe { OPP::from_raw_opp_owned(ptr) }
     }
 
     /// Enable the OPP.
@@ -834,31 +834,33 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
 }
 
 impl OPP {
-    /// Creates a reference to a [`OPP`] from a valid pointer.
+    /// Creates an owned reference to a [`OPP`] from a valid pointer.
     ///
     /// # Safety
     ///
-    /// The caller must ensure that `ptr` is valid and remains valid for the lifetime of the
-    /// returned [`OPP`] reference.
-    pub unsafe fn from_raw_opp_owned<'a>(ptr: *mut bindings::dev_pm_opp) -> Result<&'a Self> {
-        // SAFETY: The caller guarantees that the pointer is not dangling
-        // and stays valid for the duration of 'a. The cast is okay because
-        // `OPP` is `repr(transparent)`.
-        Ok(unsafe { &*ptr.cast() })
+    /// The caller must ensure that `ptr` is valid and OPP's refcount is incremented. The refcount
+    /// will be decremented by `dec_ref` when the ARef object is dropped.
+    pub unsafe fn from_raw_opp_owned(ptr: *mut bindings::dev_pm_opp) -> Result<ARef<Self>> {
+        let ptr = ptr::NonNull::new(ptr).ok_or(ENODEV)?;
+
+        // SAFETY: The safety requirements guarantee the validity of the pointer.
+        //
+        // INVARIANT: The refcount is already incremented by the C API that returned the pointer,
+        // and we pass ownership of the refcount to the new `ARef<OPP>`.
+        Ok(unsafe { ARef::from_raw(ptr.cast()) })
     }
 
     /// Creates a reference to a [`OPP`] from a valid pointer.
     ///
     /// # Safety
     ///
-    /// The caller must ensure that `ptr` is valid and remains valid for the lifetime of the
-    /// returned [`OPP`] reference.
+    /// The caller must ensure that `ptr` is valid and remains valid for the duration of 'a. The
+    /// refcount is not updated by the Rust API unless the returned reference is converted to an
+    /// ARef object.
     pub unsafe fn from_raw_opp<'a>(ptr: *mut bindings::dev_pm_opp) -> Result<&'a Self> {
-        let opp = unsafe { Self::from_raw_opp_owned(ptr) }?;
-
-        // Take an extra reference to the OPP since the caller didn't take it.
-        opp.inc_ref();
-        Ok(opp)
+        // SAFETY: The caller guarantees that the pointer is not dangling and stays valid for the
+        // duration of 'a. The cast is okay because `OPP` is `repr(transparent)`.
+        Ok(unsafe { &*ptr.cast() })
     }
 
     #[inline]
@@ -910,10 +912,3 @@ pub fn is_turbo(&self) -> bool {
         unsafe { bindings::dev_pm_opp_is_turbo(self.as_raw()) }
     }
 }
-
-impl Drop for OPP {
-    fn drop(&mut self) {
-        // SAFETY: The safety requirements guarantee that the refcount is nonzero.
-        unsafe { bindings::dev_pm_opp_put(self.as_raw()) }
-    }
-}

Makes sense ?

-- 
viresh

Re: [RFC PATCH V3 1/8] rust: Add initial bindings for OPP framework

[Boqun Feng]

On Fri, Jul 05, 2024 at 04:32:28PM +0530, Viresh Kumar wrote:
> Hi Boqun,
> 
> On 03-07-24, 08:34, Boqun Feng wrote:
> > On Wed, Jul 03, 2024 at 12:44:26PM +0530, Viresh Kumar wrote:
> > > +/// Operating performance point (OPP).
> > > +///
> > > +/// # Invariants
> > > +///
> > > +/// The pointer stored in `Self` is non-null and valid for the lifetime of the ARef instance. In
> > > +/// particular, the ARef instance owns an increment on underlying object´s reference count.
> > 
> > Since you use `ARef` pattern now, you may want to rewrite this
> > "invariants".
> 
> I copied it from the device's documentation. What all details should I
> be writing here ? A link to some other implementation would be useful.
> 

"Invariants" defines "what's a valid instance of a type", so here I
think you could drop the "# Invariants" section at all, unless you need
to use some of the invariants of fields in `dev_mp_opp` as a
justification for safety. For example if you have a pointer in
`dev_mp_opp`, and it's always pointing to a valid `T`, and in one method
of `OPP`, you need to dereference the pointer.

> > > +impl Drop for OPP {
> > 
> > I don't think you need the `drop` implementation here, since it should
> > be already handled by `impl AlwaysRefCounted`,
> 
> Right.
> 
> > could you try to a doc
> > test for this? Something like:
> > 
> > 	let opp: ARef<OPP> = <from a raw dev_pm_opp ponter whose refcount is 1>
> > 	drop(opp);
> 
> I now tested it with a kernel test to see what's going on internally
> 
> > IIUC, this will result double-free with the current implementation.
> 
> Quite the opposite actually. I am getting double get and a single put :)
> 
> Thanks a lot for pointing me to this direction as I have found that my
> implementation was incorrect. This is how I understand it, I can be
> wrong since I am okayish with Rust:
> 
> - What's getting returned from `from_raw_opp/from_raw_opp_owned` is a
>   reference: `<&'a Self>`.
> 
> - Since this is a reference, when it gets out of scope, nothing
>   happens. i.e. the `drop()` fn of `struct OPP` never gets called for
>   the OPP object, as there is no real OPP object, but just a
>   reference.
> 
> - When this gets converted to an `ARef` object (implicit typecasting),
>   we increment the count. And when that gets dropped, we decrement it.
>   But Apart from an `ARef` object, only the reference to the OPP gets
>   dropped and hence again, drop() doesn't get called.
> 
> - The important part here is that `from_raw_opp()` shouldn't be
>   incrementing the refcount, as drop() will never get called. And
>   since we reach here from the C implementation, the OPP will remain
>   valid for the function call.
> 

Right. I wasn't aware that you didn't return a `ARef<OPP>`, which mean
the return value won't handle the refcounting automatically (and because
of that, the refcount shouldn't be increased.)

> - On the other hand, I can't return <&'a Self> from
>   from_raw_opp_owned() anymore. In this case the OPP core has already
>   incremented the refcount of the OPP (while it searched the OPP on
>   behalf of the Rust code). Whatever is returned here, must drop the
>   refcount when it goes out of scope. Also the returned OPP reference
>   can live for a longer period of time in this case, since the call
>   originates from the Rust side. So, it needs to be an explicit
>   conversion to ARef which won't increment the refcount, but just
>   decrement when the ARef gets out of scope.
> 

I think you got it correct ;-) The takeaway is: there are different
types of pointers/references, 1) if users only use a `struct dev_pm_opp
*` shortly and the scope can be told at compile time, you can provide a
`&OPP`, 2) if the scope of usage is somewhat dynamic, and the users
should descrease the refcount after use it, the API should return a
`ARef<OPP>`. And since the refcount inc/dec are already maintained in
`ARef<_>`, `OPP::drop` shouldn't touch the refcount anymore.

Also as you already noticed, calling `into` on a `&OPP` will give a
`ARef<OPP>`, which includes an increment of the refcount, and usually
should be used when the users want to switch to long-term usage after a
quick short-term use (e.g. do a quick check of the status and decide
some more work is needed, and maybe need to transfer the ownership of
the pointer to a workqueue or something).

> Here is the diff that I need:
> 
> diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
> index aaf220e6aeac..a99950b4d835 100644
> --- a/rust/kernel/opp.rs
> +++ b/rust/kernel/opp.rs
> @@ -692,7 +692,7 @@ pub fn opp_from_freq(
>          })?;
>  
>          // SAFETY: The `ptr` is guaranteed by the C code to be valid.
> -        Ok(unsafe { OPP::from_raw_opp_owned(ptr)?.into() })
> +        unsafe { OPP::from_raw_opp_owned(ptr) }
>      }
>  
>      /// Finds OPP based on level.
> @@ -718,7 +718,7 @@ pub fn opp_from_level(&self, mut level: u32, stype: SearchType) -> Result<ARef<O
>          })?;
>  
>          // SAFETY: The `ptr` is guaranteed by the C code to be valid.
> -        Ok(unsafe { OPP::from_raw_opp_owned(ptr)?.into() })
> +        unsafe { OPP::from_raw_opp_owned(ptr) }
>      }
>  
>      /// Finds OPP based on bandwidth.
> @@ -743,7 +743,7 @@ pub fn opp_from_bw(&self, mut bw: u32, index: i32, stype: SearchType) -> Result<
>          })?;
>  
>          // SAFETY: The `ptr` is guaranteed by the C code to be valid.
> -        Ok(unsafe { OPP::from_raw_opp_owned(ptr)?.into() })
> +        unsafe { OPP::from_raw_opp_owned(ptr) }
>      }
>  
>      /// Enable the OPP.
> @@ -834,31 +834,33 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
>  }
>  
>  impl OPP {
> -    /// Creates a reference to a [`OPP`] from a valid pointer.
> +    /// Creates an owned reference to a [`OPP`] from a valid pointer.
>      ///
>      /// # Safety
>      ///
> -    /// The caller must ensure that `ptr` is valid and remains valid for the lifetime of the
> -    /// returned [`OPP`] reference.
> -    pub unsafe fn from_raw_opp_owned<'a>(ptr: *mut bindings::dev_pm_opp) -> Result<&'a Self> {
> -        // SAFETY: The caller guarantees that the pointer is not dangling
> -        // and stays valid for the duration of 'a. The cast is okay because
> -        // `OPP` is `repr(transparent)`.
> -        Ok(unsafe { &*ptr.cast() })
> +    /// The caller must ensure that `ptr` is valid and OPP's refcount is incremented. The refcount
> +    /// will be decremented by `dec_ref` when the ARef object is dropped.

Usually the second sentense "The refcount ..." won't need to be put in
the safety requirement, as it just describes how ARef works.

> +    pub unsafe fn from_raw_opp_owned(ptr: *mut bindings::dev_pm_opp) -> Result<ARef<Self>> {
> +        let ptr = ptr::NonNull::new(ptr).ok_or(ENODEV)?;
> +
> +        // SAFETY: The safety requirements guarantee the validity of the pointer.
> +        //
> +        // INVARIANT: The refcount is already incremented by the C API that returned the pointer,
> +        // and we pass ownership of the refcount to the new `ARef<OPP>`.

You can probably drop the "INVARIANT", as it's an invariant of `ARef`
which already guarantees since the safety requirement of
`ARef::from_raw()` meets. At least you can write them as "normal"
comments.

> +        Ok(unsafe { ARef::from_raw(ptr.cast()) })
>      }
>  
>      /// Creates a reference to a [`OPP`] from a valid pointer.
>      ///
>      /// # Safety
>      ///
> -    /// The caller must ensure that `ptr` is valid and remains valid for the lifetime of the
> -    /// returned [`OPP`] reference.
> +    /// The caller must ensure that `ptr` is valid and remains valid for the duration of 'a. The
> +    /// refcount is not updated by the Rust API unless the returned reference is converted to an
> +    /// ARef object.

Again you could drop the second sentence, or you can put it somewhere
outside the "Safety" section, if you want to.

>      pub unsafe fn from_raw_opp<'a>(ptr: *mut bindings::dev_pm_opp) -> Result<&'a Self> {
> -        let opp = unsafe { Self::from_raw_opp_owned(ptr) }?;
> -
> -        // Take an extra reference to the OPP since the caller didn't take it.
> -        opp.inc_ref();
> -        Ok(opp)
> +        // SAFETY: The caller guarantees that the pointer is not dangling and stays valid for the
> +        // duration of 'a. The cast is okay because `OPP` is `repr(transparent)`.
> +        Ok(unsafe { &*ptr.cast() })
>      }
>  
>      #[inline]
> @@ -910,10 +912,3 @@ pub fn is_turbo(&self) -> bool {
>          unsafe { bindings::dev_pm_opp_is_turbo(self.as_raw()) }
>      }
>  }
> -
> -impl Drop for OPP {
> -    fn drop(&mut self) {
> -        // SAFETY: The safety requirements guarantee that the refcount is nonzero.
> -        unsafe { bindings::dev_pm_opp_put(self.as_raw()) }
> -    }
> -}
> 
> Makes sense ?
> 

Yep, looks good to me!

Regards,
Boqun

> -- 
> viresh

Re: [RFC PATCH V3 1/8] rust: Add initial bindings for OPP framework

[Viresh Kumar]

On 03-07-24, 08:34, Boqun Feng wrote:
> On Wed, Jul 03, 2024 at 12:44:26PM +0530, Viresh Kumar wrote:
> > +// SAFETY: `OPP` only holds a pointer to a C OPP, which is safe to be used from any thread.
> > +unsafe impl Send for OPP {}
> > +
> > +// SAFETY: `OPP` only holds a pointer to a C OPP, references to which are safe to be used from any
> > +// thread.
> > +unsafe impl Sync for OPP {}
> > +
> 
> Same for the above safety comments, as they are still based on the old
> implementation.

Do I still need to change these ? Since we aren't always using ARef
now.

-- 
viresh

Re: [RFC PATCH V3 1/8] rust: Add initial bindings for OPP framework

[Boqun Feng]

On Tue, Jul 09, 2024 at 04:32:45PM +0530, Viresh Kumar wrote:
> On 03-07-24, 08:34, Boqun Feng wrote:
> > On Wed, Jul 03, 2024 at 12:44:26PM +0530, Viresh Kumar wrote:
> > > +// SAFETY: `OPP` only holds a pointer to a C OPP, which is safe to be used from any thread.
> > > +unsafe impl Send for OPP {}
> > > +
> > > +// SAFETY: `OPP` only holds a pointer to a C OPP, references to which are safe to be used from any
> > > +// thread.
> > > +unsafe impl Sync for OPP {}
> > > +
> > 
> > Same for the above safety comments, as they are still based on the old
> > implementation.
> 
> Do I still need to change these ? Since we aren't always using ARef
> now.
> 

Correct, you will still need to change these. You're welcome to submit
a draft version here and I can help take a look before you send out a
whole new version, if you prefer that way.

Regards,
Boqun

> -- 
> viresh

Re: [RFC PATCH V3 1/8] rust: Add initial bindings for OPP framework

[Viresh Kumar]

On 09-07-24, 10:45, Boqun Feng wrote:
> On Tue, Jul 09, 2024 at 04:32:45PM +0530, Viresh Kumar wrote:
> > On 03-07-24, 08:34, Boqun Feng wrote:
> > > On Wed, Jul 03, 2024 at 12:44:26PM +0530, Viresh Kumar wrote:
> > > > +// SAFETY: `OPP` only holds a pointer to a C OPP, which is safe to be used from any thread.
> > > > +unsafe impl Send for OPP {}
> > > > +
> > > > +// SAFETY: `OPP` only holds a pointer to a C OPP, references to which are safe to be used from any
> > > > +// thread.
> > > > +unsafe impl Sync for OPP {}
> > > > +
> > > 
> > > Same for the above safety comments, as they are still based on the old
> > > implementation.
> > 
> > Do I still need to change these ? Since we aren't always using ARef
> > now.
> > 
> 
> Correct, you will still need to change these. You're welcome to submit
> a draft version here and I can help take a look before you send out a
> whole new version, if you prefer that way.

I am not entirely sure what the change must be like that :)

-- 
viresh

Re: [RFC PATCH V3 1/8] rust: Add initial bindings for OPP framework

[Viresh Kumar]

On 10-07-24, 13:06, Viresh Kumar wrote:
> I am not entirely sure what the change must be like that :)

Anyway, I have looked around and made some changes. Please see how it
looks now. Thanks Boqun.

diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
new file mode 100644
index 000000000000..2ef262c4640a
--- /dev/null
+++ b/rust/kernel/opp.rs
@@ -0,0 +1,188 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Operating performance points.
+//!
+//! This module provides bindings for interacting with the OPP subsystem.
+//!
+//! C header: [`include/linux/pm_opp.h`](srctree/include/linux/pm_opp.h)
+
+use crate::{
+    bindings,
+    device::Device,
+    error::{code::*, to_result, Result},
+    types::{ARef, AlwaysRefCounted, Opaque},
+};
+
+use core::ptr;
+
+/// Dynamically created Operating performance point (OPP).
+pub struct Token {
+    dev: ARef<Device>,
+    freq: u64,
+}
+
+impl Token {
+    /// Adds an OPP dynamically.
+    pub fn new(dev: &ARef<Device>, mut data: Data) -> Result<Self> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        to_result(unsafe { bindings::dev_pm_opp_add_dynamic(dev.as_raw(), &mut data.0) })?;
+        Ok(Self {
+            dev: dev.clone(),
+            freq: data.freq(),
+        })
+    }
+}
+
+impl Drop for Token {
+    fn drop(&mut self) {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        unsafe { bindings::dev_pm_opp_remove(self.dev.as_raw(), self.freq) };
+    }
+}
+
+/// Equivalent to `struct dev_pm_opp_data` in the C Code.
+#[repr(transparent)]
+pub struct Data(bindings::dev_pm_opp_data);
+
+impl Data {
+    /// Creates new instance of [`Data`].
+    pub fn new(freq: u64, u_volt: u64, level: u32, turbo: bool) -> Self {
+        Self(bindings::dev_pm_opp_data {
+            turbo,
+            freq,
+            u_volt,
+            level,
+        })
+    }
+
+    /// Adds an OPP dynamically. The OPP is freed once the [`Token`] gets freed.
+    pub fn add_opp(self, dev: &ARef<Device>) -> Result<Token> {
+        Token::new(dev, self)
+    }
+
+    fn freq(&self) -> u64 {
+        self.0.freq
+    }
+}
+
+/// Operating performance point (OPP).
+///
+/// Wraps the kernel's `struct dev_pm_opp`.
+///
+/// The pointer stored in `Self` is non-null and valid for the lifetime of the `OPP` instance.
+///
+/// # Refcounting
+///
+/// Instances of this type are reference-counted. The reference count is incremented by the
+/// `dev_pm_opp_get()` function and decremented by `dev_pm_opp_put`. The Rust type `ARef<OPP>`
+/// represents a pointer that owns a reference count on the OPP.
+///
+/// A reference to the `OPP`, `&OPP` isn't refcounted by the Rust code. The C code guarantees that
+/// the pointer stored in `OPP` is is valid for the lifetime of the reference and hence refcounting
+/// isn't required.
+
+#[repr(transparent)]
+pub struct OPP(Opaque<bindings::dev_pm_opp>);
+
+// SAFETY: It is okay to send ownership of `OPP` across thread boundaries and `OPP::dec_ref` can be
+// called from any thread.
+unsafe impl Send for OPP {}
+
+// SAFETY: It's OK to access `OPP` through shared references from other threads because we're
+// either accessing properties that don't change or that are properly synchronised by C code.
+unsafe impl Sync for OPP {}
+
+// SAFETY: The type invariants guarantee that [`OPP`] is always refcounted.
+unsafe impl AlwaysRefCounted for OPP {
+    fn inc_ref(&self) {
+        // SAFETY: The existence of a shared reference means that the refcount is nonzero.
+        unsafe { bindings::dev_pm_opp_get(self.0.get()) };
+    }
+
+    unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
+        // SAFETY: The safety requirements guarantee that the refcount is nonzero.
+        unsafe { bindings::dev_pm_opp_put(obj.cast().as_ptr()) }
+    }
+}
+
+impl OPP {
+    /// Creates an owned reference to a [`OPP`] from a valid pointer.
+    ///
+    /// The refcount is incremented by the C code and will be decremented by `dec_ref()` when the
+    /// ARef object is dropped.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that `ptr` is valid and OPP's refcount is incremented.
+    pub unsafe fn from_raw_opp_owned(ptr: *mut bindings::dev_pm_opp) -> Result<ARef<Self>> {
+        let ptr = ptr::NonNull::new(ptr).ok_or(ENODEV)?;
+
+        // SAFETY: The safety requirements guarantee the validity of the pointer.
+        Ok(unsafe { ARef::from_raw(ptr.cast()) })
+    }
+
+    /// Creates a reference to a [`OPP`] from a valid pointer.
+    ///
+    /// The refcount is not updated by the Rust API unless the returned reference is converted to
+    /// an ARef object.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that `ptr` is valid and remains valid for the duration of 'a.
+    pub unsafe fn from_raw_opp<'a>(ptr: *mut bindings::dev_pm_opp) -> Result<&'a Self> {
+        // SAFETY: The caller guarantees that the pointer is not dangling and stays valid for the
+        // duration of 'a. The cast is okay because `OPP` is `repr(transparent)`.
+        Ok(unsafe { &*ptr.cast() })
+    }
+
+    #[inline]
+    fn as_raw(&self) -> *mut bindings::dev_pm_opp {
+        self.0.get()
+    }
+
+    /// Returns the frequency of an OPP.
+    pub fn freq(&self, index: Option<u32>) -> u64 {
+        let index = index.unwrap_or(0);
+
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // use it.
+        unsafe { bindings::dev_pm_opp_get_freq_indexed(self.as_raw(), index) }
+    }
+
+    /// Returns the voltage of an OPP.
+    pub fn voltage(&self) -> u64 {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // use it.
+        unsafe { bindings::dev_pm_opp_get_voltage(self.as_raw()) }
+    }
+
+    /// Returns the level of an OPP.
+    pub fn level(&self) -> u32 {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // use it.
+        unsafe { bindings::dev_pm_opp_get_level(self.as_raw()) }
+    }
+
+    /// Returns the power of an OPP.
+    pub fn power(&self) -> u64 {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // use it.
+        unsafe { bindings::dev_pm_opp_get_power(self.as_raw()) }
+    }
+
+    /// Returns the required pstate of an OPP.
+    pub fn required_pstate(&self, index: u32) -> u32 {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // use it.
+        unsafe { bindings::dev_pm_opp_get_required_pstate(self.as_raw(), index) }
+    }
+
+    /// Returns true if the OPP is turbo.
+    pub fn is_turbo(&self) -> bool {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // use it.
+        unsafe { bindings::dev_pm_opp_is_turbo(self.as_raw()) }
+    }
+}

Re: [RFC PATCH V3 1/8] rust: Add initial bindings for OPP framework

[Boqun Feng]

On Wed, Jul 10, 2024 at 04:36:07PM +0530, Viresh Kumar wrote:
> On 10-07-24, 13:06, Viresh Kumar wrote:
> > I am not entirely sure what the change must be like that :)
> 
> Anyway, I have looked around and made some changes. Please see how it
> looks now. Thanks Boqun.
> 
> diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
> new file mode 100644
> index 000000000000..2ef262c4640a
> --- /dev/null
> +++ b/rust/kernel/opp.rs
> @@ -0,0 +1,188 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Operating performance points.
> +//!
> +//! This module provides bindings for interacting with the OPP subsystem.
> +//!
> +//! C header: [`include/linux/pm_opp.h`](srctree/include/linux/pm_opp.h)
> +
> +use crate::{
> +    bindings,
> +    device::Device,
> +    error::{code::*, to_result, Result},
> +    types::{ARef, AlwaysRefCounted, Opaque},
> +};
> +
> +use core::ptr;
> +
> +/// Dynamically created Operating performance point (OPP).
> +pub struct Token {
> +    dev: ARef<Device>,
> +    freq: u64,
> +}
> +
> +impl Token {
> +    /// Adds an OPP dynamically.
> +    pub fn new(dev: &ARef<Device>, mut data: Data) -> Result<Self> {
> +        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
> +        // requirements.
> +        to_result(unsafe { bindings::dev_pm_opp_add_dynamic(dev.as_raw(), &mut data.0) })?;
> +        Ok(Self {
> +            dev: dev.clone(),
> +            freq: data.freq(),
> +        })
> +    }
> +}
> +
> +impl Drop for Token {
> +    fn drop(&mut self) {
> +        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
> +        // requirements.
> +        unsafe { bindings::dev_pm_opp_remove(self.dev.as_raw(), self.freq) };
> +    }
> +}
> +
> +/// Equivalent to `struct dev_pm_opp_data` in the C Code.
> +#[repr(transparent)]
> +pub struct Data(bindings::dev_pm_opp_data);
> +
> +impl Data {
> +    /// Creates new instance of [`Data`].
> +    pub fn new(freq: u64, u_volt: u64, level: u32, turbo: bool) -> Self {
> +        Self(bindings::dev_pm_opp_data {
> +            turbo,
> +            freq,
> +            u_volt,
> +            level,
> +        })
> +    }
> +
> +    /// Adds an OPP dynamically. The OPP is freed once the [`Token`] gets freed.
> +    pub fn add_opp(self, dev: &ARef<Device>) -> Result<Token> {
> +        Token::new(dev, self)
> +    }
> +
> +    fn freq(&self) -> u64 {
> +        self.0.freq
> +    }
> +}
> +
> +/// Operating performance point (OPP).
> +///
> +/// Wraps the kernel's `struct dev_pm_opp`.
> +///
> +/// The pointer stored in `Self` is non-null and valid for the lifetime of the `OPP` instance.
> +///
> +/// # Refcounting
> +///
> +/// Instances of this type are reference-counted. The reference count is incremented by the
> +/// `dev_pm_opp_get()` function and decremented by `dev_pm_opp_put`. The Rust type `ARef<OPP>`
> +/// represents a pointer that owns a reference count on the OPP.
> +///
> +/// A reference to the `OPP`, `&OPP` isn't refcounted by the Rust code. The C code guarantees that
> +/// the pointer stored in `OPP` is is valid for the lifetime of the reference and hence refcounting

"the pointer stored in `OPP`" is incorrect, I think what you tried to
say here is "the C code guarantees a pointer to `OPP` is valid with at
lease one reference count for the lifetime of the `&OPP`". But this
comment can be avoided at all since it's generally true for most
references. It's OK if you want to write this here as a special note.

> +/// isn't required.
> +
> +#[repr(transparent)]
> +pub struct OPP(Opaque<bindings::dev_pm_opp>);
> +
> +// SAFETY: It is okay to send ownership of `OPP` across thread boundaries and `OPP::dec_ref` can be
> +// called from any thread.

Whether `OPP::dec_ref` can be called from any thread is unrelated to
whether `OPP` is `Send` or not. `Send` means you could own an `OPP`
(instead of an `ARef<OPP>`) that's created by other thread/context, as
long as `Opp::drop` is safe to call from a different thread (other than
the one that creates it), it should be safe to send.

> +unsafe impl Send for OPP {}
> +
> +// SAFETY: It's OK to access `OPP` through shared references from other threads because we're
> +// either accessing properties that don't change or that are properly synchronised by C code.

LGTM.

> +unsafe impl Sync for OPP {}
> +
> +// SAFETY: The type invariants guarantee that [`OPP`] is always refcounted.

Since you use "type invariants" here, you should rename the "# Refcounting"
section before "OPP" as "# Invariants".

> +unsafe impl AlwaysRefCounted for OPP {
> +    fn inc_ref(&self) {
> +        // SAFETY: The existence of a shared reference means that the refcount is nonzero.
> +        unsafe { bindings::dev_pm_opp_get(self.0.get()) };
> +    }
> +
> +    unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
> +        // SAFETY: The safety requirements guarantee that the refcount is nonzero.
> +        unsafe { bindings::dev_pm_opp_put(obj.cast().as_ptr()) }
> +    }
> +}
> +
> +impl OPP {
> +    /// Creates an owned reference to a [`OPP`] from a valid pointer.
> +    ///
> +    /// The refcount is incremented by the C code and will be decremented by `dec_ref()` when the
> +    /// ARef object is dropped.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The caller must ensure that `ptr` is valid and OPP's refcount is incremented.

One part is missing in this safety requirement, the caller needs to
guarantee "forget"ing one reference count of the object, because that's
owned by the returned value, see the safety requirement of
`ARef::from_raw()` for more informatoin.

Regards,
Boqun

> +    pub unsafe fn from_raw_opp_owned(ptr: *mut bindings::dev_pm_opp) -> Result<ARef<Self>> {
> +        let ptr = ptr::NonNull::new(ptr).ok_or(ENODEV)?;
> +
> +        // SAFETY: The safety requirements guarantee the validity of the pointer.
> +        Ok(unsafe { ARef::from_raw(ptr.cast()) })
> +    }
> +
> +    /// Creates a reference to a [`OPP`] from a valid pointer.
> +    ///
> +    /// The refcount is not updated by the Rust API unless the returned reference is converted to
> +    /// an ARef object.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The caller must ensure that `ptr` is valid and remains valid for the duration of 'a.
> +    pub unsafe fn from_raw_opp<'a>(ptr: *mut bindings::dev_pm_opp) -> Result<&'a Self> {
> +        // SAFETY: The caller guarantees that the pointer is not dangling and stays valid for the
> +        // duration of 'a. The cast is okay because `OPP` is `repr(transparent)`.
> +        Ok(unsafe { &*ptr.cast() })
> +    }
> +
> +    #[inline]
> +    fn as_raw(&self) -> *mut bindings::dev_pm_opp {
> +        self.0.get()
> +    }
> +
> +    /// Returns the frequency of an OPP.
> +    pub fn freq(&self, index: Option<u32>) -> u64 {
> +        let index = index.unwrap_or(0);
> +
> +        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
> +        // use it.
> +        unsafe { bindings::dev_pm_opp_get_freq_indexed(self.as_raw(), index) }
> +    }
> +
> +    /// Returns the voltage of an OPP.
> +    pub fn voltage(&self) -> u64 {
> +        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
> +        // use it.
> +        unsafe { bindings::dev_pm_opp_get_voltage(self.as_raw()) }
> +    }
> +
> +    /// Returns the level of an OPP.
> +    pub fn level(&self) -> u32 {
> +        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
> +        // use it.
> +        unsafe { bindings::dev_pm_opp_get_level(self.as_raw()) }
> +    }
> +
> +    /// Returns the power of an OPP.
> +    pub fn power(&self) -> u64 {
> +        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
> +        // use it.
> +        unsafe { bindings::dev_pm_opp_get_power(self.as_raw()) }
> +    }
> +
> +    /// Returns the required pstate of an OPP.
> +    pub fn required_pstate(&self, index: u32) -> u32 {
> +        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
> +        // use it.
> +        unsafe { bindings::dev_pm_opp_get_required_pstate(self.as_raw(), index) }
> +    }
> +
> +    /// Returns true if the OPP is turbo.
> +    pub fn is_turbo(&self) -> bool {
> +        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
> +        // use it.
> +        unsafe { bindings::dev_pm_opp_is_turbo(self.as_raw()) }
> +    }
> +}

[RFC PATCH V3 2/8] rust: Extend OPP bindings for the OPP table

[Viresh Kumar]

This extends OPP bindings with the bindings for the `struct opp_table`.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 rust/kernel/opp.rs | 382 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 381 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
index b26e39a74635..4b31f99acc67 100644
--- a/rust/kernel/opp.rs
+++ b/rust/kernel/opp.rs
@@ -8,8 +8,9 @@
 
 use crate::{
     bindings,
+    cpumask::Cpumask,
     device::Device,
-    error::{code::*, to_result, Result},
+    error::{code::*, from_err_ptr, to_result, Error, Result},
     types::{ARef, AlwaysRefCounted, Opaque},
 };
 
@@ -67,6 +68,385 @@ fn freq(&self) -> u64 {
     }
 }
 
+/// OPP search types.
+#[derive(Copy, Clone, Debug, Eq, PartialEq)]
+pub enum SearchType {
+    /// Search for exact value.
+    Exact,
+    /// Search for highest value less than equal to value.
+    Floor,
+    /// Search for lowest value greater than equal to value.
+    Ceil,
+}
+
+/// Operating performance point (OPP) table.
+///
+/// # Invariants
+///
+/// The pointer stored in `Self` is non-null and valid for the lifetime of the ARef instance. In
+/// particular, the ARef instance owns an increment on underlying object’s reference count.
+pub struct Table {
+    ptr: *mut bindings::opp_table,
+    dev: ARef<Device>,
+    em: bool,
+    of: bool,
+    cpumask: Option<Cpumask>,
+}
+
+// SAFETY: The fields of `Table` are safe to be used from any thread.
+unsafe impl Send for Table {}
+
+// SAFETY: The fields of `Table` are safe to be referenced from any thread.
+unsafe impl Sync for Table {}
+
+impl Table {
+    /// Creates a new OPP table instance from raw pointer.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that `ptr` is valid and non-null.
+    unsafe fn from_raw_table(ptr: *mut bindings::opp_table, dev: &ARef<Device>) -> Self {
+        // SAFETY: By the safety requirements, ptr is valid and its refcount will be incremented.
+        unsafe { bindings::dev_pm_opp_get_opp_table_ref(ptr) };
+
+        Self {
+            ptr,
+            dev: dev.clone(),
+            em: false,
+            of: false,
+            cpumask: None,
+        }
+    }
+
+    /// Find OPP table from device.
+    pub fn from_dev(dev: &ARef<Device>) -> Result<Self> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements. Refcount of the OPP table is incremented as well.
+        let ptr = from_err_ptr(unsafe { bindings::dev_pm_opp_get_opp_table(dev.as_raw()) })?;
+
+        Ok(Self {
+            ptr,
+            dev: dev.clone(),
+            em: false,
+            of: false,
+            cpumask: None,
+        })
+    }
+
+    /// Add device tree based OPP table for the device.
+    #[cfg(CONFIG_OF)]
+    pub fn from_of(dev: &ARef<Device>, index: i32) -> Result<Self> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements. Refcount of the OPP table is incremented as well.
+        to_result(unsafe { bindings::dev_pm_opp_of_add_table_indexed(dev.as_raw(), index) })?;
+
+        // Fetch the newly created table.
+        let mut table = Self::from_dev(dev)?;
+        table.of = true;
+
+        Ok(table)
+    }
+
+    // Remove device tree based OPP table for the device.
+    #[cfg(CONFIG_OF)]
+    fn remove_of(&self) {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements. We took the reference from `from_of` earlier, it is safe to drop the same
+        // now.
+        unsafe { bindings::dev_pm_opp_of_remove_table(self.dev.as_raw()) };
+    }
+
+    /// Add device tree based OPP table for CPU devices.
+    #[cfg(CONFIG_OF)]
+    pub fn from_of_cpumask(dev: &ARef<Device>, cpumask: &Cpumask) -> Result<Self> {
+        // SAFETY: The cpumask is valid and the returned ptr will be owned by the [`Table`] instance.
+        to_result(unsafe { bindings::dev_pm_opp_of_cpumask_add_table(cpumask.as_ptr()) })?;
+
+        // Fetch the newly created table.
+        let mut table = Self::from_dev(dev)?;
+
+        let mut mask = Cpumask::new()?;
+        cpumask.copy(&mut mask);
+        table.cpumask = Some(mask);
+
+        Ok(table)
+    }
+
+    // Remove device tree based OPP table for CPU devices.
+    #[cfg(CONFIG_OF)]
+    fn remove_of_cpumask(&self, cpumask: Cpumask) {
+        // SAFETY: The cpumask is valid and we took the reference from `from_of_cpumask` earlier,
+        // it is safe to drop the same now.
+        unsafe { bindings::dev_pm_opp_of_cpumask_remove_table(cpumask.as_ptr()) };
+    }
+
+    /// Returns the number of OPPs in the table.
+    pub fn opp_count(&self) -> Result<u32> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        let ret = unsafe { bindings::dev_pm_opp_get_opp_count(self.dev.as_raw()) };
+        if ret < 0 {
+            Err(Error::from_errno(ret))
+        } else {
+            Ok(ret as u32)
+        }
+    }
+
+    /// Returns max clock latency of the OPPs in the table.
+    pub fn max_clock_latency(&self) -> u64 {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        unsafe { bindings::dev_pm_opp_get_max_clock_latency(self.dev.as_raw()) }
+    }
+
+    /// Returns max volt latency of the OPPs in the table.
+    pub fn max_volt_latency(&self) -> u64 {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        unsafe { bindings::dev_pm_opp_get_max_volt_latency(self.dev.as_raw()) }
+    }
+
+    /// Returns max transition latency of the OPPs in the table.
+    pub fn max_transition_latency(&self) -> u64 {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        unsafe { bindings::dev_pm_opp_get_max_transition_latency(self.dev.as_raw()) }
+    }
+
+    /// Returns the suspend OPP.
+    pub fn suspend_freq(&self) -> u64 {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        unsafe { bindings::dev_pm_opp_get_suspend_opp_freq(self.dev.as_raw()) }
+    }
+
+    /// Synchronizes regulators used by the OPP table.
+    pub fn sync_regulators(&self) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        to_result(unsafe { bindings::dev_pm_opp_sync_regulators(self.dev.as_raw()) })
+    }
+
+    /// Gets sharing CPUs.
+    pub fn sharing_cpus(dev: &Device, cpumask: &mut Cpumask) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        to_result(unsafe {
+            bindings::dev_pm_opp_get_sharing_cpus(dev.as_raw(), cpumask.as_mut_ptr())
+        })
+    }
+
+    /// Sets sharing CPUs.
+    pub fn set_sharing_cpus(&mut self, cpumask: &Cpumask) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        to_result(unsafe {
+            bindings::dev_pm_opp_set_sharing_cpus(self.dev.as_raw(), cpumask.as_ptr())
+        })?;
+
+        if let Some(mask) = self.cpumask.as_mut() {
+            // Update the cpumask as this will be used while removing the table.
+            cpumask.copy(mask);
+        }
+
+        Ok(())
+    }
+
+    /// Gets sharing CPUs from Device tree.
+    #[cfg(CONFIG_OF)]
+    pub fn of_sharing_cpus(dev: &Device, cpumask: &mut Cpumask) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        to_result(unsafe {
+            bindings::dev_pm_opp_of_get_sharing_cpus(dev.as_raw(), cpumask.as_mut_ptr())
+        })
+    }
+
+    /// Updates the voltage value for an OPP.
+    pub fn adjust_voltage(
+        &self,
+        freq: u64,
+        u_volt: u64,
+        u_volt_min: u64,
+        u_volt_max: u64,
+    ) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        to_result(unsafe {
+            bindings::dev_pm_opp_adjust_voltage(
+                self.dev.as_raw(),
+                freq,
+                u_volt,
+                u_volt_min,
+                u_volt_max,
+            )
+        })
+    }
+
+    /// Sets a matching OPP based on frequency.
+    pub fn set_rate(&self, freq: u64) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        to_result(unsafe { bindings::dev_pm_opp_set_rate(self.dev.as_raw(), freq) })
+    }
+
+    /// Sets exact OPP.
+    pub fn set_opp(&self, opp: &OPP) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        to_result(unsafe { bindings::dev_pm_opp_set_opp(self.dev.as_raw(), opp.as_raw()) })
+    }
+
+    /// Finds OPP based on frequency.
+    pub fn opp_from_freq(
+        &self,
+        mut freq: u64,
+        available: Option<bool>,
+        index: Option<u32>,
+        stype: SearchType,
+    ) -> Result<ARef<OPP>> {
+        let rdev = self.dev.as_raw();
+        let index = index.unwrap_or(0);
+
+        let ptr = from_err_ptr(match stype {
+            SearchType::Exact => {
+                if let Some(available) = available {
+                    // SAFETY: The requirements are satisfied by the existence of `Device` and
+                    // its safety requirements. The returned ptr will be owned by the new [`OPP`]
+                    // instance.
+                    unsafe {
+                        bindings::dev_pm_opp_find_freq_exact_indexed(rdev, freq, index, available)
+                    }
+                } else {
+                    return Err(EINVAL);
+                }
+            }
+
+            // SAFETY: The requirements are satisfied by the existence of `Device` and its
+            // safety requirements. The returned ptr will be owned by the new [`OPP`] instance.
+            SearchType::Ceil => unsafe {
+                bindings::dev_pm_opp_find_freq_ceil_indexed(rdev, &mut freq as *mut u64, index)
+            },
+
+            // SAFETY: The requirements are satisfied by the existence of `Device` and its
+            // safety requirements. The returned ptr will be owned by the new [`OPP`] instance.
+            SearchType::Floor => unsafe {
+                bindings::dev_pm_opp_find_freq_floor_indexed(rdev, &mut freq as *mut u64, index)
+            },
+        })?;
+
+        // SAFETY: The `ptr` is guaranteed by the C code to be valid.
+        Ok(unsafe { OPP::from_raw_opp_owned(ptr)?.into() })
+    }
+
+    /// Finds OPP based on level.
+    pub fn opp_from_level(&self, mut level: u32, stype: SearchType) -> Result<ARef<OPP>> {
+        let rdev = self.dev.as_raw();
+
+        let ptr = from_err_ptr(match stype {
+            // SAFETY: The requirements are satisfied by the existence of `Device` and its
+            // safety requirements. The returned ptr will be owned by the new [`OPP`] instance.
+            SearchType::Exact => unsafe { bindings::dev_pm_opp_find_level_exact(rdev, level) },
+
+            // SAFETY: The requirements are satisfied by the existence of `Device` and its
+            // safety requirements. The returned ptr will be owned by the new [`OPP`] instance.
+            SearchType::Ceil => unsafe {
+                bindings::dev_pm_opp_find_level_ceil(rdev, &mut level as *mut u32)
+            },
+
+            // SAFETY: The requirements are satisfied by the existence of `Device` and its
+            // safety requirements. The returned ptr will be owned by the new [`OPP`] instance.
+            SearchType::Floor => unsafe {
+                bindings::dev_pm_opp_find_level_floor(rdev, &mut level as *mut u32)
+            },
+        })?;
+
+        // SAFETY: The `ptr` is guaranteed by the C code to be valid.
+        Ok(unsafe { OPP::from_raw_opp_owned(ptr)?.into() })
+    }
+
+    /// Finds OPP based on bandwidth.
+    pub fn opp_from_bw(&self, mut bw: u32, index: i32, stype: SearchType) -> Result<ARef<OPP>> {
+        let rdev = self.dev.as_raw();
+
+        let ptr = from_err_ptr(match stype {
+            // The OPP core doesn't support this yet.
+            SearchType::Exact => return Err(EINVAL),
+
+            // SAFETY: The requirements are satisfied by the existence of `Device` and its
+            // safety requirements. The returned ptr will be owned by the new [`OPP`] instance.
+            SearchType::Ceil => unsafe {
+                bindings::dev_pm_opp_find_bw_ceil(rdev, &mut bw as *mut u32, index)
+            },
+
+            // SAFETY: The requirements are satisfied by the existence of `Device` and its
+            // safety requirements. The returned ptr will be owned by the new [`OPP`] instance.
+            SearchType::Floor => unsafe {
+                bindings::dev_pm_opp_find_bw_floor(rdev, &mut bw as *mut u32, index)
+            },
+        })?;
+
+        // SAFETY: The `ptr` is guaranteed by the C code to be valid.
+        Ok(unsafe { OPP::from_raw_opp_owned(ptr)?.into() })
+    }
+
+    /// Enable the OPP.
+    pub fn enable_opp(&self, freq: u64) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        to_result(unsafe { bindings::dev_pm_opp_enable(self.dev.as_raw(), freq) })
+    }
+
+    /// Disable the OPP.
+    pub fn disable_opp(&self, freq: u64) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        to_result(unsafe { bindings::dev_pm_opp_disable(self.dev.as_raw(), freq) })
+    }
+
+    /// Registers with Energy model.
+    #[cfg(CONFIG_OF)]
+    pub fn of_register_em(&mut self, cpumask: &mut Cpumask) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        to_result(unsafe {
+            bindings::dev_pm_opp_of_register_em(self.dev.as_raw(), cpumask.as_mut_ptr())
+        })?;
+
+        self.em = true;
+        Ok(())
+    }
+
+    // Unregisters with Energy model.
+    #[cfg(CONFIG_OF)]
+    fn of_unregister_em(&self) {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements. We registered with the EM framework earlier, it is safe to unregister now.
+        unsafe { bindings::em_dev_unregister_perf_domain(self.dev.as_raw()) };
+    }
+}
+
+impl Drop for Table {
+    fn drop(&mut self) {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe
+        // to relinquish it now.
+        unsafe { bindings::dev_pm_opp_put_opp_table(self.ptr) };
+
+        #[cfg(CONFIG_OF)]
+        {
+            if self.em {
+                self.of_unregister_em();
+            }
+
+            if self.of {
+                self.remove_of();
+            } else if let Some(cpumask) = self.cpumask.take() {
+                self.remove_of_cpumask(cpumask);
+            }
+        }
+    }
+}
+
 /// Operating performance point (OPP).
 ///
 /// # Invariants
-- 
2.31.1.272.g89b43f80a514

[RFC PATCH V3 3/8] rust: Extend OPP bindings for the configuration options

[Viresh Kumar]

This extends OPP bindings with the bindings for the OPP core
configuration options.

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 rust/kernel/opp.rs | 301 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 299 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
index 4b31f99acc67..92c4ac6cb89c 100644
--- a/rust/kernel/opp.rs
+++ b/rust/kernel/opp.rs
@@ -10,11 +10,28 @@
     bindings,
     cpumask::Cpumask,
     device::Device,
-    error::{code::*, from_err_ptr, to_result, Error, Result},
+    error::{code::*, from_err_ptr, from_result, to_result, Error, Result, VTABLE_DEFAULT_ERROR},
+    prelude::*,
+    str::CString,
     types::{ARef, AlwaysRefCounted, Opaque},
 };
 
-use core::ptr;
+use core::{ffi::c_char, marker::PhantomData, ptr};
+
+use macros::vtable;
+
+// Creates a null-terminated slice of pointers to Cstrings.
+fn to_c_str_array(names: &[CString]) -> Result<Vec<*const c_char>> {
+    // Allocated a null-terminated vector of pointers.
+    let mut list = Vec::with_capacity(names.len() + 1, GFP_KERNEL)?;
+
+    for name in names.iter() {
+        list.push(name.as_ptr() as _, GFP_KERNEL)?;
+    }
+
+    list.push(ptr::null(), GFP_KERNEL)?;
+    Ok(list)
+}
 
 /// Dynamically created Operating performance point (OPP).
 pub struct Token {
@@ -79,6 +96,286 @@ pub enum SearchType {
     Ceil,
 }
 
+/// Implement this trait to provide OPP Configuration callbacks.
+#[vtable]
+pub trait ConfigOps {
+    /// Called by the OPP core to configure OPP clks.
+    fn config_clks(_dev: &Device, _table: &Table, _opp: &OPP, _scaling_down: bool) -> Result<()> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Called by the OPP core to configure OPP regulators.
+    fn config_regulators(
+        _dev: &Device,
+        _opp_old: &OPP,
+        _opp_new: &OPP,
+        _data: *mut *mut bindings::regulator,
+        _count: u32,
+    ) -> Result<()> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+}
+
+/// Config token returned by the C code.
+pub struct ConfigToken(i32);
+
+impl Drop for ConfigToken {
+    fn drop(&mut self) {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe
+        // to relinquish it now.
+        unsafe { bindings::dev_pm_opp_clear_config(self.0) };
+    }
+}
+
+/// Equivalent to `struct dev_pm_opp_config` in the C Code.
+#[derive(Default)]
+pub struct Config<T: ConfigOps> {
+    clk_names: Option<Vec<CString>>,
+    prop_name: Option<CString>,
+    regulator_names: Option<Vec<CString>>,
+    genpd_names: Option<Vec<CString>>,
+    supported_hw: Option<Vec<u32>>,
+    required_devs: Option<Vec<ARef<Device>>>,
+    _data: PhantomData<T>,
+}
+
+impl<T: ConfigOps> Config<T> {
+    /// Creates a new instance of [`Config`].
+    pub fn new() -> Self {
+        Self {
+            clk_names: None,
+            prop_name: None,
+            regulator_names: None,
+            genpd_names: None,
+            supported_hw: None,
+            required_devs: None,
+            _data: PhantomData,
+        }
+    }
+
+    /// Initializes clock names.
+    pub fn set_clk_names(mut self, names: Vec<CString>) -> Result<Self> {
+        // Already configured.
+        if self.clk_names.is_some() {
+            return Err(EBUSY);
+        }
+
+        if names.is_empty() {
+            return Err(EINVAL);
+        }
+
+        self.clk_names = Some(names);
+        Ok(self)
+    }
+
+    /// Initializes property name.
+    pub fn set_prop_name(mut self, name: CString) -> Result<Self> {
+        // Already configured.
+        if self.prop_name.is_some() {
+            return Err(EBUSY);
+        }
+
+        self.prop_name = Some(name);
+        Ok(self)
+    }
+
+    /// Initializes regulator names.
+    pub fn set_regulator_names(mut self, names: Vec<CString>) -> Result<Self> {
+        // Already configured.
+        if self.regulator_names.is_some() {
+            return Err(EBUSY);
+        }
+
+        if names.is_empty() {
+            return Err(EINVAL);
+        }
+
+        self.regulator_names = Some(names);
+
+        Ok(self)
+    }
+
+    /// Initializes genpd names.
+    pub fn set_genpd_names(mut self, names: Vec<CString>) -> Result<Self> {
+        // Already configured. Only one of genpd or required devs can be configured.
+        if self.genpd_names.is_some() || self.required_devs.is_some() {
+            return Err(EBUSY);
+        }
+
+        if names.is_empty() {
+            return Err(EINVAL);
+        }
+
+        self.genpd_names = Some(names);
+        Ok(self)
+    }
+
+    /// Initializes required devices.
+    pub fn set_required_devs(mut self, devs: Vec<ARef<Device>>) -> Result<Self> {
+        // Already configured. Only one of genpd or required devs can be configured.
+        if self.genpd_names.is_some() || self.required_devs.is_some() {
+            return Err(EBUSY);
+        }
+
+        if devs.is_empty() {
+            return Err(EINVAL);
+        }
+
+        self.required_devs = Some(devs);
+        Ok(self)
+    }
+
+    /// Initializes supported hardware.
+    pub fn set_supported_hw(mut self, hw: Vec<u32>) -> Result<Self> {
+        // Already configured.
+        if self.supported_hw.is_some() {
+            return Err(EBUSY);
+        }
+
+        if hw.is_empty() {
+            return Err(EINVAL);
+        }
+
+        self.supported_hw = Some(hw);
+        Ok(self)
+    }
+
+    /// Sets the configuration with the OPP core.
+    pub fn set(self, dev: &Device) -> Result<ConfigToken> {
+        let (_clk_list, clk_names) = match &self.clk_names {
+            Some(x) => {
+                let list = to_c_str_array(x)?;
+                let ptr = list.as_ptr();
+                (Some(list), ptr)
+            }
+            None => (None, ptr::null()),
+        };
+
+        let (_regulator_list, regulator_names) = match &self.regulator_names {
+            Some(x) => {
+                let list = to_c_str_array(x)?;
+                let ptr = list.as_ptr();
+                (Some(list), ptr)
+            }
+            None => (None, ptr::null()),
+        };
+
+        let (_genpd_list, genpd_names) = match &self.genpd_names {
+            Some(x) => {
+                let list = to_c_str_array(x)?;
+                let ptr = list.as_ptr();
+                (Some(list), ptr)
+            }
+            None => (None, ptr::null()),
+        };
+
+        let prop_name = match &self.prop_name {
+            Some(x) => x.as_char_ptr(),
+            None => ptr::null(),
+        };
+
+        let (supported_hw, supported_hw_count) = match &self.supported_hw {
+            Some(x) => (x.as_ptr(), x.len() as u32),
+            None => (ptr::null(), 0),
+        };
+
+        let (_required_devs_list, required_devs) = match &self.required_devs {
+            Some(x) => {
+                // Create a non-NULL-terminated vectorof pointers.
+                let mut list = Vec::with_capacity(x.len(), GFP_KERNEL)?;
+
+                for dev in x.iter() {
+                    list.push(dev.as_raw(), GFP_KERNEL)?;
+                }
+
+                let ptr = list.as_mut_ptr();
+                (Some(list), ptr)
+            }
+            None => (None, ptr::null_mut()),
+        };
+
+        let mut config = bindings::dev_pm_opp_config {
+            clk_names,
+            config_clks: if T::HAS_CONFIG_CLKS {
+                Some(Self::config_clks)
+            } else {
+                None
+            },
+            prop_name,
+            regulator_names,
+            config_regulators: if T::HAS_CONFIG_REGULATORS {
+                Some(Self::config_regulators)
+            } else {
+                None
+            },
+            genpd_names,
+            supported_hw,
+            supported_hw_count,
+
+            // Don't need to support virt_devs for now.
+            virt_devs: ptr::null_mut(),
+            required_devs,
+        };
+
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements. The OPP core guarantees to not use fields of `config`, after this call has
+        // returned and so we don't need to save a copy of them for future use
+        let ret = unsafe { bindings::dev_pm_opp_set_config(dev.as_raw(), &mut config) };
+        if ret < 0 {
+            Err(Error::from_errno(ret))
+        } else {
+            Ok(ConfigToken(ret))
+        }
+    }
+
+    // Config's config_clks callback.
+    extern "C" fn config_clks(
+        dev: *mut bindings::device,
+        opp_table: *mut bindings::opp_table,
+        opp: *mut bindings::dev_pm_opp,
+        _data: *mut core::ffi::c_void,
+        scaling_down: bool,
+    ) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: 'dev' is guaranteed by the C code to be valid.
+            let dev = unsafe { Device::from_raw(dev) };
+            T::config_clks(
+                &dev,
+                // SAFETY: 'opp_table' is guaranteed by the C code to be valid.
+                &unsafe { Table::from_raw_table(opp_table, &dev) },
+                // SAFETY: 'opp' is guaranteed by the C code to be valid.
+                unsafe { OPP::from_raw_opp(opp)? },
+                scaling_down,
+            )
+            .map(|_| 0)
+        })
+    }
+
+    // Config's config_regulators callback.
+    extern "C" fn config_regulators(
+        dev: *mut bindings::device,
+        old_opp: *mut bindings::dev_pm_opp,
+        new_opp: *mut bindings::dev_pm_opp,
+        regulators: *mut *mut bindings::regulator,
+        count: core::ffi::c_uint,
+    ) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: 'dev' is guaranteed by the C code to be valid.
+            let dev = unsafe { Device::from_raw(dev) };
+            T::config_regulators(
+                &dev,
+                // SAFETY: 'old_opp' is guaranteed by the C code to be valid.
+                unsafe { OPP::from_raw_opp(old_opp)? },
+                // SAFETY: 'new_opp' is guaranteed by the C code to be valid.
+                unsafe { OPP::from_raw_opp(new_opp)? },
+                regulators,
+                count,
+            )
+            .map(|_| 0)
+        })
+    }
+}
+
 /// Operating performance point (OPP) table.
 ///
 /// # Invariants
-- 
2.31.1.272.g89b43f80a514

[RFC PATCH V3 4/8] rust: Add initial bindings for cpufreq framework

[Viresh Kumar]

This commit adds initial Rust bindings for the cpufreq core. This adds
basic bindings for cpufreq flags, relations and cpufreq table.

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 rust/bindings/bindings_helper.h |   1 +
 rust/helpers.c                  |  15 ++
 rust/kernel/cpufreq.rs          | 254 ++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   2 +
 4 files changed, 272 insertions(+)
 create mode 100644 rust/kernel/cpufreq.rs

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 1bf8e053c8f4..bee2b6013690 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -7,6 +7,7 @@
  */
 
 #include <kunit/test.h>
+#include <linux/cpufreq.h>
 #include <linux/errname.h>
 #include <linux/ethtool.h>
 #include <linux/firmware.h>
diff --git a/rust/helpers.c b/rust/helpers.c
index a1b52378867c..e1e37d57ef7d 100644
--- a/rust/helpers.c
+++ b/rust/helpers.c
@@ -24,6 +24,7 @@
 #include <linux/bug.h>
 #include <linux/build_bug.h>
 #include <linux/cpumask.h>
+#include <linux/cpufreq.h>
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/errname.h>
@@ -349,6 +350,20 @@ bool rust_helper_zalloc_cpumask_var(cpumask_var_t *mask, gfp_t flags)
 }
 EXPORT_SYMBOL_GPL(rust_helper_zalloc_cpumask_var);
 
+#ifdef CONFIG_CPU_FREQ
+unsigned int rust_helper_cpufreq_table_len(struct cpufreq_frequency_table *freq_table)
+{
+	return cpufreq_table_len(freq_table);
+}
+EXPORT_SYMBOL_GPL(rust_helper_cpufreq_table_len);
+
+void rust_helper_cpufreq_register_em_with_opp(struct cpufreq_policy *policy)
+{
+	cpufreq_register_em_with_opp(policy);
+}
+EXPORT_SYMBOL_GPL(rust_helper_cpufreq_register_em_with_opp);
+#endif
+
 #ifndef CONFIG_OF_DYNAMIC
 struct device_node *rust_helper_of_node_get(struct device_node *node)
 {
diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
new file mode 100644
index 000000000000..3f3ffd9abaa2
--- /dev/null
+++ b/rust/kernel/cpufreq.rs
@@ -0,0 +1,254 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! CPU frequency scaling.
+//!
+//! This module provides bindings for interacting with the cpufreq subsystem.
+//!
+//! C header: [`include/linux/cpufreq.h`](srctree/include/linux/cpufreq.h)
+
+use crate::{
+    bindings,
+    error::{code::*, to_result, Result},
+    prelude::*,
+};
+
+use core::{
+    pin::Pin,
+};
+
+/// Default transition latency value.
+pub const ETERNAL_LATENCY: u32 = bindings::CPUFREQ_ETERNAL as u32;
+
+/// Container for cpufreq driver flags.
+pub mod flags {
+    use crate::bindings;
+
+    /// Set by drivers that need to update internal upper and lower boundaries along with the
+    /// target frequency and so the core and governors should also invoke the driver if the target
+    /// frequency does not change, but the policy min or max may have changed.
+    pub const NEED_UPDATE_LIMITS: u16 = bindings::CPUFREQ_NEED_UPDATE_LIMITS as _;
+
+    /// Set by drivers for platforms where loops_per_jiffy or other kernel "constants" aren't
+    /// affected by frequency transitions.
+    pub const CONST_LOOPS: u16 = bindings::CPUFREQ_CONST_LOOPS as _;
+
+    /// Set by drivers that want the core to automatically register the cpufreq driver as a thermal
+    /// cooling device.
+    pub const IS_COOLING_DEV: u16 = bindings::CPUFREQ_IS_COOLING_DEV as _;
+
+    /// Set by drivers for platforms that have multiple clock-domains, i.e.  supporting multiple
+    /// policies. With this sysfs directories of governor would be created in cpu/cpuN/cpufreq/
+    /// directory and so they can use the same governor with different tunables for different
+    /// clusters.
+    pub const HAVE_GOVERNOR_PER_POLICY: u16 = bindings::CPUFREQ_HAVE_GOVERNOR_PER_POLICY as _;
+
+    /// Set by drivers which do POSTCHANGE notifications from outside of their ->target() routine.
+    pub const ASYNC_NOTIFICATION: u16 = bindings::CPUFREQ_ASYNC_NOTIFICATION as _;
+
+    /// Set by drivers that want cpufreq core to check if CPU is running at a frequency present in
+    /// freq-table exposed by the driver. For these drivers if CPU is found running at an out of
+    /// table freq, the cpufreq core will try to change the frequency to a value from the table.
+    /// And if that fails, it will stop further boot process by issuing a BUG_ON().
+    pub const NEED_INITIAL_FREQ_CHECK: u16 = bindings::CPUFREQ_NEED_INITIAL_FREQ_CHECK as _;
+
+    /// Set by drivers to disallow use of governors with "dynamic_switching" flag set.
+    pub const NO_AUTO_DYNAMIC_SWITCHING: u16 = bindings::CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING as _;
+}
+
+/// CPU frequency selection relations. Each value contains a `bool` argument which corresponds to
+/// the Relation being efficient.
+#[derive(Copy, Clone, Debug, Eq, PartialEq)]
+pub enum Relation {
+    /// Select the lowest frequency at or above target.
+    Low(bool),
+    /// Select the highest frequency below or at target.
+    High(bool),
+    /// Select the closest frequency to the target.
+    Close(bool),
+}
+
+impl Relation {
+    // Converts from a value compatible with the C code.
+    fn new(val: u32) -> Result<Self> {
+        let efficient = val & bindings::CPUFREQ_RELATION_E != 0;
+
+        Ok(match val & !bindings::CPUFREQ_RELATION_E {
+            bindings::CPUFREQ_RELATION_L => Self::Low(efficient),
+            bindings::CPUFREQ_RELATION_H => Self::High(efficient),
+            bindings::CPUFREQ_RELATION_C => Self::Close(efficient),
+            _ => return Err(EINVAL),
+        })
+    }
+
+    /// Converts to a value compatible with the C code.
+    pub fn val(&self) -> u32 {
+        let (mut val, e) = match self {
+            Self::Low(e) => (bindings::CPUFREQ_RELATION_L, e),
+            Self::High(e) => (bindings::CPUFREQ_RELATION_H, e),
+            Self::Close(e) => (bindings::CPUFREQ_RELATION_C, e),
+        };
+
+        if *e {
+            val |= bindings::CPUFREQ_RELATION_E;
+        }
+
+        val
+    }
+}
+
+/// Equivalent to `struct cpufreq_policy_data` in the C code.
+#[repr(transparent)]
+pub struct PolicyData(*mut bindings::cpufreq_policy_data);
+
+impl PolicyData {
+    /// Creates new instance of [`PolicyData`].
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that `ptr` is valid and non-null.
+    pub unsafe fn from_raw_policy_data(ptr: *mut bindings::cpufreq_policy_data) -> Self {
+        Self(ptr)
+    }
+
+    /// Returns the raw pointer to the C structure.
+    #[inline]
+    pub fn as_raw(&self) -> *mut bindings::cpufreq_policy_data {
+        self.0
+    }
+
+    /// Provides a wrapper to the generic verify routine.
+    pub fn generic_verify(&self) -> Result<()> {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // use it now.
+        to_result(unsafe { bindings::cpufreq_generic_frequency_table_verify(self.as_raw()) })
+    }
+}
+
+/// Builder for the `struct cpufreq_frequency_table` in the C code.
+#[repr(transparent)]
+#[derive(Default)]
+pub struct TableBuilder {
+    entries: Vec<bindings::cpufreq_frequency_table>,
+}
+
+impl TableBuilder {
+    /// Creates new instance of [`TableBuilder`].
+    pub fn new() -> Self {
+        Self {
+            entries: Vec::new(),
+        }
+    }
+
+    /// Adds a new entry to the table.
+    pub fn add(&mut self, frequency: u32, flags: u32, driver_data: u32) -> Result<()> {
+        // Adds new entry to the end of the vector.
+        Ok(self.entries.push(
+            bindings::cpufreq_frequency_table {
+                flags,
+                driver_data,
+                frequency,
+            },
+            GFP_KERNEL,
+        )?)
+    }
+
+    /// Creates [`Table`] from [`TableBuilder`].
+    pub fn into_table(mut self) -> Result<Table> {
+        // Add last entry to the table.
+        self.add(bindings::CPUFREQ_TABLE_END as u32, 0, 0)?;
+        Table::from_builder(self.entries)
+    }
+}
+
+/// A simple implementation of the cpufreq table, equivalent to the `struct
+/// cpufreq_frequency_table` in the C code.
+pub struct Table {
+    #[allow(dead_code)]
+    // Dynamically created table.
+    entries: Option<Pin<Vec<bindings::cpufreq_frequency_table>>>,
+
+    // Pointer to the statically or dynamically created table.
+    ptr: *mut bindings::cpufreq_frequency_table,
+
+    // Number of entries in the table.
+    len: usize,
+}
+
+impl Table {
+    /// Creates new instance of [`Table`] from [`TableBuilder`].
+    fn from_builder(entries: Vec<bindings::cpufreq_frequency_table>) -> Result<Self> {
+        let len = entries.len();
+        if len == 0 {
+            return Err(EINVAL);
+        }
+
+        // Pin the entries to memory, since we are passing its pointer to the C code.
+        let mut entries = Pin::new(entries);
+
+        // The pointer is valid until the table gets dropped.
+        let ptr = entries.as_mut_ptr();
+
+        Ok(Self {
+            entries: Some(entries),
+            ptr,
+            // The last entry in table is reserved for `CPUFREQ_TABLE_END`.
+            len: len - 1,
+        })
+    }
+
+    /// Creates new instance of [`Table`] from raw pointer.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that `ptr` is valid and non-null for the lifetime of the [`Table`].
+    pub unsafe fn from_raw(ptr: *mut bindings::cpufreq_frequency_table) -> Self {
+        Self {
+            entries: None,
+            ptr,
+            // SAFETY: The pointer is guaranteed to be valid for the lifetime of `Self`.
+            len: unsafe { bindings::cpufreq_table_len(ptr) } as usize,
+        }
+    }
+
+    // Validate the index.
+    fn validate(&self, index: usize) -> Result<()> {
+        if index >= self.len {
+            Err(EINVAL)
+        } else {
+            Ok(())
+        }
+    }
+
+    /// Returns raw pointer to the `struct cpufreq_frequency_table` compatible with the C code.
+    #[inline]
+    pub fn as_raw(&self) -> *mut bindings::cpufreq_frequency_table {
+        self.ptr
+    }
+
+    /// Returns `frequency` at index in the [`Table`].
+    pub fn freq(&self, index: usize) -> Result<u32> {
+        self.validate(index)?;
+
+        // SAFETY: The pointer is guaranteed to be valid for the lifetime of `self` and `index` is
+        // also validated before this and is guaranteed to be within limits of the frequency table.
+        Ok(unsafe { (*self.ptr.add(index)).frequency })
+    }
+
+    /// Returns `flags` at index in the [`Table`].
+    pub fn flags(&self, index: usize) -> Result<u32> {
+        self.validate(index)?;
+
+        // SAFETY: The pointer is guaranteed to be valid for the lifetime of `self` and `index` is
+        // also validated before this and is guaranteed to be within limits of the frequency table.
+        Ok(unsafe { (*self.ptr.add(index)).flags })
+    }
+
+    /// Returns `data` at index in the [`Table`].
+    pub fn data(&self, index: usize) -> Result<u32> {
+        self.validate(index)?;
+
+        // SAFETY: The pointer is guaranteed to be valid for the lifetime of `self` and `index` is
+        // also validated before this and is guaranteed to be within limits of the frequency table.
+        Ok(unsafe { (*self.ptr.add(index)).driver_data })
+    }
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index e309d7774cbd..77348fc33803 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -30,6 +30,8 @@
 pub mod alloc;
 mod build_assert;
 pub mod clk;
+#[cfg(CONFIG_CPU_FREQ)]
+pub mod cpufreq;
 pub mod cpumask;
 pub mod device;
 pub mod device_id;
-- 
2.31.1.272.g89b43f80a514

[RFC PATCH V3 5/8] rust: Extend cpufreq bindings for policy and driver ops

[Viresh Kumar]

This extends the cpufreq bindings with bindings for cpufreq policy and
driver operations.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 rust/kernel/cpufreq.rs | 315 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 313 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
index 3f3ffd9abaa2..6f9d34ebbcb0 100644
--- a/rust/kernel/cpufreq.rs
+++ b/rust/kernel/cpufreq.rs
@@ -7,15 +7,20 @@
 //! C header: [`include/linux/cpufreq.h`](srctree/include/linux/cpufreq.h)
 
 use crate::{
-    bindings,
-    error::{code::*, to_result, Result},
+    bindings, clk, cpumask,
+    device::Device,
+    error::{code::*, from_err_ptr, to_result, Result, VTABLE_DEFAULT_ERROR},
     prelude::*,
+    types::{ARef, ForeignOwnable},
 };
 
 use core::{
     pin::Pin,
+    ptr::self,
 };
 
+use macros::vtable;
+
 /// Default transition latency value.
 pub const ETERNAL_LATENCY: u32 = bindings::CPUFREQ_ETERNAL as u32;
 
@@ -252,3 +257,309 @@ pub fn data(&self, index: usize) -> Result<u32> {
         Ok(unsafe { (*self.ptr.add(index)).driver_data })
     }
 }
+
+/// Equivalent to `struct cpufreq_policy` in the C code.
+pub struct Policy {
+    ptr: *mut bindings::cpufreq_policy,
+    put_cpu: bool,
+    cpumask: cpumask::Cpumask,
+}
+
+impl Policy {
+    /// Creates a new instance of [`Policy`].
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that `ptr` is valid and non-null.
+    pub unsafe fn from_raw_policy(ptr: *mut bindings::cpufreq_policy) -> Self {
+        Self {
+            ptr,
+            put_cpu: false,
+            // SAFETY: The pointer is guaranteed to be valid for the lifetime of `Self`. The `cpus`
+            // pointer is guaranteed to be valid by the C code.
+            cpumask: unsafe { cpumask::Cpumask::from_raw((*ptr).cpus) },
+        }
+    }
+
+    fn from_cpu(cpu: u32) -> Result<Self> {
+        // SAFETY: It is safe to call `cpufreq_cpu_get()` for any CPU.
+        let ptr = from_err_ptr(unsafe { bindings::cpufreq_cpu_get(cpu) })?;
+
+        // SAFETY: The pointer is guaranteed to be valid by the C code.
+        let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+        policy.put_cpu = true;
+        Ok(policy)
+    }
+
+    /// Raw pointer to the underlying cpufreq policy.
+    #[inline]
+    pub fn as_raw(&self) -> *mut bindings::cpufreq_policy {
+        self.ptr
+    }
+
+    fn as_ref(&self) -> &bindings::cpufreq_policy {
+        // SAFETY: By the type invariants, we know that `self` owns a reference to the pointer.
+        unsafe { &(*self.ptr) }
+    }
+    fn as_mut_ref(&mut self) -> &mut bindings::cpufreq_policy {
+        // SAFETY: By the type invariants, we know that `self` owns a reference to the pointer.
+        unsafe { &mut (*self.ptr) }
+    }
+
+    /// Returns the primary CPU for a cpufreq policy.
+    pub fn cpu(&self) -> u32 {
+        self.as_ref().cpu
+    }
+
+    /// Returns the minimum frequency for a cpufreq policy.
+    pub fn min(&self) -> u32 {
+        self.as_ref().min
+    }
+
+    /// Returns the maximum frequency for a cpufreq policy.
+    pub fn max(&self) -> u32 {
+        self.as_ref().max
+    }
+
+    /// Returns the current frequency for a cpufreq policy.
+    pub fn cur(&self) -> u32 {
+        self.as_ref().cur
+    }
+
+    /// Sets the suspend frequency for a cpufreq policy.
+    pub fn set_suspend_freq(&mut self, freq: u32) -> &mut Self {
+        self.as_mut_ref().suspend_freq = freq;
+        self
+    }
+
+    /// Returns the suspend frequency for a cpufreq policy.
+    pub fn suspend_freq(&self) -> u32 {
+        self.as_ref().suspend_freq
+    }
+
+    /// Provides a wrapper to the generic suspend routine.
+    pub fn generic_suspend(&self) -> Result<()> {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // use it now.
+        to_result(unsafe { bindings::cpufreq_generic_suspend(self.as_raw()) })
+    }
+
+    /// Provides a wrapper to the generic get routine.
+    pub fn generic_get(&self) -> Result<u32> {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // use it now.
+        Ok(unsafe { bindings::cpufreq_generic_get(self.cpu()) })
+    }
+
+    /// Provides a wrapper to the register em with OPP routine.
+    pub fn register_em_opp(&self) {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // use it now.
+        unsafe { bindings::cpufreq_register_em_with_opp(self.as_raw()) };
+    }
+
+    /// Gets raw pointer to cpufreq policy's CPUs mask.
+    pub fn cpus(&mut self) -> &mut cpumask::Cpumask {
+        &mut self.cpumask
+    }
+
+    /// Sets clock for a cpufreq policy.
+    pub fn set_clk(&mut self, dev: &Device, name: Option<&CStr>) -> Result<clk::Clk> {
+        let clk = clk::Clk::new(dev, name)?;
+        self.as_mut_ref().clk = clk.as_raw();
+        Ok(clk)
+    }
+
+    /// Allows frequency switching code to run on any CPU.
+    pub fn set_dvfs_possible_from_any_cpu(&mut self) -> &mut Self {
+        self.as_mut_ref().dvfs_possible_from_any_cpu = true;
+        self
+    }
+
+    /// Sets transition latency for a cpufreq policy.
+    pub fn set_transition_latency(&mut self, latency: u32) -> &mut Self {
+        self.as_mut_ref().cpuinfo.transition_latency = latency;
+        self
+    }
+
+    /// Returns the cpufreq table for a cpufreq policy. The cpufreq table is recreated in a
+    /// light-weight manner from the raw pointer. The table in C code is not freed once this table
+    /// is dropped.
+    pub fn freq_table(&self) -> Result<Table> {
+        if self.as_ref().freq_table.is_null() {
+            return Err(EINVAL);
+        }
+
+        // SAFETY: The `freq_table` is guaranteed to be valid.
+        Ok(unsafe { Table::from_raw(self.as_ref().freq_table) })
+    }
+
+    /// Sets the cpufreq table for a cpufreq policy.
+    ///
+    /// The cpufreq driver must guarantee that the frequency table does not get freed while it is
+    /// still being used by the C code.
+    pub fn set_freq_table(&mut self, table: &Table) -> &mut Self {
+        self.as_mut_ref().freq_table = table.as_raw();
+        self
+    }
+
+    /// Returns the data for a cpufreq policy.
+    pub fn data<T: ForeignOwnable>(&mut self) -> Option<<T>::Borrowed<'_>> {
+        if self.as_ref().driver_data.is_null() {
+            None
+        } else {
+            // SAFETY: The data is earlier set by us from [`set_data()`].
+            Some(unsafe { T::borrow(self.as_ref().driver_data) })
+        }
+    }
+
+    // Sets the data for a cpufreq policy.
+    fn set_data<T: ForeignOwnable>(&mut self, data: T) -> Result<()> {
+        if self.as_ref().driver_data.is_null() {
+            // Pass the ownership of the data to the foreign interface.
+            self.as_mut_ref().driver_data = <T as ForeignOwnable>::into_foreign(data) as _;
+            Ok(())
+        } else {
+            Err(EBUSY)
+        }
+    }
+
+    // Returns the data for a cpufreq policy.
+    fn clear_data<T: ForeignOwnable>(&mut self) -> Option<T> {
+        if self.as_ref().driver_data.is_null() {
+            None
+        } else {
+            // SAFETY: The data is earlier set by us from [`set_data()`]. It is safe to take back
+            // the ownership of the data from the foreign interface.
+            let data =
+                Some(unsafe { <T as ForeignOwnable>::from_foreign(self.as_ref().driver_data) });
+            self.as_mut_ref().driver_data = ptr::null_mut();
+            data
+        }
+    }
+}
+
+impl Drop for Policy {
+    fn drop(&mut self) {
+        if self.put_cpu {
+            // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+            // relinquish it now.
+            unsafe { bindings::cpufreq_cpu_put(self.as_raw()) };
+        }
+    }
+}
+
+/// Operations to be implemented by a cpufreq driver.
+#[vtable]
+pub trait Driver {
+    /// Driver specific data.
+    ///
+    /// Corresponds to the data retrieved via the kernel's
+    /// `cpufreq_get_driver_data()` function.
+    ///
+    /// Require that `Data` implements `ForeignOwnable`. We guarantee to
+    /// never move the underlying wrapped data structure.
+    type Data: ForeignOwnable;
+
+    /// Policy specific data.
+    ///
+    /// Require that `PData` implements `ForeignOwnable`. We guarantee to
+    /// never move the underlying wrapped data structure.
+    type PData: ForeignOwnable;
+
+    /// Policy's init callback.
+    fn init(policy: &mut Policy) -> Result<Self::PData>;
+
+    /// Policy's exit callback.
+    fn exit(_policy: &mut Policy, _data: Option<Self::PData>) -> Result<()> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's online callback.
+    fn online(_policy: &mut Policy) -> Result<()> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's offline callback.
+    fn offline(_policy: &mut Policy) -> Result<()> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's suspend callback.
+    fn suspend(_policy: &mut Policy) -> Result<()> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's resume callback.
+    fn resume(_policy: &mut Policy) -> Result<()> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's ready callback.
+    fn ready(_policy: &mut Policy) {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's verify callback.
+    fn verify(data: &mut PolicyData) -> Result<()>;
+
+    /// Policy's setpolicy callback.
+    fn setpolicy(_policy: &mut Policy) -> Result<()> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's target callback.
+    fn target(_policy: &mut Policy, _target_freq: u32, _relation: Relation) -> Result<()> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's target_index callback.
+    fn target_index(_policy: &mut Policy, _index: u32) -> Result<()> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's fast_switch callback.
+    fn fast_switch(_policy: &mut Policy, _target_freq: u32) -> u32 {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's adjust_perf callback.
+    fn adjust_perf(_policy: &mut Policy, _min_perf: u64, _target_perf: u64, _capacity: u64) {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's get_intermediate callback.
+    fn get_intermediate(_policy: &mut Policy, _index: u32) -> u32 {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's target_intermediate callback.
+    fn target_intermediate(_policy: &mut Policy, _index: u32) -> Result<()> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's get callback.
+    fn get(_policy: &mut Policy) -> Result<u32> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's update_limits callback.
+    fn update_limits(_policy: &mut Policy) {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's bios_limit callback.
+    fn bios_limit(_policy: &mut Policy, _limit: &mut u32) -> Result<()> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's set_boost callback.
+    fn set_boost(_policy: &mut Policy, _state: i32) -> Result<()> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's register_em callback.
+    fn register_em(_policy: &mut Policy) {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+}
-- 
2.31.1.272.g89b43f80a514

[RFC PATCH V3 6/8] rust: Extend cpufreq bindings for driver registration

[Viresh Kumar]

This extends the cpufreq bindings with bindings for registering a
driver.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 rust/kernel/cpufreq.rs | 482 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 479 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
index 6f9d34ebbcb0..66dad18f4ab6 100644
--- a/rust/kernel/cpufreq.rs
+++ b/rust/kernel/cpufreq.rs
@@ -9,14 +9,16 @@
 use crate::{
     bindings, clk, cpumask,
     device::Device,
-    error::{code::*, from_err_ptr, to_result, Result, VTABLE_DEFAULT_ERROR},
+    error::{code::*, from_err_ptr, from_result, to_result, Result, VTABLE_DEFAULT_ERROR},
     prelude::*,
-    types::{ARef, ForeignOwnable},
+    types::ForeignOwnable,
 };
 
 use core::{
+    cell::UnsafeCell,
+    marker::PhantomData,
     pin::Pin,
-    ptr::self,
+    ptr::{self, addr_of_mut},
 };
 
 use macros::vtable;
@@ -563,3 +565,477 @@ fn register_em(_policy: &mut Policy) {
         kernel::build_error(VTABLE_DEFAULT_ERROR)
     }
 }
+
+/// Registration of a cpufreq driver.
+pub struct Registration<T: Driver> {
+    registered: bool,
+    drv: UnsafeCell<bindings::cpufreq_driver>,
+    _p: PhantomData<T>,
+}
+
+// SAFETY: `Registration` doesn't offer any methods or access to fields when shared between threads
+// or CPUs, so it is safe to share it.
+unsafe impl<T: Driver> Sync for Registration<T> {}
+
+// SAFETY: Registration with and unregistration from the cpufreq subsystem can happen from any thread.
+// Additionally, `T::Data` (which is dropped during unregistration) is `Send`, so it is okay to move
+// `Registration` to different threads.
+#[allow(clippy::non_send_fields_in_send_ty)]
+unsafe impl<T: Driver> Send for Registration<T> {}
+
+impl<T: Driver> Registration<T> {
+    /// Creates new [`Registration`] but does not register it yet.
+    ///
+    /// It is allowed to move.
+    fn new() -> Result<Box<Self>> {
+        Ok(Box::new(
+            Self {
+                registered: false,
+                drv: UnsafeCell::new(bindings::cpufreq_driver::default()),
+                _p: PhantomData,
+            },
+            GFP_KERNEL,
+        )?)
+    }
+
+    /// Registers a cpufreq driver with the rest of the kernel.
+    pub fn register(
+        name: &'static CStr,
+        data: T::Data,
+        flags: u16,
+        boost: bool,
+    ) -> Result<Box<Self>> {
+        let mut reg = Self::new()?;
+        let drv = reg.drv.get_mut();
+
+        // Account for the trailing null character.
+        let len = name.len() + 1;
+        if len > drv.name.len() {
+            return Err(EINVAL);
+        };
+
+        // SAFETY: `name` is a valid Cstr, and we are copying it to an array of equal or larger
+        // size.
+        let name = unsafe { &*(name.as_bytes_with_nul() as *const [u8] as *const [i8]) };
+        drv.name[..len].copy_from_slice(name);
+
+        drv.boost_enabled = boost;
+        drv.flags = flags;
+
+        // Allocate an array of 3 pointers to be passed to the C code.
+        let mut attr = Box::new([ptr::null_mut(); 3], GFP_KERNEL)?;
+        let mut next = 0;
+
+        // SAFETY: The C code returns a valid pointer here, which is again passed to the C code in
+        // an array.
+        attr[next] =
+            unsafe { addr_of_mut!(bindings::cpufreq_freq_attr_scaling_available_freqs) as *mut _ };
+        next += 1;
+
+        if boost {
+            // SAFETY: The C code returns a valid pointer here, which is again passed to the C code
+            // in an array.
+            attr[next] =
+                unsafe { addr_of_mut!(bindings::cpufreq_freq_attr_scaling_boost_freqs) as *mut _ };
+            next += 1;
+        }
+        attr[next] = ptr::null_mut();
+
+        // Pass the ownership of the memory block to the C code. This will be freed when
+        // the [`Registration`] object goes out of scope.
+        drv.attr = Box::leak(attr) as *mut _;
+
+        // Initialize mandatory callbacks.
+        drv.init = Some(Self::init_callback);
+        drv.verify = Some(Self::verify_callback);
+
+        // Initialize optional callbacks.
+        drv.setpolicy = if T::HAS_SETPOLICY {
+            Some(Self::setpolicy_callback)
+        } else {
+            None
+        };
+        drv.target = if T::HAS_TARGET {
+            Some(Self::target_callback)
+        } else {
+            None
+        };
+        drv.target_index = if T::HAS_TARGET_INDEX {
+            Some(Self::target_index_callback)
+        } else {
+            None
+        };
+        drv.fast_switch = if T::HAS_FAST_SWITCH {
+            Some(Self::fast_switch_callback)
+        } else {
+            None
+        };
+        drv.adjust_perf = if T::HAS_ADJUST_PERF {
+            Some(Self::adjust_perf_callback)
+        } else {
+            None
+        };
+        drv.get_intermediate = if T::HAS_GET_INTERMEDIATE {
+            Some(Self::get_intermediate_callback)
+        } else {
+            None
+        };
+        drv.target_intermediate = if T::HAS_TARGET_INTERMEDIATE {
+            Some(Self::target_intermediate_callback)
+        } else {
+            None
+        };
+        drv.get = if T::HAS_GET {
+            Some(Self::get_callback)
+        } else {
+            None
+        };
+        drv.update_limits = if T::HAS_UPDATE_LIMITS {
+            Some(Self::update_limits_callback)
+        } else {
+            None
+        };
+        drv.bios_limit = if T::HAS_BIOS_LIMIT {
+            Some(Self::bios_limit_callback)
+        } else {
+            None
+        };
+        drv.online = if T::HAS_ONLINE {
+            Some(Self::online_callback)
+        } else {
+            None
+        };
+        drv.offline = if T::HAS_OFFLINE {
+            Some(Self::offline_callback)
+        } else {
+            None
+        };
+        drv.exit = if T::HAS_EXIT {
+            Some(Self::exit_callback)
+        } else {
+            None
+        };
+        drv.suspend = if T::HAS_SUSPEND {
+            Some(Self::suspend_callback)
+        } else {
+            None
+        };
+        drv.resume = if T::HAS_RESUME {
+            Some(Self::resume_callback)
+        } else {
+            None
+        };
+        drv.ready = if T::HAS_READY {
+            Some(Self::ready_callback)
+        } else {
+            None
+        };
+        drv.set_boost = if T::HAS_SET_BOOST {
+            Some(Self::set_boost_callback)
+        } else {
+            None
+        };
+        drv.register_em = if T::HAS_REGISTER_EM {
+            Some(Self::register_em_callback)
+        } else {
+            None
+        };
+
+        // Set driver data before registering the driver, as the cpufreq core may call few
+        // callbacks before `cpufreq_register_driver()` returns.
+        reg.set_data(data)?;
+
+        // SAFETY: It is safe to register the driver with the cpufreq core in the C code.
+        to_result(unsafe { bindings::cpufreq_register_driver(reg.drv.get()) })?;
+        reg.registered = true;
+        Ok(reg)
+    }
+
+    /// Returns the previous set data for a cpufreq driver.
+    pub fn data<D: ForeignOwnable>() -> Option<<D>::Borrowed<'static>> {
+        // SAFETY: The driver data is earlier set by us from [`set_data()`].
+        let data = unsafe { bindings::cpufreq_get_driver_data() };
+        if data.is_null() {
+            None
+        } else {
+            // SAFETY: The driver data is earlier set by us from [`set_data()`].
+            Some(unsafe { D::borrow(data) })
+        }
+    }
+
+    // Sets the data for a cpufreq driver.
+    fn set_data(&mut self, data: T::Data) -> Result<()> {
+        let drv = self.drv.get_mut();
+
+        if drv.driver_data.is_null() {
+            // Pass the ownership of the data to the foreign interface.
+            drv.driver_data = <T::Data as ForeignOwnable>::into_foreign(data) as _;
+            Ok(())
+        } else {
+            Err(EBUSY)
+        }
+    }
+
+    // Clears and returns the data for a cpufreq driver.
+    fn clear_data(&mut self) -> Option<T::Data> {
+        let drv = self.drv.get_mut();
+
+        if drv.driver_data.is_null() {
+            None
+        } else {
+            // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+            // relinquish it now.
+            let data = Some(unsafe { <T::Data as ForeignOwnable>::from_foreign(drv.driver_data) });
+            drv.driver_data = ptr::null_mut();
+            data
+        }
+    }
+}
+
+// cpufreq driver callbacks.
+impl<T: Driver> Registration<T> {
+    // Policy's init callback.
+    extern "C" fn init_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+            // duration of this call, so it is guaranteed to remain alive for the lifetime of
+            // `ptr`.
+            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+
+            let data = T::init(&mut policy)?;
+            policy.set_data(data)?;
+            Ok(0)
+        })
+    }
+
+    // Policy's exit callback.
+    extern "C" fn exit_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+            // duration of this call, so it is guaranteed to remain alive for the lifetime of
+            // `ptr`.
+            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+
+            let data = policy.clear_data();
+            T::exit(&mut policy, data).map(|_| 0)
+        })
+    }
+
+    // Policy's online callback.
+    extern "C" fn online_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+            // duration of this call, so it is guaranteed to remain alive for the lifetime of
+            // `ptr`.
+            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+            T::online(&mut policy).map(|_| 0)
+        })
+    }
+
+    // Policy's offline callback.
+    extern "C" fn offline_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+            // duration of this call, so it is guaranteed to remain alive for the lifetime of
+            // `ptr`.
+            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+            T::offline(&mut policy).map(|_| 0)
+        })
+    }
+
+    // Policy's suspend callback.
+    extern "C" fn suspend_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+            // duration of this call, so it is guaranteed to remain alive for the lifetime of
+            // `ptr`.
+            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+            T::suspend(&mut policy).map(|_| 0)
+        })
+    }
+
+    // Policy's resume callback.
+    extern "C" fn resume_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+            // duration of this call, so it is guaranteed to remain alive for the lifetime of
+            // `ptr`.
+            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+            T::resume(&mut policy).map(|_| 0)
+        })
+    }
+
+    // Policy's ready callback.
+    extern "C" fn ready_callback(ptr: *mut bindings::cpufreq_policy) {
+        // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+        // duration of this call, so it is guaranteed to remain alive for the lifetime of
+        // `ptr`.
+        let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+        T::ready(&mut policy);
+    }
+
+    // Policy's verify callback.
+    extern "C" fn verify_callback(ptr: *mut bindings::cpufreq_policy_data) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+            // duration of this call, so it is guaranteed to remain alive for the lifetime of
+            // `ptr`.
+            let mut data = unsafe { PolicyData::from_raw_policy_data(ptr) };
+            T::verify(&mut data).map(|_| 0)
+        })
+    }
+
+    // Policy's setpolicy callback.
+    extern "C" fn setpolicy_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+            // duration of this call, so it is guaranteed to remain alive for the lifetime of
+            // `ptr`.
+            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+            T::setpolicy(&mut policy).map(|_| 0)
+        })
+    }
+
+    // Policy's target callback.
+    extern "C" fn target_callback(
+        ptr: *mut bindings::cpufreq_policy,
+        target_freq: u32,
+        relation: u32,
+    ) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+            // duration of this call, so it is guaranteed to remain alive for the lifetime of
+            // `ptr`.
+            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+            T::target(&mut policy, target_freq, Relation::new(relation)?).map(|_| 0)
+        })
+    }
+
+    // Policy's target_index callback.
+    extern "C" fn target_index_callback(
+        ptr: *mut bindings::cpufreq_policy,
+        index: u32,
+    ) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+            // duration of this call, so it is guaranteed to remain alive for the lifetime of
+            // `ptr`.
+            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+            T::target_index(&mut policy, index).map(|_| 0)
+        })
+    }
+
+    // Policy's fast_switch callback.
+    extern "C" fn fast_switch_callback(
+        ptr: *mut bindings::cpufreq_policy,
+        target_freq: u32,
+    ) -> core::ffi::c_uint {
+        // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+        // duration of this call, so it is guaranteed to remain alive for the lifetime of
+        // `ptr`.
+        let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+        T::fast_switch(&mut policy, target_freq)
+    }
+
+    // Policy's adjust_perf callback.
+    extern "C" fn adjust_perf_callback(cpu: u32, min_perf: u64, target_perf: u64, capacity: u64) {
+        if let Ok(mut policy) = Policy::from_cpu(cpu) {
+            T::adjust_perf(&mut policy, min_perf, target_perf, capacity);
+        }
+    }
+
+    // Policy's get_intermediate callback.
+    extern "C" fn get_intermediate_callback(
+        ptr: *mut bindings::cpufreq_policy,
+        index: u32,
+    ) -> core::ffi::c_uint {
+        // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+        // duration of this call, so it is guaranteed to remain alive for the lifetime of
+        // `ptr`.
+        let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+        T::get_intermediate(&mut policy, index)
+    }
+
+    // Policy's target_intermediate callback.
+    extern "C" fn target_intermediate_callback(
+        ptr: *mut bindings::cpufreq_policy,
+        index: u32,
+    ) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+            // duration of this call, so it is guaranteed to remain alive for the lifetime of
+            // `ptr`.
+            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+            T::target_intermediate(&mut policy, index).map(|_| 0)
+        })
+    }
+
+    // Policy's get callback.
+    extern "C" fn get_callback(cpu: u32) -> core::ffi::c_uint {
+        // SAFETY: Get the policy for a CPU.
+        Policy::from_cpu(cpu).map_or(0, |mut policy| T::get(&mut policy).map_or(0, |f| f))
+    }
+
+    // Policy's update_limit callback.
+    extern "C" fn update_limits_callback(cpu: u32) {
+        // SAFETY: Get the policy for a CPU.
+        if let Ok(mut policy) = Policy::from_cpu(cpu) {
+            T::update_limits(&mut policy);
+        }
+    }
+
+    // Policy's bios_limit callback.
+    extern "C" fn bios_limit_callback(cpu: i32, limit: *mut u32) -> core::ffi::c_int {
+        from_result(|| {
+            let mut policy = Policy::from_cpu(cpu as u32)?;
+
+            // SAFETY: The pointer is guaranteed by the C code to be valid.
+            T::bios_limit(&mut policy, &mut (unsafe { *limit })).map(|_| 0)
+        })
+    }
+
+    // Policy's set_boost callback.
+    extern "C" fn set_boost_callback(
+        ptr: *mut bindings::cpufreq_policy,
+        state: i32,
+    ) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+            // duration of this call, so it is guaranteed to remain alive for the lifetime of
+            // `ptr`.
+            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+            T::set_boost(&mut policy, state).map(|_| 0)
+        })
+    }
+
+    // Policy's register_em callback.
+    extern "C" fn register_em_callback(ptr: *mut bindings::cpufreq_policy) {
+        // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+        // duration of this call, so it is guaranteed to remain alive for the lifetime of
+        // `ptr`.
+        let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+        T::register_em(&mut policy);
+    }
+}
+
+impl<T: Driver> Drop for Registration<T> {
+    // Removes the registration from the kernel if it has completed successfully before.
+    fn drop(&mut self) {
+        pr_info!("Registration dropped\n");
+        let drv = self.drv.get_mut();
+
+        if self.registered {
+            // SAFETY: The driver was earlier registered from `register()`.
+            unsafe { bindings::cpufreq_unregister_driver(drv) };
+        }
+
+        // Free the previously leaked memory to the C code.
+        if !drv.attr.is_null() {
+            // SAFETY: The pointer was earlier initialized from the result of `Box::leak`.
+            unsafe { drop(Box::from_raw(drv.attr)) };
+        }
+
+        // Free data
+        drop(self.clear_data());
+    }
+}
-- 
2.31.1.272.g89b43f80a514

Re: [RFC PATCH V3 6/8] rust: Extend cpufreq bindings for driver registration

[Danilo Krummrich]

On 7/3/24 09:14, Viresh Kumar wrote:
> This extends the cpufreq bindings with bindings for registering a
> driver.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>   rust/kernel/cpufreq.rs | 482 ++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 479 insertions(+), 3 deletions(-)
> 
> diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
> index 6f9d34ebbcb0..66dad18f4ab6 100644
> --- a/rust/kernel/cpufreq.rs
> +++ b/rust/kernel/cpufreq.rs
> @@ -9,14 +9,16 @@
>   use crate::{
>       bindings, clk, cpumask,
>       device::Device,
> -    error::{code::*, from_err_ptr, to_result, Result, VTABLE_DEFAULT_ERROR},
> +    error::{code::*, from_err_ptr, from_result, to_result, Result, VTABLE_DEFAULT_ERROR},
>       prelude::*,
> -    types::{ARef, ForeignOwnable},
> +    types::ForeignOwnable,
>   };
>   
>   use core::{
> +    cell::UnsafeCell,
> +    marker::PhantomData,
>       pin::Pin,
> -    ptr::self,
> +    ptr::{self, addr_of_mut},
>   };
>   
>   use macros::vtable;
> @@ -563,3 +565,477 @@ fn register_em(_policy: &mut Policy) {
>           kernel::build_error(VTABLE_DEFAULT_ERROR)
>       }
>   }
> +
> +/// Registration of a cpufreq driver.
> +pub struct Registration<T: Driver> {
> +    registered: bool,
> +    drv: UnsafeCell<bindings::cpufreq_driver>,
> +    _p: PhantomData<T>,
> +}
> +
> +// SAFETY: `Registration` doesn't offer any methods or access to fields when shared between threads
> +// or CPUs, so it is safe to share it.
> +unsafe impl<T: Driver> Sync for Registration<T> {}
> +
> +// SAFETY: Registration with and unregistration from the cpufreq subsystem can happen from any thread.
> +// Additionally, `T::Data` (which is dropped during unregistration) is `Send`, so it is okay to move
> +// `Registration` to different threads.
> +#[allow(clippy::non_send_fields_in_send_ty)]
> +unsafe impl<T: Driver> Send for Registration<T> {}
> +
> +impl<T: Driver> Registration<T> {
> +    /// Creates new [`Registration`] but does not register it yet.
> +    ///
> +    /// It is allowed to move.
> +    fn new() -> Result<Box<Self>> {
> +        Ok(Box::new(
> +            Self {
> +                registered: false,
> +                drv: UnsafeCell::new(bindings::cpufreq_driver::default()),
> +                _p: PhantomData,
> +            },
> +            GFP_KERNEL,
> +        )?)
> +    }
> +
> +    /// Registers a cpufreq driver with the rest of the kernel.
> +    pub fn register(
> +        name: &'static CStr,
> +        data: T::Data,
> +        flags: u16,
> +        boost: bool,
> +    ) -> Result<Box<Self>> {

If you directly call `register` from `new` you can avoid having `Self::registered`.
It's also a bit cleaner, once you got an instance of `Registration` it means something
is registered, once it's dropped, it's unregistered.

> +        let mut reg = Self::new()?;
> +        let drv = reg.drv.get_mut();
> +
> +        // Account for the trailing null character.
> +        let len = name.len() + 1;
> +        if len > drv.name.len() {
> +            return Err(EINVAL);
> +        };
> +
> +        // SAFETY: `name` is a valid Cstr, and we are copying it to an array of equal or larger
> +        // size.
> +        let name = unsafe { &*(name.as_bytes_with_nul() as *const [u8] as *const [i8]) };
> +        drv.name[..len].copy_from_slice(name);
> +
> +        drv.boost_enabled = boost;
> +        drv.flags = flags;
> +
> +        // Allocate an array of 3 pointers to be passed to the C code.
> +        let mut attr = Box::new([ptr::null_mut(); 3], GFP_KERNEL)?;
> +        let mut next = 0;
> +
> +        // SAFETY: The C code returns a valid pointer here, which is again passed to the C code in
> +        // an array.
> +        attr[next] =
> +            unsafe { addr_of_mut!(bindings::cpufreq_freq_attr_scaling_available_freqs) as *mut _ };
> +        next += 1;
> +
> +        if boost {
> +            // SAFETY: The C code returns a valid pointer here, which is again passed to the C code
> +            // in an array.
> +            attr[next] =
> +                unsafe { addr_of_mut!(bindings::cpufreq_freq_attr_scaling_boost_freqs) as *mut _ };
> +            next += 1;
> +        }
> +        attr[next] = ptr::null_mut();
> +
> +        // Pass the ownership of the memory block to the C code. This will be freed when
> +        // the [`Registration`] object goes out of scope.
> +        drv.attr = Box::leak(attr) as *mut _;
> +
> +        // Initialize mandatory callbacks.
> +        drv.init = Some(Self::init_callback);
> +        drv.verify = Some(Self::verify_callback);
> +
> +        // Initialize optional callbacks.
> +        drv.setpolicy = if T::HAS_SETPOLICY {
> +            Some(Self::setpolicy_callback)
> +        } else {
> +            None
> +        };
> +        drv.target = if T::HAS_TARGET {
> +            Some(Self::target_callback)
> +        } else {
> +            None
> +        };
> +        drv.target_index = if T::HAS_TARGET_INDEX {
> +            Some(Self::target_index_callback)
> +        } else {
> +            None
> +        };
> +        drv.fast_switch = if T::HAS_FAST_SWITCH {
> +            Some(Self::fast_switch_callback)
> +        } else {
> +            None
> +        };
> +        drv.adjust_perf = if T::HAS_ADJUST_PERF {
> +            Some(Self::adjust_perf_callback)
> +        } else {
> +            None
> +        };
> +        drv.get_intermediate = if T::HAS_GET_INTERMEDIATE {
> +            Some(Self::get_intermediate_callback)
> +        } else {
> +            None
> +        };
> +        drv.target_intermediate = if T::HAS_TARGET_INTERMEDIATE {
> +            Some(Self::target_intermediate_callback)
> +        } else {
> +            None
> +        };
> +        drv.get = if T::HAS_GET {
> +            Some(Self::get_callback)
> +        } else {
> +            None
> +        };
> +        drv.update_limits = if T::HAS_UPDATE_LIMITS {
> +            Some(Self::update_limits_callback)
> +        } else {
> +            None
> +        };
> +        drv.bios_limit = if T::HAS_BIOS_LIMIT {
> +            Some(Self::bios_limit_callback)
> +        } else {
> +            None
> +        };
> +        drv.online = if T::HAS_ONLINE {
> +            Some(Self::online_callback)
> +        } else {
> +            None
> +        };
> +        drv.offline = if T::HAS_OFFLINE {
> +            Some(Self::offline_callback)
> +        } else {
> +            None
> +        };
> +        drv.exit = if T::HAS_EXIT {
> +            Some(Self::exit_callback)
> +        } else {
> +            None
> +        };
> +        drv.suspend = if T::HAS_SUSPEND {
> +            Some(Self::suspend_callback)
> +        } else {
> +            None
> +        };
> +        drv.resume = if T::HAS_RESUME {
> +            Some(Self::resume_callback)
> +        } else {
> +            None
> +        };
> +        drv.ready = if T::HAS_READY {
> +            Some(Self::ready_callback)
> +        } else {
> +            None
> +        };
> +        drv.set_boost = if T::HAS_SET_BOOST {
> +            Some(Self::set_boost_callback)
> +        } else {
> +            None
> +        };
> +        drv.register_em = if T::HAS_REGISTER_EM {
> +            Some(Self::register_em_callback)
> +        } else {
> +            None
> +        };
> +
> +        // Set driver data before registering the driver, as the cpufreq core may call few
> +        // callbacks before `cpufreq_register_driver()` returns.
> +        reg.set_data(data)?;
> +
> +        // SAFETY: It is safe to register the driver with the cpufreq core in the C code.
> +        to_result(unsafe { bindings::cpufreq_register_driver(reg.drv.get()) })?;
> +        reg.registered = true;
> +        Ok(reg)
> +    }
> +
> +    /// Returns the previous set data for a cpufreq driver.
> +    pub fn data<D: ForeignOwnable>() -> Option<<D>::Borrowed<'static>> {
> +        // SAFETY: The driver data is earlier set by us from [`set_data()`].
> +        let data = unsafe { bindings::cpufreq_get_driver_data() };
> +        if data.is_null() {
> +            None
> +        } else {
> +            // SAFETY: The driver data is earlier set by us from [`set_data()`].
> +            Some(unsafe { D::borrow(data) })
> +        }
> +    }
> +
> +    // Sets the data for a cpufreq driver.
> +    fn set_data(&mut self, data: T::Data) -> Result<()> {
> +        let drv = self.drv.get_mut();
> +
> +        if drv.driver_data.is_null() {
> +            // Pass the ownership of the data to the foreign interface.
> +            drv.driver_data = <T::Data as ForeignOwnable>::into_foreign(data) as _;
> +            Ok(())
> +        } else {
> +            Err(EBUSY)
> +        }
> +    }
> +
> +    // Clears and returns the data for a cpufreq driver.
> +    fn clear_data(&mut self) -> Option<T::Data> {
> +        let drv = self.drv.get_mut();
> +
> +        if drv.driver_data.is_null() {
> +            None
> +        } else {
> +            // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
> +            // relinquish it now.
> +            let data = Some(unsafe { <T::Data as ForeignOwnable>::from_foreign(drv.driver_data) });
> +            drv.driver_data = ptr::null_mut();
> +            data
> +        }
> +    }
> +}
> +
> +// cpufreq driver callbacks.
> +impl<T: Driver> Registration<T> {
> +    // Policy's init callback.
> +    extern "C" fn init_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
> +        from_result(|| {
> +            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
> +            // duration of this call, so it is guaranteed to remain alive for the lifetime of
> +            // `ptr`.
> +            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
> +
> +            let data = T::init(&mut policy)?;
> +            policy.set_data(data)?;
> +            Ok(0)
> +        })
> +    }
> +
> +    // Policy's exit callback.
> +    extern "C" fn exit_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
> +        from_result(|| {
> +            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
> +            // duration of this call, so it is guaranteed to remain alive for the lifetime of
> +            // `ptr`.
> +            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
> +
> +            let data = policy.clear_data();
> +            T::exit(&mut policy, data).map(|_| 0)
> +        })
> +    }
> +
> +    // Policy's online callback.
> +    extern "C" fn online_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
> +        from_result(|| {
> +            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
> +            // duration of this call, so it is guaranteed to remain alive for the lifetime of
> +            // `ptr`.
> +            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
> +            T::online(&mut policy).map(|_| 0)
> +        })
> +    }
> +
> +    // Policy's offline callback.
> +    extern "C" fn offline_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
> +        from_result(|| {
> +            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
> +            // duration of this call, so it is guaranteed to remain alive for the lifetime of
> +            // `ptr`.
> +            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
> +            T::offline(&mut policy).map(|_| 0)
> +        })
> +    }
> +
> +    // Policy's suspend callback.
> +    extern "C" fn suspend_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
> +        from_result(|| {
> +            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
> +            // duration of this call, so it is guaranteed to remain alive for the lifetime of
> +            // `ptr`.
> +            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
> +            T::suspend(&mut policy).map(|_| 0)
> +        })
> +    }
> +
> +    // Policy's resume callback.
> +    extern "C" fn resume_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
> +        from_result(|| {
> +            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
> +            // duration of this call, so it is guaranteed to remain alive for the lifetime of
> +            // `ptr`.
> +            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
> +            T::resume(&mut policy).map(|_| 0)
> +        })
> +    }
> +
> +    // Policy's ready callback.
> +    extern "C" fn ready_callback(ptr: *mut bindings::cpufreq_policy) {
> +        // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
> +        // duration of this call, so it is guaranteed to remain alive for the lifetime of
> +        // `ptr`.
> +        let mut policy = unsafe { Policy::from_raw_policy(ptr) };
> +        T::ready(&mut policy);
> +    }
> +
> +    // Policy's verify callback.
> +    extern "C" fn verify_callback(ptr: *mut bindings::cpufreq_policy_data) -> core::ffi::c_int {
> +        from_result(|| {
> +            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
> +            // duration of this call, so it is guaranteed to remain alive for the lifetime of
> +            // `ptr`.
> +            let mut data = unsafe { PolicyData::from_raw_policy_data(ptr) };
> +            T::verify(&mut data).map(|_| 0)
> +        })
> +    }
> +
> +    // Policy's setpolicy callback.
> +    extern "C" fn setpolicy_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
> +        from_result(|| {
> +            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
> +            // duration of this call, so it is guaranteed to remain alive for the lifetime of
> +            // `ptr`.
> +            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
> +            T::setpolicy(&mut policy).map(|_| 0)
> +        })
> +    }
> +
> +    // Policy's target callback.
> +    extern "C" fn target_callback(
> +        ptr: *mut bindings::cpufreq_policy,
> +        target_freq: u32,
> +        relation: u32,
> +    ) -> core::ffi::c_int {
> +        from_result(|| {
> +            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
> +            // duration of this call, so it is guaranteed to remain alive for the lifetime of
> +            // `ptr`.
> +            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
> +            T::target(&mut policy, target_freq, Relation::new(relation)?).map(|_| 0)
> +        })
> +    }
> +
> +    // Policy's target_index callback.
> +    extern "C" fn target_index_callback(
> +        ptr: *mut bindings::cpufreq_policy,
> +        index: u32,
> +    ) -> core::ffi::c_int {
> +        from_result(|| {
> +            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
> +            // duration of this call, so it is guaranteed to remain alive for the lifetime of
> +            // `ptr`.
> +            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
> +            T::target_index(&mut policy, index).map(|_| 0)
> +        })
> +    }
> +
> +    // Policy's fast_switch callback.
> +    extern "C" fn fast_switch_callback(
> +        ptr: *mut bindings::cpufreq_policy,
> +        target_freq: u32,
> +    ) -> core::ffi::c_uint {
> +        // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
> +        // duration of this call, so it is guaranteed to remain alive for the lifetime of
> +        // `ptr`.
> +        let mut policy = unsafe { Policy::from_raw_policy(ptr) };
> +        T::fast_switch(&mut policy, target_freq)
> +    }
> +
> +    // Policy's adjust_perf callback.
> +    extern "C" fn adjust_perf_callback(cpu: u32, min_perf: u64, target_perf: u64, capacity: u64) {
> +        if let Ok(mut policy) = Policy::from_cpu(cpu) {
> +            T::adjust_perf(&mut policy, min_perf, target_perf, capacity);
> +        }
> +    }
> +
> +    // Policy's get_intermediate callback.
> +    extern "C" fn get_intermediate_callback(
> +        ptr: *mut bindings::cpufreq_policy,
> +        index: u32,
> +    ) -> core::ffi::c_uint {
> +        // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
> +        // duration of this call, so it is guaranteed to remain alive for the lifetime of
> +        // `ptr`.
> +        let mut policy = unsafe { Policy::from_raw_policy(ptr) };
> +        T::get_intermediate(&mut policy, index)
> +    }
> +
> +    // Policy's target_intermediate callback.
> +    extern "C" fn target_intermediate_callback(
> +        ptr: *mut bindings::cpufreq_policy,
> +        index: u32,
> +    ) -> core::ffi::c_int {
> +        from_result(|| {
> +            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
> +            // duration of this call, so it is guaranteed to remain alive for the lifetime of
> +            // `ptr`.
> +            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
> +            T::target_intermediate(&mut policy, index).map(|_| 0)
> +        })
> +    }
> +
> +    // Policy's get callback.
> +    extern "C" fn get_callback(cpu: u32) -> core::ffi::c_uint {
> +        // SAFETY: Get the policy for a CPU.
> +        Policy::from_cpu(cpu).map_or(0, |mut policy| T::get(&mut policy).map_or(0, |f| f))
> +    }
> +
> +    // Policy's update_limit callback.
> +    extern "C" fn update_limits_callback(cpu: u32) {
> +        // SAFETY: Get the policy for a CPU.
> +        if let Ok(mut policy) = Policy::from_cpu(cpu) {
> +            T::update_limits(&mut policy);
> +        }
> +    }
> +
> +    // Policy's bios_limit callback.
> +    extern "C" fn bios_limit_callback(cpu: i32, limit: *mut u32) -> core::ffi::c_int {
> +        from_result(|| {
> +            let mut policy = Policy::from_cpu(cpu as u32)?;
> +
> +            // SAFETY: The pointer is guaranteed by the C code to be valid.
> +            T::bios_limit(&mut policy, &mut (unsafe { *limit })).map(|_| 0)
> +        })
> +    }
> +
> +    // Policy's set_boost callback.
> +    extern "C" fn set_boost_callback(
> +        ptr: *mut bindings::cpufreq_policy,
> +        state: i32,
> +    ) -> core::ffi::c_int {
> +        from_result(|| {
> +            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
> +            // duration of this call, so it is guaranteed to remain alive for the lifetime of
> +            // `ptr`.
> +            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
> +            T::set_boost(&mut policy, state).map(|_| 0)
> +        })
> +    }
> +
> +    // Policy's register_em callback.
> +    extern "C" fn register_em_callback(ptr: *mut bindings::cpufreq_policy) {
> +        // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
> +        // duration of this call, so it is guaranteed to remain alive for the lifetime of
> +        // `ptr`.
> +        let mut policy = unsafe { Policy::from_raw_policy(ptr) };
> +        T::register_em(&mut policy);
> +    }
> +}
> +
> +impl<T: Driver> Drop for Registration<T> {
> +    // Removes the registration from the kernel if it has completed successfully before.
> +    fn drop(&mut self) {
> +        pr_info!("Registration dropped\n");
> +        let drv = self.drv.get_mut();
> +
> +        if self.registered {
> +            // SAFETY: The driver was earlier registered from `register()`.
> +            unsafe { bindings::cpufreq_unregister_driver(drv) };
> +        }
> +
> +        // Free the previously leaked memory to the C code.
> +        if !drv.attr.is_null() {
> +            // SAFETY: The pointer was earlier initialized from the result of `Box::leak`.
> +            unsafe { drop(Box::from_raw(drv.attr)) };
> +        }
> +
> +        // Free data
> +        drop(self.clear_data());
> +    }
> +}

Re: [RFC PATCH V3 6/8] rust: Extend cpufreq bindings for driver registration

[Danilo Krummrich]

On 7/5/24 13:39, Danilo Krummrich wrote:
> On 7/3/24 09:14, Viresh Kumar wrote:
>> This extends the cpufreq bindings with bindings for registering a
>> driver.
>>
>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>> ---
>>   rust/kernel/cpufreq.rs | 482 ++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 479 insertions(+), 3 deletions(-)
>>
>> diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
>> index 6f9d34ebbcb0..66dad18f4ab6 100644
>> --- a/rust/kernel/cpufreq.rs
>> +++ b/rust/kernel/cpufreq.rs
>> @@ -9,14 +9,16 @@
>>   use crate::{
>>       bindings, clk, cpumask,
>>       device::Device,
>> -    error::{code::*, from_err_ptr, to_result, Result, VTABLE_DEFAULT_ERROR},
>> +    error::{code::*, from_err_ptr, from_result, to_result, Result, VTABLE_DEFAULT_ERROR},
>>       prelude::*,
>> -    types::{ARef, ForeignOwnable},
>> +    types::ForeignOwnable,
>>   };
>>   use core::{
>> +    cell::UnsafeCell,
>> +    marker::PhantomData,
>>       pin::Pin,
>> -    ptr::self,
>> +    ptr::{self, addr_of_mut},
>>   };
>>   use macros::vtable;
>> @@ -563,3 +565,477 @@ fn register_em(_policy: &mut Policy) {
>>           kernel::build_error(VTABLE_DEFAULT_ERROR)
>>       }
>>   }
>> +
>> +/// Registration of a cpufreq driver.
>> +pub struct Registration<T: Driver> {
>> +    registered: bool,
>> +    drv: UnsafeCell<bindings::cpufreq_driver>,
>> +    _p: PhantomData<T>,
>> +}
>> +
>> +// SAFETY: `Registration` doesn't offer any methods or access to fields when shared between threads
>> +// or CPUs, so it is safe to share it.
>> +unsafe impl<T: Driver> Sync for Registration<T> {}
>> +
>> +// SAFETY: Registration with and unregistration from the cpufreq subsystem can happen from any thread.
>> +// Additionally, `T::Data` (which is dropped during unregistration) is `Send`, so it is okay to move
>> +// `Registration` to different threads.
>> +#[allow(clippy::non_send_fields_in_send_ty)]
>> +unsafe impl<T: Driver> Send for Registration<T> {}
>> +
>> +impl<T: Driver> Registration<T> {
>> +    /// Creates new [`Registration`] but does not register it yet.
>> +    ///
>> +    /// It is allowed to move.
>> +    fn new() -> Result<Box<Self>> {
>> +        Ok(Box::new(
>> +            Self {
>> +                registered: false,
>> +                drv: UnsafeCell::new(bindings::cpufreq_driver::default()),
>> +                _p: PhantomData,
>> +            },
>> +            GFP_KERNEL,
>> +        )?)
>> +    }
>> +
>> +    /// Registers a cpufreq driver with the rest of the kernel.
>> +    pub fn register(
>> +        name: &'static CStr,
>> +        data: T::Data,
>> +        flags: u16,
>> +        boost: bool,
>> +    ) -> Result<Box<Self>> {
> 
> If you directly call `register` from `new` you can avoid having `Self::registered`.
> It's also a bit cleaner, once you got an instance of `Registration` it means something
> is registered, once it's dropped, it's unregistered.

Nevermind, I didn't notice `new` is private and you actually already do that. However,
this means you can drop `Self::registered`.

> 
>> +        let mut reg = Self::new()?;
>> +        let drv = reg.drv.get_mut();
>> +
>> +        // Account for the trailing null character.
>> +        let len = name.len() + 1;
>> +        if len > drv.name.len() {
>> +            return Err(EINVAL);
>> +        };
>> +
>> +        // SAFETY: `name` is a valid Cstr, and we are copying it to an array of equal or larger
>> +        // size.
>> +        let name = unsafe { &*(name.as_bytes_with_nul() as *const [u8] as *const [i8]) };
>> +        drv.name[..len].copy_from_slice(name);
>> +
>> +        drv.boost_enabled = boost;
>> +        drv.flags = flags;
>> +
>> +        // Allocate an array of 3 pointers to be passed to the C code.
>> +        let mut attr = Box::new([ptr::null_mut(); 3], GFP_KERNEL)?;
>> +        let mut next = 0;
>> +
>> +        // SAFETY: The C code returns a valid pointer here, which is again passed to the C code in
>> +        // an array.
>> +        attr[next] =
>> +            unsafe { addr_of_mut!(bindings::cpufreq_freq_attr_scaling_available_freqs) as *mut _ };
>> +        next += 1;
>> +
>> +        if boost {
>> +            // SAFETY: The C code returns a valid pointer here, which is again passed to the C code
>> +            // in an array.
>> +            attr[next] =
>> +                unsafe { addr_of_mut!(bindings::cpufreq_freq_attr_scaling_boost_freqs) as *mut _ };
>> +            next += 1;
>> +        }
>> +        attr[next] = ptr::null_mut();
>> +
>> +        // Pass the ownership of the memory block to the C code. This will be freed when
>> +        // the [`Registration`] object goes out of scope.
>> +        drv.attr = Box::leak(attr) as *mut _;
>> +
>> +        // Initialize mandatory callbacks.
>> +        drv.init = Some(Self::init_callback);
>> +        drv.verify = Some(Self::verify_callback);
>> +
>> +        // Initialize optional callbacks.
>> +        drv.setpolicy = if T::HAS_SETPOLICY {
>> +            Some(Self::setpolicy_callback)
>> +        } else {
>> +            None
>> +        };
>> +        drv.target = if T::HAS_TARGET {
>> +            Some(Self::target_callback)
>> +        } else {
>> +            None
>> +        };
>> +        drv.target_index = if T::HAS_TARGET_INDEX {
>> +            Some(Self::target_index_callback)
>> +        } else {
>> +            None
>> +        };
>> +        drv.fast_switch = if T::HAS_FAST_SWITCH {
>> +            Some(Self::fast_switch_callback)
>> +        } else {
>> +            None
>> +        };
>> +        drv.adjust_perf = if T::HAS_ADJUST_PERF {
>> +            Some(Self::adjust_perf_callback)
>> +        } else {
>> +            None
>> +        };
>> +        drv.get_intermediate = if T::HAS_GET_INTERMEDIATE {
>> +            Some(Self::get_intermediate_callback)
>> +        } else {
>> +            None
>> +        };
>> +        drv.target_intermediate = if T::HAS_TARGET_INTERMEDIATE {
>> +            Some(Self::target_intermediate_callback)
>> +        } else {
>> +            None
>> +        };
>> +        drv.get = if T::HAS_GET {
>> +            Some(Self::get_callback)
>> +        } else {
>> +            None
>> +        };
>> +        drv.update_limits = if T::HAS_UPDATE_LIMITS {
>> +            Some(Self::update_limits_callback)
>> +        } else {
>> +            None
>> +        };
>> +        drv.bios_limit = if T::HAS_BIOS_LIMIT {
>> +            Some(Self::bios_limit_callback)
>> +        } else {
>> +            None
>> +        };
>> +        drv.online = if T::HAS_ONLINE {
>> +            Some(Self::online_callback)
>> +        } else {
>> +            None
>> +        };
>> +        drv.offline = if T::HAS_OFFLINE {
>> +            Some(Self::offline_callback)
>> +        } else {
>> +            None
>> +        };
>> +        drv.exit = if T::HAS_EXIT {
>> +            Some(Self::exit_callback)
>> +        } else {
>> +            None
>> +        };
>> +        drv.suspend = if T::HAS_SUSPEND {
>> +            Some(Self::suspend_callback)
>> +        } else {
>> +            None
>> +        };
>> +        drv.resume = if T::HAS_RESUME {
>> +            Some(Self::resume_callback)
>> +        } else {
>> +            None
>> +        };
>> +        drv.ready = if T::HAS_READY {
>> +            Some(Self::ready_callback)
>> +        } else {
>> +            None
>> +        };
>> +        drv.set_boost = if T::HAS_SET_BOOST {
>> +            Some(Self::set_boost_callback)
>> +        } else {
>> +            None
>> +        };
>> +        drv.register_em = if T::HAS_REGISTER_EM {
>> +            Some(Self::register_em_callback)
>> +        } else {
>> +            None
>> +        };
>> +
>> +        // Set driver data before registering the driver, as the cpufreq core may call few
>> +        // callbacks before `cpufreq_register_driver()` returns.
>> +        reg.set_data(data)?;
>> +
>> +        // SAFETY: It is safe to register the driver with the cpufreq core in the C code.
>> +        to_result(unsafe { bindings::cpufreq_register_driver(reg.drv.get()) })?;
>> +        reg.registered = true;
>> +        Ok(reg)
>> +    }
>> +
>> +    /// Returns the previous set data for a cpufreq driver.
>> +    pub fn data<D: ForeignOwnable>() -> Option<<D>::Borrowed<'static>> {
>> +        // SAFETY: The driver data is earlier set by us from [`set_data()`].
>> +        let data = unsafe { bindings::cpufreq_get_driver_data() };
>> +        if data.is_null() {
>> +            None
>> +        } else {
>> +            // SAFETY: The driver data is earlier set by us from [`set_data()`].
>> +            Some(unsafe { D::borrow(data) })
>> +        }
>> +    }
>> +
>> +    // Sets the data for a cpufreq driver.
>> +    fn set_data(&mut self, data: T::Data) -> Result<()> {
>> +        let drv = self.drv.get_mut();
>> +
>> +        if drv.driver_data.is_null() {
>> +            // Pass the ownership of the data to the foreign interface.
>> +            drv.driver_data = <T::Data as ForeignOwnable>::into_foreign(data) as _;
>> +            Ok(())
>> +        } else {
>> +            Err(EBUSY)
>> +        }
>> +    }
>> +
>> +    // Clears and returns the data for a cpufreq driver.
>> +    fn clear_data(&mut self) -> Option<T::Data> {
>> +        let drv = self.drv.get_mut();
>> +
>> +        if drv.driver_data.is_null() {
>> +            None
>> +        } else {
>> +            // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
>> +            // relinquish it now.
>> +            let data = Some(unsafe { <T::Data as ForeignOwnable>::from_foreign(drv.driver_data) });
>> +            drv.driver_data = ptr::null_mut();
>> +            data
>> +        }
>> +    }
>> +}
>> +
>> +// cpufreq driver callbacks.
>> +impl<T: Driver> Registration<T> {
>> +    // Policy's init callback.
>> +    extern "C" fn init_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
>> +        from_result(|| {
>> +            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
>> +            // duration of this call, so it is guaranteed to remain alive for the lifetime of
>> +            // `ptr`.
>> +            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
>> +
>> +            let data = T::init(&mut policy)?;
>> +            policy.set_data(data)?;
>> +            Ok(0)
>> +        })
>> +    }
>> +
>> +    // Policy's exit callback.
>> +    extern "C" fn exit_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
>> +        from_result(|| {
>> +            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
>> +            // duration of this call, so it is guaranteed to remain alive for the lifetime of
>> +            // `ptr`.
>> +            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
>> +
>> +            let data = policy.clear_data();
>> +            T::exit(&mut policy, data).map(|_| 0)
>> +        })
>> +    }
>> +
>> +    // Policy's online callback.
>> +    extern "C" fn online_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
>> +        from_result(|| {
>> +            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
>> +            // duration of this call, so it is guaranteed to remain alive for the lifetime of
>> +            // `ptr`.
>> +            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
>> +            T::online(&mut policy).map(|_| 0)
>> +        })
>> +    }
>> +
>> +    // Policy's offline callback.
>> +    extern "C" fn offline_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
>> +        from_result(|| {
>> +            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
>> +            // duration of this call, so it is guaranteed to remain alive for the lifetime of
>> +            // `ptr`.
>> +            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
>> +            T::offline(&mut policy).map(|_| 0)
>> +        })
>> +    }
>> +
>> +    // Policy's suspend callback.
>> +    extern "C" fn suspend_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
>> +        from_result(|| {
>> +            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
>> +            // duration of this call, so it is guaranteed to remain alive for the lifetime of
>> +            // `ptr`.
>> +            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
>> +            T::suspend(&mut policy).map(|_| 0)
>> +        })
>> +    }
>> +
>> +    // Policy's resume callback.
>> +    extern "C" fn resume_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
>> +        from_result(|| {
>> +            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
>> +            // duration of this call, so it is guaranteed to remain alive for the lifetime of
>> +            // `ptr`.
>> +            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
>> +            T::resume(&mut policy).map(|_| 0)
>> +        })
>> +    }
>> +
>> +    // Policy's ready callback.
>> +    extern "C" fn ready_callback(ptr: *mut bindings::cpufreq_policy) {
>> +        // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
>> +        // duration of this call, so it is guaranteed to remain alive for the lifetime of
>> +        // `ptr`.
>> +        let mut policy = unsafe { Policy::from_raw_policy(ptr) };
>> +        T::ready(&mut policy);
>> +    }
>> +
>> +    // Policy's verify callback.
>> +    extern "C" fn verify_callback(ptr: *mut bindings::cpufreq_policy_data) -> core::ffi::c_int {
>> +        from_result(|| {
>> +            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
>> +            // duration of this call, so it is guaranteed to remain alive for the lifetime of
>> +            // `ptr`.
>> +            let mut data = unsafe { PolicyData::from_raw_policy_data(ptr) };
>> +            T::verify(&mut data).map(|_| 0)
>> +        })
>> +    }
>> +
>> +    // Policy's setpolicy callback.
>> +    extern "C" fn setpolicy_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
>> +        from_result(|| {
>> +            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
>> +            // duration of this call, so it is guaranteed to remain alive for the lifetime of
>> +            // `ptr`.
>> +            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
>> +            T::setpolicy(&mut policy).map(|_| 0)
>> +        })
>> +    }
>> +
>> +    // Policy's target callback.
>> +    extern "C" fn target_callback(
>> +        ptr: *mut bindings::cpufreq_policy,
>> +        target_freq: u32,
>> +        relation: u32,
>> +    ) -> core::ffi::c_int {
>> +        from_result(|| {
>> +            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
>> +            // duration of this call, so it is guaranteed to remain alive for the lifetime of
>> +            // `ptr`.
>> +            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
>> +            T::target(&mut policy, target_freq, Relation::new(relation)?).map(|_| 0)
>> +        })
>> +    }
>> +
>> +    // Policy's target_index callback.
>> +    extern "C" fn target_index_callback(
>> +        ptr: *mut bindings::cpufreq_policy,
>> +        index: u32,
>> +    ) -> core::ffi::c_int {
>> +        from_result(|| {
>> +            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
>> +            // duration of this call, so it is guaranteed to remain alive for the lifetime of
>> +            // `ptr`.
>> +            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
>> +            T::target_index(&mut policy, index).map(|_| 0)
>> +        })
>> +    }
>> +
>> +    // Policy's fast_switch callback.
>> +    extern "C" fn fast_switch_callback(
>> +        ptr: *mut bindings::cpufreq_policy,
>> +        target_freq: u32,
>> +    ) -> core::ffi::c_uint {
>> +        // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
>> +        // duration of this call, so it is guaranteed to remain alive for the lifetime of
>> +        // `ptr`.
>> +        let mut policy = unsafe { Policy::from_raw_policy(ptr) };
>> +        T::fast_switch(&mut policy, target_freq)
>> +    }
>> +
>> +    // Policy's adjust_perf callback.
>> +    extern "C" fn adjust_perf_callback(cpu: u32, min_perf: u64, target_perf: u64, capacity: u64) {
>> +        if let Ok(mut policy) = Policy::from_cpu(cpu) {
>> +            T::adjust_perf(&mut policy, min_perf, target_perf, capacity);
>> +        }
>> +    }
>> +
>> +    // Policy's get_intermediate callback.
>> +    extern "C" fn get_intermediate_callback(
>> +        ptr: *mut bindings::cpufreq_policy,
>> +        index: u32,
>> +    ) -> core::ffi::c_uint {
>> +        // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
>> +        // duration of this call, so it is guaranteed to remain alive for the lifetime of
>> +        // `ptr`.
>> +        let mut policy = unsafe { Policy::from_raw_policy(ptr) };
>> +        T::get_intermediate(&mut policy, index)
>> +    }
>> +
>> +    // Policy's target_intermediate callback.
>> +    extern "C" fn target_intermediate_callback(
>> +        ptr: *mut bindings::cpufreq_policy,
>> +        index: u32,
>> +    ) -> core::ffi::c_int {
>> +        from_result(|| {
>> +            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
>> +            // duration of this call, so it is guaranteed to remain alive for the lifetime of
>> +            // `ptr`.
>> +            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
>> +            T::target_intermediate(&mut policy, index).map(|_| 0)
>> +        })
>> +    }
>> +
>> +    // Policy's get callback.
>> +    extern "C" fn get_callback(cpu: u32) -> core::ffi::c_uint {
>> +        // SAFETY: Get the policy for a CPU.
>> +        Policy::from_cpu(cpu).map_or(0, |mut policy| T::get(&mut policy).map_or(0, |f| f))
>> +    }
>> +
>> +    // Policy's update_limit callback.
>> +    extern "C" fn update_limits_callback(cpu: u32) {
>> +        // SAFETY: Get the policy for a CPU.
>> +        if let Ok(mut policy) = Policy::from_cpu(cpu) {
>> +            T::update_limits(&mut policy);
>> +        }
>> +    }
>> +
>> +    // Policy's bios_limit callback.
>> +    extern "C" fn bios_limit_callback(cpu: i32, limit: *mut u32) -> core::ffi::c_int {
>> +        from_result(|| {
>> +            let mut policy = Policy::from_cpu(cpu as u32)?;
>> +
>> +            // SAFETY: The pointer is guaranteed by the C code to be valid.
>> +            T::bios_limit(&mut policy, &mut (unsafe { *limit })).map(|_| 0)
>> +        })
>> +    }
>> +
>> +    // Policy's set_boost callback.
>> +    extern "C" fn set_boost_callback(
>> +        ptr: *mut bindings::cpufreq_policy,
>> +        state: i32,
>> +    ) -> core::ffi::c_int {
>> +        from_result(|| {
>> +            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
>> +            // duration of this call, so it is guaranteed to remain alive for the lifetime of
>> +            // `ptr`.
>> +            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
>> +            T::set_boost(&mut policy, state).map(|_| 0)
>> +        })
>> +    }
>> +
>> +    // Policy's register_em callback.
>> +    extern "C" fn register_em_callback(ptr: *mut bindings::cpufreq_policy) {
>> +        // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
>> +        // duration of this call, so it is guaranteed to remain alive for the lifetime of
>> +        // `ptr`.
>> +        let mut policy = unsafe { Policy::from_raw_policy(ptr) };
>> +        T::register_em(&mut policy);
>> +    }
>> +}
>> +
>> +impl<T: Driver> Drop for Registration<T> {
>> +    // Removes the registration from the kernel if it has completed successfully before.
>> +    fn drop(&mut self) {
>> +        pr_info!("Registration dropped\n");
>> +        let drv = self.drv.get_mut();
>> +
>> +        if self.registered {
>> +            // SAFETY: The driver was earlier registered from `register()`.
>> +            unsafe { bindings::cpufreq_unregister_driver(drv) };
>> +        }
>> +
>> +        // Free the previously leaked memory to the C code.
>> +        if !drv.attr.is_null() {
>> +            // SAFETY: The pointer was earlier initialized from the result of `Box::leak`.
>> +            unsafe { drop(Box::from_raw(drv.attr)) };
>> +        }
>> +
>> +        // Free data
>> +        drop(self.clear_data());
>> +    }
>> +}

[RFC PATCH V3 7/8] rust: Extend OPP bindings with CPU frequency table

[Viresh Kumar]

This commit adds bindings for CPUFreq core related API.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 rust/kernel/opp.rs | 61 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 60 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
index 92c4ac6cb89c..bc16b8c407fb 100644
--- a/rust/kernel/opp.rs
+++ b/rust/kernel/opp.rs
@@ -16,7 +16,10 @@
     types::{ARef, AlwaysRefCounted, Opaque},
 };
 
-use core::{ffi::c_char, marker::PhantomData, ptr};
+#[cfg(CONFIG_CPU_FREQ)]
+use crate::cpufreq;
+
+use core::{ffi::c_char, marker::PhantomData, ops::Deref, ptr};
 
 use macros::vtable;
 
@@ -376,6 +379,56 @@ extern "C" fn config_regulators(
     }
 }
 
+/// CPU Frequency table created from OPP entries.
+#[cfg(CONFIG_CPU_FREQ)]
+pub struct FreqTable {
+    dev: ARef<Device>,
+    table: cpufreq::Table,
+}
+
+#[cfg(CONFIG_CPU_FREQ)]
+impl FreqTable {
+    /// Creates new instance of [`FreqTable`] from raw pointer.
+    fn new(table: &Table) -> Result<Self> {
+        let mut ptr: *mut bindings::cpufreq_frequency_table = ptr::null_mut();
+
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        to_result(unsafe {
+            bindings::dev_pm_opp_init_cpufreq_table(table.dev.as_raw(), &mut ptr)
+        })?;
+        Ok(Self {
+            dev: table.dev.clone(),
+            // SAFETY: The `ptr` is guaranteed by the C code to be valid.
+            table: unsafe { cpufreq::Table::from_raw(ptr) },
+        })
+    }
+
+    /// Returns reference to the underlying [`cpufreq::Table`].
+    pub fn table(&self) -> &cpufreq::Table {
+        &self.table
+    }
+}
+
+#[cfg(CONFIG_CPU_FREQ)]
+impl Deref for FreqTable {
+    type Target = cpufreq::Table;
+
+    #[inline]
+    fn deref(&self) -> &Self::Target {
+        &self.table
+    }
+}
+
+#[cfg(CONFIG_CPU_FREQ)]
+impl Drop for FreqTable {
+    fn drop(&mut self) {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // relinquish it now.
+        unsafe { bindings::dev_pm_opp_free_cpufreq_table(self.dev.as_raw(), &mut self.as_raw()) };
+    }
+}
+
 /// Operating performance point (OPP) table.
 ///
 /// # Invariants
@@ -580,6 +633,12 @@ pub fn adjust_voltage(
         })
     }
 
+    /// Create cpufreq table from OPP table.
+    #[cfg(CONFIG_CPU_FREQ)]
+    pub fn to_cpufreq_table(&mut self) -> Result<FreqTable> {
+        FreqTable::new(self)
+    }
+
     /// Sets a matching OPP based on frequency.
     pub fn set_rate(&self, freq: u64) -> Result<()> {
         // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
-- 
2.31.1.272.g89b43f80a514

[RFC PATCH V3 8/8] cpufreq: Add Rust based cpufreq-dt driver

[Viresh Kumar]

This commit adds a Rust based cpufreq-dt driver, which covers most of
the functionality of the existing C based driver. Only a handful of
things are left, like fetching platform data from cpufreq-dt-platdev.c.

This is tested with the help of QEMU for now and switching of
frequencies work as expected.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/Kconfig        |  12 ++
 drivers/cpufreq/Makefile       |   1 +
 drivers/cpufreq/rcpufreq_dt.rs | 225 +++++++++++++++++++++++++++++++++
 3 files changed, 238 insertions(+)
 create mode 100644 drivers/cpufreq/rcpufreq_dt.rs

diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
index 94e55c40970a..eb9359bd3c5c 100644
--- a/drivers/cpufreq/Kconfig
+++ b/drivers/cpufreq/Kconfig
@@ -217,6 +217,18 @@ config CPUFREQ_DT
 
 	  If in doubt, say N.
 
+config CPUFREQ_DT_RUST
+	tristate "Rust based Generic DT based cpufreq driver"
+	depends on HAVE_CLK && OF && RUST
+	select CPUFREQ_DT_PLATDEV
+	select PM_OPP
+	help
+	  This adds a Rust based generic DT based cpufreq driver for frequency
+	  management.  It supports both uniprocessor (UP) and symmetric
+	  multiprocessor (SMP) systems.
+
+	  If in doubt, say N.
+
 config CPUFREQ_DT_PLATDEV
 	tristate "Generic DT based cpufreq platdev driver"
 	depends on OF
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index 8d141c71b016..4981d908b803 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_CPU_FREQ_GOV_COMMON)		+= cpufreq_governor.o
 obj-$(CONFIG_CPU_FREQ_GOV_ATTR_SET)	+= cpufreq_governor_attr_set.o
 
 obj-$(CONFIG_CPUFREQ_DT)		+= cpufreq-dt.o
+obj-$(CONFIG_CPUFREQ_DT_RUST)		+= rcpufreq_dt.o
 obj-$(CONFIG_CPUFREQ_DT_PLATDEV)	+= cpufreq-dt-platdev.o
 
 # Traces
diff --git a/drivers/cpufreq/rcpufreq_dt.rs b/drivers/cpufreq/rcpufreq_dt.rs
new file mode 100644
index 000000000000..652458e7a3b9
--- /dev/null
+++ b/drivers/cpufreq/rcpufreq_dt.rs
@@ -0,0 +1,225 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Rust based implementation of the cpufreq-dt driver.
+
+use core::format_args;
+
+use kernel::{
+    b_str, c_str, clk, cpufreq, cpumask::Cpumask, define_of_id_table, device::Device,
+    error::code::*, fmt, macros::vtable, module_platform_driver, of, opp, platform, prelude::*,
+    str::CString, sync::Arc,
+};
+
+// Finds exact supply name from the OF node.
+fn find_supply_name_exact(np: &of::DeviceNode, name: &str) -> Option<CString> {
+    let name_cstr = CString::try_from_fmt(fmt!("{}-supply", name)).ok()?;
+
+    np.find_property(&name_cstr).ok()?;
+    CString::try_from_fmt(fmt!("{}", name)).ok()
+}
+
+// Finds supply name for the CPU from DT.
+fn find_supply_names(dev: &Device, cpu: u32) -> Option<Vec<CString>> {
+    let np = of::DeviceNode::from_dev(dev).ok()?;
+
+    // Try "cpu0" for older DTs.
+    let name = match cpu {
+        0 => find_supply_name_exact(&np, "cpu0"),
+        _ => None,
+    }
+    .or(find_supply_name_exact(&np, "cpu"))?;
+
+    let mut list = Vec::with_capacity(1, GFP_KERNEL).ok()?;
+    list.push(name, GFP_KERNEL).ok()?;
+
+    Some(list)
+}
+
+// Represents the cpufreq dt device.
+struct CPUFreqDTDevice {
+    opp_table: opp::Table,
+    freq_table: opp::FreqTable,
+    #[allow(dead_code)]
+    mask: Cpumask,
+    #[allow(dead_code)]
+    token: Option<opp::ConfigToken>,
+    #[allow(dead_code)]
+    clk: clk::Clk,
+}
+
+struct CPUFreqDTDriver;
+
+#[vtable]
+impl opp::ConfigOps for CPUFreqDTDriver {}
+
+#[vtable]
+impl cpufreq::Driver for CPUFreqDTDriver {
+    type Data = ();
+    type PData = Arc<CPUFreqDTDevice>;
+
+    fn init(policy: &mut cpufreq::Policy) -> Result<Self::PData> {
+        let cpu = policy.cpu();
+        let dev = Device::from_cpu(cpu)?;
+        let mut mask = Cpumask::new()?;
+
+        mask.set(cpu);
+
+        let token = match find_supply_names(&dev, cpu) {
+            Some(names) => Some(
+                opp::Config::<Self>::new()
+                    .set_regulator_names(names)?
+                    .set(&dev)?,
+            ),
+            _ => None,
+        };
+
+        // Get OPP-sharing information from "operating-points-v2" bindings.
+        let fallback = match opp::Table::of_sharing_cpus(&dev, &mut mask) {
+            Ok(_) => false,
+            Err(e) => {
+                if e != ENOENT {
+                    return Err(e);
+                }
+
+                // "operating-points-v2" not supported. If the platform hasn't
+                // set sharing CPUs, fallback to all CPUs share the `Policy`
+                // for backward compatibility.
+                opp::Table::sharing_cpus(&dev, &mut mask).is_err()
+            }
+        };
+
+        // Initialize OPP tables for all policy cpus.
+        //
+        // For platforms not using "operating-points-v2" bindings, we do this
+        // before updating policy cpus. Otherwise, we will end up creating
+        // duplicate OPPs for the CPUs.
+        //
+        // OPPs might be populated at runtime, don't fail for error here unless
+        // it is -EPROBE_DEFER.
+        let mut opp_table = match opp::Table::from_of_cpumask(&dev, &mask) {
+            Ok(table) => table,
+            Err(e) => {
+                if e == EPROBE_DEFER {
+                    return Err(e);
+                }
+
+                // The table is added dynamically ?
+                opp::Table::from_dev(&dev)?
+            }
+        };
+
+        // The OPP table must be initialized, statically or dynamically, by this point.
+        opp_table.opp_count()?;
+
+        // Set sharing cpus for fallback scenario.
+        if fallback {
+            mask.set_all();
+            opp_table.set_sharing_cpus(&mask)?;
+        }
+
+        let mut transition_latency = opp_table.max_transition_latency() as u32;
+        if transition_latency == 0 {
+            transition_latency = cpufreq::ETERNAL_LATENCY;
+        }
+
+        let freq_table = opp_table.to_cpufreq_table()?;
+        let clk = policy
+            .set_freq_table(freq_table.table())
+            .set_dvfs_possible_from_any_cpu()
+            .set_suspend_freq((opp_table.suspend_freq() / 1000) as u32)
+            .set_transition_latency(transition_latency)
+            .set_clk(&dev, None)?;
+
+        mask.copy(policy.cpus());
+
+        Ok(Arc::new(
+            CPUFreqDTDevice {
+                opp_table,
+                freq_table,
+                mask,
+                token,
+                clk,
+            },
+            GFP_KERNEL,
+        )?)
+    }
+
+    fn exit(_policy: &mut cpufreq::Policy, _data: Option<Self::PData>) -> Result<()> {
+        Ok(())
+    }
+
+    fn online(_policy: &mut cpufreq::Policy) -> Result<()> {
+        // We did light-weight tear down earlier, nothing to do here.
+        Ok(())
+    }
+
+    fn offline(_policy: &mut cpufreq::Policy) -> Result<()> {
+        // Preserve policy->data and don't free resources on light-weight
+        // tear down.
+        Ok(())
+    }
+
+    fn suspend(policy: &mut cpufreq::Policy) -> Result<()> {
+        policy.generic_suspend()
+    }
+
+    fn verify(data: &mut cpufreq::PolicyData) -> Result<()> {
+        data.generic_verify()
+    }
+
+    fn target_index(policy: &mut cpufreq::Policy, index: u32) -> Result<()> {
+        let data = match policy.data::<Self::PData>() {
+            Some(data) => data,
+            None => return Err(ENOENT),
+        };
+
+        let freq = data.freq_table.freq(index.try_into().unwrap())? as u64;
+        data.opp_table.set_rate(freq * 1000)
+    }
+
+    fn get(policy: &mut cpufreq::Policy) -> Result<u32> {
+        policy.generic_get()
+    }
+
+    fn set_boost(_policy: &mut cpufreq::Policy, _state: i32) -> Result<()> {
+        Ok(())
+    }
+
+    fn register_em(policy: &mut cpufreq::Policy) {
+        policy.register_em_opp()
+    }
+}
+
+type DeviceData = Box<cpufreq::Registration<CPUFreqDTDriver>>;
+
+impl platform::Driver for CPUFreqDTDriver {
+    type Data = Arc<DeviceData>;
+
+    define_of_id_table! {(), [
+        (of::DeviceId(b_str!("operating-points-v2")), None),
+    ]}
+
+    fn probe(_dev: &mut platform::Device, _id_info: Option<&Self::IdInfo>) -> Result<Self::Data> {
+        let drv = Arc::new(
+            cpufreq::Registration::<CPUFreqDTDriver>::register(
+                c_str!("cpufreq-dt"),
+                (),
+                cpufreq::flags::NEED_INITIAL_FREQ_CHECK | cpufreq::flags::IS_COOLING_DEV,
+                true,
+            )?,
+            GFP_KERNEL,
+        )?;
+
+        pr_info!("CPUFreq DT driver registered\n");
+
+        Ok(drv)
+    }
+}
+
+module_platform_driver! {
+    type: CPUFreqDTDriver,
+    name: "cpufreq_dt",
+    author: "Viresh Kumar <viresh.kumar@linaro.org>",
+    description: "Generic CPUFreq DT driver",
+    license: "GPL v2",
+}
-- 
2.31.1.272.g89b43f80a514

Re: [RFC PATCH V3 8/8] cpufreq: Add Rust based cpufreq-dt driver

[Danilo Krummrich]

Hi Viresh,

On 7/3/24 09:14, Viresh Kumar wrote:
> This commit adds a Rust based cpufreq-dt driver, which covers most of
> the functionality of the existing C based driver. Only a handful of
> things are left, like fetching platform data from cpufreq-dt-platdev.c.
> 
> This is tested with the help of QEMU for now and switching of
> frequencies work as expected.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>   drivers/cpufreq/Kconfig        |  12 ++
>   drivers/cpufreq/Makefile       |   1 +
>   drivers/cpufreq/rcpufreq_dt.rs | 225 +++++++++++++++++++++++++++++++++
>   3 files changed, 238 insertions(+)
>   create mode 100644 drivers/cpufreq/rcpufreq_dt.rs
> 
> diff --git a/drivers/cpufreq/rcpufreq_dt.rs b/drivers/cpufreq/rcpufreq_dt.rs
> new file mode 100644
> index 000000000000..652458e7a3b9
> --- /dev/null
> +++ b/drivers/cpufreq/rcpufreq_dt.rs

<snip>

> +
> +type DeviceData = Box<cpufreq::Registration<CPUFreqDTDriver>>;
> +
> +impl platform::Driver for CPUFreqDTDriver {
> +    type Data = Arc<DeviceData>;
> +
> +    define_of_id_table! {(), [
> +        (of::DeviceId(b_str!("operating-points-v2")), None),
> +    ]}
> +
> +    fn probe(_dev: &mut platform::Device, _id_info: Option<&Self::IdInfo>) -> Result<Self::Data> {
> +        let drv = Arc::new(
> +            cpufreq::Registration::<CPUFreqDTDriver>::register(
> +                c_str!("cpufreq-dt"),
> +                (),
> +                cpufreq::flags::NEED_INITIAL_FREQ_CHECK | cpufreq::flags::IS_COOLING_DEV,
> +                true,
> +            )?,
> +            GFP_KERNEL,
> +        )?;

Putting the `cpufreq::Registration` into `Arc<DeviceData>` is unsafe from a
lifetime point of view. Nothing prevents this `Arc` to out-live the
`platform::Driver`.

Instead, you should wrap `cpufreq::Registration` into `Devres`. See
`drm::drv::Registration` for an example [1].

[1] https://gitlab.freedesktop.org/drm/nova/-/blob/nova-next/rust/kernel/drm/drv.rs?ref_type=heads#L173

> +
> +        pr_info!("CPUFreq DT driver registered\n");
> +
> +        Ok(drv)
> +    }
> +}
> +
> +module_platform_driver! {
> +    type: CPUFreqDTDriver,
> +    name: "cpufreq_dt",
> +    author: "Viresh Kumar <viresh.kumar@linaro.org>",
> +    description: "Generic CPUFreq DT driver",
> +    license: "GPL v2",
> +}

Re: [RFC PATCH V3 8/8] cpufreq: Add Rust based cpufreq-dt driver

[Viresh Kumar]

On 05-07-24, 13:32, Danilo Krummrich wrote:
> On 7/3/24 09:14, Viresh Kumar wrote:
> > +    fn probe(_dev: &mut platform::Device, _id_info: Option<&Self::IdInfo>) -> Result<Self::Data> {
> > +        let drv = Arc::new(
> > +            cpufreq::Registration::<CPUFreqDTDriver>::register(
> > +                c_str!("cpufreq-dt"),
> > +                (),
> > +                cpufreq::flags::NEED_INITIAL_FREQ_CHECK | cpufreq::flags::IS_COOLING_DEV,
> > +                true,
> > +            )?,
> > +            GFP_KERNEL,
> > +        )?;
> 
> Putting the `cpufreq::Registration` into `Arc<DeviceData>` is unsafe from a
> lifetime point of view. Nothing prevents this `Arc` to out-live the
> `platform::Driver`.

Hmm, the platform driver layer (in Rust) should guarantee that the
data will be freed from the driver removal path. Isn't it ?

> Instead, you should wrap `cpufreq::Registration` into `Devres`. See
> `drm::drv::Registration` for an example [1].
> 
> [1] https://gitlab.freedesktop.org/drm/nova/-/blob/nova-next/rust/kernel/drm/drv.rs?ref_type=heads#L173

I can convert to that too, will do it anyway.

-- 
viresh

Re: [RFC PATCH V3 8/8] cpufreq: Add Rust based cpufreq-dt driver

[Danilo Krummrich]

On Wed, Jul 10, 2024 at 02:26:52PM +0530, Viresh Kumar wrote:
> On 05-07-24, 13:32, Danilo Krummrich wrote:
> > On 7/3/24 09:14, Viresh Kumar wrote:
> > > +    fn probe(_dev: &mut platform::Device, _id_info: Option<&Self::IdInfo>) -> Result<Self::Data> {
> > > +        let drv = Arc::new(
> > > +            cpufreq::Registration::<CPUFreqDTDriver>::register(
> > > +                c_str!("cpufreq-dt"),
> > > +                (),
> > > +                cpufreq::flags::NEED_INITIAL_FREQ_CHECK | cpufreq::flags::IS_COOLING_DEV,
> > > +                true,
> > > +            )?,
> > > +            GFP_KERNEL,
> > > +        )?;
> > 
> > Putting the `cpufreq::Registration` into `Arc<DeviceData>` is unsafe from a
> > lifetime point of view. Nothing prevents this `Arc` to out-live the
> > `platform::Driver`.
> 
> Hmm, the platform driver layer (in Rust) should guarantee that the
> data will be freed from the driver removal path. Isn't it ?

No, the platform driver layer will only guarantee that it decreses the reference
count of the `Arc` by one, that doesn't guarantee a free. If something else
still holds a reference to the `Arc` it will keep the `Registration` alive,
unless it's wrapped by `Devres`.

> 
> > Instead, you should wrap `cpufreq::Registration` into `Devres`. See
> > `drm::drv::Registration` for an example [1].
> > 
> > [1] https://gitlab.freedesktop.org/drm/nova/-/blob/nova-next/rust/kernel/drm/drv.rs?ref_type=heads#L173
> 
> I can convert to that too, will do it anyway.
> 
> -- 
> viresh
> 

Re: [RFC PATCH V3 8/8] cpufreq: Add Rust based cpufreq-dt driver

[Viresh Kumar]

On 10-07-24, 17:28, Danilo Krummrich wrote:
> No, the platform driver layer will only guarantee that it decreses the reference
> count of the `Arc` by one, that doesn't guarantee a free. If something else
> still holds a reference to the `Arc` it will keep the `Registration` alive,
> unless it's wrapped by `Devres`.

I see. Thanks.

There is one problem that I haven't found a solution to yet. If I make
the following change to the driver:

diff --git a/drivers/cpufreq/rcpufreq_dt.rs b/drivers/cpufreq/rcpufreq_dt.rs
index 315adca2a747..052ea2db095a 100644
--- a/drivers/cpufreq/rcpufreq_dt.rs
+++ b/drivers/cpufreq/rcpufreq_dt.rs
@@ -236,7 +236,7 @@ fn probe(dev: &mut platform::Device, _id_info: Option<&Self::IdInfo>) -> Result<

 module_platform_driver! {
     type: CPUFreqDTDriver,
-    name: "cpufreq_dt",
+    name: "cpufreq-dt",
     author: "Viresh Kumar <viresh.kumar@linaro.org>",
     description: "Generic CPUFreq DT driver",
     license: "GPL v2",

then I get this error:

  CLIPPY     drivers/cpufreq/rcpufreq_dt.o
error: expected one of `:`, `;`, or `=`, found `-`
   --> /mnt/ssd/all/work/repos/kernel/linux/drivers/cpufreq/rcpufreq_dt.rs:237:1
    |
237 | / module_platform_driver! {
238 | |     type: CPUFreqDTDriver,
239 | |     name: "cpufreq-dt",
240 | |     author: "Viresh Kumar <viresh.kumar@linaro.org>",
241 | |     description: "Generic CPUFreq DT driver",
242 | |     license: "GPL v2",
243 | | }
    | |_^ expected one of `:`, `;`, or `=`
    |
    = note: this error originates in the macro
    `$crate::prelude::module` which comes from the expansion of the
    macro `module_platform_driver` (in Nightly builds, run with -Z
    macro-backtrace for more info)


And because of that I had to change the name of the platform device
too in the existing kernel.

-- 
viresh