Extend the cpufreq abstractions to support driver registration from
Rust.
Reviewed-by: Danilo Krummrich <dakr@kernel.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
rust/kernel/cpufreq.rs | 493 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 490 insertions(+), 3 deletions(-)
diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
index 4e6d85bd06f4..826710c4f4b0 100644
--- a/rust/kernel/cpufreq.rs
+++ b/rust/kernel/cpufreq.rs
@@ -11,9 +11,10 @@
use crate::{
clk::Hertz,
cpumask,
- device::Device,
- error::{code::*, from_err_ptr, to_result, Result, VTABLE_DEFAULT_ERROR},
- ffi::c_ulong,
+ device::{Bound, Device},
+ devres::Devres,
+ error::{code::*, from_err_ptr, from_result, to_result, Result, VTABLE_DEFAULT_ERROR},
+ ffi::{c_char, c_ulong},
prelude::*,
types::ForeignOwnable,
types::Opaque,
@@ -23,6 +24,9 @@
use crate::clk::Clk;
use core::{
+ cell::UnsafeCell,
+ marker::PhantomData,
+ mem::MaybeUninit,
ops::{Deref, DerefMut},
pin::Pin,
ptr,
@@ -30,6 +34,9 @@
use macros::vtable;
+/// Maximum length of CPU frequency driver's name.
+const CPUFREQ_NAME_LEN: usize = bindings::CPUFREQ_NAME_LEN as usize;
+
/// Default transition latency value in nanoseconds.
pub const ETERNAL_LATENCY_NS: u32 = bindings::CPUFREQ_ETERNAL as u32;
@@ -821,3 +828,483 @@ fn register_em(_policy: &mut Policy) {
build_error!(VTABLE_DEFAULT_ERROR)
}
}
+
+/// CPU frequency driver Registration.
+///
+/// ## Examples
+///
+/// The following example demonstrates how to register a cpufreq driver.
+///
+/// ```
+/// use kernel::{
+/// cpu, cpufreq,
+/// c_str,
+/// device::{Bound, Device},
+/// macros::vtable,
+/// sync::Arc,
+/// };
+/// struct FooDevice;
+///
+/// #[derive(Default)]
+/// struct FooDriver;
+///
+/// #[vtable]
+/// impl cpufreq::Driver for FooDriver {
+/// const NAME: &'static CStr = c_str!("cpufreq-foo");
+/// const FLAGS: u16 = cpufreq::flags::NEED_INITIAL_FREQ_CHECK | cpufreq::flags::IS_COOLING_DEV;
+/// const BOOST_ENABLED: bool = true;
+///
+/// type PData = Arc<FooDevice>;
+///
+/// fn init(policy: &mut cpufreq::Policy) -> Result<Self::PData> {
+/// // Initialize here
+/// Ok(Arc::new(FooDevice, GFP_KERNEL)?)
+/// }
+///
+/// fn exit(_policy: &mut cpufreq::Policy, _data: Option<Self::PData>) -> Result<()> {
+/// 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: cpufreq::TableIndex) -> Result<()> {
+/// // Update CPU frequency
+/// Ok(())
+/// }
+///
+/// fn get(policy: &mut cpufreq::Policy) -> Result<u32> {
+/// policy.generic_get()
+/// }
+/// }
+///
+/// fn foo_probe(dev: &Device<Bound>) {
+/// cpufreq::Registration::<FooDriver>::new_foreign_owned(dev).unwrap();
+/// }
+/// ```
+#[repr(transparent)]
+pub struct Registration<T: Driver>(KBox<UnsafeCell<bindings::cpufreq_driver>>, 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> {}
+
+#[allow(clippy::non_send_fields_in_send_ty)]
+/// SAFETY: Registration with and unregistration from the cpufreq subsystem can happen from any
+/// thread.
+unsafe impl<T: Driver> Send for Registration<T> {}
+
+impl<T: Driver> Registration<T> {
+ const VTABLE: bindings::cpufreq_driver = bindings::cpufreq_driver {
+ name: Self::copy_name(T::NAME),
+ boost_enabled: T::BOOST_ENABLED,
+ flags: T::FLAGS,
+
+ // Initialize mandatory callbacks.
+ init: Some(Self::init_callback),
+ verify: Some(Self::verify_callback),
+
+ // Initialize optional callbacks based on the traits of `T`.
+ setpolicy: if T::HAS_SETPOLICY {
+ Some(Self::setpolicy_callback)
+ } else {
+ None
+ },
+ target: if T::HAS_TARGET {
+ Some(Self::target_callback)
+ } else {
+ None
+ },
+ target_index: if T::HAS_TARGET_INDEX {
+ Some(Self::target_index_callback)
+ } else {
+ None
+ },
+ fast_switch: if T::HAS_FAST_SWITCH {
+ Some(Self::fast_switch_callback)
+ } else {
+ None
+ },
+ adjust_perf: if T::HAS_ADJUST_PERF {
+ Some(Self::adjust_perf_callback)
+ } else {
+ None
+ },
+ get_intermediate: if T::HAS_GET_INTERMEDIATE {
+ Some(Self::get_intermediate_callback)
+ } else {
+ None
+ },
+ target_intermediate: if T::HAS_TARGET_INTERMEDIATE {
+ Some(Self::target_intermediate_callback)
+ } else {
+ None
+ },
+ get: if T::HAS_GET {
+ Some(Self::get_callback)
+ } else {
+ None
+ },
+ update_limits: if T::HAS_UPDATE_LIMITS {
+ Some(Self::update_limits_callback)
+ } else {
+ None
+ },
+ bios_limit: if T::HAS_BIOS_LIMIT {
+ Some(Self::bios_limit_callback)
+ } else {
+ None
+ },
+ online: if T::HAS_ONLINE {
+ Some(Self::online_callback)
+ } else {
+ None
+ },
+ offline: if T::HAS_OFFLINE {
+ Some(Self::offline_callback)
+ } else {
+ None
+ },
+ exit: if T::HAS_EXIT {
+ Some(Self::exit_callback)
+ } else {
+ None
+ },
+ suspend: if T::HAS_SUSPEND {
+ Some(Self::suspend_callback)
+ } else {
+ None
+ },
+ resume: if T::HAS_RESUME {
+ Some(Self::resume_callback)
+ } else {
+ None
+ },
+ ready: if T::HAS_READY {
+ Some(Self::ready_callback)
+ } else {
+ None
+ },
+ set_boost: if T::HAS_SET_BOOST {
+ Some(Self::set_boost_callback)
+ } else {
+ None
+ },
+ register_em: if T::HAS_REGISTER_EM {
+ Some(Self::register_em_callback)
+ } else {
+ None
+ },
+ // SAFETY: All zeros is a valid value for `bindings::cpufreq_driver`.
+ ..unsafe { MaybeUninit::zeroed().assume_init() }
+ };
+
+ const fn copy_name(name: &'static CStr) -> [c_char; CPUFREQ_NAME_LEN] {
+ let src = name.as_bytes_with_nul();
+ let mut dst = [0; CPUFREQ_NAME_LEN];
+
+ build_assert!(src.len() <= CPUFREQ_NAME_LEN);
+
+ let mut i = 0;
+ while i < src.len() {
+ dst[i] = src[i];
+ i += 1;
+ }
+
+ dst
+ }
+
+ /// Registers a CPU frequency driver with the cpufreq core.
+ pub fn new() -> Result<Self> {
+ // We can't use `&Self::VTABLE` directly because the cpufreq core modifies some fields in
+ // the C `struct cpufreq_driver`, which requires a mutable reference.
+ let mut drv = KBox::new(UnsafeCell::new(Self::VTABLE), GFP_KERNEL)?;
+
+ // SAFETY: `drv` is guaranteed to be valid for the lifetime of `Registration`.
+ to_result(unsafe { bindings::cpufreq_register_driver(drv.get_mut()) })?;
+
+ Ok(Self(drv, PhantomData))
+ }
+
+ /// Same as [`Registration::new`], but does not return a [`Registration`] instance.
+ ///
+ /// Instead the [`Registration`] is owned by [`Devres`] and will be revoked / dropped, once the
+ /// device is detached.
+ pub fn new_foreign_owned(dev: &Device<Bound>) -> Result<()> {
+ Devres::new_foreign_owned(dev, Self::new()?, GFP_KERNEL)
+ }
+}
+
+/// CPU frequency driver callbacks.
+impl<T: Driver> Registration<T> {
+ /// Driver's `init` callback.
+ ///
+ /// SAFETY: Called from C. Inputs must be valid pointers.
+ extern "C" fn init_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::ffi::c_int {
+ from_result(|| {
+ // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the
+ // lifetime of `policy`.
+ let policy = unsafe { Policy::from_raw_mut(ptr) };
+
+ let data = T::init(policy)?;
+ policy.set_data(data)?;
+ Ok(0)
+ })
+ }
+
+ /// Driver's `exit` callback.
+ ///
+ /// SAFETY: Called from C. Inputs must be valid pointers.
+ extern "C" fn exit_callback(ptr: *mut bindings::cpufreq_policy) {
+ // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the
+ // lifetime of `policy`.
+ let policy = unsafe { Policy::from_raw_mut(ptr) };
+
+ let data = policy.clear_data();
+ let _ = T::exit(policy, data);
+ }
+
+ /// Driver's `online` callback.
+ ///
+ /// SAFETY: Called from C. Inputs must be valid pointers.
+ extern "C" fn online_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::ffi::c_int {
+ from_result(|| {
+ // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the
+ // lifetime of `policy`.
+ let policy = unsafe { Policy::from_raw_mut(ptr) };
+ T::online(policy).map(|()| 0)
+ })
+ }
+
+ /// Driver's `offline` callback.
+ ///
+ /// SAFETY: Called from C. Inputs must be valid pointers.
+ extern "C" fn offline_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::ffi::c_int {
+ from_result(|| {
+ // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the
+ // lifetime of `policy`.
+ let policy = unsafe { Policy::from_raw_mut(ptr) };
+ T::offline(policy).map(|()| 0)
+ })
+ }
+
+ /// Driver's `suspend` callback.
+ ///
+ /// SAFETY: Called from C. Inputs must be valid pointers.
+ extern "C" fn suspend_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::ffi::c_int {
+ from_result(|| {
+ // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the
+ // lifetime of `policy`.
+ let policy = unsafe { Policy::from_raw_mut(ptr) };
+ T::suspend(policy).map(|()| 0)
+ })
+ }
+
+ /// Driver's `resume` callback.
+ ///
+ /// SAFETY: Called from C. Inputs must be valid pointers.
+ extern "C" fn resume_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::ffi::c_int {
+ from_result(|| {
+ // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the
+ // lifetime of `policy`.
+ let policy = unsafe { Policy::from_raw_mut(ptr) };
+ T::resume(policy).map(|()| 0)
+ })
+ }
+
+ /// Driver's `ready` callback.
+ ///
+ /// SAFETY: Called from C. Inputs must be valid pointers.
+ extern "C" fn ready_callback(ptr: *mut bindings::cpufreq_policy) {
+ // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the
+ // lifetime of `policy`.
+ let policy = unsafe { Policy::from_raw_mut(ptr) };
+ T::ready(policy);
+ }
+
+ /// Driver's `verify` callback.
+ ///
+ /// SAFETY: Called from C. Inputs must be valid pointers.
+ extern "C" fn verify_callback(ptr: *mut bindings::cpufreq_policy_data) -> kernel::ffi::c_int {
+ from_result(|| {
+ // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the
+ // lifetime of `policy`.
+ let data = unsafe { PolicyData::from_raw_mut(ptr) };
+ T::verify(data).map(|()| 0)
+ })
+ }
+
+ /// Driver's `setpolicy` callback.
+ ///
+ /// SAFETY: Called from C. Inputs must be valid pointers.
+ extern "C" fn setpolicy_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::ffi::c_int {
+ from_result(|| {
+ // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the
+ // lifetime of `policy`.
+ let policy = unsafe { Policy::from_raw_mut(ptr) };
+ T::setpolicy(policy).map(|()| 0)
+ })
+ }
+
+ /// Driver's `target` callback.
+ ///
+ /// SAFETY: Called from C. Inputs must be valid pointers.
+ extern "C" fn target_callback(
+ ptr: *mut bindings::cpufreq_policy,
+ target_freq: u32,
+ relation: u32,
+ ) -> kernel::ffi::c_int {
+ from_result(|| {
+ // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the
+ // lifetime of `policy`.
+ let policy = unsafe { Policy::from_raw_mut(ptr) };
+ T::target(policy, target_freq, Relation::new(relation)?).map(|()| 0)
+ })
+ }
+
+ /// Driver's `target_index` callback.
+ ///
+ /// SAFETY: Called from C. Inputs must be valid pointers.
+ extern "C" fn target_index_callback(
+ ptr: *mut bindings::cpufreq_policy,
+ index: u32,
+ ) -> kernel::ffi::c_int {
+ from_result(|| {
+ // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the
+ // lifetime of `policy`.
+ let policy = unsafe { Policy::from_raw_mut(ptr) };
+
+ // SAFETY: The C code guarantees that `index` corresponds to a valid entry in the
+ // frequency table.
+ let index = unsafe { TableIndex::new(index as usize) };
+
+ T::target_index(policy, index).map(|()| 0)
+ })
+ }
+
+ /// Driver's `fast_switch` callback.
+ ///
+ /// SAFETY: Called from C. Inputs must be valid pointers.
+ extern "C" fn fast_switch_callback(
+ ptr: *mut bindings::cpufreq_policy,
+ target_freq: u32,
+ ) -> kernel::ffi::c_uint {
+ // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the
+ // lifetime of `policy`.
+ let policy = unsafe { Policy::from_raw_mut(ptr) };
+ T::fast_switch(policy, target_freq)
+ }
+
+ /// Driver's `adjust_perf` callback.
+ extern "C" fn adjust_perf_callback(
+ cpu: u32,
+ min_perf: usize,
+ target_perf: usize,
+ capacity: usize,
+ ) {
+ if let Ok(mut policy) = PolicyCpu::from_cpu(cpu) {
+ T::adjust_perf(&mut policy, min_perf, target_perf, capacity);
+ }
+ }
+
+ /// Driver's `get_intermediate` callback.
+ ///
+ /// SAFETY: Called from C. Inputs must be valid pointers.
+ extern "C" fn get_intermediate_callback(
+ ptr: *mut bindings::cpufreq_policy,
+ index: u32,
+ ) -> kernel::ffi::c_uint {
+ // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the
+ // lifetime of `policy`.
+ let policy = unsafe { Policy::from_raw_mut(ptr) };
+
+ // SAFETY: The C code guarantees that `index` corresponds to a valid entry in the
+ // frequency table.
+ let index = unsafe { TableIndex::new(index as usize) };
+
+ T::get_intermediate(policy, index)
+ }
+
+ /// Driver's `target_intermediate` callback.
+ ///
+ /// SAFETY: Called from C. Inputs must be valid pointers.
+ extern "C" fn target_intermediate_callback(
+ ptr: *mut bindings::cpufreq_policy,
+ index: u32,
+ ) -> kernel::ffi::c_int {
+ from_result(|| {
+ // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the
+ // lifetime of `policy`.
+ let policy = unsafe { Policy::from_raw_mut(ptr) };
+
+ // SAFETY: The C code guarantees that `index` corresponds to a valid entry in the
+ // frequency table.
+ let index = unsafe { TableIndex::new(index as usize) };
+
+ T::target_intermediate(policy, index).map(|()| 0)
+ })
+ }
+
+ /// Driver's `get` callback.
+ extern "C" fn get_callback(cpu: u32) -> kernel::ffi::c_uint {
+ PolicyCpu::from_cpu(cpu).map_or(0, |mut policy| T::get(&mut policy).map_or(0, |f| f))
+ }
+
+ /// Driver's `update_limit` callback.
+ extern "C" fn update_limits_callback(ptr: *mut bindings::cpufreq_policy) {
+ // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the
+ // lifetime of `policy`.
+ let policy = unsafe { Policy::from_raw_mut(ptr) };
+ T::update_limits(policy);
+ }
+
+ /// Driver's `bios_limit` callback.
+ ///
+ /// SAFETY: Called from C. Inputs must be valid pointers.
+ extern "C" fn bios_limit_callback(cpu: i32, limit: *mut u32) -> kernel::ffi::c_int {
+ from_result(|| {
+ let mut policy = PolicyCpu::from_cpu(cpu as u32)?;
+
+ // SAFETY: `limit` is guaranteed by the C code to be valid.
+ T::bios_limit(&mut policy, &mut (unsafe { *limit })).map(|()| 0)
+ })
+ }
+
+ /// Driver's `set_boost` callback.
+ ///
+ /// SAFETY: Called from C. Inputs must be valid pointers.
+ extern "C" fn set_boost_callback(
+ ptr: *mut bindings::cpufreq_policy,
+ state: i32,
+ ) -> kernel::ffi::c_int {
+ from_result(|| {
+ // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the
+ // lifetime of `policy`.
+ let policy = unsafe { Policy::from_raw_mut(ptr) };
+ T::set_boost(policy, state).map(|()| 0)
+ })
+ }
+
+ /// Driver's `register_em` callback.
+ ///
+ /// SAFETY: Called from C. Inputs must be valid pointers.
+ extern "C" fn register_em_callback(ptr: *mut bindings::cpufreq_policy) {
+ // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the
+ // lifetime of `policy`.
+ let policy = unsafe { Policy::from_raw_mut(ptr) };
+ T::register_em(policy);
+ }
+}
+
+impl<T: Driver> Drop for Registration<T> {
+ /// Unregisters with the cpufreq core.
+ fn drop(&mut self) {
+ // SAFETY: `self.0` is guaranteed to be valid for the lifetime of `Registration`.
+ unsafe { bindings::cpufreq_unregister_driver(self.0.get_mut()) };
+ }
+}
--
2.31.1.272.g89b43f80a514On Mon, May 19, 2025 at 12:37:18PM +0530, Viresh Kumar wrote:
> +/// CPU frequency driver Registration.
> +///
> +/// ## Examples
> +///
> +/// The following example demonstrates how to register a cpufreq driver.
> +///
> +/// ```
> +/// use kernel::{
> +/// cpu, cpufreq,
> +/// c_str,
> +/// device::{Bound, Device},
> +/// macros::vtable,
> +/// sync::Arc,
> +/// };
> +/// struct FooDevice;
> +///
> +/// #[derive(Default)]
> +/// struct FooDriver;
> +///
> +/// #[vtable]
> +/// impl cpufreq::Driver for FooDriver {
> +/// const NAME: &'static CStr = c_str!("cpufreq-foo");
> +/// const FLAGS: u16 = cpufreq::flags::NEED_INITIAL_FREQ_CHECK | cpufreq::flags::IS_COOLING_DEV;
> +/// const BOOST_ENABLED: bool = true;
> +///
> +/// type PData = Arc<FooDevice>;
> +///
> +/// fn init(policy: &mut cpufreq::Policy) -> Result<Self::PData> {
> +/// // Initialize here
> +/// Ok(Arc::new(FooDevice, GFP_KERNEL)?)
> +/// }
> +///
> +/// fn exit(_policy: &mut cpufreq::Policy, _data: Option<Self::PData>) -> Result<()> {
This can just be `Result`, here and below.
> +/// 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: cpufreq::TableIndex) -> Result<()> {
> +/// // Update CPU frequency
> +/// Ok(())
> +/// }
> +///
> +/// fn get(policy: &mut cpufreq::Policy) -> Result<u32> {
> +/// policy.generic_get()
> +/// }
> +/// }
> +///
> +/// fn foo_probe(dev: &Device<Bound>) {
You could use a real probe function, e.g. from platform:
# struct Driver;
impl platform::Driver for SampleDriver {
# type IdInfo = ();
# const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None;
fn probe(
pdev: &platform::Device<Core>,
info: Option<&Self::IdInfo>,
) -> Result<Pin<KBox<Self>>> {
...
}
}
> +/// cpufreq::Registration::<FooDriver>::new_foreign_owned(dev).unwrap();
I prefer if we do not use unwrap() in doctests, since they also serve as example
and people might think that calling unwrap() is valid thing to do.
Sorry, I didn't catch the above in my previous review -- fine for me if you do
those improvements in a subsequent patch.
On 19-05-25, 13:06, Danilo Krummrich wrote:
> Sorry, I didn't catch the above in my previous review -- fine for me if you do
> those improvements in a subsequent patch.
That's fine. Thanks a lot for reviewing the series.
--
viresh
diff --git a/drivers/cpufreq/rcpufreq_dt.rs b/drivers/cpufreq/rcpufreq_dt.rs
index d0e60b7db81f..94ed81644fe1 100644
--- a/drivers/cpufreq/rcpufreq_dt.rs
+++ b/drivers/cpufreq/rcpufreq_dt.rs
@@ -152,30 +152,30 @@ fn init(policy: &mut cpufreq::Policy) -> Result<Self::PData> {
)?)
}
- fn exit(_policy: &mut cpufreq::Policy, _data: Option<Self::PData>) -> Result<()> {
+ fn exit(_policy: &mut cpufreq::Policy, _data: Option<Self::PData>) -> Result {
Ok(())
}
- fn online(_policy: &mut cpufreq::Policy) -> Result<()> {
+ 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<()> {
+ 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<()> {
+ fn suspend(policy: &mut cpufreq::Policy) -> Result {
policy.generic_suspend()
}
- fn verify(data: &mut cpufreq::PolicyData) -> Result<()> {
+ fn verify(data: &mut cpufreq::PolicyData) -> Result {
data.generic_verify()
}
- fn target_index(policy: &mut cpufreq::Policy, index: cpufreq::TableIndex) -> Result<()> {
+ fn target_index(policy: &mut cpufreq::Policy, index: cpufreq::TableIndex) -> Result {
let Some(data) = policy.data::<Self::PData>() else {
return Err(ENOENT);
};
@@ -188,7 +188,7 @@ fn get(policy: &mut cpufreq::Policy) -> Result<u32> {
policy.generic_get()
}
- fn set_boost(_policy: &mut cpufreq::Policy, _state: i32) -> Result<()> {
+ fn set_boost(_policy: &mut cpufreq::Policy, _state: i32) -> Result {
Ok(())
}
@@ -213,10 +213,7 @@ fn probe(
_id_info: Option<&Self::IdInfo>,
) -> Result<Pin<KBox<Self>>> {
cpufreq::Registration::<CPUFreqDTDriver>::new_foreign_owned(pdev.as_ref())?;
-
- let drvdata = KBox::new(Self {}, GFP_KERNEL)?;
-
- Ok(drvdata.into())
+ Ok(KBox::new(Self {}, GFP_KERNEL)?.into())
}
}
diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
index 826710c4f4b0..09b856bb297b 100644
--- a/rust/kernel/cpufreq.rs
+++ b/rust/kernel/cpufreq.rs
@@ -154,7 +154,7 @@ pub fn as_raw(&self) -> *mut bindings::cpufreq_policy_data {
/// Wrapper for `cpufreq_generic_frequency_table_verify`.
#[inline]
- pub fn generic_verify(&self) -> Result<()> {
+ pub fn generic_verify(&self) -> Result {
// SAFETY: By the type invariant, the pointer stored in `self` is valid.
to_result(unsafe { bindings::cpufreq_generic_frequency_table_verify(self.as_raw()) })
}
@@ -208,15 +208,16 @@ fn from(index: TableIndex) -> Self {
/// ```
/// use kernel::cpufreq::{Policy, TableIndex};
///
-/// fn show_freq(policy: &Policy) {
-/// let table = policy.freq_table().unwrap();
+/// fn show_freq(policy: &Policy) -> Result {
+/// let table = policy.freq_table()?;
///
/// // SAFETY: Index is a valid entry in the table.
/// let index = unsafe { TableIndex::new(0) };
///
-/// pr_info!("The frequency at index 0 is: {:?}\n", table.freq(index).unwrap());
+/// pr_info!("The frequency at index 0 is: {:?}\n", table.freq(index)?);
/// pr_info!("The flags at index 0 is: {}\n", table.flags(index));
/// pr_info!("The data at index 0 is: {}\n", table.data(index));
+/// Ok(())
/// }
/// ```
#[repr(transparent)]
@@ -361,7 +362,7 @@ pub fn new() -> Self {
}
/// Adds a new entry to the table.
- pub fn add(&mut self, freq: Hertz, flags: u32, driver_data: u32) -> Result<()> {
+ pub fn add(&mut self, freq: Hertz, flags: u32, driver_data: u32) -> Result {
// Adds the new entry at the end of the vector.
Ok(self.entries.push(
bindings::cpufreq_frequency_table {
@@ -515,7 +516,7 @@ pub fn set_suspend_freq(&mut self, freq: Hertz) -> &mut Self {
/// Provides a wrapper to the generic suspend routine.
#[inline]
- pub fn generic_suspend(&mut self) -> Result<()> {
+ pub fn generic_suspend(&mut self) -> Result {
// SAFETY: By the type invariant, the pointer stored in `self` is valid.
to_result(unsafe { bindings::cpufreq_generic_suspend(self.as_mut_ref()) })
}
@@ -643,7 +644,7 @@ pub fn data<T: ForeignOwnable>(&mut self) -> Option<<T>::Borrowed<'_>> {
/// # Errors
///
/// Returns `EBUSY` if private data is already set.
- fn set_data<T: ForeignOwnable>(&mut self, data: T) -> Result<()> {
+ fn set_data<T: ForeignOwnable>(&mut self, data: T) -> Result {
if self.as_ref().driver_data.is_null() {
// Transfer the ownership of the data to the foreign interface.
self.as_mut_ref().driver_data = <T as ForeignOwnable>::into_foreign(data) as _;
@@ -736,27 +737,27 @@ pub trait Driver {
fn init(policy: &mut Policy) -> Result<Self::PData>;
/// Driver's `exit` callback.
- fn exit(_policy: &mut Policy, _data: Option<Self::PData>) -> Result<()> {
+ fn exit(_policy: &mut Policy, _data: Option<Self::PData>) -> Result {
build_error!(VTABLE_DEFAULT_ERROR)
}
/// Driver's `online` callback.
- fn online(_policy: &mut Policy) -> Result<()> {
+ fn online(_policy: &mut Policy) -> Result {
build_error!(VTABLE_DEFAULT_ERROR)
}
/// Driver's `offline` callback.
- fn offline(_policy: &mut Policy) -> Result<()> {
+ fn offline(_policy: &mut Policy) -> Result {
build_error!(VTABLE_DEFAULT_ERROR)
}
/// Driver's `suspend` callback.
- fn suspend(_policy: &mut Policy) -> Result<()> {
+ fn suspend(_policy: &mut Policy) -> Result {
build_error!(VTABLE_DEFAULT_ERROR)
}
/// Driver's `resume` callback.
- fn resume(_policy: &mut Policy) -> Result<()> {
+ fn resume(_policy: &mut Policy) -> Result {
build_error!(VTABLE_DEFAULT_ERROR)
}
@@ -766,20 +767,20 @@ fn ready(_policy: &mut Policy) {
}
/// Driver's `verify` callback.
- fn verify(data: &mut PolicyData) -> Result<()>;
+ fn verify(data: &mut PolicyData) -> Result;
/// Driver's `setpolicy` callback.
- fn setpolicy(_policy: &mut Policy) -> Result<()> {
+ fn setpolicy(_policy: &mut Policy) -> Result {
build_error!(VTABLE_DEFAULT_ERROR)
}
/// Driver's `target` callback.
- fn target(_policy: &mut Policy, _target_freq: u32, _relation: Relation) -> Result<()> {
+ fn target(_policy: &mut Policy, _target_freq: u32, _relation: Relation) -> Result {
build_error!(VTABLE_DEFAULT_ERROR)
}
/// Driver's `target_index` callback.
- fn target_index(_policy: &mut Policy, _index: TableIndex) -> Result<()> {
+ fn target_index(_policy: &mut Policy, _index: TableIndex) -> Result {
build_error!(VTABLE_DEFAULT_ERROR)
}
@@ -799,7 +800,7 @@ fn get_intermediate(_policy: &mut Policy, _index: TableIndex) -> u32 {
}
/// Driver's `target_intermediate` callback.
- fn target_intermediate(_policy: &mut Policy, _index: TableIndex) -> Result<()> {
+ fn target_intermediate(_policy: &mut Policy, _index: TableIndex) -> Result {
build_error!(VTABLE_DEFAULT_ERROR)
}
@@ -814,12 +815,12 @@ fn update_limits(_policy: &mut Policy) {
}
/// Driver's `bios_limit` callback.
- fn bios_limit(_policy: &mut Policy, _limit: &mut u32) -> Result<()> {
+ fn bios_limit(_policy: &mut Policy, _limit: &mut u32) -> Result {
build_error!(VTABLE_DEFAULT_ERROR)
}
/// Driver's `set_boost` callback.
- fn set_boost(_policy: &mut Policy, _state: i32) -> Result<()> {
+ fn set_boost(_policy: &mut Policy, _state: i32) -> Result {
build_error!(VTABLE_DEFAULT_ERROR)
}
@@ -837,43 +838,44 @@ fn register_em(_policy: &mut Policy) {
///
/// ```
/// use kernel::{
-/// cpu, cpufreq,
+/// cpufreq,
/// c_str,
-/// device::{Bound, Device},
+/// device::{Core, Device},
/// macros::vtable,
+/// of, platform,
/// sync::Arc,
/// };
-/// struct FooDevice;
+/// struct SampleDevice;
///
/// #[derive(Default)]
-/// struct FooDriver;
+/// struct SampleDriver;
///
/// #[vtable]
-/// impl cpufreq::Driver for FooDriver {
-/// const NAME: &'static CStr = c_str!("cpufreq-foo");
+/// impl cpufreq::Driver for SampleDriver {
+/// const NAME: &'static CStr = c_str!("cpufreq-sample");
/// const FLAGS: u16 = cpufreq::flags::NEED_INITIAL_FREQ_CHECK | cpufreq::flags::IS_COOLING_DEV;
/// const BOOST_ENABLED: bool = true;
///
-/// type PData = Arc<FooDevice>;
+/// type PData = Arc<SampleDevice>;
///
/// fn init(policy: &mut cpufreq::Policy) -> Result<Self::PData> {
/// // Initialize here
-/// Ok(Arc::new(FooDevice, GFP_KERNEL)?)
+/// Ok(Arc::new(SampleDevice, GFP_KERNEL)?)
/// }
///
-/// fn exit(_policy: &mut cpufreq::Policy, _data: Option<Self::PData>) -> Result<()> {
+/// fn exit(_policy: &mut cpufreq::Policy, _data: Option<Self::PData>) -> Result {
/// Ok(())
/// }
///
-/// fn suspend(policy: &mut cpufreq::Policy) -> Result<()> {
+/// fn suspend(policy: &mut cpufreq::Policy) -> Result {
/// policy.generic_suspend()
/// }
///
-/// fn verify(data: &mut cpufreq::PolicyData) -> Result<()> {
+/// fn verify(data: &mut cpufreq::PolicyData) -> Result {
/// data.generic_verify()
/// }
///
-/// fn target_index(policy: &mut cpufreq::Policy, index: cpufreq::TableIndex) -> Result<()> {
+/// fn target_index(policy: &mut cpufreq::Policy, index: cpufreq::TableIndex) -> Result {
/// // Update CPU frequency
/// Ok(())
/// }
@@ -883,8 +885,17 @@ fn register_em(_policy: &mut Policy) {
/// }
/// }
///
-/// fn foo_probe(dev: &Device<Bound>) {
-/// cpufreq::Registration::<FooDriver>::new_foreign_owned(dev).unwrap();
+/// impl platform::Driver for SampleDriver {
+/// type IdInfo = ();
+/// const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None;
+///
+/// fn probe(
+/// pdev: &platform::Device<Core>,
+/// _id_info: Option<&Self::IdInfo>,
+/// ) -> Result<Pin<KBox<Self>>> {
+/// cpufreq::Registration::<SampleDriver>::new_foreign_owned(pdev.as_ref())?;
+/// Ok(KBox::new(Self {}, GFP_KERNEL)?.into())
+/// }
/// }
/// ```
#[repr(transparent)]
@@ -1035,7 +1046,7 @@ pub fn new() -> Result<Self> {
///
/// Instead the [`Registration`] is owned by [`Devres`] and will be revoked / dropped, once the
/// device is detached.
- pub fn new_foreign_owned(dev: &Device<Bound>) -> Result<()> {
+ pub fn new_foreign_owned(dev: &Device<Bound>) -> Result {
Devres::new_foreign_owned(dev, Self::new()?, GFP_KERNEL)
}
}
diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
index 1e5fd9887b3a..212555dacd45 100644
--- a/rust/kernel/opp.rs
+++ b/rust/kernel/opp.rs
@@ -292,7 +292,7 @@ pub enum SearchType {
pub trait ConfigOps {
/// This is typically used to scale clocks when transitioning between OPPs.
#[inline]
- fn config_clks(_dev: &Device, _table: &Table, _opp: &OPP, _scaling_down: bool) -> Result<()> {
+ fn config_clks(_dev: &Device, _table: &Table, _opp: &OPP, _scaling_down: bool) -> Result {
build_error!(VTABLE_DEFAULT_ERROR)
}
@@ -304,7 +304,7 @@ fn config_regulators(
_opp_new: &OPP,
_data: *mut *mut bindings::regulator,
_count: u32,
- ) -> Result<()> {
+ ) -> Result {
build_error!(VTABLE_DEFAULT_ERROR)
}
}
@@ -753,7 +753,7 @@ pub fn suspend_freq(&self) -> Hertz {
/// Synchronizes regulators used by the [`Table`].
#[inline]
- pub fn sync_regulators(&self) -> Result<()> {
+ 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()) })
@@ -761,14 +761,14 @@ pub fn sync_regulators(&self) -> Result<()> {
/// Gets sharing CPUs.
#[inline]
- pub fn sharing_cpus(dev: &Device, cpumask: &mut Cpumask) -> Result<()> {
+ 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_raw()) })
}
/// Sets sharing CPUs.
- pub fn set_sharing_cpus(&mut self, cpumask: &mut Cpumask) -> Result<()> {
+ pub fn set_sharing_cpus(&mut self, cpumask: &mut Cpumask) -> Result {
// SAFETY: The requirements are satisfied by the existence of [`Device`] and its safety
// requirements.
to_result(unsafe {
@@ -786,7 +786,7 @@ pub fn set_sharing_cpus(&mut self, cpumask: &mut Cpumask) -> Result<()> {
/// Gets sharing CPUs from device tree.
#[cfg(CONFIG_OF)]
#[inline]
- pub fn of_sharing_cpus(dev: &Device, cpumask: &mut Cpumask) -> Result<()> {
+ 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 {
@@ -802,7 +802,7 @@ pub fn adjust_voltage(
volt: MicroVolt,
volt_min: MicroVolt,
volt_max: MicroVolt,
- ) -> Result<()> {
+ ) -> Result {
// SAFETY: The requirements are satisfied by the existence of [`Device`] and its safety
// requirements.
to_result(unsafe {
@@ -825,7 +825,7 @@ pub fn cpufreq_table(&mut self) -> Result<FreqTable> {
/// Configures device with [`OPP`] matching the frequency value.
#[inline]
- pub fn set_rate(&self, freq: Hertz) -> Result<()> {
+ pub fn set_rate(&self, freq: Hertz) -> 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.into()) })
@@ -833,7 +833,7 @@ pub fn set_rate(&self, freq: Hertz) -> Result<()> {
/// Configures device with [`OPP`].
#[inline]
- pub fn set_opp(&self, opp: &OPP) -> Result<()> {
+ 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()) })
@@ -937,7 +937,7 @@ pub fn opp_from_bw(&self, mut bw: u32, index: i32, stype: SearchType) -> Result<
/// Enables the [`OPP`].
#[inline]
- pub fn enable_opp(&self, freq: Hertz) -> Result<()> {
+ pub fn enable_opp(&self, freq: Hertz) -> 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.into()) })
@@ -945,7 +945,7 @@ pub fn enable_opp(&self, freq: Hertz) -> Result<()> {
/// Disables the [`OPP`].
#[inline]
- pub fn disable_opp(&self, freq: Hertz) -> Result<()> {
+ pub fn disable_opp(&self, freq: Hertz) -> 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.into()) })
@@ -953,7 +953,7 @@ pub fn disable_opp(&self, freq: Hertz) -> Result<()> {
/// Registers with the Energy model.
#[cfg(CONFIG_OF)]
- pub fn of_register_em(&mut self, cpumask: &mut Cpumask) -> Result<()> {
+ 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 {
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index 27cc72d474f0..6ff34096d7ee 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -185,9 +185,9 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
let info = ModuleInfo::parse(&mut it);
- /* Rust does not allow hyphens in identifiers, use underscore instead */
- let name_identifier = info.name.replace('-', "_");
- let mut modinfo = ModInfoBuilder::new(name_identifier.as_ref());
+ // Rust does not allow hyphens in identifiers, use underscore instead.
+ let ident = info.name.replace('-', "_");
+ let mut modinfo = ModInfoBuilder::new(ident.as_ref());
if let Some(author) = info.author {
modinfo.emit("author", &author);
}
@@ -312,15 +312,15 @@ mod __module_init {{
#[doc(hidden)]
#[link_section = \"{initcall_section}\"]
#[used]
- pub static __{name_identifier}_initcall: extern \"C\" fn() ->
- kernel::ffi::c_int = __{name_identifier}_init;
+ pub static __{ident}_initcall: extern \"C\" fn() ->
+ kernel::ffi::c_int = __{ident}_init;
#[cfg(not(MODULE))]
#[cfg(CONFIG_HAVE_ARCH_PREL32_RELOCATIONS)]
core::arch::global_asm!(
r#\".section \"{initcall_section}\", \"a\"
- __{name_identifier}_initcall:
- .long __{name_identifier}_init - .
+ __{ident}_initcall:
+ .long __{ident}_init - .
.previous
\"#
);
@@ -328,7 +328,7 @@ mod __module_init {{
#[cfg(not(MODULE))]
#[doc(hidden)]
#[no_mangle]
- pub extern \"C\" fn __{name_identifier}_init() -> kernel::ffi::c_int {{
+ pub extern \"C\" fn __{ident}_init() -> kernel::ffi::c_int {{
// SAFETY: This function is inaccessible to the outside due to the double
// module wrapping it. It is called exactly once by the C side via its
// placement above in the initcall section.
@@ -338,12 +338,12 @@ mod __module_init {{
#[cfg(not(MODULE))]
#[doc(hidden)]
#[no_mangle]
- pub extern \"C\" fn __{name_identifier}_exit() {{
+ pub extern \"C\" fn __{ident}_exit() {{
// SAFETY:
// - This function is inaccessible to the outside due to the double
// module wrapping it. It is called exactly once by the C side via its
// unique name,
- // - furthermore it is only called after `__{name_identifier}_init` has
+ // - furthermore it is only called after `__{ident}_init` has
// returned `0` (which delegates to `__init`).
unsafe {{ __exit() }}
}}
@@ -384,7 +384,7 @@ unsafe fn __exit() {{
",
type_ = info.type_,
name = info.name,
- name_identifier = name_identifier,
+ ident = ident,
modinfo = modinfo.buffer,
initcall_section = ".initcall6.init"
)
On Mon May 19, 2025 at 1:06 PM CEST, Danilo Krummrich wrote:
> On Mon, May 19, 2025 at 12:37:18PM +0530, Viresh Kumar wrote:
>> +/// fn exit(_policy: &mut cpufreq::Policy, _data: Option<Self::PData>) -> Result<()> {
>
> This can just be `Result`, here and below.
Since I saw you mention this multiple times and I agree, I created a
clippy issue: https://github.com/rust-lang/rust-clippy/issues/14848
---
Cheers,
Benno
On Mon, May 19, 2025 at 1:41 PM Benno Lossin <lossin@kernel.org> wrote: > > Since I saw you mention this multiple times and I agree, I created a > clippy issue: https://github.com/rust-lang/rust-clippy/issues/14848 This is https://github.com/Rust-for-Linux/linux/issues/1128 -- I agree having it done in Clippy in a general way would be ideal. Thanks! Cheers, Miguel
© 2016 - 2025 Red Hat, Inc.