> On 19 Jan 2026, at 09:35, Alice Ryhl <[email protected]> wrote:
>
> 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);
Not sure I understand. You can get a Clk<Enabled>, do what you need, and then
consume Clk<Enabled> to go back to Clk<Prepared>. I think I added this, but if
I didn’t, it’s a trivial thing to do.
>
> 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.
Right, I need to update the docs to account for this, as they imply that you
can do this with only two states.
>
>>> 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.
I think we should discourage RawClk if at all possible. But if the consensus
is that we *really* need this easily-abused thing, I can provide a follow-up.
>
> The current type-state based API is the least-boilerplate option for
> drivers that exist today.
>
> Alice
