[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <2a510219-4681-4915-9c27-c7ecdc6df133@arm.com>
Date: Thu, 15 Aug 2024 10:30:04 +0200
From: Pierre Gondois <pierre.gondois@....com>
To: Viresh Kumar <viresh.kumar@...aro.org>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Miguel Ojeda <miguel.ojeda.sandonis@...il.com>,
Danilo Krummrich <dakr@...hat.com>, Miguel Ojeda <ojeda@...nel.org>,
Alex Gaynor <alex.gaynor@...il.com>,
Wedson Almeida Filho <wedsonaf@...il.com>, Boqun Feng
<boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <benno.lossin@...ton.me>,
Andreas Hindborg <a.hindborg@...sung.com>, Alice Ryhl <aliceryhl@...gle.com>
Cc: linux-pm@...r.kernel.org, Vincent Guittot <vincent.guittot@...aro.org>,
Stephen Boyd <sboyd@...nel.org>, Nishanth Menon <nm@...com>,
rust-for-linux@...r.kernel.org,
Manos Pitsidianakis <manos.pitsidianakis@...aro.org>,
Erik Schilling <erik.schilling@...aro.org>,
Alex Bennée <alex.bennee@...aro.org>,
Joakim Bech <joakim.bech@...aro.org>, Rob Herring <robh@...nel.org>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH V5 6/8] rust: Extend cpufreq bindings for driver
registration
Hello Viresh,
On 7/30/24 12:27, Viresh Kumar wrote:
> This extends the cpufreq bindings with bindings for registering a
> driver.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@...aro.org>
> ---
> rust/kernel/cpufreq.rs | 478 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 476 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
> index d58bb0bbaad4..2631dbb4865f 100644
> --- a/rust/kernel/cpufreq.rs
> +++ b/rust/kernel/cpufreq.rs
> @@ -9,14 +9,17 @@
> use crate::{
> bindings, clk, cpumask,
> device::Device,
> - error::{code::*, from_err_ptr, to_result, Result, VTABLE_DEFAULT_ERROR},
> + devres::Devres,
> + error::{code::*, from_err_ptr, from_result, to_result, Result, VTABLE_DEFAULT_ERROR},
> prelude::*,
> types::ForeignOwnable,
> };
>
> use core::{
> + cell::UnsafeCell,
> + marker::PhantomData,
> pin::Pin,
> - ptr::self,
> + ptr::{self, addr_of_mut},
> };
>
> use macros::vtable;
> @@ -563,3 +566,474 @@ fn register_em(_policy: &mut Policy) {
> kernel::build_error(VTABLE_DEFAULT_ERROR)
> }
> }
> +
> +/// Registration of a cpufreq driver.
> +pub struct Registration<T: Driver> {
> + drv: Box<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> {
> + /// Registers a cpufreq driver with the rest of the kernel.
> + pub fn new(name: &'static CStr, data: T::Data, flags: u16, boost: bool) -> Result<Self> {
> + let mut drv = Box::new(
> + UnsafeCell::new(bindings::cpufreq_driver::default()),
> + GFP_KERNEL,
> + )?;
> + let drv_ref = drv.get_mut();
> +
> + // Account for the trailing null character.
> + let len = name.len() + 1;
> + if len > drv_ref.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_ref.name[..len].copy_from_slice(name);
> +
> + drv_ref.boost_enabled = boost;
> + drv_ref.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_ref.attr = Box::leak(attr) as *mut _;
I think it would be better to give the possibility to the cpufreq driver to pass the
attr array, as not all drivers might want these attributes,
Regards,
Pierre
Powered by blists - more mailing lists