Message-Id: <DG5HJPIYMS7B.152WPGG9MZZJ5@garyguo.net>
Date: Tue, 03 Feb 2026 16:59:48 +0000
From: "Gary Guo" <gary@...yguo.net>
To: "Daniel Almeida" <daniel.almeida@...labora.com>, "Boris Brezillon"
 <boris.brezillon@...labora.com>
Cc: "Alice Ryhl" <aliceryhl@...gle.com>, "Maxime Ripard"
 <mripard@...nel.org>, "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>,
 "Gary Guo" <gary@...yguo.net>, 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 Tue Feb 3, 2026 at 4:28 PM GMT, Daniel Almeida wrote:
> <snip>
>
>>> 
>>> The problem with what you have suggested is that the previous state is not
>>> consumed if you can clone it, and consuming the previous state is a pretty key
>>> element in ensuring you cannot misuse it. For example, here:
>>> 
>>> let enabled_clk = prepared_clk.clone().enable()?;
>>> // do stuff
>>> // enabled_clk goes out of scope and releases the enable
>>> // ref it had
>>> 
>>> prepared_clk is still alive.
>> 
>> That was intentional in this example. Think about a prepared_clk that's
>> stored in some driver-internal object, because you want to keep the clk
>> prepared at all times between the probe() and unbind(). Then you have
>> some sections where you want to briefly enable the clk to access
>> registers, and immediately disable it when you're done. The clone()
>> here guarantees that the initial prepared_clk stays valid.
>> 
>> If you were to disable, unprepare and put the clk when enabled_clk goes
>> out of scope, you'd go
>
>> 
>> let enabled_clk = prepared_clk.enable()?;
>> 
>> and that would still work, it's just not the same use-case.
>> 
>
> Ok, let’s have clone() then.
>
>
>>> Now, this may not be the end of the world in this
>>> particular case, but for API consistency, I'd say we should probably avoid this
>>> behavior.
>>> 
>>> I see that Alice suggested a closure approach. IMHO, we should use that
>>> instead.
>> 
>> The closure, while being useful for the above local clk-enablement
>> example, doesn't allow for passing some Clk<Enabled> guard around, like
>> you would do with a lock Guard, and I believe that's a useful thing to
>> have.
>
>
> Wdym? You’d still get a &Clk<Enabled> that you can pass around, i.e.:
>
>    self.prepared_clk.with_enabled(|clk: &Clk<Enabled> | {
>        ... use registers, pass &Clk<Enabled> as needed
>    });
>
> This is now not about clone() vs not clone(), but more about limiting the scope of the
> Enabled state, which would cater to the use-case you mentioned IIUC.

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

>
> — Daniel

Powered by blists - more mailing lists