Re: [PATCH v5 1/4] rust: clk: use the type-state pattern

On Mon Aug 3, 2026 at 9:29 PM JST, Gary Guo wrote:
> On Mon Aug 3, 2026 at 5:00 AM BST, Alexandre Courbot wrote:
>> On Mon Aug 3, 2026 at 3:30 AM JST, Gary Guo wrote:
>>> On Mon Jul 6, 2026 at 3:37 PM BST, Daniel Almeida wrote:
>> <...>
>>>> It solves d) by directly encoding the state of the Clk into the type, e.g.:
>>>> Clk<Enabled> is now known to be a Clk that is enabled.
>>>
>>> The design conflate states with actions. Our existing type state for devices
>>> don't do this: `Device<Bound>` means that the device is currently bound, not
>>> that dropping it will unbind it. Yet, a `Clk<Prepared>` doesn't mean just that
>>> "clock is prepared" but rather "clock is prepared and needs to be unprepared on
>>> drop".
>>>
>>> One way around this is to mimic the "Registration" pattern: have a type to
>>> indicate that a `Clk` has been prepared and its `drop` will undo it, and then
>>> this type can `Deref` to `Clk<Prepared>` which just mean a prepared clock.
>>
>> Just as the driver core hands over `&Device<Bound>` to a driver as a
>> guarantee that the device is currently bound, so can the driver pass a
>> `&Clk<Prepared>` to a function to assert a similar proof. Here the
>> reference only means "clock is prepared", without any action implied.
>>
>> The typestate has real practical benefits, as unlike `Device` which has
>> a well-defined life cycle entirely controlled by the driver core, clock
>> handles are owned by drivers and their use can go all over the place.
>>
>> Driver A might want to enable a clock in short bursts in order to
>> preserve power, and keep it prepared otherwise. For this, a
>> `Clk<Prepared>` with the `EnabledGuard` I mentioned in patch 2 would be
>> a good fit. Driver B might need to keep a given clock enabled all the
>> time and only change its rate, and thus will store a `Clk<Enabled>`.
>> Driver C may have different PM states, and can encode these in an enum
>> where relevant clocks are either `Prepared` or `Enabled` depending on
>> the variant.
>>
>> Mandating a registration-like pattern here looks a bit overkill to me
>> and I am not sure what this would grant us. It would definitely
>> introduce some complexity: say that you want to keep a prepared clock in
>> your driver data, does it mean you need to store the `Clk` itself, and
>> then its prepared guard, which references the `Clk` in the same
>> structure?
>
> You could have the `PreparedGuard` takes a reference to the clock, no need to
> store `Clk` separately. We can have a method that gives out `Clk<Prepared<'_>>`.

The tricky part is "take a reference to the clock". `struct clk` does
not have its own get/put counter, so in order for the guard to not be
constrained by lifetimes, we would need to add our own sharing
mechanism, which is basically what patch 2 of this series does.

My main problem with patch 2 is that it adds additional constraints
(heap allocation) for a Rust driver to keep several references to the
same clock handle. A C driver doesn't need to do that; a Rust driver
shouldn't need to either.

The typestate pattern of patch 1 is nice and simple, but it also adds
constraints of its own to the C API, in that a clock handle can
contribute at most a single prepare and a single enable count to the
clock. Patch 2 tries to work around that limitation by adding the
reference count we wish `struct clk` had; but at the end of the day what
it really does is create another indirection for clock handles, and each
of these indirections can still only contribute a single prepare/enable
count to the clock.

Adding guards alleviates that limitation, with the caveat that the `Clk`
that provided these guards cannot transition into another state while
any guard exists, as the transition methods consume it. And these guards
cannot easily be stored long-term - not without unsafe code anyway.

So after sleeping twice on it, I still cannot think of a design that
would solve it all. But I sense that providing both the typestate and
guards would largely cover most use-cases until we converge towards the
perfect fit for the C API.