Linux Power Management development
 help / color / mirror / Atom feed
* Re: [regression] 6.8.1 and 6.7.12: fails to hibernate with pm_runtime_force_suspend+0x0/0x120 returns -16
From: Greg KH @ 2024-04-05 13:34 UTC (permalink / raw)
  To: Martin Steigerwald; +Cc: linux-pm, regressions, linux-kernel
In-Reply-To: <12414153.O9o76ZdvQC@lichtvoll.de>

On Fri, Apr 05, 2024 at 02:11:11PM +0200, Martin Steigerwald wrote:
> Martin Steigerwald - 05.04.24, 13:54:34 CEST:
> > > Not doing it today or probably the weekend, but now I have some
> > > actionable git bisect plan without bisecting between major kernel
> > > releases which as I have been told between 6.7 an 6.8-rc1 can lead to
> > > non working modeset graphics on AMD Ryzen in between.
> > > 
> > > It seems now I only need to git bisect between 6.7.11 and 6.7.12 to
> > > find the patch that breaks hibernation on 6.8.1 as well. However
> > > first I will briefly check whether 6.8.4 hibernates okay.
> > 
> > 6.8.4 is still affected and fails hibernation with the same error
> > message.
> 
> Both kernels also fail to reboot the machine. Runit says:
> 
> - runit: system reboot
> 
> And then nothing anymore.
> 
> It is not just hibernation.
> 
> I think ThinkPad T14 AMD Gen 1 or similar systems with Linux are not that 
> rare. I am surprised this is not hitting more people. Well maybe it does 
> and no one reported it here.
> 
> Well let's see what happens when 6.7.12 hits distribution kernels.
> 
> Anyway, I have an actionable git bisect between 6.7.11 and 6.7.12, just 
> not today. Maybe beginning of next week.

6.7.y is end-of-life, I wouldn't worry too much about it, there's
nothing we can do about it anymore, sorry.

greg k-h

^ permalink raw reply

* [PATCH] ACPI: DPTF: Add Lunar Lake support
From: Sumeet Pawnikar @ 2024-04-05 12:18 UTC (permalink / raw)
  To: rafael, srinivas.pandruvada, linux-acpi, linux-kernel, linux-pm
  Cc: sumeet.r.pawnikar

Add Lunar Lake ACPI IDs for DPTF devices.

Signed-off-by: Sumeet Pawnikar <sumeet.r.pawnikar@intel.com>
---
 drivers/acpi/dptf/dptf_pch_fivr.c                       | 1 +
 drivers/acpi/dptf/dptf_power.c                          | 2 ++
 drivers/acpi/dptf/int340x_thermal.c                     | 6 ++++++
 drivers/acpi/fan.h                                      | 1 +
 drivers/thermal/intel/int340x_thermal/int3400_thermal.c | 1 +
 drivers/thermal/intel/int340x_thermal/int3403_thermal.c | 1 +
 6 files changed, 12 insertions(+)

diff --git a/drivers/acpi/dptf/dptf_pch_fivr.c b/drivers/acpi/dptf/dptf_pch_fivr.c
index 654aaa53c67f..d202730fafd8 100644
--- a/drivers/acpi/dptf/dptf_pch_fivr.c
+++ b/drivers/acpi/dptf/dptf_pch_fivr.c
@@ -150,6 +150,7 @@ static const struct acpi_device_id pch_fivr_device_ids[] = {
 	{"INTC1045", 0},
 	{"INTC1049", 0},
 	{"INTC1064", 0},
+	{"INTC106B", 0},
 	{"INTC10A3", 0},
 	{"", 0},
 };
