[PATCH V6 00/15] Rust bindings for cpufreq and OPP core + sample driver

[PATCH V6 00/15] Rust bindings for cpufreq and OPP core + sample driver

[Viresh Kumar]

Miguel / Danilo,

I would like to get some of this (if not all) merged now into the rust/dev
branch, since this stuff has been around for some time and many other
dependencies are already merged new (like OF, platform, device/driver). Please
suggest how can we move forward to get that done.

-------------------------8<-------------------------

This patch 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.

Included in this series is a sample `cpufreq` driver, `rcpufreq-dt`, which is a
duplicate of the existing `cpufreq-dt` driver. The `cpufreq-dt` driver is a
generic, platform-agnostic, device-tree-based driver used on many ARM platforms.

Currently, the implementation has been tested using QEMU, verifying that
frequency transitions, various configurations, and driver binding/unbinding
functions as expected. However, performance measurements have not been
conducted.

For those interested in trying these patches, along with a few dependencies,
they can be found at:

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

The series includes all the patches to make it work:
- few new APIs to cpufreq/OPP frameworks.
- Avoiding using BIT() macro in cpufreq core (we need to make Rust work with
  it).
- Renaming of the cpufreq-dt platform device (this too needs to be fixed in
  Rust).
- Basic Rust bindings for clk and cpumask layers.

The work is based on `staging/dev` from the Rust tree, which is derived
from v6.13-rc3.

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.

Viresh Kumar (15):
  PM / OPP: Expose refcounting helpers for the Rust implementation
  cpufreq: Add cpufreq_table_len()
  cpufreq: Rust implementation doesn't parse BIT() macro
  rust: device: Add few 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
  DO-NOT_MERGE: cpufreq: Rename cpufreq-dt platdev

 MAINTAINERS                          |    2 +
 drivers/cpufreq/Kconfig              |   12 +
 drivers/cpufreq/Makefile             |    1 +
 drivers/cpufreq/cpufreq-dt-platdev.c |    2 +-
 drivers/cpufreq/rcpufreq_dt.rs       |  232 ++++++
 drivers/opp/core.c                   |   17 +-
 drivers/opp/opp.h                    |    1 -
 include/linux/cpufreq.h              |   27 +-
 include/linux/pm_opp.h               |    6 +
 rust/bindings/bindings_helper.h      |    5 +
 rust/helpers/cpufreq.c               |   16 +
 rust/helpers/cpumask.c               |   35 +
 rust/helpers/helpers.c               |    2 +
 rust/kernel/clk.rs                   |   48 ++
 rust/kernel/cpufreq.rs               | 1079 ++++++++++++++++++++++++++
 rust/kernel/cpumask.rs               |   85 ++
 rust/kernel/device.rs                |   21 +
 rust/kernel/lib.rs                   |    6 +
 rust/kernel/opp.rs                   |  886 +++++++++++++++++++++
 19 files changed, 2468 insertions(+), 15 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/cpufreq.rs
 create mode 100644 rust/kernel/cpumask.rs
 create mode 100644 rust/kernel/opp.rs

-- 
2.31.1.272.g89b43f80a514

[PATCH V6 01/15] PM / OPP: Expose refcounting helpers for the Rust implementation

[Viresh Kumar]

The Rust implementation needs these APIs for its working. Expose them.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/opp/core.c     | 17 ++++++++++++-----
 drivers/opp/opp.h      |  1 -
 include/linux/pm_opp.h |  6 ++++++
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 0311b18319a4..f950bf1f78ca 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -1480,11 +1480,6 @@ static struct opp_table *_allocate_opp_table(struct device *dev, int index)
 	return ERR_PTR(ret);
 }
 
-void _get_opp_table_kref(struct opp_table *opp_table)
-{
-	kref_get(&opp_table->kref);
-}
-
 static struct opp_table *_update_opp_table_clk(struct device *dev,
 					       struct opp_table *opp_table,
 					       bool getclk)
@@ -1645,6 +1640,17 @@ static void _opp_table_kref_release(struct kref *kref)
 	kfree(opp_table);
 }
 
