[PATCH RESEND V6 1/2] arm64: dts: freescale: imx8qxp: add cpu opp table

[PATCH RESEND V6 1/2] arm64: dts: freescale: imx8qxp: add cpu opp table

[Anson Huang]

Add i.MX8QXP CPU opp table to support cpufreq.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
No changes.
---
 arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
index 4c3dd95..41bf0ce 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
@@ -34,6 +34,9 @@
 			reg = <0x0 0x0>;
 			enable-method = "psci";
 			next-level-cache = <&A35_L2>;
+			clocks = <&clk IMX_A35_CLK>;
+			operating-points-v2 = <&a35_0_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		A35_1: cpu@1 {
@@ -42,6 +45,9 @@
 			reg = <0x0 0x1>;
 			enable-method = "psci";
 			next-level-cache = <&A35_L2>;
+			clocks = <&clk IMX_A35_CLK>;
+			operating-points-v2 = <&a35_0_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		A35_2: cpu@2 {
@@ -50,6 +56,9 @@
 			reg = <0x0 0x2>;
 			enable-method = "psci";
 			next-level-cache = <&A35_L2>;
+			clocks = <&clk IMX_A35_CLK>;
+			operating-points-v2 = <&a35_0_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		A35_3: cpu@3 {
@@ -58,6 +67,9 @@
 			reg = <0x0 0x3>;
 			enable-method = "psci";
 			next-level-cache = <&A35_L2>;
+			clocks = <&clk IMX_A35_CLK>;
+			operating-points-v2 = <&a35_0_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		A35_L2: l2-cache0 {
@@ -65,6 +77,24 @@
 		};
 	};
 
+	a35_0_opp_table: opp-table {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		opp-900000000 {
+			opp-hz = /bits/ 64 <900000000>;
+			opp-microvolt = <1000000>;
+			clock-latency-ns = <150000>;
+		};
+
+		opp-1200000000 {
+			opp-hz = /bits/ 64 <1200000000>;
+			opp-microvolt = <1100000>;
+			clock-latency-ns = <150000>;
+			opp-suspend;
+		};
+	};
+
 	gic: interrupt-controller@51a00000 {
 		compatible = "arm,gic-v3";
 		reg = <0x0 0x51a00000 0 0x10000>, /* GIC Dist */
-- 
2.7.4

[PATCH RESEND V6 1/2] arm64: dts: freescale: imx8qxp: add cpu opp table

[Anson Huang]

Add i.MX8QXP CPU opp table to support cpufreq.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
No changes.
---
 arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
index 4c3dd95..41bf0ce 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
@@ -34,6 +34,9 @@
 			reg = <0x0 0x0>;
 			enable-method = "psci";
 			next-level-cache = <&A35_L2>;
+			clocks = <&clk IMX_A35_CLK>;
+			operating-points-v2 = <&a35_0_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		A35_1: cpu@1 {
@@ -42,6 +45,9 @@
 			reg = <0x0 0x1>;
 			enable-method = "psci";
 			next-level-cache = <&A35_L2>;
+			clocks = <&clk IMX_A35_CLK>;
+			operating-points-v2 = <&a35_0_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		A35_2: cpu@2 {
@@ -50,6 +56,9 @@
 			reg = <0x0 0x2>;
 			enable-method = "psci";
 			next-level-cache = <&A35_L2>;
+			clocks = <&clk IMX_A35_CLK>;
+			operating-points-v2 = <&a35_0_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		A35_3: cpu@3 {
@@ -58,6 +67,9 @@
 			reg = <0x0 0x3>;
 			enable-method = "psci";
 			next-level-cache = <&A35_L2>;
+			clocks = <&clk IMX_A35_CLK>;
+			operating-points-v2 = <&a35_0_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		A35_L2: l2-cache0 {
@@ -65,6 +77,24 @@
 		};
 	};
 
+	a35_0_opp_table: opp-table {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		opp-900000000 {
+			opp-hz = /bits/ 64 <900000000>;
+			opp-microvolt = <1000000>;
+			clock-latency-ns = <150000>;
+		};
+
+		opp-1200000000 {
+			opp-hz = /bits/ 64 <1200000000>;
+			opp-microvolt = <1100000>;
+			clock-latency-ns = <150000>;
+			opp-suspend;
+		};
+	};
+
 	gic: interrupt-controller@51a00000 {
 		compatible = "arm,gic-v3";
 		reg = <0x0 0x51a00000 0 0x10000>, /* GIC Dist */
-- 
2.7.4

[PATCH RESEND V6 1/2] arm64: dts: freescale: imx8qxp: add cpu opp table

[Anson Huang]

Add i.MX8QXP CPU opp table to support cpufreq.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
No changes.
---
 arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
index 4c3dd95..41bf0ce 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
@@ -34,6 +34,9 @@
 			reg = <0x0 0x0>;
 			enable-method = "psci";
 			next-level-cache = <&A35_L2>;
+			clocks = <&clk IMX_A35_CLK>;
+			operating-points-v2 = <&a35_0_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		A35_1: cpu@1 {
@@ -42,6 +45,9 @@
 			reg = <0x0 0x1>;
 			enable-method = "psci";
 			next-level-cache = <&A35_L2>;
+			clocks = <&clk IMX_A35_CLK>;
+			operating-points-v2 = <&a35_0_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		A35_2: cpu@2 {
@@ -50,6 +56,9 @@
 			reg = <0x0 0x2>;
 			enable-method = "psci";
 			next-level-cache = <&A35_L2>;
+			clocks = <&clk IMX_A35_CLK>;
+			operating-points-v2 = <&a35_0_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		A35_3: cpu@3 {
@@ -58,6 +67,9 @@
 			reg = <0x0 0x3>;
 			enable-method = "psci";
 			next-level-cache = <&A35_L2>;
+			clocks = <&clk IMX_A35_CLK>;
+			operating-points-v2 = <&a35_0_opp_table>;
+			#cooling-cells = <2>;
 		};
 
 		A35_L2: l2-cache0 {
@@ -65,6 +77,24 @@
 		};
 	};
 
+	a35_0_opp_table: opp-table {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		opp-900000000 {
+			opp-hz = /bits/ 64 <900000000>;
+			opp-microvolt = <1000000>;
+			clock-latency-ns = <150000>;
+		};
+
+		opp-1200000000 {
+			opp-hz = /bits/ 64 <1200000000>;
+			opp-microvolt = <1100000>;
+			clock-latency-ns = <150000>;
+			opp-suspend;
+		};
+	};
+
 	gic: interrupt-controller@51a00000 {
 		compatible = "arm,gic-v3";
 		reg = <0x0 0x51a00000 0 0x10000>, /* GIC Dist */
-- 
2.7.4


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

[PATCH RESEND V6 2/2] clk: imx: scu: add cpu frequency scaling support

[Anson Huang]

On NXP's i.MX SoCs with system controller inside, CPU frequency
scaling can ONLY be done by system controller firmware, and it
can ONLY be requested from secure mode, so Linux kernel has to
call ARM SMC to trap to ARM-Trusted-Firmware to request system
controller firmware to do CPU frequency scaling.

This patch adds i.MX system controller CPU frequency scaling support,
it reuses cpufreq-dt driver and implement the CPU frequency scaling
inside SCU clock driver.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No changes compared to last version, just redo patch based on clk-next to fix patch conflicts.
---
 drivers/clk/imx/clk-scu.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
index f460526..34c7cdf4 100644
--- a/drivers/clk/imx/clk-scu.c
+++ b/drivers/clk/imx/clk-scu.c
@@ -4,12 +4,17 @@
  *   Dong Aisheng <aisheng.dong@nxp.com>
  */
 
+#include <dt-bindings/firmware/imx/rsrc.h>
+#include <linux/arm-smccc.h>
 #include <linux/clk-provider.h>
 #include <linux/err.h>
 #include <linux/slab.h>
 
 #include "clk-scu.h"
 
+#define IMX_SIP_CPUFREQ			0xC2000001
+#define IMX_SIP_SET_CPUFREQ		0x00
+
 static struct imx_sc_ipc *ccm_ipc_handle;
 
 /*
@@ -180,6 +185,23 @@ static long clk_scu_round_rate(struct clk_hw *hw, unsigned long rate,
 	return rate;
 }
 
+static int clk_scu_atf_set_cpu_rate(struct clk_hw *hw, unsigned long rate,
+				    unsigned long parent_rate)
+{
+	struct clk_scu *clk = to_clk_scu(hw);
+	struct arm_smccc_res res;
+	unsigned long cluster_id;
+
+	if (clk->rsrc_id == IMX_SC_R_A35)
+		cluster_id = 0;
+
+	/* CPU frequency scaling can ONLY be done by ARM-Trusted-Firmware */
+	arm_smccc_smc(IMX_SIP_CPUFREQ, IMX_SIP_SET_CPUFREQ,
+		      cluster_id, rate, 0, 0, 0, 0, &res);
+
+	return 0;
+}
+
 /*
  * clk_scu_set_rate - Set rate for a SCU clock
  * @hw: clock to change rate for
@@ -312,6 +334,14 @@ static const struct clk_ops clk_scu_ops = {
 	.unprepare = clk_scu_unprepare,
 };
 
+static const struct clk_ops clk_scu_cpu_ops = {
+	.recalc_rate = clk_scu_recalc_rate,
+	.round_rate = clk_scu_round_rate,
+	.set_rate = clk_scu_atf_set_cpu_rate,
+	.prepare = clk_scu_prepare,
+	.unprepare = clk_scu_unprepare,
+};
+
 struct clk_hw *__imx_clk_scu(const char *name, const char * const *parents,
 			     int num_parents, u32 rsrc_id, u8 clk_type)
 {
@@ -329,6 +359,10 @@ struct clk_hw *__imx_clk_scu(const char *name, const char * const *parents,
 
 	init.name = name;
 	init.ops = &clk_scu_ops;
+	if (rsrc_id == IMX_SC_R_A35)
+		init.ops = &clk_scu_cpu_ops;
+	else
+		init.ops = &clk_scu_ops;
 	init.parent_names = parents;
 	init.num_parents = num_parents;
 
-- 
2.7.4

[PATCH RESEND V6 2/2] clk: imx: scu: add cpu frequency scaling support

[Anson Huang]

On NXP's i.MX SoCs with system controller inside, CPU frequency
scaling can ONLY be done by system controller firmware, and it
can ONLY be requested from secure mode, so Linux kernel has to
call ARM SMC to trap to ARM-Trusted-Firmware to request system
controller firmware to do CPU frequency scaling.

This patch adds i.MX system controller CPU frequency scaling support,
it reuses cpufreq-dt driver and implement the CPU frequency scaling
inside SCU clock driver.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No changes compared to last version, just redo patch based on clk-next to fix patch conflicts.
---
 drivers/clk/imx/clk-scu.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
index f460526..34c7cdf4 100644
--- a/drivers/clk/imx/clk-scu.c
+++ b/drivers/clk/imx/clk-scu.c
@@ -4,12 +4,17 @@
  *   Dong Aisheng <aisheng.dong@nxp.com>
  */
 
+#include <dt-bindings/firmware/imx/rsrc.h>
+#include <linux/arm-smccc.h>
 #include <linux/clk-provider.h>
 #include <linux/err.h>
 #include <linux/slab.h>
 
 #include "clk-scu.h"
 
+#define IMX_SIP_CPUFREQ			0xC2000001
+#define IMX_SIP_SET_CPUFREQ		0x00
+
 static struct imx_sc_ipc *ccm_ipc_handle;
 
 /*
@@ -180,6 +185,23 @@ static long clk_scu_round_rate(struct clk_hw *hw, unsigned long rate,
 	return rate;
 }
 
+static int clk_scu_atf_set_cpu_rate(struct clk_hw *hw, unsigned long rate,
+				    unsigned long parent_rate)
+{
+	struct clk_scu *clk = to_clk_scu(hw);
+	struct arm_smccc_res res;
+	unsigned long cluster_id;
+
+	if (clk->rsrc_id == IMX_SC_R_A35)
+		cluster_id = 0;
+
+	/* CPU frequency scaling can ONLY be done by ARM-Trusted-Firmware */
+	arm_smccc_smc(IMX_SIP_CPUFREQ, IMX_SIP_SET_CPUFREQ,
+		      cluster_id, rate, 0, 0, 0, 0, &res);
+
+	return 0;
+}
+
 /*
  * clk_scu_set_rate - Set rate for a SCU clock
  * @hw: clock to change rate for
@@ -312,6 +334,14 @@ static const struct clk_ops clk_scu_ops = {
 	.unprepare = clk_scu_unprepare,
 };
 
+static const struct clk_ops clk_scu_cpu_ops = {
+	.recalc_rate = clk_scu_recalc_rate,
+	.round_rate = clk_scu_round_rate,
+	.set_rate = clk_scu_atf_set_cpu_rate,
+	.prepare = clk_scu_prepare,
+	.unprepare = clk_scu_unprepare,
+};
+
 struct clk_hw *__imx_clk_scu(const char *name, const char * const *parents,
 			     int num_parents, u32 rsrc_id, u8 clk_type)
 {
@@ -329,6 +359,10 @@ struct clk_hw *__imx_clk_scu(const char *name, const char * const *parents,
 
 	init.name = name;
 	init.ops = &clk_scu_ops;
+	if (rsrc_id == IMX_SC_R_A35)
+		init.ops = &clk_scu_cpu_ops;
+	else
+		init.ops = &clk_scu_ops;
 	init.parent_names = parents;
 	init.num_parents = num_parents;
 
-- 
2.7.4

[PATCH RESEND V6 2/2] clk: imx: scu: add cpu frequency scaling support

[Anson Huang]

On NXP's i.MX SoCs with system controller inside, CPU frequency
scaling can ONLY be done by system controller firmware, and it
can ONLY be requested from secure mode, so Linux kernel has to
call ARM SMC to trap to ARM-Trusted-Firmware to request system
controller firmware to do CPU frequency scaling.

This patch adds i.MX system controller CPU frequency scaling support,
it reuses cpufreq-dt driver and implement the CPU frequency scaling
inside SCU clock driver.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No changes compared to last version, just redo patch based on clk-next to fix patch conflicts.
---
 drivers/clk/imx/clk-scu.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
index f460526..34c7cdf4 100644
--- a/drivers/clk/imx/clk-scu.c
+++ b/drivers/clk/imx/clk-scu.c
@@ -4,12 +4,17 @@
  *   Dong Aisheng <aisheng.dong@nxp.com>
  */
 
+#include <dt-bindings/firmware/imx/rsrc.h>
+#include <linux/arm-smccc.h>
 #include <linux/clk-provider.h>
 #include <linux/err.h>
 #include <linux/slab.h>
 
 #include "clk-scu.h"
 
+#define IMX_SIP_CPUFREQ			0xC2000001
+#define IMX_SIP_SET_CPUFREQ		0x00
+
 static struct imx_sc_ipc *ccm_ipc_handle;
 
 /*
@@ -180,6 +185,23 @@ static long clk_scu_round_rate(struct clk_hw *hw, unsigned long rate,
 	return rate;
 }
 
+static int clk_scu_atf_set_cpu_rate(struct clk_hw *hw, unsigned long rate,
+				    unsigned long parent_rate)
+{
+	struct clk_scu *clk = to_clk_scu(hw);
+	struct arm_smccc_res res;
+	unsigned long cluster_id;
+
+	if (clk->rsrc_id == IMX_SC_R_A35)
+		cluster_id = 0;
+
+	/* CPU frequency scaling can ONLY be done by ARM-Trusted-Firmware */
+	arm_smccc_smc(IMX_SIP_CPUFREQ, IMX_SIP_SET_CPUFREQ,
+		      cluster_id, rate, 0, 0, 0, 0, &res);
+
+	return 0;
+}
+
 /*
  * clk_scu_set_rate - Set rate for a SCU clock
  * @hw: clock to change rate for
@@ -312,6 +334,14 @@ static const struct clk_ops clk_scu_ops = {
 	.unprepare = clk_scu_unprepare,
 };
 
+static const struct clk_ops clk_scu_cpu_ops = {
+	.recalc_rate = clk_scu_recalc_rate,
+	.round_rate = clk_scu_round_rate,
+	.set_rate = clk_scu_atf_set_cpu_rate,
+	.prepare = clk_scu_prepare,
+	.unprepare = clk_scu_unprepare,
+};
+
 struct clk_hw *__imx_clk_scu(const char *name, const char * const *parents,
 			     int num_parents, u32 rsrc_id, u8 clk_type)
 {
@@ -329,6 +359,10 @@ struct clk_hw *__imx_clk_scu(const char *name, const char * const *parents,
 
 	init.name = name;
 	init.ops = &clk_scu_ops;
+	if (rsrc_id == IMX_SC_R_A35)
+		init.ops = &clk_scu_cpu_ops;
+	else
+		init.ops = &clk_scu_ops;
 	init.parent_names = parents;
 	init.num_parents = num_parents;
 
-- 
2.7.4


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

Re: [PATCH RESEND V6 2/2] clk: imx: scu: add cpu frequency scaling support

[Stephen Boyd]

Quoting Anson Huang (2019-02-21 18:32:10)
> On NXP's i.MX SoCs with system controller inside, CPU frequency
> scaling can ONLY be done by system controller firmware, and it
> can ONLY be requested from secure mode, so Linux kernel has to
> call ARM SMC to trap to ARM-Trusted-Firmware to request system
> controller firmware to do CPU frequency scaling.
> 
> This patch adds i.MX system controller CPU frequency scaling support,
> it reuses cpufreq-dt driver and implement the CPU frequency scaling
> inside SCU clock driver.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---

Applied to clk-next

Re: [PATCH RESEND V6 2/2] clk: imx: scu: add cpu frequency scaling support

[Stephen Boyd]

Quoting Anson Huang (2019-02-21 18:32:10)
> On NXP's i.MX SoCs with system controller inside, CPU frequency
> scaling can ONLY be done by system controller firmware, and it
> can ONLY be requested from secure mode, so Linux kernel has to
> call ARM SMC to trap to ARM-Trusted-Firmware to request system
> controller firmware to do CPU frequency scaling.
> 
> This patch adds i.MX system controller CPU frequency scaling support,
> it reuses cpufreq-dt driver and implement the CPU frequency scaling
> inside SCU clock driver.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---

Applied to clk-next

Re: [PATCH RESEND V6 2/2] clk: imx: scu: add cpu frequency scaling support

[Stephen Boyd]

Quoting Anson Huang (2019-02-21 18:32:10)
> On NXP's i.MX SoCs with system controller inside, CPU frequency
> scaling can ONLY be done by system controller firmware, and it
> can ONLY be requested from secure mode, so Linux kernel has to
> call ARM SMC to trap to ARM-Trusted-Firmware to request system
> controller firmware to do CPU frequency scaling.
> 
> This patch adds i.MX system controller CPU frequency scaling support,
> it reuses cpufreq-dt driver and implement the CPU frequency scaling
> inside SCU clock driver.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---

Applied to clk-next


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

Re: [PATCH RESEND V6 2/2] clk: imx: scu: add cpu frequency scaling support

[Stephen Boyd]

Quoting Anson Huang (2019-02-21 18:32:10)
> On NXP's i.MX SoCs with system controller inside, CPU frequency
> scaling can ONLY be done by system controller firmware, and it
> can ONLY be requested from secure mode, so Linux kernel has to
> call ARM SMC to trap to ARM-Trusted-Firmware to request system
> controller firmware to do CPU frequency scaling.
> 
> This patch adds i.MX system controller CPU frequency scaling support,
> it reuses cpufreq-dt driver and implement the CPU frequency scaling
> inside SCU clock driver.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>

Ah I missed one thing, see below.

> @@ -180,6 +185,23 @@ static long clk_scu_round_rate(struct clk_hw *hw, unsigned long rate,
>         return rate;
>  }
>  
> +static int clk_scu_atf_set_cpu_rate(struct clk_hw *hw, unsigned long rate,
> +                                   unsigned long parent_rate)
> +{
> +       struct clk_scu *clk = to_clk_scu(hw);
> +       struct arm_smccc_res res;
> +       unsigned long cluster_id;
> +
> +       if (clk->rsrc_id == IMX_SC_R_A35)
> +               cluster_id = 0;

Do we still need to check this anymore? Why not just always use
cluster_id 0?

> +
> +       /* CPU frequency scaling can ONLY be done by ARM-Trusted-Firmware */
> +       arm_smccc_smc(IMX_SIP_CPUFREQ, IMX_SIP_SET_CPUFREQ,
> +                     cluster_id, rate, 0, 0, 0, 0, &res);

Because not checking would make this work, vs. checking causes this code
to sometimes use an uninitialized value from the stack.

> +
> +       return 0;
> +}
> +

Re: [PATCH RESEND V6 2/2] clk: imx: scu: add cpu frequency scaling support

[Stephen Boyd]

Quoting Anson Huang (2019-02-21 18:32:10)
> On NXP's i.MX SoCs with system controller inside, CPU frequency
> scaling can ONLY be done by system controller firmware, and it
> can ONLY be requested from secure mode, so Linux kernel has to
> call ARM SMC to trap to ARM-Trusted-Firmware to request system
> controller firmware to do CPU frequency scaling.
> 
> This patch adds i.MX system controller CPU frequency scaling support,
> it reuses cpufreq-dt driver and implement the CPU frequency scaling
> inside SCU clock driver.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>

Ah I missed one thing, see below.

> @@ -180,6 +185,23 @@ static long clk_scu_round_rate(struct clk_hw *hw, unsigned long rate,
>         return rate;
>  }
>  
> +static int clk_scu_atf_set_cpu_rate(struct clk_hw *hw, unsigned long rate,
> +                                   unsigned long parent_rate)
> +{
> +       struct clk_scu *clk = to_clk_scu(hw);
> +       struct arm_smccc_res res;
> +       unsigned long cluster_id;
> +
> +       if (clk->rsrc_id == IMX_SC_R_A35)
> +               cluster_id = 0;

Do we still need to check this anymore? Why not just always use
cluster_id 0?

> +
> +       /* CPU frequency scaling can ONLY be done by ARM-Trusted-Firmware */
> +       arm_smccc_smc(IMX_SIP_CPUFREQ, IMX_SIP_SET_CPUFREQ,
> +                     cluster_id, rate, 0, 0, 0, 0, &res);

Because not checking would make this work, vs. checking causes this code
to sometimes use an uninitialized value from the stack.

> +
> +       return 0;
> +}
> +

Re: [PATCH RESEND V6 2/2] clk: imx: scu: add cpu frequency scaling support

[Stephen Boyd]

Quoting Anson Huang (2019-02-21 18:32:10)
> On NXP's i.MX SoCs with system controller inside, CPU frequency
> scaling can ONLY be done by system controller firmware, and it
> can ONLY be requested from secure mode, so Linux kernel has to
> call ARM SMC to trap to ARM-Trusted-Firmware to request system
> controller firmware to do CPU frequency scaling.
> 
> This patch adds i.MX system controller CPU frequency scaling support,
> it reuses cpufreq-dt driver and implement the CPU frequency scaling
> inside SCU clock driver.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>

Ah I missed one thing, see below.

> @@ -180,6 +185,23 @@ static long clk_scu_round_rate(struct clk_hw *hw, unsigned long rate,
>         return rate;
>  }
>  
> +static int clk_scu_atf_set_cpu_rate(struct clk_hw *hw, unsigned long rate,
> +                                   unsigned long parent_rate)
> +{
> +       struct clk_scu *clk = to_clk_scu(hw);
> +       struct arm_smccc_res res;
> +       unsigned long cluster_id;
> +
> +       if (clk->rsrc_id == IMX_SC_R_A35)
> +               cluster_id = 0;

Do we still need to check this anymore? Why not just always use
cluster_id 0?

> +
> +       /* CPU frequency scaling can ONLY be done by ARM-Trusted-Firmware */
> +       arm_smccc_smc(IMX_SIP_CPUFREQ, IMX_SIP_SET_CPUFREQ,
> +                     cluster_id, rate, 0, 0, 0, 0, &res);

Because not checking would make this work, vs. checking causes this code
to sometimes use an uninitialized value from the stack.

> +
> +       return 0;
> +}
> +

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

RE: [PATCH RESEND V6 2/2] clk: imx: scu: add cpu frequency scaling support

[Anson Huang]

Hi, Stehpen

Best Regards!
Anson Huang

> -----Original Message-----
> From: Stephen Boyd [mailto:sboyd@kernel.org]
> Sent: 2019年2月23日 3:08
> To: devicetree@vger.kernel.org; festevam@gmail.com;
> kernel@pengutronix.de; linux-arm-kernel@lists.infradead.org; linux-
> clk@vger.kernel.org; linux-kernel@vger.kernel.org; mark.rutland@arm.com;
> mturquette@baylibre.com; robh+dt@kernel.org; s.hauer@pengutronix.de;
> shawnguo@kernel.org; Aisheng Dong <aisheng.dong@nxp.com>; Anson
> Huang <anson.huang@nxp.com>; Daniel Baluta <daniel.baluta@nxp.com>
> Cc: dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [PATCH RESEND V6 2/2] clk: imx: scu: add cpu frequency scaling
> support
> 
> Quoting Anson Huang (2019-02-21 18:32:10)
> > On NXP's i.MX SoCs with system controller inside, CPU frequency
> > scaling can ONLY be done by system controller firmware, and it can
> > ONLY be requested from secure mode, so Linux kernel has to call ARM
> > SMC to trap to ARM-Trusted-Firmware to request system controller
> > firmware to do CPU frequency scaling.
> >
> > This patch adds i.MX system controller CPU frequency scaling support,
> > it reuses cpufreq-dt driver and implement the CPU frequency scaling
> > inside SCU clock driver.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> 
> Ah I missed one thing, see below.
> 
> > @@ -180,6 +185,23 @@ static long clk_scu_round_rate(struct clk_hw *hw,
> unsigned long rate,
> >         return rate;
> >  }
> >
> > +static int clk_scu_atf_set_cpu_rate(struct clk_hw *hw, unsigned long rate,
> > +                                   unsigned long parent_rate) {
> > +       struct clk_scu *clk = to_clk_scu(hw);
> > +       struct arm_smccc_res res;
> > +       unsigned long cluster_id;
> > +
> > +       if (clk->rsrc_id == IMX_SC_R_A35)
> > +               cluster_id = 0;
> 
> Do we still need to check this anymore? Why not just always use cluster_id 0?

The i.MX8QXP ONLY has 1 cluster named A35, while on i.MX8QM there will be
2 clusters, A53 and A72, so we need to use the resource ID to initialize the cluster_id.

> 
> > +
> > +       /* CPU frequency scaling can ONLY be done by ARM-Trusted-Firmware
> */
> > +       arm_smccc_smc(IMX_SIP_CPUFREQ, IMX_SIP_SET_CPUFREQ,
> > +                     cluster_id, rate, 0, 0, 0, 0, &res);
> 
> Because not checking would make this work, vs. checking causes this code to
> sometimes use an uninitialized value from the stack.

 89 +       if (rsrc_id == IMX_SC_R_A35)
 90 +               init.ops = &clk_scu_cpu_ops;
 91 +       else
 92 +               init.ops = &clk_scu_ops;

I think it should be good. Only when plan to support cpu-freq scaling, then the
CPU clock will be switched to use clk_scu_cpu_ops and the clutser_id initialization
will be done according to CPU resource. For example, when we plan to support i.MX8QM
cpu-freq scaling, we will add A53 and A72 check here and switch the clock ops to clk_scu_cpu_ops,
also we will add the cluster_id initialization in the SMC clock set rate.

Thanks,
Anson.

> 
> > +
> > +       return 0;
> > +}
> > +

RE: [PATCH RESEND V6 2/2] clk: imx: scu: add cpu frequency scaling support

[Anson Huang]

Hi, Stehpen

Best Regards!
Anson Huang

> -----Original Message-----
> From: Stephen Boyd [mailto:sboyd@kernel.org]
> Sent: 2019年2月23日 3:08
> To: devicetree@vger.kernel.org; festevam@gmail.com;
> kernel@pengutronix.de; linux-arm-kernel@lists.infradead.org; linux-
> clk@vger.kernel.org; linux-kernel@vger.kernel.org; mark.rutland@arm.com;
> mturquette@baylibre.com; robh+dt@kernel.org; s.hauer@pengutronix.de;
> shawnguo@kernel.org; Aisheng Dong <aisheng.dong@nxp.com>; Anson
> Huang <anson.huang@nxp.com>; Daniel Baluta <daniel.baluta@nxp.com>
> Cc: dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [PATCH RESEND V6 2/2] clk: imx: scu: add cpu frequency scaling
> support
> 
> Quoting Anson Huang (2019-02-21 18:32:10)
> > On NXP's i.MX SoCs with system controller inside, CPU frequency
> > scaling can ONLY be done by system controller firmware, and it can
> > ONLY be requested from secure mode, so Linux kernel has to call ARM
> > SMC to trap to ARM-Trusted-Firmware to request system controller
> > firmware to do CPU frequency scaling.
> >
> > This patch adds i.MX system controller CPU frequency scaling support,
> > it reuses cpufreq-dt driver and implement the CPU frequency scaling
> > inside SCU clock driver.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> 
> Ah I missed one thing, see below.
> 
> > @@ -180,6 +185,23 @@ static long clk_scu_round_rate(struct clk_hw *hw,
> unsigned long rate,
> >         return rate;
> >  }
> >
> > +static int clk_scu_atf_set_cpu_rate(struct clk_hw *hw, unsigned long rate,
> > +                                   unsigned long parent_rate) {
> > +       struct clk_scu *clk = to_clk_scu(hw);
> > +       struct arm_smccc_res res;
> > +       unsigned long cluster_id;
> > +
> > +       if (clk->rsrc_id == IMX_SC_R_A35)
> > +               cluster_id = 0;
> 
> Do we still need to check this anymore? Why not just always use cluster_id 0?

The i.MX8QXP ONLY has 1 cluster named A35, while on i.MX8QM there will be
2 clusters, A53 and A72, so we need to use the resource ID to initialize the cluster_id.

> 
> > +
> > +       /* CPU frequency scaling can ONLY be done by ARM-Trusted-Firmware
> */
> > +       arm_smccc_smc(IMX_SIP_CPUFREQ, IMX_SIP_SET_CPUFREQ,
> > +                     cluster_id, rate, 0, 0, 0, 0, &res);
> 
> Because not checking would make this work, vs. checking causes this code to
> sometimes use an uninitialized value from the stack.

 89 +       if (rsrc_id == IMX_SC_R_A35)
 90 +               init.ops = &clk_scu_cpu_ops;
 91 +       else
 92 +               init.ops = &clk_scu_ops;

I think it should be good. Only when plan to support cpu-freq scaling, then the
CPU clock will be switched to use clk_scu_cpu_ops and the clutser_id initialization
will be done according to CPU resource. For example, when we plan to support i.MX8QM
cpu-freq scaling, we will add A53 and A72 check here and switch the clock ops to clk_scu_cpu_ops,
also we will add the cluster_id initialization in the SMC clock set rate.

Thanks,
Anson.

> 
> > +
> > +       return 0;
> > +}
> > +

RE: [PATCH RESEND V6 2/2] clk: imx: scu: add cpu frequency scaling support

[Anson Huang]

Hi, Stehpen

Best Regards!
Anson Huang

> -----Original Message-----
> From: Stephen Boyd [mailto:sboyd@kernel.org]
> Sent: 2019年2月23日 3:08
> To: devicetree@vger.kernel.org; festevam@gmail.com;
> kernel@pengutronix.de; linux-arm-kernel@lists.infradead.org; linux-
> clk@vger.kernel.org; linux-kernel@vger.kernel.org; mark.rutland@arm.com;
> mturquette@baylibre.com; robh+dt@kernel.org; s.hauer@pengutronix.de;
> shawnguo@kernel.org; Aisheng Dong <aisheng.dong@nxp.com>; Anson
> Huang <anson.huang@nxp.com>; Daniel Baluta <daniel.baluta@nxp.com>
> Cc: dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [PATCH RESEND V6 2/2] clk: imx: scu: add cpu frequency scaling
> support
> 
> Quoting Anson Huang (2019-02-21 18:32:10)
> > On NXP's i.MX SoCs with system controller inside, CPU frequency
> > scaling can ONLY be done by system controller firmware, and it can
> > ONLY be requested from secure mode, so Linux kernel has to call ARM
> > SMC to trap to ARM-Trusted-Firmware to request system controller
> > firmware to do CPU frequency scaling.
> >
> > This patch adds i.MX system controller CPU frequency scaling support,
> > it reuses cpufreq-dt driver and implement the CPU frequency scaling
> > inside SCU clock driver.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> 
> Ah I missed one thing, see below.
> 
> > @@ -180,6 +185,23 @@ static long clk_scu_round_rate(struct clk_hw *hw,
> unsigned long rate,
> >         return rate;
> >  }
> >
> > +static int clk_scu_atf_set_cpu_rate(struct clk_hw *hw, unsigned long rate,
> > +                                   unsigned long parent_rate) {
> > +       struct clk_scu *clk = to_clk_scu(hw);
> > +       struct arm_smccc_res res;
> > +       unsigned long cluster_id;
> > +
> > +       if (clk->rsrc_id == IMX_SC_R_A35)
> > +               cluster_id = 0;
> 
> Do we still need to check this anymore? Why not just always use cluster_id 0?

The i.MX8QXP ONLY has 1 cluster named A35, while on i.MX8QM there will be
2 clusters, A53 and A72, so we need to use the resource ID to initialize the cluster_id.

> 
> > +
> > +       /* CPU frequency scaling can ONLY be done by ARM-Trusted-Firmware
> */
> > +       arm_smccc_smc(IMX_SIP_CPUFREQ, IMX_SIP_SET_CPUFREQ,
> > +                     cluster_id, rate, 0, 0, 0, 0, &res);
> 
> Because not checking would make this work, vs. checking causes this code to
> sometimes use an uninitialized value from the stack.

 89 +       if (rsrc_id == IMX_SC_R_A35)
 90 +               init.ops = &clk_scu_cpu_ops;
 91 +       else
 92 +               init.ops = &clk_scu_ops;

I think it should be good. Only when plan to support cpu-freq scaling, then the
CPU clock will be switched to use clk_scu_cpu_ops and the clutser_id initialization
will be done according to CPU resource. For example, when we plan to support i.MX8QM
cpu-freq scaling, we will add A53 and A72 check here and switch the clock ops to clk_scu_cpu_ops,
also we will add the cluster_id initialization in the SMC clock set rate.

Thanks,
Anson.

> 
> > +
> > +       return 0;
> > +}
> > +
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH RESEND V6 2/2] clk: imx: scu: add cpu frequency scaling support

[Stephen Boyd]

Quoting Anson Huang (2019-02-22 18:32:28)
> > > +               cluster_id = 0;
> > 
> > Do we still need to check this anymore? Why not just always use cluster_id 0?
> 
> The i.MX8QXP ONLY has 1 cluster named A35, while on i.MX8QM there will be
> 2 clusters, A53 and A72, so we need to use the resource ID to initialize the cluster_id.
> 
> > 
> > > +
> > > +       /* CPU frequency scaling can ONLY be done by ARM-Trusted-Firmware
> > */
> > > +       arm_smccc_smc(IMX_SIP_CPUFREQ, IMX_SIP_SET_CPUFREQ,
> > > +                     cluster_id, rate, 0, 0, 0, 0, &res);
> > 
> > Because not checking would make this work, vs. checking causes this code to
> > sometimes use an uninitialized value from the stack.
> 
>  89 +       if (rsrc_id == IMX_SC_R_A35)
>  90 +               init.ops = &clk_scu_cpu_ops;
>  91 +       else
>  92 +               init.ops = &clk_scu_ops;
> 
> I think it should be good. Only when plan to support cpu-freq scaling, then the
> CPU clock will be switched to use clk_scu_cpu_ops and the clutser_id initialization
> will be done according to CPU resource. For example, when we plan to support i.MX8QM
> cpu-freq scaling, we will add A53 and A72 check here and switch the clock ops to clk_scu_cpu_ops,
> also we will add the cluster_id initialization in the SMC clock set rate.
> 

Ok. So then please make the set_rate function fail if the rsrc_id
doesn't match something expected. As the code is written right now, the
compiler can't figure out that cluster_id will always be assigned, so it
complains that it may be using uninitialized data.

RE: [PATCH RESEND V6 2/2] clk: imx: scu: add cpu frequency scaling support

[Stephen Boyd]

Quoting Anson Huang (2019-02-22 18:32:28)
> > > +               cluster_id = 0;
> > 
> > Do we still need to check this anymore? Why not just always use cluster_id 0?
> 
> The i.MX8QXP ONLY has 1 cluster named A35, while on i.MX8QM there will be
> 2 clusters, A53 and A72, so we need to use the resource ID to initialize the cluster_id.
> 
> > 
> > > +
> > > +       /* CPU frequency scaling can ONLY be done by ARM-Trusted-Firmware
> > */
> > > +       arm_smccc_smc(IMX_SIP_CPUFREQ, IMX_SIP_SET_CPUFREQ,
> > > +                     cluster_id, rate, 0, 0, 0, 0, &res);
> > 
> > Because not checking would make this work, vs. checking causes this code to
> > sometimes use an uninitialized value from the stack.
> 
>  89 +       if (rsrc_id == IMX_SC_R_A35)
>  90 +               init.ops = &clk_scu_cpu_ops;
>  91 +       else
>  92 +               init.ops = &clk_scu_ops;
> 
> I think it should be good. Only when plan to support cpu-freq scaling, then the
> CPU clock will be switched to use clk_scu_cpu_ops and the clutser_id initialization
> will be done according to CPU resource. For example, when we plan to support i.MX8QM
> cpu-freq scaling, we will add A53 and A72 check here and switch the clock ops to clk_scu_cpu_ops,
> also we will add the cluster_id initialization in the SMC clock set rate.
> 

Ok. So then please make the set_rate function fail if the rsrc_id
doesn't match something expected. As the code is written right now, the
compiler can't figure out that cluster_id will always be assigned, so it
complains that it may be using uninitialized data.

RE: [PATCH RESEND V6 2/2] clk: imx: scu: add cpu frequency scaling support

[Stephen Boyd]

Quoting Anson Huang (2019-02-22 18:32:28)
> > > +               cluster_id = 0;
> > 
> > Do we still need to check this anymore? Why not just always use cluster_id 0?
> 
> The i.MX8QXP ONLY has 1 cluster named A35, while on i.MX8QM there will be
> 2 clusters, A53 and A72, so we need to use the resource ID to initialize the cluster_id.
> 
> > 
> > > +
> > > +       /* CPU frequency scaling can ONLY be done by ARM-Trusted-Firmware
> > */
> > > +       arm_smccc_smc(IMX_SIP_CPUFREQ, IMX_SIP_SET_CPUFREQ,
> > > +                     cluster_id, rate, 0, 0, 0, 0, &res);
> > 
> > Because not checking would make this work, vs. checking causes this code to
> > sometimes use an uninitialized value from the stack.
> 
>  89 +       if (rsrc_id == IMX_SC_R_A35)
>  90 +               init.ops = &clk_scu_cpu_ops;
>  91 +       else
>  92 +               init.ops = &clk_scu_ops;
> 
> I think it should be good. Only when plan to support cpu-freq scaling, then the
> CPU clock will be switched to use clk_scu_cpu_ops and the clutser_id initialization
> will be done according to CPU resource. For example, when we plan to support i.MX8QM
> cpu-freq scaling, we will add A53 and A72 check here and switch the clock ops to clk_scu_cpu_ops,
> also we will add the cluster_id initialization in the SMC clock set rate.
> 

Ok. So then please make the set_rate function fail if the rsrc_id
doesn't match something expected. As the code is written right now, the
compiler can't figure out that cluster_id will always be assigned, so it
complains that it may be using uninitialized data.


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

RE: [PATCH RESEND V6 2/2] clk: imx: scu: add cpu frequency scaling support

[Anson Huang]

Hi, Stephen

Best Regards!
Anson Huang

> -----Original Message-----
> From: Stephen Boyd [mailto:sboyd@kernel.org]
> Sent: 2019年2月26日 1:22
> To: devicetree@vger.kernel.org; festevam@gmail.com;
> kernel@pengutronix.de; linux-arm-kernel@lists.infradead.org; linux-
> clk@vger.kernel.org; linux-kernel@vger.kernel.org; mark.rutland@arm.com;
> mturquette@baylibre.com; robh+dt@kernel.org; s.hauer@pengutronix.de;
> shawnguo@kernel.org; Aisheng Dong <aisheng.dong@nxp.com>; Anson
> Huang <anson.huang@nxp.com>; Daniel Baluta <daniel.baluta@nxp.com>
> Cc: dl-linux-imx <linux-imx@nxp.com>
> Subject: RE: [PATCH RESEND V6 2/2] clk: imx: scu: add cpu frequency scaling
> support
> 
> Quoting Anson Huang (2019-02-22 18:32:28)
> > > > +               cluster_id = 0;
> > >
> > > Do we still need to check this anymore? Why not just always use
> cluster_id 0?
> >
> > The i.MX8QXP ONLY has 1 cluster named A35, while on i.MX8QM there will
> > be
> > 2 clusters, A53 and A72, so we need to use the resource ID to initialize the
> cluster_id.
> >
> > >
> > > > +
> > > > +       /* CPU frequency scaling can ONLY be done by
> > > > + ARM-Trusted-Firmware
> > > */
> > > > +       arm_smccc_smc(IMX_SIP_CPUFREQ, IMX_SIP_SET_CPUFREQ,
> > > > +                     cluster_id, rate, 0, 0, 0, 0, &res);
> > >
> > > Because not checking would make this work, vs. checking causes this
> > > code to sometimes use an uninitialized value from the stack.
> >
> >  89 +       if (rsrc_id == IMX_SC_R_A35)
> >  90 +               init.ops = &clk_scu_cpu_ops;
> >  91 +       else
> >  92 +               init.ops = &clk_scu_ops;
> >
> > I think it should be good. Only when plan to support cpu-freq scaling,
> > then the CPU clock will be switched to use clk_scu_cpu_ops and the
> > clutser_id initialization will be done according to CPU resource. For
> > example, when we plan to support i.MX8QM cpu-freq scaling, we will add
> > A53 and A72 check here and switch the clock ops to clk_scu_cpu_ops, also
> we will add the cluster_id initialization in the SMC clock set rate.
> >
> 
> Ok. So then please make the set_rate function fail if the rsrc_id doesn't
> match something expected. As the code is written right now, the compiler
> can't figure out that cluster_id will always be assigned, so it complains that it
> may be using uninitialized data.

Agreed, I have sent V7 patch series to add return fail if the resource ID is NOT expected.

Thanks,
Anson.

RE: [PATCH RESEND V6 2/2] clk: imx: scu: add cpu frequency scaling support

[Anson Huang]

Hi, Stephen

Best Regards!
Anson Huang

> -----Original Message-----
> From: Stephen Boyd [mailto:sboyd@kernel.org]
> Sent: 2019年2月26日 1:22
> To: devicetree@vger.kernel.org; festevam@gmail.com;
> kernel@pengutronix.de; linux-arm-kernel@lists.infradead.org; linux-
> clk@vger.kernel.org; linux-kernel@vger.kernel.org; mark.rutland@arm.com;
> mturquette@baylibre.com; robh+dt@kernel.org; s.hauer@pengutronix.de;
> shawnguo@kernel.org; Aisheng Dong <aisheng.dong@nxp.com>; Anson
> Huang <anson.huang@nxp.com>; Daniel Baluta <daniel.baluta@nxp.com>
> Cc: dl-linux-imx <linux-imx@nxp.com>
> Subject: RE: [PATCH RESEND V6 2/2] clk: imx: scu: add cpu frequency scaling
> support
> 
> Quoting Anson Huang (2019-02-22 18:32:28)
> > > > +               cluster_id = 0;
> > >
> > > Do we still need to check this anymore? Why not just always use
> cluster_id 0?
> >
> > The i.MX8QXP ONLY has 1 cluster named A35, while on i.MX8QM there will
> > be
> > 2 clusters, A53 and A72, so we need to use the resource ID to initialize the
> cluster_id.
> >
> > >
> > > > +
> > > > +       /* CPU frequency scaling can ONLY be done by
> > > > + ARM-Trusted-Firmware
> > > */
> > > > +       arm_smccc_smc(IMX_SIP_CPUFREQ, IMX_SIP_SET_CPUFREQ,
> > > > +                     cluster_id, rate, 0, 0, 0, 0, &res);
> > >
> > > Because not checking would make this work, vs. checking causes this
> > > code to sometimes use an uninitialized value from the stack.
> >
> >  89 +       if (rsrc_id == IMX_SC_R_A35)
> >  90 +               init.ops = &clk_scu_cpu_ops;
> >  91 +       else
> >  92 +               init.ops = &clk_scu_ops;
> >
> > I think it should be good. Only when plan to support cpu-freq scaling,
> > then the CPU clock will be switched to use clk_scu_cpu_ops and the
> > clutser_id initialization will be done according to CPU resource. For
> > example, when we plan to support i.MX8QM cpu-freq scaling, we will add
> > A53 and A72 check here and switch the clock ops to clk_scu_cpu_ops, also
> we will add the cluster_id initialization in the SMC clock set rate.
> >
> 
> Ok. So then please make the set_rate function fail if the rsrc_id doesn't
> match something expected. As the code is written right now, the compiler
> can't figure out that cluster_id will always be assigned, so it complains that it
> may be using uninitialized data.

Agreed, I have sent V7 patch series to add return fail if the resource ID is NOT expected.

Thanks,
Anson.

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