diff --git a/drivers/acpi/dptf/dptf_power.c b/drivers/acpi/dptf/dptf_power.c
index b8187babbbbb..8023b3e23315 100644
--- a/drivers/acpi/dptf/dptf_power.c
+++ b/drivers/acpi/dptf/dptf_power.c
@@ -232,6 +232,8 @@ static const struct acpi_device_id int3407_device_ids[] = {
 	{"INTC1061", 0},
 	{"INTC1065", 0},
 	{"INTC1066", 0},
+	{"INTC106C", 0},
+	{"INTC106D", 0},
 	{"INTC10A4", 0},
 	{"INTC10A5", 0},
 	{"", 0},
diff --git a/drivers/acpi/dptf/int340x_thermal.c b/drivers/acpi/dptf/int340x_thermal.c
index b7113fa92fa6..014ada759954 100644
--- a/drivers/acpi/dptf/int340x_thermal.c
+++ b/drivers/acpi/dptf/int340x_thermal.c
@@ -43,6 +43,12 @@ static const struct acpi_device_id int340x_thermal_device_ids[] = {
 	{"INTC1064"},
 	{"INTC1065"},
 	{"INTC1066"},
+	{"INTC1068"},
+	{"INTC1069"},
+	{"INTC106A"},
+	{"INTC106B"},
+	{"INTC106C"},
+	{"INTC106D"},
 	{"INTC10A0"},
 	{"INTC10A1"},
 	{"INTC10A2"},
diff --git a/drivers/acpi/fan.h b/drivers/acpi/fan.h
index e7b4b4e4a55e..f89d19c922dc 100644
--- a/drivers/acpi/fan.h
+++ b/drivers/acpi/fan.h
@@ -15,6 +15,7 @@
 	{"INTC1044", }, /* Fan for Tiger Lake generation */ \
 	{"INTC1048", }, /* Fan for Alder Lake generation */ \
 	{"INTC1063", }, /* Fan for Meteor Lake generation */ \
+	{"INTC106A", }, /* Fan for Lunar Lake generation */ \
 	{"INTC10A2", }, /* Fan for Raptor Lake generation */ \
 	{"PNP0C0B", } /* Generic ACPI fan */
 
diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
index 427d370648d5..f8ebdd19d340 100644
--- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
+++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
@@ -705,6 +705,7 @@ static const struct acpi_device_id int3400_thermal_match[] = {
 	{"INTC1040", 0},
 	{"INTC1041", 0},
 	{"INTC1042", 0},
+	{"INTC1068", 0},
 	{"INTC10A0", 0},
 	{}
 };
diff --git a/drivers/thermal/intel/int340x_thermal/int3403_thermal.c b/drivers/thermal/intel/int340x_thermal/int3403_thermal.c
index 9b33fd3a66da..86901f9f54d8 100644
--- a/drivers/thermal/intel/int340x_thermal/int3403_thermal.c
+++ b/drivers/thermal/intel/int340x_thermal/int3403_thermal.c
@@ -284,6 +284,7 @@ static const struct acpi_device_id int3403_device_ids[] = {
 	{"INTC1043", 0},
 	{"INTC1046", 0},
 	{"INTC1062", 0},
+	{"INTC1069", 0},
 	{"INTC10A1", 0},
 	{"", 0},
 };
-- 
2.17.1


^ permalink raw reply related

* Re: [regression] 6.8.1: fails to hibernate with pm_runtime_force_suspend+0x0/0x120 returns -16
From: Martin Steigerwald @ 2024-04-05 12:18 UTC (permalink / raw)
  To: linux-pm, regressions, linux-kernel,
	Linux regressions mailing list
In-Reply-To: <12413606.O9o76ZdvQC@lichtvoll.de>

Martin Steigerwald - 05.04.24, 13:54:34 CEST:
> > The plot thickens, now 6.7.12 as compared to 6.7.11 which failed
> > hibernation with busy work queues¹ and 6.7.9 + bcachefs downgrade
> > fixes
> 
> > fails hibernation with the same error message than 6.8.1:
> […]
> 
> > Not doing it today or probably the weekend, but now I have some
> > actionable git bisect plan without bisecting between major kernel
> > releases which as I have been told between 6.7 an 6.8-rc1 can lead to
> > non working modeset graphics on AMD Ryzen in between.
> > 
> > It seems now I only need to git bisect between 6.7.11 and 6.7.12 to
> > find the patch that breaks hibernation on 6.8.1 as well. However
> > first I will briefly check whether 6.8.4 hibernates okay.
> 
> 6.8.4 is still affected and fails hibernation with the same error
> message.

Also this time it is just the ThinkPad T14 AMD Gen 1 with a Lenovo USB
mouse. No second monitor, no USB hub, no nothing else attached.

Should have tested with such a minimal setup to begin with to limit the
test case to a minimum.

-- 
Martin



^ permalink raw reply

* Re: [regression] 6.8.1 and 6.7.12: fails to hibernate with pm_runtime_force_suspend+0x0/0x120 returns -16
From: Martin Steigerwald @ 2024-04-05 12:11 UTC (permalink / raw)
  To: linux-pm, regressions, linux-kernel,
	Linux regressions mailing list
In-Reply-To: <12413606.O9o76ZdvQC@lichtvoll.de>

Martin Steigerwald - 05.04.24, 13:54:34 CEST:
> > Not doing it today or probably the weekend, but now I have some
> > actionable git bisect plan without bisecting between major kernel
> > releases which as I have been told between 6.7 an 6.8-rc1 can lead to
> > non working modeset graphics on AMD Ryzen in between.
> > 
> > It seems now I only need to git bisect between 6.7.11 and 6.7.12 to
> > find the patch that breaks hibernation on 6.8.1 as well. However
> > first I will briefly check whether 6.8.4 hibernates okay.
> 
> 6.8.4 is still affected and fails hibernation with the same error
> message.

Both kernels also fail to reboot the machine. Runit says:

- runit: system reboot

And then nothing anymore.

It is not just hibernation.

I think ThinkPad T14 AMD Gen 1 or similar systems with Linux are not that 
rare. I am surprised this is not hitting more people. Well maybe it does 
and no one reported it here.

Well let's see what happens when 6.7.12 hits distribution kernels.

Anyway, I have an actionable git bisect between 6.7.11 and 6.7.12, just 
not today. Maybe beginning of next week.

-- 
Martin



^ permalink raw reply

* Re: [regression] 6.8.1: fails to hibernate with pm_runtime_force_suspend+0x0/0x120 returns -16
From: Martin Steigerwald @ 2024-04-05 11:54 UTC (permalink / raw)
  To: linux-pm, regressions, linux-kernel,
	Linux regressions mailing list
In-Reply-To: <6034738.lOV4Wx5bFT@lichtvoll.de>

Martin Steigerwald - 05.04.24, 13:49:00 CEST:
> Linux regression tracking (Thorsten Leemhuis) - 19.03.24, 09:40:06 CEST:
> > On 16.03.24 17:12, Martin Steigerwald wrote:
> > > Martin Steigerwald - 16.03.24, 17:02:44 CET:
> > >> ThinkPad T14 AMD Gen 1 fails to hibernate with self-compiled 6.8.1.
> > >> Hibernation works correctly with self-compiled 6.7.9.
> > > 
> > > Apparently 6.8.1 does not even reboot correctly anymore. runit on
> > > Devuan. It says it is doing the system reboot but then nothing
> > > happens.
> > > 
> > > As for hibernation the kernel cancels the attempt and returns back
> > > to
> > > user space desktop session.
> > > 
> > >> Trying to use "no_console_suspend" to debug next. Will not do
> > >> bisect
> > >> between major kernel releases on a production machine.
> > 
> > FWIW, without a bisection I guess no developer will take a closer look
> > (but I might be wrong and you lucky here!), as any change in those
> > hundreds of drivers used on that machine can possibly lead to problems
> > like yours. So without a bisection we are likely stuck here, unless
> > someone else runs into the same problem and bisects or fixes it.
> > Sorry, but that's just how it is.
> 
> The plot thickens, now 6.7.12 as compared to 6.7.11 which failed
> hibernation with busy work queues¹ and 6.7.9 + bcachefs downgrade fixes
> fails hibernation with the same error message than 6.8.1:
[…]
> Not doing it today or probably the weekend, but now I have some
> actionable git bisect plan without bisecting between major kernel
> releases which as I have been told between 6.7 an 6.8-rc1 can lead to
> non working modeset graphics on AMD Ryzen in between.
> 
> It seems now I only need to git bisect between 6.7.11 and 6.7.12 to find
> the patch that breaks hibernation on 6.8.1 as well. However first I
> will briefly check whether 6.8.4 hibernates okay.

6.8.4 is still affected and fails hibernation with the same error message.

> [1] * 6.7.11: Fails to hibernate - work queues still busy
> @ 2024-04-02 19:29 Martin Steigerwald
> 
> https://lore.kernel.org/regressions/2ad93b57-8fdc-476e-83b7-2c0af1cfd41d
> @leemhuis.info/T/#t

-- 
Martin



^ permalink raw reply

* Re: [regression] 6.8.1: fails to hibernate with pm_runtime_force_suspend+0x0/0x120 returns -16
From: Martin Steigerwald @ 2024-04-05 11:49 UTC (permalink / raw)
  To: linux-pm, regressions, linux-kernel,
	Linux regressions mailing list
In-Reply-To: <be5a7213-78b3-4917-b95b-ec31cd2350e4@leemhuis.info>

Linux regression tracking (Thorsten Leemhuis) - 19.03.24, 09:40:06 CEST:
> On 16.03.24 17:12, Martin Steigerwald wrote:
> > Martin Steigerwald - 16.03.24, 17:02:44 CET:
> >> ThinkPad T14 AMD Gen 1 fails to hibernate with self-compiled 6.8.1.
> >> Hibernation works correctly with self-compiled 6.7.9.
> > 
> > Apparently 6.8.1 does not even reboot correctly anymore. runit on
> > Devuan. It says it is doing the system reboot but then nothing
> > happens.
> > 
> > As for hibernation the kernel cancels the attempt and returns back to
> > user space desktop session.
> > 
> >> Trying to use "no_console_suspend" to debug next. Will not do bisect
> >> between major kernel releases on a production machine.
> 
> FWIW, without a bisection I guess no developer will take a closer look
> (but I might be wrong and you lucky here!), as any change in those
> hundreds of drivers used on that machine can possibly lead to problems
> like yours. So without a bisection we are likely stuck here, unless
> someone else runs into the same problem and bisects or fixes it. Sorry,
> but that's just how it is.

The plot thickens, now 6.7.12 as compared to 6.7.11 which failed hibernation
with busy work queues¹ and 6.7.9 + bcachefs downgrade fixes fails
hibernation with the same error message than 6.8.1:

[   74.330285] PM: hibernation: hibernation entry
[   74.617009] Filesystems sync: 0.097 seconds
[   74.617309] Freezing user space processes
[   74.620021] Freezing user space processes completed (elapsed 0.002 seconds)
[   74.620048] OOM killer disabled.
[   74.620250] PM: hibernation: Marking nosave pages: [mem 0x00000000-0x00000fff]
[   74.620253] PM: hibernation: Marking nosave pages: [mem 0x0009f000-0x000fffff]
[   74.620256] PM: hibernation: Marking nosave pages: [mem 0x09c00000-0x09d00fff]
[   74.620261] PM: hibernation: Marking nosave pages: [mem 0x09f00000-0x09f0ffff]
[   74.620263] PM: hibernation: Marking nosave pages: [mem 0xa22b7000-0xa22b7fff]
[   74.620264] PM: hibernation: Marking nosave pages: [mem 0xa22c4000-0xa22c5fff]
[   74.620266] PM: hibernation: Marking nosave pages: [mem 0xa22d3000-0xa22d4fff]
[   74.620267] PM: hibernation: Marking nosave pages: [mem 0xa22e5000-0xa22e5fff]
[   74.620269] PM: hibernation: Marking nosave pages: [mem 0xb9533000-0xb95c3fff]
[   74.620272] PM: hibernation: Marking nosave pages: [mem 0xbd9de000-0xcc3fdfff]
[   74.620752] PM: hibernation: Marking nosave pages: [mem 0xce000000-0xffffffff]
[   74.622717] PM: hibernation: Basic memory bitmaps created
[   74.629269] PM: hibernation: Preallocating image memory
[   76.658495] PM: hibernation: Allocated 2042025 pages for snapshot
[   76.658875] PM: hibernation: Allocated 8168100 kbytes in 2.02 seconds (4043.61 MB/s)
[   76.658901] Freezing remaining freezable tasks
[   76.660457] Freezing remaining freezable tasks completed (elapsed 0.001 seconds)
[   76.666115] port 0000:02:00.1:0.0: PM: dpm_run_callback(): pm_runtime_force_suspend+0x0/0x120 returns -16
[   76.666255] port 0000:02:00.1:0.0: PM: failed to freeze: error -16
[   78.616495] psmouse serio1: synaptics: queried max coordinates: x [..5678], y [..4694]
[   78.654349] psmouse serio1: synaptics: queried min coordinates: x [1266..], y [1162..]
[   79.106531] PM: hibernation: Basic memory bitmaps freed
[   79.106855] OOM killer enabled.
[   79.106873] Restarting tasks ... done.
[   79.111514] PM: hibernation: hibernation exit

Not doing it today or probably the weekend, but now I have some actionable
git bisect plan without bisecting between major kernel releases which as I
have been told between 6.7 an 6.8-rc1 can lead to non working modeset
graphics on AMD Ryzen in between.

It seems now I only need to git bisect between 6.7.11 and 6.7.12 to find the
patch that breaks hibernation on 6.8.1 as well. However first I will briefly
check whether 6.8.4 hibernates okay.

[1] * 6.7.11: Fails to hibernate - work queues still busy
@ 2024-04-02 19:29 Martin Steigerwald

https://lore.kernel.org/regressions/2ad93b57-8fdc-476e-83b7-2c0af1cfd41d@leemhuis.info/T/#t

Best,
-- 
Martin



^ permalink raw reply

* [RFC PATCH 3/3] cpufreq: Add Rust based cpufreq-dt driver
From: Viresh Kumar @ 2024-04-05 11:09 UTC (permalink / raw)
  To: Rafael J. Wysocki, Miguel Ojeda, Viresh Kumar, Miguel Ojeda,
	Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl
  Cc: linux-pm, Vincent Guittot, Stephen Boyd, Nishanth Menon,
	rust-for-linux, Manos Pitsidianakis, Erik Schilling,
	Alex Bennée, Joakim Bech, linux-kernel
In-Reply-To: <cover.1712314032.git.viresh.kumar@linaro.org>

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 | 264 +++++++++++++++++++++++++++++++++
 3 files changed, 277 insertions(+)
 create mode 100644 drivers/cpufreq/rcpufreq_dt.rs

diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
index 94e55c40970a..eb9359bd3c5c 100644
--- a/drivers/cpufreq/Kconfig
+++ b/drivers/cpufreq/Kconfig
@@ -217,6 +217,18 @@ config CPUFREQ_DT
 
 	  If in doubt, say N.
 
+config CPUFREQ_DT_RUST
+	tristate "Rust based Generic DT based cpufreq driver"
+	depends on HAVE_CLK && OF && RUST
+	select CPUFREQ_DT_PLATDEV
+	select PM_OPP
+	help
+	  This adds a Rust based generic DT based cpufreq driver for frequency
+	  management.  It supports both uniprocessor (UP) and symmetric
+	  multiprocessor (SMP) systems.
+
+	  If in doubt, say N.
+
 config CPUFREQ_DT_PLATDEV
 	tristate "Generic DT based cpufreq platdev driver"
 	depends on OF
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index 8d141c71b016..4981d908b803 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_CPU_FREQ_GOV_COMMON)		+= cpufreq_governor.o
 obj-$(CONFIG_CPU_FREQ_GOV_ATTR_SET)	+= cpufreq_governor_attr_set.o
 
 obj-$(CONFIG_CPUFREQ_DT)		+= cpufreq-dt.o
+obj-$(CONFIG_CPUFREQ_DT_RUST)		+= rcpufreq_dt.o
 obj-$(CONFIG_CPUFREQ_DT_PLATDEV)	+= cpufreq-dt-platdev.o
 
 # Traces
diff --git a/drivers/cpufreq/rcpufreq_dt.rs b/drivers/cpufreq/rcpufreq_dt.rs
new file mode 100644
index 000000000000..d29182eba75e
--- /dev/null
+++ b/drivers/cpufreq/rcpufreq_dt.rs
@@ -0,0 +1,264 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Rust based implementation of the cpufreq-dt driver.
+
+use core::{format_args, ptr};
+
+use kernel::{
+    b_str, bindings, c_str, cpufreq, define_of_id_table,
+    device::{self, Device, RawDevice},
+    error::{code::*, to_result},
+    fmt,
+    macros::vtable,
+    module_platform_driver, of, opp, platform,
+    prelude::*,
+    str::CString,
+    sync::Arc,
+};
+
+// Finds exact supply name from the OF node.
+fn find_supply_name_exact(np: *mut bindings::device_node, name: &str) -> Option<CString> {
+    let sname = CString::try_from_fmt(fmt!("{}-supply", name)).ok()?;
+
+    // SAFETY: The OF node is guaranteed by the C code to be valid.
+    let pp = unsafe { bindings::of_find_property(np, sname.as_ptr() as *mut _, ptr::null_mut()) };
+    if pp.is_null() {
+        None
+    } else {
+        CString::try_from_fmt(fmt!("{}", name)).ok()
+    }
+}
+
+// Finds supply name for the CPU from DT.
+fn find_supply_names(dev: &Device, cpu: u32) -> Option<Vec<CString>> {
+    // SAFETY: The requirements are satisfied by the existence of `RawDevice` and its safety
+    // requirements. The OF node is guaranteed by the C code to be valid or NULL and
+    // `of_node_get()` handles both the cases safely.
+    let np = unsafe { bindings::of_node_get((*dev.raw_device()).of_node) };
+    if np.is_null() {
+        return None;
+    }
+
+    // Try "cpu0" for older DTs.
+    let mut name = if cpu == 0 {
+        find_supply_name_exact(np, "cpu0")
+    } else {
+        None
+    };
+
+    if name.is_none() {
+        name = find_supply_name_exact(np, "cpu");
+    }
+
+    // SAFETY: It is safe to drop reference to the OF node we just took.
+    unsafe { bindings::of_node_put(np) };
+
+    match name {
+        Some(name) => {
+            let mut list = Vec::try_with_capacity(1).ok()?;
+            list.try_push(name).ok()?;
+
+            // SAFETY: The slice is fully initialized now.
+            Some(list)
+        }
+
+        None => None,
+    }
+}
+
+// Represents the cpufreq dt device.
+struct CPUFreqDTDevice {
+    opp_table: opp::Table,
+    freq_table: opp::FreqTable,
+    #[allow(dead_code)]
+    config: Option<opp::Config<Self>>,
+    #[allow(dead_code)]
+    clk: cpufreq::Clk,
+}
+
+#[vtable]
+impl opp::ConfigOps for CPUFreqDTDevice {}
+
+#[vtable]
+impl cpufreq::DriverOps for CPUFreqDTDevice {
+    type PData = Arc<Self>;
+
+    fn init(policy: &mut cpufreq::Policy) -> Result<Self::PData> {
+        let cpu = policy.cpu();
+
+        // SAFETY: It is safe to call `get_cpu_device()` for any CPU.
+        let dev = unsafe { bindings::get_cpu_device(cpu) };
+        if dev.is_null() {
+            return Err(ENODEV);
+        }
+
+        // SAFETY: `dev` is valid, non-null, and has a non-zero reference count.
+        let dev = unsafe { Device::new(dev) };
+
+        policy.set_cpus(cpu);
+
+        let config = if let Some(names) = find_supply_names(&dev, cpu) {
+            let mut config = opp::Config::<Self>::new();
+            config.set_regulator_names(names)?;
+            config.set(&dev)?;
+            Some(config)
+        } else {
+            None
+        };
+
+        // Get OPP-sharing information from "operating-points-v2" bindings.
+        let fallback = match opp::Table::of_sharing_cpus(&dev, policy.cpus()) {
+            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, policy.cpus()).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, policy.cpus()) {
+            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 {
+            policy.set_all_cpus();
+            opp_table.set_sharing_cpus(policy.cpus())?;
+        }
+
+        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()?;
+        policy.set_freq_table(freq_table.table());
+        policy.set_dvfs_possible_from_any_cpu();
+        policy.set_suspend_freq((opp_table.suspend_freq() / 1000) as u32);
+        policy.set_transition_latency(transition_latency);
+        let clk = policy.set_clk(&dev, None)?;
+
+        Ok(Arc::try_new(CPUFreqDTDevice {
+            opp_table,
+            config,
+            freq_table,
+            clk,
+        })?)
+    }
+
+    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<()> {
+        // 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(policy.as_ptr()) })
+    }
+
+    fn verify(data: &mut cpufreq::PolicyData) -> Result<()> {
+        // SAFETY: By the type invariants, we know that `data` owns a reference received from the C
+        // code, so it is safe to use it now.
+        to_result(unsafe { bindings::cpufreq_generic_frequency_table_verify(data.as_ptr()) })
+    }
+
+    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 u64;
+        data.opp_table.set_rate(freq * 1000)
+    }
+
+    fn get(policy: &mut cpufreq::Policy) -> Result<u32> {
+        // SAFETY: It is safe to call `cpufreq_generic_get()` for policy's CPU.
+        Ok(unsafe { bindings::cpufreq_generic_get(policy.cpu()) })
+    }
+
+    fn set_boost(_policy: &mut cpufreq::Policy, _state: i32) -> Result<()> {
+        Ok(())
+    }
+
+    fn register_em(policy: &mut cpufreq::Policy) {
+        // 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(policy.as_ptr()) };
+    }
+}
+
+type DeviceData = device::Data<cpufreq::Registration<CPUFreqDTDevice>, (), ()>;
+
+struct CPUFreqDTDriver;
+
+impl platform::Driver for CPUFreqDTDriver {
+    type Data = Arc<DeviceData>;
+
+    define_of_id_table! {(), [
+        (of::DeviceId::Compatible(b_str!("operating-points-v2")), None),
+    ]}
+
+    fn probe(_dev: &mut platform::Device, _id_info: Option<&Self::IdInfo>) -> Result<Self::Data> {
+        let data = Arc::<DeviceData>::from(kernel::new_device_data!(
+            cpufreq::Registration::new(),
+            (),
+            (),
+            "CPUFreqDT::Registration"
+        )?);
+        let flags = cpufreq::flags::NEED_INITIAL_FREQ_CHECK | cpufreq::flags::IS_COOLING_DEV;
+        let boost = true;
+
+        data.registrations()
+            .ok_or(ENXIO)?
+            .as_pinned_mut()
+            .register(c_str!("cpufreq-dt"), (), flags, boost)?;
+
+        pr_info!("CPUFreq DT driver registered\n");
+
+        Ok(data)
+    }
+}
+
+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