+void _get_opp_table_kref(struct opp_table *opp_table)
+{
+	kref_get(&opp_table->kref);
+}
+
+void dev_pm_opp_get_opp_table_ref(struct opp_table *opp_table)
+{
+	_get_opp_table_kref(opp_table);
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_get_opp_table_ref);
+
 void dev_pm_opp_put_opp_table(struct opp_table *opp_table)
 {
 	kref_put_mutex(&opp_table->kref, _opp_table_kref_release,
@@ -1679,6 +1685,7 @@ void dev_pm_opp_get(struct dev_pm_opp *opp)
 {
 	kref_get(&opp->kref);
 }
+EXPORT_SYMBOL_GPL(dev_pm_opp_get);
 
 void dev_pm_opp_put(struct dev_pm_opp *opp)
 {
diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h
index 430651e7424a..5c7c81190e41 100644
--- a/drivers/opp/opp.h
+++ b/drivers/opp/opp.h
@@ -250,7 +250,6 @@ struct opp_table {
 };
 
 /* Routines internal to opp core */
-void dev_pm_opp_get(struct dev_pm_opp *opp);
 bool _opp_remove_all_static(struct opp_table *opp_table);
 void _get_opp_table_kref(struct opp_table *opp_table);
 int _get_opp_count(struct opp_table *opp_table);
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 568183e3e641..fd817815a0f9 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -100,6 +100,7 @@ struct dev_pm_opp_data {
 #if defined(CONFIG_PM_OPP)
 
 struct opp_table *dev_pm_opp_get_opp_table(struct device *dev);
+void dev_pm_opp_get_opp_table_ref(struct opp_table *opp_table);
 void dev_pm_opp_put_opp_table(struct opp_table *opp_table);
 
 unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp);
@@ -158,6 +159,7 @@ struct dev_pm_opp *dev_pm_opp_find_bw_ceil(struct device *dev,
 struct dev_pm_opp *dev_pm_opp_find_bw_floor(struct device *dev,
 					   unsigned int *bw, int index);
 
+void dev_pm_opp_get(struct dev_pm_opp *opp);
 void dev_pm_opp_put(struct dev_pm_opp *opp);
 
 int dev_pm_opp_add_dynamic(struct device *dev, struct dev_pm_opp_data *opp);
@@ -203,6 +205,8 @@ static inline struct opp_table *dev_pm_opp_get_opp_table_indexed(struct device *
 	return ERR_PTR(-EOPNOTSUPP);
 }
 
+static inline void dev_pm_opp_get_opp_table_ref(struct opp_table *opp_table) {}
+
 static inline void dev_pm_opp_put_opp_table(struct opp_table *opp_table) {}
 
 static inline unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp)
@@ -334,6 +338,8 @@ static inline struct dev_pm_opp *dev_pm_opp_find_bw_floor(struct device *dev,
 	return ERR_PTR(-EOPNOTSUPP);
 }
 
+static inline void dev_pm_opp_get(struct dev_pm_opp *opp) {}
+
 static inline void dev_pm_opp_put(struct dev_pm_opp *opp) {}
 
 static inline int
-- 
2.31.1.272.g89b43f80a514

Re: [PATCH V6 01/15] PM / OPP: Expose refcounting helpers for the Rust implementation

[Greg KH]

On Tue, Jan 07, 2025 at 04:51:34PM +0530, Viresh Kumar wrote:
> The Rust implementation needs these APIs for its working. Expose them.

Why is the rust code unique here?  Why does C code not need these
exported?

And that first sentance isn't really good grammer :)

Also, you created a new function here and didn't document it anywhere,
nor do you mention it here in the changelog text, making this a
non-starter right off :(

thanks,

greg k-h

Re: [PATCH V6 01/15] PM / OPP: Expose refcounting helpers for the Rust implementation

[Viresh Kumar]

On 07-01-25, 12:58, Greg KH wrote:
> On Tue, Jan 07, 2025 at 04:51:34PM +0530, Viresh Kumar wrote:
> > The Rust implementation needs these APIs for its working. Expose them.
> 
> Why is the rust code unique here?  Why does C code not need these
> exported?
> 
> And that first sentance isn't really good grammer :)
> 
> Also, you created a new function here and didn't document it anywhere,
> nor do you mention it here in the changelog text, making this a
> non-starter right off :(

How about this ?

    PM / OPP: Add reference counting helpers for Rust implementation

    To ensure that resources such as OPP tables or OPP nodes are not freed
    while in use by the Rust implementation, it is necessary to increment
    their reference count from Rust code.

    This commit introduces a new helper function,
    `dev_pm_opp_get_opp_table_ref()`, to increment the reference count of an
    OPP table and declares the existing helper `dev_pm_opp_get()` in
    `pm_opp.h`.

-- 
viresh

Re: [PATCH V6 01/15] PM / OPP: Expose refcounting helpers for the Rust implementation

[Greg KH]

On Wed, Jan 08, 2025 at 02:41:24PM +0530, Viresh Kumar wrote:
> On 07-01-25, 12:58, Greg KH wrote:
> > On Tue, Jan 07, 2025 at 04:51:34PM +0530, Viresh Kumar wrote:
> > > The Rust implementation needs these APIs for its working. Expose them.
> > 
> > Why is the rust code unique here?  Why does C code not need these
> > exported?
> > 
> > And that first sentance isn't really good grammer :)
> > 
> > Also, you created a new function here and didn't document it anywhere,
> > nor do you mention it here in the changelog text, making this a
> > non-starter right off :(
> 
> How about this ?
> 
>     PM / OPP: Add reference counting helpers for Rust implementation
> 
>     To ensure that resources such as OPP tables or OPP nodes are not freed
>     while in use by the Rust implementation, it is necessary to increment
>     their reference count from Rust code.
> 
>     This commit introduces a new helper function,
>     `dev_pm_opp_get_opp_table_ref()`, to increment the reference count of an
>     OPP table and declares the existing helper `dev_pm_opp_get()` in
>     `pm_opp.h`.

That works, if you drop the `` stuff, not needed :)

thanks,

greg k-h

[PATCH V6 02/15] cpufreq: Add cpufreq_table_len()

[Viresh Kumar]

Add a function to calculate number of entries in the cpufreq table. This
will be used by the Rust implementation.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 include/linux/cpufreq.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 7fe0981a7e46..6b882ff4dc24 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -783,6 +783,17 @@ bool cpufreq_boost_enabled(void);
 int cpufreq_enable_boost_support(void);
 bool policy_has_boost_freq(struct cpufreq_policy *policy);
 
+static inline unsigned int cpufreq_table_len(struct cpufreq_frequency_table *freq_table)
+{
+	struct cpufreq_frequency_table *pos;
+	unsigned int count = 0;
+
+	cpufreq_for_each_entry(pos, freq_table)
+		count++;
+
+	return count;
+}
+
 /* Find lowest freq at or above target in a table in ascending order */
 static inline int cpufreq_table_find_index_al(struct cpufreq_policy *policy,
 					      unsigned int target_freq,
-- 
2.31.1.272.g89b43f80a514

Re: [PATCH V6 02/15] cpufreq: Add cpufreq_table_len()

[Greg KH]

On Tue, Jan 07, 2025 at 04:51:35PM +0530, Viresh Kumar wrote:
> Add a function to calculate number of entries in the cpufreq table. This
> will be used by the Rust implementation.

Again, why is Rust unique here?  Why wouldn't the C code also need this?

> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  include/linux/cpufreq.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 7fe0981a7e46..6b882ff4dc24 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -783,6 +783,17 @@ bool cpufreq_boost_enabled(void);
>  int cpufreq_enable_boost_support(void);
>  bool policy_has_boost_freq(struct cpufreq_policy *policy);
>  
> +static inline unsigned int cpufreq_table_len(struct cpufreq_frequency_table *freq_table)
> +{
> +	struct cpufreq_frequency_table *pos;
> +	unsigned int count = 0;
> +
> +	cpufreq_for_each_entry(pos, freq_table)
> +		count++;

No locking is needed?  Why does anyone care about the length of this
table?

thanks,

greg k-h

Re: [PATCH V6 02/15] cpufreq: Add cpufreq_table_len()

[Viresh Kumar]

On 07-01-25, 12:59, Greg KH wrote:
> On Tue, Jan 07, 2025 at 04:51:35PM +0530, Viresh Kumar wrote:
> > Add a function to calculate number of entries in the cpufreq table. This
> > will be used by the Rust implementation.
> 
> Again, why is Rust unique here?  Why wouldn't the C code also need this?

How about this:

    cpufreq: Add cpufreq_table_len()

    The last entry of a cpufreq table is marked by setting the frequency
    field to a special value: CPUFREQ_TABLE_END. The C code manages to
    traverse the table by checking the frequency field, until it reaches
    CPUFREQ_TABLE_END.

    The Rust cpufreq bindings though will need to know the length of the
    cpufreq table in advance, for example to check against an invalid index
    value.

    Provide a helper to calculate number of entries in the cpufreq table.
    will be used by the Rust implementation.

> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> >  include/linux/cpufreq.h | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> > index 7fe0981a7e46..6b882ff4dc24 100644
> > --- a/include/linux/cpufreq.h
> > +++ b/include/linux/cpufreq.h
> > @@ -783,6 +783,17 @@ bool cpufreq_boost_enabled(void);
> >  int cpufreq_enable_boost_support(void);
> >  bool policy_has_boost_freq(struct cpufreq_policy *policy);
> >  
> > +static inline unsigned int cpufreq_table_len(struct cpufreq_frequency_table *freq_table)
> > +{
> > +	struct cpufreq_frequency_table *pos;
> > +	unsigned int count = 0;
> > +
> > +	cpufreq_for_each_entry(pos, freq_table)
> > +		count++;
> 
> No locking is needed?

No, the cpufreq table is implemented as an array and is never altered once
created.

-- 
viresh

Re: [PATCH V6 02/15] cpufreq: Add cpufreq_table_len()

[Greg KH]

On Wed, Jan 08, 2025 at 04:42:53PM +0530, Viresh Kumar wrote:
> On 07-01-25, 12:59, Greg KH wrote:
> > On Tue, Jan 07, 2025 at 04:51:35PM +0530, Viresh Kumar wrote:
> > > Add a function to calculate number of entries in the cpufreq table. This
> > > will be used by the Rust implementation.
> > 
> > Again, why is Rust unique here?  Why wouldn't the C code also need this?
> 
> How about this:
> 
>     cpufreq: Add cpufreq_table_len()
> 
>     The last entry of a cpufreq table is marked by setting the frequency
>     field to a special value: CPUFREQ_TABLE_END. The C code manages to
>     traverse the table by checking the frequency field, until it reaches
>     CPUFREQ_TABLE_END.
> 
>     The Rust cpufreq bindings though will need to know the length of the
>     cpufreq table in advance, for example to check against an invalid index
>     value.
> 
>     Provide a helper to calculate number of entries in the cpufreq table.
>     will be used by the Rust implementation.

Odd, why can't Rust also know where CPUFREQ_TABLE_END is?  Why do you
have to do extra work here?  That feels wrong.

thanks,

greg k-h

Re: [PATCH V6 02/15] cpufreq: Add cpufreq_table_len()

[Viresh Kumar]

On 08-01-25, 12:50, Greg KH wrote:
> Odd, why can't Rust also know where CPUFREQ_TABLE_END is?  Why do you
> have to do extra work here?  That feels wrong.

Well, it can, sure.

The freq table the Rust code receives is part of the C code:
bindings::cpufreq_frequency_table. Since it is a C managed pointer, I thought it
is better to do the parsing in C code itself. In case the implementation of the
struct changes in future (unlikely though), we would only need to fix the C code
and not Rust, which looks to be the right expectation.

-- 
viresh

Re: [PATCH V6 02/15] cpufreq: Add cpufreq_table_len()

[Greg KH]

On Thu, Jan 09, 2025 at 10:11:17AM +0530, Viresh Kumar wrote:
> On 08-01-25, 12:50, Greg KH wrote:
> > Odd, why can't Rust also know where CPUFREQ_TABLE_END is?  Why do you
> > have to do extra work here?  That feels wrong.
> 
> Well, it can, sure.
> 
> The freq table the Rust code receives is part of the C code:
> bindings::cpufreq_frequency_table. Since it is a C managed pointer, I thought it
> is better to do the parsing in C code itself. In case the implementation of the
> struct changes in future (unlikely though), we would only need to fix the C code
> and not Rust, which looks to be the right expectation.

Then why not make the C code use this function as well, to keep all
cpufreq drivers from having to manually walk the list and that way both
C and Rust drivers all do the same thing?  That makes more sense to me,
there's no reason you can't change C code today first to make things
more unified, in fact, that's usually a better idea overall anyway.

thanks,

greg k-h

Re: [PATCH V6 02/15] cpufreq: Add cpufreq_table_len()

[Viresh Kumar]

On 09-01-25, 08:35, Greg KH wrote:
> Then why not make the C code use this function as well, to keep all
> cpufreq drivers from having to manually walk the list and that way both
> C and Rust drivers all do the same thing?  That makes more sense to me,
> there's no reason you can't change C code today first to make things
> more unified, in fact, that's usually a better idea overall anyway.

I investigated a bit on this..

- The cpufreq core normally gets (from cpufreq governor's for example)
  a frequency value to be matched against in the freq-table, and the
  loop which run over the freq-table is already optimized enough (it
  checks for CPUFREQ_TABLE_END) for this. Using length in this loop
  won't improve it anymore.

- The cpufreq core then calls cpufreq driver's callbacks and passes an
  index to the freq-table, which the drivers don't need to verify
  against table length, since the index came from the core itself.

- The same happens on the Rust side, where the cpufreq core calls the
  target_index() callback of the driver. While writing the Rust code,
  I thought maybe I should validate that the index is within limits
  (before I do pointer manipulation in Rust code). And so required
  this extra function (which C code never uses).

- Now I can either keep doing this verification in the Rust code (and
  so keep the new API, only used by Rust code). Or I can just remove
  the verification and trust that the index passed by the
  cpufreq-drivers is correct (since they have received them from the
  cpufreq C code).

What should I do ?

-- 
viresh

Re: [PATCH V6 02/15] cpufreq: Add cpufreq_table_len()

[Greg KH]

On Mon, Jan 13, 2025 at 01:00:40PM +0530, Viresh Kumar wrote:
> On 09-01-25, 08:35, Greg KH wrote:
> > Then why not make the C code use this function as well, to keep all
> > cpufreq drivers from having to manually walk the list and that way both
> > C and Rust drivers all do the same thing?  That makes more sense to me,
> > there's no reason you can't change C code today first to make things
> > more unified, in fact, that's usually a better idea overall anyway.
> 
> I investigated a bit on this..
> 
> - The cpufreq core normally gets (from cpufreq governor's for example)
>   a frequency value to be matched against in the freq-table, and the
>   loop which run over the freq-table is already optimized enough (it
>   checks for CPUFREQ_TABLE_END) for this. Using length in this loop
>   won't improve it anymore.
> 
> - The cpufreq core then calls cpufreq driver's callbacks and passes an
>   index to the freq-table, which the drivers don't need to verify
>   against table length, since the index came from the core itself.
> 
> - The same happens on the Rust side, where the cpufreq core calls the
>   target_index() callback of the driver. While writing the Rust code,
>   I thought maybe I should validate that the index is within limits
>   (before I do pointer manipulation in Rust code). And so required
>   this extra function (which C code never uses).
> 
> - Now I can either keep doing this verification in the Rust code (and
>   so keep the new API, only used by Rust code). Or I can just remove
>   the verification and trust that the index passed by the
>   cpufreq-drivers is correct (since they have received them from the
>   cpufreq C code).
> 
> What should I do ?

I would trust the C code, keeping the apis the same between the two
types of drivers.  If in the future you want to fix up the api, then fix
both drivers.  Don't try to have unique apis for only Rust drivers if
you can help it at all, that is just going to drive us all crazy over
time.

And really, you might want to fix up the api, that too sounds crazy :)

thanks,

greg k-h

[PATCH V6 03/15] cpufreq: Rust implementation doesn't parse BIT() macro

[Viresh Kumar]

Rust isn't able to parse the macro for now, avoid using it.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 include/linux/cpufreq.h | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 6b882ff4dc24..aa7b105a952f 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -296,7 +296,7 @@ static inline void cpufreq_stats_record_transition(struct cpufreq_policy *policy
 #define CPUFREQ_RELATION_H 1  /* highest frequency below or at target */
 #define CPUFREQ_RELATION_C 2  /* closest frequency to target */
 /* relation flags */
-#define CPUFREQ_RELATION_E BIT(2) /* Get if possible an efficient frequency */
+#define CPUFREQ_RELATION_E (1 << 2) /* Get if possible an efficient frequency */
 
 #define CPUFREQ_RELATION_LE (CPUFREQ_RELATION_L | CPUFREQ_RELATION_E)
 #define CPUFREQ_RELATION_HE (CPUFREQ_RELATION_H | CPUFREQ_RELATION_E)
@@ -424,16 +424,16 @@ struct cpufreq_driver {
  * the diver if the target frequency does not change, but the policy min or max
  * may have changed.
  */
-#define CPUFREQ_NEED_UPDATE_LIMITS		BIT(0)
+#define CPUFREQ_NEED_UPDATE_LIMITS		(1 << 0)
 
 /* loops_per_jiffy or other kernel "constants" aren't affected by frequency transitions */
-#define CPUFREQ_CONST_LOOPS			BIT(1)
+#define CPUFREQ_CONST_LOOPS			(1 << 1)
 
 /*
  * Set by drivers that want the core to automatically register the cpufreq
  * driver as a thermal cooling device.
  */
-#define CPUFREQ_IS_COOLING_DEV			BIT(2)
+#define CPUFREQ_IS_COOLING_DEV			(1 << 2)
 
 /*
  * This should be set by platforms having multiple clock-domains, i.e.
@@ -441,14 +441,14 @@ struct cpufreq_driver {
  * be created in cpu/cpu<num>/cpufreq/ directory and so they can use the same
  * governor with different tunables for different clusters.
  */
-#define CPUFREQ_HAVE_GOVERNOR_PER_POLICY	BIT(3)
+#define CPUFREQ_HAVE_GOVERNOR_PER_POLICY	(1 << 3)
 
 /*
  * Driver will do POSTCHANGE notifications from outside of their ->target()
  * routine and so must set cpufreq_driver->flags with this flag, so that core
  * can handle them specially.
  */
-#define CPUFREQ_ASYNC_NOTIFICATION		BIT(4)
+#define CPUFREQ_ASYNC_NOTIFICATION		(1 << 4)
 
 /*
  * Set by drivers which want cpufreq core to check if CPU is running at a
@@ -457,13 +457,13 @@ struct cpufreq_driver {
  * from the table. And if that fails, we will stop further boot process by
  * issuing a BUG_ON().
  */
-#define CPUFREQ_NEED_INITIAL_FREQ_CHECK	BIT(5)
+#define CPUFREQ_NEED_INITIAL_FREQ_CHECK	(1 << 5)
 
 /*
  * Set by drivers to disallow use of governors with "dynamic_switching" flag
  * set.
  */
-#define CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING	BIT(6)
+#define CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING	(1 << 6)
 
 int cpufreq_register_driver(struct cpufreq_driver *driver_data);
 void cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
-- 
2.31.1.272.g89b43f80a514

Re: [PATCH V6 03/15] cpufreq: Rust implementation doesn't parse BIT() macro

[Greg KH]

On Tue, Jan 07, 2025 at 04:51:36PM +0530, Viresh Kumar wrote:
> Rust isn't able to parse the macro for now, avoid using it.

No, please fix it.  You don't want to have to fend off the checkpatch.pl
cleanups that this would cause of people putting the BIT() macro back.

Make BIT() work properly for Rust code as well, it has to be done
eventually, right?

thanks,

greg k-h

Re: [PATCH V6 03/15] cpufreq: Rust implementation doesn't parse BIT() macro

[Daniel Almeida]

Hi,

> On 7 Jan 2025, at 09:00, Greg KH <gregkh@linuxfoundation.org> wrote:
> 
> On Tue, Jan 07, 2025 at 04:51:36PM +0530, Viresh Kumar wrote:
>> Rust isn't able to parse the macro for now, avoid using it.
> 
> No, please fix it.  You don't want to have to fend off the checkpatch.pl
> cleanups that this would cause of people putting the BIT() macro back.
> 
> Make BIT() work properly for Rust code as well, it has to be done
> eventually, right?
> 
> thanks,
> 
> greg k-h
> 


Viresh, FYI, there’s already a patch for this floating around [0].

I can send a new version today. From Alice’s last comments, it only
needs a minor fix anyways.

— Daniel

[0] https://lore.kernel.org/rust-for-linux/20241024-topic-panthor-rs-genmask-v2-1-85237c1f0cea@collabora.com/

Re: [PATCH V6 03/15] cpufreq: Rust implementation doesn't parse BIT() macro

[Viresh Kumar]

On 07-01-25, 10:29, Daniel Almeida wrote:
> Viresh, FYI, there’s already a patch for this floating around [0].
> 
> I can send a new version today. From Alice’s last comments, it only
> needs a minor fix anyways.

Thanks for the pointer Daniel. I was expecting that I would be able to use the
definition from bindings generated with bindgen somehow.

The C header contains:

#define BIT(nr)			(1 << (nr))
#define CPUFREQ_NEED_UPDATE_LIMITS		BIT(0)

Bindgen doesn't get me a definition to CPUFREQ_NEED_UPDATE_LIMITS now. Is there
a way to make that work with nested macros ? I wanted to avoid defining them
again in the Rust side.

-- 
viresh

Re: [PATCH V6 03/15] cpufreq: Rust implementation doesn't parse BIT() macro

[Alice Ryhl]

On Wed, Jan 8, 2025 at 7:53 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 07-01-25, 10:29, Daniel Almeida wrote:
> > Viresh, FYI, there’s already a patch for this floating around [0].
> >
> > I can send a new version today. From Alice’s last comments, it only
> > needs a minor fix anyways.
>
> Thanks for the pointer Daniel. I was expecting that I would be able to use the
> definition from bindings generated with bindgen somehow.
>
> The C header contains:
>
> #define BIT(nr)                 (1 << (nr))
> #define CPUFREQ_NEED_UPDATE_LIMITS              BIT(0)
>
> Bindgen doesn't get me a definition to CPUFREQ_NEED_UPDATE_LIMITS now. Is there
> a way to make that work with nested macros ? I wanted to avoid defining them
> again in the Rust side.

Change the #define to an enum instead. See commit 3634783be125
("binder: use enum for binder ioctls") for an example of this
strategy.

Alice

Re: [PATCH V6 03/15] cpufreq: Rust implementation doesn't parse BIT() macro

[Viresh Kumar]

On 08-01-25, 10:01, Alice Ryhl wrote:
> Change the #define to an enum instead. See commit 3634783be125
> ("binder: use enum for binder ioctls") for an example of this
> strategy.

Thanks Alice. Works with this now:

Subject: [PATCH] cpufreq: Use enum for cpufreq flags that use BIT()

The BIT() macro is too complex for Rust's bindgen to interpret as
integer constants. This can cause issues when generating bindings for
use in Rust. By replacing the `#define` macros with an `enum`, we ensure
that bindgen can properly evaluate these values, enabling their seamless
use in Rust code.

No intentional functional impact.

Suggested-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 include/linux/cpufreq.h | 96 ++++++++++++++++++++++-------------------
 1 file changed, 51 insertions(+), 45 deletions(-)

diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 6b882ff4dc24..6f4283007b8c 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -292,11 +292,12 @@ static inline void cpufreq_stats_record_transition(struct cpufreq_policy *policy
  *                      CPUFREQ DRIVER INTERFACE                     *
  *********************************************************************/
 
-#define CPUFREQ_RELATION_L 0  /* lowest frequency at or above target */
-#define CPUFREQ_RELATION_H 1  /* highest frequency below or at target */
-#define CPUFREQ_RELATION_C 2  /* closest frequency to target */
-/* relation flags */
-#define CPUFREQ_RELATION_E BIT(2) /* Get if possible an efficient frequency */
+enum {
+	CPUFREQ_RELATION_L = 0, /* lowest frequency at or above target */
+	CPUFREQ_RELATION_H = BIT(0), /* highest frequency below or at target */
+	CPUFREQ_RELATION_C = BIT(1), /* closest frequency to target */
+	CPUFREQ_RELATION_E = BIT(2), /* Get if possible an efficient frequency */
+};
 
 #define CPUFREQ_RELATION_LE (CPUFREQ_RELATION_L | CPUFREQ_RELATION_E)
 #define CPUFREQ_RELATION_HE (CPUFREQ_RELATION_H | CPUFREQ_RELATION_E)
@@ -418,52 +419,57 @@ struct cpufreq_driver {
 
 /* flags */
 
-/*
- * Set by drivers that need to update internal upper and lower boundaries along
- * with the target frequency and so the core and governors should also invoke
- * the diver if the target frequency does not change, but the policy min or max
- * may have changed.
- */
-#define CPUFREQ_NEED_UPDATE_LIMITS		BIT(0)
+enum {
+	/*
+	 * Set by drivers that need to update internal upper and lower
+	 * boundaries along with the target frequency and so the core and
+	 * governors should also invoke the diver if the target frequency does
+	 * not change, but the policy min or max may have changed.
+	 */
+	CPUFREQ_NEED_UPDATE_LIMITS		= BIT(0),
 
-/* loops_per_jiffy or other kernel "constants" aren't affected by frequency transitions */
-#define CPUFREQ_CONST_LOOPS			BIT(1)
+	/*
+	 * loops_per_jiffy or other kernel "constants" aren't affected by
+	 * frequency transitions.
+	 */
+	CPUFREQ_CONST_LOOPS			= BIT(1),
 
-/*
- * Set by drivers that want the core to automatically register the cpufreq
- * driver as a thermal cooling device.
- */
-#define CPUFREQ_IS_COOLING_DEV			BIT(2)
+	/*
+	 * Set by drivers that want the core to automatically register the
+	 * cpufreq driver as a thermal cooling device.
+	 */
+	CPUFREQ_IS_COOLING_DEV			= BIT(2),
 
-/*
- * This should be set by platforms having multiple clock-domains, i.e.
- * supporting multiple policies. With this sysfs directories of governor would
- * be created in cpu/cpu<num>/cpufreq/ directory and so they can use the same
- * governor with different tunables for different clusters.
- */
-#define CPUFREQ_HAVE_GOVERNOR_PER_POLICY	BIT(3)
+	/*
+	 * This should be set by platforms having multiple clock-domains, i.e.
+	 * supporting multiple policies. With this sysfs directories of governor
+	 * would be created in cpu/cpu<num>/cpufreq/ directory and so they can
+	 * use the same governor with different tunables for different clusters.
+	 */
+	CPUFREQ_HAVE_GOVERNOR_PER_POLICY	= BIT(3),
 
-/*
- * Driver will do POSTCHANGE notifications from outside of their ->target()
- * routine and so must set cpufreq_driver->flags with this flag, so that core
- * can handle them specially.
- */
-#define CPUFREQ_ASYNC_NOTIFICATION		BIT(4)
+	/*
+	 * Driver will do POSTCHANGE notifications from outside of their
+	 * ->target() routine and so must set cpufreq_driver->flags with this
+	 *  flag, so that core can handle them specially.
+	 */
+	CPUFREQ_ASYNC_NOTIFICATION		= BIT(4),
 
-/*
- * Set by drivers which want cpufreq core to check if CPU is running at a
- * frequency present in freq-table exposed by the driver. For these drivers if
- * CPU is found running at an out of table freq, we will try to set it to a freq
- * from the table. And if that fails, we will stop further boot process by
- * issuing a BUG_ON().
- */
-#define CPUFREQ_NEED_INITIAL_FREQ_CHECK	BIT(5)
+	/*
+	 * Set by drivers which want cpufreq core to check if CPU is running at
+	 * a frequency present in freq-table exposed by the driver. For these
+	 * drivers if CPU is found running at an out of table freq, we will try
+	 * to set it to a freq from the table. And if that fails, we will stop
+	 * further boot process by issuing a BUG_ON().
+	 */
+	CPUFREQ_NEED_INITIAL_FREQ_CHECK		= BIT(5),
 
-/*
- * Set by drivers to disallow use of governors with "dynamic_switching" flag
- * set.
- */
-#define CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING	BIT(6)
+	/*
+	 * Set by drivers to disallow use of governors with "dynamic_switching"
+	 * flag set.
+	 */
+	CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING	= BIT(6),
+};
 
 int cpufreq_register_driver(struct cpufreq_driver *driver_data);
 void cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
-- 
viresh

Re: [PATCH V6 03/15] cpufreq: Rust implementation doesn't parse BIT() macro

[Viresh Kumar]

On 07-01-25, 13:00, Greg KH wrote:
> On Tue, Jan 07, 2025 at 04:51:36PM +0530, Viresh Kumar wrote:
> > Rust isn't able to parse the macro for now, avoid using it.
> 
> No, please fix it.  You don't want to have to fend off the checkpatch.pl
> cleanups that this would cause of people putting the BIT() macro back.

Yeah, I mentioned this in the cover-letter that we need to make Rust work with
it. I should have marked it as DO-NOT-MERGE patch.

I wanted to get an idea from others on how to fix it, which Danilo shared now.
Will try that.

> Make BIT() work properly for Rust code as well, it has to be done
> eventually, right?

-- 
viresh

Re: [PATCH V6 03/15] cpufreq: Rust implementation doesn't parse BIT() macro

[Miguel Ojeda]

[-- Attachment #1: Type: text/plain, Size: 2679 bytes --]

On Tue, Jan 7, 2025 at 12:22 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> Rust isn't able to parse the macro for now, avoid using it.

To clarify, the thing that struggles with `BIT` is `bindgen`, rather than Rust.

The reason is that `bindgen` does not handle non-trivial `#define`s.
The good news is that recently work got merged to make `bindgen`
understand those that resolve to a constant by (essentially) asking
the C compiler to do so. It is the `--clang-macro-fallback` flag.

Since I wanted to try it within the kernel for a while, I just did so
for `BIT` here, and it works in an standalone header, e.g.

    #include <vdso/bits.h>
    #define CPUFREQ_HAVE_GOVERNOR_PER_POLICY BIT(3)

gives me:

    pub const CPUFREQ_HAVE_GOVERNOR_PER_POLICY: u32 = 8;

with `bindgen` 0.71.1.

However, when I tried to make it work within the full kernel build, it
did not work. I reduced it to using `-include` flags for the C
compiler (which we pass a few in the kernel build). I created an issue
at:

    https://github.com/rust-lang/rust-bindgen/issues/3069

I confirmed this by including them manually at the top of the
`bindings_helper.h` file, and it worked (I get about ~1500 extra
constants). It requires something like:

    #ifdef __BINDGEN__
    #include <linux/compiler-version.h>
    #include <linux/kconfig.h>
    #include <linux/compiler_types.h>
    #endif

Plus filtering out the flags.

See the attached patch in case someone wants to play with the feature
and/or improve `bindgen`. Note: it is not a finalized patch at all,
and not intended to be applied, but it gets you a build with the
`CPUFREQ` constant available.

So we could, in principle, use it that way as a workaround. Having
said that, we still need to upgrade the `bindgen` minimum supported
version. So it may be best to simply fix the issue upstream and then
make the upgrade eventually -- `--clang-macro-fallback` is a new
feature anyway.

I also noticed a couple other issues, which strengths that suggestion,
i.e. to try to fix the sharp edges or at least get an agreed
workaround before start using the feature:

  - The dependencies file (`-Wp,-MMD,file.d`) changing behavior: I
filed https://github.com/rust-lang/rust-bindgen/issues/3070.

  - Duplicate definitions created (`pub const`s), e.g. for
https://elixir.bootlin.com/linux/v6.12.6/source/include/uapi/linux/pkt_sched.h#L598-L604.

    I filed https://github.com/rust-lang/rust-bindgen/issues/3071 (it
may be related to
https://github.com/rust-lang/rust-bindgen/issues/3066 too).

I hope that helps!

(Cc'ing John, Christian and Emilio)

Cheers,
Miguel

[-- Attachment #2: 0001-TEST-rust-try-clang-macro-feedback.patch --]
[-- Type: text/x-patch, Size: 5763 bytes --]

From 35b761799526a54cf63bf6bbb8508fd05091bcbd Mon Sep 17 00:00:00 2001
From: Miguel Ojeda <ojeda@kernel.org>
Date: Tue, 7 Jan 2025 19:55:17 +0000
Subject: [PATCH] TEST rust: try `--clang-macro-feedback`

Only intended to test the feature, with workarounds that we probably do
not want to have.

Only briefly build-tested for `srctree` builds.

Link: https://lore.kernel.org/rust-for-linux/9719ba8b3a921bd9f2cb7ebf902c54c708b5409d.1736248242.git.viresh.kumar@linaro.org/
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 rust/Makefile                           | 13 ++++++++++---
 rust/bindgen_include_issue_workaround.h | 17 +++++++++++++++++
 rust/bindgen_parameters                 | 19 +++++++++++++++++++
 rust/bindings/bindings_helper.h         |  3 +++
 rust/helpers/helpers.c                  |  2 ++
 rust/uapi/uapi_helper.h                 |  2 ++
 6 files changed, 53 insertions(+), 3 deletions(-)
 create mode 100644 rust/bindgen_include_issue_workaround.h

diff --git a/rust/Makefile b/rust/Makefile
index 9da9042fd627..3d3c1e2c0e6e 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -278,7 +278,11 @@ endif
 # prototypes for functions like `memcpy` -- if this flag is not passed,
 # `bindgen`-generated prototypes use `c_ulong` or `c_uint` depending on
 # architecture instead of generating `usize`.
-bindgen_c_flags_final = $(bindgen_c_flags_lto) -fno-builtin -D__BINDGEN__
+bindgen_c_flags_final = $(filter-out -include \
+	%include/linux/compiler-version.h \
+	%include/linux/kconfig.h \
+	%include/linux/compiler_types.h \
+,$(bindgen_c_flags_lto)) -fno-builtin -D__BINDGEN__
 
 quiet_cmd_bindgen = BINDGEN $@
       cmd_bindgen = \
@@ -288,19 +292,22 @@ quiet_cmd_bindgen = BINDGEN $@
 		-o $@ -- $(bindgen_c_flags_final) -DMODULE \
 		$(bindgen_target_cflags) $(bindgen_target_extra)
 
+# TODO: proper workaround for `--clang-macro-fallback` interaction with `-Wp,-MMD,file.d`:
+#
+#     https://github.com/rust-lang/rust-bindgen/issues/3070
 $(obj)/bindings/bindings_generated.rs: private bindgen_target_flags = \
     $(shell grep -Ev '^#|^$$' $(src)/bindgen_parameters)
 $(obj)/bindings/bindings_generated.rs: private bindgen_target_extra = ; \
     sed -Ei 's/pub const RUST_CONST_HELPER_([a-zA-Z0-9_]*)/pub const \1/g' $@
 $(obj)/bindings/bindings_generated.rs: $(src)/bindings/bindings_helper.h \
     $(src)/bindgen_parameters FORCE
-	$(call if_changed_dep,bindgen)
+	$(call if_changed,bindgen)
 
 $(obj)/uapi/uapi_generated.rs: private bindgen_target_flags = \
     $(shell grep -Ev '^#|^$$' $(src)/bindgen_parameters)
 $(obj)/uapi/uapi_generated.rs: $(src)/uapi/uapi_helper.h \
     $(src)/bindgen_parameters FORCE
-	$(call if_changed_dep,bindgen)
+	$(call if_changed,bindgen)
 
 # See `CFLAGS_REMOVE_helpers.o` above. In addition, Clang on C does not warn
 # with `-Wmissing-declarations` (unlike GCC), so it is not strictly needed here
diff --git a/rust/bindgen_include_issue_workaround.h b/rust/bindgen_include_issue_workaround.h
new file mode 100644
index 000000000000..d3ad51210732
--- /dev/null
+++ b/rust/bindgen_include_issue_workaround.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Workaround `bindgen`'s `--clang-macro-fallback` issue with `-include`:
+ *
+ *    https://github.com/rust-lang/rust-bindgen/issues/3069
+ */
+
+#ifndef __BINDGEN_INCLUDE_ISSUE_WORKAROUND_H
+#define __BINDGEN_INCLUDE_ISSUE_WORKAROUND_H
+
+#ifdef __BINDGEN__
+#include <linux/compiler-version.h>
+#include <linux/kconfig.h>
+#include <linux/compiler_types.h>
+#endif /* __BINDGEN__ */
+
+#endif /* __BINDGEN_INCLUDE_ISSUE_WORKAROUND_H */
diff --git a/rust/bindgen_parameters b/rust/bindgen_parameters
index 0f96af8b9a7f..3452b88e1a1c 100644
--- a/rust/bindgen_parameters
+++ b/rust/bindgen_parameters
@@ -1,5 +1,24 @@
 # SPDX-License-Identifier: GPL-2.0
 
+--clang-macro-fallback
+
+# Workaround for `--clang-macro-fallback` duplicate definitions:
+#
+#     https://github.com/rust-lang/rust-bindgen/issues/3071
+--blocklist-item __TC_MQPRIO_MODE_MAX
+--blocklist-item __TC_MQPRIO_SHAPER_MAX
+--blocklist-item TCA_POLICE_RESULT
+--blocklist-item BPF_F_LINK
+--blocklist-item PAGE_SIZE
+--blocklist-item GFP_ATOMIC
+--blocklist-item GFP_KERNEL
+--blocklist-item GFP_KERNEL_ACCOUNT
+--blocklist-item GFP_NOWAIT
+--blocklist-item __GFP_ZERO
+--blocklist-item __GFP_HIGHMEM
+--blocklist-item __GFP_NOWARN
+--blocklist-item BLK_FEAT_ROTATIONAL
+
 # We want to map these types to `isize`/`usize` manually, instead of
 # define them as `int`/`long` depending on platform bitwidth.
 --blocklist-type __kernel_s?size_t
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 5c4dfe22f41a..e98ba87efd4b 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -6,10 +6,13 @@
  * Sorted alphabetically.
  */
 
+#include "../bindgen_include_issue_workaround.h"
+
 #include <kunit/test.h>
 #include <linux/blk-mq.h>
 #include <linux/blk_types.h>
 #include <linux/blkdev.h>
+#include <linux/cpufreq.h>
 #include <linux/cred.h>
 #include <linux/errname.h>
 #include <linux/ethtool.h>
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index dcf827a61b52..4048afe1008f 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -7,6 +7,8 @@
  * Sorted alphabetically.
  */
 
+#include "../bindgen_include_issue_workaround.h"
+
 #include "blk.c"
 #include "bug.c"
 #include "build_assert.c"
diff --git a/rust/uapi/uapi_helper.h b/rust/uapi/uapi_helper.h
index 76d3f103e764..e54d4177d20e 100644
--- a/rust/uapi/uapi_helper.h
+++ b/rust/uapi/uapi_helper.h
@@ -6,6 +6,8 @@
  * Sorted alphabetically.
  */
 
+#include "../bindgen_include_issue_workaround.h"
+
 #include <uapi/asm-generic/ioctl.h>
 #include <uapi/linux/mdio.h>
 #include <uapi/linux/mii.h>
-- 
2.47.1

[PATCH V6 04/15] rust: device: Add few helpers

[Viresh Kumar]

Add from_cpu() and property_present() helpers to the device bindings.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 rust/bindings/bindings_helper.h |  1 +
 rust/kernel/device.rs           | 21 +++++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 43f5c381aab0..70e4b7b0f638 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -31,6 +31,7 @@
 #include <linux/pid_namespace.h>
 #include <linux/platform_device.h>
 #include <linux/poll.h>
+#include <linux/property.h>
 #include <linux/refcount.h>
 #include <linux/sched.h>
 #include <linux/security.h>
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index d5e6a19ff6b7..5bfbc4bdfadc 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -6,6 +6,9 @@
 
 use crate::{
     bindings,
+    error::Result,
+    prelude::ENODEV,
+    str::CString,
     types::{ARef, Opaque},
 };
 use core::{fmt, ptr};
@@ -59,6 +62,18 @@ pub unsafe fn get_device(ptr: *mut bindings::device) -> ARef<Self> {
         unsafe { Self::as_ref(ptr) }.into()
     }
 
+    /// Creates a new ref-counted instance of device of a CPU.
+    pub fn from_cpu(cpu: u32) -> Result<ARef<Self>> {
+        // SAFETY: It is safe to call `get_cpu_device()` for any CPU number.
+        let ptr = unsafe { bindings::get_cpu_device(cpu) };
+        if ptr.is_null() {
+            return Err(ENODEV);
+        }
+
+        // SAFETY: By the safety requirements, ptr is valid.
+        Ok(unsafe { Device::get_device(ptr) })
+    }
+
     /// Obtain the raw `struct device *`.
     pub(crate) fn as_raw(&self) -> *mut bindings::device {
         self.0.get()
@@ -180,6 +195,12 @@ unsafe fn printk(&self, klevel: &[u8], msg: fmt::Arguments<'_>) {
             )
         };
     }
+
+    /// Checks if property is present or not.
+    pub fn property_present(&self, name: &CString) -> bool {
+        // SAFETY: `name` is null-terminated. `self.as_raw` is valid because `self` is valid.
+        unsafe { bindings::device_property_present(self.as_raw(), name.as_ptr() as *const _) }
+    }
 }
 
 // SAFETY: Instances of `Device` are always reference-counted.
-- 
2.31.1.272.g89b43f80a514

Re: [PATCH V6 04/15] rust: device: Add few helpers

[Greg Kroah-Hartman]

On Tue, Jan 07, 2025 at 04:51:37PM +0530, Viresh Kumar wrote:
> Add from_cpu() and property_present() helpers to the device bindings.

That says what, but not why.

Also those are two totally different things, I'm going to argue with you
about one of them...

> +    /// Creates a new ref-counted instance of device of a CPU.
> +    pub fn from_cpu(cpu: u32) -> Result<ARef<Self>> {

Why is this a reference counted device, yet the C structure is NOT
properly reference counted at all?  Are you _sure_ this is going to work
properly?

And really, we should fix up the C side to properly reference count all
of this.  Just read the comment in cpu_device_release() for a hint at
what needs to be done here.

> +        // SAFETY: It is safe to call `get_cpu_device()` for any CPU number.

For any number at all, no need to say "CPU" here, right?

> +        let ptr = unsafe { bindings::get_cpu_device(cpu) };
> +        if ptr.is_null() {
> +            return Err(ENODEV);
> +        }
> +
> +        // SAFETY: By the safety requirements, ptr is valid.
> +        Ok(unsafe { Device::get_device(ptr) })

So why is this device reference counted?  I get it that it should be,
but how does that play with the "real" device here?

> +    }
> +
>      /// Obtain the raw `struct device *`.
>      pub(crate) fn as_raw(&self) -> *mut bindings::device {
>          self.0.get()
> @@ -180,6 +195,12 @@ unsafe fn printk(&self, klevel: &[u8], msg: fmt::Arguments<'_>) {
>              )
>          };
>      }
> +
> +    /// Checks if property is present or not.
> +    pub fn property_present(&self, name: &CString) -> bool {
> +        // SAFETY: `name` is null-terminated. `self.as_raw` is valid because `self` is valid.
> +        unsafe { bindings::device_property_present(self.as_raw(), name.as_ptr() as *const _) }

is "self.as_raw()" a constant pointer too?

thanks,

greg k-h

Re: [PATCH V6 04/15] rust: device: Add few helpers

[Viresh Kumar]

On 07-01-25, 12:56, Greg Kroah-Hartman wrote:
> On Tue, Jan 07, 2025 at 04:51:37PM +0530, Viresh Kumar wrote:
> > +    /// Creates a new ref-counted instance of device of a CPU.
> > +    pub fn from_cpu(cpu: u32) -> Result<ARef<Self>> {
> 
> Why is this a reference counted device, yet the C structure is NOT
> properly reference counted at all?

Ahh, I completely missed that it is not reference counted at all.

> Are you _sure_ this is going to work properly?
> 
> And really, we should fix up the C side to properly reference count all
> of this.  Just read the comment in cpu_device_release() for a hint at
> what needs to be done here.
> 
> > +        // SAFETY: It is safe to call `get_cpu_device()` for any CPU number.
> 
> For any number at all, no need to say "CPU" here, right?
> 
> > +        let ptr = unsafe { bindings::get_cpu_device(cpu) };
> > +        if ptr.is_null() {
> > +            return Err(ENODEV);
> > +        }
> > +
> > +        // SAFETY: By the safety requirements, ptr is valid.
> > +        Ok(unsafe { Device::get_device(ptr) })
> 
> So why is this device reference counted?  I get it that it should be,
> but how does that play with the "real" device here?

How about this:

Subject: [PATCH] rust: device: Add from_cpu()

This implements Device::from_cpu(), which returns a reference to
`Device` for a CPU. The C struct is created at initialization time for
CPUs and is never freed and so `ARef` isn't returned from this function.

The new helper will be used by Rust based cpufreq drivers.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 rust/kernel/device.rs | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index 66ba0782551a..007f9ffab08b 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -6,6 +6,8 @@
 
 use crate::{
     bindings,
+    error::Result,
+    prelude::ENODEV,
     str::CString,
     types::{ARef, Opaque},
 };
@@ -60,6 +62,20 @@ pub unsafe fn get_device(ptr: *mut bindings::device) -> ARef<Self> {
         unsafe { Self::as_ref(ptr) }.into()
     }
 
+    /// Creates a new instance of CPU's device.
+    pub fn from_cpu(cpu: u32) -> Result<&'static Self> {
+        // SAFETY: The pointer returned by `get_cpu_device()`, if not `NULL`, is a valid pointer to
+        // a `struct device` and is never freed by the C code.
+        let ptr = unsafe { bindings::get_cpu_device(cpu) };
+        if ptr.is_null() {
+            return Err(ENODEV);
+        }
+
+        // SAFETY: The pointer returned by `get_cpu_device()`, if not `NULL`, is a valid pointer to
+        // a `struct device` and is never freed by the C code.
+        Ok(unsafe { Self::as_ref(ptr) })
+    }
+
     /// Obtain the raw `struct device *`.
     pub(crate) fn as_raw(&self) -> *mut bindings::device {
         self.0.get()

-------------------------8<-------------------------

> > +    /// Checks if property is present or not.
> > +    pub fn property_present(&self, name: &CString) -> bool {
> > +        // SAFETY: `name` is null-terminated. `self.as_raw` is valid because `self` is valid.
> > +        unsafe { bindings::device_property_present(self.as_raw(), name.as_ptr() as *const _) }
> 
> is "self.as_raw()" a constant pointer too?

Subject: [PATCH] rust: device: Add property_present()

This implements Device::property_present(), which calls C APIs
device_property_present() helper.

The new helper will be used by Rust based cpufreq drivers.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 rust/bindings/bindings_helper.h | 1 +
 rust/kernel/device.rs           | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 43f5c381aab0..70e4b7b0f638 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -31,6 +31,7 @@
 #include <linux/pid_namespace.h>
 #include <linux/platform_device.h>
 #include <linux/poll.h>
+#include <linux/property.h>
 #include <linux/refcount.h>
 #include <linux/sched.h>
 #include <linux/security.h>
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index d5e6a19ff6b7..66ba0782551a 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -6,6 +6,7 @@
 
 use crate::{
     bindings,
+    str::CString,
     types::{ARef, Opaque},
 };
 use core::{fmt, ptr};
@@ -180,6 +181,12 @@ unsafe fn printk(&self, klevel: &[u8], msg: fmt::Arguments<'_>) {
             )
         };
     }
+
+    /// Checks if property is present or not.
+    pub fn property_present(&self, name: &CString) -> bool {
+        // SAFETY: By the invariant of `CString`, `name` is null-terminated.
+        unsafe { bindings::device_property_present(self.as_raw() as *const _, name.as_ptr() as *const _) }
+    }
 }
 
 // SAFETY: Instances of `Device` are always reference-counted.

-- 
viresh

Re: [PATCH V6 04/15] rust: device: Add few helpers

[Greg Kroah-Hartman]

On Wed, Jan 08, 2025 at 04:32:42PM +0530, Viresh Kumar wrote:
> On 07-01-25, 12:56, Greg Kroah-Hartman wrote:
> > On Tue, Jan 07, 2025 at 04:51:37PM +0530, Viresh Kumar wrote:
> > > +    /// Creates a new ref-counted instance of device of a CPU.
> > > +    pub fn from_cpu(cpu: u32) -> Result<ARef<Self>> {
> > 
> > Why is this a reference counted device, yet the C structure is NOT
> > properly reference counted at all?
> 
> Ahh, I completely missed that it is not reference counted at all.
> 
> > Are you _sure_ this is going to work properly?
> > 
> > And really, we should fix up the C side to properly reference count all
> > of this.  Just read the comment in cpu_device_release() for a hint at
> > what needs to be done here.
> > 
> > > +        // SAFETY: It is safe to call `get_cpu_device()` for any CPU number.
> > 
> > For any number at all, no need to say "CPU" here, right?
> > 
> > > +        let ptr = unsafe { bindings::get_cpu_device(cpu) };
> > > +        if ptr.is_null() {
> > > +            return Err(ENODEV);
> > > +        }
> > > +
> > > +        // SAFETY: By the safety requirements, ptr is valid.
> > > +        Ok(unsafe { Device::get_device(ptr) })
> > 
> > So why is this device reference counted?  I get it that it should be,
> > but how does that play with the "real" device here?
> 
> How about this:
> 
> Subject: [PATCH] rust: device: Add from_cpu()
> 
> This implements Device::from_cpu(), which returns a reference to
> `Device` for a CPU. The C struct is created at initialization time for
> CPUs and is never freed and so `ARef` isn't returned from this function.

How about fixing the reference count of the cpu device?  :)

But seriously, this is NOT a generic 'struct device' thing, it is a 'cpu
device' thing.  So putting this function in device.rs is probably not
the proper place for it at all, sorry.  Why not put it in the cpu.rs
file instead?

> The new helper will be used by Rust based cpufreq drivers.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  rust/kernel/device.rs | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index 66ba0782551a..007f9ffab08b 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs
> @@ -6,6 +6,8 @@
>  
>  use crate::{
>      bindings,
> +    error::Result,
> +    prelude::ENODEV,
>      str::CString,
>      types::{ARef, Opaque},
>  };
> @@ -60,6 +62,20 @@ pub unsafe fn get_device(ptr: *mut bindings::device) -> ARef<Self> {
>          unsafe { Self::as_ref(ptr) }.into()
>      }
>  
> +    /// Creates a new instance of CPU's device.
> +    pub fn from_cpu(cpu: u32) -> Result<&'static Self> {
> +        // SAFETY: The pointer returned by `get_cpu_device()`, if not `NULL`, is a valid pointer to
> +        // a `struct device` and is never freed by the C code.
> +        let ptr = unsafe { bindings::get_cpu_device(cpu) };
> +        if ptr.is_null() {
> +            return Err(ENODEV);
> +        }
> +
> +        // SAFETY: The pointer returned by `get_cpu_device()`, if not `NULL`, is a valid pointer to
> +        // a `struct device` and is never freed by the C code.
> +        Ok(unsafe { Self::as_ref(ptr) })
> +    }
> +
>      /// Obtain the raw `struct device *`.
>      pub(crate) fn as_raw(&self) -> *mut bindings::device {
>          self.0.get()
> 
> -------------------------8<-------------------------
> 
> > > +    /// Checks if property is present or not.
> > > +    pub fn property_present(&self, name: &CString) -> bool {
> > > +        // SAFETY: `name` is null-terminated. `self.as_raw` is valid because `self` is valid.
> > > +        unsafe { bindings::device_property_present(self.as_raw(), name.as_ptr() as *const _) }
> > 
> > is "self.as_raw()" a constant pointer too?
> 
> Subject: [PATCH] rust: device: Add property_present()
> 
> This implements Device::property_present(), which calls C APIs
> device_property_present() helper.
> 
> The new helper will be used by Rust based cpufreq drivers.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  rust/bindings/bindings_helper.h | 1 +
>  rust/kernel/device.rs           | 7 +++++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 43f5c381aab0..70e4b7b0f638 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -31,6 +31,7 @@
>  #include <linux/pid_namespace.h>
>  #include <linux/platform_device.h>
>  #include <linux/poll.h>
> +#include <linux/property.h>
>  #include <linux/refcount.h>
>  #include <linux/sched.h>
>  #include <linux/security.h>
> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index d5e6a19ff6b7..66ba0782551a 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs
> @@ -6,6 +6,7 @@
>  
>  use crate::{
>      bindings,
> +    str::CString,
>      types::{ARef, Opaque},
>  };
>  use core::{fmt, ptr};
> @@ -180,6 +181,12 @@ unsafe fn printk(&self, klevel: &[u8], msg: fmt::Arguments<'_>) {
>              )
>          };
>      }
> +
> +    /// Checks if property is present or not.
> +    pub fn property_present(&self, name: &CString) -> bool {
> +        // SAFETY: By the invariant of `CString`, `name` is null-terminated.
> +        unsafe { bindings::device_property_present(self.as_raw() as *const _, name.as_ptr() as *const _) }

I hate to ask, but how was this compiling if the const wasn't there
before?  There's no type-checking happening here?  If not, how are we
ever going to notice when function parameters change?  If there is type
checking, how did this ever build without the const?

confused,

greg k-h

Re: [PATCH V6 04/15] rust: device: Add few helpers

[Alice Ryhl]

On Wed, Jan 8, 2025 at 12:52 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> > +    /// Checks if property is present or not.
> > +    pub fn property_present(&self, name: &CString) -> bool {
> > +        // SAFETY: By the invariant of `CString`, `name` is null-terminated.
> > +        unsafe { bindings::device_property_present(self.as_raw() as *const _, name.as_ptr() as *const _) }
>
> I hate to ask, but how was this compiling if the const wasn't there
> before?  There's no type-checking happening here?  If not, how are we
> ever going to notice when function parameters change?  If there is type
> checking, how did this ever build without the const?
>
> confused,

Rust auto-converts `*mut` pointers to `*const` when necessary.

Note that this should really be `self.as_raw().cast_const()` if you're
just casting mut to const without changing the pointee type.

Alice

Re: [PATCH V6 04/15] rust: device: Add few helpers

[Greg Kroah-Hartman]

On Wed, Jan 08, 2025 at 12:55:38PM +0100, Alice Ryhl wrote:
> On Wed, Jan 8, 2025 at 12:52 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > > +    /// Checks if property is present or not.
> > > +    pub fn property_present(&self, name: &CString) -> bool {
> > > +        // SAFETY: By the invariant of `CString`, `name` is null-terminated.
> > > +        unsafe { bindings::device_property_present(self.as_raw() as *const _, name.as_ptr() as *const _) }
> >
> > I hate to ask, but how was this compiling if the const wasn't there
> > before?  There's no type-checking happening here?  If not, how are we
> > ever going to notice when function parameters change?  If there is type
> > checking, how did this ever build without the const?
> >
> > confused,
> 
> Rust auto-converts `*mut` pointers to `*const` when necessary.

Ah, so it's magic :)

thanks for the explaination.

greg k-h

Re: [PATCH V6 04/15] rust: device: Add few helpers

[Danilo Krummrich]

On Wed, Jan 08, 2025 at 12:52:54PM +0100, Greg Kroah-Hartman wrote:
> On Wed, Jan 08, 2025 at 04:32:42PM +0530, Viresh Kumar wrote:
> > On 07-01-25, 12:56, Greg Kroah-Hartman wrote:
> > > On Tue, Jan 07, 2025 at 04:51:37PM +0530, Viresh Kumar wrote:
> > > > +    /// Creates a new ref-counted instance of device of a CPU.
> > > > +    pub fn from_cpu(cpu: u32) -> Result<ARef<Self>> {
> > > 
> > > Why is this a reference counted device, yet the C structure is NOT
> > > properly reference counted at all?
> > 
> > Ahh, I completely missed that it is not reference counted at all.
> > 
> > > Are you _sure_ this is going to work properly?
> > > 
> > > And really, we should fix up the C side to properly reference count all
> > > of this.  Just read the comment in cpu_device_release() for a hint at
> > > what needs to be done here.
> > > 
> > > > +        // SAFETY: It is safe to call `get_cpu_device()` for any CPU number.
> > > 
> > > For any number at all, no need to say "CPU" here, right?
> > > 
> > > > +        let ptr = unsafe { bindings::get_cpu_device(cpu) };
> > > > +        if ptr.is_null() {
> > > > +            return Err(ENODEV);
> > > > +        }
> > > > +
> > > > +        // SAFETY: By the safety requirements, ptr is valid.
> > > > +        Ok(unsafe { Device::get_device(ptr) })
> > > 
> > > So why is this device reference counted?  I get it that it should be,
> > > but how does that play with the "real" device here?
> > 
> > How about this:
> > 
> > Subject: [PATCH] rust: device: Add from_cpu()
> > 
> > This implements Device::from_cpu(), which returns a reference to
> > `Device` for a CPU. The C struct is created at initialization time for
> > CPUs and is never freed and so `ARef` isn't returned from this function.
> 
> How about fixing the reference count of the cpu device?  :)

I think that's really what is needed, otherwise it'll never work with the
guarantees the Rust `Device` abstraction provides.

The patch below is still not valid I think. It assumes that a CPU device never
becomes invalid, but that isn't true.

There's a hotplug path [1] where the device is unregistered.

[1] https://elixir.bootlin.com/linux/v6.12.6/source/drivers/base/cpu.c#L94

> 
> But seriously, this is NOT a generic 'struct device' thing, it is a 'cpu
> device' thing.  So putting this function in device.rs is probably not
> the proper place for it at all, sorry.  Why not put it in the cpu.rs
> file instead?
> 
> > The new helper will be used by Rust based cpufreq drivers.
> > 
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> >  rust/kernel/device.rs | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> > 
> > diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> > index 66ba0782551a..007f9ffab08b 100644
> > --- a/rust/kernel/device.rs
> > +++ b/rust/kernel/device.rs
> > @@ -6,6 +6,8 @@
> >  
> >  use crate::{
> >      bindings,
> > +    error::Result,
> > +    prelude::ENODEV,
> >      str::CString,
> >      types::{ARef, Opaque},
> >  };
> > @@ -60,6 +62,20 @@ pub unsafe fn get_device(ptr: *mut bindings::device) -> ARef<Self> {
> >          unsafe { Self::as_ref(ptr) }.into()
> >      }
> >  
> > +    /// Creates a new instance of CPU's device.
> > +    pub fn from_cpu(cpu: u32) -> Result<&'static Self> {
> > +        // SAFETY: The pointer returned by `get_cpu_device()`, if not `NULL`, is a valid pointer to
> > +        // a `struct device` and is never freed by the C code.
> > +        let ptr = unsafe { bindings::get_cpu_device(cpu) };
> > +        if ptr.is_null() {
> > +            return Err(ENODEV);
> > +        }
> > +
> > +        // SAFETY: The pointer returned by `get_cpu_device()`, if not `NULL`, is a valid pointer to
> > +        // a `struct device` and is never freed by the C code.
> > +        Ok(unsafe { Self::as_ref(ptr) })
> > +    }
> > +
> >      /// Obtain the raw `struct device *`.
> >      pub(crate) fn as_raw(&self) -> *mut bindings::device {
> >          self.0.get()
> > 
> > -------------------------8<-------------------------
> > 
> > > > +    /// Checks if property is present or not.
> > > > +    pub fn property_present(&self, name: &CString) -> bool {
> > > > +        // SAFETY: `name` is null-terminated. `self.as_raw` is valid because `self` is valid.
> > > > +        unsafe { bindings::device_property_present(self.as_raw(), name.as_ptr() as *const _) }
> > > 
> > > is "self.as_raw()" a constant pointer too?
> > 
> > Subject: [PATCH] rust: device: Add property_present()
> > 
> > This implements Device::property_present(), which calls C APIs
> > device_property_present() helper.
> > 
> > The new helper will be used by Rust based cpufreq drivers.
> > 
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> >  rust/bindings/bindings_helper.h | 1 +
> >  rust/kernel/device.rs           | 7 +++++++
> >  2 files changed, 8 insertions(+)
> > 
> > diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> > index 43f5c381aab0..70e4b7b0f638 100644
> > --- a/rust/bindings/bindings_helper.h
> > +++ b/rust/bindings/bindings_helper.h
> > @@ -31,6 +31,7 @@
> >  #include <linux/pid_namespace.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/poll.h>
> > +#include <linux/property.h>
> >  #include <linux/refcount.h>
> >  #include <linux/sched.h>
> >  #include <linux/security.h>
> > diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> > index d5e6a19ff6b7..66ba0782551a 100644
> > --- a/rust/kernel/device.rs
> > +++ b/rust/kernel/device.rs
> > @@ -6,6 +6,7 @@
> >  
> >  use crate::{
> >      bindings,
> > +    str::CString,
> >      types::{ARef, Opaque},
> >  };
> >  use core::{fmt, ptr};
> > @@ -180,6 +181,12 @@ unsafe fn printk(&self, klevel: &[u8], msg: fmt::Arguments<'_>) {
> >              )
> >          };
> >      }
> > +
> > +    /// Checks if property is present or not.
> > +    pub fn property_present(&self, name: &CString) -> bool {
> > +        // SAFETY: By the invariant of `CString`, `name` is null-terminated.
> > +        unsafe { bindings::device_property_present(self.as_raw() as *const _, name.as_ptr() as *const _) }
> 
> I hate to ask, but how was this compiling if the const wasn't there
> before?  There's no type-checking happening here?  If not, how are we
> ever going to notice when function parameters change?  If there is type
> checking, how did this ever build without the const?
> 
> confused,
> 
> greg k-h
> 

Re: [PATCH V6 04/15] rust: device: Add few helpers

[Viresh Kumar]

On 08-01-25, 14:42, Danilo Krummrich wrote:
> I think that's really what is needed, otherwise it'll never work with the
> guarantees the Rust `Device` abstraction provides.

> The patch below is still not valid I think. It assumes that a CPU device never
> becomes invalid, but that isn't true.
> 
> There's a hotplug path [1] where the device is unregistered.

Right, though the device pointer always points to valid memory as the struct cpu
is never freed. Isn't that enough for a pointer passed over FFI ? All the
from_cpu() method returns is a reference, which will be used for a short period
only (yes it is about the possibility of something going wrong in that period
only and we need to ensure it doesn't break in such corner cases).

FWIW, the cpufreq framework is registered with CPU hotplug layer, and so
whenever a CPU disappears, the cpufreq core will stop using its device pointer
before the CPU is removed. So technically we shouldn't land in a situation where
the CPU is unregistered and cpufreq core is still using the CPU's device
pointer.

-- 
viresh

Re: [PATCH V6 04/15] rust: device: Add few helpers

[Viresh Kumar]

On 08-01-25, 12:52, Greg Kroah-Hartman wrote:
> How about fixing the reference count of the cpu device?  :)

I would try my best to avoid getting into that trap :)

> But seriously, this is NOT a generic 'struct device' thing, it is a 'cpu
> device' thing.  So putting this function in device.rs is probably not
> the proper place for it at all, sorry.  Why not put it in the cpu.rs
> file instead?

I can do that, so it will be a standalone helper function in cpu.rs and not a
method in an `impl` block.

-- 
viresh

Re: [PATCH V6 04/15] rust: device: Add few helpers

[Danilo Krummrich]

On Tue, Jan 07, 2025 at 04:51:37PM +0530, Viresh Kumar wrote:
> Add from_cpu() and property_present() helpers to the device bindings.

I think you should split this into two separate patches.

> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  rust/bindings/bindings_helper.h |  1 +
>  rust/kernel/device.rs           | 21 +++++++++++++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 43f5c381aab0..70e4b7b0f638 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -31,6 +31,7 @@
>  #include <linux/pid_namespace.h>
>  #include <linux/platform_device.h>
>  #include <linux/poll.h>
> +#include <linux/property.h>
>  #include <linux/refcount.h>
>  #include <linux/sched.h>
>  #include <linux/security.h>
> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index d5e6a19ff6b7..5bfbc4bdfadc 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs
> @@ -6,6 +6,9 @@
>  
>  use crate::{
>      bindings,
> +    error::Result,
> +    prelude::ENODEV,
> +    str::CString,
>      types::{ARef, Opaque},
>  };
>  use core::{fmt, ptr};
> @@ -59,6 +62,18 @@ pub unsafe fn get_device(ptr: *mut bindings::device) -> ARef<Self> {
>          unsafe { Self::as_ref(ptr) }.into()
>      }
>  
> +    /// Creates a new ref-counted instance of device of a CPU.
> +    pub fn from_cpu(cpu: u32) -> Result<ARef<Self>> {
> +        // SAFETY: It is safe to call `get_cpu_device()` for any CPU number.
> +        let ptr = unsafe { bindings::get_cpu_device(cpu) };
> +        if ptr.is_null() {
> +            return Err(ENODEV);
> +        }
> +
> +        // SAFETY: By the safety requirements, ptr is valid.

There are no safety requirements for from_cpu().

Instead, you should say that the pointer returned by get_cpu_device(), if not
NULL, is a valid pointer to a struct device with a non-zero reference count.

> +        Ok(unsafe { Device::get_device(ptr) })
> +    }
> +
>      /// Obtain the raw `struct device *`.
>      pub(crate) fn as_raw(&self) -> *mut bindings::device {
>          self.0.get()
> @@ -180,6 +195,12 @@ unsafe fn printk(&self, klevel: &[u8], msg: fmt::Arguments<'_>) {
>              )
>          };
>      }
> +
> +    /// Checks if property is present or not.
> +    pub fn property_present(&self, name: &CString) -> bool {
> +        // SAFETY: `name` is null-terminated. `self.as_raw` is valid because `self` is valid.

Maybe "by the invariant of `CString` `name` is null-terminated."?

> +        unsafe { bindings::device_property_present(self.as_raw(), name.as_ptr() as *const _) }
> +    }
>  }
>  
>  // SAFETY: Instances of `Device` are always reference-counted.
> -- 
> 2.31.1.272.g89b43f80a514
> 

[PATCH V6 05/15] rust: Add bindings for cpumask

[Viresh Kumar]

Add basic Rust bindings for `struct cpumask`. Also add few Rust helpers
for the same.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 rust/bindings/bindings_helper.h |  1 +
 rust/helpers/cpumask.c          | 35 ++++++++++++++
 rust/helpers/helpers.c          |  1 +
 rust/kernel/cpumask.rs          | 85 +++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |  1 +
 5 files changed, 123 insertions(+)
 create mode 100644 rust/helpers/cpumask.c
 create mode 100644 rust/kernel/cpumask.rs

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 70e4b7b0f638..3225379abd2b 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -15,6 +15,7 @@
 #include <linux/blk-mq.h>
 #include <linux/blk_types.h>
 #include <linux/blkdev.h>
+#include <linux/cpumask.h>
 #include <linux/cred.h>
 #include <linux/errname.h>
 #include <linux/ethtool.h>
diff --git a/rust/helpers/cpumask.c b/rust/helpers/cpumask.c
new file mode 100644
index 000000000000..0b371826a13c
--- /dev/null
+++ b/rust/helpers/cpumask.c
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/cpumask.h>
+
+void rust_helper_cpumask_set_cpu(unsigned int cpu, struct cpumask *dstp)
+{
+	cpumask_set_cpu(cpu, dstp);
+}
+
+void rust_helper_cpumask_clear_cpu(int cpu, struct cpumask *dstp)
+{
+	cpumask_clear_cpu(cpu, dstp);
+}
+
+void rust_helper_cpumask_setall(struct cpumask *dstp)
+{
+	cpumask_setall(dstp);
+}
+
+void rust_helper_cpumask_copy(struct cpumask *dstp, const struct cpumask *srcp)
+{
+	cpumask_copy(dstp, srcp);
+}
+
+bool rust_helper_zalloc_cpumask_var(cpumask_var_t *mask, gfp_t flags)
+{
+	return zalloc_cpumask_var(mask, flags);
+}
+
+#ifndef CONFIG_CPUMASK_OFFSTACK
+void rust_helper_free_cpumask_var(cpumask_var_t mask)
+{
+	free_cpumask_var(mask);
+}
+#endif
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 2f2070c15f09..1943b98aec2b 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -11,6 +11,7 @@
 #include "bug.c"
 #include "build_assert.c"
 #include "build_bug.c"
+#include "cpumask.c"
 #include "cred.c"
 #include "device.c"
 #include "drm.c"
diff --git a/rust/kernel/cpumask.rs b/rust/kernel/cpumask.rs
new file mode 100644
index 000000000000..e3b15bc12798
--- /dev/null
+++ b/rust/kernel/cpumask.rs
@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! CPU mask abstractions.
+//!
+//! C header: [`include/linux/cpumask.h`](srctree/include/linux/cpumask.h)
+
+use crate::{bindings, error::Result, prelude::ENOMEM};
+use core::ptr;
+
+/// A simple implementation of `struct cpumask` from the C code.
+pub struct Cpumask {
+    ptr: *mut bindings::cpumask,
+    owned: bool,
+}
+
+impl Cpumask {
+    /// Creates empty cpumask.
+    pub fn new() -> Result<Self> {
+        let mut ptr: *mut bindings::cpumask = ptr::null_mut();
+
+        // SAFETY: Depending on the value of `gfp_flags`, this call may sleep. Other than that, it
+        // is always safe to call this method.
+        if !unsafe { bindings::zalloc_cpumask_var(&mut ptr, bindings::GFP_KERNEL) } {
+            return Err(ENOMEM);
+        }
+
+        Ok(Self { ptr, owned: true })
+    }
+
+    /// Creates a new abstraction instance of an existing `struct cpumask` pointer.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that `ptr` is valid, and non-null.
+    pub unsafe fn get_cpumask(ptr: *mut bindings::cpumask) -> Self {
+        Self { ptr, owned: false }
+    }
+
+    /// Obtain the raw `struct cpumask *`.
+    pub fn as_raw(&mut self) -> *mut bindings::cpumask {
+        self.ptr
+    }
+
+    /// Sets CPU in the cpumask.
+    ///
+    /// Update the cpumask with a single CPU.
+    pub fn set(&mut self, cpu: u32) {
+        // SAFETY: `ptr` is guaranteed to be valid for the lifetime of `self`. And it is safe to
+        // call `cpumask_set_cpus()` for any CPU.
+        unsafe { bindings::cpumask_set_cpu(cpu, self.ptr) };
+    }
+
+    /// Clears CPU in the cpumask.
+    ///
+    /// Update the cpumask with a single CPU.
+    pub fn clear(&mut self, cpu: i32) {
+        // SAFETY: `ptr` is guaranteed to be valid for the lifetime of `self`. And it is safe to
+        // call `cpumask_clear_cpu()` for any CPU.
+        unsafe { bindings::cpumask_clear_cpu(cpu, self.ptr) };
+    }
+
+    /// Sets all CPUs in the cpumask.
+    pub fn set_all(&mut self) {
+        // SAFETY: `ptr` is guaranteed to be valid for the lifetime of `self`. And it is safe to
+        // call `cpumask_setall()`.
+        unsafe { bindings::cpumask_setall(self.ptr) };
+    }
+
+    /// Copies cpumask.
+    pub fn copy(&self, dstp: &mut Self) {
+        // SAFETY: `ptr` is guaranteed to be valid for the lifetime of `self`. And it is safe to
+        // call `cpumask_copy()`.
+        unsafe { bindings::cpumask_copy(dstp.as_raw(), self.ptr) };
+    }
+}
+
+impl Drop for Cpumask {
+    fn drop(&mut self) {
+        if self.owned {
+            // SAFETY: `ptr` is guaranteed to be valid for the lifetime of `self`. And it is safe
+            // to call `free_cpumask_var()`.
+            unsafe { bindings::free_cpumask_var(self.ptr) }
+        }
+    }
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 584d95c1eb5d..8a0cd60eb6cc 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -38,6 +38,7 @@
 #[cfg(CONFIG_BLOCK)]
 pub mod block;
 mod build_assert;
+pub mod cpumask;
 pub mod cred;
 pub mod device;
 pub mod device_id;
-- 
2.31.1.272.g89b43f80a514

Re: [PATCH V6 05/15] rust: Add bindings for cpumask

[Greg KH]

On Tue, Jan 07, 2025 at 04:51:38PM +0530, Viresh Kumar wrote:
> Add basic Rust bindings for `struct cpumask`. Also add few Rust helpers
> for the same.

Shouldn't this be 2 patches?  You know that when you say "also" in a
changelog that's a huge hint for a reviewer to tell you to split it up.
You need to describe _why_ you can't split it up for us to not tell you
to split it up...

greg k-h

[PATCH V6 06/15] rust: Add bare minimal bindings for clk framework

[Viresh Kumar]

This adds very basic bindings for the clk framework, implements only
clk_get() and clk_put(). These are the bare minimum bindings required
for many users and are simple enough to add in the first attempt.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 rust/bindings/bindings_helper.h |  1 +
 rust/kernel/clk.rs              | 48 +++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |  1 +
 3 files changed, 50 insertions(+)
 create mode 100644 rust/kernel/clk.rs

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 3225379abd2b..7a95d999ead8 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -15,6 +15,7 @@
 #include <linux/blk-mq.h>
 #include <linux/blk_types.h>
 #include <linux/blkdev.h>
+#include <linux/clk.h>
 #include <linux/cpumask.h>
 #include <linux/cred.h>
 #include <linux/errname.h>
diff --git a/rust/kernel/clk.rs b/rust/kernel/clk.rs
new file mode 100644
index 000000000000..123cdb43b115
--- /dev/null
+++ b/rust/kernel/clk.rs
@@ -0,0 +1,48 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Clock abstractions.
+//!
+//! C header: [`include/linux/clk.h`](srctree/include/linux/clk.h)
+
+use crate::{
+    bindings,
+    device::Device,
+    error::{from_err_ptr, Result},
+    prelude::*,
+};
+
+use core::ptr;
+
+/// A simple implementation of `struct clk` from the C code.
+#[repr(transparent)]
+pub struct Clk(*mut bindings::clk);
+
+impl Clk {
+    /// Creates `Clk` instance for a device and a connection id.
+    pub fn new(dev: &Device, name: Option<&CStr>) -> Result<Self> {
+        let con_id = if let Some(name) = name {
+            name.as_ptr() as *const _
+        } else {
+            ptr::null()
+        };
+
+        // SAFETY: It is safe to call `clk_get()`, on a device pointer earlier received from the C
+        // code.
+        Ok(Self(from_err_ptr(unsafe {
+            bindings::clk_get(dev.as_raw(), con_id)
+        })?))
+    }
+
+    /// Obtain the raw `struct clk *`.
+    pub fn as_raw(&self) -> *mut bindings::clk {
+        self.0
+    }
+}
+
+impl Drop for Clk {
+    fn drop(&mut self) {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // relinquish it now.
+        unsafe { bindings::clk_put(self.0) };
+    }
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 8a0cd60eb6cc..70694becc9ff 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -38,6 +38,7 @@
 #[cfg(CONFIG_BLOCK)]
 pub mod block;
 mod build_assert;
+pub mod clk;
 pub mod cpumask;
 pub mod cred;
 pub mod device;
-- 
2.31.1.272.g89b43f80a514

[PATCH V6 07/15] rust: Add initial bindings for OPP framework

[Viresh Kumar]

This commit adds initial Rust bindings for the Operating performance
points (OPP) core. This adds bindings for `struct dev_pm_opp` and
`struct dev_pm_opp_data` to begin with.

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 MAINTAINERS                     |   1 +
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/lib.rs              |   2 +
 rust/kernel/opp.rs              | 189 ++++++++++++++++++++++++++++++++
 4 files changed, 193 insertions(+)
 create mode 100644 rust/kernel/opp.rs

diff --git a/MAINTAINERS b/MAINTAINERS
index 15c8df5b3590..bfcdbe4aa119 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17579,6 +17579,7 @@ F:	Documentation/devicetree/bindings/opp/
 F:	Documentation/power/opp.rst
 F:	drivers/opp/
 F:	include/linux/pm_opp.h
+F:	rust/kernel/opp.rs
 
 OPL4 DRIVER
 M:	Clemens Ladisch <clemens@ladisch.de>
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 7a95d999ead8..5f900532e5ec 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -32,6 +32,7 @@
 #include <linux/phy.h>
 #include <linux/pid_namespace.h>
 #include <linux/platform_device.h>
+#include <linux/pm_opp.h>
 #include <linux/poll.h>
 #include <linux/property.h>
 #include <linux/refcount.h>
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 70694becc9ff..16e534024323 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -61,6 +61,8 @@
 #[cfg(CONFIG_NET)]
 pub mod net;
 pub mod of;
+#[cfg(CONFIG_PM_OPP)]
+pub mod opp;
 pub mod page;
 pub mod pid_namespace;
 pub mod platform;
diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
new file mode 100644
index 000000000000..becb33880c92
--- /dev/null
+++ b/rust/kernel/opp.rs
@@ -0,0 +1,189 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Operating performance points.
+//!
+//! This module provides bindings for interacting with the OPP subsystem.
+//!
+//! C header: [`include/linux/pm_opp.h`](srctree/include/linux/pm_opp.h)
+
+use crate::{
+    bindings,
+    device::Device,
+    error::{code::*, to_result, Result},
+    types::{ARef, AlwaysRefCounted, Opaque},
+};
+
+use core::ptr;
+
+/// Dynamically created Operating performance point (OPP).
+pub struct Token {
+    dev: ARef<Device>,
+    freq: usize,
+}
+
+impl Token {
+    /// Adds an OPP dynamically.
+    pub fn new(dev: &ARef<Device>, mut data: Data) -> Result<Self> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        to_result(unsafe { bindings::dev_pm_opp_add_dynamic(dev.as_raw(), &mut data.0) })?;
+        Ok(Self {
+            dev: dev.clone(),
+            freq: data.freq(),
+        })
+    }
+}
+
+impl Drop for Token {
+    fn drop(&mut self) {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        unsafe { bindings::dev_pm_opp_remove(self.dev.as_raw(), self.freq) };
+    }
+}
+
+/// Equivalent to `struct dev_pm_opp_data` in the C Code.
+#[repr(transparent)]
+pub struct Data(bindings::dev_pm_opp_data);
+
+impl Data {
+    /// Creates new instance of [`Data`].
+    pub fn new(freq: usize, u_volt: usize, level: u32, turbo: bool) -> Self {
+        Self(bindings::dev_pm_opp_data {
+            turbo,
+            freq,
+            u_volt,
+            level,
+        })
+    }
+
+    /// Adds an OPP dynamically. The OPP is freed once the [`Token`] gets freed.
+    pub fn add_opp(self, dev: &ARef<Device>) -> Result<Token> {
+        Token::new(dev, self)
+    }
+
+    fn freq(&self) -> usize {
+        self.0.freq
+    }
+}
+
+/// Operating performance point (OPP).
+///
+/// Wraps the kernel's `struct dev_pm_opp`.
+///
+/// The pointer to `struct dev_pm_opp` is non-null and valid for the lifetime of the `OPP`
+/// instance.
+///
+/// # Invariants
+///
+/// Instances of this type are reference-counted. The reference count is incremented by the
+/// `dev_pm_opp_get()` function and decremented by `dev_pm_opp_put`. The Rust type `ARef<OPP>`
+/// 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>);
+
+// SAFETY: It's OK to send the ownership of `OPP` across thread boundaries.
+unsafe impl Send for OPP {}
+
+// SAFETY: It's OK to access `OPP` through shared references from other threads because we're
+// either accessing properties that don't change or that are properly synchronised by C code.
+unsafe impl Sync for OPP {}
+
+// SAFETY: The type invariants guarantee that [`OPP`] is always refcounted.
+unsafe impl AlwaysRefCounted for OPP {
+    fn inc_ref(&self) {
+        // SAFETY: The existence of a shared reference means that the refcount is nonzero.
+        unsafe { bindings::dev_pm_opp_get(self.0.get()) };
+    }
+
+    unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
+        // SAFETY: The safety requirements guarantee that the refcount is nonzero.
+        unsafe { bindings::dev_pm_opp_put(obj.cast().as_ptr()) }
+    }
+}
+
+impl OPP {
+    /// Creates an owned reference to a [`OPP`] from a valid pointer.
+    ///
+    /// The refcount is incremented by the C code and will be decremented by `dec_ref()` when the
+    /// ARef object is dropped.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that `ptr` is valid and the OPP's refcount is incremented. The
+    /// caller must also ensure that it doesn't explicitly drop the refcount of the OPP, as the
+    /// returned ARef object takes over the refcount increment on the underlying object and the
+    /// same will be dropped along with it.
+    pub unsafe fn from_raw_opp_owned(ptr: *mut bindings::dev_pm_opp) -> Result<ARef<Self>> {
+        let ptr = ptr::NonNull::new(ptr).ok_or(ENODEV)?;
+
+        // SAFETY: The safety requirements guarantee the validity of the pointer.
+        Ok(unsafe { ARef::from_raw(ptr.cast()) })
+    }
+
+    /// Creates a reference to a [`OPP`] from a valid pointer.
+    ///
+    /// The refcount is not updated by the Rust API unless the returned reference is converted to
+    /// an ARef object.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that `ptr` is valid and remains valid for the duration of 'a.
+    pub unsafe fn from_raw_opp<'a>(ptr: *mut bindings::dev_pm_opp) -> Result<&'a Self> {
+        // SAFETY: The caller guarantees that the pointer is not dangling and stays valid for the
+        // duration of 'a. The cast is okay because `OPP` is `repr(transparent)`.
+        Ok(unsafe { &*ptr.cast() })
+    }
+
+    #[inline]
+    fn as_raw(&self) -> *mut bindings::dev_pm_opp {
+        self.0.get()
+    }
+
+    /// Returns the frequency of an OPP.
+    pub fn freq(&self, index: Option<u32>) -> usize {
+        let index = index.unwrap_or(0);
+
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // use it.
+        unsafe { bindings::dev_pm_opp_get_freq_indexed(self.as_raw(), index) }
+    }
+
+    /// Returns the voltage of an OPP.
+    pub fn voltage(&self) -> usize {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // use it.
+        unsafe { bindings::dev_pm_opp_get_voltage(self.as_raw()) }
+    }
+
+    /// Returns the level of an OPP.
+    pub fn level(&self) -> u32 {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // use it.
+        unsafe { bindings::dev_pm_opp_get_level(self.as_raw()) }
+    }
+
+    /// Returns the power of an OPP.
+    pub fn power(&self) -> usize {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // use it.
+        unsafe { bindings::dev_pm_opp_get_power(self.as_raw()) }
+    }
+
+    /// Returns the required pstate of an OPP.
+    pub fn required_pstate(&self, index: u32) -> u32 {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // use it.
+        unsafe { bindings::dev_pm_opp_get_required_pstate(self.as_raw(), index) }
+    }
+
+    /// Returns true if the OPP is turbo.
+    pub fn is_turbo(&self) -> bool {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // use it.
+        unsafe { bindings::dev_pm_opp_is_turbo(self.as_raw()) }
+    }
+}
-- 
2.31.1.272.g89b43f80a514

[PATCH V6 08/15] rust: Extend OPP bindings for the OPP table

[Viresh Kumar]

This extends OPP bindings with the bindings for the `struct opp_table`.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 rust/kernel/opp.rs | 382 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 381 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
index becb33880c92..b1e277c660d4 100644
--- a/rust/kernel/opp.rs
+++ b/rust/kernel/opp.rs
@@ -8,8 +8,9 @@
 
 use crate::{
     bindings,
+    cpumask::Cpumask,
     device::Device,
-    error::{code::*, to_result, Result},
+    error::{code::*, from_err_ptr, to_result, Error, Result},
     types::{ARef, AlwaysRefCounted, Opaque},
 };
 
@@ -67,6 +68,385 @@ fn freq(&self) -> usize {
     }
 }
 
+/// OPP search types.
+#[derive(Copy, Clone, Debug, Eq, PartialEq)]
+pub enum SearchType {
+    /// Search for exact value.
+    Exact,
+    /// Search for highest value less than equal to value.
+    Floor,
+    /// Search for lowest value greater than equal to value.
+    Ceil,
+}
+
+/// Operating performance point (OPP) table.
+///
+/// Wraps the kernel's `struct opp_table`.
+///
+/// The pointer stored in `Self` is non-null and valid for the lifetime of the `Table`.
+pub struct Table {
+    ptr: *mut bindings::opp_table,
+    dev: ARef<Device>,
+    em: bool,
+    of: bool,
+    cpumask: Option<Cpumask>,
+}
+
+// SAFETY: It is okay to send ownership of `Table` across thread boundaries.
+unsafe impl Send for Table {}
+
+// SAFETY: It's OK to access `Table` through shared references from other threads because we're
+// either accessing properties that don't change or that are properly synchronised by C code.
+unsafe impl Sync for Table {}
+
+impl Table {
+    /// Creates a new OPP table instance from raw pointer.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that `ptr` is valid and non-null.
+    unsafe fn from_raw_table(ptr: *mut bindings::opp_table, dev: &ARef<Device>) -> Self {
+        // SAFETY: By the safety requirements, ptr is valid and its refcount will be incremented.
+        unsafe { bindings::dev_pm_opp_get_opp_table_ref(ptr) };
+
+        Self {
+            ptr,
+            dev: dev.clone(),
+            em: false,
+            of: false,
+            cpumask: None,
+        }
+    }
+
+    /// Find OPP table from device.
+    pub fn from_dev(dev: &ARef<Device>) -> Result<Self> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements. Refcount of the OPP table is incremented by the C code.
+        let ptr = from_err_ptr(unsafe { bindings::dev_pm_opp_get_opp_table(dev.as_raw()) })?;
+
+        Ok(Self {
+            ptr,
+            dev: dev.clone(),
+            em: false,
+            of: false,
+            cpumask: None,
+        })
+    }
+
+    /// Add device tree based OPP table for the device.
+    #[cfg(CONFIG_OF)]
+    pub fn from_of(dev: &ARef<Device>, index: i32) -> Result<Self> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements. Refcount of the OPP table is incremented by the C code.
+        to_result(unsafe { bindings::dev_pm_opp_of_add_table_indexed(dev.as_raw(), index) })?;
+
+        // Fetch the newly created table.
+        let mut table = Self::from_dev(dev)?;
+        table.of = true;
+
+        Ok(table)
+    }
+
+    // Remove device tree based OPP table for the device.
+    #[cfg(CONFIG_OF)]
+    fn remove_of(&self) {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements. We took the reference earlier from `from_of` earlier, it is safe to drop
+        // the same now.
+        unsafe { bindings::dev_pm_opp_of_remove_table(self.dev.as_raw()) };
+    }
+
+    /// Add device tree based OPP table for CPU devices.
+    #[cfg(CONFIG_OF)]
+    pub fn from_of_cpumask(dev: &ARef<Device>, cpumask: &mut Cpumask) -> Result<Self> {
+        // SAFETY: The cpumask is valid and the returned ptr will be owned by the [`Table`] instance.
+        to_result(unsafe { bindings::dev_pm_opp_of_cpumask_add_table(cpumask.as_raw()) })?;
+
+        // Fetch the newly created table.
+        let mut table = Self::from_dev(dev)?;
+
+        let mut mask = Cpumask::new()?;
+        cpumask.copy(&mut mask);
+        table.cpumask = Some(mask);
+
+        Ok(table)
+    }
+
+    // Remove device tree based OPP table for CPU devices.
+    #[cfg(CONFIG_OF)]
+    fn remove_of_cpumask(&self, mut cpumask: Cpumask) {
+        // SAFETY: The cpumask is valid and we took the reference from `from_of_cpumask` earlier,
+        // it is safe to drop the same now.
+        unsafe { bindings::dev_pm_opp_of_cpumask_remove_table(cpumask.as_raw()) };
+    }
+
+    /// Returns the number of OPPs in the table.
+    pub fn opp_count(&self) -> Result<u32> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        let ret = unsafe { bindings::dev_pm_opp_get_opp_count(self.dev.as_raw()) };
+        if ret < 0 {
+            Err(Error::from_errno(ret))
+        } else {
+            Ok(ret as u32)
+        }
+    }
+
+    /// Returns max clock latency of the OPPs in the table.
+    pub fn max_clock_latency(&self) -> usize {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        unsafe { bindings::dev_pm_opp_get_max_clock_latency(self.dev.as_raw()) }
+    }
+
+    /// Returns max volt latency of the OPPs in the table.
+    pub fn max_volt_latency(&self) -> usize {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        unsafe { bindings::dev_pm_opp_get_max_volt_latency(self.dev.as_raw()) }
+    }
+
+    /// Returns max transition latency of the OPPs in the table.
+    pub fn max_transition_latency(&self) -> usize {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        unsafe { bindings::dev_pm_opp_get_max_transition_latency(self.dev.as_raw()) }
+    }
+
+    /// Returns the suspend OPP.
+    pub fn suspend_freq(&self) -> usize {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        unsafe { bindings::dev_pm_opp_get_suspend_opp_freq(self.dev.as_raw()) }
+    }
+
+    /// Synchronizes regulators used by the OPP table.
+    pub fn sync_regulators(&self) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        to_result(unsafe { bindings::dev_pm_opp_sync_regulators(self.dev.as_raw()) })
+    }
+
+    /// Gets sharing CPUs.
+    pub fn sharing_cpus(dev: &Device, cpumask: &mut Cpumask) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        to_result(unsafe {
+            bindings::dev_pm_opp_get_sharing_cpus(dev.as_raw(), cpumask.as_raw())
+        })
+    }
+
+    /// Sets sharing CPUs.
+    pub fn set_sharing_cpus(&mut self, cpumask: &mut Cpumask) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        to_result(unsafe {
+            bindings::dev_pm_opp_set_sharing_cpus(self.dev.as_raw(), cpumask.as_raw())
+        })?;
+
+        if let Some(mask) = self.cpumask.as_mut() {
+            // Update the cpumask as this will be used while removing the table.
+            cpumask.copy(mask);
+        }
+
+        Ok(())
+    }
+
+    /// Gets sharing CPUs from Device tree.
+    #[cfg(CONFIG_OF)]
+    pub fn of_sharing_cpus(dev: &Device, cpumask: &mut Cpumask) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        to_result(unsafe {
+            bindings::dev_pm_opp_of_get_sharing_cpus(dev.as_raw(), cpumask.as_raw())
+        })
+    }
+
+    /// Updates the voltage value for an OPP.
+    pub fn adjust_voltage(
+        &self,
+        freq: usize,
+        u_volt: usize,
+        u_volt_min: usize,
+        u_volt_max: usize,
+    ) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        to_result(unsafe {
+            bindings::dev_pm_opp_adjust_voltage(
+                self.dev.as_raw(),
+                freq,
+                u_volt,
+                u_volt_min,
+                u_volt_max,
+            )
+        })
+    }
+
+    /// Sets a matching OPP based on frequency.
+    pub fn set_rate(&self, freq: usize) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        to_result(unsafe { bindings::dev_pm_opp_set_rate(self.dev.as_raw(), freq) })
+    }
+
+    /// Sets exact OPP.
+    pub fn set_opp(&self, opp: &OPP) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        to_result(unsafe { bindings::dev_pm_opp_set_opp(self.dev.as_raw(), opp.as_raw()) })
+    }
+
+    /// Finds OPP based on frequency.
+    pub fn opp_from_freq(
+        &self,
+        mut freq: usize,
+        available: Option<bool>,
+        index: Option<u32>,
+        stype: SearchType,
+    ) -> Result<ARef<OPP>> {
+        let rdev = self.dev.as_raw();
+        let index = index.unwrap_or(0);
+
+        let ptr = from_err_ptr(match stype {
+            SearchType::Exact => {
+                if let Some(available) = available {
+                    // SAFETY: The requirements are satisfied by the existence of `Device` and
+                    // its safety requirements. The returned ptr will be owned by the new [`OPP`]
+                    // instance.
+                    unsafe {
+                        bindings::dev_pm_opp_find_freq_exact_indexed(rdev, freq, index, available)
+                    }
+                } else {
+                    return Err(EINVAL);
+                }
+            }
+
+            // SAFETY: The requirements are satisfied by the existence of `Device` and its
+            // safety requirements. The returned ptr will be owned by the new [`OPP`] instance.
+            SearchType::Ceil => unsafe {
+                bindings::dev_pm_opp_find_freq_ceil_indexed(rdev, &mut freq as *mut usize, index)
+            },
+
+            // SAFETY: The requirements are satisfied by the existence of `Device` and its
+            // safety requirements. The returned ptr will be owned by the new [`OPP`] instance.
+            SearchType::Floor => unsafe {
+                bindings::dev_pm_opp_find_freq_floor_indexed(rdev, &mut freq as *mut usize, index)
+            },
+        })?;
+
+        // SAFETY: The `ptr` is guaranteed by the C code to be valid.
+        unsafe { OPP::from_raw_opp_owned(ptr) }
+    }
+
+    /// Finds OPP based on level.
+    pub fn opp_from_level(&self, mut level: u32, stype: SearchType) -> Result<ARef<OPP>> {
+        let rdev = self.dev.as_raw();
+
+        let ptr = from_err_ptr(match stype {
+            // SAFETY: The requirements are satisfied by the existence of `Device` and its
+            // safety requirements. The returned ptr will be owned by the new [`OPP`] instance.
+            SearchType::Exact => unsafe { bindings::dev_pm_opp_find_level_exact(rdev, level) },
+
+            // SAFETY: The requirements are satisfied by the existence of `Device` and its
+            // safety requirements. The returned ptr will be owned by the new [`OPP`] instance.
+            SearchType::Ceil => unsafe {
+                bindings::dev_pm_opp_find_level_ceil(rdev, &mut level as *mut u32)
+            },
+
+            // SAFETY: The requirements are satisfied by the existence of `Device` and its
+            // safety requirements. The returned ptr will be owned by the new [`OPP`] instance.
+            SearchType::Floor => unsafe {
+                bindings::dev_pm_opp_find_level_floor(rdev, &mut level as *mut u32)
+            },
+        })?;
+
+        // SAFETY: The `ptr` is guaranteed by the C code to be valid.
+        unsafe { OPP::from_raw_opp_owned(ptr) }
+    }
+
+    /// Finds OPP based on bandwidth.
+    pub fn opp_from_bw(&self, mut bw: u32, index: i32, stype: SearchType) -> Result<ARef<OPP>> {
+        let rdev = self.dev.as_raw();
+
+        let ptr = from_err_ptr(match stype {
+            // The OPP core doesn't support this yet.
+            SearchType::Exact => return Err(EINVAL),
+
+            // SAFETY: The requirements are satisfied by the existence of `Device` and its
+            // safety requirements. The returned ptr will be owned by the new [`OPP`] instance.
+            SearchType::Ceil => unsafe {
+                bindings::dev_pm_opp_find_bw_ceil(rdev, &mut bw as *mut u32, index)
+            },
+
+            // SAFETY: The requirements are satisfied by the existence of `Device` and its
+            // safety requirements. The returned ptr will be owned by the new [`OPP`] instance.
+            SearchType::Floor => unsafe {
+                bindings::dev_pm_opp_find_bw_floor(rdev, &mut bw as *mut u32, index)
+            },
+        })?;
+
+        // SAFETY: The `ptr` is guaranteed by the C code to be valid.
+        unsafe { OPP::from_raw_opp_owned(ptr) }
+    }
+
+    /// Enable the OPP.
+    pub fn enable_opp(&self, freq: usize) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        to_result(unsafe { bindings::dev_pm_opp_enable(self.dev.as_raw(), freq) })
+    }
+
+    /// Disable the OPP.
+    pub fn disable_opp(&self, freq: usize) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        to_result(unsafe { bindings::dev_pm_opp_disable(self.dev.as_raw(), freq) })
+    }
+
+    /// Registers with Energy model.
+    #[cfg(CONFIG_OF)]
+    pub fn of_register_em(&mut self, cpumask: &mut Cpumask) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        to_result(unsafe {
+            bindings::dev_pm_opp_of_register_em(self.dev.as_raw(), cpumask.as_raw())
+        })?;
+
+        self.em = true;
+        Ok(())
+    }
+
+    // Unregisters with Energy model.
+    #[cfg(CONFIG_OF)]
+    fn of_unregister_em(&self) {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements. We registered with the EM framework earlier, it is safe to unregister now.
+        unsafe { bindings::em_dev_unregister_perf_domain(self.dev.as_raw()) };
+    }
+}
+
+impl Drop for Table {
+    fn drop(&mut self) {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe
+        // to relinquish it now.
+        unsafe { bindings::dev_pm_opp_put_opp_table(self.ptr) };
+
+        #[cfg(CONFIG_OF)]
+        {
+            if self.em {
+                self.of_unregister_em();
+            }
+
+            if self.of {
+                self.remove_of();
+            } else if let Some(cpumask) = self.cpumask.take() {
+                self.remove_of_cpumask(cpumask);
+            }
+        }
+    }
+}
+
 /// Operating performance point (OPP).
 ///
 /// Wraps the kernel's `struct dev_pm_opp`.
