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: <1ad3e7e2f8f2fc375b472d7676e47f5d.sboyd@kernel.org>
Date: Thu, 06 Mar 2025 12:58:58 -0800
From: Stephen Boyd <sboyd@...nel.org>
To: Viresh Kumar <viresh.kumar@...aro.org>
Cc: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>, 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 20:40:28)
> On 05-03-25, 14:31, Stephen Boyd wrote:
> > Does this mean that a clk consumer has to keep the Result returned from
> > enable() in scope until they want to disable the clk?
> 
> Yes and no.
> 
> > 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?
> 
> Broadly there are two type of clk users I believe:
> 
> 1. clk is enabled / disabled from same routine:
> 
>    In this case the result can be kept in a local variable and the matching
>    cleanup fn will be called at exit.

This is almost never the case. Listing these as two types of clk users
tries to make the two equal, when the vast majority of users are the
second. Please don't.

> 
>    fn transfer_data(...) -> Result {
>         let _guard = clk.enable()?;
> 
>         ...
>         transfer-data here
>         ...
>         // clk.disable() will be called automatically as soon as _guard goes out
>         // of scope.
>    }
> 
> 2. clk is enabled / disabled from different routines:
> 
>    In this case the caller needs to call dismiss to avoid the automatic freeing
>    of resource. Alternatively the returned value can be stored too somewhere,
>    but I am not sure if it what users will end up doing.
> 
>    fn probe(...) -> Result {
>         clk.enable()?.dismiss();

Yuck. Can't we tie the lifetime of the clk to the consumer device driver
so that when the driver is unbound the clk is dropped and it decrements
all the enables/prepares and puts the clk with clk_put()? A ScopeGuard
could probably be used for that on the struct Clk itself, but we would
want to track the enables and prepares in the rust wrapper code until
the struct clk can be inspected directly.

The problem is we don't know how a platform may implement the clk API,
and CCF hasn't taken over the entire kernel yet so we can't rely on some
private API between the CCF and the rust wrapper to know how many
clk_disable()s to call, or even rely on clk_put() to do the work for us.
Can the rust wrappers depend on CONFIG_COMMON_CLK? If they did then we
could have some private API between rust and CCF. We probably don't want
rust code to _not_ use COMMON_CLK underneath so we can encourage the
last few holdouts to migrate to CCF. I'd lean towards depending on
COMMON_CLK for the rust wrappers in this case.