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

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 [off-list ref]; Anson
Huang [off-list ref]; Daniel Baluta [off-list ref]
Cc: dl-linux-imx <redacted>
Subject: RE: [PATCH RESEND V6 2/2] clk: imx: scu: add cpu frequency scaling
support

Quoting Anson Huang (2019-02-22 18:32:28)

quoted

quoted

quoted

+               cluster_id = 0;

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

cluster_id 0?

quoted

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.

quoted

quoted

quoted

+
+       /* CPU frequency scaling can ONLY be done by
+ ARM-Trusted-Firmware

*/

quoted

+       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.

quoted

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