[PATCH v3 1/3] rust: clk: use the type-state pattern

[Daniel Almeida]

> 
> I'm probably missing something then, but let's assume you have a driver
> that wants its clock prepared and enabled in an hypothetical enable()
> callback, and disabled / unprepared in a disable() callback.
> 
> From a PM management perspective, this usecase makes total sense, is a
> valid usecase, is widely used in the kernel, and is currently supported
> by both the C and Rust clk APIs.
> 
> The only solution to this you suggested so far (I think?) to implement
> this on top of the new clk API you propose is to have a driver specific
> enum that would store each of the possible state transition.

Yes, you need an enum _if_ you want to model transitions at runtime. IIUC you
only need two variants to implement the pattern you described. I do not
consider this  “boilerplate”, but rather a small cost to pay.

I would understand if this was some elaborate pattern that had to be
implemented by all drivers, but a two-variant enum is as straightforward as it
gets.


> 
> That's the boilerplate I'm talking about. If every driver wanting to
> implement that pattern has to make such an enum, with all the relevant
> traits implementation that might come with it, we go from an API where
> everything works at no-cost from a code-size perspective to a situation
> where every driver has to develop and maintain that enum.
> 
> Maxime

There are no "traits that come with it". It's just an enum, with two variants.

> API where everything works at no-cost

The previous API was far from “everything works”. It was fundamentally
broken by design in multiple ways, i.e.:

> 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().

IMHO, what we have here is an improvement that has been long overdue.

— Daniel