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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260203151828.669c29cb@fedora>
Date: Tue, 3 Feb 2026 15:18:28 +0100
From: Boris Brezillon <boris.brezillon@...labora.com>
To: Daniel Almeida <daniel.almeida@...labora.com>
Cc: Gary Guo <gary@...yguo.net>, "Rafael J. Wysocki" <rafael@...nel.org>,
 Viresh Kumar <viresh.kumar@...aro.org>, Danilo Krummrich <dakr@...nel.org>,
 Alice Ryhl <aliceryhl@...gle.com>, Maarten Lankhorst
 <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
 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>, 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 Tue, 3 Feb 2026 10:37:15 -0300
Daniel Almeida <daniel.almeida@...labora.com> wrote:

> > On 3 Feb 2026, at 06:09, Boris Brezillon <boris.brezillon@...labora.com> wrote:
> > 
> > Hello Daniel,
> > 
> > On Mon, 2 Feb 2026 17:10:38 +0100
> > Boris Brezillon <boris.brezillon@...labora.com> wrote:
> >   
> >>>> -#[pin_data(PinnedDrop)]
> >>>> +#[pin_data]
> >>>> pub(crate) struct TyrData {
> >>>>     pub(crate) pdev: ARef<platform::Device>,
> >>>> 
> >>>> @@ -92,13 +92,9 @@ fn probe(
> >>>>         pdev: &platform::Device<Core>,
> >>>>         _info: Option<&Self::IdInfo>,
> >>>>     ) -> impl PinInit<Self, Error> {
> >>>> -        let core_clk = Clk::get(pdev.as_ref(), Some(c_str!("core")))?;
> >>>> -        let stacks_clk = OptionalClk::get(pdev.as_ref(), Some(c_str!("stacks")))?;
> >>>> -        let coregroup_clk = OptionalClk::get(pdev.as_ref(), Some(c_str!("coregroup")))?;
> >>>> -
> >>>> -        core_clk.prepare_enable()?;
> >>>> -        stacks_clk.prepare_enable()?;
> >>>> -        coregroup_clk.prepare_enable()?;
> >>>> +        let core_clk = Clk::<Enabled>::get(pdev.as_ref(), Some(c_str!("core")))?;      
> >>> 
> >>> Ah, more turbofish.. I'd really want to avoid them if possible.
> >>> 
> >>> Any disadvantage on just ask the user to chain `.get().prepare_enable()?`? This
> >>> way it is also clear that some action is performed.    
> >> 
> >> I've just disc  
> > 
> > Sorry, I've hit the reply button before I had finished writing my
> > answer. So I was about to say that I had started writing something
> > similar without knowing this series existed, and I feel like we'd don't
> > really need those prepare_enable() shortcuts that exist in C. We might
> > has well just go:
> > 
> > Clk::get(dev, Some(c_str!("core"))).prepare()?.enable()?;
> > 
> > and have the following variant-specofoc functions
> > 
> > - Clk<Unprepared>::get[_optional]() (no get on Prepared and Enabled
> >  variants)
> > - Clk<Unprepared>::prepare()
> > - Clk<Prepared>::{enable,unprepare}()
> > - Clk<Enabled>::{disable}()
> > 
> > Regards,
> > 
> > Boris
> > 
> >   
> 
> 
> I don’t understand how is this better than the turbofish we currently have.
> 
> In other words, how is this:
> 
> Clk::get(dev, Some(c_str!("core"))).prepare()?.enable()?;
> 
> Better than this:
> 
> Clk::<Enabled>::get(/*…*/);

For one, it doesn't force you to expose multiple functions in the
implementation (::get[_optional]() is only present in the Unprepared
impl variant, no shortcut to combine state transition, ...) which means
less code to maintain overall. But I also prefer the fact this clearly
reflects the state transitions that exist to get an Enabled clk (first
you get an Unprepared clk that you have to prepare and enable to turn
that into an Enabled clk). That's a matter of taste of course, just
saying that if we get rid of the turbofish syntax like Gary suggested
at some point, I think I'd find it clearer to also just expose the
transitions between two consecutive states, and let the caller go
through all the steps.