Re: [PATCH v6 2/7] rust: clk: replace `OptionalClk` with `Clk::get_optional()`

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] rust: clk: missing #[inline] on get_optional() abstraction wrapper
--

commit 796077cfd7119a6144686ab76ca00413f2a3071b
Author: Daniel Almeida <daniel.almeida@collabora.com>
rust: clk: replace OptionalClk with Clk::get_optional()

This commit replaces the OptionalClk wrapper with a get_optional() method
on the Clk struct, as clk_get_optional() already returns a valid struct clk
pointer. The tyr driver is converted to use this new API.

> diff --git a/rust/kernel/clk.rs b/rust/kernel/clk.rs
> index c73fe7c572511..1de2484df8cf5 100644
> --- a/rust/kernel/clk.rs
> +++ b/rust/kernel/clk.rs
[ ... ]
> @@ -175,6 +172,25 @@ pub fn get(dev: &Device, name: Option<&CStr>) -> Result<Self> {

[Severity: Low]
This problem wasn't introduced by this patch, but should this preexisting
function have an #[inline] attribute? According to the Rust subsystem
guidelines, small functions or functions forwarding to a C binding call in
abstraction crates should be annotated with #[inline].

>              })?))
>          }
>  
> +        /// 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`].
> +        ///
> +        /// Equivalent to the kernel's [`clk_get_optional`] API.
> +        ///
> +        /// [`clk_get_optional`]:
> +        /// https://docs.kernel.org/core-api/kernel-api.html#c.clk_get_optional
> +        pub fn get_optional(dev: &Device, name: Option<&CStr>) -> Result<Self> {

[Severity: Low]
Should this new function also include the #[inline] attribute? Since it
primarily prepares arguments and forwards them to
bindings::clk_get_optional(), adding #[inline] would align it with the
subsystem guidelines for abstraction wrappers.

> +            let con_id = name.map_or(ptr::null(), |n| n.as_char_ptr());
> +
> +            // SAFETY: It is safe to call [`clk_get_optional`] for a valid device pointer.
> +            //
> +            // INVARIANT: The reference-count is decremented when [`Clk`] goes out of scope.
> +            Ok(Self(from_err_ptr(unsafe {
> +                bindings::clk_get_optional(dev.as_raw(), con_id)
> +            })?))
> +        }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013-clk-type-state-v6-0-a5e9977b3509@collabora.com?part=2