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

[Michal Wilczynski <m.wilczynski@samsung.com>]

On 6/6/25 17:21, Miguel Ojeda wrote:
> 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.

You're right, I should have said "mathematically incorrect" due to the
overflow, not unsafe

> 
>> 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.

Thanks ! Seems like providing a safe Rust wrapper for
mul_u64_u64_div_u64 is the correct path forward. Since it's a standard
helper in the PWM subsystem, using it would allow me to remove the
temporary math implementation from this driver in favor of the kernel's
optimized version.

> 
> I hope that helps & thanks!
> 
> Cheers,
> Miguel
> 

Best regards,
-- 
Michal Wilczynski <m.wilczynski@samsung.com>