Re: [PATCH v3 2/3] rust: clk: add devres-managed clks

Gary Guo
2026-01-19T14:33:41Z

On Wed Jan 7, 2026 at 3:09 PM GMT, Daniel Almeida wrote:
> The clk API allows fine-grained control, but some drivers might be
> more interested in a "set and forget" API.
>
> Expand the current API to support this. The clock will automatically be
> disabled, unprepared and freed when the device is unbound from the bus
> without further intervention by the driver.
>
> Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
> ---
> rust/kernel/clk.rs | 43 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
> diff --git a/rust/kernel/clk.rs b/rust/kernel/clk.rs
> index 6323b40dc7ba..e840e7c20af7 100644
> --- a/rust/kernel/clk.rs
> +++ b/rust/kernel/clk.rs
> @@ -95,6 +95,49 @@ impl Sealed for super::Prepared {}
> impl Sealed for super::Enabled {}
> }
>
> + /// Obtains and enables a [`devres`]-managed [`Clk`] for a bound device.
> + ///
> + /// [`devres`]: crate::devres::Devres
> + pub fn devm_enable(dev: &Device<Bound>, name: Option<&CStr>) -> Result {
> + let name = name.map_or(ptr::null(), |n| n.as_char_ptr());
> +
> + // SAFETY: It is safe to call [`devm_clk_get_enabled`] with a valid
> + // device pointer.
> + from_err_ptr(unsafe { bindings::devm_clk_get_enabled(dev.as_raw(), name) })?;
> + Ok(())
> + }

Would it make sense to have them as assoc functions instead of standalone?

Regardless, the code looks correct to me.

Reviewed-by: Gary Guo <gary@garyguo.net>

Best,
Gary

> +
> + /// Obtains and enables a [`devres`]-managed [`Clk`] for a bound device.
> + ///
> + /// This does not print any error messages if the clock is not found.
> + ///
> + /// [`devres`]: crate::devres::Devres
> + pub fn devm_enable_optional(dev: &Device<Bound>, name: Option<&CStr>) -> Result {
> + let name = name.map_or(ptr::null(), |n| n.as_char_ptr());
> +
> + // SAFETY: It is safe to call [`devm_clk_get_optional_enabled`] with a
> + // valid device pointer.
> + from_err_ptr(unsafe { bindings::devm_clk_get_optional_enabled(dev.as_raw(), name) })?;
> + Ok(())
> + }
> +
> + /// Same as [`devm_enable_optional`], but also sets the rate.
> + pub fn devm_enable_optional_with_rate(
> + dev: &Device,
> + name: Option<&CStr>,
> + rate: Hertz,
> + ) -> Result {
> + let name = name.map_or(ptr::null(), |n| n.as_char_ptr());
> +
> + // SAFETY: It is safe to call
> + // [`devm_clk_get_optional_enabled_with_rate`] with a valid device
> + // pointer.
> + from_err_ptr(unsafe {
> + bindings::devm_clk_get_optional_enabled_with_rate(dev.as_raw(), name, rate.as_hz())
> + })?;
> + Ok(())
> + }
> +
> /// A trait representing the different states that a [`Clk`] can be in.
> pub trait ClkState: private::Sealed {
> /// Whether the clock should be disabled when dropped.