Re: [PATCH net-next v7 3/3] dpll: add SiTime SiT9531x DPLL clock driver

From: Ali Rouhi

Date: Thu Aug 20 2026 - 21:06:46 EST


On 20/08/2026 19:57, Ivan Vecera wrote:
>> + u64 t_out_ps = div64_u64(1000000000000ULL, freq);
>> + u64 advance = (u64)(-(s64)phase_ps);
>> +
>> + if (t_out_ps == 0)
>> + return -EINVAL;
>> + advance %= t_out_ps;
>
> This causes build failure on 32bit systems... You could use something
> like this:
>
> div64_u64_rem(advance, t_out_ps, &advance);

Fixed exactly as you suggest - div64_u64_rem() in that one place, which
was the only 64-bit modulo in the driver. Jakub reported the same
failure from a 32-bit x86 build, so the commit carries his Reported-by.

A 32-bit build is now a standing gate on every patch of the series, not
only on the final tree.

>> +/*
>> + * I2C register model:
>> + * - Page select register at offset 0x01
>> + * - Each page has 128 registers (0x00-0x7F)
>> + * - Some pages are paired (e.g. 0x0A/0x1A for PLLA)
>> + */
>> +#define SIT9531X_PAGE_SEL 0xFF
>> +#define SIT9531X_PAGE_SIZE 0x100
>> +#define SIT9531X_NUM_PAGES 32
>
> The comment at the top of regs.h says the page select register is at
> offset 0x01 and each page has 128 registers (0x00-0x7F) but the code
> defines it differently (selector at 0xff and page size 256).
>
> One of them is wrong.

The comment was wrong; the defines are correct. The page select lives
at 0xFF and is visible from every page, and a page is a full 256
registers. The per-PLL manual input selection registers sit at
0xE8-0xEB, past the 0x7F the comment claims as the end of a page.

Corrected in our tree, with your Reported-by and a Closes: link to this
mail.

Thanks for the review,
Ali