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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20260204-angelic-vermilion-beagle-fd1507@houat>
Date: Wed, 4 Feb 2026 15:34:29 +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 Wed, Feb 04, 2026 at 09:43:55AM -0300, Daniel Almeida wrote:
> > 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.

A maintenance cost to pay by every driver is kind of the textbook
definition of boilerplate to me.

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

And yet, that framework has dozens of helpers that do not remove
anything from drivers but a couple of lines. So surely its users must
find value in reducing that boilerplate as much as possible. And you do
implement some of them, so you must find value in that too.

> > 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.
>
> 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.:

Out of context and not what I meant, but ok.

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

Nothing is absolute. It is indeed an improvement on the refcounting side
of things and general safety of the API for the general case. I don't
think I ever questionned that.

However, for the use-cases we've been discussing (and dozens of drivers
implementing it), it also comes with a regression in the amount of code
to create and maintain. They used to be able to only deal with the Clk
structure, and now they can't anymore.

You might find that neglible, you might have a plan to address that in
the future, etc. and that's fine, but if you can't acknowledge that it's
indeed happening, there's no point in me raising the issue and
continuing the discussion.

Maxime

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