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

[Boris Brezillon <boris.brezillon@collabora.com>]

On Tue, 3 Feb 2026 10:37:15 -0300
Daniel Almeida <daniel.almeida@collabora.com> wrote:

> > On 3 Feb 2026, at 06:09, Boris Brezillon <boris.brezillon@collabora.com> wrote:
> > 
> > Hello Daniel,
> > 
> > On Mon, 2 Feb 2026 17:10:38 +0100
> > Boris Brezillon <boris.brezillon@collabora.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.