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

[Alice Ryhl <aliceryhl@google.com>]

On Mon, Jan 19, 2026 at 11:45:57AM +0100, Maxime Ripard wrote:
> On Thu, Jan 08, 2026 at 11:14:37AM -0300, Daniel Almeida wrote:
> > > For example, it's quite typical to have (at least) one clock for the bus
> > > interface that drives the register, and one that drives the main
> > > component logic. The former needs to be enabled only when you're
> > > accessing the registers (and can be abstracted with
> > > regmap_mmio_attach_clk for example), and the latter needs to be enabled
> > > only when the device actually starts operating.
> > > 
> > > You have a similar thing for the prepare vs enable thing. The difference
> > > between the two is that enable can be called into atomic context but
> > > prepare can't.
> > > 
> > > So for drivers that would care about this, you would create your device
> > > with an unprepared clock, and then at various times during the driver
> > > lifetime, you would mutate that state.

The case where you're doing it only while accessing registers is
interesting, because that means the Enable bit may be owned by a local
variable. We may imagine an:

    let enabled = self.prepared_clk.enable_scoped();
    ... use registers
    drop(enabled);

Now ... this doesn't quite work with the current API - the current
Enabled stated owns both a prepare and enable count, but the above keeps
the prepare count in `self` and the enabled count in a local variable.
But it could be done with a fourth state, or by a closure method:

    self.prepared_clk.with_enabled(|| {
        ... use registers
    });

All of this would work with an immutable variable of type Clk<Prepared>.

> > > AFAIU, encoding the state of the clock into the Clk type (and thus
> > > forcing the structure that holds it) prevents that mutation. If not, we
> > > should make it clearer (by expanding the doc maybe?) how such a pattern
> > > can be supported.
> > > 
> > > Maxime
> > 
> > IIUC, your main point seems to be about mutating the state at runtime? This is
> > possible with this code. You can just have an enum, for example:
> > 
> > enum MyClocks {
> >     Unprepared(Clk<Unprepared>),
> >     Prepared(Clk<Prepared>),
> >     Enabled(Clk<Enabled>), 
> > }

I believe you need an extra state if the state is not bound to the scope
of a function:

enum MyClocks {
    Unprepared(Clk<Unprepared>),
    Prepared(Clk<Prepared>),
    Enabled(Clk<Enabled>), 
    Transitioning,
}

since mem::replace() needs a new value before you can take ownership of
the existing Clk value.

> > In fact, I specifically wanted to ensure that this was possible when writing
> > these patches, as it’s needed by drivers. If you want to, I can cover that in
> > the examples, no worries.
> 
> Yes, that would be great. I do wonder though if it wouldn't make sense
> to turn it the other way around. It creates a fair share of boilerplate
> for a number of drivers. Can't we keep Clk the way it is as a
> lower-level type, and crate a ManagedClk (or whatever name you prefer)
> that drivers can use, and would be returned by higher-level helpers, if
> they so choose?
> 
> That way, we do have the typestate API for whoever wants to, without
> creating too much boilerplate for everybody else.

I think that if you still want an API where you just call enable/disable
directly on it with no protection against unbalanced calls, then that
should be the special API. Probably called RawClk and functions marked
unsafe. Unbalanced calls seem really dangerous and use should not be
encouraged.

The current type-state based API is the least-boilerplate option for
drivers that exist today.

Alice