-- 
2.31.1.272.g89b43f80a514

[PATCH V6 09/15] rust: Extend OPP bindings for the configuration options

[Viresh Kumar]

This extends OPP bindings with the bindings for the OPP core
configuration options.

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 rust/kernel/opp.rs | 262 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 260 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
index b1e277c660d4..d92e88af0cec 100644
--- a/rust/kernel/opp.rs
+++ b/rust/kernel/opp.rs
@@ -10,11 +10,28 @@
     bindings,
     cpumask::Cpumask,
     device::Device,
-    error::{code::*, from_err_ptr, to_result, Error, Result},
+    error::{code::*, from_err_ptr, from_result, to_result, Error, Result, VTABLE_DEFAULT_ERROR},
+    prelude::*,
+    str::CString,
     types::{ARef, AlwaysRefCounted, Opaque},
 };
 
-use core::ptr;
+use core::{marker::PhantomData, ptr};
+
+use macros::vtable;
+
+// Creates a null-terminated slice of pointers to Cstrings.
+fn to_c_str_array(names: &[CString]) -> Result<KVec<*const u8>> {
+    // Allocated a null-terminated vector of pointers.
+    let mut list = KVec::with_capacity(names.len() + 1, GFP_KERNEL)?;
+
+    for name in names.iter() {
+        list.push(name.as_ptr() as _, GFP_KERNEL)?;
+    }
+
+    list.push(ptr::null(), GFP_KERNEL)?;
+    Ok(list)
+}
 
 /// Dynamically created Operating performance point (OPP).
 pub struct Token {
@@ -79,6 +96,247 @@ pub enum SearchType {
     Ceil,
 }
 
+/// Implement this trait to provide OPP Configuration callbacks.
+#[vtable]
+pub trait ConfigOps {
+    /// Called by the OPP core to configure OPP clks.
+    fn config_clks(_dev: &Device, _table: &Table, _opp: &OPP, _scaling_down: bool) -> Result<()> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Called by the OPP core to configure OPP regulators.
+    fn config_regulators(
+        _dev: &Device,
+        _opp_old: &OPP,
+        _opp_new: &OPP,
+        _data: *mut *mut bindings::regulator,
+        _count: u32,
+    ) -> Result<()> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+}
+
+/// Config token returned by the C code.
+pub struct ConfigToken(i32);
+
+impl Drop for ConfigToken {
+    fn drop(&mut self) {
+        // SAFETY: Its safe to return the configuration token number previously received from the C
+        // code.
+        unsafe { bindings::dev_pm_opp_clear_config(self.0) };
+    }
+}
+
+/// Equivalent to `struct dev_pm_opp_config` in the C Code.
+#[derive(Default)]
+pub struct Config<T: ConfigOps> {
+    clk_names: Option<KVec<CString>>,
+    prop_name: Option<CString>,
+    regulator_names: Option<KVec<CString>>,
+    supported_hw: Option<KVec<u32>>,
+    required_dev: Option<ARef<Device>>,
+    required_dev_index: Option<u32>,
+    _data: PhantomData<T>,
+}
+
+impl<T: ConfigOps> Config<T> {
+    /// Creates a new instance of [`Config`].
+    pub fn new() -> Self {
+        Self {
+            clk_names: None,
+            prop_name: None,
+            regulator_names: None,
+            supported_hw: None,
+            required_dev: None,
+            required_dev_index: None,
+            _data: PhantomData,
+        }
+    }
+
+    /// Initializes clock names.
+    pub fn set_clk_names(mut self, names: KVec<CString>) -> Result<Self> {
+        // Already configured.
+        if self.clk_names.is_some() {
+            return Err(EBUSY);
+        }
+
+        if names.is_empty() {
+            return Err(EINVAL);
+        }
+
+        self.clk_names = Some(names);
+        Ok(self)
+    }
+
+    /// Initializes property name.
+    pub fn set_prop_name(mut self, name: CString) -> Result<Self> {
+        // Already configured.
+        if self.prop_name.is_some() {
+            return Err(EBUSY);
+        }
+
+        self.prop_name = Some(name);
+        Ok(self)
+    }
+
+    /// Initializes regulator names.
+    pub fn set_regulator_names(mut self, names: KVec<CString>) -> Result<Self> {
+        // Already configured.
+        if self.regulator_names.is_some() {
+            return Err(EBUSY);
+        }
+
+        if names.is_empty() {
+            return Err(EINVAL);
+        }
+
+        self.regulator_names = Some(names);
+
+        Ok(self)
+    }
+
+    /// Initializes required devices.
+    pub fn set_required_dev(mut self, dev: ARef<Device>, index: u32) -> Result<Self> {
+        // Already configured.
+        if self.required_dev.is_some() {
+            return Err(EBUSY);
+        }
+
+        self.required_dev = Some(dev);
+        self.required_dev_index = Some(index);
+        Ok(self)
+    }
+
+    /// Initializes supported hardware.
+    pub fn set_supported_hw(mut self, hw: KVec<u32>) -> Result<Self> {
+        // Already configured.
+        if self.supported_hw.is_some() {
+            return Err(EBUSY);
+        }
+
+        if hw.is_empty() {
+            return Err(EINVAL);
+        }
+
+        self.supported_hw = Some(hw);
+        Ok(self)
+    }
+
+    /// Sets the configuration with the OPP core.
+    pub fn set(self, dev: &Device) -> Result<ConfigToken> {
+        let (_clk_list, clk_names) = match &self.clk_names {
+            Some(x) => {
+                let list = to_c_str_array(x)?;
+                let ptr = list.as_ptr();
+                (Some(list), ptr)
+            }
+            None => (None, ptr::null()),
+        };
+
+        let (_regulator_list, regulator_names) = match &self.regulator_names {
+            Some(x) => {
+                let list = to_c_str_array(x)?;
+                let ptr = list.as_ptr();
+                (Some(list), ptr)
+            }
+            None => (None, ptr::null()),
+        };
+
+        let prop_name = match &self.prop_name {
+            Some(x) => x.as_char_ptr(),
+            None => ptr::null(),
+        };
+
+        let (supported_hw, supported_hw_count) = match &self.supported_hw {
+            Some(x) => (x.as_ptr(), x.len() as u32),
+            None => (ptr::null(), 0),
+        };
+
+        let (required_dev, required_dev_index) = match &self.required_dev {
+            Some(x) => (x.as_raw(), self.required_dev_index.unwrap()),
+            None => (ptr::null_mut(), 0),
+        };
+
+        let mut config = bindings::dev_pm_opp_config {
+            clk_names,
+            config_clks: if T::HAS_CONFIG_CLKS {
+                Some(Self::config_clks)
+            } else {
+                None
+            },
+            prop_name,
+            regulator_names,
+            config_regulators: if T::HAS_CONFIG_REGULATORS {
+                Some(Self::config_regulators)
+            } else {
+                None
+            },
+            supported_hw,
+            supported_hw_count,
+
+            required_dev,
+            required_dev_index,
+        };
+
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements. The OPP core guarantees to not use fields of `config`, after this call has
+        // returned and so we don't need to save a copy of them for future use
+        let ret = unsafe { bindings::dev_pm_opp_set_config(dev.as_raw(), &mut config) };
+        if ret < 0 {
+            Err(Error::from_errno(ret))
+        } else {
+            Ok(ConfigToken(ret))
+        }
+    }
+
+    // Config's config_clks callback.
+    extern "C" fn config_clks(
+        dev: *mut bindings::device,
+        opp_table: *mut bindings::opp_table,
+        opp: *mut bindings::dev_pm_opp,
+        _data: *mut core::ffi::c_void,
+        scaling_down: bool,
+    ) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: 'dev' is guaranteed by the C code to be valid.
+            let dev = unsafe { Device::get_device(dev) };
+            T::config_clks(
+                &dev,
+                // SAFETY: 'opp_table' is guaranteed by the C code to be valid.
+                &unsafe { Table::from_raw_table(opp_table, &dev) },
+                // SAFETY: 'opp' is guaranteed by the C code to be valid.
+                unsafe { OPP::from_raw_opp(opp)? },
+                scaling_down,
+            )
+            .map(|()| 0)
+        })
+    }
+
+    // Config's config_regulators callback.
+    extern "C" fn config_regulators(
+        dev: *mut bindings::device,
+        old_opp: *mut bindings::dev_pm_opp,
+        new_opp: *mut bindings::dev_pm_opp,
+        regulators: *mut *mut bindings::regulator,
+        count: core::ffi::c_uint,
+    ) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: 'dev' is guaranteed by the C code to be valid.
+            let dev = unsafe { Device::get_device(dev) };
+            T::config_regulators(
+                &dev,
+                // SAFETY: 'old_opp' is guaranteed by the C code to be valid.
+                unsafe { OPP::from_raw_opp(old_opp)? },
+                // SAFETY: 'new_opp' is guaranteed by the C code to be valid.
+                unsafe { OPP::from_raw_opp(new_opp)? },
+                regulators,
+                count,
+            )
+            .map(|()| 0)
+        })
+    }
+}
+
 /// Operating performance point (OPP) table.
 ///
 /// Wraps the kernel's `struct opp_table`.