^ permalink raw reply related

* [RFC PATCH 2/3] rust: Add bindings for cpufreq framework
From: Viresh Kumar @ 2024-04-05 11:09 UTC (permalink / raw)
  To: Rafael J. Wysocki, Miguel Ojeda, Miguel Ojeda, Alex Gaynor,
	Wedson Almeida Filho, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl
  Cc: Viresh Kumar, linux-pm, Vincent Guittot, Stephen Boyd,
	Nishanth Menon, rust-for-linux, Manos Pitsidianakis,
	Erik Schilling, Alex Bennée, Joakim Bech, linux-kernel
In-Reply-To: <cover.1712314032.git.viresh.kumar@linaro.org>

This commit adds Rust bindings for the cpufreq core. The current
implementation doesn't implement Rust wrappers for all the APIs, but
mostly the ones usable by the cpufreq-dt driver.

The missing APIs will be added later once required.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 rust/bindings/bindings_helper.h |    1 +
 rust/helpers.c                  |   15 +
 rust/kernel/cpufreq.rs          | 1090 +++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |    2 +
 4 files changed, 1108 insertions(+)
 create mode 100644 rust/kernel/cpufreq.rs

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 370043838a54..b88eb1a14eee 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -9,6 +9,7 @@
 #include <kunit/test.h>
 #include <linux/cred.h>
 #include <linux/cpu.h>
+#include <linux/cpufreq.h>
 #include <linux/device.h>
 #include <linux/errname.h>
 #include <linux/ethtool.h>
diff --git a/rust/helpers.c b/rust/helpers.c
index 05dac59941e4..a2108ac89f5c 100644
--- a/rust/helpers.c
+++ b/rust/helpers.c
@@ -24,6 +24,7 @@
 #include <linux/bug.h>
 #include <linux/build_bug.h>
 #include <linux/cpumask.h>
+#include <linux/cpufreq.h>
 #include <linux/cred.h>
 #include <linux/device.h>
 #include <linux/err.h>
@@ -315,6 +316,20 @@ void rust_helper_cpumask_setall(struct cpumask *dstp)
 }
 EXPORT_SYMBOL_GPL(rust_helper_cpumask_setall);
 
