Re: [PATCH V3 2/2] rust: Add initial clk abstractions

[Stephen Boyd]

Quoting Viresh Kumar (2025-03-05 03:46:59)
> On 04-03-25, 10:37, Miguel Ojeda wrote:
> > On Tue, Mar 4, 2025 at 9:53 AM Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
> > > +///     clk.disable_unprepare();
> > 
> > Looking at the example, a question that one may have is: should we
> > have something like a scope guard or a closure-passing API for this,
> > or does it not make sense in general?
> 
> Something like this (untested) ?
> 
> +/// Runs a cleanup function/closure when dropped.
> +///
> +/// The [`ClkGuard::dismiss`] function prevents the cleanup function from running.
> +///
> +pub type ClkGuard<'a> = ScopeGuard<&'a Clk, fn(&Clk)>;
> +
>  /// A reference-counted clock.
>  ///
>  /// This represents the Rust abstraction for the C [`struct clk`].
> @@ -139,10 +146,12 @@ pub fn as_raw(&self) -> *mut bindings::clk {
>      ///
>      /// [`clk_enable`]: https://docs.kernel.org/core-api/kernel-api.html#c.clk_enable
>      #[inline]
> -    pub fn enable(&self) -> Result {
> +    pub fn enable(&self) -> Result<ClkGuard<'_>> {
>          // SAFETY: By the type invariants, it is safe to call clk APIs of the C code for a clock
>          // pointer earlier returned by [`clk_get`].
> -        to_result(unsafe { bindings::clk_enable(self.as_raw()) })
> +        to_result(unsafe { bindings::clk_enable(self.as_raw()) })?;
> +
> +        Ok(ClkGuard::new_with_data(self, Clk::disable))

Does this mean that a clk consumer has to keep the Result returned from
enable() in scope until they want to disable the clk? I don't see how
that makes sense, because most of the time a consumer will enable a clk
during probe and leave it enabled until system suspend or runtime PM
suspend time. At that point, they would disable the clk explicitly with
disable(), but now they would need to drop a reference to do that?