All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Gary Guo" <gary@garyguo.net>
To: "Alexandre Courbot" <acourbot@nvidia.com>, "Gary Guo" <gary@garyguo.net>
Cc: "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>,
	"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>,
	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: Mon, 03 Aug 2026 13:29:29 +0100	[thread overview]
Message-ID: <DKFB5CL2I1FO.7ULPVEF0VG5Z@garyguo.net> (raw)
In-Reply-To: <DKF0BU8N7JJX.T802R11VZ0A5@nvidia.com>

On Mon Aug 3, 2026 at 5:00 AM BST, Alexandre Courbot wrote:
> On Mon Aug 3, 2026 at 3:30 AM JST, Gary Guo wrote:
>> On Mon Jul 6, 2026 at 3:37 PM BST, Daniel Almeida wrote:
> <...>
>>> 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.
>
> Just as the driver core hands over `&Device<Bound>` to a driver as a
> guarantee that the device is currently bound, so can the driver pass a
> `&Clk<Prepared>` to a function to assert a similar proof. Here the
> reference only means "clock is prepared", without any action implied.
>
> The typestate has real practical benefits, as unlike `Device` which has
> a well-defined life cycle entirely controlled by the driver core, clock
> handles are owned by drivers and their use can go all over the place.
>
> Driver A might want to enable a clock in short bursts in order to
> preserve power, and keep it prepared otherwise. For this, a
> `Clk<Prepared>` with the `EnabledGuard` I mentioned in patch 2 would be
> a good fit. Driver B might need to keep a given clock enabled all the
> time and only change its rate, and thus will store a `Clk<Enabled>`.
> Driver C may have different PM states, and can encode these in an enum
> where relevant clocks are either `Prepared` or `Enabled` depending on
> the variant.
>
> Mandating a registration-like pattern here looks a bit overkill to me
> and I am not sure what this would grant us. It would definitely
> introduce some complexity: say that you want to keep a prepared clock in
> your driver data, does it mean you need to store the `Clk` itself, and
> then its prepared guard, which references the `Clk` in the same
> structure?

You could have the `PreparedGuard` takes a reference to the clock, no need to
store `Clk` separately. We can have a method that gives out `Clk<Prepared<'_>>`.
The lifetime would be that of the guard, so the token cannot outlive
`PreparedGuard`.

(Using `klint` annotation below just to demonstrate if they can be used from
atomic context)

    struct PreparedGuard { ... }
    struct PrepareEnabledGuard { ... }
    struct EnabledGuard<'a> { ... }

    impl PreparedGuard {
        fn as_clk(&self) -> &Clk<Prepared<'_>> { .. }

        #[klint::atomic_context]
        fn enable(self) -> PrepareEnabledGuard { .. }
    }
    impl PrepareEnabledGuard {
        fn as_clk(&self) -> &Clk<Enabled<'_>> { .. }
    }
    impl<'a> EnabledGuard<'a> {
        fn as_clk(&self) -> &Clk<Enabled<'_>> { .. }
    }

    impl<S> RefCounted for Clk<S> {
        #[klint::atomic_context]
        unsafe fn inc_ref(..) {
           /* just add inc count here */
        }
    }

    impl<S> Clk<S> {
        #[klint::process_context]
        fn prepare(&self) -> PreparedGuard { ... }
        #[klint::process_context]
        fn prepare_enable(&self) -> PrepareEnabledGuard { ... }
    }

    impl Clk<Prepared<'_>> {
        #[klint::atomic_context]
        fn enable(&self) -> EnabledGuard { .. }
    }

Best,
Gary


WARNING: multiple messages have this Message-ID (diff)
From: "Gary Guo" <gary@garyguo.net>
To: "Alexandre Courbot" <acourbot@nvidia.com>, "Gary Guo" <gary@garyguo.net>
Cc: "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>,
	"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>,
	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: Mon, 03 Aug 2026 13:29:29 +0100	[thread overview]
Message-ID: <DKFB5CL2I1FO.7ULPVEF0VG5Z@garyguo.net> (raw)
In-Reply-To: <DKF0BU8N7JJX.T802R11VZ0A5@nvidia.com>

On Mon Aug 3, 2026 at 5:00 AM BST, Alexandre Courbot wrote:
> On Mon Aug 3, 2026 at 3:30 AM JST, Gary Guo wrote:
>> On Mon Jul 6, 2026 at 3:37 PM BST, Daniel Almeida wrote:
> <...>
>>> 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.
>
> Just as the driver core hands over `&Device<Bound>` to a driver as a
> guarantee that the device is currently bound, so can the driver pass a
> `&Clk<Prepared>` to a function to assert a similar proof. Here the
> reference only means "clock is prepared", without any action implied.
>
> The typestate has real practical benefits, as unlike `Device` which has
> a well-defined life cycle entirely controlled by the driver core, clock
> handles are owned by drivers and their use can go all over the place.
>
> Driver A might want to enable a clock in short bursts in order to
> preserve power, and keep it prepared otherwise. For this, a
> `Clk<Prepared>` with the `EnabledGuard` I mentioned in patch 2 would be
> a good fit. Driver B might need to keep a given clock enabled all the
> time and only change its rate, and thus will store a `Clk<Enabled>`.
> Driver C may have different PM states, and can encode these in an enum
> where relevant clocks are either `Prepared` or `Enabled` depending on
> the variant.
>
> Mandating a registration-like pattern here looks a bit overkill to me
> and I am not sure what this would grant us. It would definitely
> introduce some complexity: say that you want to keep a prepared clock in
> your driver data, does it mean you need to store the `Clk` itself, and
> then its prepared guard, which references the `Clk` in the same
> structure?

You could have the `PreparedGuard` takes a reference to the clock, no need to
store `Clk` separately. We can have a method that gives out `Clk<Prepared<'_>>`.
The lifetime would be that of the guard, so the token cannot outlive
`PreparedGuard`.

(Using `klint` annotation below just to demonstrate if they can be used from
atomic context)

    struct PreparedGuard { ... }
    struct PrepareEnabledGuard { ... }
    struct EnabledGuard<'a> { ... }

    impl PreparedGuard {
        fn as_clk(&self) -> &Clk<Prepared<'_>> { .. }

        #[klint::atomic_context]
        fn enable(self) -> PrepareEnabledGuard { .. }
    }
    impl PrepareEnabledGuard {
        fn as_clk(&self) -> &Clk<Enabled<'_>> { .. }
    }
    impl<'a> EnabledGuard<'a> {
        fn as_clk(&self) -> &Clk<Enabled<'_>> { .. }
    }

    impl<S> RefCounted for Clk<S> {
        #[klint::atomic_context]
        unsafe fn inc_ref(..) {
           /* just add inc count here */
        }
    }

    impl<S> Clk<S> {
        #[klint::process_context]
        fn prepare(&self) -> PreparedGuard { ... }
        #[klint::process_context]
        fn prepare_enable(&self) -> PrepareEnabledGuard { ... }
    }

    impl Clk<Prepared<'_>> {
        #[klint::atomic_context]
        fn enable(&self) -> EnabledGuard { .. }
    }

Best,
Gary


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

  reply	other threads:[~2026-08-03 12:29 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
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 [this message]
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=DKFB5CL2I1FO.7ULPVEF0VG5Z@garyguo.net \
    --to=gary@garyguo.net \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --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.