Re: [PATCH RFC 2/6] pwm: Add Rust driver for T-HEAD TH1520 SoC

Miguel Ojeda
2025-06-06T15:21:37Z

On Fri, Jun 6, 2025 at 4:08 PM Michal Wilczynski
<m.wilczynski@samsung.com> wrote:
>
> You're right, thank you. The wrapping_mul is unsafe due to the overflow
> risk you pointed out.

What do you mean by "unsafe"? `wrapping_mul` does not trigger UB and
it intentionally provides wrapping arithmetic.

> The ideal solution would be to use the kernel's own mul_u64_u64_div_u64
> helper function.
>
> Rust maintainers: This binding doesn't seem to be available yet. Would a
> preparatory patch introducing a minimal rust/kernel/math.rs with only
> this binding be the best way to proceed? It seems like a useful utility
> for more than just this driver.

Sounds good to me. We also recently had related discussions about
64-bit divisions in 32-bit architectures and `const fn`s, you may want
to take a look:

https://lore.kernel.org/rust-for-linux/CANiq72ke45eOwckMhWHvmwxc03dxr4rnxxKvx+HvWdBLopZfrQ@mail.gmail.com/
https://lore.kernel.org/rust-for-linux/20250502004524.230553-1-fujita.tomonori@gmail.com/
https://lore.kernel.org/rust-for-linux/20250501015818.226376-1-fujita.tomonori@gmail.com/

I would also be careful choosing the names: if they are supposed the
same behavior, then please pick the same name as C. Otherwise, we
should avoid it. Either way, we should document them.

For instance, is this supposed to be `mul_u64_u64_div_u64`? I guess
not, given e.g. the `c == 0` case.

I hope that helps & thanks!

Cheers,
Miguel