-- 
2.31.1.272.g89b43f80a514

[PATCH V6 10/15] rust: Add initial bindings for cpufreq framework

[Viresh Kumar]

This commit adds initial Rust bindings for the cpufreq core. This adds
basic bindings for cpufreq flags, relations and cpufreq table.

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 MAINTAINERS                     |   1 +
 rust/bindings/bindings_helper.h |   1 +
 rust/helpers/cpufreq.c          |  16 ++
 rust/helpers/helpers.c          |   1 +
 rust/kernel/cpufreq.rs          | 254 ++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   2 +
 6 files changed, 275 insertions(+)
 create mode 100644 rust/helpers/cpufreq.c
 create mode 100644 rust/kernel/cpufreq.rs

diff --git a/MAINTAINERS b/MAINTAINERS
index bfcdbe4aa119..35ab20b99f90 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5957,6 +5957,7 @@ F:	drivers/cpufreq/
 F:	include/linux/cpufreq.h
 F:	include/linux/sched/cpufreq.h
 F:	kernel/sched/cpufreq*.c
+F:	rust/kernel/cpufreq.rs
 F:	tools/testing/selftests/cpufreq/
 
 CPU HOTPLUG
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 5f900532e5ec..bd706ad1e58a 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -16,6 +16,7 @@
 #include <linux/blk_types.h>
 #include <linux/blkdev.h>
 #include <linux/clk.h>
