Hello,

I am seeking a few Acks for this patch series before merging it into the PM tree
for the 6.15 merge window, unless there are any objections.

This series introduces initial Rust bindings for two subsystems: cpufreq and
Operating Performance Points (OPP). The bindings cover most of the interfaces
exposed by these subsystems. It also includes minimal bindings for the clk and
cpumask frameworks, which are required by the cpufreq bindings.

Additionally, a sample cpufreq driver, rcpufreq-dt, is included. This is a
duplicate of the existing cpufreq-dt driver, which is a platform-agnostic,
device-tree-based driver commonly used on ARM platforms.

The implementation has been tested using QEMU, ensuring that frequency
transitions, various configurations, and driver binding/unbinding work as
expected. However, performance measurements have not been conducted yet.

For those interested in testing these patches, they can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/linux.git rust/cpufreq-dt

This version is rebased on v6.14-rc1.

--
Viresh

V7->V8:
- Updated cpumask bindings to work with !CONFIG_CPUMASK_OFFSTACK case.
- Dropped few patches (property_present() and opp helpers), as they are already
  merged.
- from_cpu() is marked unsafe.
- Included a patch by Anisse Astier, to solve a long standing issue with this
  series.
- Dropped: "DO-NOT_MERGE: cpufreq: Rename cpufreq-dt platdev."
- Updated MAINTAINERS for new files.
- Other minor changes / cleanups.

V6->V7:
- from_cpu() is moved to cpu.rs and doesn't return ARef anymore, but just a
  reference.
- Dropped cpufreq_table_len() and related validation in cpufreq core.
- Solved the issue with BIT() macro differently, using an enum now.
- Few patches are broken into smaller / independent patches.
- Improved Commit logs and SAFETY comments at few places.
- Removed print message from cpufreq driver.
- Rebased over linux-next/master.
- Few other minor changes.

V5->V6:
- Rebase over latest rust/dev branch, which changed few interfaces that the
  patches were using.
- Included all other patches, which weren't included until now to focus only on
  core APIs.
- Other minor cleanups, additions.

V4->V5:
- Rename Registration::register() as new().
- Provide a new API: Registration::new_foreign_owned() and use it for
  rcpufreq_dt driver.
- Update MAINTAINERS file.

V3->V4:
- Fix bugs with freeing of OPP structure. Dropped the Drop routine and fixed
  reference counting.
- Registration object of the cpufreq core is modified a bit to remove the
  registered field, and few other cleanups.
- Use Devres for instead of platform data.
- Improve SAFETY comments.

V2->V3:
- Rebased on latest rust-device changes, which removed `Data` and so few changes
  were required to make it work.
- use srctree links (Alice Ryhl).
- Various changes the OPP creation APIs, new APIs: from_raw_opp() and
  from_raw_opp_owned() (Alice Ryhl).
- Inline as_raw() helpers (Alice Ryhl).
- Add new interface (`OPP::Token`) for dynamically created OPPs.
- Add Reviewed-by tag from Manos.
- Modified/simplified cpufreq registration structure / method a bit.

V1->V2:
- Create and use separate bindings for OF, clk, cpumask, etc (not included in
  this patchset but pushed to the above branch). This helped removing direct
  calls from the driver.
- Fix wrong usage of Pinning + Vec.
- Use Token for OPP Config.
- Use Opaque, transparent and Aref for few structures.
- Broken down into smaller patches to make it easy for reviewers.
- Based over staging/rust-device.

Thanks.

Anisse Astier (1):
  rust: macros: enable use of hyphens in module names

