[PATCH] imx93-u-boot: Describe the CPU clocks in the devicetree

[PATCH] imx93-u-boot: Describe the CPU clocks in the devicetree

[Fabio Estevam]

Currently, there is an error when the i.MX93 CPU frequency is
read:

Could not read CPU frequency: -2
CPU:   NXP i.MX93(52) Rev1.1 A55 at 0 MHz

Fix it by describing the A55 clock nodes in the devicetree, like done
on other i.MX SoCs.

With this change, the CPU frequency error is gone and it can be correctly
retrieved:

CPU:   NXP i.MX93(52) Rev1.1 A55 at 1700 MHz
CPU:   Industrial temperature grade  (-40C to 105C) at 35C

As the upstream imx93.dtsi does not describe the CPU clocks,
keep the clock node in imx93-u-boot.dtsi for now.

Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
 arch/arm/dts/imx93-u-boot.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/dts/imx93-u-boot.dtsi b/arch/arm/dts/imx93-u-boot.dtsi
index 289aba175843..b2b3d57318fc 100644
--- a/arch/arm/dts/imx93-u-boot.dtsi
+++ b/arch/arm/dts/imx93-u-boot.dtsi
@@ -9,6 +9,14 @@
 	};
 };
 
+&A55_0 {
+	clocks = <&clk IMX93_CLK_ARM_PLL>;
+};
+
+&A55_1 {
+	clocks = <&clk IMX93_CLK_ARM_PLL>;
+};
+
 &binman {
 	u-boot-spl-ddr {
 		align = <4>;
-- 
2.34.1

RE: [PATCH] imx93-u-boot: Describe the CPU clocks in the devicetree

[Peng Fan]

> Subject: [PATCH] imx93-u-boot: Describe the CPU clocks in the
> devicetree
> 
> Currently, there is an error when the i.MX93 CPU frequency is
> read:
> 
> Could not read CPU frequency: -2
> CPU:   NXP i.MX93(52) Rev1.1 A55 at 0 MHz
> 
> Fix it by describing the A55 clock nodes in the devicetree, like done on
> other i.MX SoCs.
> 
> With this change, the CPU frequency error is gone and it can be
> correctly
> retrieved:
> 
> CPU:   NXP i.MX93(52) Rev1.1 A55 at 1700 MHz
> CPU:   Industrial temperature grade  (-40C to 105C) at 35C
> 
> As the upstream imx93.dtsi does not describe the CPU clocks, keep the
> clock node in imx93-u-boot.dtsi for now.
> 
> Signed-off-by: Fabio Estevam <festevam@gmail.com>

Reviewed-by: Peng Fan <peng.fan@nxp.com>

Re: [PATCH] imx93-u-boot: Describe the CPU clocks in the devicetree

[Sébastien Szymanski]

Hello Fabio,

On 7/18/24 18:54, Fabio Estevam wrote:
> Currently, there is an error when the i.MX93 CPU frequency is
> read:
> 
> Could not read CPU frequency: -2
> CPU:   NXP i.MX93(52) Rev1.1 A55 at 0 MHz
> 
> Fix it by describing the A55 clock nodes in the devicetree, like done
> on other i.MX SoCs.
> 
> With this change, the CPU frequency error is gone and it can be correctly
> retrieved:
> 
> CPU:   NXP i.MX93(52) Rev1.1 A55 at 1700 MHz
> CPU:   Industrial temperature grade  (-40C to 105C) at 35C
> 
> As the upstream imx93.dtsi does not describe the CPU clocks,
> keep the clock node in imx93-u-boot.dtsi for now.
> 
> Signed-off-by: Fabio Estevam <festevam@gmail.com>
> ---
>   arch/arm/dts/imx93-u-boot.dtsi | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm/dts/imx93-u-boot.dtsi b/arch/arm/dts/imx93-u-boot.dtsi
> index 289aba175843..b2b3d57318fc 100644
> --- a/arch/arm/dts/imx93-u-boot.dtsi
> +++ b/arch/arm/dts/imx93-u-boot.dtsi
> @@ -9,6 +9,14 @@
>   	};
>   };
>   
> +&A55_0 {
> +	clocks = <&clk IMX93_CLK_ARM_PLL>;

Why IMX93_CLK_ARM_PLL and not IMX93_CLK_A55_SEL ?

Best regards,

> +};
> +
> +&A55_1 {
> +	clocks = <&clk IMX93_CLK_ARM_PLL>;
> +};
> +
>   &binman {
>   	u-boot-spl-ddr {
>   		align = <4>;

-- 
Sébastien Szymanski, Armadeus Systems
Software engineer

Re: [PATCH] imx93-u-boot: Describe the CPU clocks in the devicetree

[Fabio Estevam]

Hi Sébastien,

On Fri, Jul 19, 2024 at 11:00 AM Sébastien Szymanski
<sebastien.szymanski@armadeus.com> wrote:

> > +&A55_0 {
> > +     clocks = <&clk IMX93_CLK_ARM_PLL>;
>
> Why IMX93_CLK_ARM_PLL and not IMX93_CLK_A55_SEL ?

IMX93_CLK_A55_SEL is a mux that can select between a55_alt versus arm_pll.

The real CPU clock is IMX93_CLK_ARM_PLL, so this one better represents it.

This is also consistent with the other i.MX8M SoCs.

Take imx8mm.dtsi for example:

A53_0: cpu@0 {
....
     clocks = <&clk IMX8MM_CLK_ARM>;

This also uses the "real" CPU clock, not the mux one (imx8mm_a53_sels).

I also plan to upstream this clock change to the Linux devicetree.

Thanks

Re: [PATCH] imx93-u-boot: Describe the CPU clocks in the devicetree

[Sébastien Szymanski]

On 7/19/24 16:13, Fabio Estevam wrote:
> Hi Sébastien,
> 
> On Fri, Jul 19, 2024 at 11:00 AM Sébastien Szymanski
> <sebastien.szymanski@armadeus.com> wrote:
> 
>>> +&A55_0 {
>>> +     clocks = <&clk IMX93_CLK_ARM_PLL>;
>>
>> Why IMX93_CLK_ARM_PLL and not IMX93_CLK_A55_SEL ?
> 
> IMX93_CLK_A55_SEL is a mux that can select between a55_alt versus arm_pll.
> 
> The real CPU clock is IMX93_CLK_ARM_PLL, so this one better represents it.

Ok, thank for the explanation.

> 
> This is also consistent with the other i.MX8M SoCs.
> 
> Take imx8mm.dtsi for example:
> 
> A53_0: cpu@0 {
> ....
>       clocks = <&clk IMX8MM_CLK_ARM>;
> 
> This also uses the "real" CPU clock, not the mux one (imx8mm_a53_sels).
> 
> I also plan to upstream this clock change to the Linux devicetree.

I did look at imx8mm.dtsi but the clock IMX8MM_CLK_ARM is not defined in 
U-Boot and in Linux it's defined as:

hws[IMX8MM_CLK_ARM] = imx_clk_hw_cpu("arm", "arm_a53_core",
                                      hws[IMX8MM_CLK_A53_CORE]->clk,
				     hws[IMX8MM_CLK_A53_CORE]->clk,
                                      hws[IMX8MM_ARM_PLL_OUT]->clk,
				     hws[IMX8MM_CLK_A53_DIV]->clk);

So in Linux for the i.MX93, the clocks property in the device tree has 
to be IMX93_CLK_A55_CORE:

clks[IMX93_CLK_A55_CORE] = imx_clk_hw_cpu("a55_core", "a55_sel",
					clks[IMX93_CLK_A55_SEL]->clk,
					clks[IMX93_CLK_A55_SEL]->clk,
					clks[IMX93_CLK_ARM_PLL]->clk,
					clks[IMX93_CLK_A55_GATE]->clk);

?

Regards,

> 
> Thanks

-- 
Sébastien Szymanski, Armadeus Systems
Software engineer

Re: [PATCH] imx93-u-boot: Describe the CPU clocks in the devicetree

[Fabio Estevam]

Hi Sébastien,

On Fri, Jul 19, 2024 at 12:47 PM Sébastien Szymanski
<sebastien.szymanski@armadeus.com> wrote:

> I did look at imx8mm.dtsi but the clock IMX8MM_CLK_ARM is not defined in
> U-Boot and in Linux it's defined as:
>
> hws[IMX8MM_CLK_ARM] = imx_clk_hw_cpu("arm", "arm_a53_core",
>                                       hws[IMX8MM_CLK_A53_CORE]->clk,
>                                      hws[IMX8MM_CLK_A53_CORE]->clk,
>                                       hws[IMX8MM_ARM_PLL_OUT]->clk,
>                                      hws[IMX8MM_CLK_A53_DIV]->clk);
>
> So in Linux for the i.MX93, the clocks property in the device tree has
> to be IMX93_CLK_A55_CORE:
>
> clks[IMX93_CLK_A55_CORE] = imx_clk_hw_cpu("a55_core", "a55_sel",
>                                         clks[IMX93_CLK_A55_SEL]->clk,
>                                         clks[IMX93_CLK_A55_SEL]->clk,
>                                         clks[IMX93_CLK_ARM_PLL]->clk,
>                                         clks[IMX93_CLK_A55_GATE]->clk);
>
> ?

Yes, that's correct. I will use IMX93_CLK_A55_CORE when upstreaming it.

At some point, we should align these clock names between Linux and U-Boot.

For now, as I am touching arch/arm/dts/imx93-u-boot.dtsi this is OK
for U-Boot purposes.

Thanks

Re: [PATCH] imx93-u-boot: Describe the CPU clocks in the devicetree

[Fabio Estevam]

On Thu, Jul 18, 2024 at 1:55 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> Currently, there is an error when the i.MX93 CPU frequency is
> read:
>
> Could not read CPU frequency: -2
> CPU:   NXP i.MX93(52) Rev1.1 A55 at 0 MHz
>
> Fix it by describing the A55 clock nodes in the devicetree, like done
> on other i.MX SoCs.
>
> With this change, the CPU frequency error is gone and it can be correctly
> retrieved:
>
> CPU:   NXP i.MX93(52) Rev1.1 A55 at 1700 MHz
> CPU:   Industrial temperature grade  (-40C to 105C) at 35C
>
> As the upstream imx93.dtsi does not describe the CPU clocks,
> keep the clock node in imx93-u-boot.dtsi for now.
>
> Signed-off-by: Fabio Estevam <festevam@gmail.com>

Applied, thanks.