+#include <linux/cpufreq.h>
 #include <linux/cpumask.h>
 #include <linux/cred.h>
 #include <linux/errname.h>
diff --git a/rust/helpers/cpufreq.c b/rust/helpers/cpufreq.c
new file mode 100644
index 000000000000..28f8ebf96032
--- /dev/null
+++ b/rust/helpers/cpufreq.c
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/cpufreq.h>
+
+#ifdef CONFIG_CPU_FREQ
+unsigned int
+rust_helper_cpufreq_table_len(struct cpufreq_frequency_table *freq_table)
+{
+	return cpufreq_table_len(freq_table);
+}
+
+void rust_helper_cpufreq_register_em_with_opp(struct cpufreq_policy *policy)
+{
+	cpufreq_register_em_with_opp(policy);
+}
+#endif
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 1943b98aec2b..f258e3b1a0bb 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -11,6 +11,7 @@
 #include "bug.c"
 #include "build_assert.c"
 #include "build_bug.c"
+#include "cpufreq.c"
 #include "cpumask.c"
 #include "cred.c"
 #include "device.c"
diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
new file mode 100644
index 000000000000..07ff03cb2991
--- /dev/null
+++ b/rust/kernel/cpufreq.rs
@@ -0,0 +1,254 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! CPU frequency scaling.
+//!
+//! This module provides bindings for interacting with the cpufreq subsystem.
+//!
+//! C header: [`include/linux/cpufreq.h`](srctree/include/linux/cpufreq.h)
+
+use crate::{
+    bindings,
+    error::{code::*, to_result, Result},
+    prelude::*,
+};
+
+use core::{
+    pin::Pin,
+};
+
+/// Default transition latency value.
+pub const ETERNAL_LATENCY: u32 = bindings::CPUFREQ_ETERNAL as u32;
+
+/// Container for cpufreq driver flags.
+pub mod flags {
+    use crate::bindings;
+
+    /// Set by drivers that need to update internal upper and lower boundaries along with the
+    /// target frequency and so the core and governors should also invoke the driver if the target
+    /// frequency does not change, but the policy min or max may have changed.
+    pub const NEED_UPDATE_LIMITS: u16 = bindings::CPUFREQ_NEED_UPDATE_LIMITS as _;
+
+    /// Set by drivers for platforms where loops_per_jiffy or other kernel "constants" aren't
+    /// affected by frequency transitions.
+    pub const CONST_LOOPS: u16 = bindings::CPUFREQ_CONST_LOOPS as _;
+
+    /// Set by drivers that want the core to automatically register the cpufreq driver as a thermal
+    /// cooling device.
+    pub const IS_COOLING_DEV: u16 = bindings::CPUFREQ_IS_COOLING_DEV as _;
+
+    /// Set by drivers for platforms that have multiple clock-domains, i.e. supporting multiple
+    /// policies. With this sysfs directories of governor would be created in cpu/cpuN/cpufreq/
+    /// directory and so they can use the same governor with different tunables for different
+    /// clusters.
+    pub const HAVE_GOVERNOR_PER_POLICY: u16 = bindings::CPUFREQ_HAVE_GOVERNOR_PER_POLICY as _;
+
+    /// Set by drivers which do POSTCHANGE notifications from outside of their ->target() routine.
+    pub const ASYNC_NOTIFICATION: u16 = bindings::CPUFREQ_ASYNC_NOTIFICATION as _;
+
+    /// Set by drivers that want cpufreq core to check if CPU is running at a frequency present in
+    /// freq-table exposed by the driver. For these drivers if CPU is found running at an out of
+    /// table freq, the cpufreq core will try to change the frequency to a value from the table.
+    /// And if that fails, it will stop further boot process by issuing a BUG_ON().
+    pub const NEED_INITIAL_FREQ_CHECK: u16 = bindings::CPUFREQ_NEED_INITIAL_FREQ_CHECK as _;
+
+    /// Set by drivers to disallow use of governors with "dynamic_switching" flag set.
+    pub const NO_AUTO_DYNAMIC_SWITCHING: u16 = bindings::CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING as _;
+}
+
+/// CPU frequency selection relations. Each value contains a `bool` argument which corresponds to
+/// the Relation being efficient.
+#[derive(Copy, Clone, Debug, Eq, PartialEq)]
+pub enum Relation {
+    /// Select the lowest frequency at or above target.
+    Low(bool),
+    /// Select the highest frequency below or at target.
+    High(bool),
+    /// Select the closest frequency to the target.
+    Close(bool),
+}
+
+impl Relation {
+    // Converts from a value compatible with the C code.
+    fn new(val: u32) -> Result<Self> {
+        let efficient = val & bindings::CPUFREQ_RELATION_E != 0;
+
+        Ok(match val & !bindings::CPUFREQ_RELATION_E {
+            bindings::CPUFREQ_RELATION_L => Self::Low(efficient),
+            bindings::CPUFREQ_RELATION_H => Self::High(efficient),
+            bindings::CPUFREQ_RELATION_C => Self::Close(efficient),
+            _ => return Err(EINVAL),
+        })
+    }
+
+    /// Converts to a value compatible with the C code.
+    pub fn val(&self) -> u32 {
+        let (mut val, e) = match self {
+            Self::Low(e) => (bindings::CPUFREQ_RELATION_L, e),
+            Self::High(e) => (bindings::CPUFREQ_RELATION_H, e),
+            Self::Close(e) => (bindings::CPUFREQ_RELATION_C, e),
+        };
+
+        if *e {
+            val |= bindings::CPUFREQ_RELATION_E;
+        }
+
+        val
+    }
+}
+
+/// Equivalent to `struct cpufreq_policy_data` in the C code.
+#[repr(transparent)]
+pub struct PolicyData(*mut bindings::cpufreq_policy_data);
+
+impl PolicyData {
+    /// Creates new instance of [`PolicyData`].
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that `ptr` is valid and non-null.
+    pub unsafe fn from_raw_policy_data(ptr: *mut bindings::cpufreq_policy_data) -> Self {
+        Self(ptr)
+    }
+
+    /// Returns the raw pointer to the C structure.
+    #[inline]
+    pub fn as_raw(&self) -> *mut bindings::cpufreq_policy_data {
+        self.0
+    }
+
+    /// Provides a wrapper to the generic verify routine.
+    pub fn generic_verify(&self) -> Result<()> {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // use it now.
+        to_result(unsafe { bindings::cpufreq_generic_frequency_table_verify(self.as_raw()) })
+    }
+}
+
+/// Builder for the `struct cpufreq_frequency_table` in the C code.
+#[repr(transparent)]
+#[derive(Default)]
+pub struct TableBuilder {
+    entries: KVec<bindings::cpufreq_frequency_table>,
+}
+
+impl TableBuilder {
+    /// Creates new instance of [`TableBuilder`].
+    pub fn new() -> Self {
+        Self {
+            entries: KVec::new(),
+        }
+    }
+
+    /// Adds a new entry to the table.
+    pub fn add(&mut self, frequency: u32, flags: u32, driver_data: u32) -> Result<()> {
+        // Adds new entry to the end of the vector.
+        Ok(self.entries.push(
+            bindings::cpufreq_frequency_table {
+                flags,
+                driver_data,
+                frequency,
+            },
+            GFP_KERNEL,
+        )?)
+    }
+
+    /// Creates [`Table`] from [`TableBuilder`].
+    pub fn into_table(mut self) -> Result<Table> {
+        // Add last entry to the table.
+        self.add(bindings::CPUFREQ_TABLE_END as u32, 0, 0)?;
+        Table::from_builder(self.entries)
+    }
+}
+
+/// A simple implementation of the cpufreq table, equivalent to the `struct
+/// cpufreq_frequency_table` in the C code.
+pub struct Table {
+    #[allow(dead_code)]
+    // Dynamically created table.
+    entries: Option<Pin<KVec<bindings::cpufreq_frequency_table>>>,
+
+    // Pointer to the statically or dynamically created table.
+    ptr: *mut bindings::cpufreq_frequency_table,
+
+    // Number of entries in the table.
+    len: usize,
+}
+
+impl Table {
+    /// Creates new instance of [`Table`] from [`TableBuilder`].
+    fn from_builder(entries: KVec<bindings::cpufreq_frequency_table>) -> Result<Self> {
+        let len = entries.len();
+        if len == 0 {
+            return Err(EINVAL);
+        }
+
+        // Pin the entries to memory, since we are passing its pointer to the C code.
+        let mut entries = Pin::new(entries);
+
+        // The pointer is valid until the table gets dropped.
+        let ptr = entries.as_mut_ptr();
+
+        Ok(Self {
+            entries: Some(entries),
+            ptr,
+            // The last entry in table is reserved for `CPUFREQ_TABLE_END`.
+            len: len - 1,
+        })
+    }
+
+    /// Creates new instance of [`Table`] from raw pointer.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that `ptr` is valid and non-null for the lifetime of the [`Table`].
+    pub unsafe fn from_raw(ptr: *mut bindings::cpufreq_frequency_table) -> Self {
+        Self {
+            entries: None,
+            ptr,
+            // SAFETY: The pointer is guaranteed to be valid for the lifetime of `Self`.
+            len: unsafe { bindings::cpufreq_table_len(ptr) } as usize,
+        }
+    }
+
+    // Validate the index.
+    fn validate(&self, index: usize) -> Result<()> {
+        if index >= self.len {
+            Err(EINVAL)
+        } else {
+            Ok(())
+        }
+    }
+
+    /// Returns raw pointer to the `struct cpufreq_frequency_table` compatible with the C code.
+    #[inline]
+    pub fn as_raw(&self) -> *mut bindings::cpufreq_frequency_table {
+        self.ptr
+    }
+
+    /// Returns `frequency` at index in the [`Table`].
+    pub fn freq(&self, index: usize) -> Result<u32> {
+        self.validate(index)?;
+
+        // SAFETY: The pointer is guaranteed to be valid for the lifetime of `self` and `index` is
+        // also validated before this and is guaranteed to be within limits of the frequency table.
+        Ok(unsafe { (*self.ptr.add(index)).frequency })
+    }
+
+    /// Returns `flags` at index in the [`Table`].
+    pub fn flags(&self, index: usize) -> Result<u32> {
+        self.validate(index)?;
+
+        // SAFETY: The pointer is guaranteed to be valid for the lifetime of `self` and `index` is
+        // also validated before this and is guaranteed to be within limits of the frequency table.
+        Ok(unsafe { (*self.ptr.add(index)).flags })
+    }
+
+    /// Returns `data` at index in the [`Table`].
+    pub fn data(&self, index: usize) -> Result<u32> {
+        self.validate(index)?;
+
+        // SAFETY: The pointer is guaranteed to be valid for the lifetime of `self` and `index` is
+        // also validated before this and is guaranteed to be within limits of the frequency table.
+        Ok(unsafe { (*self.ptr.add(index)).driver_data })
+    }
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 16e534024323..1c5a20679c7e 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -39,6 +39,8 @@
 pub mod block;
 mod build_assert;
 pub mod clk;
+#[cfg(CONFIG_CPU_FREQ)]
+pub mod cpufreq;
 pub mod cpumask;
 pub mod cred;
 pub mod device;
-- 
2.31.1.272.g89b43f80a514

[PATCH V6 11/15] rust: Extend cpufreq bindings for policy and driver ops

[Viresh Kumar]

This extends the cpufreq bindings with bindings for cpufreq policy and
driver operations.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 rust/kernel/cpufreq.rs | 357 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 355 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
index 07ff03cb2991..f95e38291b15 100644
--- a/rust/kernel/cpufreq.rs
+++ b/rust/kernel/cpufreq.rs
@@ -7,15 +7,20 @@
 //! C header: [`include/linux/cpufreq.h`](srctree/include/linux/cpufreq.h)
 
 use crate::{
-    bindings,
-    error::{code::*, to_result, Result},
+    bindings, clk, cpumask,
+    device::Device,
+    error::{code::*, from_err_ptr, to_result, Result, VTABLE_DEFAULT_ERROR},
     prelude::*,
+    types::ForeignOwnable,
 };
 
 use core::{
     pin::Pin,
+    ptr::self,
 };
 
+use macros::vtable;
+
 /// Default transition latency value.
 pub const ETERNAL_LATENCY: u32 = bindings::CPUFREQ_ETERNAL as u32;
 
@@ -252,3 +257,351 @@ pub fn data(&self, index: usize) -> Result<u32> {
         Ok(unsafe { (*self.ptr.add(index)).driver_data })
     }
 }