+#ifdef CONFIG_CPU_FREQ
+unsigned int rust_helper_cpufreq_table_len(struct cpufreq_frequency_table *freq_table)
+{
+	return cpufreq_table_len(freq_table);
+}
+EXPORT_SYMBOL_GPL(rust_helper_cpufreq_table_len);
+
+void rust_helper_cpufreq_register_em_with_opp(struct cpufreq_policy *policy)
+{
+	cpufreq_register_em_with_opp(policy);
+}
+EXPORT_SYMBOL_GPL(rust_helper_cpufreq_register_em_with_opp);
+#endif
+
 #ifndef CONFIG_OF_DYNAMIC
 struct device_node *rust_helper_of_node_get(struct device_node *node)
 {
diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
new file mode 100644
index 000000000000..4c9a7534179c
--- /dev/null
+++ b/rust/kernel/cpufreq.rs
@@ -0,0 +1,1090 @@
+// 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`](../../../../../../include/linux/cpufreq.h)
+
+use crate::{
+    bindings,
+    device::{Device, RawDevice},
+    error::{code::*, from_err_ptr, from_result, to_result, Result, VTABLE_DEFAULT_ERROR},
+    prelude::*,
+    types::ForeignOwnable,
+};
+
+use core::{
+    cell::UnsafeCell,
+    marker::{PhantomData, PhantomPinned},
+    pin::Pin,
+    ptr::{self, addr_of_mut},
+};
+
+use macros::vtable;
+
+/// 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_ptr(ptr: *mut bindings::cpufreq_policy_data) -> Self {
+        Self(ptr)
+    }
+
+    /// Returns the raw pointer to the C structure.
+    pub fn as_ptr(&self) -> *mut bindings::cpufreq_policy_data {
+        self.0
+    }
+}
+
+/// Builder for the `struct cpufreq_frequency_table` in the C code.
+#[repr(transparent)]
+pub struct TableBuilder {
+    entries: Vec<bindings::cpufreq_frequency_table>,
+}
+
+impl TableBuilder {
+    /// Creates new instance of [`TableBuilder`].
+    pub fn new() -> Self {
+        Self {
+            entries: Vec::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.try_push(bindings::cpufreq_frequency_table {
+            flags,
+            driver_data,
+            frequency,
+        })?)
+    }
+
+    /// 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<Vec<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: Vec<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.
+    pub fn as_ptr(&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 })
+    }
+}
+
+/// A simple implementation of `struct clk` from the C code.
+#[repr(transparent)]
+pub struct Clk(*mut bindings::clk);
+
+impl Clk {
+    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.raw_device(), con_id)
+        })?))
+    }
+
+    fn as_ptr(&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) };
+    }
+}
+
+/// A simple implementation of `struct cpumask` from the C code.
+#[repr(transparent)]
+pub struct Cpumask(*mut bindings::cpumask);
+
+impl Cpumask {
+    /// Creates cpumask from raw pointer.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that `ptr` is valid, and non-null.
+    pub unsafe fn new(ptr: *mut bindings::cpumask) -> Self {
+        Self(ptr)
+    }
+
+    /// Returns pointer to the underlying cpumask from the C code.
+    pub fn as_ptr(&self) -> *const bindings::cpumask {
+        self.0
+    }
+
+    /// Returns mutable pointer to the underlying cpumask from the C code.
+    pub fn as_mut_ptr(&mut self) -> *mut bindings::cpumask {
+        self.0
+    }
+}
+
+/// Equivalent to `struct cpufreq_policy` in the C code.
+pub struct Policy {
+    ptr: *mut bindings::cpufreq_policy,
+    put_cpu: bool,
+    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_ptr(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::new((*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_ptr(ptr) };
+        policy.put_cpu = true;
+        Ok(policy)
+    }
+
+    /// Raw pointer to the underlying cpufreq policy.
+    pub fn as_ptr(&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
+    }
+
+    /// Returns the maximum frequency for a cpufreq policy.
+    pub fn max(&self) -> u32 {
+        self.as_ref().max
+    }
+
+    /// 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) {
+        self.as_mut_ref().suspend_freq = freq;
+    }
+
+    /// Returns the suspend frequency for a cpufreq policy.
+    pub fn suspend_freq(&self) -> u32 {
+        self.as_ref().suspend_freq
+    }
+
+    /// Gets raw pointer to cpufreq policy's CPUs mask.
+    pub fn cpus(&mut self) -> &mut Cpumask {
+        &mut self.cpumask
+    }
+
+    /// Sets CPUs mask for a cpufreq policy.
+    ///
+    /// Update the `cpus` mask with a single CPU.
+    pub fn set_cpus(&mut self, cpu: u32) {
+        // SAFETY: The `cpus` pointer 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.cpus().as_mut_ptr()) };
+    }
+
+    /// Sets CPUs mask for a cpufreq policy.
+    ///
+    /// Update the `cpus` mask with a single CPU if `cpu` is set to `Some(cpu)`, else sets all
+    /// CPUs.
+    pub fn set_all_cpus(&mut self) {
+        // SAFETY: The `cpus` pointer is guaranteed to be valid for the lifetime of `self`. And it
+        // is safe to call `cpumask_setall()`.
+        unsafe { bindings::cpumask_setall(self.cpus().as_mut_ptr()) };
+    }
+
+    /// Sets clock for a cpufreq policy.
+    pub fn set_clk(&mut self, dev: &Device, name: Option<&CStr>) -> Result<Clk> {
+        let clk = Clk::new(dev, name)?;
+        self.as_mut_ref().clk = clk.as_ptr();
+        Ok(clk)
+    }
+
+    /// Allows frequency switching code to run on any CPU.
+    pub fn set_dvfs_possible_from_any_cpu(&mut self) {
+        self.as_mut_ref().dvfs_possible_from_any_cpu = true;
+    }
+
+    /// Sets transition latency for a cpufreq policy.
+    pub fn set_transition_latency(&mut self, latency: u32) {
+        self.as_mut_ref().cpuinfo.transition_latency = latency;
+    }
+
+    /// 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 == ptr::null_mut() {
+            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) {
+        self.as_mut_ref().freq_table = table.as_ptr();
+    }
+
+    /// 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 {
+            // 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.
+            let data =
+                Some(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_ptr()) };
+        }
+    }
+}
+
+/// Operations to be implemented by a cpufreq driver.
+#[vtable]
+pub trait DriverOps {
+    /// 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: u64, _target_perf: u64, _capacity: u64) {
+        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)
+    }
+}
+
+/// Registration of a cpufreq driver.
+pub struct Registration<T: DriverOps> {
+    registered: bool,
+    drv: UnsafeCell<bindings::cpufreq_driver>,
+    _p: PhantomData<T>,
+    _pin: PhantomPinned,
+}
+
+// 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: DriverOps> Sync for Registration<T> {}
+
+// 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.
+#[allow(clippy::non_send_fields_in_send_ty)]
+unsafe impl<T: DriverOps> Send for Registration<T> {}
+
+impl<T: DriverOps> Default for Registration<T> {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+impl<T: DriverOps> Registration<T> {
+    /// Creates new [`Registration`] but does not register it yet.
+    ///
+    /// It is allowed to move.
+    pub fn new() -> Self {
+        Self {
+            registered: false,
+            drv: UnsafeCell::new(bindings::cpufreq_driver::default()),
+            _pin: PhantomPinned,
+            _p: PhantomData,
+        }
+    }
+
+    /// Registers a cpufreq driver with the rest of the kernel.
+    pub fn register(
+        self: Pin<&mut Self>,
+        name: &'static CStr,
+        data: T::Data,
+        flags: u16,
+        boost: bool,
+    ) -> Result {
+        // SAFETY: We never move out of `this`.
+        let this = unsafe { self.get_unchecked_mut() };
+
+        if this.registered {
+            return Err(EINVAL);
+        }
+
+        let drv = this.drv.get_mut();
+
+        // Account for the trailing null character.
+        let len = name.len() + 1;
+        if len > drv.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] as *const [i8]) };
+        drv.name[..len].copy_from_slice(name);
+
+        drv.boost_enabled = boost;
+        drv.flags = flags;
+
+        // Allocate an array of 3 pointers to be passed to the C code.
+        let mut attr = Box::try_new([ptr::null_mut(); 3])?;
+        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.attr = Box::leak(attr) as *mut _;
+
+        // Initialize mandatory callbacks.
+        drv.init = Some(Self::init_callback);
+        drv.verify = Some(Self::verify_callback);
+
+        // Initialize optional callbacks.
+        drv.setpolicy = if T::HAS_SETPOLICY {
+            Some(Self::setpolicy_callback)
+        } else {
+            None
+        };
+        drv.target = if T::HAS_TARGET {
+            Some(Self::target_callback)
+        } else {
+            None
+        };
+        drv.target_index = if T::HAS_TARGET_INDEX {
+            Some(Self::target_index_callback)
+        } else {
+            None
+        };
+        drv.fast_switch = if T::HAS_FAST_SWITCH {
+            Some(Self::fast_switch_callback)
+        } else {
+            None
+        };
+        drv.adjust_perf = if T::HAS_ADJUST_PERF {
+            Some(Self::adjust_perf_callback)
+        } else {
+            None
+        };
+        drv.get_intermediate = if T::HAS_GET_INTERMEDIATE {
+            Some(Self::get_intermediate_callback)
+        } else {
+            None
+        };
+        drv.target_intermediate = if T::HAS_TARGET_INTERMEDIATE {
+            Some(Self::target_intermediate_callback)
+        } else {
+            None
+        };
+        drv.get = if T::HAS_GET {
+            Some(Self::get_callback)
+        } else {
+            None
+        };
+        drv.update_limits = if T::HAS_UPDATE_LIMITS {
+            Some(Self::update_limits_callback)
+        } else {
+            None
+        };
+        drv.bios_limit = if T::HAS_BIOS_LIMIT {
+            Some(Self::bios_limit_callback)
+        } else {
+            None
+        };
+        drv.online = if T::HAS_ONLINE {
+            Some(Self::online_callback)
+        } else {
+            None
+        };
+        drv.offline = if T::HAS_OFFLINE {
+            Some(Self::offline_callback)
+        } else {
+            None
+        };
+        drv.exit = if T::HAS_EXIT {
+            Some(Self::exit_callback)
+        } else {
+            None
+        };
+        drv.suspend = if T::HAS_SUSPEND {
+            Some(Self::suspend_callback)
+        } else {
+            None
+        };
+        drv.resume = if T::HAS_RESUME {
+            Some(Self::resume_callback)
+        } else {
+            None
+        };
+        drv.ready = if T::HAS_READY {
+            Some(Self::ready_callback)
+        } else {
+            None
+        };
+        drv.set_boost = if T::HAS_SET_BOOST {
+            Some(Self::set_boost_callback)
+        } else {
+            None
+        };
+        drv.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.
+        this.set_data(data)?;
+
+        // SAFETY: It is safe to register the driver with the cpufreq core in the C code.
+        to_result(unsafe { bindings::cpufreq_register_driver(this.drv.get_mut()) })?;
+
+        this.registered = true;
+        Ok(())
+    }
+
+    /// Returns the previous set data for a cpufreq driver.
+    pub fn data<D: ForeignOwnable>() -> Option<<D>::Borrowed<'static>> {
+        // SAFETY: The driver data is earlier set by us from [`set_data()`].
+        let data = unsafe { bindings::cpufreq_get_driver_data() };
+        if data.is_null() {
+            None
+        } else {
+            // SAFETY: The driver data is earlier set by us from [`set_data()`].
+            Some(unsafe { D::borrow(data) })
+        }
+    }
+
+    // Sets the data for a cpufreq driver.
+    fn set_data(&mut self, data: T::Data) -> Result<()> {
+        let drv = self.drv.get_mut();
+
+        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)
+        }
+    }
+
+    // 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: DriverOps> 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_ptr(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) -> 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_ptr(ptr) };
+
+            let data = policy.clear_data();
+            T::exit(&mut policy, data).map(|_| 0)
+        })
+    }
+
+    // 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_ptr(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_ptr(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_ptr(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_ptr(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_ptr(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_ptr(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_ptr(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_ptr(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_ptr(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_ptr(ptr) };
+        T::fast_switch(&mut policy, target_freq)
+    }
+
+    // Policy's adjust_perf callback.
+    extern "C" fn adjust_perf_callback(cpu: u32, min_perf: u64, target_perf: u64, capacity: u64) {
+        if let Some(mut policy) = Policy::from_cpu(cpu).ok() {
+            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_ptr(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_ptr(ptr) };
+            T::target_intermediate(&mut policy, index).map(|_| 0)
+        })
+    }
+
+    // Policy's get callback.
+    extern "C" fn get_callback(cpu: u32) -> core::ffi::c_uint {
+        // SAFETY: Get the policy for a CPU.
+        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) {
+        // SAFETY: Get the policy for a CPU.
+        if let Some(mut policy) = Policy::from_cpu(cpu).ok() {
+            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_ptr(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_ptr(ptr) };
+        T::register_em(&mut policy);
+    }
+}
+
+impl<T: DriverOps> Drop for Registration<T> {
+    // Removes the registration from the kernel if it has completed successfully before.
+    fn drop(&mut self) {
+        let drv = self.drv.get_mut();
+
+        if self.registered {
+            // SAFETY: The driver was earlier registered from `register()`.
+            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 `Box::leak`.
+            unsafe { drop(Box::from_raw(drv.attr)) };
+        }
+
+        // Free data
+        drop(self.clear_data());
+    }
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 56b666f466a0..1c2f8903577a 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -34,6 +34,8 @@
 #[cfg(not(testlib))]
 mod allocator;
 mod build_assert;
+#[cfg(CONFIG_CPU_FREQ)]
+pub mod cpufreq;
 pub mod cred;
 pub mod device;
 pub mod driver;
-- 
2.31.1.272.g89b43f80a514


^ permalink raw reply related

* [RFC PATCH 1/3] rust: Add bindings for OPP framework
From: Viresh Kumar @ 2024-04-05 11:09 UTC (permalink / raw)
  To: Rafael J. Wysocki, Miguel Ojeda, Miguel Ojeda, Alex Gaynor,
	Wedson Almeida Filho, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl
  Cc: Viresh Kumar, linux-pm, Vincent Guittot, Stephen Boyd,
	Nishanth Menon, rust-for-linux, Manos Pitsidianakis,
	Erik Schilling, Alex Bennée, Joakim Bech, linux-kernel
In-Reply-To: <cover.1712314032.git.viresh.kumar@linaro.org>

This commit adds Rust bindings for the Operating performance points
(OPP) core. The current implementation doesn't implement Rust wrappers
for all the APIs, but mostly the ones usable by the cpufreq-dt driver.

The missing APIs will be added later once required.

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

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index baabff999bd5..370043838a54 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -22,6 +22,7 @@
 #include <linux/platform_device.h>
 #include <linux/phy.h>
 #include <linux/pid_namespace.h>
+#include <linux/pm_opp.h>
 #include <linux/poll.h>
 #include <linux/refcount.h>
 #include <linux/sched.h>
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index a153336973e6..56b666f466a0 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -46,6 +46,8 @@
 #[cfg(CONFIG_NET)]
 pub mod net;
 pub mod of;
+#[cfg(CONFIG_PM_OPP)]
+pub mod opp;
 pub mod platform;
 pub mod prelude;
 pub mod print;
diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
new file mode 100644
index 000000000000..dba4b8592ac0
--- /dev/null
+++ b/rust/kernel/opp.rs
@@ -0,0 +1,895 @@
+// 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`](../../../../../../include/linux/pm_opp.h)
+
+use crate::{
+    bindings, cpufreq,
+    device::{Device, RawDevice},
+    error::{code::*, from_err_ptr, from_result, to_result, Error, Result, VTABLE_DEFAULT_ERROR},
+    prelude::*,
+    str::CString,
+};
+
+use core::{ffi::c_char, marker::PhantomData, ops::Deref, ptr};
+
+use macros::vtable;
+
+// Creates a pinned null-terminated slice of pointers to Cstrings.
+fn to_c_str_array(names: &Pin<Vec<CString>>) -> Result<Vec<*const c_char>> {
+    // Allocated a null-terminated vector of pointers.
+    let mut list = Vec::try_with_capacity(names.len() + 1)?;
+
+    for name in names.iter() {
+        list.try_push(name.as_ptr() as _)?;
+    }
+
+    list.try_push(ptr::null())?;
+    Ok(list)
+}
+
+/// 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: u64, u_volt: u64, level: u32, turbo: bool) -> Self {
+        Self(bindings::dev_pm_opp_data {
+            turbo,
+            freq,
+            u_volt,
+            level,
+        })
+    }
+}
+
+/// 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,
+}
+
+/// 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)
+    }
+}
+
+/// Equivalent to `struct dev_pm_opp_config` in the C Code.
+pub struct Config<T: ConfigOps> {
+    token: Option<i32>,
+    clk_names: Option<Pin<Vec<CString>>>,
+    prop_name: Option<Pin<CString>>,
+    regulator_names: Option<Pin<Vec<CString>>>,
+    genpd_names: Option<Pin<Vec<CString>>>,
+    supported_hw: Option<Pin<Vec<u32>>>,
+    required_devs: Option<Pin<Vec<Device>>>,
+    _data: PhantomData<T>,
+}
+
+impl<T: ConfigOps> Config<T> {
+    /// Creates a new instance of [`Config`].
+    pub fn new() -> Self {
+        Self {
+            token: None,
+            clk_names: None,
+            prop_name: None,
+            regulator_names: None,
+            genpd_names: None,
+            supported_hw: None,
+            required_devs: None,
+            _data: PhantomData,
+        }
+    }
+
+    /// Initializes clock names.
+    pub fn set_clk_names(&mut self, names: Vec<CString>) -> Result<()> {
+        // Already configured.
+        if self.token.is_some() {
+            return Err(EBUSY);
+        }
+
+        // Already configured.
+        if self.clk_names.is_some() {
+            return Err(EBUSY);
+        }
+
+        if names.is_empty() {
+            return Err(EINVAL);
+        }
+
+        self.clk_names = Some(Pin::new(names));
+        Ok(())
+    }
+
+    /// Initializes property name.
+    pub fn set_prop_name(&mut self, name: CString) -> Result<()> {
+        // Already configured.
+        if self.token.is_some() {
+            return Err(EBUSY);
+        }
+
+        // Already configured.
+        if self.prop_name.is_some() {
+            return Err(EBUSY);
+        }
+
+        self.prop_name = Some(Pin::new(name));
+        Ok(())
+    }
+
+    /// Initializes regulator names.
+    pub fn set_regulator_names(&mut self, names: Vec<CString>) -> Result<()> {
+        // Already configured.
+        if self.token.is_some() {
+            return Err(EBUSY);
+        }
+
+        // Already configured.
+        if self.regulator_names.is_some() {
+            return Err(EBUSY);
+        }
+
+        if names.is_empty() {
+            return Err(EINVAL);
+        }
+
+        self.regulator_names = Some(Pin::new(names));
+
+        Ok(())
+    }
+
+    /// Initializes genpd names.
+    pub fn set_genpd_names(&mut self, names: Vec<CString>) -> Result<()> {
+        // Already configured.
+        if self.token.is_some() {
+            return Err(EBUSY);
+        }
+
+        // Already configured. Only one of genpd or required devs can be configured.
+        if self.genpd_names.is_some() || self.required_devs.is_some() {
+            return Err(EBUSY);
+        }
+
+        if names.is_empty() {
+            return Err(EINVAL);
+        }
+
+        self.genpd_names = Some(Pin::new(names));
+        Ok(())
+    }
+
+    /// Initializes required devices.
+    pub fn set_required_devs(&mut self, devs: Vec<Device>) -> Result<()> {
+        // Already configured.
+        if self.token.is_some() {
+            return Err(EBUSY);
+        }
+
+        // Already configured. Only one of genpd or required devs can be configured.
+        if self.genpd_names.is_some() || self.required_devs.is_some() {
+            return Err(EBUSY);
+        }
+
+        if devs.is_empty() {
+            return Err(EINVAL);
+        }
+
+        self.required_devs = Some(Pin::new(devs));
+        Ok(())
+    }
+
+    /// Initializes supported hardware.
+    pub fn set_supported_hw(&mut self, hw: Vec<u32>) -> Result<()> {
+        // Already configured.
+        if self.token.is_some() {
+            return Err(EBUSY);
+        }
+
+        // Already configured.
+        if self.supported_hw.is_some() {
+            return Err(EBUSY);
+        }
+
+        if hw.is_empty() {
+            return Err(EINVAL);
+        }
+
+        self.supported_hw = Some(Pin::new(hw));
+        Ok(())
+    }
+
+    /// Sets the configuration with the OPP core.
+    pub fn set(&mut self, dev: &Device) -> Result<()> {
+        // Already configured.
+        if self.token.is_some() {
+            return Err(EBUSY);
+        }
+
+        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 (_genpd_list, genpd_names) = match &self.genpd_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_devs_list, required_devs) = match &self.required_devs {
+            Some(x) => {
+                // Create a non-NULL-terminated vectorof pointers.
+                let mut list = Vec::try_with_capacity(x.len())?;
+
+                for dev in x.iter() {
+                    list.try_push(dev.raw_device())?;
+                }
+
+                let ptr = list.as_mut_ptr();
+                (Some(list), ptr)
+            }
+            None => (None, ptr::null_mut()),
+        };
+
+        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
+            },
+            genpd_names,
+            supported_hw,
+            supported_hw_count,
+
+            // Don't need to support virt_devs for now.
+            virt_devs: ptr::null_mut(),
+            required_devs,
+        };
+
+        // SAFETY: The requirements are satisfied by the existence of `RawDevice` 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.raw_device(), &mut config) };
+        if ret < 0 {
+            Err(Error::from_errno(ret))
+        } else {
+            self.token = Some(ret);
+            Ok(())
+        }
+    }
+
+    // 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::new(dev) };
+            T::config_clks(
+                &dev,
+                // SAFETY: 'opp_table' is guaranteed by the C code to be valid.
+                &unsafe { Table::from_ptr(opp_table, dev.clone()) },
+                // SAFETY: 'opp' is guaranteed by the C code to be valid.
+                &unsafe { OPP::new(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::new(dev) };
+            T::config_regulators(
+                &dev,
+                // SAFETY: 'old_opp' is guaranteed by the C code to be valid.
+                &unsafe { OPP::new(old_opp) },
+                // SAFETY: 'new_opp' is guaranteed by the C code to be valid.
+                &unsafe { OPP::new(new_opp) },
+                regulators,
+                count,
+            )
+            .map(|_| 0)
+        })
+    }
+}
+
+impl<T: ConfigOps> Drop for Config<T> {
+    fn drop(&mut self) {
+        if let Some(token) = self.token.take() {
+            // 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_clear_config(token) };
+        }
+    }
+}
+
+/// CPU Frequency table created from OPP entries.
+pub struct FreqTable {
+    dev: Device,
+    table: cpufreq::Table,
+}
+
+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 `RawDevice` and its safety
+        // requirements.
+        to_result(unsafe {
+            bindings::dev_pm_opp_init_cpufreq_table(table.dev.raw_device(), &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
+    }
+}
+
+impl Deref for FreqTable {
+    type Target = cpufreq::Table;
+
+    #[inline]
+    fn deref(&self) -> &Self::Target {
+        &self.table
+    }
+}
+
+impl Drop for FreqTable {
+    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_free_cpufreq_table(self.dev.raw_device(), &mut self.as_ptr())
+        };
+    }
+}
+
+/// Operating performance point (OPP) table.
+///
+/// # Invariants
+///
+/// `ptr` is valid, non-null, and has a non-zero reference count. One of the references is owned by
+/// `self`, and will be decremented when `self` is dropped.
+pub struct Table {
+    ptr: *mut bindings::opp_table,
+    dev: Device,
+    em: bool,
+    of: bool,
+    cpumask: Option<cpufreq::Cpumask>,
+}
+
+// SAFETY: The fields of `Table` are safe to be used from any thread.
+unsafe impl Send for Table {}
+
+// SAFETY: The fields of `Table` are safe to be referenced from any thread.
+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_ptr(ptr: *mut bindings::opp_table, dev: 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,
+            em: false,
+            of: false,
+            cpumask: None,
+        }
+    }
+
+    /// Find OPP table from device.
+    pub fn from_dev(dev: &Device) -> Result<Self> {
+        // SAFETY: The requirements are satisfied by the existence of `RawDevice` and its safety
+        // requirements. Refcount of the OPP table is incremented as well.
+        let ptr = from_err_ptr(unsafe { bindings::dev_pm_opp_get_opp_table(dev.raw_device()) })?;
+
+        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: &Device, index: i32) -> Result<Self> {
+        // SAFETY: The requirements are satisfied by the existence of `RawDevice` and its safety
+        // requirements. Refcount of the OPP table is incremented as well.
+        to_result(unsafe { bindings::dev_pm_opp_of_add_table_indexed(dev.raw_device(), 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 `RawDevice` and its safety
+        // requirements. We took the reference from `from_of` earlier, it is safe to drop the same
+        // now.
+        unsafe { bindings::dev_pm_opp_of_remove_table(self.dev.raw_device()) };
+    }
+
+    /// Add device tree based OPP table for CPU devices.
+    #[cfg(CONFIG_OF)]
+    pub fn from_of_cpumask(dev: &Device, cpumask: &mut cpufreq::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_ptr()) })?;
+
+        // Fetch the newly created table.
+        let mut table = Self::from_dev(dev)?;
+        // SAFETY: The `cpumask` is guaranteed by the C code to be valid.
+        table.cpumask = Some(unsafe { cpufreq::Cpumask::new(cpumask.as_mut_ptr()) });
+
+        Ok(table)
+    }
+
+    // Remove device tree based OPP table for CPU devices.
+    #[cfg(CONFIG_OF)]
+    fn remove_of_cpumask(&self, cpumask: &cpufreq::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_ptr()) };
+    }
+
+    /// Returns the number of OPPs in the table.
+    pub fn opp_count(&self) -> Result<u32> {
+        // SAFETY: The requirements are satisfied by the existence of `RawDevice` and its safety
+        // requirements.
+        let ret = unsafe { bindings::dev_pm_opp_get_opp_count(self.dev.raw_device()) };
+        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) -> u64 {
+        // SAFETY: The requirements are satisfied by the existence of `RawDevice` and its safety
+        // requirements.
+        unsafe { bindings::dev_pm_opp_get_max_clock_latency(self.dev.raw_device()) }
+    }
+
+    /// Returns max volt latency of the OPPs in the table.
+    pub fn max_volt_latency(&self) -> u64 {
+        // SAFETY: The requirements are satisfied by the existence of `RawDevice` and its safety
+        // requirements.
+        unsafe { bindings::dev_pm_opp_get_max_volt_latency(self.dev.raw_device()) }
+    }
+
+    /// Returns max transition latency of the OPPs in the table.
+    pub fn max_transition_latency(&self) -> u64 {
+        // SAFETY: The requirements are satisfied by the existence of `RawDevice` and its safety
+        // requirements.
+        unsafe { bindings::dev_pm_opp_get_max_transition_latency(self.dev.raw_device()) }
+    }
+
+    /// Returns the suspend OPP.
+    pub fn suspend_freq(&self) -> u64 {
+        // SAFETY: The requirements are satisfied by the existence of `RawDevice` and its safety
+        // requirements.
+        unsafe { bindings::dev_pm_opp_get_suspend_opp_freq(self.dev.raw_device()) }
+    }
+
+    /// Synchronizes regulators used by the OPP table.
+    pub fn sync_regulators(&self) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `RawDevice` and its safety
+        // requirements.
+        to_result(unsafe { bindings::dev_pm_opp_sync_regulators(self.dev.raw_device()) })
+    }
+
+    /// Gets sharing CPUs.
+    pub fn sharing_cpus(dev: &Device, cpumask: &mut cpufreq::Cpumask) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `RawDevice` and its safety
+        // requirements.
+        to_result(unsafe {
+            bindings::dev_pm_opp_get_sharing_cpus(dev.raw_device(), cpumask.as_mut_ptr())
+        })
+    }
+
+    /// Sets sharing CPUs.
+    pub fn set_sharing_cpus(&self, cpumask: &cpufreq::Cpumask) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `RawDevice` and its safety
+        // requirements.
+        to_result(unsafe {
+            bindings::dev_pm_opp_set_sharing_cpus(self.dev.raw_device(), cpumask.as_ptr())
+        })
+    }
+
+    /// Gets sharing CPUs from Device tree.
+    #[cfg(CONFIG_OF)]
+    pub fn of_sharing_cpus(dev: &Device, cpumask: &mut cpufreq::Cpumask) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `RawDevice` and its safety
+        // requirements.
+        to_result(unsafe {
+            bindings::dev_pm_opp_of_get_sharing_cpus(dev.raw_device(), cpumask.as_mut_ptr())
+        })
+    }
+
+    /// Updates the voltage value for an OPP.
+    pub fn adjust_voltage(
+        &self,
+        freq: u64,
+        u_volt: u64,
+        u_volt_min: u64,
+        u_volt_max: u64,
+    ) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `RawDevice` and its safety
+        // requirements.
+        to_result(unsafe {
+            bindings::dev_pm_opp_adjust_voltage(
+                self.dev.raw_device(),
+                freq,
+                u_volt,
+                u_volt_min,
+                u_volt_max,
+            )
+        })
+    }
+
+    /// Create cpufreq table from OPP table.
+    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: u64) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `RawDevice` and its safety
+        // requirements.
+        to_result(unsafe { bindings::dev_pm_opp_set_rate(self.dev.raw_device(), freq) })
+    }
+
+    /// Sets exact OPP.
+    pub fn set_opp(&self, opp: &OPP) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `RawDevice` and its safety
+        // requirements.
+        to_result(unsafe { bindings::dev_pm_opp_set_opp(self.dev.raw_device(), opp.as_mut_ptr()) })
+    }
+
+    /// Finds OPP based on frequency.
+    pub fn opp_from_freq(
+        &self,
+        mut freq: u64,
+        available: Option<bool>,
+        index: Option<u32>,
+        stype: SearchType,
+    ) -> Result<OPP> {
+        let rdev = self.dev.raw_device();
+        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 `RawDevice` 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 `RawDevice` 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 u64, index)
+            },
+
+            // SAFETY: The requirements are satisfied by the existence of `RawDevice` 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 u64, index)
+            },
+        })?;
+
+        // SAFETY: The `ptr` is guaranteed by the C code to be valid.
+        Ok(unsafe { OPP::new_owned(ptr) })
+    }
+
+    /// Finds OPP based on level.
+    pub fn opp_from_level(&self, mut level: u32, stype: SearchType) -> Result<OPP> {
+        let rdev = self.dev.raw_device();
+
+        let ptr = from_err_ptr(match stype {
+            // SAFETY: The requirements are satisfied by the existence of `RawDevice` 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 `RawDevice` 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 `RawDevice` 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.
+        Ok(unsafe { OPP::new_owned(ptr) })
+    }
+
+    /// Finds OPP based on bandwidth.
+    pub fn opp_from_bw(&self, mut bw: u32, index: i32, stype: SearchType) -> Result<OPP> {
+        let rdev = self.dev.raw_device();
+
+        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 `RawDevice` 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 `RawDevice` 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.
+        Ok(unsafe { OPP::new_owned(ptr) })
+    }
+
+    /// Enable the OPP.
+    pub fn enable_opp(&self, freq: u64) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `RawDevice` and its safety
+        // requirements.
+        to_result(unsafe { bindings::dev_pm_opp_enable(self.dev.raw_device(), freq) })
+    }
+
+    /// Disable the OPP.
+    pub fn disable_opp(&self, freq: u64) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `RawDevice` and its safety
+        // requirements.
+        to_result(unsafe { bindings::dev_pm_opp_disable(self.dev.raw_device(), freq) })
+    }
+
+    /// Registers with Energy model.
+    #[cfg(CONFIG_OF)]
+    pub fn of_register_em(&mut self, cpumask: &mut cpufreq::Cpumask) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `RawDevice` and its safety
+        // requirements.
+        to_result(unsafe {
+            bindings::dev_pm_opp_of_register_em(self.dev.raw_device(), cpumask.as_mut_ptr())
+        })?;
+
+        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 `RawDevice` 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.raw_device()) };
+    }
+}
+
+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 {
+                self.remove_of_cpumask(cpumask);
+            }
+        }
+    }
+}
+
+/// Operating performance point (OPP).
+///
+/// # Invariants
+///
+/// `ptr` is valid, non-null, and has a non-zero reference count. One of the references is owned by
+/// `self`, and will be decremented when `self` is dropped.
+#[repr(transparent)]
+pub struct OPP(*mut bindings::dev_pm_opp);
+
+// SAFETY: `OPP` only holds a pointer to a C OPP, which is safe to be used from any thread.
+unsafe impl Send for OPP {}
+
+// SAFETY: `OPP` only holds a pointer to a C OPP, references to which are safe to be used from any
+// thread.
+unsafe impl Sync for OPP {}
+
+impl OPP {
+    /// Creates a new OPP instance.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that `ptr` is valid, and non-null.
+    unsafe fn new(ptr: *mut bindings::dev_pm_opp) -> Self {
+        // SAFETY: The `ptr` is guaranteed by the C code to be valid.
+        unsafe { bindings::dev_pm_opp_get(ptr) };
+        Self(ptr)
+    }
+
+    /// Creates a new owned OPP instance.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that `ptr` is valid, and non-null.
+    unsafe fn new_owned(ptr: *mut bindings::dev_pm_opp) -> Self {
+        Self(ptr)
+    }
+
+    fn as_mut_ptr(&self) -> *mut bindings::dev_pm_opp {
+        self.0
+    }
+
+    /// Adds an OPP dynamically.
+    pub fn add(dev: &Device, mut data: Data) -> Result<()> {
+        // SAFETY: The requirements are satisfied by the existence of `RawDevice` and its safety
+        // requirements.
+        to_result(unsafe { bindings::dev_pm_opp_add_dynamic(dev.raw_device(), &mut data.0) })
+    }
+
+    /// Removes a dynamically added OPP.
+    pub fn remove(dev: &Device, freq: u64) {
+        // SAFETY: The requirements are satisfied by the existence of `RawDevice` and its safety
+        // requirements.
+        unsafe { bindings::dev_pm_opp_remove(dev.raw_device(), freq) };
+    }
+
+    /// Returns the frequency of an OPP.
+    pub fn freq(&self, index: Option<u32>) -> u64 {
+        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.0, index) }
+    }
+
+    /// Returns the voltage of an OPP.
+    pub fn voltage(&self) -> u64 {
+        // 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.0) }
+    }
+
+    /// 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.0) }
+    }
+
+    /// Returns the power of an OPP.
+    pub fn power(&self) -> u64 {
+        // 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.0) }
+    }
+
+    /// 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.0, 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.0) }
+    }
+}
+
+impl Drop for OPP {
+    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(self.0) };
+    }
+}
-- 
2.31.1.272.g89b43f80a514


