Re: [PATCH v2 3/3] clk: rockchip: Add RK3572 clock and reset support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] clk: rockchip: s16 k overflow in rk3572_pll_rates[] struct initializers
--

commit c2a7bfa93851b6e038e2eb4b577e1b1e53dcecfb
Author: Elaine Zhang <zhangqing@rock-chips.com>

clk: rockchip: Add RK3572 clock and reset support

This patch adds clock and reset controller support for the Rockchip RK3572
SoC, including PLLs, CPU clock trees, branch clocks, and soft reset lookup
tables.

> diff --git a/drivers/clk/rockchip/clk-rk3572.c b/drivers/clk/rockchip/clk-rk3572.c
> new file mode 100644
> index 0000000000000..712651c6b8cf0
> --- /dev/null
> +++ b/drivers/clk/rockchip/clk-rk3572.c
> @@ -0,0 +1,2004 @@

[ ... ]

> +static struct rockchip_pll_rate_table rk3572_pll_rates[] = {
> +	/* _mhz, _p, _m, _s, _k */

[ ... ]

> +	RK3588_PLL_RATE(1188000000, 2, 198, 1, 0),
> +	RK3588_PLL_RATE(1186814000, 2, 198, 1, 52581),
> +	RK3588_PLL_RATE(1186812000, 2, 198, 1, 52559),
> +	RK3588_PLL_RATE(1109000000, 3, 554, 2, 32767),
> +	RK3588_PLL_RATE(1100000000, 3, 550, 2, 0),
> +	RK3588_PLL_RATE(1051000000, 3, 525, 2, 32767),
> +	RK3588_PLL_RATE(1008000000, 2, 336, 2, 0),
> +	RK3588_PLL_RATE(1000000000, 3, 500, 2, 0),
> +	RK3588_PLL_RATE(983040000, 4, 655, 2, 23592),
> +	RK3588_PLL_RATE(955520000, 3, 478, 2, -15730),
> +	RK3588_PLL_RATE(903168000, 6, 903, 2, 11009),
> +	RK3588_PLL_RATE(900000000, 2, 300, 2, 0),
> +	RK3588_PLL_RATE(816000000, 2, 272, 2, 0),
> +	RK3588_PLL_RATE(786432000, 2, 262, 2, 9437),
> +	RK3588_PLL_RATE(786000000, 1, 131, 2, 0),
> +	RK3588_PLL_RATE(785560000, 3, 393, 2, -14419),
> +	RK3588_PLL_RATE(773000000, 2, 258, 2, 43690),
> +	RK3588_PLL_RATE(722534400, 8, 963, 2, 24850),
> +	RK3588_PLL_RATE(697000000, 2, 232, 2, 21845),
> +	RK3588_PLL_RATE(610400000, 3, 305, 2, 13107),
> +	RK3588_PLL_RATE(604800000, 1, 101, 2, 52428),
> +	RK3588_PLL_RATE(600000000, 2, 200, 2, 0),
> +	RK3588_PLL_RATE(594000000, 2, 198, 2, 0),
> +	RK3588_PLL_RATE(408000000, 2, 272, 3, 0),
> +	RK3588_PLL_RATE(312000000, 2, 208, 3, 0),
> +	RK3588_PLL_RATE(266580000, 1, 178, 4, 47185),

[Severity: Low]
Does initializing the k field in rk3572_pll_rates[] with positive constants
above 32767 overflow s16 and trigger compiler warnings?

Commit 581478883477 ("clk: rockchip: Fractional PLL coefficient on
RK3588/RK3576 is two's complement") changed the fractional coefficient k in
struct rockchip_pll_rate_table to s16:

drivers/clk/rockchip/clk.h:
struct rockchip_pll_rate_table {
	...
		struct {
			/* for RK3588 */
			unsigned int m;	/* main divider, 10 bit unsigned */
			unsigned int p;	/* pre-divider, 6 bit unsigned */
			unsigned int s;	/* scaler, 3 bit unsigned */
			s16 k;		/* fractional part, 16 bit two's complement */
		};
	...
};

While the 955520000 and 785560000 entries use signed negative values (-15730
and -14419), five other entries with adjusted m values still pass unsigned
16-bit two's complement constants exceeding 32767:

- 1186814000: 52581 (instead of -12955)
- 1186812000: 52559 (instead of -12977)
- 773000000: 43690 (instead of -21846)
- 604800000: 52428 (instead of -13108)
- 266580000: 47185 (instead of -18351)

When CONFIG_CLK_RK3572 is enabled, this triggers -Woverflow in GCC and
-Wconstant-conversion in Clang, causing the build to fail under
CONFIG_WERROR=y. Should these five entries use their negative signed
equivalents?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023-add-board-ok3572-c-clk-v2-0-90b98836be6e@qq.com?part=3