+
+/// Equivalent to `struct cpufreq_policy` in the C code.
+pub struct Policy {
+    ptr: *mut bindings::cpufreq_policy,
+    put_cpu: bool,
+    cpumask: cpumask::Cpumask,
+}
+
+impl Policy {
+    /// Creates a new instance of [`Policy`].
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that `ptr` is valid and non-null.
+    pub unsafe fn from_raw_policy(ptr: *mut bindings::cpufreq_policy) -> Self {
+        Self {
+            ptr,
+            put_cpu: false,
+            // SAFETY: The pointer is guaranteed to be valid for the lifetime of `Self`. The `cpus`
+            // pointer is guaranteed to be valid by the C code.
+            cpumask: unsafe { cpumask::Cpumask::get_cpumask((*ptr).cpus) },
+        }
+    }
+
+    fn from_cpu(cpu: u32) -> Result<Self> {
+        // SAFETY: It is safe to call `cpufreq_cpu_get()` for any CPU.
+        let ptr = from_err_ptr(unsafe { bindings::cpufreq_cpu_get(cpu) })?;
+
+        // SAFETY: The pointer is guaranteed to be valid by the C code.
+        let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+        policy.put_cpu = true;
+        Ok(policy)
+    }
+
+    /// Raw pointer to the underlying cpufreq policy.
+    #[inline]
+    pub fn as_raw(&self) -> *mut bindings::cpufreq_policy {
+        self.ptr
+    }
+
+    fn as_ref(&self) -> &bindings::cpufreq_policy {
+        // SAFETY: By the type invariants, we know that `self` owns a reference to the pointer.
+        unsafe { &(*self.ptr) }
+    }
+    fn as_mut_ref(&mut self) -> &mut bindings::cpufreq_policy {
+        // SAFETY: By the type invariants, we know that `self` owns a reference to the pointer.
+        unsafe { &mut (*self.ptr) }
+    }
+
+    /// Returns the primary CPU for a cpufreq policy.
+    pub fn cpu(&self) -> u32 {
+        self.as_ref().cpu
+    }
+
+    /// Returns the minimum frequency for a cpufreq policy.
+    pub fn min(&self) -> u32 {
+        self.as_ref().min
+    }
+
+    /// Set the minimum frequency for a cpufreq policy.
+    pub fn set_min(&mut self, min: u32) -> &mut Self {
+        self.as_mut_ref().min = min;
+        self
+    }
+
+    /// Returns the maximum frequency for a cpufreq policy.
+    pub fn max(&self) -> u32 {
+        self.as_ref().max
+    }
+
+    /// Set the maximum frequency for a cpufreq policy.
+    pub fn set_max(&mut self, max: u32) -> &mut Self {
+        self.as_mut_ref().max = max;
+        self
+    }
+
+    /// Returns the current frequency for a cpufreq policy.
+    pub fn cur(&self) -> u32 {
+        self.as_ref().cur
+    }
+
+    /// Sets the suspend frequency for a cpufreq policy.
+    pub fn set_suspend_freq(&mut self, freq: u32) -> &mut Self {
+        self.as_mut_ref().suspend_freq = freq;
+        self
+    }
+
+    /// Returns the suspend frequency for a cpufreq policy.
+    pub fn suspend_freq(&self) -> u32 {
+        self.as_ref().suspend_freq
+    }
+
+    /// Provides a wrapper to the generic suspend routine.
+    pub fn generic_suspend(&self) -> Result<()> {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // use it now.
+        to_result(unsafe { bindings::cpufreq_generic_suspend(self.as_raw()) })
+    }
+
+    /// Provides a wrapper to the generic get routine.
+    pub fn generic_get(&self) -> Result<u32> {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // use it now.
+        Ok(unsafe { bindings::cpufreq_generic_get(self.cpu()) })
+    }
+
+    /// Provides a wrapper to the register em with OPP routine.
+    pub fn register_em_opp(&self) {
+        // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+        // use it now.
+        unsafe { bindings::cpufreq_register_em_with_opp(self.as_raw()) };
+    }
+
+    /// Gets raw pointer to cpufreq policy's CPUs mask.
+    pub fn cpus(&mut self) -> &mut cpumask::Cpumask {
+        &mut self.cpumask
+    }
+
+    /// Sets clock for a cpufreq policy.
+    pub fn set_clk(&mut self, dev: &Device, name: Option<&CStr>) -> Result<clk::Clk> {
+        let clk = clk::Clk::new(dev, name)?;
+        self.as_mut_ref().clk = clk.as_raw();
+        Ok(clk)
+    }
+
+    /// Allows frequency switching code to run on any CPU.
+    pub fn set_dvfs_possible_from_any_cpu(&mut self) -> &mut Self {
+        self.as_mut_ref().dvfs_possible_from_any_cpu = true;
+        self
+    }
+
+    /// Get fast_switch_possible value.
+    pub fn fast_switch_possible(&self) -> bool {
+        self.as_ref().fast_switch_possible
+    }
+
+    /// Enable/disable fast frequency switching.
+    pub fn set_fast_switch_possible(&mut self, val: bool) -> &mut Self {
+        self.as_mut_ref().fast_switch_possible = val;
+        self
+    }
+
+    /// Sets transition latency for a cpufreq policy.
+    pub fn set_transition_latency(&mut self, latency: u32) -> &mut Self {
+        self.as_mut_ref().cpuinfo.transition_latency = latency;
+        self
+    }
+
+    /// Set cpuinfo.min_freq.
+    pub fn set_cpuinfo_min_freq(&mut self, min_freq: u32) -> &mut Self {
+        self.as_mut_ref().cpuinfo.min_freq = min_freq;
+        self
+    }
+
+    /// Set cpuinfo.max_freq.
+    pub fn set_cpuinfo_max_freq(&mut self, max_freq: u32) -> &mut Self {
+        self.as_mut_ref().cpuinfo.max_freq = max_freq;
+        self
+    }
+
+    /// Set transition_delay_us, i.e. time between successive freq. change requests.
+    pub fn set_transition_delay_us(&mut self, transition_delay_us: u32) -> &mut Self {
+        self.as_mut_ref().transition_delay_us = transition_delay_us;
+        self
+    }
+
+    /// Returns the cpufreq table for a cpufreq policy. The cpufreq table is recreated in a
+    /// light-weight manner from the raw pointer. The table in C code is not freed once this table
+    /// is dropped.
+    pub fn freq_table(&self) -> Result<Table> {
+        if self.as_ref().freq_table.is_null() {
+            return Err(EINVAL);
+        }
+
+        // SAFETY: The `freq_table` is guaranteed to be valid.
+        Ok(unsafe { Table::from_raw(self.as_ref().freq_table) })
+    }
+
+    /// Sets the cpufreq table for a cpufreq policy.
+    ///
+    /// The cpufreq driver must guarantee that the frequency table does not get freed while it is
+    /// still being used by the C code.
+    pub fn set_freq_table(&mut self, table: &Table) -> &mut Self {
+        self.as_mut_ref().freq_table = table.as_raw();
+        self
+    }
+
+    /// Returns the data for a cpufreq policy.
+    pub fn data<T: ForeignOwnable>(&mut self) -> Option<<T>::Borrowed<'_>> {
+        if self.as_ref().driver_data.is_null() {
+            None
+        } else {
+            // SAFETY: The data is earlier set by us from [`set_data()`].
+            Some(unsafe { T::borrow(self.as_ref().driver_data) })
+        }
+    }
+
+    // Sets the data for a cpufreq policy.
+    fn set_data<T: ForeignOwnable>(&mut self, data: T) -> Result<()> {
+        if self.as_ref().driver_data.is_null() {
+            // Pass the ownership of the data to the foreign interface.
+            self.as_mut_ref().driver_data = <T as ForeignOwnable>::into_foreign(data) as _;
+            Ok(())
+        } else {
+            Err(EBUSY)
+        }
+    }
+
+    // Returns the data for a cpufreq policy.
+    fn clear_data<T: ForeignOwnable>(&mut self) -> Option<T> {
+        if self.as_ref().driver_data.is_null() {
+            None
+        } else {
+            let data = Some(
+                // SAFETY: The data is earlier set by us from [`set_data()`]. It is safe to take
+                // back the ownership of the data from the foreign interface.
+                unsafe { <T as ForeignOwnable>::from_foreign(self.as_ref().driver_data) }
+            );
+            self.as_mut_ref().driver_data = ptr::null_mut();
+            data
+        }
+    }
+}
+
+impl Drop for Policy {
+    fn drop(&mut self) {
+        if self.put_cpu {
+            // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+            // relinquish it now.
+            unsafe { bindings::cpufreq_cpu_put(self.as_raw()) };
+        }
+    }
+}
+
+/// Operations to be implemented by a cpufreq driver.
+#[vtable]
+pub trait Driver {
+    /// Driver specific data.
+    ///
+    /// Corresponds to the data retrieved via the kernel's
+    /// `cpufreq_get_driver_data()` function.
+    ///
+    /// Require that `Data` implements `ForeignOwnable`. We guarantee to
+    /// never move the underlying wrapped data structure.
+    type Data: ForeignOwnable;
+
+    /// Policy specific data.
+    ///
+    /// Require that `PData` implements `ForeignOwnable`. We guarantee to
+    /// never move the underlying wrapped data structure.
+    type PData: ForeignOwnable;
+
+    /// Policy's init callback.
+    fn init(policy: &mut Policy) -> Result<Self::PData>;
+
+    /// Policy's exit callback.
+    fn exit(_policy: &mut Policy, _data: Option<Self::PData>) -> Result<()> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's online callback.
+    fn online(_policy: &mut Policy) -> Result<()> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's offline callback.
+    fn offline(_policy: &mut Policy) -> Result<()> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's suspend callback.
+    fn suspend(_policy: &mut Policy) -> Result<()> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's resume callback.
+    fn resume(_policy: &mut Policy) -> Result<()> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's ready callback.
+    fn ready(_policy: &mut Policy) {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's verify callback.
+    fn verify(data: &mut PolicyData) -> Result<()>;
+
+    /// Policy's setpolicy callback.
+    fn setpolicy(_policy: &mut Policy) -> Result<()> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's target callback.
+    fn target(_policy: &mut Policy, _target_freq: u32, _relation: Relation) -> Result<()> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's target_index callback.
+    fn target_index(_policy: &mut Policy, _index: u32) -> Result<()> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's fast_switch callback.
+    fn fast_switch(_policy: &mut Policy, _target_freq: u32) -> u32 {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's adjust_perf callback.
+    fn adjust_perf(_policy: &mut Policy, _min_perf: usize, _target_perf: usize, _capacity: usize) {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's get_intermediate callback.
+    fn get_intermediate(_policy: &mut Policy, _index: u32) -> u32 {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's target_intermediate callback.
+    fn target_intermediate(_policy: &mut Policy, _index: u32) -> Result<()> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's get callback.
+    fn get(_policy: &mut Policy) -> Result<u32> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's update_limits callback.
+    fn update_limits(_policy: &mut Policy) {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's bios_limit callback.
+    fn bios_limit(_policy: &mut Policy, _limit: &mut u32) -> Result<()> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's set_boost callback.
+    fn set_boost(_policy: &mut Policy, _state: i32) -> Result<()> {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Policy's register_em callback.
+    fn register_em(_policy: &mut Policy) {
+        kernel::build_error(VTABLE_DEFAULT_ERROR)
+    }
+}
-- 
2.31.1.272.g89b43f80a514

[PATCH V6 12/15] rust: Extend cpufreq bindings for driver registration

[Viresh Kumar]

This extends the cpufreq bindings with bindings for registering a
driver.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 rust/kernel/cpufreq.rs | 476 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 474 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
index f95e38291b15..e651989cb142 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;
@@ -605,3 +608,472 @@ fn register_em(_policy: &mut Policy) {
         kernel::build_error(VTABLE_DEFAULT_ERROR)
     }
 }
+
+/// Registration of a cpufreq driver.
+pub struct Registration<T: Driver> {
+    drv: KBox<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> {}
+
+#[allow(clippy::non_send_fields_in_send_ty)]
+// 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.
+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 = KBox::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]) };
+        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 = 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 _
+        };
+        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 = KBox::leak(attr) as *mut _;
+
+        // Initialize mandatory callbacks.
+        drv_ref.init = Some(Self::init_callback);
+        drv_ref.verify = Some(Self::verify_callback);
+
+        // Initialize optional callbacks.
+        drv_ref.setpolicy = if T::HAS_SETPOLICY {
+            Some(Self::setpolicy_callback)
+        } else {
+            None
+        };
+        drv_ref.target = if T::HAS_TARGET {
+            Some(Self::target_callback)
+        } else {
+            None
+        };
+        drv_ref.target_index = if T::HAS_TARGET_INDEX {
+            Some(Self::target_index_callback)
+        } else {
+            None
+        };
+        drv_ref.fast_switch = if T::HAS_FAST_SWITCH {
+            Some(Self::fast_switch_callback)
+        } else {
+            None
+        };
+        drv_ref.adjust_perf = if T::HAS_ADJUST_PERF {
+            Some(Self::adjust_perf_callback)
+        } else {
+            None
+        };
+        drv_ref.get_intermediate = if T::HAS_GET_INTERMEDIATE {
+            Some(Self::get_intermediate_callback)
+        } else {
+            None
+        };
+        drv_ref.target_intermediate = if T::HAS_TARGET_INTERMEDIATE {
+            Some(Self::target_intermediate_callback)
+        } else {
+            None
+        };
+        drv_ref.get = if T::HAS_GET {
+            Some(Self::get_callback)
+        } else {
+            None
+        };
+        drv_ref.update_limits = if T::HAS_UPDATE_LIMITS {
+            Some(Self::update_limits_callback)
+        } else {
+            None
+        };
+        drv_ref.bios_limit = if T::HAS_BIOS_LIMIT {
+            Some(Self::bios_limit_callback)
+        } else {
+            None
+        };
+        drv_ref.online = if T::HAS_ONLINE {
+            Some(Self::online_callback)
+        } else {
+            None
+        };
+        drv_ref.offline = if T::HAS_OFFLINE {
+            Some(Self::offline_callback)
+        } else {
+            None
+        };
+        drv_ref.exit = if T::HAS_EXIT {
+            Some(Self::exit_callback)
+        } else {
+            None
+        };
+        drv_ref.suspend = if T::HAS_SUSPEND {
+            Some(Self::suspend_callback)
+        } else {
+            None
+        };
+        drv_ref.resume = if T::HAS_RESUME {
+            Some(Self::resume_callback)
+        } else {
+            None
+        };
+        drv_ref.ready = if T::HAS_READY {
+            Some(Self::ready_callback)
+        } else {
+            None
+        };
+        drv_ref.set_boost = if T::HAS_SET_BOOST {
+            Some(Self::set_boost_callback)
+        } else {
+            None
+        };
+        drv_ref.register_em = if T::HAS_REGISTER_EM {
+            Some(Self::register_em_callback)
+        } else {
+            None
+        };
+
+        // Set driver data before registering the driver, as the cpufreq core may call few
+        // callbacks before `cpufreq_register_driver()` returns.
+        Self::set_data(drv_ref, data)?;
+
+        // SAFETY: It is safe to register the driver with the cpufreq core in the C code.
+        to_result(unsafe { bindings::cpufreq_register_driver(drv_ref) })?;
+
+        Ok(Self {
+            drv,
+            _p: PhantomData,
+        })
+    }
+
+    /// Same as [Registration::new`], but does not return a `Registration` instance.
+    /// Instead the `Registration` is owned by devres and will be revoked / dropped, once the
+    /// device is detached.
+    pub fn new_foreign_owned(
+        dev: &Device,
+        name: &'static CStr,
+        data: T::Data,
+        flags: u16,
+        boost: bool,
+    ) -> Result<()> {
+        let reg = Self::new(name, data, flags, boost)?;
+        Devres::new_foreign_owned(dev, reg, GFP_KERNEL)?;
+        Ok(())
+    }
+
+    // Sets the data for a cpufreq driver.
+    fn set_data(drv: &mut bindings::cpufreq_driver, data: T::Data) -> Result<()> {
+        if drv.driver_data.is_null() {
+            // Pass the ownership of the data to the foreign interface.
+            drv.driver_data = <T::Data as ForeignOwnable>::into_foreign(data) as _;
+            Ok(())
+        } else {
+            Err(EBUSY)
+        }
+    }
+
+    /// Returns the previous set data for a cpufreq driver.
+    pub fn data(&mut self) -> Option<<T::Data as ForeignOwnable>::Borrowed<'static>> {
+        let drv = self.drv.get_mut();
+
+        if drv.driver_data.is_null() {
+            None
+        } else {
+            // SAFETY: The data is earlier set by us from [`set_data()`].
+            Some(unsafe { <T::Data as ForeignOwnable>::borrow(drv.driver_data) })
+        }
+    }
+
+    // Clears and returns the data for a cpufreq driver.
+    fn clear_data(&mut self) -> Option<T::Data> {
+        let drv = self.drv.get_mut();
+
+        if drv.driver_data.is_null() {
+            None
+        } else {
+            // SAFETY: By the type invariants, we know that `self` owns a reference, so it is safe to
+            // relinquish it now.
+            let data = Some(unsafe { <T::Data as ForeignOwnable>::from_foreign(drv.driver_data) });
+            drv.driver_data = ptr::null_mut();
+            data
+        }
+    }
+}
+
+// cpufreq driver callbacks.
+impl<T: Driver> Registration<T> {
+    // Policy's init callback.
+    extern "C" fn init_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+            // duration of this call, so it is guaranteed to remain alive for the lifetime of
+            // `ptr`.
+            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+
+            let data = T::init(&mut policy)?;
+            policy.set_data(data)?;
+            Ok(0)
+        })
+    }
+
+    // Policy's exit callback.
+    extern "C" fn exit_callback(ptr: *mut bindings::cpufreq_policy) {
+        // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+        // duration of this call, so it is guaranteed to remain alive for the lifetime of
+        // `ptr`.
+        let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+
+        let data = policy.clear_data();
+        let _ = T::exit(&mut policy, data);
+    }
+
+    // Policy's online callback.
+    extern "C" fn online_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+            // duration of this call, so it is guaranteed to remain alive for the lifetime of
+            // `ptr`.
+            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+            T::online(&mut policy).map(|()| 0)
+        })
+    }
+
+    // Policy's offline callback.
+    extern "C" fn offline_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+            // duration of this call, so it is guaranteed to remain alive for the lifetime of
+            // `ptr`.
+            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+            T::offline(&mut policy).map(|()| 0)
+        })
+    }
+
+    // Policy's suspend callback.
+    extern "C" fn suspend_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+            // duration of this call, so it is guaranteed to remain alive for the lifetime of
+            // `ptr`.
+            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+            T::suspend(&mut policy).map(|()| 0)
+        })
+    }
+
+    // Policy's resume callback.
+    extern "C" fn resume_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+            // duration of this call, so it is guaranteed to remain alive for the lifetime of
+            // `ptr`.
+            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+            T::resume(&mut policy).map(|()| 0)
+        })
+    }
+
+    // Policy's ready callback.
+    extern "C" fn ready_callback(ptr: *mut bindings::cpufreq_policy) {
+        // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+        // duration of this call, so it is guaranteed to remain alive for the lifetime of
+        // `ptr`.
+        let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+        T::ready(&mut policy);
+    }
+
+    // Policy's verify callback.
+    extern "C" fn verify_callback(ptr: *mut bindings::cpufreq_policy_data) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+            // duration of this call, so it is guaranteed to remain alive for the lifetime of
+            // `ptr`.
+            let mut data = unsafe { PolicyData::from_raw_policy_data(ptr) };
+            T::verify(&mut data).map(|()| 0)
+        })
+    }
+
+    // Policy's setpolicy callback.
+    extern "C" fn setpolicy_callback(ptr: *mut bindings::cpufreq_policy) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+            // duration of this call, so it is guaranteed to remain alive for the lifetime of
+            // `ptr`.
+            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+            T::setpolicy(&mut policy).map(|()| 0)
+        })
+    }
+
+    // Policy's target callback.
+    extern "C" fn target_callback(
+        ptr: *mut bindings::cpufreq_policy,
+        target_freq: u32,
+        relation: u32,
+    ) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+            // duration of this call, so it is guaranteed to remain alive for the lifetime of
+            // `ptr`.
+            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+            T::target(&mut policy, target_freq, Relation::new(relation)?).map(|()| 0)
+        })
+    }
+
+    // Policy's target_index callback.
+    extern "C" fn target_index_callback(
+        ptr: *mut bindings::cpufreq_policy,
+        index: u32,
+    ) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+            // duration of this call, so it is guaranteed to remain alive for the lifetime of
+            // `ptr`.
+            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+            T::target_index(&mut policy, index).map(|()| 0)
+        })
+    }
+
+    // Policy's fast_switch callback.
+    extern "C" fn fast_switch_callback(
+        ptr: *mut bindings::cpufreq_policy,
+        target_freq: u32,
+    ) -> core::ffi::c_uint {
+        // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+        // duration of this call, so it is guaranteed to remain alive for the lifetime of
+        // `ptr`.
+        let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+        T::fast_switch(&mut policy, target_freq)
+    }
+
+    // Policy's adjust_perf callback.
+    extern "C" fn adjust_perf_callback(cpu: u32, min_perf: usize, target_perf: usize, capacity: usize) {
+        if let Ok(mut policy) = Policy::from_cpu(cpu) {
+            T::adjust_perf(&mut policy, min_perf, target_perf, capacity);
+        }
+    }
+
+    // Policy's get_intermediate callback.
+    extern "C" fn get_intermediate_callback(
+        ptr: *mut bindings::cpufreq_policy,
+        index: u32,
+    ) -> core::ffi::c_uint {
+        // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+        // duration of this call, so it is guaranteed to remain alive for the lifetime of
+        // `ptr`.
+        let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+        T::get_intermediate(&mut policy, index)
+    }
+
+    // Policy's target_intermediate callback.
+    extern "C" fn target_intermediate_callback(
+        ptr: *mut bindings::cpufreq_policy,
+        index: u32,
+    ) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+            // duration of this call, so it is guaranteed to remain alive for the lifetime of
+            // `ptr`.
+            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+            T::target_intermediate(&mut policy, index).map(|()| 0)
+        })
+    }
+
+    // Policy's get callback.
+    extern "C" fn get_callback(cpu: u32) -> core::ffi::c_uint {
+        Policy::from_cpu(cpu).map_or(0, |mut policy| T::get(&mut policy).map_or(0, |f| f))
+    }
+
+    // Policy's update_limit callback.
+    extern "C" fn update_limits_callback(cpu: u32) {
+        if let Ok(mut policy) = Policy::from_cpu(cpu) {
+            T::update_limits(&mut policy);
+        }
+    }
+
+    // Policy's bios_limit callback.
+    extern "C" fn bios_limit_callback(cpu: i32, limit: *mut u32) -> core::ffi::c_int {
+        from_result(|| {
+            let mut policy = Policy::from_cpu(cpu as u32)?;
+
+            // SAFETY: The pointer is guaranteed by the C code to be valid.
+            T::bios_limit(&mut policy, &mut (unsafe { *limit })).map(|()| 0)
+        })
+    }
+
+    // Policy's set_boost callback.
+    extern "C" fn set_boost_callback(
+        ptr: *mut bindings::cpufreq_policy,
+        state: i32,
+    ) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+            // duration of this call, so it is guaranteed to remain alive for the lifetime of
+            // `ptr`.
+            let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+            T::set_boost(&mut policy, state).map(|()| 0)
+        })
+    }
+
+    // Policy's register_em callback.
+    extern "C" fn register_em_callback(ptr: *mut bindings::cpufreq_policy) {
+        // SAFETY: `ptr` is valid by the contract with the C code. `policy` is alive only for the
+        // duration of this call, so it is guaranteed to remain alive for the lifetime of
+        // `ptr`.
+        let mut policy = unsafe { Policy::from_raw_policy(ptr) };
+        T::register_em(&mut policy);
+    }
+}
+
+impl<T: Driver> Drop for Registration<T> {
+    // Removes the registration from the kernel if it has completed successfully before.
+    fn drop(&mut self) {
+        pr_info!("Registration dropped\n");
+        let drv = self.drv.get_mut();
+
+        // SAFETY: The driver was earlier registered from `new()`.
+        unsafe { bindings::cpufreq_unregister_driver(drv) };
+
+        // Free the previously leaked memory to the C code.
+        if !drv.attr.is_null() {
+            // SAFETY: The pointer was earlier initialized from the result of `KBox::leak`.
+            unsafe { drop(KBox::from_raw(drv.attr)) };
+        }
+
+        // Free data
+        drop(self.clear_data());
+    }
+}
-- 
2.31.1.272.g89b43f80a514

