Re: [PATCH 6/6] clk: rockchip: pll: fix overflow and drop manual two's complement in rk3588_pll_get_rate

[Quentin Schulz <quentin.schulz@cherry.de>]

Hi Alexey,

On 7/23/26 2:25 PM, Alexey Charkov wrote:
> On Thu, Jul 23, 2026 at 1:23 PM Quentin Schulz <quentin.schulz@cherry.de> wrote:
>>
>> Hi Alexey,
>>
>> On 7/13/26 8:35 PM, Alexey Charkov wrote:
>>> Current code calculates the fractional component in 32 bits before
>>> assigning it to a 64-bit holding variable, causing overflow for real-world
>>> values of k, given that OSC_HZ is 24000000U. It also does bitwise manual
>>> massaging of an unsigned representation of what is actually a two's
>>> complement signed value, which is confusing and makes the code harder to
>>> read.
>>>
>>> Read k into a properly signed type and promote operands to avoid overflow,
>>> which also enables the use of div_s64() to express the math more clearly.
>>>
>>
>> Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>
>>
>> I've quickly looked at the rest of the threads in this series and I
>> think there's no open questions left for me? You sent patches to the
>> kernel for making k an s16 so I'm assuming we'll go for that in U-Boot
>> as well. The PLLCON(2) thing for integer PLLs will need to be fixed one
>> way or another, but I'm assuming this is also something we need to fix
>> in the kernel so a similar approach would be nice. I don't think we need
>> to make this a big series fixing everything in one go, so feel free to
>> send smallish series whenever you're ready. Anything I'm missing?
> 
> Thanks Quentin!
> 
> I'll rework the helper to take a pointer to the table entry instead,
> as we've discussed in the other sub-thread, and update the whole thing
> to match the types etc. of what I submitted to Linux. Hopefully the
> unsigned->signed conversion will then get localized in a single
> commit.
> 

We're on the same page then :)

> Looks like https://lore.kernel.org/u-boot/ doesn't pick up new emails
> from the list since July 20, so `b4 trailers -u` doesn't work either.
> I've picked up your review tags manually, hope I haven't missed
> anything in process.
> 

Yes that's unfortunate. We know, the LF (handling lore.kernel.org) 
knows. The main IT was on holidays and just came back a few days ago, 
and he's overloaded at the moment. I'm guessing there are more urgent 
fires for him to put out at the moment :)

Cheers,
Quentin