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

[Boris Brezillon <boris.brezillon@collabora.com>]

On Tue, 3 Feb 2026 16:26:22 -0300
Daniel Almeida <daniel.almeida@collabora.com> 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,

It's actually Arc<RawClk> with

    struct RawClk(*mut bindings::clk);

    impl Drop for RawClk {
        fn drop(&mut self) {
            // SAFETY: By the type invariants, self.as_raw() is a valid argument for // [`clk_put`].
            unsafe { bindings::clk_put(self.0) };
        }
    }

This is because struct clk is not refcounted, so cloning
implies wrapping this object in an Arc, and only calling
clk_put() when the Arc refcnt reaches zero.

> so this requires an extra allocation.

That's true. But the memory overhead should be pretty negligible,
and I don't think the extra indirection makes any noticeable
difference for an actual clk implementation (one that's not a NOP),
since we have indirections all over the place already (clk -> clk_hw,
clk_ops, ...). So I think I'd value ease of use over this small
perfs/mem-usage hit.