^ permalink raw reply related

* [RFC PATCH 0/3] Rust bindings for cpufreq and OPP core + sample driver
From: Viresh Kumar @ 2024-04-05 11:09 UTC (permalink / raw)
  To: Rafael J. Wysocki, Miguel Ojeda, Alex Gaynor, Alice Ryhl,
	Andreas Hindborg, Benno Lossin, Björn Roy Baron, Boqun Feng,
	Gary Guo, Miguel Ojeda, Viresh Kumar, Wedson Almeida Filho
  Cc: linux-pm, Vincent Guittot, Stephen Boyd, Nishanth Menon,
	rust-for-linux, Manos Pitsidianakis, Erik Schilling,
	Alex Bennée, Joakim Bech, linux-kernel

Hello,

This RFC adds initial rust bindings for two subsystems, cpufreq and operating
performance points (OPP). The bindings are provided for most of the interface
these subsystems expose.

This series also provides a sample cpufreq driver rcpufreq-dt, which is a
duplicate of the merged cpufreq-dt driver (A generic platform agnostic device
tree based cpufreq driver) used on most of the ARM platforms.

This is tested with the help of QEMU for now and frequency transitions and
configurations work as expected. No performance measurement is done as of now
with this.

These patches (along with a lot of other dependencies) are pushed here for
anyone to give them a try:

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


