Re: [PATCH V10 14/15] rust: opp: Extend OPP abstractions with cpufreq support

[Viresh Kumar]

On 16-04-25, 10:52, Danilo Krummrich wrote:
> This config is needed quite often, it probably makes sense to move this code in
> its own Rust module, i.e.:
> 
> 	#[cfg(CONFIG_CPU_FREQ)]
> 	pub mod freq;

Like this ?

diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
index 734be8b6d0ef..f4cabe859c43 100644
--- a/rust/kernel/opp.rs
+++ b/rust/kernel/opp.rs
@@ -20,10 +20,67 @@
 };

 #[cfg(CONFIG_CPU_FREQ)]
-use crate::cpufreq;
+// Frequency table implementation.
+mod freq {
+    use crate::cpufreq;
+    use core::ops::Deref;
+    use super::*;
+
+    /// OPP frequency table.
+    ///
+    /// A [`cpufreq::Table`] created from [`Table`].
+    pub struct FreqTable {
+        dev: ARef<Device>,
+        ptr: *mut bindings::cpufreq_frequency_table,
+    }
+
+    impl FreqTable {
+        /// Creates a new instance of [`FreqTable`] from [`Table`].
+        pub(crate) 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(),
+                ptr,
+            })
+        }
+
+        // Returns a reference to the underlying [`cpufreq::Table`].
+        #[inline]
+        fn table(&self) -> &cpufreq::Table {
+            // SAFETY: The `ptr` is guaranteed by the C code to be valid.
+            unsafe { cpufreq::Table::from_raw(self.ptr) }
+        }
+    }
+
+    impl Deref for FreqTable {
+        type Target = cpufreq::Table;
+
+        #[inline]
+        fn deref(&self) -> &Self::Target {
+            self.table()
+        }
+    }
+
+    impl Drop for FreqTable {
+        fn drop(&mut self) {
+            // SAFETY: The pointer was created via `dev_pm_opp_init_cpufreq_table`, and is only
+            // freed here.
+            unsafe {
+                bindings::dev_pm_opp_free_cpufreq_table(self.dev.as_raw(), &mut self.as_raw())
+            };
+        }
+    }
+}

 #[cfg(CONFIG_CPU_FREQ)]
-use core::ops::Deref;
+pub use freq::FreqTable;

 use core::{marker::PhantomData, ptr};

@@ -502,60 +559,6 @@ extern "C" fn config_regulators(
     }
 }

-/// OPP frequency table.
-///
-/// A [`cpufreq::Table`] created from [`Table`].
-#[cfg(CONFIG_CPU_FREQ)]
-pub struct FreqTable {
-    dev: ARef<Device>,
-    ptr: *mut bindings::cpufreq_frequency_table,
-}
-
-#[cfg(CONFIG_CPU_FREQ)]
-impl FreqTable {
-    /// Creates a new instance of [`FreqTable`] from [`Table`].
-    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(),
-            ptr,
-        })
-    }
-
-    // Returns a reference to the underlying [`cpufreq::Table`].
-    #[inline]
-    fn table(&self) -> &cpufreq::Table {
-        // SAFETY: The `ptr` is guaranteed by the C code to be valid.
-        unsafe { cpufreq::Table::from_raw(self.ptr) }
-    }
-}
-
-#[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: The pointer was created via `dev_pm_opp_init_cpufreq_table`, and is only freed
-        // here.
-        unsafe { bindings::dev_pm_opp_free_cpufreq_table(self.dev.as_raw(), &mut self.as_raw()) };
-    }
-}
-
 /// A reference-counted OPP table.
 ///
 /// Rust abstraction for the C `struct opp_table`.

-- 
viresh