All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Gary Guo" <gary@garyguo.net>
To: "Daniel Almeida" <daniel.almeida@collabora.com>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Viresh Kumar" <viresh.kumar@linaro.org>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Drew Fustini" <fustini@kernel.org>,
	"Guo Ren" <guoren@kernel.org>, "Fu Wei" <wefu@redhat.com>,
	"Uwe Kleine-König" <ukleinek@kernel.org>,
	"Michael Turquette" <mturquette@baylibre.com>,
	"Stephen Boyd" <sboyd@kernel.org>,
	"Miguel Ojeda" <ojeda@kernel.org>, "Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Michal Wilczynski" <m.wilczynski@samsung.com>,
	"Boqun Feng" <boqun@kernel.org>
Cc: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
	dri-devel@lists.freedesktop.org, linux-riscv@lists.infradead.org,
	linux-pwm@vger.kernel.org, linux-clk@vger.kernel.org,
	rust-for-linux@vger.kernel.org,
	"Boris Brezillon" <boris.brezillon@collabora.com>,
	"Onur Özkan" <work@onurozkan.dev>, Maurice <mhi@mailbox.org>
Subject: Re: [PATCH v5 1/4] rust: clk: use the type-state pattern
Date: Sun, 02 Aug 2026 19:30:43 +0100	[thread overview]
Message-ID: <DKEO7DXQ8604.1CB09QFM2EO68@garyguo.net> (raw)
In-Reply-To: <20260706-clk-type-state-v5-1-67c5f326a16c@collabora.com>

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 <daniel.almeida@collabora.com>


WARNING: multiple messages have this Message-ID (diff)
From: "Gary Guo" <gary@garyguo.net>
To: "Daniel Almeida" <daniel.almeida@collabora.com>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Viresh Kumar" <viresh.kumar@linaro.org>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Drew Fustini" <fustini@kernel.org>,
	"Guo Ren" <guoren@kernel.org>, "Fu Wei" <wefu@redhat.com>,
	"Uwe Kleine-König" <ukleinek@kernel.org>,
	"Michael Turquette" <mturquette@baylibre.com>,
	"Stephen Boyd" <sboyd@kernel.org>,
	"Miguel Ojeda" <ojeda@kernel.org>, "Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Michal Wilczynski" <m.wilczynski@samsung.com>,
	"Boqun Feng" <boqun@kernel.org>
Cc: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
	dri-devel@lists.freedesktop.org, linux-riscv@lists.infradead.org,
	linux-pwm@vger.kernel.org, linux-clk@vger.kernel.org,
	rust-for-linux@vger.kernel.org,
	"Boris Brezillon" <boris.brezillon@collabora.com>,
	"Onur Özkan" <work@onurozkan.dev>, Maurice <mhi@mailbox.org>
Subject: Re: [PATCH v5 1/4] rust: clk: use the type-state pattern
Date: Sun, 02 Aug 2026 19:30:43 +0100	[thread overview]
Message-ID: <DKEO7DXQ8604.1CB09QFM2EO68@garyguo.net> (raw)
In-Reply-To: <20260706-clk-type-state-v5-1-67c5f326a16c@collabora.com>

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 <daniel.almeida@collabora.com>


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

  parent reply	other threads:[~2026-08-02 18:30 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-06 14:37 [PATCH v5 0/4] Clk improvements Daniel Almeida
2026-07-06 14:37 ` Daniel Almeida
2026-07-06 14:37 ` [PATCH v5 1/4] rust: clk: use the type-state pattern Daniel Almeida
2026-07-06 14:37   ` Daniel Almeida
2026-08-01 14:33   ` Alexandre Courbot
2026-08-01 14:33     ` Alexandre Courbot
2026-08-02 18:30   ` Gary Guo [this message]
2026-08-02 18:30     ` Gary Guo
2026-08-03  4:00     ` Alexandre Courbot
2026-08-03  4:00       ` Alexandre Courbot
2026-08-03 12:29       ` Gary Guo
2026-08-03 12:29         ` Gary Guo
2026-08-06 13:54         ` Alexandre Courbot
2026-08-06 13:54           ` Alexandre Courbot
2026-07-06 14:37 ` [PATCH v5 2/4] rust: clk: implement Clone for Clk<T> Daniel Almeida
2026-07-06 14:37   ` Daniel Almeida
2026-08-02  1:05   ` Alexandre Courbot
2026-08-02  1:05     ` Alexandre Courbot
2026-08-02 18:32     ` Gary Guo
2026-08-02 18:32       ` Gary Guo
2026-07-06 14:37 ` [PATCH v5 3/4] rust: clk: add devres-managed clks Daniel Almeida
2026-07-06 14:37   ` Daniel Almeida
2026-07-06 14:51   ` sashiko-bot
2026-08-01 11:22   ` Onur Özkan
2026-08-01 11:22     ` Onur Özkan
2026-08-02  5:24   ` Alexandre Courbot
2026-08-02  5:24     ` Alexandre Courbot
2026-08-02 18:34   ` Gary Guo
2026-08-02 18:34     ` Gary Guo
2026-08-03  3:57     ` Alexandre Courbot
2026-08-03  3:57       ` Alexandre Courbot
2026-08-03 12:32       ` Gary Guo
2026-08-03 12:32         ` Gary Guo
2026-07-06 14:37 ` [PATCH v5 4/4] rust: clk: use 'kernel vertical style' for imports Daniel Almeida
2026-07-06 14:37   ` Daniel Almeida
2026-08-01 17:44   ` Onur Özkan
2026-08-01 17:44     ` Onur Özkan
2026-08-02  5:36   ` Alexandre Courbot
2026-08-02  5:36     ` Alexandre Courbot
2026-07-31 18:37 ` [PATCH v5 0/4] Clk improvements Maurice Hieronymus
2026-07-31 18:37   ` Maurice Hieronymus
2026-07-31 21:32   ` Daniel Almeida
2026-07-31 21:32     ` Daniel Almeida
2026-08-03 13:06     ` Brian Masney
2026-08-03 13:06       ` Brian Masney
2026-08-03 13:24       ` Daniel Almeida
2026-08-03 13:24         ` Daniel Almeida

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DKEO7DXQ8604.1CB09QFM2EO68@garyguo.net \
    --to=gary@garyguo.net \
    --cc=a.hindborg@kernel.org \
    --cc=airlied@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=boris.brezillon@collabora.com \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=fustini@kernel.org \
    --cc=guoren@kernel.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=lossin@kernel.org \
    --cc=m.wilczynski@samsung.com \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mhi@mailbox.org \
    --cc=mripard@kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=ojeda@kernel.org \
    --cc=rafael@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=sboyd@kernel.org \
    --cc=simona@ffwll.ch \
    --cc=tmgross@umich.edu \
    --cc=tzimmermann@suse.de \
    --cc=ukleinek@kernel.org \
    --cc=viresh.kumar@linaro.org \
    --cc=wefu@redhat.com \
    --cc=work@onurozkan.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.