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

["Gary Guo" <gary@garyguo.net>]

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