Re: [PATCH v5 1/4] rust: clk: use the type-state pattern
"Gary Guo" <[email protected]> Mon, 03 Aug 2026 13:29:29 +0100
| Newsgroups | org.kernel.vger.linux-pm,org.freedesktop.lists.dri-devel,org.infradead.lists.linux-riscv,org.kernel.vger.linux-clk,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pwm,org.kernel.vger.rust-for-linux |
|---|---|
| Message-ID | <[email protected]> |
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 dev=
ices
>> 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 jus=
t that
>> "clock is prepared" but rather "clock is prepared and needs to be unprep=
ared 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 cloc=
k.
>
> 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 lifetime would be that of the guard, so the token cannot outlive
`PreparedGuard`.
(Using `klint` annotation below just to demonstrate if they can be used fro=
m
atomic context)
struct PreparedGuard { ... }
struct PrepareEnabledGuard { ... }
struct EnabledGuard<'a> { ... }
impl PreparedGuard {
fn as_clk(&self) -> &Clk<Prepared<'_>> { .. }
#[klint::atomic_context]
fn enable(self) -> PrepareEnabledGuard { .. }
}
impl PrepareEnabledGuard {
fn as_clk(&self) -> &Clk<Enabled<'_>> { .. }
}
impl<'a> EnabledGuard<'a> {
fn as_clk(&self) -> &Clk<Enabled<'_>> { .. }
}
impl<S> RefCounted for Clk<S> {
#[klint::atomic_context]
unsafe fn inc_ref(..) {
/* just add inc count here */
}
}
impl<S> Clk<S> {
#[klint::process_context]
fn prepare(&self) -> PreparedGuard { ... }
#[klint::process_context]
fn prepare_enable(&self) -> PrepareEnabledGuard { ... }
}
impl Clk<Prepared<'_>> {
#[klint::atomic_context]
fn enable(&self) -> EnabledGuard { .. }
}
Best,
Gary