Re: [PATCH V12 13/15] rust: cpufreq: Extend abstractions for driver registration

[Danilo Krummrich]

On Mon, May 19, 2025 at 12:37:18PM +0530, Viresh Kumar wrote:
> +/// CPU frequency driver Registration.
> +///
> +/// ## Examples
> +///
> +/// The following example demonstrates how to register a cpufreq driver.
> +///
> +/// ```
> +/// use kernel::{
> +///     cpu, cpufreq,
> +///     c_str,
> +///     device::{Bound, Device},
> +///     macros::vtable,
> +///     sync::Arc,
> +/// };
> +/// struct FooDevice;
> +///
> +/// #[derive(Default)]
> +/// struct FooDriver;
> +///
> +/// #[vtable]
> +/// impl cpufreq::Driver for FooDriver {
> +///     const NAME: &'static CStr = c_str!("cpufreq-foo");
> +///     const FLAGS: u16 = cpufreq::flags::NEED_INITIAL_FREQ_CHECK | cpufreq::flags::IS_COOLING_DEV;
> +///     const BOOST_ENABLED: bool = true;
> +///
> +///     type PData = Arc<FooDevice>;
> +///
> +///     fn init(policy: &mut cpufreq::Policy) -> Result<Self::PData> {
> +///         // Initialize here
> +///         Ok(Arc::new(FooDevice, GFP_KERNEL)?)
> +///     }
> +///
> +///     fn exit(_policy: &mut cpufreq::Policy, _data: Option<Self::PData>) -> Result<()> {

This can just be `Result`, here and below.

> +///         Ok(())
> +///     }
> +///
> +///     fn suspend(policy: &mut cpufreq::Policy) -> Result<()> {
> +///         policy.generic_suspend()
> +///     }
> +///
> +///     fn verify(data: &mut cpufreq::PolicyData) -> Result<()> {
> +///         data.generic_verify()
> +///     }
> +///
> +///     fn target_index(policy: &mut cpufreq::Policy, index: cpufreq::TableIndex) -> Result<()> {
> +///         // Update CPU frequency
> +///         Ok(())
> +///     }
> +///
> +///     fn get(policy: &mut cpufreq::Policy) -> Result<u32> {
> +///         policy.generic_get()
> +///     }
> +/// }
> +///
> +/// fn foo_probe(dev: &Device<Bound>) {

You could use a real probe function, e.g. from platform:

	# struct Driver;
	
	impl platform::Driver for SampleDriver {
	   # type IdInfo = ();
	   # const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None;
	
	   fn probe(
	      pdev: &platform::Device<Core>,
	      info: Option<&Self::IdInfo>,
	   ) -> Result<Pin<KBox<Self>>> {
	      ...
	   }
	}

> +///     cpufreq::Registration::<FooDriver>::new_foreign_owned(dev).unwrap();

I prefer if we do not use unwrap() in doctests, since they also serve as example
and people might think that calling unwrap() is valid thing to do.

Sorry, I didn't catch the above in my previous review -- fine for me if you do
those improvements in a subsequent patch.