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]>