[PATCH] rust: clk: document overflow panics in `Hertz` constructors

[PATCH] rust: clk: document overflow panics in `Hertz` constructors

[Georgios Androutsopoulos]

`Hertz::from_khz()`, `from_mhz()` and `from_ghz()` multiply their
argument by 1_000, 1_000_000 and 1_000_000_000 respectively without
checking for overflow. When `CONFIG_RUST_OVERFLOW_CHECKS` is enabled,
each panics once its argument exceeds `c_ulong::MAX` divided by that
factor. None of the three documents this. The panic occurs only at
runtime, when the argument is not a constant expression.

Add the missing `# Panics` sections stating the bound for each unit.

Fixes: d01d70205601 ("rust: clk: Add initial abstractions")
Signed-off-by: Georgios Androutsopoulos <georgeandrout13@gmail.com>
---
 rust/kernel/clk.rs | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/rust/kernel/clk.rs b/rust/kernel/clk.rs
index 7abbd0767d8c..f04f5c4a03d6 100644
--- a/rust/kernel/clk.rs
+++ b/rust/kernel/clk.rs
@@ -35,16 +35,31 @@ impl Hertz {
     const GHZ_TO_HZ: c_ulong = 1_000_000_000;
 
     /// Create a new instance from kilohertz (kHz)
+    ///
+    /// # Panics
+    ///
+    /// Panics if `CONFIG_RUST_OVERFLOW_CHECKS` is enabled and `khz` is greater
+    /// than `c_ulong::MAX / 1_000`.
     pub const fn from_khz(khz: c_ulong) -> Self {
         Self(khz * Self::KHZ_TO_HZ)
     }
 
     /// Create a new instance from megahertz (MHz)
+    ///
+    /// # Panics
+    ///
+    /// Panics if `CONFIG_RUST_OVERFLOW_CHECKS` is enabled and `mhz` is greater
+    /// than `c_ulong::MAX / 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)
+    ///
+    /// # Panics
+    ///
+    /// Panics if `CONFIG_RUST_OVERFLOW_CHECKS` is enabled and `ghz` is greater
+    /// than `c_ulong::MAX / 1_000_000_000`.
     pub const fn from_ghz(ghz: c_ulong) -> Self {
         Self(ghz * Self::GHZ_TO_HZ)
     }

base-commit: 73e3f0710014fe6d4ed98cfc02292f6121db7558
-- 
2.47.3

Re: [PATCH] rust: clk: document overflow panics in `Hertz` constructors

[Alexandre Courbot]

On Tue Sep 8, 2026 at 11:56 PM BST, Georgios Androutsopoulos wrote:
> `Hertz::from_khz()`, `from_mhz()` and `from_ghz()` multiply their
> argument by 1_000, 1_000_000 and 1_000_000_000 respectively without
> checking for overflow. When `CONFIG_RUST_OVERFLOW_CHECKS` is enabled,
> each panics once its argument exceeds `c_ulong::MAX` divided by that
> factor. None of the three documents this. The panic occurs only at
> runtime, when the argument is not a constant expression.
>
> Add the missing `# Panics` sections stating the bound for each unit.
>
> Fixes: d01d70205601 ("rust: clk: Add initial abstractions")
> Signed-off-by: Georgios Androutsopoulos <georgeandrout13@gmail.com>

As we just discussed at Kangrejos we might want to harden these a bit,
but meanwhile documenting the behavior is indeed a good idea.

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

Re: [PATCH] rust: clk: document overflow panics in `Hertz` constructors

[Brian Masney]

On Thu, Sep 17, 2026 at 07:10:42PM +0100, Alexandre Courbot wrote:
> On Tue Sep 8, 2026 at 11:56 PM BST, Georgios Androutsopoulos wrote:
> > `Hertz::from_khz()`, `from_mhz()` and `from_ghz()` multiply their
> > argument by 1_000, 1_000_000 and 1_000_000_000 respectively without
> > checking for overflow. When `CONFIG_RUST_OVERFLOW_CHECKS` is enabled,
> > each panics once its argument exceeds `c_ulong::MAX` divided by that
> > factor. None of the three documents this. The panic occurs only at
> > runtime, when the argument is not a constant expression.
> >
> > Add the missing `# Panics` sections stating the bound for each unit.
> >
> > Fixes: d01d70205601 ("rust: clk: Add initial abstractions")
> > Signed-off-by: Georgios Androutsopoulos <georgeandrout13@gmail.com>
> 
> As we just discussed at Kangrejos we might want to harden these a bit,
> but meanwhile documenting the behavior is indeed a good idea.
> 
> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

Acked-by: Brian Masney <bmasney@redhat.com>

I assume this will go through the rust tree? If not, I'm happy to take
it through the clk tree, assuming the rust devs are happy with this.

Brian

Re: [PATCH] rust: clk: document overflow panics in `Hertz` constructors

[Alice Ryhl]

On Mon, Sep 21, 2026 at 5:37 PM Brian Masney <bmasney@redhat.com> wrote:
>
> On Thu, Sep 17, 2026 at 07:10:42PM +0100, Alexandre Courbot wrote:
> > On Tue Sep 8, 2026 at 11:56 PM BST, Georgios Androutsopoulos wrote:
> > > `Hertz::from_khz()`, `from_mhz()` and `from_ghz()` multiply their
> > > argument by 1_000, 1_000_000 and 1_000_000_000 respectively without
> > > checking for overflow. When `CONFIG_RUST_OVERFLOW_CHECKS` is enabled,
> > > each panics once its argument exceeds `c_ulong::MAX` divided by that
> > > factor. None of the three documents this. The panic occurs only at
> > > runtime, when the argument is not a constant expression.
> > >
> > > Add the missing `# Panics` sections stating the bound for each unit.
> > >
> > > Fixes: d01d70205601 ("rust: clk: Add initial abstractions")
> > > Signed-off-by: Georgios Androutsopoulos <georgeandrout13@gmail.com>
> >
> > As we just discussed at Kangrejos we might want to harden these a bit,
> > but meanwhile documenting the behavior is indeed a good idea.
> >
> > Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
>
> Acked-by: Brian Masney <bmasney@redhat.com>
>
> I assume this will go through the rust tree? If not, I'm happy to take
> it through the clk tree, assuming the rust devs are happy with this.

My understanding is that the "default" way patches land is that the
subsystem also takes Rust patches related to the subsystem, and that
the rust tree is a fallback tree.

Alice

Re: [PATCH] rust: clk: document overflow panics in `Hertz` constructors

[Miguel Ojeda]

On Mon, Sep 21, 2026 at 5:51 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> My understanding is that the "default" way patches land is that the
> subsystem also takes Rust patches related to the subsystem, and that
> the rust tree is a fallback tree.

Indeed, the idea on how we suggest setting up the `MAINTAINERS`
entries is that, by default, maintainers keep control of both sides
(they are the experts, after all) and that, hopefully, they get more
involved on the Rust side etc.

It makes sense for reworking the subsystem, too, since it may happen
that a rework on the C side may need adjustments on the Rust side etc.

The file is already under "COMMON CLK FRAMEWORK", so in this instance
it seems fine.

Brian: in case you want it (but it wouldn't be needed!):

Acked-by: Miguel Ojeda <ojeda@kernel.org>

As Alexandre said, we may want to rework how these work (instead of
panicking), but documenting helps for now (and whether this counts as
a Fix or not is, I guess, up to what you usually do in your
subsystem).

I hope that helps!

Cheers,
Miguel