A few changes to improve the clock abstractions and make them a little
more idiomatic:

1. `impl Hertz` functions are now constant and compile-time evaluable.
2. `Hertz` conversions are now done with constant variables, which should
    make them more readable.
3. `con_id` is handled in a single line using `map_or` instead of using
    nested if-else blocks.

Signed-off-by: onur-ozkan <work@onurozkan.dev>
---
 rust/kernel/clk.rs | 42 +++++++++++++++++++-----------------------
 1 file changed, 19 insertions(+), 23 deletions(-)

diff --git a/rust/kernel/clk.rs b/rust/kernel/clk.rs
index 6041c6d07527..fbcea31dbcca 100644
--- a/rust/kernel/clk.rs
+++ b/rust/kernel/clk.rs
@@ -30,39 +30,43 @@
 pub struct Hertz(pub c_ulong);
 
 impl Hertz {
+    const KHZ_TO_HZ: c_ulong = 1_000;
+    const MHZ_TO_HZ: c_ulong = 1_000_000;
+    const GHZ_TO_HZ: c_ulong = 1_000_000_000;
+
     /// Create a new instance from kilohertz (kHz)
-    pub fn from_khz(khz: c_ulong) -> Self {
-        Self(khz * 1_000)
+    pub const fn from_khz(khz: c_ulong) -> Self {
+        Self(khz * Self::KHZ_TO_HZ)
     }
 
     /// Create a new instance from megahertz (MHz)
-    pub fn from_mhz(mhz: c_ulong) -> Self {
-        Self(mhz * 1_000_000)
+    pub const fn from_mhz(mhz: c_ulong) -> Self {
+        Self(mhz * Self::MHZ_TO_HZ)
     }
 
     /// Create a new instance from gigahertz (GHz)
-    pub fn from_ghz(ghz: c_ulong) -> Self {
-        Self(ghz * 1_000_000_000)
+    pub const fn from_ghz(ghz: c_ulong) -> Self {
+        Self(ghz * Self::GHZ_TO_HZ)
     }
 
     /// Get the frequency in hertz
-    pub fn as_hz(&self) -> c_ulong {
+    pub const fn as_hz(&self) -> c_ulong {
         self.0
     }
 
     /// Get the frequency in kilohertz
-    pub fn as_khz(&self) -> c_ulong {
-        self.0 / 1_000
+    pub const fn as_khz(&self) -> c_ulong {
+        self.0 / Self::KHZ_TO_HZ
     }
 
     /// Get the frequency in megahertz
-    pub fn as_mhz(&self) -> c_ulong {
-        self.0 / 1_000_000
+    pub const fn as_mhz(&self) -> c_ulong {
+        self.0 / Self::MHZ_TO_HZ
     }
 
     /// Get the frequency in gigahertz
-    pub fn as_ghz(&self) -> c_ulong {
-        self.0 / 1_000_000_000
+    pub const fn as_ghz(&self) -> c_ulong {
+        self.0 / Self::GHZ_TO_HZ
     }
 }
 
@@ -132,11 +136,7 @@ impl Clk {
         ///
         /// [`clk_get`]: https://docs.kernel.org/core-api/kernel-api.html#c.clk_get
         pub fn get(dev: &Device, name: Option<&CStr>) -> Result<Self> {
-            let con_id = if let Some(name) = name {
-                name.as_ptr()
-            } else {
-                ptr::null()
-            };
+            let con_id = name.map_or(ptr::null(), |n| n.as_ptr());
 
             // SAFETY: It is safe to call [`clk_get`] for a valid device pointer.
             //
@@ -304,11 +304,7 @@ impl OptionalClk {
         /// [`clk_get_optional`]:
         /// https://docs.kernel.org/core-api/kernel-api.html#c.clk_get_optional
         pub fn get(dev: &Device, name: Option<&CStr>) -> Result<Self> {
-            let con_id = if let Some(name) = name {
-                name.as_ptr()
-            } else {
-                ptr::null()
-            };
+            let con_id = name.map_or(ptr::null(), |n| n.as_ptr());
 
             // SAFETY: It is safe to call [`clk_get_optional`] for a valid device pointer.
             //
-- 
2.49.0

On Tue Jun 17, 2025 at 5:01 AM JST, onur-ozkan wrote:
> A few changes to improve the clock abstractions and make them a little
> more idiomatic:
>
> 1. `impl Hertz` functions are now constant and compile-time evaluable.
> 2. `Hertz` conversions are now done with constant variables, which should

"constant variable" is an oxymoron. :) I think you just want to say
"constant" here.

>     make them more readable.
> 3. `con_id` is handled in a single line using `map_or` instead of using
>     nested if-else blocks.

Please split these 3 changes into 3 patches, I agree that they are
trivial but a patch should do a single thing. This makes review simpler
and allows to apply only part of the changes if e.g. one of them needs
further discussion or is rejected.

The changes in themselves look good though!

On Mon, Jun 16, 2025 at 10:01 PM onur-ozkan <work@onurozkan.dev> wrote:
>
> Signed-off-by: onur-ozkan <work@onurozkan.dev>

Should this be "Onur Özkan"? Apologies if not -- I am just asking
because the SoB typically uses the full name as usually written, and I
saw it in Zulip spelled differently.

Cc'ing Viresh, since he introduced the file.

On the changes themselves, the `const fn` one seems obvious.

The constants one is OK, though we could perhaps have a `units.h`
equivalent eventually.

The `map_or` is up to style, though I am not sure why you say "nested
if-else blocks" in the commit message, i.e. what is nested?

(By the way, this could (or not) have been split into different
patches -- it is up to the maintainers/reviewers, and it depends on
how complex a given patch is and other factors, but I thought I would
mention it just in case you thought it needed to be one.)

Thanks!

Cheers,
Miguel