[PATCH 0/4] clk: meson: g12a: fix a few clock definition errors

[PATCH 0/4] clk: meson: g12a: fix a few clock definition errors

[Ronald Claveau via B4 Relay]

While reviewing the G12A clock controller, I noticed two unrelated
mistakes in the clock definitions:

 - fclk_div2p5 uses mult = 1 instead of 2, so it runs at fclk / 5
   instead of fclk / 2.5.
 - CLKID_VID_PLL and CLKID_VID_PLL_DIV are swapped in the hw_onecell_data
   tables.

The first one is a single fix. The second one is split per SoC (g12a,
g12b, sm1) since each table was introduced by a different commit so
I provide different Fixes: tag.

I do not see any consumer today, so nobody should be affected.
I don't think there is any values in backporting them btw.

Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
---
Ronald Claveau (4):
      clk: meson: g12a: fix fclk_div2p5 multiplier
      clk: meson: g12a: fix swapped VID_PLL and VID_PLL_DIV clock ids
      clk: meson: g12a: fix swapped VID_PLL and VID_PLL_DIV clock ids for g12b
      clk: meson: g12a: fix swapped VID_PLL and VID_PLL_DIV clock ids for sm1

 drivers/clk/meson/g12a.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
---
base-commit: d419d682257d813f6cb7a28e42b35d0edf4ad038
change-id: 20261022-fix-g12a-clk-1fb52b15cfa6

Best regards,
--  
Ronald Claveau <linux-kernel-dev@aliel.fr>



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

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

[Ronald Claveau via B4 Relay]

From: Ronald Claveau <linux-kernel-dev@aliel.fr>

g12a_fclk_div2p5 should be fclk divided by 2.5.
The mult has been declared as 1 which results
as fclk/5 instead of fclk/2.5.

Set mult=2 to get the correct div 2.5 rate.

Fixes: 085a4ea93d54 ("clk: meson: g12a: add peripheral clock controller")
Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
---
 drivers/clk/meson/g12a.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
index d0d4c7b6dc82..4ec5f15daf98 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",

-- 
2.43.0



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

[PATCH 2/4] clk: meson: g12a: fix swapped VID_PLL and VID_PLL_DIV clock ids

[Ronald Claveau via B4 Relay]

From: Ronald Claveau <linux-kernel-dev@aliel.fr>

In g12a onecell data table CLKID_VID_PLL points to g12a_vid_pll_div
and CLKID_VID_PLL_DIV points to g12a_vid_pll. So the ids are swapped.

Swap the ids back to their right entries.

Fixes: 085a4ea93d54 ("clk: meson: g12a: add peripheral clock controller")
Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
---
 drivers/clk/meson/g12a.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
