Hi Alexey,
On 7/21/26 3:34 PM, Alexey Charkov wrote:
Hi Quentin,
On Tue, Jul 21, 2026 at 4:56 PM Quentin Schulz <[email protected]> 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,
I'm not sure this is correct? Converting an unsigned integer (readl
returns an u32 and con is a u32) to signed (k is s16) is
implementation-defined as far as I understood (c.f.
https://en.cppreference.com/c/language/conversion). I'm sure I
misunderstood the spec but considering signed integer overflow is
undefined (and I guess one could understand casting a u32 storing a
number bigger than S16_MAX into an s16 to be some kind of overflow), I'm
a bit concerned here. I probably forgot important stuff I learned a
decade ago :) Can you point me where/what I misunderstood?
It is indeed implementation defined in the C standard, but given that
U-Boot enforces the gnu11 convention, it's defined to be reduction
modulo 2^16 [1] along with two's complement representation, which is
exactly what we need here.
That is helpful thank you! I was sure I was missing something as I don't
think we explicitly handle this anywhere in U-Boot or the Linux kernel
:) A few more related questions though if you don't mind :)
I'm assuming we're simply masking the bits 16+ to be 0 (modulo 2**16) as
stored in the u32 (which already carries an s16 value just "as" u32) and
not care about signedness when doing that (otherwise we would have an
issue since the MSB of the u32 is necessarily a 0, thus a positive
value, since the register returns 0 for [31:16] according to the TRM).
Is that what you meant by "along with two's complement representation"?
We also build with clang, but I'm assuming it respects the GNU
implementation with the -std=gnu11 argument we have in KBUILD_CFLAGS.
May I ask how you know this implementation is part of gnu11 from that
webpage?
Cheers,
Quentin