I understand that there is a long road ahead before these patches can get merged
as a lot of other stuff needs to get in first, i.e. Rust bindings for:
- generic device / driver
- platform device / driver
- clk, regulator, OF, cpumask, etc

Because of the missing bindings for above APIs, lot of calls are made directly
into the bindgen generated bindings for now, to make this work. Hopefully we can
have a lot less unsafe code once we get all those things merged.

Based over v6.9-rc1.

Thanks.

Viresh Kumar (3):
  rust: Add bindings for OPP framework
  rust: Add bindings for cpufreq framework
  cpufreq: Add Rust based cpufreq-dt driver

 drivers/cpufreq/Kconfig         |   12 +
 drivers/cpufreq/Makefile        |    1 +
 drivers/cpufreq/rcpufreq_dt.rs  |  264 ++++++++
 rust/bindings/bindings_helper.h |    2 +
 rust/helpers.c                  |   15 +
 rust/kernel/cpufreq.rs          | 1090 +++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |    4 +
 rust/kernel/opp.rs              |  895 +++++++++++++++++++++++++
 8 files changed, 2283 insertions(+)
 create mode 100644 drivers/cpufreq/rcpufreq_dt.rs
 create mode 100644 rust/kernel/cpufreq.rs
 create mode 100644 rust/kernel/opp.rs

-- 
2.31.1.272.g89b43f80a514


^ permalink raw reply

* Re: [PATCH] cppc_cpufreq: Fix possible null pointer dereference
From: Viresh Kumar @ 2024-04-05 10:43 UTC (permalink / raw)
  To: Aleksandr Mishin
  Cc: Ionela Voinescu, Rafael J. Wysocki, linux-pm, linux-kernel,
	lvc-project
In-Reply-To: <20240405094005.18545-1-amishin@t-argos.ru>

