linux-kernel - Re: [PATCH V3 2/2] rust: Add initial clk abstractions

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 for Android: free password hash cracker in your pocket

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

Message-ID: <2c17361891c4eb7edd947e5384cc9741.sboyd@kernel.org>
Date: Wed, 05 Mar 2025 14:31:02 -0800
From: Stephen Boyd <sboyd@...nel.org>
To: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>, Viresh Kumar <viresh.kumar@...aro.org>
Cc: Michael Turquette <mturquette@...libre.com>, 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>, Russell King <linux@...linux.org.uk>, linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org, Vincent Guittot <vincent.guittot@...aro.org>, Daniel Almeida <daniel.almeida@...labora.com>
Subject: Re: [PATCH V3 2/2] rust: Add initial clk abstractions

Quoting Viresh Kumar (2025-03-05 03:46:59)
> On 04-03-25, 10:37, Miguel Ojeda wrote:
> > On Tue, Mar 4, 2025 at 9:53 AM Viresh Kumar <viresh.kumar@...aro.org> wrote:
> > > +///     clk.disable_unprepare();
> > 
> > Looking at the example, a question that one may have is: should we
> > have something like a scope guard or a closure-passing API for this,
> > or does it not make sense in general?
> 
> Something like this (untested) ?
> 
> +/// Runs a cleanup function/closure when dropped.
> +///
> +/// The [`ClkGuard::dismiss`] function prevents the cleanup function from running.
> +///
> +pub type ClkGuard<'a> = ScopeGuard<&'a Clk, fn(&Clk)>;
> +
>  /// A reference-counted clock.
>  ///
>  /// This represents the Rust abstraction for the C [`struct clk`].
> @@ -139,10 +146,12 @@ pub fn as_raw(&self) -> *mut bindings::clk {
>      ///
>      /// [`clk_enable`]: https://docs.kernel.org/core-api/kernel-api.html#c.clk_enable
>      #[inline]
> -    pub fn enable(&self) -> Result {
> +    pub fn enable(&self) -> Result<ClkGuard<'_>> {
>          // SAFETY: By the type invariants, it is safe to call clk APIs of the C code for a clock
>          // pointer earlier returned by [`clk_get`].
> -        to_result(unsafe { bindings::clk_enable(self.as_raw()) })
> +        to_result(unsafe { bindings::clk_enable(self.as_raw()) })?;
> +
> +        Ok(ClkGuard::new_with_data(self, Clk::disable))

Does this mean that a clk consumer has to keep the Result returned from
enable() in scope until they want to disable the clk? I don't see how
that makes sense, because most of the time a consumer will enable a clk
during probe and leave it enabled until system suspend or runtime PM
suspend time. At that point, they would disable the clk explicitly with
disable(), but now they would need to drop a reference to do that?