[PATCH V6 13/15] rust: Extend OPP bindings with CPU frequency table

[Viresh Kumar]

This commit adds bindings for CPUFreq core related API.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 rust/kernel/opp.rs | 61 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 60 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
index d92e88af0cec..d48f33a2ab8d 100644
--- a/rust/kernel/opp.rs
+++ b/rust/kernel/opp.rs
@@ -16,7 +16,10 @@
     types::{ARef, AlwaysRefCounted, Opaque},
 };
 
-use core::{marker::PhantomData, ptr};
+#[cfg(CONFIG_CPU_FREQ)]
+use crate::cpufreq;
+
+use core::{marker::PhantomData, ops::Deref, ptr};
 
 use macros::vtable;
 
@@ -337,6 +340,56 @@ extern "C" fn config_regulators(
     }
 }
 
+/// CPU Frequency table created from OPP entries.
+#[cfg(CONFIG_CPU_FREQ)]
+pub struct FreqTable {
+    dev: ARef<Device>,
+    table: cpufreq::Table,
+}
+
+#[cfg(CONFIG_CPU_FREQ)]
+impl FreqTable {
+    /// Creates new instance of [`FreqTable`] from raw pointer.
+    fn new(table: &Table) -> Result<Self> {
+        let mut ptr: *mut bindings::cpufreq_frequency_table = ptr::null_mut();
+
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        to_result(unsafe {
+            bindings::dev_pm_opp_init_cpufreq_table(table.dev.as_raw(), &mut ptr)
+        })?;
+        Ok(Self {
+            dev: table.dev.clone(),
+            // SAFETY: The `ptr` is guaranteed by the C code to be valid.
+            table: unsafe { cpufreq::Table::from_raw(ptr) },
+        })
+    }
+
+    /// Returns reference to the underlying [`cpufreq::Table`].
+    pub fn table(&self) -> &cpufreq::Table {
+        &self.table
+    }
+}
+
+#[cfg(CONFIG_CPU_FREQ)]
+impl Deref for FreqTable {
+    type Target = cpufreq::Table;
+
+    #[inline]
+    fn deref(&self) -> &Self::Target {
+        &self.table
+    }
+}
+
+#[cfg(CONFIG_CPU_FREQ)]
+impl Drop for FreqTable {
+    fn drop(&mut self) {
+        // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
+        // requirements.
+        unsafe { bindings::dev_pm_opp_free_cpufreq_table(self.dev.as_raw(), &mut self.as_raw()) };
+    }
+}
+
 /// Operating performance point (OPP) table.
 ///
 /// Wraps the kernel's `struct opp_table`.
