[PATCH] rust: opp: Move `cfg(CONFIG_OF)` attribute to the top of doc test

Viresh Kumar posted 1 patch 6 months, 3 weeks ago
rust/kernel/opp.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] rust: opp: Move `cfg(CONFIG_OF)` attribute to the top of doc test
Posted by Viresh Kumar 6 months, 3 weeks ago
Move the `#[cfg(CONFIG_OF)]` attribute to the top of the documentation test
block and hide it. This applies the condition to the entire test and improves
readability.

Placing configuration flags like `CONFIG_OF` at the top serves as a clear
indicator of the conditions under which the example is valid, effectively
acting like configuration metadata for the example itself.

Suggested-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
Rafael,

Sorry for the trouble, one more patch to apply :(

 rust/kernel/opp.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
index c2bdc11f3999..a566fc3e7dcb 100644
--- a/rust/kernel/opp.rs
+++ b/rust/kernel/opp.rs
@@ -575,6 +575,7 @@ extern "C" fn config_regulators(
 /// frequency.
 ///
 /// ```
+/// # #![cfg(CONFIG_OF)]
 /// use kernel::clk::Hertz;
 /// use kernel::cpumask::Cpumask;
 /// use kernel::device::Device;
@@ -582,7 +583,6 @@ extern "C" fn config_regulators(
 /// use kernel::opp::Table;
 /// use kernel::types::ARef;
 ///
-/// #[cfg(CONFIG_OF)]
 /// fn get_table(dev: &ARef<Device>, mask: &mut Cpumask, freq: Hertz) -> Result<Table> {
 ///     let mut opp_table = Table::from_of_cpumask(dev, mask)?;
 ///
-- 
2.31.1.272.g89b43f80a514
Re: [PATCH] rust: opp: Move `cfg(CONFIG_OF)` attribute to the top of doc test
Posted by Rafael J. Wysocki 6 months, 3 weeks ago
On Tue, May 27, 2025 at 7:33 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> Move the `#[cfg(CONFIG_OF)]` attribute to the top of the documentation test
> block and hide it. This applies the condition to the entire test and improves
> readability.
>
> Placing configuration flags like `CONFIG_OF` at the top serves as a clear
> indicator of the conditions under which the example is valid, effectively
> acting like configuration metadata for the example itself.
>
> Suggested-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> Rafael,
>
> Sorry for the trouble, one more patch to apply :(

No problem, applied.

Thanks!

>  rust/kernel/opp.rs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
> index c2bdc11f3999..a566fc3e7dcb 100644
> --- a/rust/kernel/opp.rs
> +++ b/rust/kernel/opp.rs
> @@ -575,6 +575,7 @@ extern "C" fn config_regulators(
>  /// frequency.
>  ///
>  /// ```
> +/// # #![cfg(CONFIG_OF)]
>  /// use kernel::clk::Hertz;
>  /// use kernel::cpumask::Cpumask;
>  /// use kernel::device::Device;
> @@ -582,7 +583,6 @@ extern "C" fn config_regulators(
>  /// use kernel::opp::Table;
>  /// use kernel::types::ARef;
>  ///
> -/// #[cfg(CONFIG_OF)]
>  /// fn get_table(dev: &ARef<Device>, mask: &mut Cpumask, freq: Hertz) -> Result<Table> {
>  ///     let mut opp_table = Table::from_of_cpumask(dev, mask)?;
>  ///
> --
> 2.31.1.272.g89b43f80a514
>
>
Re: [PATCH] rust: opp: Move `cfg(CONFIG_OF)` attribute to the top of doc test
Posted by Miguel Ojeda 6 months, 3 weeks ago
On Tue, May 27, 2025 at 7:33 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> Sorry for the trouble, one more patch to apply :(

Sorry, to be clear, I meant it as a discussion, i.e. no need to send a
patch for this in a hurry!

It is just something we did for another example, but it is not a
guideline or anything yet (it may not be a good idea, in fact -- I was
hoping for others to chime in). Even if it were, there is no rush to
"fix" it :)

Thanks!

Cheers,
Miguel
Re: [PATCH] rust: opp: Move `cfg(CONFIG_OF)` attribute to the top of doc test
Posted by Viresh Kumar 6 months, 3 weeks ago
On 27-05-25, 10:39, Miguel Ojeda wrote:
> Sorry, to be clear, I meant it as a discussion, i.e. no need to send a
> patch for this in a hurry!

Yeah, I know you started it as a discussion, but I did like it. And
honestly I didn't realize earlier that I can add this at the top of
the whole block (and hide it too), otherwise that would have been part
of my initial patch itself.

> It is just something we did for another example, but it is not a
> guideline or anything yet (it may not be a good idea, in fact -- I was
> hoping for others to chime in). Even if it were, there is no rush to
> "fix" it :)

That's why I didn't mark it as a fix, but just an improvement that I
agree with :)

Specially for this particular case, there is only function in this doc
test, and so marking the whole block with cfg looks better.

-- 
viresh