index 4ec5f15daf98..07105b2089c3 100644
--- a/drivers/clk/meson/g12a.c
+++ b/drivers/clk/meson/g12a.c
@@ -4622,9 +4622,9 @@ static struct clk_hw *g12a_hw_clks[] = {
 	[CLKID_HDMI_PLL_OD]		= &g12a_hdmi_pll_od.hw,
 	[CLKID_HDMI_PLL_OD2]		= &g12a_hdmi_pll_od2.hw,
 	[CLKID_HDMI_PLL]		= &g12a_hdmi_pll.hw,
-	[CLKID_VID_PLL]			= &g12a_vid_pll_div.hw,
+	[CLKID_VID_PLL]			= &g12a_vid_pll.hw,
 	[CLKID_VID_PLL_SEL]		= &g12a_vid_pll_sel.hw,
-	[CLKID_VID_PLL_DIV]		= &g12a_vid_pll.hw,
+	[CLKID_VID_PLL_DIV]		= &g12a_vid_pll_div.hw,
 	[CLKID_VCLK_SEL]		= &g12a_vclk_sel.hw,
 	[CLKID_VCLK2_SEL]		= &g12a_vclk2_sel.hw,
 	[CLKID_VCLK_INPUT]		= &g12a_vclk_input.hw,

-- 
2.43.0



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

[PATCH 3/4] clk: meson: g12a: fix swapped VID_PLL and VID_PLL_DIV clock ids for g12b

[Ronald Claveau via B4 Relay]

From: Ronald Claveau <linux-kernel-dev@aliel.fr>

In g12b onecell data table CLKID_VID_PLL points to g12a_vid_pll_div
and CLKID_VID_PLL_DIV points to g12a_vid_pll. So the ids are swapped.

Swap the ids back to their right entries.

Fixes: d43628e9bca6 ("clk: meson: g12a: Add support for G12B CPUB clocks")
Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
---
 drivers/clk/meson/g12a.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
index 07105b2089c3..0a738c3c7fb5 100644
--- a/drivers/clk/meson/g12a.c
+++ b/drivers/clk/meson/g12a.c
@@ -4849,9 +4849,9 @@ static struct clk_hw *g12b_hw_clks[] = {
 	[CLKID_HDMI_PLL_OD]		= &g12a_hdmi_pll_od.hw,
 	[CLKID_HDMI_PLL_OD2]		= &g12a_hdmi_pll_od2.hw,
 	[CLKID_HDMI_PLL]		= &g12a_hdmi_pll.hw,
-	[CLKID_VID_PLL]			= &g12a_vid_pll_div.hw,
+	[CLKID_VID_PLL]			= &g12a_vid_pll.hw,
 	[CLKID_VID_PLL_SEL]		= &g12a_vid_pll_sel.hw,
-	[CLKID_VID_PLL_DIV]		= &g12a_vid_pll.hw,
+	[CLKID_VID_PLL_DIV]		= &g12a_vid_pll_div.hw,
 	[CLKID_VCLK_SEL]		= &g12a_vclk_sel.hw,
 	[CLKID_VCLK2_SEL]		= &g12a_vclk2_sel.hw,
 	[CLKID_VCLK_INPUT]		= &g12a_vclk_input.hw,

-- 
2.43.0



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

[PATCH 4/4] clk: meson: g12a: fix swapped VID_PLL and VID_PLL_DIV clock ids for sm1

[Ronald Claveau via B4 Relay]

From: Ronald Claveau <linux-kernel-dev@aliel.fr>

In sm1 onecell data table CLKID_VID_PLL points to g12a_vid_pll_div
and CLKID_VID_PLL_DIV points to g12a_vid_pll. So the ids are swapped.

Swap the ids back to their right entries.

Fixes: 3dd02b7334ec ("clk: meson: g12a: add support for SM1 GP1 PLL")
Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
---
 drivers/clk/meson/g12a.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
index 0a738c3c7fb5..e18cc151f4b6 100644
--- a/drivers/clk/meson/g12a.c
+++ b/drivers/clk/meson/g12a.c
@@ -5117,9 +5117,9 @@ static struct clk_hw *sm1_hw_clks[] = {
 	[CLKID_HDMI_PLL_OD]		= &g12a_hdmi_pll_od.hw,
 	[CLKID_HDMI_PLL_OD2]		= &g12a_hdmi_pll_od2.hw,
 	[CLKID_HDMI_PLL]		= &g12a_hdmi_pll.hw,
-	[CLKID_VID_PLL]			= &g12a_vid_pll_div.hw,
+	[CLKID_VID_PLL]			= &g12a_vid_pll.hw,
 	[CLKID_VID_PLL_SEL]		= &g12a_vid_pll_sel.hw,
-	[CLKID_VID_PLL_DIV]		= &g12a_vid_pll.hw,
+	[CLKID_VID_PLL_DIV]		= &g12a_vid_pll_div.hw,
 	[CLKID_VCLK_SEL]		= &g12a_vclk_sel.hw,
 	[CLKID_VCLK2_SEL]		= &g12a_vclk2_sel.hw,
 	[CLKID_VCLK_INPUT]		= &g12a_vclk_input.hw,

-- 
2.43.0



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

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

[sashiko-bot]

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