linux-kernel - Re: [PATCH v3 1/3] rust: clk: use the type-state pattern

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: <20260204-cunning-strange-bittern-2bb569@houat>
Date: Wed, 4 Feb 2026 10:18:08 +0100
From: Maxime Ripard <mripard@...nel.org>
To: Boris Brezillon <boris.brezillon@...labora.com>
Cc: Gary Guo <gary@...yguo.net>, 
	Daniel Almeida <daniel.almeida@...labora.com>, Alice Ryhl <aliceryhl@...gle.com>, 
	"Rafael J. Wysocki" <rafael@...nel.org>, Viresh Kumar <viresh.kumar@...aro.org>, 
	Danilo Krummrich <dakr@...nel.org>, Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 
	Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, 
	Simona Vetter <simona@...ll.ch>, Drew Fustini <fustini@...nel.org>, Guo Ren <guoren@...nel.org>, 
	Fu Wei <wefu@...hat.com>, Uwe Kleine-König <ukleinek@...nel.org>, 
	Michael Turquette <mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>, Miguel Ojeda <ojeda@...nel.org>, 
	Boqun Feng <boqun.feng@...il.com>, Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Benno Lossin <lossin@...nel.org>, Andreas Hindborg <a.hindborg@...nel.org>, 
	Trevor Gross <tmgross@...ch.edu>, linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	dri-devel@...ts.freedesktop.org, linux-riscv@...ts.infradead.org, linux-pwm@...r.kernel.org, 
	linux-clk@...r.kernel.org, rust-for-linux@...r.kernel.org
Subject: Re: [PATCH v3 1/3] rust: clk: use the type-state pattern

On Wed, Feb 04, 2026 at 09:11:04AM +0100, Boris Brezillon wrote:
> Hi Gary, Daniel,
> 
> On Tue, 03 Feb 2026 20:36:30 +0000
> "Gary Guo" <gary@...yguo.net> wrote:
> 
> > On Tue Feb 3, 2026 at 7:26 PM GMT, Daniel Almeida wrote:
> > >  
> > >> 
> > >> I think it's fine to have all of these:
> > >> * `Clone` impl
> > >> * `enable` which consumes `Clk<Prepared>` by value and spit out `Clk<Enabled>`
> > >> * `with_enabled` that gives `&Clk<Enabled>`
> > >> 
> > >> This way, if you only want to enable in short time, you can do `with_enabled`.
> > >> If the closure callback wants to keep clock enabled for longer, it can just do
> > >> `.clone()` inside the closure and obtain an owned `Clk<Enabled>`.
> > >> 
> > >> If the user just have a reference and want to enable the callback they can do
> > >> `prepared_clk.clone().enable()` which gives an owned `Clk<Enabled>`. Thoughts?
> > >> 
> > >> Best,
> > >> Gary  
> > >
> > >
> > > I’m ok with what you proposed above. The only problem is that implementing
> > > clone() is done through an Arc<*mut bindings::clk>  in Boris’ current
> > > design, so this requires an extra allocation.  
> > 
> > Hmm, that's a very good point. `struct clk` is already a reference into
> > clk_core, so having to put another level of indirection over is not ideal.
> > However, if we're going to keep C code unchanged and do a zero-cost abstraction
> > on the Rust side, then we won't be able to have have multiple prepare/enable to
> > the same `struct clk` with the current design.
> > 
> > It feels like we can to do a trade-off and choose from:
> > 1. Not be able to have multiple prepare/enable calls on the same `clk` (this can
> >    limit users that need dynamically enable/disable clocks, with the very limited
> >    exception that closure-callback is fine).
> > 2. Do an extra allocation
> > 3. Put lifetime on types that represent a prepared/enabled `Clk`
> > 4. Change C to make `struct clk` refcounted.
> 
> It probably comes to no surprise that I'd be more in favor of option 2
> or 4. Maybe option 2 first, so we can get the user-facing API merged
> without too much churn, and then we can see if the clk maintainers are
> happy adding a refcnt to struct clk to optimize things.
> 
> If we really feel that the indirection/memory overhead is going to
> hurt us, we can also start with option 1, and extend it to 2 and/or 4
> (needed to add a Clone support) when it becomes evident we can't do
> without it. But as I was saying in my previous reply to Daniel, I
> expect the extra indirection/memory overhead to be negligible since:
> 
> 1. clks are usually not {prepared,enabled}/{disabled,unprepared} in a
>    hot path
> 2. in the rare occasions where they might be ({dev,cpu}freq ?), this
>    clk state change is usually one operation in an ocean of other
>    slower operations (regulator reconfiguration, for instance, which
>    usually goes over a slow I2C bus, or a
>    relatively-faster-but-still-slow SPI one, at least when we compare
>    it to an IoMem access for in-SoCs clks). So overall, the clk state
>    change might account for a very small portion of the CPU cycles
>    spent in this bigger operation

I'm not even too worried about that one. devfreq and cpufreq will
typically adjust the clock rate while device is active, and thus the
clock is enabled. So we shouldn't have a prepared -> enabled transition
in the path that adjust the device / cpu rate, it would have happened
before.

Maxime

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