On 05-04-24, 12:40, Aleksandr Mishin wrote:
> cppc_cpufreq_get_rate() and hisi_cppc_cpufreq_get_rate() can be called from
> different places with various parameters. So cpufreq_cpu_get() can return
> null as 'policy' in some circumstances.
> Fix this bug by adding null return check.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: a28b2bfc099c ("cppc_cpufreq: replace per-cpu data array with a list")
> Signed-off-by: Aleksandr Mishin <amishin@t-argos.ru>
> ---
>  drivers/cpufreq/cppc_cpufreq.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 64420d9cfd1e..5f7e04e8497b 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -741,6 +741,9 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpu)
>  {
>  	struct cppc_perf_fb_ctrs fb_ctrs_t0 = {0}, fb_ctrs_t1 = {0};
>  	struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
> +	if (!policy)
> +		return -ENODEV;
> +
>  	struct cppc_cpudata *cpu_data = policy->driver_data;
>  	u64 delivered_perf;
>  	int ret;
> @@ -822,6 +825,9 @@ static struct cpufreq_driver cppc_cpufreq_driver = {
>  static unsigned int hisi_cppc_cpufreq_get_rate(unsigned int cpu)
>  {
>  	struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
> +	if (!policy)
> +		return -ENODEV;
> +
>  	struct cppc_cpudata *cpu_data = policy->driver_data;
>  	u64 desired_perf;
>  	int ret;

Does this compile fine ?

-- 
viresh

^ permalink raw reply

* Re: [PATCH] cppc_cpufreq: Fix possible null pointer dereference
From: Mukesh Ojha @ 2024-04-05 10:38 UTC (permalink / raw)
  To: Aleksandr Mishin, Ionela Voinescu
  Cc: Rafael J. Wysocki, Viresh Kumar, linux-pm, linux-kernel,
	lvc-project
In-Reply-To: <20240405094005.18545-1-amishin@t-argos.ru>



On 4/5/2024 3:10 PM, Aleksandr Mishin wrote:
> cppc_cpufreq_get_rate() and hisi_cppc_cpufreq_get_rate() can be called from
> different places with various parameters. So cpufreq_cpu_get() can return
> null as 'policy' in some circumstances.
> Fix this bug by adding null return check.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: a28b2bfc099c ("cppc_cpufreq: replace per-cpu data array with a list")
> Signed-off-by: Aleksandr Mishin <amishin@t-argos.ru>
> ---
>   drivers/cpufreq/cppc_cpufreq.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 64420d9cfd1e..5f7e04e8497b 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -741,6 +741,9 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpu)
>   {
>   	struct cppc_perf_fb_ctrs fb_ctrs_t0 = {0}, fb_ctrs_t1 = {0};
>   	struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
> +	if (!policy)
> +		return -ENODEV;
> +

You should be doing this after all variable declaration, somewhere

...
...
struct cppc_cpudata *cpu_data;
u64 delivered_perf;
int ret;

if (!policy)
	return -ENODEV;

cpu_data = policy->driver_data;
..
..

>   	struct cppc_cpudata *cpu_data = policy->driver_data;
>   	u64 delivered_perf;
>   	int ret;
> @@ -822,6 +825,9 @@ static struct cpufreq_driver cppc_cpufreq_driver = {
>   static unsigned int hisi_cppc_cpufreq_get_rate(unsigned int cpu)
>   {
>   	struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);

same here.

> +	if (!policy)
> +		return -ENODEV;
> +
>   	struct cppc_cpudata *cpu_data = policy->driver_data;
>   	u64 desired_perf;
>   	int ret;

-Mukesh

^ permalink raw reply

* Re: [PATCH 0/2] Update Energy Model with perfromance limits
From: Lukasz Luba @ 2024-04-05 10:11 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-kernel, linux-pm, dietmar.eggemann, linux-arm-kernel,
	sudeep.holla, cristian.marussi, linux-samsung-soc, rafael,
	viresh.kumar, quic_sibis
In-Reply-To: <20240405105600.000019fd@Huawei.com>



On 4/5/24 10:56, Jonathan Cameron wrote:
> On Wed,  3 Apr 2024 17:23:13 +0100
> Lukasz Luba <lukasz.luba@arm.com> wrote:
> 
> Typo in patch title.  performance

Thank you, good catch!

Regards,
Lukasz

^ permalink raw reply

* Re: [PATCH 0/2] Update Energy Model with perfromance limits
From: Jonathan Cameron @ 2024-04-05  9:56 UTC (permalink / raw)
  To: Lukasz Luba
  Cc: linux-kernel, linux-pm, dietmar.eggemann, linux-arm-kernel,
	sudeep.holla, cristian.marussi, linux-samsung-soc, rafael,
	viresh.kumar, quic_sibis
In-Reply-To: <20240403162315.1458337-1-lukasz.luba@arm.com>

On Wed,  3 Apr 2024 17:23:13 +0100
Lukasz Luba <lukasz.luba@arm.com> wrote:

Typo in patch title.  performance

^ permalink raw reply

* Re: [PATCH v6 05/16] dt-bindings: net: wireless: describe the ath12k PCI module
From: Bartosz Golaszewski @ 2024-04-05  9:52 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Marcel Holtmann, Luiz Augusto von Dentz, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
	Liam Girdwood, Mark Brown, Catalin Marinas, Will Deacon,
	Bjorn Helgaas, Saravana Kannan, Geert Uytterhoeven, Arnd Bergmann,
	Neil Armstrong, Marek Szyprowski, Alex Elder, Srini Kandagatla,
	Greg Kroah-Hartman, Abel Vesa, Manivannan Sadhasivam,
	Lukas Wunner, Dmitry Baryshkov, linux-bluetooth, netdev,
	devicetree, linux-kernel, linux-wireless, linux-arm-msm,
	linux-arm-kernel, linux-pci, linux-pm, Bartosz Golaszewski
In-Reply-To: <87cyr440hr.fsf@kernel.org>

On Fri, Apr 5, 2024 at 11:34 AM Kalle Valo <kvalo@kernel.org> wrote:
>
> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>
> > On Mon, Mar 25, 2024 at 3:01 PM Kalle Valo <kvalo@kernel.org> wrote:
> >>
> >> Bartosz Golaszewski <brgl@bgdev.pl> writes:
> >>
> >> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >> >
> >> > +
> >> > +maintainers:
> >> > +  - Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >>
> >> IMHO it would be better to have just driver maintainers listed here.
> >>
> >
> > Why? What's wrong with having the author of the bindings in the Cc list?
>
> If you want follow the ath12k development and review patches then you
> can join the ath12k list. I'm not fond of having too many maintainers,
> it's not really helping anything and just extra work to periodically
> cleanup the silent maintainers.
>
> I would ask the opposite question: why add you as the maintainer?
> There's not even a single ath12k patch from you, nor I haven't seen you
> doing any patch review or otherwise helping others related to ath12k.
> Don't get me wrong, I value the work you do with this important powerseq
> feature and hopefully we get it into the tree soon. But I don't see
> adding you as a maintainer at this point.
>

In addition to what Krzysztof already said about you seamingly
confusing the maintenance of the driver vs maintenance of the
device-tree bindings (IOW: structured hardware description) and in
response to your question: I don't see any functional change to any
dt-bindings neither from you nor from Jeff. Are you convinced you can
maintain and properly review any changes?

If so, I don't really care, I can drop myself and have less work.

Bartosz

> --
> https://patchwork.kernel.org/project/linux-wireless/list/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply

* [PATCH] cppc_cpufreq: Fix possible null pointer dereference
From: Aleksandr Mishin @ 2024-04-05  9:40 UTC (permalink / raw)
  To: Ionela Voinescu
  Cc: Aleksandr Mishin, Rafael J. Wysocki, Viresh Kumar, linux-pm,
	linux-kernel, lvc-project

cppc_cpufreq_get_rate() and hisi_cppc_cpufreq_get_rate() can be called from
different places with various parameters. So cpufreq_cpu_get() can return
null as 'policy' in some circumstances.
Fix this bug by adding null return check.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: a28b2bfc099c ("cppc_cpufreq: replace per-cpu data array with a list")
Signed-off-by: Aleksandr Mishin <amishin@t-argos.ru>
---
 drivers/cpufreq/cppc_cpufreq.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index 64420d9cfd1e..5f7e04e8497b 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -741,6 +741,9 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpu)
 {
 	struct cppc_perf_fb_ctrs fb_ctrs_t0 = {0}, fb_ctrs_t1 = {0};
 	struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
+	if (!policy)
+		return -ENODEV;
+
 	struct cppc_cpudata *cpu_data = policy->driver_data;
 	u64 delivered_perf;
 	int ret;
@@ -822,6 +825,9 @@ static struct cpufreq_driver cppc_cpufreq_driver = {
 static unsigned int hisi_cppc_cpufreq_get_rate(unsigned int cpu)
 {
 	struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
+	if (!policy)
+		return -ENODEV;
+
 	struct cppc_cpudata *cpu_data = policy->driver_data;
 	u64 desired_perf;
 	int ret;
-- 
2.30.2


^ permalink raw reply related

* Re: [PATCH v6 05/16] dt-bindings: net: wireless: describe the ath12k PCI module
From: Krzysztof Kozlowski @ 2024-04-05  9:41 UTC (permalink / raw)
  To: Kalle Valo, Bartosz Golaszewski
  Cc: Marcel Holtmann, Luiz Augusto von Dentz, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
	Liam Girdwood, Mark Brown, Catalin Marinas, Will Deacon,
	Bjorn Helgaas, Saravana Kannan, Geert Uytterhoeven, Arnd Bergmann,
	Neil Armstrong, Marek Szyprowski, Alex Elder, Srini Kandagatla,
	Greg Kroah-Hartman, Abel Vesa, Manivannan Sadhasivam,
	Lukas Wunner, Dmitry Baryshkov, linux-bluetooth, netdev,
	devicetree, linux-kernel, linux-wireless, linux-arm-msm,
	linux-arm-kernel, linux-pci, linux-pm, Bartosz Golaszewski
In-Reply-To: <83dccd7e-e690-4803-adb9-aaedcee7dc94@linaro.org>

On 05/04/2024 11:37, Krzysztof Kozlowski wrote:
> On 05/04/2024 11:33, Kalle Valo wrote:
>> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>>
>>> On Mon, Mar 25, 2024 at 3:01 PM Kalle Valo <kvalo@kernel.org> wrote:
>>>>
>>>> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>>>>
>>>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>>>
>>>>> +
>>>>> +maintainers:
>>>>> +  - Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>>
>>>> IMHO it would be better to have just driver maintainers listed here.
>>>>
>>>
>>> Why? What's wrong with having the author of the bindings in the Cc list?
>>
>> If you want follow the ath12k development and review patches then you
>> can join the ath12k list. I'm not fond of having too many maintainers,
>> it's not really helping anything and just extra work to periodically
>> cleanup the silent maintainers.
>>
>> I would ask the opposite question: why add you as the maintainer?
>> There's not even a single ath12k patch from you, nor I haven't seen you
>> doing any patch review or otherwise helping others related to ath12k.
>> Don't get me wrong, I value the work you do with this important powerseq
>> feature and hopefully we get it into the tree soon. But I don't see
>> adding you as a maintainer at this point.
> 
> This is not a maintainer of driver. This is maintainer of bindings, so
> someone who has hardware, datasheets, knowledge and/or interest in
> keeping the bindings accurate.
> 
> All your arguments above suggest you talk about the driver. This is not
> the point here.
> 

And to clarify: I do not have opinion whether Bartosz is a suitable
person here and whether driver maintainers should be instead or not. I
only want to clarify the purpose of the binding maintainer.

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v6 05/16] dt-bindings: net: wireless: describe the ath12k PCI module
From: Krzysztof Kozlowski @ 2024-04-05  9:37 UTC (permalink / raw)
  To: Kalle Valo, Bartosz Golaszewski
  Cc: Marcel Holtmann, Luiz Augusto von Dentz, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
	Liam Girdwood, Mark Brown, Catalin Marinas, Will Deacon,
	Bjorn Helgaas, Saravana Kannan, Geert Uytterhoeven, Arnd Bergmann,
	Neil Armstrong, Marek Szyprowski, Alex Elder, Srini Kandagatla,
	Greg Kroah-Hartman, Abel Vesa, Manivannan Sadhasivam,
	Lukas Wunner, Dmitry Baryshkov, linux-bluetooth, netdev,
	devicetree, linux-kernel, linux-wireless, linux-arm-msm,
	linux-arm-kernel, linux-pci, linux-pm, Bartosz Golaszewski
In-Reply-To: <87cyr440hr.fsf@kernel.org>

On 05/04/2024 11:33, Kalle Valo wrote:
> Bartosz Golaszewski <brgl@bgdev.pl> writes:
> 
>> On Mon, Mar 25, 2024 at 3:01 PM Kalle Valo <kvalo@kernel.org> wrote:
>>>
>>> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>>>
>>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>>
>>>> +
>>>> +maintainers:
>>>> +  - Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>
>>> IMHO it would be better to have just driver maintainers listed here.
>>>
>>
>> Why? What's wrong with having the author of the bindings in the Cc list?
> 
> If you want follow the ath12k development and review patches then you
> can join the ath12k list. I'm not fond of having too many maintainers,
> it's not really helping anything and just extra work to periodically
> cleanup the silent maintainers.
> 
> I would ask the opposite question: why add you as the maintainer?
> There's not even a single ath12k patch from you, nor I haven't seen you
> doing any patch review or otherwise helping others related to ath12k.
> Don't get me wrong, I value the work you do with this important powerseq
> feature and hopefully we get it into the tree soon. But I don't see
> adding you as a maintainer at this point.

This is not a maintainer of driver. This is maintainer of bindings, so
someone who has hardware, datasheets, knowledge and/or interest in
keeping the bindings accurate.

All your arguments above suggest you talk about the driver. This is not
the point here.

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v6 05/16] dt-bindings: net: wireless: describe the ath12k PCI module
From: Kalle Valo @ 2024-04-05  9:33 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Marcel Holtmann, Luiz Augusto von Dentz, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
	Liam Girdwood, Mark Brown, Catalin Marinas, Will Deacon,
	Bjorn Helgaas, Saravana Kannan, Geert Uytterhoeven, Arnd Bergmann,
	Neil Armstrong, Marek Szyprowski, Alex Elder, Srini Kandagatla,
	Greg Kroah-Hartman, Abel Vesa, Manivannan Sadhasivam,
	Lukas Wunner, Dmitry Baryshkov, linux-bluetooth, netdev,
	devicetree, linux-kernel, linux-wireless, linux-arm-msm,
	linux-arm-kernel, linux-pci, linux-pm, Bartosz Golaszewski
In-Reply-To: <CAMRc=MeCjNn7QdDrcQMuj32JFYoemQ6A8WOYcwKJo1YhDTfY+Q@mail.gmail.com>

Bartosz Golaszewski <brgl@bgdev.pl> writes:

> On Mon, Mar 25, 2024 at 3:01 PM Kalle Valo <kvalo@kernel.org> wrote:
>>
>> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>>
>> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>> >
>> > +
>> > +maintainers:
>> > +  - Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>
>> IMHO it would be better to have just driver maintainers listed here.
>>
>
> Why? What's wrong with having the author of the bindings in the Cc list?

If you want follow the ath12k development and review patches then you
can join the ath12k list. I'm not fond of having too many maintainers,
it's not really helping anything and just extra work to periodically
cleanup the silent maintainers.

I would ask the opposite question: why add you as the maintainer?
There's not even a single ath12k patch from you, nor I haven't seen you
doing any patch review or otherwise helping others related to ath12k.
Don't get me wrong, I value the work you do with this important powerseq
feature and hopefully we get it into the tree soon. But I don't see
adding you as a maintainer at this point.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply

* [rafael-pm:bleeding-edge] BUILD SUCCESS 19c31f086fbfc78ea4f695d1d2e666448c35d42f
From: kernel test robot @ 2024-04-05  9:10 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: linux-acpi, devel, linux-pm

tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git bleeding-edge
branch HEAD: 19c31f086fbfc78ea4f695d1d2e666448c35d42f  Merge branch 'acpi-scan' into bleeding-edge

elapsed time: 782m

configs tested: 179
configs skipped: 3

The following configs have been built successfully.
More configs may be tested in the coming days.

tested configs:
alpha                             allnoconfig   gcc  
alpha                            allyesconfig   gcc  
alpha                               defconfig   gcc  
arc                              allmodconfig   gcc  
arc                               allnoconfig   gcc  
arc                              allyesconfig   gcc  
arc                                 defconfig   gcc  
arc                     haps_hs_smp_defconfig   gcc  
arc                   randconfig-001-20240405   gcc  
arc                   randconfig-002-20240405   gcc  
arm                              allmodconfig   gcc  
arm                               allnoconfig   clang
arm                              allyesconfig   gcc  
arm                                 defconfig   clang
arm                         lpc32xx_defconfig   clang
arm                   randconfig-001-20240405   gcc  
arm                   randconfig-002-20240405   clang
arm                   randconfig-003-20240405   gcc  
arm                   randconfig-004-20240405   clang
arm                           tegra_defconfig   gcc  
arm                           u8500_defconfig   gcc  
arm                        vexpress_defconfig   gcc  
arm                         vf610m4_defconfig   gcc  
arm64                            allmodconfig   clang
arm64                             allnoconfig   gcc  
arm64                               defconfig   gcc  
arm64                 randconfig-001-20240405   gcc  
arm64                 randconfig-002-20240405   gcc  
arm64                 randconfig-003-20240405   gcc  
arm64                 randconfig-004-20240405   gcc  
csky                             allmodconfig   gcc  
csky                              allnoconfig   gcc  
csky                             allyesconfig   gcc  
csky                                defconfig   gcc  
csky                  randconfig-001-20240405   gcc  
csky                  randconfig-002-20240405   gcc  
hexagon                          allmodconfig   clang
hexagon                           allnoconfig   clang
hexagon                          allyesconfig   clang
hexagon                             defconfig   clang
hexagon               randconfig-001-20240405   clang
hexagon               randconfig-002-20240405   clang
i386                             allmodconfig   gcc  
i386                              allnoconfig   gcc  
i386                             allyesconfig   gcc  
i386         buildonly-randconfig-001-20240405   gcc  
i386         buildonly-randconfig-002-20240405   gcc  
i386         buildonly-randconfig-003-20240405   clang
i386         buildonly-randconfig-004-20240405   gcc  
i386         buildonly-randconfig-005-20240405   clang
i386         buildonly-randconfig-006-20240405   clang
i386                                defconfig   clang
i386                  randconfig-001-20240405   clang
i386                  randconfig-002-20240405   gcc  
i386                  randconfig-003-20240405   clang
i386                  randconfig-004-20240405   clang
i386                  randconfig-005-20240405   clang
i386                  randconfig-006-20240405   gcc  
i386                  randconfig-011-20240405   clang
i386                  randconfig-012-20240405   gcc  
i386                  randconfig-013-20240405   gcc  
i386                  randconfig-014-20240405   gcc  
i386                  randconfig-015-20240405   gcc  
i386                  randconfig-016-20240405   clang
loongarch                        allmodconfig   gcc  
loongarch                         allnoconfig   gcc  
loongarch                           defconfig   gcc  
loongarch             randconfig-001-20240405   gcc  
loongarch             randconfig-002-20240405   gcc  
m68k                             allmodconfig   gcc  
m68k                              allnoconfig   gcc  
m68k                             allyesconfig   gcc  
m68k                                defconfig   gcc  
microblaze                       allmodconfig   gcc  
microblaze                        allnoconfig   gcc  
microblaze                       allyesconfig   gcc  
microblaze                          defconfig   gcc  
mips                              allnoconfig   gcc  
mips                             allyesconfig   gcc  
mips                      bmips_stb_defconfig   clang
mips                        omega2p_defconfig   clang
nios2                            allmodconfig   gcc  
nios2                             allnoconfig   gcc  
nios2                            allyesconfig   gcc  
nios2                               defconfig   gcc  
nios2                 randconfig-001-20240405   gcc  
nios2                 randconfig-002-20240405   gcc  
openrisc                          allnoconfig   gcc  
openrisc                         allyesconfig   gcc  
openrisc                            defconfig   gcc  
parisc                           allmodconfig   gcc  
parisc                            allnoconfig   gcc  
parisc                           allyesconfig   gcc  
parisc                              defconfig   gcc  
parisc                randconfig-001-20240405   gcc  
parisc                randconfig-002-20240405   gcc  
parisc64                            defconfig   gcc  
powerpc                          allmodconfig   gcc  
powerpc                           allnoconfig   gcc  
powerpc                          allyesconfig   clang
powerpc                   currituck_defconfig   clang
powerpc                     mpc5200_defconfig   clang
powerpc                 mpc834x_itx_defconfig   clang
powerpc                      pmac32_defconfig   clang
powerpc               randconfig-001-20240405   clang
powerpc               randconfig-002-20240405   clang
powerpc               randconfig-003-20240405   clang
powerpc                         wii_defconfig   gcc  
powerpc64             randconfig-001-20240405   gcc  
powerpc64             randconfig-002-20240405   gcc  
powerpc64             randconfig-003-20240405   clang
riscv                            allmodconfig   clang
riscv                             allnoconfig   gcc  
riscv                            allyesconfig   clang
riscv                               defconfig   clang
riscv                 randconfig-001-20240405   clang
riscv                 randconfig-002-20240405   gcc  
s390                             allmodconfig   clang
s390                              allnoconfig   clang
s390                             allyesconfig   gcc  
s390                          debug_defconfig   gcc  
s390                                defconfig   clang
s390                  randconfig-001-20240405   clang
s390                  randconfig-002-20240405   gcc  
s390                       zfcpdump_defconfig   clang
sh                               allmodconfig   gcc  
sh                                allnoconfig   gcc  
sh                               allyesconfig   gcc  
sh                                  defconfig   gcc  
sh                    randconfig-001-20240405   gcc  
sh                    randconfig-002-20240405   gcc  
sh                        sh7757lcr_defconfig   gcc  
sparc                            allmodconfig   gcc  
sparc                             allnoconfig   gcc  
sparc                               defconfig   gcc  
sparc64                          allmodconfig   gcc  
sparc64                          allyesconfig   gcc  
sparc64                             defconfig   gcc  
sparc64               randconfig-001-20240405   gcc  
sparc64               randconfig-002-20240405   gcc  
um                               allmodconfig   clang
um                                allnoconfig   clang
um                               allyesconfig   gcc  
um                                  defconfig   clang
um                             i386_defconfig   gcc  
um                    randconfig-001-20240405   gcc  
um                    randconfig-002-20240405   clang
um                           x86_64_defconfig   clang
x86_64                            allnoconfig   clang
x86_64                           allyesconfig   clang
x86_64       buildonly-randconfig-001-20240405   clang
x86_64       buildonly-randconfig-002-20240405   clang
x86_64       buildonly-randconfig-003-20240405   gcc  
x86_64       buildonly-randconfig-004-20240405   gcc  
x86_64       buildonly-randconfig-005-20240405   clang
x86_64       buildonly-randconfig-006-20240405   clang
x86_64                              defconfig   gcc  
x86_64                randconfig-001-20240405   clang
x86_64                randconfig-002-20240405   clang
x86_64                randconfig-003-20240405   clang
x86_64                randconfig-004-20240405   gcc  
x86_64                randconfig-005-20240405   clang
x86_64                randconfig-006-20240405   gcc  
x86_64                randconfig-011-20240405   clang
x86_64                randconfig-012-20240405   gcc  
x86_64                randconfig-013-20240405   gcc  
x86_64                randconfig-014-20240405   clang
x86_64                randconfig-015-20240405   gcc  
x86_64                randconfig-016-20240405   gcc  
x86_64                randconfig-071-20240405   gcc  
x86_64                randconfig-072-20240405   gcc  
x86_64                randconfig-073-20240405   gcc  
x86_64                randconfig-074-20240405   gcc  
x86_64                randconfig-075-20240405   gcc  
x86_64                randconfig-076-20240405   gcc  
x86_64                          rhel-8.3-rust   clang
xtensa                            allnoconfig   gcc  
xtensa                  audio_kc705_defconfig   gcc  
xtensa                randconfig-001-20240405   gcc  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply

* [PATCH] drivers: thermal: tsens: Fix null pointer dereference
From: Aleksandr Mishin @ 2024-04-05  9:07 UTC (permalink / raw)
  To: Christian Marangi
  Cc: Aleksandr Mishin, Amit Kucheria, Thara Gopinath, Bjorn Andersson,
	Konrad Dybcio, Rafael J. Wysocki, Daniel Lezcano, Zhang Rui,
	Lukasz Luba, linux-pm, linux-arm-msm, linux-kernel, lvc-project

compute_intercept_slope() is called from calibrate_8960() (in tsens-8960.c)
as compute_intercept_slope(priv, p1, NULL, ONE_PT_CALIB) which lead to null
pointer dereference (if DEBUG or DYNAMIC_DEBUG set).
Fix this bug by adding null pointer check.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: dfc1193d4dbd ("thermal/drivers/tsens: Replace custom 8960 apis with generic apis")
Signed-off-by: Aleksandr Mishin <amishin@t-argos.ru>
---
 drivers/thermal/qcom/tsens.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index 6d7c16ccb44d..f7dd70e8d158 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -262,9 +262,10 @@ void compute_intercept_slope(struct tsens_priv *priv, u32 *p1,
 	int num, den;
 
 	for (i = 0; i < priv->num_sensors; i++) {
-		dev_dbg(priv->dev,
-			"%s: sensor%d - data_point1:%#x data_point2:%#x\n",
-			__func__, i, p1[i], p2[i]);
+		if (p1 && p2)
+			dev_dbg(priv->dev,
+				"%s: sensor%d - data_point1:%#x data_point2:%#x\n",
+				__func__, i, p1[i], p2[i]);
 
 		if (!priv->sensor[i].slope)
 			priv->sensor[i].slope = SLOPE_DEFAULT;
-- 
2.30.2


^ permalink raw reply related

* [PATCH] PM: s2idle: Make sure CPUs will wakeup directly on resume
From: Anna-Maria Behnsen @ 2024-04-05  8:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: Rafael J . Wysocki, Pavel Machek, Len Brown, Ulf Hansson,
	linux-pm, Frederic Weisbecker, Thomas Gleixner, x86,
	Anna-Maria Behnsen, Mario Limonciello, stable

s2idle works like a regular suspend with freezing processes and freezing
devices. All CPUs except the control CPU go into idle. Once this is
completed the control CPU kicks all other CPUs out of idle, so that they
reenter the idle loop and then enter s2idle state. The control CPU then
issues an swait() on the suspend state and therefore enters the idle loop
as well.

Due to being kicked out of idle, the other CPUs leave their NOHZ states,
which means the tick is active and the corresponding hrtimer is programmed
to the next jiffie.

On entering s2idle the CPUs shut down their local clockevent device to
prevent wakeups. The last CPU which enters s2idle shuts down its local
clockevent and freezes timekeeping.

On resume, one of the CPUs receives the wakeup interrupt, unfreezes
timekeeping and its local clockevent and starts the resume process. At that
point all other CPUs are still in s2idle with their clockevents switched
off. They only resume when they are kicked by another CPU or after resuming
devices and then receiving a device interrupt.

That means there is no guarantee that all CPUs will wakeup directly on
resume. As the consequence there is no guarantee that timers which are
queued on those CPUs and should expire directly after resume, are
handled. Also timer list timers which are remotely queued to one of those
CPUs after resume will not result in a reporgramming IPI as the tick is
active. A queue hrtimer will also not result in a reprogramming IPI because
the first hrtimer event is already in the past.

The recent introduction of the timer pull model (7ee988770326 ("timers:
Implement the hierarchical pull model")) amplifies this problem, if the
current migrator is one of the non woken up CPUs. When a non pinned timer
list timer is queued and the queueing CPU goes idle, it relies on the still
suspended migrator CPU to expire the timer which will happen by chance.

The problem existis since commit 8d89835b0467 ("PM: suspend: Do not pause
cpuidle in the suspend-to-idle path"). There the cpuidle_pause() call which
in turn invoked a wakeup for all idle CPUs was moved to a later point in
the resume process. This might not be reached or reached very late because
it waits on a timer of a still suspended CPU.

Address this by kicking all CPUs out of idle after the control CPU returns
from swait() so that they resume their timers and restore consistent system
state.

Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218641
Fixes: 8d89835b0467 ("PM: suspend: Do not pause cpuidle in the suspend-to-idle path")
Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de>
Tested-by: Mario Limonciello <mario.limonciello@amd.com>
Cc: stable@kernel.org
---
 kernel/power/suspend.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index e3ae93bbcb9b..09f8397bae15 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -106,6 +106,12 @@ static void s2idle_enter(void)
 	swait_event_exclusive(s2idle_wait_head,
 		    s2idle_state == S2IDLE_STATE_WAKE);
 
+	/*
+	 * Kick all CPUs to ensure that they resume their timers and restore
+	 * consistent system state.
+	 */
+	wake_up_all_idle_cpus();
+
 	cpus_read_unlock();
 
 	raw_spin_lock_irq(&s2idle_lock);
-- 
2.39.2


^ permalink raw reply related

* Re: [PATCH v3 3/6] thermal: core: Rewrite comments in handle_thermal_trip()
From: Daniel Lezcano @ 2024-04-05  8:30 UTC (permalink / raw)
  To: Rafael J. Wysocki, Linux PM
  Cc: LKML, Srinivas Pandruvada, Lukasz Luba,
	AngeloGioacchino Del Regno
In-Reply-To: <3284691.aeNJFYEL58@kreacher>

On 02/04/2024 20:59, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Make the comments regarding trip crossing and threshold updates in
> handle_thermal_trip() slightly more clear.
> 
> No functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


^ permalink raw reply

* Re: [PATCH v3 2/6] thermal: core: Make struct thermal_zone_device definition internal
From: Daniel Lezcano @ 2024-04-05  8:26 UTC (permalink / raw)
  To: Rafael J. Wysocki, Linux PM
  Cc: LKML, Srinivas Pandruvada, Lukasz Luba,
	AngeloGioacchino Del Regno
In-Reply-To: <13485814.uLZWGnKmhe@kreacher>

On 02/04/2024 20:57, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Move the definitions of struct thermal_trip_desc and struct
> thermal_zone_device to an internal header file in the thermal core,
> as they don't need to be accessible to any code other than the thermal
> core and so they don't need to be present in a global header.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


^ permalink raw reply

* Re: [PATCH v1] thermal: core: Relocate the struct thermal_governor definition
From: Lukasz Luba @ 2024-04-05  8:16 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: LKML, Daniel Lezcano, Linux PM
In-Reply-To: <2725268.mvXUDI8C0e@kreacher>



On 4/4/24 20:27, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Notice that struct thermal_governor is only used by the thermal core
> and so move its definition to thermal_core.h.
> 
> No functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>   drivers/thermal/thermal_core.h |   25 +++++++++++++++++++++++++
>   include/linux/thermal.h        |   25 -------------------------
>   2 files changed, 25 insertions(+), 25 deletions(-)
> 
> Index: linux-pm/drivers/thermal/thermal_core.h
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.h
> +++ linux-pm/drivers/thermal/thermal_core.h
> @@ -23,6 +23,31 @@ struct thermal_trip_desc {
>   };
>   
>   /**
> + * struct thermal_governor - structure that holds thermal governor information
> + * @name:	name of the governor
> + * @bind_to_tz: callback called when binding to a thermal zone.  If it
> + *		returns 0, the governor is bound to the thermal zone,
> + *		otherwise it fails.
> + * @unbind_from_tz:	callback called when a governor is unbound from a
> + *			thermal zone.
> + * @throttle:	callback called for every trip point even if temperature is
> + *		below the trip point temperature
> + * @update_tz:	callback called when thermal zone internals have changed, e.g.
> + *		thermal cooling instance was added/removed
> + * @governor_list:	node in thermal_governor_list (in thermal_core.c)
> + */
> +struct thermal_governor {
> +	const char *name;
> +	int (*bind_to_tz)(struct thermal_zone_device *tz);
> +	void (*unbind_from_tz)(struct thermal_zone_device *tz);
> +	int (*throttle)(struct thermal_zone_device *tz,
> +			const struct thermal_trip *trip);
> +	void (*update_tz)(struct thermal_zone_device *tz,
> +			  enum thermal_notify_event reason);
> +	struct list_head	governor_list;
> +};
> +
> +/**
>    * struct thermal_zone_device - structure for a thermal zone
>    * @id:		unique id number for each thermal zone
>    * @type:	the thermal zone device type
> Index: linux-pm/include/linux/thermal.h
> ===================================================================
> --- linux-pm.orig/include/linux/thermal.h
> +++ linux-pm/include/linux/thermal.h
> @@ -126,31 +126,6 @@ struct thermal_cooling_device {
>   #endif
>   };
>   
> -/**
> - * struct thermal_governor - structure that holds thermal governor information
> - * @name:	name of the governor
> - * @bind_to_tz: callback called when binding to a thermal zone.  If it
> - *		returns 0, the governor is bound to the thermal zone,
> - *		otherwise it fails.
> - * @unbind_from_tz:	callback called when a governor is unbound from a
> - *			thermal zone.
> - * @throttle:	callback called for every trip point even if temperature is
> - *		below the trip point temperature
> - * @update_tz:	callback called when thermal zone internals have changed, e.g.
> - *		thermal cooling instance was added/removed
> - * @governor_list:	node in thermal_governor_list (in thermal_core.c)
> - */
> -struct thermal_governor {
> -	const char *name;
> -	int (*bind_to_tz)(struct thermal_zone_device *tz);
> -	void (*unbind_from_tz)(struct thermal_zone_device *tz);
> -	int (*throttle)(struct thermal_zone_device *tz,
> -			const struct thermal_trip *trip);
> -	void (*update_tz)(struct thermal_zone_device *tz,
> -			  enum thermal_notify_event reason);
> -	struct list_head	governor_list;
> -};
> -
>   /* Structure to define Thermal Zone parameters */
>   struct thermal_zone_params {
>   	const char *governor_name;
> 
> 
> 

That makes perfectly sense IMO. We don't have drivers which come
together with some custom governor just for them (like IIRC some
devfreq more complex devices do/did).

Lets close that open dimension.

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox