On Tue, 3 Feb 2026 10:37:15 -0300 Daniel Almeida <[email protected]> wrote:
> > On 3 Feb 2026, at 06:09, Boris Brezillon <[email protected]> > > wrote: > > > > Hello Daniel, > > > > On Mon, 2 Feb 2026 17:10:38 +0100 > > Boris Brezillon <[email protected]> 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.
