linux-kernel - Re: [PATCH V12 13/15] rust: cpufreq: Extend abstractions for driver registration

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aCsQylyW7R5rC15m@pollux>
Date: Mon, 19 May 2025 13:06:50 +0200
From: Danilo Krummrich <dakr@...nel.org>
To: Viresh Kumar <viresh.kumar@...aro.org>
Cc: "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>,
	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@...nel.org>,
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
	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>,
	Alex Bennée <alex.bennee@...aro.org>,
	Joakim Bech <joakim.bech@...aro.org>, Rob Herring <robh@...nel.org>,
	Yury Norov <yury.norov@...il.com>, Burak Emir <bqe@...gle.com>,
	Rasmus Villemoes <linux@...musvillemoes.dk>,
	Russell King <linux@...linux.org.uk>, linux-clk@...r.kernel.org,
	Michael Turquette <mturquette@...libre.com>,
	Andrew Ballance <andrewjballance@...il.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH V12 13/15] rust: cpufreq: Extend abstractions for driver
 registration

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