Message-ID: <20260204-nickel-seal-of-poetry-8fdefb@houat>
Date: Wed, 4 Feb 2026 10:15:45 +0100
From: Maxime Ripard <mripard@...nel.org>
To: Daniel Almeida <daniel.almeida@...labora.com>
Cc: Danilo Krummrich <dakr@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>, 
	"Rafael J. Wysocki" <rafael@...nel.org>, Viresh Kumar <viresh.kumar@...aro.org>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Thomas Zimmermann <tzimmermann@...e.de>, 
	David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, 
	Drew Fustini <fustini@...nel.org>, Guo Ren <guoren@...nel.org>, Fu Wei <wefu@...hat.com>, 
	Uwe Kleine-König <ukleinek@...nel.org>, Michael Turquette <mturquette@...libre.com>, 
	Stephen Boyd <sboyd@...nel.org>, Miguel Ojeda <ojeda@...nel.org>, 
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, 
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, Benno Lossin <lossin@...nel.org>, 
	Andreas Hindborg <a.hindborg@...nel.org>, Trevor Gross <tmgross@...ch.edu>, linux-pm@...r.kernel.org, 
	linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org, linux-riscv@...ts.infradead.org, 
	linux-pwm@...r.kernel.org, linux-clk@...r.kernel.org, rust-for-linux@...r.kernel.org
Subject: Re: [PATCH v3 1/3] rust: clk: use the type-state pattern

On Thu, Jan 22, 2026 at 09:29:30PM -0300, Daniel Almeida wrote:
> 
> 
> > On 22 Jan 2026, at 10:44, Maxime Ripard <mripard@...nel.org> wrote:
> > 
> > On Mon, Jan 19, 2026 at 03:37:17PM +0100, Danilo Krummrich wrote:
> >> On Mon Jan 19, 2026 at 3:18 PM CET, Maxime Ripard wrote:
> >>> On Mon, Jan 19, 2026 at 02:13:48PM +0100, Danilo Krummrich wrote:
> >>>> On Mon Jan 19, 2026 at 1:54 PM CET, Daniel Almeida wrote:
> >>>>>> On 19 Jan 2026, at 09:35, Alice Ryhl <aliceryhl@...gle.com> wrote:
> >>>>>> I think that if you still want an API where you just call enable/disable
> >>>>>> directly on it with no protection against unbalanced calls, then that
> >>>>>> should be the special API. Probably called RawClk and functions marked
> >>>>>> unsafe. Unbalanced calls seem really dangerous and use should not be
> >>>>>> encouraged.
> >>>> 
> >>>> +1; and unless there is a use-case that requires otherwise, it should not even
> >>>> be possible to do this at all -- at least for driver code.
> >>> 
> >>> I mean, it's great, it's safe, etc. but it's also suboptimal from a PM
> >>> perspective on many platforms. It's totally fine to provide nice, safe,
> >>> ergonomic wrappers for the drivers that don't care (or can't, really),
> >>> but treating a legitimate optimisation as something we should consider
> >>> impossible to do is just weird to me.
> >> 
> >> I said that an unsafe API with potentially unbalanced calls is something we
> >> should clearly avoid for drivers. This is *not* equivalent to "treating a
> >> legitimate optimisation as something we should consider impossible".
> >> 
> >> If we discover use-cases where the current API doesn't work well, we can
> >> invenstigate further.
> > 
> > I'm not sure I'm following what you're saying, sorry. I've pointed out
> > such a use-case already.
> > 
> >>>>> I think we should discourage RawClk if at all possible. But if the consensus
> >>>>> is that we *really* need this easily-abused thing, I can provide a follow-up.
> >>>> 
> >>>> I think we should only do this if there are use-case with no alternative, so far
> >>>> there haven't been any AFAIK.
> >>> 
> >>> I don't really care about which alternative we come up with, but look at
> >>> devm_regmap_init_mmio_clk for example. It is a valid use-case that
> >>> already exists today, and has had for more than a decade at this point.
> >> 
> >> I don't see the issue with devm_regmap_init_mmio_clk()? It takes a reference
> >> count of the clock and prepares it when called and unprepares the clk in drops
> >> its reference in regmap_mmio_free_context() called from the devres callback.
> >> 
> >> That something we can easily do with the current API, no?
> > 
> > The current one, yes. Doing that in the API suggested here would involve
> > some boilerplate in all those drivers they don't have right now.
> > 
> > Maxime
> 
> Maxime, I know you’ve already pointed out a use-case, but I think the
> confusion stems from why you seem to think that the current solution cannot
> cater to the API you mentioned in a clean way. You seem to imply that there
> will be a lot of boilerplate involved, but we (or I) cannot see this. Perhaps
> it would help if you highlighted how exactly the type state solution would be
> verbose using some pseudocode. I guess that would make your point clearer for
> us.

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.

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

Download attachment "signature.asc" of type "application/pgp-signature" (274 bytes)

Powered by blists - more mailing lists