@@ -541,6 +594,12 @@ pub fn adjust_voltage(
         })
     }
 
+    /// Create cpufreq table from OPP table.
+    #[cfg(CONFIG_CPU_FREQ)]
+    pub fn to_cpufreq_table(&mut self) -> Result<FreqTable> {
+        FreqTable::new(self)
+    }
+
     /// Sets a matching OPP based on frequency.
     pub fn set_rate(&self, freq: usize) -> Result<()> {
         // SAFETY: The requirements are satisfied by the existence of `Device` and its safety
-- 
2.31.1.272.g89b43f80a514

[PATCH V6 14/15] cpufreq: Add Rust based cpufreq-dt driver

[Viresh Kumar]

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@linaro.org>
---
 drivers/cpufreq/Kconfig        |  12 ++
 drivers/cpufreq/Makefile       |   1 +
 drivers/cpufreq/rcpufreq_dt.rs | 232 +++++++++++++++++++++++++++++++++
 3 files changed, 245 insertions(+)
 create mode 100644 drivers/cpufreq/rcpufreq_dt.rs

diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
index 92a83a9bb2e1..5e60dfb6f93e 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_VIRT
 	tristate "Virtual cpufreq driver"
 	depends on GENERIC_ARCH_TOPOLOGY
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index d35a28dd9463..db38d1d5562d 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
 obj-$(CONFIG_CPUFREQ_VIRT)		+= virtual-cpufreq.o
 
diff --git a/drivers/cpufreq/rcpufreq_dt.rs b/drivers/cpufreq/rcpufreq_dt.rs
new file mode 100644
index 000000000000..62831629bd93
--- /dev/null
+++ b/drivers/cpufreq/rcpufreq_dt.rs
@@ -0,0 +1,232 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Rust based implementation of the cpufreq-dt driver.
+
+use core::format_args;
+
+use kernel::{
+    c_str, clk, cpufreq, cpumask::Cpumask, 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(dev: &Device, name: &str) -> Option<CString> {
+    let name_cstr = CString::try_from_fmt(fmt!("{}-supply", name)).ok()?;
+
+    if dev.property_present(&name_cstr) {
+        CString::try_from_fmt(fmt!("{}", name)).ok()
+    } else {
+        None
+    }
+}
+
+// Finds supply name for the CPU from DT.
+fn find_supply_names(dev: &Device, cpu: u32) -> Option<KVec<CString>> {
+    // Try "cpu0" for older DTs.
+    let name = match cpu {
+        0 => find_supply_name_exact(dev, "cpu0"),
+        _ => None,
+    }
+    .or(find_supply_name_exact(dev, "cpu"))?;
+
+    let mut list = KVec::with_capacity(1, GFP_KERNEL).ok()?;
+    list.push(name, GFP_KERNEL).ok()?;
+
+    Some(list)
+}
+
+// Represents the cpufreq dt device.
+struct CPUFreqDTDevice {
+    opp_table: opp::Table,
+    freq_table: opp::FreqTable,
+    #[allow(dead_code)]
+    mask: Cpumask,
+    #[allow(dead_code)]
+    token: Option<opp::ConfigToken>,
+    #[allow(dead_code)]
+    clk: clk::Clk,
+}
+
+struct CPUFreqDTDriver {
+    _pdev: platform::Device,
+}
+
+#[vtable]
+impl opp::ConfigOps for CPUFreqDTDriver {}
+
+#[vtable]
+impl cpufreq::Driver for CPUFreqDTDriver {
+    type Data = ();
+    type PData = Arc<CPUFreqDTDevice>;
+
+    fn init(policy: &mut cpufreq::Policy) -> Result<Self::PData> {
+        let cpu = policy.cpu();
+        let dev = Device::from_cpu(cpu)?;
+        let mut mask = Cpumask::new()?;
+
+        mask.set(cpu);
+
+        let token = match find_supply_names(&dev, cpu) {
+            Some(names) => Some(
+                opp::Config::<Self>::new()
+                    .set_regulator_names(names)?
+                    .set(&dev)?,
+            ),
+            _ => None,
+        };
+
+        // Get OPP-sharing information from "operating-points-v2" bindings.
+        let fallback = match opp::Table::of_sharing_cpus(&dev, &mut mask) {
+            Ok(()) => false,
+            Err(e) => {
+                if e != ENOENT {
+                    return Err(e);
+                }
+
+                // "operating-points-v2" not supported. If the platform hasn't
+                // set sharing CPUs, fallback to all CPUs share the `Policy`
+                // for backward compatibility.
+                opp::Table::sharing_cpus(&dev, &mut mask).is_err()
+            }
+        };
+
+        // Initialize OPP tables for all policy cpus.
+        //
+        // For platforms not using "operating-points-v2" bindings, we do this
+        // before updating policy cpus. Otherwise, we will end up creating
+        // duplicate OPPs for the CPUs.
+        //
+        // OPPs might be populated at runtime, don't fail for error here unless
+        // it is -EPROBE_DEFER.
+        let mut opp_table = match opp::Table::from_of_cpumask(&dev, &mut mask) {
+            Ok(table) => table,
+            Err(e) => {
+                if e == EPROBE_DEFER {
+                    return Err(e);
+                }
+
+                // The table is added dynamically ?
+                opp::Table::from_dev(&dev)?
+            }
+        };
+
+        // The OPP table must be initialized, statically or dynamically, by this point.
+        opp_table.opp_count()?;
+
+        // Set sharing cpus for fallback scenario.
+        if fallback {
+            mask.set_all();
+            opp_table.set_sharing_cpus(&mut mask)?;
+        }
+
+        let mut transition_latency = opp_table.max_transition_latency() as u32;
+        if transition_latency == 0 {
+            transition_latency = cpufreq::ETERNAL_LATENCY;
+        }
+
+        let freq_table = opp_table.to_cpufreq_table()?;
+        let clk = policy
+            .set_freq_table(freq_table.table())
+            .set_dvfs_possible_from_any_cpu()
+            .set_suspend_freq((opp_table.suspend_freq() / 1000) as u32)
+            .set_transition_latency(transition_latency)
+            .set_clk(&dev, None)?;
+
+        mask.copy(policy.cpus());
+
+        Ok(Arc::new(
+            CPUFreqDTDevice {
+                opp_table,
+                freq_table,
+                mask,
+                token,
+                clk,
+            },
+            GFP_KERNEL,
+        )?)
+    }
+
+    fn exit(_policy: &mut cpufreq::Policy, _data: Option<Self::PData>) -> Result<()> {
+        Ok(())
+    }
+
+    fn online(_policy: &mut cpufreq::Policy) -> Result<()> {
+        // We did light-weight tear down earlier, nothing to do here.
+        Ok(())
+    }
+
+    fn offline(_policy: &mut cpufreq::Policy) -> Result<()> {
+        // Preserve policy->data and don't free resources on light-weight
+        // tear down.
+        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: u32) -> Result<()> {
+        let data = match policy.data::<Self::PData>() {
+            Some(data) => data,
+            None => return Err(ENOENT),
+        };
+
+        let freq = data.freq_table.freq(index.try_into().unwrap())? as usize;
+        data.opp_table.set_rate(freq * 1000)
+    }
+
+    fn get(policy: &mut cpufreq::Policy) -> Result<u32> {
+        policy.generic_get()
+    }
+
+    fn set_boost(_policy: &mut cpufreq::Policy, _state: i32) -> Result<()> {
+        Ok(())
+    }
+
+    fn register_em(policy: &mut cpufreq::Policy) {
+        policy.register_em_opp()
+    }
+}
+
+kernel::of_device_table!(
+    OF_TABLE,
+    MODULE_OF_TABLE,
+    <CPUFreqDTDriver as platform::Driver>::IdInfo,
+    [
+    (of::DeviceId::new(c_str!("operating-points-v2")), ())
+    ]
+);
+
+impl platform::Driver for CPUFreqDTDriver {
+    type IdInfo = ();
+    const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
+
+    fn probe(pdev: &mut platform::Device, _id_info: Option<&Self::IdInfo>) -> Result<Pin<KBox<Self>>> {
+        cpufreq::Registration::<CPUFreqDTDriver>::new_foreign_owned(
+            pdev.as_ref(),
+            c_str!("cpufreq-dt"),
+            (),
+            cpufreq::flags::NEED_INITIAL_FREQ_CHECK | cpufreq::flags::IS_COOLING_DEV,
+            true,
+        )?;
+
+        pr_info!("CPUFreq DT driver registered\n");
+
+        let drvdata = KBox::new(Self { _pdev: pdev.clone() }, GFP_KERNEL)?;
+
+        Ok(drvdata.into())
+    }
+}
+
+module_platform_driver! {
+    type: CPUFreqDTDriver,
+    name: "cpufreq_dt",
+    author: "Viresh Kumar <viresh.kumar@linaro.org>",
+    description: "Generic CPUFreq DT driver",
+    license: "GPL v2",
+}
-- 
2.31.1.272.g89b43f80a514

Re: [PATCH V6 14/15] cpufreq: Add Rust based cpufreq-dt driver

[Greg KH]

On Tue, Jan 07, 2025 at 04:51:47PM +0530, Viresh Kumar wrote:
> +impl platform::Driver for CPUFreqDTDriver {
> +    type IdInfo = ();
> +    const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
> +
> +    fn probe(pdev: &mut platform::Device, _id_info: Option<&Self::IdInfo>) -> Result<Pin<KBox<Self>>> {
> +        cpufreq::Registration::<CPUFreqDTDriver>::new_foreign_owned(
> +            pdev.as_ref(),
> +            c_str!("cpufreq-dt"),
> +            (),
> +            cpufreq::flags::NEED_INITIAL_FREQ_CHECK | cpufreq::flags::IS_COOLING_DEV,
> +            true,
> +        )?;
> +
> +        pr_info!("CPUFreq DT driver registered\n");

When drivers work properly, they are quiet.  Please don't make us have
to go and remove lines like this from new drivers being added to the
tree like we did for older drivers in the past :(

thanks,

greg k-h

[PATCH V6 15/15] DO-NOT_MERGE: cpufreq: Rename cpufreq-dt platdev

[Viresh Kumar]

The module! implementation in the Rust code expects a module name
without a '-' symbol, else it fails to compile with following errors:

error: expected one of `:`, `;`, or `=`, found `-`
   --> drivers/cpufreq/rcpufreq_dt.rs:247:1
    |
247 | / module_platform_driver! {
248 | |     type: CPUFreqDTDriver,
249 | |     name: "cpufreq-dt",
250 | |     author: "Viresh Kumar <viresh.kumar@linaro.org>",
251 | |     description: "Generic CPUFreq DT driver",
252 | |     license: "GPL v2",
253 | | }
    | |_^ expected one of `:`, `;`, or `=`
    |
    = note: this error originates in the macro `$crate::prelude::module` which comes from the expansion of the macro `module_platform_driver` (in Nightly builds, run with -Z macro-backtrace for more info)

This must be fixed properly in the Rust code instead. Not to be merged.

Not-Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq-dt-platdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
index 2a3e8bd317c9..e69ff11eccbc 100644
--- a/drivers/cpufreq/cpufreq-dt-platdev.c
+++ b/drivers/cpufreq/cpufreq-dt-platdev.c
@@ -230,7 +230,7 @@ static int __init cpufreq_dt_platdev_init(void)
 	return -ENODEV;
 
 create_pdev:
-	return PTR_ERR_OR_ZERO(platform_device_register_data(NULL, "cpufreq-dt",
+	return PTR_ERR_OR_ZERO(platform_device_register_data(NULL, "cpufreq_dt",
 			       -1, data,
 			       sizeof(struct cpufreq_dt_platform_data)));
 }
-- 
2.31.1.272.g89b43f80a514

Re: [PATCH V6 00/15] Rust bindings for cpufreq and OPP core + sample driver

[Danilo Krummrich]

Hi Viresh,

On Tue, Jan 07, 2025 at 04:51:33PM +0530, Viresh Kumar wrote:
> Miguel / Danilo,
> 
> I would like to get some of this (if not all) merged now into the rust/dev

You mean rust-dev or staging/dev?

> branch, since this stuff has been around for some time and many other
> dependencies are already merged new (like OF, platform, device/driver). Please
> suggest how can we move forward to get that done.

Now that the device / driver, PCI and platform code has been merged, rust-device
and rust-pci are obsolete. The only thing that's left that I keep merging into
staging/dev is the DRM abstractions from the drm-misc tree. So, maybe we may
just want to drop staging/dev?

- Danilo

> 
> -------------------------8<-------------------------
> 
> This patch 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.
> 
> Included in this series is a sample `cpufreq` driver, `rcpufreq-dt`, which is a
> duplicate of the existing `cpufreq-dt` driver. The `cpufreq-dt` driver is a
> generic, platform-agnostic, device-tree-based driver used on many ARM platforms.
> 
> Currently, the implementation has been tested using QEMU, verifying that
> frequency transitions, various configurations, and driver binding/unbinding
> functions as expected. However, performance measurements have not been
> conducted.
> 
> For those interested in trying these patches, along with a few dependencies,
> they can be found at:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/linux.git rust/cpufreq-dt
> 
> The series includes all the patches to make it work:
> - few new APIs to cpufreq/OPP frameworks.
> - Avoiding using BIT() macro in cpufreq core (we need to make Rust work with
>   it).
> - Renaming of the cpufreq-dt platform device (this too needs to be fixed in
>   Rust).
> - Basic Rust bindings for clk and cpumask layers.
> 
> The work is based on `staging/dev` from the Rust tree, which is derived
> from v6.13-rc3.
> 
> 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.
> 
> Viresh Kumar (15):
>   PM / OPP: Expose refcounting helpers for the Rust implementation
>   cpufreq: Add cpufreq_table_len()
>   cpufreq: Rust implementation doesn't parse BIT() macro
>   rust: device: Add few 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
>   DO-NOT_MERGE: cpufreq: Rename cpufreq-dt platdev
> 
>  MAINTAINERS                          |    2 +
>  drivers/cpufreq/Kconfig              |   12 +
>  drivers/cpufreq/Makefile             |    1 +
>  drivers/cpufreq/cpufreq-dt-platdev.c |    2 +-
>  drivers/cpufreq/rcpufreq_dt.rs       |  232 ++++++
>  drivers/opp/core.c                   |   17 +-
>  drivers/opp/opp.h                    |    1 -
>  include/linux/cpufreq.h              |   27 +-
>  include/linux/pm_opp.h               |    6 +
>  rust/bindings/bindings_helper.h      |    5 +
>  rust/helpers/cpufreq.c               |   16 +
>  rust/helpers/cpumask.c               |   35 +
>  rust/helpers/helpers.c               |    2 +
>  rust/kernel/clk.rs                   |   48 ++
>  rust/kernel/cpufreq.rs               | 1079 ++++++++++++++++++++++++++
>  rust/kernel/cpumask.rs               |   85 ++
>  rust/kernel/device.rs                |   21 +
>  rust/kernel/lib.rs                   |    6 +
>  rust/kernel/opp.rs                   |  886 +++++++++++++++++++++
>  19 files changed, 2468 insertions(+), 15 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/cpufreq.rs
>  create mode 100644 rust/kernel/cpumask.rs
>  create mode 100644 rust/kernel/opp.rs
> 
> -- 
> 2.31.1.272.g89b43f80a514
> 

Re: [PATCH V6 00/15] Rust bindings for cpufreq and OPP core + sample driver

[Viresh Kumar]

On 07-01-25, 12:47, Danilo Krummrich wrote:
> On Tue, Jan 07, 2025 at 04:51:33PM +0530, Viresh Kumar wrote:
> > I would like to get some of this (if not all) merged now into the rust/dev
> 
> You mean rust-dev or staging/dev?

Right.

-- 
viresh

Re: [PATCH V6 00/15] Rust bindings for cpufreq and OPP core + sample driver

[Miguel Ojeda]

On Wed, Jan 8, 2025 at 6:20 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 07-01-25, 12:47, Danilo Krummrich wrote:
> > On Tue, Jan 07, 2025 at 04:51:33PM +0530, Viresh Kumar wrote:
> > > I would like to get some of this (if not all) merged now into the rust/dev
> >
> > You mean rust-dev or staging/dev?
>
> Right.

I think something was missing there :) i.e. which one?

In any case, for upstreaming eventually, then this should ideally go
through the relevant maintainers. So I guess that means yourself in a
few cases here.

Cheers,
Miguel

Re: [PATCH V6 00/15] Rust bindings for cpufreq and OPP core + sample driver

[Viresh Kumar]

On 08-01-25, 16:03, Miguel Ojeda wrote:
> On Wed, Jan 8, 2025 at 6:20 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > On 07-01-25, 12:47, Danilo Krummrich wrote:
> > > On Tue, Jan 07, 2025 at 04:51:33PM +0530, Viresh Kumar wrote:
> > > > I would like to get some of this (if not all) merged now into the rust/dev
> > >
> > > You mean rust-dev or staging/dev?
> >
> > Right.
> 
> I think something was missing there :) i.e. which one?

/facepalm

https://github.com/Rust-for-Linux/linux staging/dev

> In any case, for upstreaming eventually, then this should ideally go
> through the relevant maintainers. So I guess that means yourself in a
> few cases here.

Ahh, I thought they are all getting merged via the Rust tree. Sure I can take
them in once I manage to get Acks from you guys.

Thanks.

-- 
viresh