Viresh Kumar (13):
  cpufreq: Use enum for cpufreq flags that use BIT()
  rust: cpu: Add from_cpu()
  rust: Add cpumask helpers
  rust: Add bindings for cpumask
  rust: Add bare minimal bindings for clk framework
  rust: Add initial bindings for OPP framework
  rust: Extend OPP bindings for the OPP table
  rust: Extend OPP bindings for the configuration options
  rust: Add initial bindings for cpufreq framework
  rust: Extend cpufreq bindings for policy and driver ops
  rust: Extend cpufreq bindings for driver registration
  rust: Extend OPP bindings with CPU frequency table
  cpufreq: Add Rust based cpufreq-dt driver

 MAINTAINERS                     |    6 +
 drivers/cpufreq/Kconfig         |   12 +
 drivers/cpufreq/Makefile        |    1 +
 drivers/cpufreq/rcpufreq_dt.rs  |  238 +++++++
 include/linux/cpufreq.h         |   96 +--
 rust/bindings/bindings_helper.h |    5 +
 rust/helpers/cpufreq.c          |   10 +
 rust/helpers/cpumask.c          |   40 ++
 rust/helpers/helpers.c          |    2 +
 rust/kernel/clk.rs              |   48 ++
 rust/kernel/cpu.rs              |   31 +
 rust/kernel/cpufreq.rs          | 1054 +++++++++++++++++++++++++++++++
 rust/kernel/cpumask.rs          |  138 ++++
 rust/kernel/lib.rs              |    8 +
 rust/kernel/opp.rs              |  889 ++++++++++++++++++++++++++
 rust/macros/module.rs           |   17 +-
 16 files changed, 2543 insertions(+), 52 deletions(-)
 create mode 100644 drivers/cpufreq/rcpufreq_dt.rs
 create mode 100644 rust/helpers/cpufreq.c
 create mode 100644 rust/helpers/cpumask.c
 create mode 100644 rust/kernel/clk.rs
 create mode 100644 rust/kernel/cpu.rs
 create mode 100644 rust/kernel/cpufreq.rs
 create mode 100644 rust/kernel/cpumask.rs
 create mode 100644 rust/kernel/opp.rs

-- 
2.31.1.272.g89b43f80a514

Hi Viresh,

On Thu, Feb 06, 2025 at 02:58:21PM +0530, Viresh Kumar wrote:
> Hello,
> 
> I am seeking a few Acks for this patch series before merging it into the PM tree
> for the 6.15 merge window, unless there are any objections.
> 
> This series introduces initial Rust bindings for two subsystems: cpufreq and
> Operating Performance Points (OPP). The bindings cover most of the interfaces
> exposed by these subsystems. It also includes minimal bindings for the clk and
> cpumask frameworks, which are required by the cpufreq bindings.
> 
> Additionally, a sample cpufreq driver, rcpufreq-dt, is included. This is a
> duplicate of the existing cpufreq-dt driver, which is a platform-agnostic,
> device-tree-based driver commonly used on ARM platforms.
> 
> The implementation has been tested using QEMU, ensuring that frequency
> transitions, various configurations, and driver binding/unbinding work as
> expected. However, performance measurements have not been conducted yet.
> 
> For those interested in testing these patches, they can be found at:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/linux.git rust/cpufreq-dt
> 
> This version is rebased on v6.14-rc1.

I gave it a quick shot and it seems there are a few Clippy warnings, plus
rustfmtcheck complains.

There are also two minor checkpatch complaints about line length.

Hi Danilo,

On 06-02-25, 12:45, Danilo Krummrich wrote:
> I gave it a quick shot and it seems there are a few Clippy warnings,

I could find only one (related to core::format_args), are there more ?

(Earlier I had a debug commit on top of the series in my branch and
Clippy didn't give any warnings as format_flags was getting used from
there.)

> plus rustfmtcheck complains.

I am not sure how to solve them.

Diff in rust/kernel/cpufreq.rs at line 628:

         // 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 _
