Re: [PATCH V8 12/14] rust: Extend cpufreq bindings for driver registration
From: Viresh Kumar
Date: Fri Feb 07 2025 - 04:15:50 EST
On 06-02-25, 13:04, Danilo Krummrich wrote:
> > + unsafe { drop(KBox::from_raw(drv.attr)) };
>
> This could just be
>
> let _ = unsafe { KBox::from_raw(drv.attr) };
>
> At least drop() should not be within the unsafe block.
>
> > + }
> > +
> > + // Free data
> > + drop(self.clear_data());
>
> No need for drop(), but I also don't mind to be explicit.
For both of these I kept the explicit drop() to avoid any potential
confusion. I do prefer them.
--
viresh
-------------------------8<-------------------------
diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
index ecf7c6e2cb89..d2e7913e170b 100644
--- a/rust/kernel/cpufreq.rs
+++ b/rust/kernel/cpufreq.rs
@@ -645,7 +645,7 @@ pub fn new(name: &'static CStr, data: T::Data, flags: u16, boost: bool) -> Resul
// Pass the ownership of the memory block to the C code. This will be freed when
// the [`Registration`] object goes out of scope.
- drv_ref.attr = KBox::leak(attr) as *mut _;
+ drv_ref.attr = KBox::into_raw(attr) as *mut _;
// Initialize mandatory callbacks.
drv_ref.init = Some(Self::init_callback);
@@ -813,7 +813,7 @@ fn clear_data(&mut self) -> Option<T::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 {
+ extern "C" fn init_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::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
@@ -838,7 +838,7 @@ extern "C" fn exit_callback(ptr: *mut bindings::cpufreq_policy) {
}
// Policy's online callback.
- extern "C" fn online_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
+ extern "C" fn online_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::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
@@ -849,7 +849,7 @@ extern "C" fn online_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::
}
// Policy's offline callback.
- extern "C" fn offline_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
+ extern "C" fn offline_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::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
@@ -860,7 +860,7 @@ extern "C" fn offline_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi:
}
// Policy's suspend callback.
- extern "C" fn suspend_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
+ extern "C" fn suspend_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::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
@@ -871,7 +871,7 @@ extern "C" fn suspend_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi:
}
// Policy's resume callback.
- extern "C" fn resume_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
+ extern "C" fn resume_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::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
@@ -890,7 +890,7 @@ extern "C" fn ready_callback(ptr: *mut bindings::cpufreq_policy) {
}
// Policy's verify callback.
- extern "C" fn verify_callback(ptr: *mut bindings::cpufreq_policy_data) -> core::ffi::c_int {
+ extern "C" fn verify_callback(ptr: *mut bindings::cpufreq_policy_data) -> kernel::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
@@ -901,7 +901,7 @@ extern "C" fn verify_callback(ptr: *mut bindings::cpufreq_policy_data) -> core::
}
// Policy's setpolicy callback.
- extern "C" fn setpolicy_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
+ extern "C" fn setpolicy_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::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
@@ -916,7 +916,7 @@ extern "C" fn target_callback(
ptr: *mut bindings::cpufreq_policy,
target_freq: u32,
relation: u32,
- ) -> core::ffi::c_int {
+ ) -> kernel::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
@@ -930,7 +930,7 @@ extern "C" fn target_callback(
extern "C" fn target_index_callback(
ptr: *mut bindings::cpufreq_policy,
index: u32,
- ) -> core::ffi::c_int {
+ ) -> kernel::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
@@ -944,7 +944,7 @@ extern "C" fn target_index_callback(
extern "C" fn fast_switch_callback(
ptr: *mut bindings::cpufreq_policy,
target_freq: u32,
- ) -> core::ffi::c_uint {
+ ) -> kernel::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) };
@@ -967,7 +967,7 @@ extern "C" fn adjust_perf_callback(
extern "C" fn get_intermediate_callback(
ptr: *mut bindings::cpufreq_policy,
index: u32,
- ) -> core::ffi::c_uint {
+ ) -> kernel::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) };
@@ -978,7 +978,7 @@ extern "C" fn get_intermediate_callback(
extern "C" fn target_intermediate_callback(
ptr: *mut bindings::cpufreq_policy,
index: u32,
- ) -> core::ffi::c_int {
+ ) -> kernel::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
@@ -989,7 +989,7 @@ extern "C" fn target_intermediate_callback(
}
// Policy's get callback.
- extern "C" fn get_callback(cpu: u32) -> core::ffi::c_uint {
+ extern "C" fn get_callback(cpu: u32) -> kernel::ffi::c_uint {
Policy::from_cpu(cpu).map_or(0, |mut policy| T::get(&mut policy).map_or(0, |f| f))
}
@@ -1001,7 +1001,7 @@ extern "C" fn update_limits_callback(cpu: u32) {
}
// Policy's bios_limit callback.
- extern "C" fn bios_limit_callback(cpu: i32, limit: *mut u32) -> core::ffi::c_int {
+ extern "C" fn bios_limit_callback(cpu: i32, limit: *mut u32) -> kernel::ffi::c_int {
from_result(|| {
let mut policy = Policy::from_cpu(cpu as u32)?;
@@ -1014,7 +1014,7 @@ extern "C" fn bios_limit_callback(cpu: i32, limit: *mut u32) -> core::ffi::c_int
extern "C" fn set_boost_callback(
ptr: *mut bindings::cpufreq_policy,
state: i32,
- ) -> core::ffi::c_int {
+ ) -> kernel::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
@@ -1036,7 +1036,6 @@ extern "C" fn register_em_callback(ptr: *mut bindings::cpufreq_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();
// SAFETY: The driver was earlier registered from `new()`.
@@ -1044,8 +1043,8 @@ fn drop(&mut self) {
// Free the previously leaked memory to the C code.
if !drv.attr.is_null() {
- // SAFETY: The pointer was earlier initialized from the result of `KBox::leak`.
- unsafe { drop(KBox::from_raw(drv.attr)) };
+ // SAFETY: The pointer was earlier initialized from the result of `KBox::into_raw()`.
+ drop(unsafe { KBox::from_raw(drv.attr) });
}
// Free data
diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
index 4030953c2001..b83bd97a4f37 100644
--- a/rust/kernel/opp.rs
+++ b/rust/kernel/opp.rs
@@ -300,9 +300,9 @@ 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,
+ _data: *mut kernel::ffi::c_void,
scaling_down: bool,
- ) -> core::ffi::c_int {
+ ) -> kernel::ffi::c_int {
from_result(|| {
// SAFETY: 'dev' is guaranteed by the C code to be valid.
let dev = unsafe { Device::get_device(dev) };
@@ -324,8 +324,8 @@ extern "C" fn config_regulators(
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 {
+ count: kernel::ffi::c_uint,
+ ) -> kernel::ffi::c_int {
from_result(|| {
// SAFETY: 'dev' is guaranteed by the C code to be valid.
let dev = unsafe { Device::get_device(dev) };