linux-kernel - Re: [PATCH V4 8/8] cpufreq: Add Rust based cpufreq-dt driver

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: <Zo-3QIPbhBsv8EjB@pollux>
Date: Thu, 11 Jul 2024 12:43:12 +0200
From: Danilo Krummrich <dakr@...hat.com>
To: Viresh Kumar <viresh.kumar@...aro.org>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>,
	Miguel Ojeda <miguel.ojeda.sandonis@...il.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>, 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 V4 8/8] cpufreq: Add Rust based cpufreq-dt driver

On Thu, Jul 11, 2024 at 12:27:50PM +0530, Viresh Kumar wrote:
> This commit adds a Rust based cpufreq-dt driver, which covers most of
> the functionality of the existing C based driver. Only a handful of
> things are left, like fetching platform data from cpufreq-dt-platdev.c.
> 
> This is tested with the help of QEMU for now and switching of
> frequencies work as expected.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@...aro.org>
> ---
>  drivers/cpufreq/Kconfig        |  12 ++
>  drivers/cpufreq/Makefile       |   1 +
>  drivers/cpufreq/rcpufreq_dt.rs | 222 +++++++++++++++++++++++++++++++++
>  3 files changed, 235 insertions(+)
>  create mode 100644 drivers/cpufreq/rcpufreq_dt.rs
> 
> diff --git a/drivers/cpufreq/rcpufreq_dt.rs b/drivers/cpufreq/rcpufreq_dt.rs
> new file mode 100644
> index 000000000000..3e86ad134e13
> --- /dev/null
> +++ b/drivers/cpufreq/rcpufreq_dt.rs
> +
> +impl platform::Driver for CPUFreqDTDriver {
> +    type Data = ();
> +
> +    define_of_id_table! {(), [
> +        (of::DeviceId(b_str!("operating-points-v2")), None),
> +    ]}
> +
> +    fn probe(dev: &mut platform::Device, _id_info: Option<&Self::IdInfo>) -> Result<Self::Data> {
> +        let drv = cpufreq::Registration::<CPUFreqDTDriver>::register(

Please just call this function `cpufreq::Registration::new`.

The existance of a `cpufreq::Registration` means that it's registered. Once it
is dropped, it's unregistered. It's the whole point of a `Registration` type
to bind the period of a driver being registered to the lifetime of a
`Registration` instance.

Having `Registration::register` implies a bit, that we could ever have an
unregistered `Registration`, which can never happen.

Besides that, it'd be nice to follow the same naming scheme everywhere.

> +            c_str!("cpufreq-dt"),
> +            (),
> +            cpufreq::flags::NEED_INITIAL_FREQ_CHECK | cpufreq::flags::IS_COOLING_DEV,
> +            true,
> +        )?;
> +
> +        Devres::new_foreign_owned(dev.as_ref(), drv, GFP_KERNEL)?;

This should be called by `cpufreq::Registration` directly, otherwise it's every
driver's responsibility to take care of the registration lifetime.

> +
> +        pr_info!("CPUFreq DT driver registered\n");
> +
> +        Ok(())
> +    }
> +}
> +
> +module_platform_driver! {
> +    type: CPUFreqDTDriver,
> +    name: "cpufreq_dt",
> +    author: "Viresh Kumar <viresh.kumar@...aro.org>",
> +    description: "Generic CPUFreq DT driver",
> +    license: "GPL v2",
> +}
> -- 
> 2.31.1.272.g89b43f80a514
>