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

[Ronald Claveau <linux-kernel-dev@aliel.fr>]

On 10/23/26 12:00 AM, sashiko-bot@kernel.org wrote:
> 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?
> 

You right, I misread the parent and will drop this patch in v2.

-- 
Best regards,
Ronald

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