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

[Daniel Almeida]

Hi Boris,

> On 3 Feb 2026, at 06:17, Boris Brezillon <boris.brezillon at collabora.com> wrote:
> 
> Hello Daniel,
> 
> On Wed, 07 Jan 2026 12:09:52 -0300
> Daniel Almeida <daniel.almeida at collabora.com> wrote:
> 
>> -        /// Disable and unprepare the clock.
>> +    impl Clk<Enabled> {
>> +        /// Gets [`Clk`] corresponding to a bound [`Device`] and a connection id
>> +        /// and then prepares and enables it.
>>         ///
>> -        /// Equivalent to calling [`Clk::disable`] followed by [`Clk::unprepare`].
>> +        /// Equivalent to calling [`Clk::get`], followed by [`Clk::prepare`],
>> +        /// followed by [`Clk::enable`].
>>         #[inline]
>> -        pub fn disable_unprepare(&self) {
>> -            // SAFETY: By the type invariants, self.as_raw() is a valid argument for
>> -            // [`clk_disable_unprepare`].
>> -            unsafe { bindings::clk_disable_unprepare(self.as_raw()) };
>> +        pub fn get(dev: &Device<Bound>, name: Option<&CStr>) -> Result<Clk<Enabled>> {
>> +            Clk::<Prepared>::get(dev, name)?
>> +                .enable()
>> +                .map_err(|error| error.error)
>> +        }
>> +
>> +        /// Behaves the same as [`Self::get`], except when there is no clock
>> +        /// producer. In this case, instead of returning [`ENOENT`], it returns
>> +        /// a dummy [`Clk`].
>> +        #[inline]
>> +        pub fn get_optional(dev: &Device<Bound>, name: Option<&CStr>) -> Result<Clk<Enabled>> {
>> +            Clk::<Prepared>::get_optional(dev, name)?
>> +                .enable()
>> +                .map_err(|error| error.error)
>> +        }
>> +
>> +        /// Attempts to disable the [`Clk`] and convert it to the [`Prepared`]
>> +        /// state.
>> +        #[inline]
>> +        pub fn disable(self) -> Result<Clk<Prepared>, Error<Enabled>> {
>> +            // We will be transferring the ownership of our `clk_get()` and
>> +            // `clk_enable()` counts to `Clk<Prepared>`.
>> +            let clk = ManuallyDrop::new(self);
>> +
>> +            // SAFETY: By the type invariants, `self.0` is a valid argument for
>> +            // [`clk_disable`].
>> +            unsafe { bindings::clk_disable(clk.as_raw()) };
>> +
>> +            Ok(Clk {
>> +                inner: clk.inner,
>> +                _phantom: PhantomData,
>> +            })
>>         }
>> 
>>         /// Get clock's rate.
> 
> Dunno if this has been mentioned already, but I belive the rate
> getter/setter should be in the generic implementation. Indeed, it's
> quite common for clock users to change the rate when the clk is
> disabled to avoid unstable transitional state. The usual pattern for
> that is:
> 
> - clk_set_parent(my_clk, secondary_parent)
> - clk_disable[_unprepare](primary_parent) // (usually a PLL)
> - clk_set_rate(primary_parent)
> - clk[_prepare]_enable(primary_parent)
> - clk_set_parent(my_clk, primary_parent)
> 
> The case where the clk rate is changed while the clk is active is also
> valid (usually fine when it's just a divider that's changed, because
> there's no stabilization period).
> 
>> @@ -252,83 +429,31 @@ pub fn set_rate(&self, rate: Hertz) -> Result {
>>         }
>>     }
> 


I’m ok with this. I just assumed that these operations were only valid on enabled clks.

Will change this in the next version.

— Daniel