-        };
+        attr[next] =
+            unsafe { addr_of_mut!(bindings::cpufreq_freq_attr_scaling_available_freqs) as *mut _ };
         next += 1;

         if boost {

If I move the code as suggested here, then I get warning about not
adding a SAFETY comment for unsafe code (which looks to be a tool
specific bug).

I can move the entire thing into the unsafe block, but the assignment
to attr[next] isn't unsafe.

What do yo suggest here ?

> There are also two minor checkpatch complaints about line length.

Yeah, they came from the first commit (which wasn't written by me and
so I avoided touching it), fixed now.

-- 
viresh

On Fri, Feb 7, 2025 at 8:15 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> If I move the code as suggested here, then I get warning about not
> adding a SAFETY comment for unsafe code (which looks to be a tool
> specific bug).

The warning is there even if you don't run `rustfmt`, and it does not
look like a bug to me -- what Clippy is complaining about is that you
don't actually need the `unsafe` block to begin with:

    error: unnecessary `unsafe` block
    --> rust/kernel/cpufreq.rs:631:22
        |
    631 |         attr[next] = unsafe {
        |                      ^^^^^^ unnecessary `unsafe` block
        |
        = note: `-D unused-unsafe` implied by `-D warnings`
        = help: to override `-D warnings` add `#[allow(unused_unsafe)]`

since those operations are safe. Or am I missing something?

Then, when you remove it, Clippy will complain that there should not
be a SAFETY comment:

    error: statement has unnecessary safety comment
    --> rust/kernel/cpufreq.rs:625:9
        |
    625 |         attr[next] =
addr_of_mut!(bindings::cpufreq_freq_attr_scaling_available_freqs) as
*mut _;
        |
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        |
    help: consider removing the safety comment
    --> rust/kernel/cpufreq.rs:623:9
        |
    623 |         // SAFETY: The C code returns a valid pointer here,
which is again passed to the C code in
        |
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        = help: for further information visit
https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_safety_comment
        = note: `-D clippy::unnecessary-safety-comment` implied by `-D warnings`
        = help: to override `-D warnings` add
`#[allow(clippy::unnecessary_safety_comment)]`

And `rustfmt` will put things in a single line, since now they fit.

I would suggest reviewing all the SAFETY comments around this code,
i.e. something may be wrong, since these were not needed, and thus you
may have wanted to describe them elsewhere.

In any case, passing `rustfmtcheck` is a requirement. So in the worst
case, if you do find such a bug in e.g. Clippy, you may always
`expect` or `allow` the lint or disable `rustfmt` in that region of
code. But that should be really rare, and in such a case it should be
reported upstream.

I also found other build issues in the branch you mention in your
cover letter, so please double-check everything looks good before
adding it to linux-next. Please also make it Clippy-clean.

I hope that helps!

Cheers,
Miguel

Hi Miguel,

On 07-02-25, 12:07, Miguel Ojeda wrote:
> The warning is there even if you don't run `rustfmt`, and it does not
> look like a bug to me -- what Clippy is complaining about is that you
> don't actually need the `unsafe` block to begin with:
> 
>     error: unnecessary `unsafe` block
>     --> rust/kernel/cpufreq.rs:631:22
>         |
>     631 |         attr[next] = unsafe {
>         |                      ^^^^^^ unnecessary `unsafe` block
>         |
>         = note: `-D unused-unsafe` implied by `-D warnings`
>         = help: to override `-D warnings` add `#[allow(unused_unsafe)]`
> 
> since those operations are safe. Or am I missing something?

One thing you are missing is the right branch to test. I mentioned the
wrong tree in cover-letter (though I don't remember getting above
errors earlier too, not sure why you are getting them) :(

git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git rust/cpufreq-dt

This patchset was generated correctly though.

I don't get anything with CLIPPY with this branch, with rustfmtcheck I
get:

         // 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 _
-        };
+        attr[next] =
+            unsafe { addr_of_mut!(bindings::cpufreq_freq_attr_scaling_available_freqs) as *mut _ };
         next += 1;


If I make the above changes I get following with CLIPPY:

$ make CLIPPY=1 ARCH=arm64 O=../barm64t/ -j8 CROSS_COMPILE=aarch64-linux-gnu- CONFIG_DEBUG_SECTION_MISMATCH=y

warning: unsafe block missing a safety comment
   --> /mnt/ssd/all/work/repos/kernel/linux/rust/kernel/cpufreq.rs:632:13
    |
632 |             unsafe { addr_of_mut!(bindings::cpufreq_freq_attr_scaling_available_freqs) as *mut _ };
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider adding a safety comment on the preceding line
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#undocumented_unsafe_blocks
    = note: requested on the command line with `-W clippy::undocumented-unsafe-blocks`

warning: unsafe block missing a safety comment
   --> /mnt/ssd/all/work/repos/kernel/linux/rust/kernel/cpufreq.rs:639:17
    |
639 |                 unsafe { addr_of_mut!(bindings::cpufreq_freq_attr_scaling_boost_freqs) as *mut _ };
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider adding a safety comment on the preceding line
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#undocumented_unsafe_blocks

warning: 2 warnings emitted


This I thought was a bug (I may have seen this with other Rust
projects too, from what I can remember).

If I drop the unsafe here I get:

error[E0133]: use of mutable static is unsafe and requires unsafe block
   --> /mnt/ssd/all/work/repos/kernel/linux/rust/kernel/cpufreq.rs:632:26
    |
632 |             addr_of_mut!(bindings::cpufreq_freq_attr_scaling_available_freqs) as *mut _;
    |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ use of mutable static
    |
    = note: mutable statics can be mutated by multiple threads: aliasing violations or data races will cause undefined behavior

I don't remember seeing a CLIPPY warning ever about removing the
unsafe here, as reported in your case.

> In any case, passing `rustfmtcheck` is a requirement. So in the worst
> case, if you do find such a bug in e.g. Clippy, you may always
> `expect` or `allow` the lint or disable `rustfmt` in that region of
> code. But that should be really rare, and in such a case it should be
> reported upstream.

It would require clippy::undocumented-unsafe-blocks to be disabled, in
this case.

> I also found other build issues in the branch you mention in your
> cover letter, so please double-check everything looks good before
> adding it to linux-next. Please also make it Clippy-clean.

Sorry about that, maybe there were other issues with the earlier
branch. Can you please try again from the tree I mentioned above ?

-- 
viresh

On Mon, Feb 10, 2025 at 9:06 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> error[E0133]: use of mutable static is unsafe and requires unsafe block
>    --> /mnt/ssd/all/work/repos/kernel/linux/rust/kernel/cpufreq.rs:632:26
>     |
> 632 |             addr_of_mut!(bindings::cpufreq_freq_attr_scaling_available_freqs) as *mut _;
>     |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ use of mutable static

Ah, I see now -- yeah, this is due to:

    https://blog.rust-lang.org/2024/10/17/Rust-1.82.0.html#safely-addressing-unsafe-statics

You could do (probably with a comment):

        pub fn new(name: &'static CStr, data: T::Data, flags: u16,
boost: bool) -> Result<Self> {
    +        #![allow(unused_unsafe)]
    +
            let mut drv = KBox::new(

Yeah, a bit annoying... :(

> I don't remember seeing a CLIPPY warning ever about removing the
> unsafe here, as reported in your case.

Please use a newer version to see them, e.g. the latest stable 1.84.1.

In general, please test patches with the minimum version and the
latest stable. The latest will give you more lints in general, and the
minimum will make sure it builds for older versions.

> It would require clippy::undocumented-unsafe-blocks to be disabled, in
> this case.

Hmm... why? We need the `allow` above because we need to keep the
`unsafe` for older Rust. But you can provide a comment nevertheless,
even if "dummy", so you should not need to disable anything else.

With your branch + the `allow` above + running `rustfmt`, it is Clippy
clean for me. Please see the diff below as an example (I also cleaned
the other Clippy warning -- and sorry for the wrapping).

Cheers,
Miguel

diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
index d2e7913e170b..e7c62770fc3b 100644
--- a/rust/kernel/cpufreq.rs
+++ b/rust/kernel/cpufreq.rs
@@ -602,6 +602,8 @@ 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> {
+        #![allow(unused_unsafe)]
+
         let mut drv = KBox::new(
             UnsafeCell::new(bindings::cpufreq_driver::default()),
             GFP_KERNEL,
@@ -626,19 +628,15 @@ pub fn new(name: &'static CStr, data: T::Data,
flags: u16, boost: bool) -> Resul
         let mut attr = KBox::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 _
-        };
+        attr[next] =
+            // SAFETY: ...
+            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
_
-            };
+            attr[next] =
+                // SAFETY: ...
+                unsafe {
addr_of_mut!(bindings::cpufreq_freq_attr_scaling_boost_freqs) as *mut
_ };
             next += 1;
         }
         attr[next] = ptr::null_mut();
diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
index b83bd97a4f37..aaf650f46ccb 100644
--- a/rust/kernel/opp.rs
+++ b/rust/kernel/opp.rs
@@ -781,7 +781,6 @@ fn drop(&mut self) {
 /// represents a pointer that owns a reference count on the OPP.
 ///
 /// A reference to the `OPP`, `&OPP` isn't refcounted by the Rust code.
-
 #[repr(transparent)]
 pub struct OPP(Opaque<bindings::dev_pm_opp>);

On 17-02-25, 09:39, Miguel Ojeda wrote:
> Ah, I see now -- yeah, this is due to:
> 
>     https://blog.rust-lang.org/2024/10/17/Rust-1.82.0.html#safely-addressing-unsafe-statics
> 
> You could do (probably with a comment):
> 
>         pub fn new(name: &'static CStr, data: T::Data, flags: u16,
> boost: bool) -> Result<Self> {
>     +        #![allow(unused_unsafe)]
>     +
>             let mut drv = KBox::new(
> 
> Yeah, a bit annoying... :(

+        // Required due to Rust 1.82's stricter handling of `unsafe` in mutable statics. The
+        // `unsafe` blocks aren't required anymore with later versions.
+        #![allow(unused_unsafe)]

> diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
> index d2e7913e170b..e7c62770fc3b 100644
> --- a/rust/kernel/cpufreq.rs
> +++ b/rust/kernel/cpufreq.rs

> +        attr[next] =
> +            // SAFETY: ...

Ah, I wasn't sure if adding a SAFETY comment after `=` is fine.

Thanks.

-- 
viresh