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

From: Miguel Ojeda
Date: Mon Mar 03 2025 - 10:50:54 EST


On Mon, Mar 3, 2025 at 12:44 PM Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
>
> Actually Daneil did suggest to make this "Struct Hertz(c_ulong)", but then I
> looked at rust/kernel/time.rs:
>
> pub type Jiffies = crate::ffi::c_ulong;
>
> And I thought this is probably what everyone would have agreed to and did it
> this way.

I see, thanks!

Personally, I would prefer that we consider using "strong typedefs"
wherever possible, as early as possible -- otherwise it will be more
painful later on.

> Something like this (from print.rs) ?
>
> /// [`pr_debug`]: https://docs.kernel.org/core-api/printk-basics.html#c.pr_debug

Yeah. The docs in the C side may or may not make perfect sense on the
Rust side though, i.e. it it may make sense to adapt them, add
examples, etc., but it may be that they are exactly what you want, in
which case only linking may be OK (i.e. normally linking is something
to do on top of the new docs).

> Yes, I was using this under `cfg` earlier, but removed that recently after
> testing this without CONFIG_HAVE_CLK. clk.h provides wrappers for cases where
> the config option isn't available.

Great, thanks a lot for testing that!

Cheers,
Miguel