Re: [PATCH 1/4] clk: meson: g12a: fix fclk_div2p5 multiplier

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] clk: meson: g12a: incorrect rate for g12a_fclk_div2p5_div due to 4 GHz parent
--

commit 63eec9c39f2ccd77037a929408928c357e150075
Author: Ronald Claveau <linux-kernel-dev@aliel.fr>

clk: meson: g12a: fix fclk_div2p5 multiplier

This commit changes the multiplier of g12a_fclk_div2p5_div in
drivers/clk/meson/g12a.c from 1 to 2 so that its fixed factor ratio becomes
2/5 instead of 1/5.

> diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
> index d0d4c7b6dc827..4ec5f15daf980 100644
> --- a/drivers/clk/meson/g12a.c
> +++ b/drivers/clk/meson/g12a.c
> @@ -995,7 +995,7 @@ static struct clk_regmap g12a_fclk_div7 = {
>  };
>  
>  static struct clk_fixed_factor g12a_fclk_div2p5_div = {
> -	.mult = 1,
> +	.mult = 2,
>  	.div = 5,
>  	.hw.init = &(struct clk_init_data){
>  		.name = "fclk_div2p5_div",

[Severity: High]
Won't setting .mult = 2 here cause g12a_fclk_div2p5_div and g12a_fclk_div2p5
to report 1.6 GHz instead of the expected 800 MHz when clk_fixed_factor_ops
calculates the clock rate?

Unlike the other g12a_fclk_div*_div clocks in drivers/clk/meson/g12a.c, which
use g12a_fixed_pll (~2 GHz) as their parent:

drivers/clk/meson/g12a.c:
static struct clk_fixed_factor g12a_fclk_div5_div = {
	.mult = 1,
	.div = 5,
	.hw.init = &(struct clk_init_data){
		.name = "fclk_div5_div",
		.ops = &clk_fixed_factor_ops,
		.parent_hws = (const struct clk_hw *[]) { &g12a_fixed_pll.hw },
		.num_parents = 1,
	},
};

g12a_fclk_div2p5_div is parented directly to g12a_fixed_pll_dco (~4 GHz),
before the g12a_fixed_pll post-divider divides the DCO by 2:

drivers/clk/meson/g12a.c:
static struct clk_fixed_factor g12a_fclk_div2p5_div = {
	.mult = 2,
	.div = 5,
	.hw.init = &(struct clk_init_data){
		.name = "fclk_div2p5_div",
		.ops = &clk_fixed_factor_ops,
		.parent_hws = (const struct clk_hw *[]) {
			&g12a_fixed_pll_dco.hw
		},
		.num_parents = 1,
	},
};

Because g12a_fixed_pll_dco runs at ~4 GHz (3,999,999,938 Hz, as also seen in
g12a_mpll_50m_div which divides g12a_fixed_pll_dco by 80 to get 50 MHz),
dividing g12a_fixed_pll_dco by 5 with .mult = 1 already produces the 800 MHz
rate (799,999,987 Hz, or g12a_fixed_pll / 2.5) expected by consumers such as
gpu_opp_table in arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi:

arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi:
		opp-799999987 {
			opp-hz = /bits/ 64 <799999987>;
			opp-microvolt = <800000>;
		};

With .mult = 2 and .div = 5 on g12a_fixed_pll_dco, wouldn't fclk_div2p5
report ~1.6 GHz (1,599,999,975 Hz) and cause downstream dividers such as the
Mali GPU, NNA, VDEC, VAPB, and MIPI ISP clocks to program a /2 divider and
run at half their requested frequency?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022-fix-g12a-clk-v1-0-e2bd8f3f1afa@aliel.fr?part=1

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic