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

[Danilo Krummrich <dakr@kernel.org>]

On Wed, Apr 16, 2025 at 03:29:43PM +0530, Viresh Kumar wrote:
> 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 ?

Yes, I thought of a separate file, but I that should work as well.

> 
> 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