Hi, Stephen Best Regards! Anson Huang
> -----Original Message----- > From: Stephen Boyd [mailto:sb...@kernel.org] > Sent: 2019年2月26日 1:22 > To: devicet...@vger.kernel.org; feste...@gmail.com; > ker...@pengutronix.de; linux-arm-ker...@lists.infradead.org; linux- > c...@vger.kernel.org; linux-kernel@vger.kernel.org; mark.rutl...@arm.com; > mturque...@baylibre.com; robh...@kernel.org; s.ha...@pengutronix.de; > shawn...@kernel.org; Aisheng Dong <aisheng.d...@nxp.com>; Anson > Huang <anson.hu...@nxp.com>; Daniel Baluta <daniel.bal...@nxp.com> > Cc: dl-linux-imx <linux-...@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.
