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

"Gary Guo" <[email protected]> Sun, 02 Aug 2026 19:30:43 +0100
Newsgroups org.kernel.vger.linux-pwm,org.freedesktop.lists.dri-devel,org.infradead.lists.linux-riscv,org.kernel.vger.linux-clk,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pm,org.kernel.vger.rust-for-linux
Message-ID <[email protected]>
On Mon Jul 6, 2026 at 3:37 PM BST, Daniel Almeida wrote:
> The current Clk abstraction can still be improved on the following issues=
:
>
> a) It only keeps track of a count to clk_get(), which means that users ha=
ve
> to manually call disable() and unprepare(), or a variation of those, like
> disable_unprepare().
>
> b) It allows repeated calls to prepare() or enable(), but it keeps no tra=
ck
> of how often these were called, i.e., it's currently legal to write the
> following:
>
> clk.prepare();
> clk.prepare();
> clk.enable();
> clk.enable();
>
> And nothing gets undone on drop().
>
> c) It adds a OptionalClk type that is probably not needed. There is no
> "struct optional_clk" in C and we should probably not add one.
>
> d) It does not let a user express the state of the clk through the
> type system. For example, there is currently no way to encode that a Clk =
is
> enabled via the type system alone.
>
> In light of the Regulator abstraction that was recently merged, switch th=
is
> abstraction to use the type-state pattern instead. It solves both a) and =
b)
> by establishing a number of states and the valid ways to transition betwe=
en
> them. It also automatically undoes any call to clk_get(), clk_prepare() a=
nd
> clk_enable() as applicable on drop(), so users do not have to do anything
> special before Clk goes out of scope.
>
> It solves c) by removing the OptionalClk type, which is now simply encode=
d
> as a Clk whose inner pointer is NULL.

This change could be a commit on its own?

>
> 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 device=
s
don't do this: `Device<Bound>` means that the device is currently bound, no=
t
that dropping it will unbind it. Yet, a `Clk<Prepared>` doesn't mean just t=
hat
"clock is prepared" but rather "clock is prepared and needs to be unprepare=
d 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 th=
en
this type can `Deref` to `Clk<Prepared>` which just mean a prepared clock.

>
> The INVARIANTS section for Clk is expanded to highlight the relationship
> between the states and the respective reference counts that are owned by
> each of them.
>
> The examples are expanded to highlight how a user can transition between
> states, as well as highlight some of the shortcuts built into the API.
>
> The current implementation is also more flexible, in the sense that it
> allows for more states to be added in the future. This lets us implement
> different strategies for handling clocks, including one that mimics the
> current API, allowing for multiple calls to prepare() and enable().
>
> The users (cpufreq.rs/ rcpufreq_dt.rs) were updated by this patch (and no=
t
> a separate one) to reflect the new changes. This is needed, because
> otherwise this patch would break the build.
>
> Link: https://crates.io/crates/sealed [1]

Link that doesn't seem to be referenced?

Best,
Gary


> Signed-off-by: Daniel Almeida <[email protected]>