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

["Benno Lossin" <lossin@kernel.org>]

On Fri Jun 6, 2025 at 4:08 PM CEST, Michal Wilczynski wrote:
> On 6/5/25 12:39, Uwe Kleine-König wrote:
>> On Sat, May 24, 2025 at 11:14:56PM +0200, Michal Wilczynski wrote:
>>> +impl Drop for Th1520PwmChipData {
>>> +    fn drop(&mut self) {
>>> +        self.clk.disable_unprepare();
>>> +    }
>>> +}
>>> +
>>> +fn mul_div_u64(a: u64, b: u64, c: u64) -> u64 {
>>> +    if c == 0 {
>>> +        return 0;
>>> +    }
>>> +    a.wrapping_mul(b) / c
>>> +}
>> 
>> Is this save if a * b > U64_MAX? I would have expected such a function
>> to already exist in generic code.
>
> You're right, thank you. The wrapping_mul is unsafe due to the overflow
> risk you pointed out.

`wrapping_mul` is not `unsafe`, but the above code will result in an
incorrect result if `a * b` overflows (it won't be UB, since
`wrapping_mul` just wraps around to 0).

> 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, but I would separate it into its own patch series, since
then you can iterate on both at the same time.

Are there also similar functions for signed and other sizes? If yes,
then we should add them via an extension trait on the primitive types.

---
Cheers,
Benno