linux-kernel - Re: [PATCH RESEND v2] rust: regulator: add a bare minimum regulator abstraction

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a98eb789-4c49-4607-ad15-76e260888d64@sirena.org.uk>
Date: Wed, 26 Mar 2025 18:56:54 +0000
From: Mark Brown <broonie@...nel.org>
To: Daniel Almeida <daniel.almeida@...labora.com>
Cc: Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>,
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>,
	Benno Lossin <benno.lossin@...ton.me>,
	Andreas Hindborg <a.hindborg@...nel.org>,
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
	Danilo Krummrich <dakr@...nel.org>,
	Boris Brezillon <boris.brezillon@...labora.com>,
	Sebastian Reichel <sebastian.reichel@...labora.com>,
	lgirdwood@...il.com, linux-kernel@...r.kernel.org,
	rust-for-linux@...r.kernel.org
Subject: Re: [PATCH RESEND v2] rust: regulator: add a bare minimum regulator
 abstraction

On Wed, Mar 26, 2025 at 03:39:33PM -0300, Daniel Almeida wrote:

This is flagged as a resend but appears to be the first copy I've
seen...

> 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 are modeled with two types: [`Regulator`] and
> +//! [`EnabledRegulator`].

It would be helpful to see what the client code actually looks like
here.

> +    /// Enables the regulator.
> +    pub fn enable(self) -> Result<EnabledRegulator> {
> +        // SAFETY: Safe as per the type invariants of `Regulator`.
> +        let res = to_result(unsafe { bindings::regulator_enable(self.inner.as_ptr()) });
> +        res.map(|()| EnabledRegulator { inner: self })
> +    }

I assume this map does soemthing to report errors?

> +impl EnabledRegulator {

> +    /// Disables the regulator.
> +    pub fn disable(self) -> Result<Regulator> {
> +        // Keep the count on `regulator_get()`.
> +        let regulator = ManuallyDrop::new(self);
> +
> +        // SAFETY: Safe as per the type invariants of `Self`.
> +        let res = to_result(unsafe { bindings::regulator_disable(regulator.as_ptr()) });
> +
> +        res.map(|()| Regulator {
> +            inner: regulator.inner.inner,
> +        })
> +    }

This looks like user code could manually call it which feels like asking
for trouble?

> +    /// Sets the voltage for the regulator.
> +    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.as_ptr(), min_uv.0, max_uv.0) })
> +    }

set_voltage() is only implemented for enabled regulators.  It is
reasonable for a driver to want to set the voltage for a regulator prior
to enabling it in order to ensure that the device powers up cleanly,
there may be different requirements for power on from on and idle so the
constraints may not be enough to ensure that a device can power on
cleanly.

> +    /// 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.as_ptr()) };
> +        if voltage < 0 {
> +            Err(Error::from_errno(voltage))
> +        } else {
> +            Ok(Microvolt(voltage))
> +        }
> +    }

Same issue here.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)