Re: [PATCH V5 8/8] cpufreq: Add Rust based cpufreq-dt driver

[Rob Herring]

On Tue, Jul 30, 2024 at 4:27 AM Viresh Kumar <viresh.kumar@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
>  drivers/cpufreq/Kconfig        |  12 ++
>  drivers/cpufreq/Makefile       |   1 +
>  drivers/cpufreq/rcpufreq_dt.rs | 221 +++++++++++++++++++++++++++++++++
>  3 files changed, 234 insertions(+)
>  create mode 100644 drivers/cpufreq/rcpufreq_dt.rs
>
> diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
> index 94e55c40970a..eb9359bd3c5c 100644
> --- a/drivers/cpufreq/Kconfig
> +++ b/drivers/cpufreq/Kconfig
> @@ -217,6 +217,18 @@ config CPUFREQ_DT
>
>           If in doubt, say N.
>
> +config CPUFREQ_DT_RUST
> +       tristate "Rust based Generic DT based cpufreq driver"
> +       depends on HAVE_CLK && OF && RUST
> +       select CPUFREQ_DT_PLATDEV
> +       select PM_OPP
> +       help
> +         This adds a Rust based generic DT based cpufreq driver for frequency
> +         management.  It supports both uniprocessor (UP) and symmetric
> +         multiprocessor (SMP) systems.
> +
> +         If in doubt, say N.
> +
>  config CPUFREQ_DT_PLATDEV
>         tristate "Generic DT based cpufreq platdev driver"
>         depends on OF
> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> index 8d141c71b016..4981d908b803 100644
> --- a/drivers/cpufreq/Makefile
> +++ b/drivers/cpufreq/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_CPU_FREQ_GOV_COMMON)             += cpufreq_governor.o
>  obj-$(CONFIG_CPU_FREQ_GOV_ATTR_SET)    += cpufreq_governor_attr_set.o
>
>  obj-$(CONFIG_CPUFREQ_DT)               += cpufreq-dt.o
> +obj-$(CONFIG_CPUFREQ_DT_RUST)          += rcpufreq_dt.o
>  obj-$(CONFIG_CPUFREQ_DT_PLATDEV)       += cpufreq-dt-platdev.o
>
>  # Traces
> diff --git a/drivers/cpufreq/rcpufreq_dt.rs b/drivers/cpufreq/rcpufreq_dt.rs
> new file mode 100644
> index 000000000000..9204d92d3eec
> --- /dev/null
> +++ b/drivers/cpufreq/rcpufreq_dt.rs
> @@ -0,0 +1,221 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Rust based implementation of the cpufreq-dt driver.
> +
> +use core::format_args;
> +
> +use kernel::{
> +    b_str, c_str, clk, cpufreq, cpumask::Cpumask, define_of_id_table, device::Device,
> +    error::code::*, fmt, macros::vtable, module_platform_driver, of, opp, platform, prelude::*,
> +    str::CString, sync::Arc,
> +};
> +
> +// Finds exact supply name from the OF node.
> +fn find_supply_name_exact(np: &of::DeviceNode, name: &str) -> Option<CString> {
> +    let name_cstr = CString::try_from_fmt(fmt!("{}-supply", name)).ok()?;
> +
> +    np.find_property(&name_cstr).ok()?;

We don't want anything in Rust based on of_find_property() which this
is. That function assumes a device node and its properties are never
freed which is no longer a valid assumption (since OF_DYNAMIC and then
overlays). There's some work starting to address that, and my plan is
using of_find_property() on dynamic nodes will start warning. The
of_property_* API mostly avoids that issue (string types are an issue)
.

Also, it's probably the device property API we want to build Rust
bindings on top of rather than DT and ACPI. OTOH, the device property
API may be missing some features needed with OPP bindings.

Rob