* [PATCH v7 0/2] Add a bare-minimum Regulator abstraction
@ 2025-07-04 18:43 Daniel Almeida
2025-07-04 18:43 ` [PATCH v7 1/2] rust: regulator: add a bare minimum regulator abstraction Daniel Almeida
2025-07-04 18:43 ` [PATCH v7 2/2] MAINAINTERS: add regulator.rs to the regulator API entry Daniel Almeida
0 siblings, 2 replies; 7+ messages in thread
From: Daniel Almeida @ 2025-07-04 18:43 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Boris Brezillon, Sebastian Reichel,
Liam Girdwood, Mark Brown, Benno Lossin
Cc: linux-kernel, rust-for-linux, Daniel Almeida
---
Changes in v7:
- Add RegulatorState::DISABLE_ON_DROP (Alice)
- Remove #[cfg(CONFIG_REGULATOR)] in lib.rs (if this is N we will use
the stubs)
- Add the bound on 'static directly on RegulatorState to avoid
repetition
- Removed the `data` member on the example (Alice)
- Removed the `mut` token from try_into_enabled() and
try_into_disabled() (Miguel & Intel bot)
- Link to v6: https://lore.kernel.org/r/20250627-topics-tyr-regulator-v6-0-1d015219b454@collabora.com
Changes in v6:
- Use ManuallyDrop<T> to avoid running the destructor in
try_into_enabled() and try_into_disabled(). This is the same strategy
that was being used successfully in the pre-typestate version of this
patch
- Link to v5: https://lore.kernel.org/r/20250623-topics-tyr-regulator-v5-0-99069658cb54@collabora.com
Changes in v5:
- Remove TryIntoEnabled and TryIntoDisabled traits (they were only
implemented for a single type anyways)
- Added regulator.rs to VOLTAGE AND CURRENT REGULATOR FRAMEWORK
- Applied the diff from Miguel Ojeda to format the docs
- Link to v4: https://lore.kernel.org/r/20250609-topics-tyr-regulator-v4-1-b4fdcf1385a7@collabora.com
Changes in v4:
- Rewrote the abstraction to use typestates as per the suggestions by
Benno and Alex.
- Introduced the `Dynamic` state.
- Added more examples.
- Fixed some broken docs.
- Link to v3: https://lore.kernel.org/r/20250513-topics-tyr-regulator-v3-1-4cc2704dfec6@collabora.com
Changes in v3:
- Rebased on rust-next
- Added examples to showcase the API
- Fixed some rendering issues in the docs
- Exposed {get|set}_voltage for both Regulator and EnabledRegulator
- Derived Clone, Copy, PartialEq and Eq for Microvolt
- Link to v2: https://lore.kernel.org/r/20250326-topics-tyr-regulator-v2-1-c0ea6a861be6@collabora.com
Resend v2:
- cc Regulator maintainers
Changes from v1:
- Rebased on rust-next
- Split the design into two types as suggested by Alice Ryhl.
- Modify the docs to highlight how users can use kernel::types::Either
or an enum to enable and disable the regulator at runtime.
- Link to v1: https://lore.kernel.org/rust-for-linux/20250219162517.278362-1-daniel.almeida@collabora.com/
---
Daniel Almeida (2):
rust: regulator: add a bare minimum regulator abstraction
MAINAINTERS: add regulator.rs to the regulator API entry
MAINTAINERS | 1 +
rust/bindings/bindings_helper.h | 1 +
rust/kernel/lib.rs | 1 +
rust/kernel/regulator.rs | 403 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 406 insertions(+)
---
base-commit: 2009a2d5696944d85c34d75e691a6f3884e787c0
change-id: 20250326-topics-tyr-regulator-e8b98f6860d7
Best regards,
--
Daniel Almeida <daniel.almeida@collabora.com>
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v7 1/2] rust: regulator: add a bare minimum regulator abstraction 2025-07-04 18:43 [PATCH v7 0/2] Add a bare-minimum Regulator abstraction Daniel Almeida @ 2025-07-04 18:43 ` Daniel Almeida 2025-07-06 0:39 ` kernel test robot 2025-07-07 5:39 ` Alexandre Courbot 2025-07-04 18:43 ` [PATCH v7 2/2] MAINAINTERS: add regulator.rs to the regulator API entry Daniel Almeida 1 sibling, 2 replies; 7+ messages in thread From: Daniel Almeida @ 2025-07-04 18:43 UTC (permalink / raw) To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich, Boris Brezillon, Sebastian Reichel, Liam Girdwood, Mark Brown, Benno Lossin Cc: linux-kernel, rust-for-linux, Daniel Almeida Add a bare minimum regulator abstraction to be used by Rust drivers. This abstraction adds a small subset of the regulator API, which is thought to be sufficient for the drivers we have now. Regulators provide the power needed by many hardware blocks and thus are likely to be needed by a lot of drivers. It was tested on rk3588, where it was used to power up the "mali" regulator in order to power up the GPU. Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com> --- rust/bindings/bindings_helper.h | 1 + rust/kernel/lib.rs | 1 + rust/kernel/regulator.rs | 403 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 405 insertions(+) diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h index 8cbb660e2ec218021d16e6e0144acf6f4d7cca13..2d51f9d056091e34120b4ade9ff7cc4a7f53e111 100644 --- a/rust/bindings/bindings_helper.h +++ b/rust/bindings/bindings_helper.h @@ -65,6 +65,7 @@ #include <linux/poll.h> #include <linux/property.h> #include <linux/refcount.h> +#include <linux/regulator/consumer.h> #include <linux/sched.h> #include <linux/security.h> #include <linux/slab.h> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index 6b4774b2b1c37f4da1866e993be6230bc6715841..5e4cd8c5e6ff1c4af52a5b1be4c4c32b5104e233 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -100,6 +100,7 @@ pub mod prelude; pub mod print; pub mod rbtree; +pub mod regulator; pub mod revocable; pub mod security; pub mod seq_file; diff --git a/rust/kernel/regulator.rs b/rust/kernel/regulator.rs new file mode 100644 index 0000000000000000000000000000000000000000..56f3a91469549551f54c7b4c7ec67aa941acd572 --- /dev/null +++ b/rust/kernel/regulator.rs @@ -0,0 +1,403 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Regulator abstractions, providing a standard kernel interface to control +//! voltage and current regulators. +//! +//! The intention is to allow systems to dynamically control regulator power +//! output in order to save power and prolong battery life. This applies to both +//! voltage regulators (where voltage output is controllable) and current sinks +//! (where current limit is controllable). +//! +//! C header: [`include/linux/regulator/consumer.h`](srctree/include/linux/regulator/consumer.h) +//! +//! Regulators are modeled in Rust with a collection of states. Each state may +//! enforce a given invariant, and they may convert between each other where applicable. +//! +//! See [Voltage and current regulator API](https://docs.kernel.org/driver-api/regulator.html) +//! for more information. + +use crate::{ + bindings, + device::Device, + error::{from_err_ptr, to_result, Result}, + prelude::*, +}; + +use core::{marker::PhantomData, mem::ManuallyDrop, ptr::NonNull}; + +mod private { + pub trait Sealed {} + + impl Sealed for super::Enabled {} + impl Sealed for super::Disabled {} + impl Sealed for super::Dynamic {} +} + +/// A trait representing the different states a [`Regulator`] can be in. +pub trait RegulatorState: private::Sealed + 'static { + /// Whether the regulator should be disabled when dropped. + const DISABLE_ON_DROP: bool; +} + +/// A state where the [`Regulator`] is known to be enabled. +pub struct Enabled; + +/// A state where this [`Regulator`] handle has not specifically asked for the +/// underlying regulator to be enabled. This means that this reference does not +/// own an `enable` reference count, but the regulator may still be on. +pub struct Disabled; + +/// A state that models the C API. The [`Regulator`] can be either enabled or +/// disabled, and the user is in control of the reference count. This is also +/// the default state. +/// +/// Use [`Regulator::is_enabled`] to check the regulator's current state. +pub struct Dynamic; + +impl RegulatorState for Enabled { + const DISABLE_ON_DROP: bool = true; +} + +impl RegulatorState for Disabled { + const DISABLE_ON_DROP: bool = false; +} + +impl RegulatorState for Dynamic { + const DISABLE_ON_DROP: bool = false; +} + +/// A trait that abstracts the ability to check if a [`Regulator`] is enabled. +pub trait IsEnabled: RegulatorState {} +impl IsEnabled for Disabled {} +impl IsEnabled for Dynamic {} + +/// An error that can occur when trying to convert a [`Regulator`] between states. +pub struct Error<State: RegulatorState> { + /// The error that occurred. + pub error: kernel::error::Error, + + /// The regulator that caused the error, so that the operation may be retried. + pub regulator: Regulator<State>, +} + +/// A `struct regulator` abstraction. +/// +/// # Examples +/// +/// ## Enabling a regulator +/// +/// This example uses [`Regulator<Enabled>`], which is suitable for drivers that +/// enable a regulator at probe time and leave them on until the device is +/// removed or otherwise shutdown. +/// +/// These users can store [`Regulator<Enabled>`] directly in their driver's +/// private data struct. +/// +/// ``` +/// # use kernel::prelude::*; +/// # use kernel::c_str; +/// # use kernel::device::Device; +/// # use kernel::regulator::{Microvolt, Regulator, Disabled, Enabled}; +/// fn enable(dev: &Device, min_uv: Microvolt, max_uv: Microvolt) -> Result { +/// // Obtain a reference to a (fictitious) regulator. +/// let regulator: Regulator<Disabled> = Regulator::<Disabled>::get(dev, c_str!("vcc"))?; +/// +/// // The voltage can be set before enabling the regulator if needed, e.g.: +/// regulator.set_voltage(min_uv, max_uv)?; +/// +/// // The same applies for `get_voltage()`, i.e.: +/// let voltage: Microvolt = regulator.get_voltage()?; +/// +/// // Enables the regulator, consuming the previous value. +/// // +/// // From now on, the regulator is known to be enabled because of the type +/// // `Enabled`. +/// // +/// // If this operation fails, the `Error` will contain the regulator +/// // reference, so that the operation may be retried. +/// let regulator: Regulator<Enabled> = +/// regulator.try_into_enabled().map_err(|error| error.error)?; +/// +/// // The voltage can also be set after enabling the regulator, e.g.: +/// regulator.set_voltage(min_uv, max_uv)?; +/// +/// // The same applies for `get_voltage()`, i.e.: +/// let voltage: Microvolt = regulator.get_voltage()?; +/// +/// // Dropping an enabled regulator will disable it. The refcount will be +/// // decremented. +/// drop(regulator); +/// +/// // ... +/// +/// Ok(()) +/// } +/// ``` +/// +/// A more concise shortcut is available for enabling a regulator. This is +/// equivalent to `regulator_get_enable()`: +/// +/// ``` +/// # use kernel::prelude::*; +/// # use kernel::c_str; +/// # use kernel::device::Device; +/// # use kernel::regulator::{Microvolt, Regulator, Enabled}; +/// fn enable(dev: &Device, min_uv: Microvolt, max_uv: Microvolt) -> Result { +/// // Obtain a reference to a (fictitious) regulator and enable it. +/// let regulator: Regulator<Enabled> = Regulator::<Enabled>::get(dev, c_str!("vcc"))?; +/// +/// // Dropping an enabled regulator will disable it. The refcount will be +/// // decremented. +/// drop(regulator); +/// +/// // ... +/// +/// Ok(()) +/// } +/// ``` +/// +/// ## Disabling a regulator +/// +/// ``` +/// # use kernel::prelude::*; +/// # use kernel::device::Device; +/// # use kernel::regulator::{Regulator, Enabled, Disabled}; +/// fn disable(dev: &Device, regulator: Regulator<Enabled>) -> Result { +/// // We can also disable an enabled regulator without reliquinshing our +/// // refcount: +/// // +/// // If this operation fails, the `Error` will contain the regulator +/// // reference, so that the operation may be retried. +/// let regulator: Regulator<Disabled> = +/// regulator.try_into_disabled().map_err(|error| error.error)?; +/// +/// // The refcount will be decremented when `regulator` is dropped. +/// drop(regulator); +/// +/// // ... +/// +/// Ok(()) +/// } +/// ``` +/// +/// ## Using [`Regulator<Dynamic>`] +/// +/// This example mimics the behavior of the C API, where the user is in +/// control of the enabled reference count. This is useful for drivers that +/// might call enable and disable to manage the `enable` reference count at +/// runtime, perhaps as a result of `open()` and `close()` calls or whatever +/// other driver-specific or subsystem-specific hooks. +/// +/// ``` +/// # use kernel::prelude::*; +/// # use kernel::c_str; +/// # use kernel::device::Device; +/// # use kernel::regulator::{Regulator, Dynamic}; +/// struct PrivateData { +/// regulator: Regulator<Dynamic>, +/// } +/// +/// // A fictictious probe function that obtains a regulator and sets it up. +/// fn probe(dev: &Device) -> Result<PrivateData> { +/// // Obtain a reference to a (fictitious) regulator. +/// let mut regulator = Regulator::<Dynamic>::get(dev, c_str!("vcc"))?; +/// +/// // Enable the regulator. The type is still `Regulator<Dynamic>`. +/// regulator.enable()?; +/// +/// Ok(PrivateData { regulator }) +/// } +/// +/// // A fictictious function that indicates that the device is going to be used. +/// fn open(dev: &Device, data: &mut PrivateData) -> Result { +/// // Increase the `enabled` reference count. +/// data.regulator.enable()?; +/// +/// Ok(()) +/// } +/// +/// fn close(dev: &Device, data: &mut PrivateData) -> Result { +/// // Decrease the `enabled` reference count. +/// data.regulator.disable()?; +/// +/// Ok(()) +/// } +/// +/// fn remove(dev: &Device, data: PrivateData) -> Result { +/// // `PrivateData` is dropped here, which will drop the +/// // `Regulator<Dynamic>` in turn. +/// // +/// // The reference that was obtained by `regulator_get()` will be +/// // released, but it is up to the user to make sure that the number of calls +/// // to `enable()` and `disabled()` are balanced before this point. +/// Ok(()) +/// } +/// ``` +/// +/// # Invariants +/// +/// - `inner` is a non-null wrapper over a pointer to a `struct +/// regulator` obtained from [`regulator_get()`]. +/// +/// [`regulator_get()`]: https://docs.kernel.org/driver-api/regulator.html#c.regulator_get +pub struct Regulator<State = Dynamic> +where + State: RegulatorState, +{ + inner: NonNull<bindings::regulator>, + _phantom: PhantomData<State>, +} + +impl<T: RegulatorState> Regulator<T> { + /// Sets the voltage for the regulator. + /// + /// This can be used to ensure that the device powers up cleanly. + pub fn set_voltage(&self, min_uv: Microvolt, max_uv: Microvolt) -> Result { + // SAFETY: Safe as per the type invariants of `Regulator`. + to_result(unsafe { + bindings::regulator_set_voltage(self.inner.as_ptr(), min_uv.0, max_uv.0) + }) + } + + /// Gets the current voltage of the regulator. + pub fn get_voltage(&self) -> Result<Microvolt> { + // SAFETY: Safe as per the type invariants of `Regulator`. + let voltage = unsafe { bindings::regulator_get_voltage(self.inner.as_ptr()) }; + if voltage < 0 { + Err(kernel::error::Error::from_errno(voltage)) + } else { + Ok(Microvolt(voltage)) + } + } + + fn get_internal(dev: &Device, name: &CStr) -> Result<Regulator<T>> { + // SAFETY: It is safe to call `regulator_get()`, on a device pointer + // received from the C code. + let inner = from_err_ptr(unsafe { bindings::regulator_get(dev.as_raw(), name.as_ptr()) })?; + + // SAFETY: We can safely trust `inner` to be a pointer to a valid + // regulator if `ERR_PTR` was not returned. + let inner = unsafe { NonNull::new_unchecked(inner) }; + + Ok(Self { + inner, + _phantom: PhantomData, + }) + } + + fn enable_internal(&mut self) -> Result { + // SAFETY: Safe as per the type invariants of `Regulator`. + to_result(unsafe { bindings::regulator_enable(self.inner.as_ptr()) }) + } + + fn disable_internal(&mut self) -> Result { + // SAFETY: Safe as per the type invariants of `Regulator`. + to_result(unsafe { bindings::regulator_disable(self.inner.as_ptr()) }) + } +} + +impl Regulator<Disabled> { + /// Obtains a [`Regulator`] instance from the system. + pub fn get(dev: &Device, name: &CStr) -> Result<Self> { + Regulator::get_internal(dev, name) + } + + /// Attempts to convert the regulator to an enabled state. + pub fn try_into_enabled(self) -> Result<Regulator<Enabled>, Error<Disabled>> { + // We will be transferring the ownership of our `regulator_get()` count to + // `Regulator<Enabled>`. + let mut regulator = ManuallyDrop::new(self); + + regulator + .enable_internal() + .map(|()| Regulator { + inner: regulator.inner, + _phantom: PhantomData, + }) + .map_err(|error| Error { + error, + regulator: ManuallyDrop::into_inner(regulator), + }) + } +} + +impl Regulator<Enabled> { + /// Obtains a [`Regulator`] instance from the system and enables it. + /// + /// This is equivalent to calling `regulator_get_enable()` in the C API. + pub fn get(dev: &Device, name: &CStr) -> Result<Self> { + Regulator::<Disabled>::get_internal(dev, name)? + .try_into_enabled() + .map_err(|error| error.error) + } + + /// Attempts to convert the regulator to a disabled state. + pub fn try_into_disabled(self) -> Result<Regulator<Disabled>, Error<Enabled>> { + // We will be transferring the ownership of our `regulator_get()` count + // to `Regulator<Disabled>`. + let mut regulator = ManuallyDrop::new(self); + + regulator + .disable_internal() + .map(|()| Regulator { + inner: regulator.inner, + _phantom: PhantomData, + }) + .map_err(|error| Error { + error, + regulator: ManuallyDrop::into_inner(regulator), + }) + } +} + +impl Regulator<Dynamic> { + /// Obtains a [`Regulator`] instance from the system. The current state of + /// the regulator is unknown and it is up to the user to manage the enabled + /// reference count. + /// + /// This closely mimics the behavior of the C API and can be used to + /// dynamically manage the enabled reference count at runtime. + pub fn get(dev: &Device, name: &CStr) -> Result<Self> { + Regulator::get_internal(dev, name) + } + + /// Increases the `enabled` reference count. + pub fn enable(&mut self) -> Result { + self.enable_internal() + } + + /// Decreases the `enabled` reference count. + pub fn disable(&mut self) -> Result { + self.disable_internal() + } +} + +impl<T: IsEnabled> Regulator<T> { + /// Checks if the regulator is enabled. + pub fn is_enabled(&self) -> bool { + // SAFETY: Safe as per the type invariants of `Regulator`. + unsafe { bindings::regulator_is_enabled(self.inner.as_ptr()) != 0 } + } +} + +impl<T: RegulatorState> Drop for Regulator<T> { + fn drop(&mut self) { + if T::DISABLE_ON_DROP { + // SAFETY: By the type invariants, we know that `self` owns a + // reference on the enabled refcount, so it is safe to relinquish it + // now. + unsafe { bindings::regulator_disable(self.inner.as_ptr()) }; + } + // SAFETY: By the type invariants, we know that `self` owns a reference, + // so it is safe to relinquish it now. + unsafe { bindings::regulator_put(self.inner.as_ptr()) }; + } +} + +/// A voltage in microvolts. +/// +/// The explicit type is used to avoid confusion with other multiples of the +/// volt, which can be disastrous. +#[repr(transparent)] +#[derive(Copy, Clone, PartialEq, Eq)] +pub struct Microvolt(pub i32); -- 2.50.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v7 1/2] rust: regulator: add a bare minimum regulator abstraction 2025-07-04 18:43 ` [PATCH v7 1/2] rust: regulator: add a bare minimum regulator abstraction Daniel Almeida @ 2025-07-06 0:39 ` kernel test robot 2025-07-07 16:23 ` Daniel Almeida 2025-07-07 5:39 ` Alexandre Courbot 1 sibling, 1 reply; 7+ messages in thread From: kernel test robot @ 2025-07-06 0:39 UTC (permalink / raw) To: Daniel Almeida, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich, Boris Brezillon, Sebastian Reichel, Liam Girdwood, Mark Brown, Benno Lossin Cc: llvm, oe-kbuild-all, linux-kernel, rust-for-linux, Daniel Almeida Hi Daniel, kernel test robot noticed the following build errors: [auto build test ERROR on 2009a2d5696944d85c34d75e691a6f3884e787c0] url: https://github.com/intel-lab-lkp/linux/commits/Daniel-Almeida/rust-regulator-add-a-bare-minimum-regulator-abstraction/20250705-024526 base: 2009a2d5696944d85c34d75e691a6f3884e787c0 patch link: https://lore.kernel.org/r/20250704-topics-tyr-regulator-v7-1-77bfca2e22dc%40collabora.com patch subject: [PATCH v7 1/2] rust: regulator: add a bare minimum regulator abstraction config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20250706/202507060837.ziqhqXVk-lkp@intel.com/config) compiler: clang version 20.1.7 (https://github.com/llvm/llvm-project 6146a88f60492b520a36f8f8f3231e15f3cc6082) rustc: rustc 1.88.0 (6b00bc388 2025-06-23) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250706/202507060837.ziqhqXVk-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202507060837.ziqhqXVk-lkp@intel.com/ All errors (new ones prefixed by >>): >> error[E0425]: cannot find function `regulator_set_voltage` in crate `bindings` --> rust/kernel/regulator.rs:258:23 | 258 | bindings::regulator_set_voltage(self.inner.as_ptr(), min_uv.0, max_uv.0) | ^^^^^^^^^^^^^^^^^^^^^ not found in `bindings` -- >> error[E0425]: cannot find function `regulator_get_voltage` in crate `bindings` --> rust/kernel/regulator.rs:265:42 | 265 | let voltage = unsafe { bindings::regulator_get_voltage(self.inner.as_ptr()) }; | ^^^^^^^^^^^^^^^^^^^^^ not found in `bindings` -- >> error[E0425]: cannot find function `regulator_get` in crate `bindings` --> rust/kernel/regulator.rs:276:53 | 276 | let inner = from_err_ptr(unsafe { bindings::regulator_get(dev.as_raw(), name.as_ptr()) })?; | ^^^^^^^^^^^^^ not found in `bindings` -- >> error[E0425]: cannot find function `regulator_enable` in crate `bindings` --> rust/kernel/regulator.rs:290:38 | 290 | to_result(unsafe { bindings::regulator_enable(self.inner.as_ptr()) }) | ^^^^^^^^^^^^^^^^ not found in `bindings` -- >> error[E0425]: cannot find function `regulator_disable` in crate `bindings` --> rust/kernel/regulator.rs:295:38 | 295 | to_result(unsafe { bindings::regulator_disable(self.inner.as_ptr()) }) | ^^^^^^^^^^^^^^^^^ not found in `bindings` -- >> error[E0425]: cannot find function `regulator_is_enabled` in crate `bindings` --> rust/kernel/regulator.rs:379:28 | 379 | unsafe { bindings::regulator_is_enabled(self.inner.as_ptr()) != 0 } | ^^^^^^^^^^^^^^^^^^^^ not found in `bindings` -- >> error[E0425]: cannot find function `regulator_disable` in crate `bindings` --> rust/kernel/regulator.rs:389:32 | 389 | unsafe { bindings::regulator_disable(self.inner.as_ptr()) }; | ^^^^^^^^^^^^^^^^^ not found in `bindings` -- >> error[E0425]: cannot find function `regulator_put` in crate `bindings` --> rust/kernel/regulator.rs:393:28 | 393 | unsafe { bindings::regulator_put(self.inner.as_ptr()) }; | ^^^^^^^^^^^^^ not found in `bindings` -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v7 1/2] rust: regulator: add a bare minimum regulator abstraction 2025-07-06 0:39 ` kernel test robot @ 2025-07-07 16:23 ` Daniel Almeida 0 siblings, 0 replies; 7+ messages in thread From: Daniel Almeida @ 2025-07-07 16:23 UTC (permalink / raw) To: kernel test robot Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich, Boris Brezillon, Sebastian Reichel, Liam Girdwood, Mark Brown, Benno Lossin, llvm, oe-kbuild-all, linux-kernel, rust-for-linux […] > All errors (new ones prefixed by >>): > >>> error[E0425]: cannot find function `regulator_set_voltage` in crate `bindings` > --> rust/kernel/regulator.rs:258:23 > | > 258 | bindings::regulator_set_voltage(self.inner.as_ptr(), min_uv.0, max_uv.0) > | ^^^^^^^^^^^^^^^^^^^^^ not found in `bindings` > -- >>> error[E0425]: cannot find function `regulator_get_voltage` in crate `bindings` > --> rust/kernel/regulator.rs:265:42 > | > 265 | let voltage = unsafe { bindings::regulator_get_voltage(self.inner.as_ptr()) }; > | ^^^^^^^^^^^^^^^^^^^^^ not found in `bindings` > -- >>> error[E0425]: cannot find function `regulator_get` in crate `bindings` > --> rust/kernel/regulator.rs:276:53 > | > 276 | let inner = from_err_ptr(unsafe { bindings::regulator_get(dev.as_raw(), name.as_ptr()) })?; > | ^^^^^^^^^^^^^ not found in `bindings` > -- >>> error[E0425]: cannot find function `regulator_enable` in crate `bindings` > --> rust/kernel/regulator.rs:290:38 > | > 290 | to_result(unsafe { bindings::regulator_enable(self.inner.as_ptr()) }) > | ^^^^^^^^^^^^^^^^ not found in `bindings` > -- >>> error[E0425]: cannot find function `regulator_disable` in crate `bindings` > --> rust/kernel/regulator.rs:295:38 > | > 295 | to_result(unsafe { bindings::regulator_disable(self.inner.as_ptr()) }) > | ^^^^^^^^^^^^^^^^^ not found in `bindings` > -- >>> error[E0425]: cannot find function `regulator_is_enabled` in crate `bindings` > --> rust/kernel/regulator.rs:379:28 > | > 379 | unsafe { bindings::regulator_is_enabled(self.inner.as_ptr()) != 0 } > | ^^^^^^^^^^^^^^^^^^^^ not found in `bindings` > -- >>> error[E0425]: cannot find function `regulator_disable` in crate `bindings` > --> rust/kernel/regulator.rs:389:32 > | > 389 | unsafe { bindings::regulator_disable(self.inner.as_ptr()) }; > | ^^^^^^^^^^^^^^^^^ not found in `bindings` > -- >>> error[E0425]: cannot find function `regulator_put` in crate `bindings` > --> rust/kernel/regulator.rs:393:28 > | > 393 | unsafe { bindings::regulator_put(self.inner.as_ptr()) }; > | ^^^^^^^^^^^^^ not found in `bindings` > > -- > 0-DAY CI Kernel Test Service > https://github.com/intel/lkp-tests/wiki > I think these need to be in rust/helpers if CONFIG_REGULATOR is not set, because the stubs are inline functions. I will try this on the next iteration. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v7 1/2] rust: regulator: add a bare minimum regulator abstraction 2025-07-04 18:43 ` [PATCH v7 1/2] rust: regulator: add a bare minimum regulator abstraction Daniel Almeida 2025-07-06 0:39 ` kernel test robot @ 2025-07-07 5:39 ` Alexandre Courbot 1 sibling, 0 replies; 7+ messages in thread From: Alexandre Courbot @ 2025-07-07 5:39 UTC (permalink / raw) To: Daniel Almeida, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich, Boris Brezillon, Sebastian Reichel, Liam Girdwood, Mark Brown, Benno Lossin Cc: linux-kernel, rust-for-linux Hi Daniel, On Sat Jul 5, 2025 at 3:43 AM JST, Daniel Almeida wrote: > Add a bare minimum regulator abstraction to be used by Rust drivers. > This abstraction adds a small subset of the regulator API, which is > thought to be sufficient for the drivers we have now. > > Regulators provide the power needed by many hardware blocks and thus are > likely to be needed by a lot of drivers. > > It was tested on rk3588, where it was used to power up the "mali" > regulator in order to power up the GPU. > > Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com> I think this looks great. Reviewed-by: Alexandre Courbot <acourbot@nvidia.com> A few comments inline. > --- > rust/bindings/bindings_helper.h | 1 + > rust/kernel/lib.rs | 1 + > rust/kernel/regulator.rs | 403 ++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 405 insertions(+) > > diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h > index 8cbb660e2ec218021d16e6e0144acf6f4d7cca13..2d51f9d056091e34120b4ade9ff7cc4a7f53e111 100644 > --- a/rust/bindings/bindings_helper.h > +++ b/rust/bindings/bindings_helper.h > @@ -65,6 +65,7 @@ > #include <linux/poll.h> > #include <linux/property.h> > #include <linux/refcount.h> > +#include <linux/regulator/consumer.h> > #include <linux/sched.h> > #include <linux/security.h> > #include <linux/slab.h> > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs > index 6b4774b2b1c37f4da1866e993be6230bc6715841..5e4cd8c5e6ff1c4af52a5b1be4c4c32b5104e233 100644 > --- a/rust/kernel/lib.rs > +++ b/rust/kernel/lib.rs > @@ -100,6 +100,7 @@ > pub mod prelude; > pub mod print; > pub mod rbtree; > +pub mod regulator; > pub mod revocable; > pub mod security; > pub mod seq_file; > diff --git a/rust/kernel/regulator.rs b/rust/kernel/regulator.rs > new file mode 100644 > index 0000000000000000000000000000000000000000..56f3a91469549551f54c7b4c7ec67aa941acd572 > --- /dev/null > +++ b/rust/kernel/regulator.rs > @@ -0,0 +1,403 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +//! Regulator abstractions, providing a standard kernel interface to control > +//! voltage and current regulators. > +//! > +//! The intention is to allow systems to dynamically control regulator power > +//! output in order to save power and prolong battery life. This applies to both > +//! voltage regulators (where voltage output is controllable) and current sinks > +//! (where current limit is controllable). > +//! > +//! C header: [`include/linux/regulator/consumer.h`](srctree/include/linux/regulator/consumer.h) > +//! > +//! Regulators are modeled in Rust with a collection of states. Each state may > +//! enforce a given invariant, and they may convert between each other where applicable. > +//! > +//! See [Voltage and current regulator API](https://docs.kernel.org/driver-api/regulator.html) > +//! for more information. > + > +use crate::{ > + bindings, > + device::Device, > + error::{from_err_ptr, to_result, Result}, > + prelude::*, > +}; > + > +use core::{marker::PhantomData, mem::ManuallyDrop, ptr::NonNull}; > + > +mod private { > + pub trait Sealed {} > + > + impl Sealed for super::Enabled {} > + impl Sealed for super::Disabled {} > + impl Sealed for super::Dynamic {} > +} > + > +/// A trait representing the different states a [`Regulator`] can be in. > +pub trait RegulatorState: private::Sealed + 'static { > + /// Whether the regulator should be disabled when dropped. > + const DISABLE_ON_DROP: bool; > +} > + > +/// A state where the [`Regulator`] is known to be enabled. For safety, let's maybe explicitly state that the enable reference held by this state is released when it is dropped. The example below mentions it, so I think we should do it here as well. <snip> > +/// A `struct regulator` abstraction. > +/// > +/// # Examples > +/// > +/// ## Enabling a regulator > +/// > +/// This example uses [`Regulator<Enabled>`], which is suitable for drivers that > +/// enable a regulator at probe time and leave them on until the device is > +/// removed or otherwise shutdown. > +/// > +/// These users can store [`Regulator<Enabled>`] directly in their driver's > +/// private data struct. > +/// > +/// ``` > +/// # use kernel::prelude::*; > +/// # use kernel::c_str; > +/// # use kernel::device::Device; > +/// # use kernel::regulator::{Microvolt, Regulator, Disabled, Enabled}; > +/// fn enable(dev: &Device, min_uv: Microvolt, max_uv: Microvolt) -> Result { > +/// // Obtain a reference to a (fictitious) regulator. > +/// let regulator: Regulator<Disabled> = Regulator::<Disabled>::get(dev, c_str!("vcc"))?; > +/// > +/// // The voltage can be set before enabling the regulator if needed, e.g.: > +/// regulator.set_voltage(min_uv, max_uv)?; > +/// > +/// // The same applies for `get_voltage()`, i.e.: > +/// let voltage: Microvolt = regulator.get_voltage()?; > +/// > +/// // Enables the regulator, consuming the previous value. > +/// // > +/// // From now on, the regulator is known to be enabled because of the type > +/// // `Enabled`. > +/// // > +/// // If this operation fails, the `Error` will contain the regulator > +/// // reference, so that the operation may be retried. > +/// let regulator: Regulator<Enabled> = > +/// regulator.try_into_enabled().map_err(|error| error.error)?; > +/// > +/// // The voltage can also be set after enabling the regulator, e.g.: > +/// regulator.set_voltage(min_uv, max_uv)?; > +/// > +/// // The same applies for `get_voltage()`, i.e.: > +/// let voltage: Microvolt = regulator.get_voltage()?; > +/// > +/// // Dropping an enabled regulator will disable it. The refcount will be > +/// // decremented. > +/// drop(regulator); > +/// > +/// // ... > +/// > +/// Ok(()) > +/// } > +/// ``` > +/// > +/// A more concise shortcut is available for enabling a regulator. This is > +/// equivalent to `regulator_get_enable()`: > +/// > +/// ``` > +/// # use kernel::prelude::*; > +/// # use kernel::c_str; > +/// # use kernel::device::Device; > +/// # use kernel::regulator::{Microvolt, Regulator, Enabled}; > +/// fn enable(dev: &Device, min_uv: Microvolt, max_uv: Microvolt) -> Result { > +/// // Obtain a reference to a (fictitious) regulator and enable it. > +/// let regulator: Regulator<Enabled> = Regulator::<Enabled>::get(dev, c_str!("vcc"))?; > +/// > +/// // Dropping an enabled regulator will disable it. The refcount will be > +/// // decremented. > +/// drop(regulator); > +/// > +/// // ... > +/// > +/// Ok(()) > +/// } > +/// ``` > +/// > +/// ## Disabling a regulator > +/// > +/// ``` > +/// # use kernel::prelude::*; > +/// # use kernel::device::Device; > +/// # use kernel::regulator::{Regulator, Enabled, Disabled}; > +/// fn disable(dev: &Device, regulator: Regulator<Enabled>) -> Result { > +/// // We can also disable an enabled regulator without reliquinshing our > +/// // refcount: > +/// // > +/// // If this operation fails, the `Error` will contain the regulator > +/// // reference, so that the operation may be retried. > +/// let regulator: Regulator<Disabled> = > +/// regulator.try_into_disabled().map_err(|error| error.error)?; > +/// > +/// // The refcount will be decremented when `regulator` is dropped. > +/// drop(regulator); > +/// > +/// // ... > +/// > +/// Ok(()) > +/// } > +/// ``` > +/// > +/// ## Using [`Regulator<Dynamic>`] > +/// > +/// This example mimics the behavior of the C API, where the user is in > +/// control of the enabled reference count. This is useful for drivers that > +/// might call enable and disable to manage the `enable` reference count at > +/// runtime, perhaps as a result of `open()` and `close()` calls or whatever > +/// other driver-specific or subsystem-specific hooks. > +/// > +/// ``` > +/// # use kernel::prelude::*; > +/// # use kernel::c_str; > +/// # use kernel::device::Device; > +/// # use kernel::regulator::{Regulator, Dynamic}; > +/// struct PrivateData { > +/// regulator: Regulator<Dynamic>, > +/// } > +/// > +/// // A fictictious probe function that obtains a regulator and sets it up. > +/// fn probe(dev: &Device) -> Result<PrivateData> { > +/// // Obtain a reference to a (fictitious) regulator. > +/// let mut regulator = Regulator::<Dynamic>::get(dev, c_str!("vcc"))?; > +/// > +/// // Enable the regulator. The type is still `Regulator<Dynamic>`. > +/// regulator.enable()?; If we enable the regulator at probe-time, then it is guaranteed to be enabled for the time the device is bound - in that case, doesn't it turn the `enable()` and `disable()` calls in `open` and `close` into no-ops? I would remove this line as the `enable()` method is demonstrated in `open()` anyway. > +/// > +/// Ok(PrivateData { regulator }) > +/// } > +/// > +/// // A fictictious function that indicates that the device is going to be used. > +/// fn open(dev: &Device, data: &mut PrivateData) -> Result { > +/// // Increase the `enabled` reference count. > +/// data.regulator.enable()?; > +/// > +/// Ok(()) > +/// } > +/// > +/// fn close(dev: &Device, data: &mut PrivateData) -> Result { > +/// // Decrease the `enabled` reference count. > +/// data.regulator.disable()?; > +/// > +/// Ok(()) > +/// } > +/// > +/// fn remove(dev: &Device, data: PrivateData) -> Result { > +/// // `PrivateData` is dropped here, which will drop the > +/// // `Regulator<Dynamic>` in turn. > +/// // > +/// // The reference that was obtained by `regulator_get()` will be > +/// // released, but it is up to the user to make sure that the number of calls > +/// // to `enable()` and `disabled()` are balanced before this point. Ironically the `enable()` line in `probe()` is not balanced here. :) > +/// Ok(()) > +/// } > +/// ``` These examples are great! <snip> > +/// A voltage in microvolts. > +/// > +/// The explicit type is used to avoid confusion with other multiples of the > +/// volt, which can be disastrous. > +#[repr(transparent)] > +#[derive(Copy, Clone, PartialEq, Eq)] > +pub struct Microvolt(pub i32); I still think a `Voltage` type is preferable and more idiomatic here as the unit would be made explicit by the methods anyway, but your call. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v7 2/2] MAINAINTERS: add regulator.rs to the regulator API entry 2025-07-04 18:43 [PATCH v7 0/2] Add a bare-minimum Regulator abstraction Daniel Almeida 2025-07-04 18:43 ` [PATCH v7 1/2] rust: regulator: add a bare minimum regulator abstraction Daniel Almeida @ 2025-07-04 18:43 ` Daniel Almeida 2025-07-07 5:39 ` Alexandre Courbot 1 sibling, 1 reply; 7+ messages in thread From: Daniel Almeida @ 2025-07-04 18:43 UTC (permalink / raw) To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich, Boris Brezillon, Sebastian Reichel, Liam Girdwood, Mark Brown, Benno Lossin Cc: linux-kernel, rust-for-linux, Daniel Almeida Add this file to the regulator API entry as requested by Mark Brown. Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com> --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index c3f7fbd0d67afe8376ea84bc17d70e9fa4b89bf6..235a33b013473b6ff83c4fbd67c0b635c2ed2366 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -26522,6 +26522,7 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git F: Documentation/devicetree/bindings/regulator/ F: Documentation/power/regulator/ F: drivers/regulator/ +F: rust/kernel/regulator.rs F: include/dt-bindings/regulator/ F: include/linux/regulator/ K: regulator_get_optional -- 2.50.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v7 2/2] MAINAINTERS: add regulator.rs to the regulator API entry 2025-07-04 18:43 ` [PATCH v7 2/2] MAINAINTERS: add regulator.rs to the regulator API entry Daniel Almeida @ 2025-07-07 5:39 ` Alexandre Courbot 0 siblings, 0 replies; 7+ messages in thread From: Alexandre Courbot @ 2025-07-07 5:39 UTC (permalink / raw) To: Daniel Almeida, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich, Boris Brezillon, Sebastian Reichel, Liam Girdwood, Mark Brown, Benno Lossin Cc: linux-kernel, rust-for-linux On Sat Jul 5, 2025 at 3:43 AM JST, Daniel Almeida wrote: > Add this file to the regulator API entry as requested by Mark Brown. > > Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com> Just to point out that "MAINTAINERS" is misspelled in the subject. :) ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-07-07 16:24 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-07-04 18:43 [PATCH v7 0/2] Add a bare-minimum Regulator abstraction Daniel Almeida 2025-07-04 18:43 ` [PATCH v7 1/2] rust: regulator: add a bare minimum regulator abstraction Daniel Almeida 2025-07-06 0:39 ` kernel test robot 2025-07-07 16:23 ` Daniel Almeida 2025-07-07 5:39 ` Alexandre Courbot 2025-07-04 18:43 ` [PATCH v7 2/2] MAINAINTERS: add regulator.rs to the regulator API entry Daniel Almeida 2025-07-07 5:39 ` Alexandre Courbot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).