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

"Gary Guo" <[email protected]>
Newsgroups gmane.linux.pwm,gmane.linux.power-management.general,gmane.linux.kernel,gmane.comp.video.dri.devel,gmane.linux.ports.riscv,gmane.linux.kernel.clk,gmane.linux.kernel.rust
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 have
> 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 track
> 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 this
> 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 between
> them. It also automatically undoes any call to clk_get(), clk_prepare() and
> 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 encoded
> 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 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.

>
> 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 not
> 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]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.