Re: [PATCH v3 6/7] clk: add a driver for the Atheros/Qualcomm ath79 PLL controller

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] clk: ath79: zeroed hfrac shift in qca956x_pll_rate()
--

commit 423abf763de32c1b127032d8210181c833b02aa9
Author: Rosen Penev <rosenp@gmail.com>

clk: add a driver for the Atheros/Qualcomm ath79 PLL controller

This commit adds a common clock driver for the Atheros/Qualcomm ath79 PLL
controller to replace the arch/mips/ath79/clock.c setup code.

> diff --git a/drivers/clk/clk-ath79.c b/drivers/clk/clk-ath79.c
> new file mode 100644
> index 0000000000000..6eab4cd1a91f7
> --- /dev/null
> +++ b/drivers/clk/clk-ath79.c
> @@ -0,0 +1,782 @@

[ ... ]

> +#define QCA956X_PLL_CONFIG1_NFRAC_L			GENMASK(4, 0)
> +#define QCA956X_PLL_CONFIG1_NFRAC_H			GENMASK(17, 5)
> +#define QCA956X_PLL_CONFIG1_NINT			GENMASK(26, 18)

[ ... ]

> +static unsigned long qca956x_pll_rate(unsigned long ref, u32 config,
> +				      u32 config1, u32 refdiv_mask,
> +				      u32 outdiv_mask)
> +{
> +	u32 ref_div, out_div, nint, hfrac, lfrac;
> +	u64 rate;
> +
> +	ref_div = (config & refdiv_mask) >> __ffs(refdiv_mask);
> +	out_div = (config & outdiv_mask) >> __ffs(outdiv_mask);
> +	nint = FIELD_GET(QCA956X_PLL_CONFIG1_NINT, config1);
> +	hfrac = FIELD_GET(QCA956X_PLL_CONFIG1_NFRAC_H, config1);
> +	lfrac = FIELD_GET(QCA956X_PLL_CONFIG1_NFRAC_L, config1);
> +
> +	if (!ref_div)
> +		return 0;
> +
> +	rate = div_u64((u64)ref * nint, ref_div);
> +	rate += div_u64((u64)ref * lfrac, (ref_div * 25) << 13);
> +	rate += div_u64((u64)ref * (hfrac >> 13), ref_div);

[Severity: Medium]
Does (hfrac >> 13) always evaluate to 0 here?  QCA956X_PLL_CONFIG1_NFRAC_H
is a 13-bit mask (GENMASK(17, 5)), so hfrac is at most 0x1fff and shifting
it right by 13 drops the upper fractional multiplier completely when
computing the QCA956x CPU, DDR, and AHB clock rates.  Matching the lfrac
term above, it looks like the divisor should be shifted instead:
div_u64((u64)ref * hfrac, ref_div << 13).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111215729.10683-1-rosenp@gmail.com?part=6