Hi, Stephen Best Regards! Anson Huang > -----Original Message----- > From: Stephen Boyd [mailto:sboyd@xxxxxxxxxx] > Sent: 2019年2月26日 1:22 > To: devicetree@xxxxxxxxxxxxxxx; festevam@xxxxxxxxx; > kernel@xxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux- > clk@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; mark.rutland@xxxxxxx; > mturquette@xxxxxxxxxxxx; robh+dt@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx; > shawnguo@xxxxxxxxxx; Aisheng Dong <aisheng.dong@xxxxxxx>; Anson > Huang <anson.huang@xxxxxxx>; Daniel Baluta <daniel.baluta@xxxxxxx> > Cc: dl-linux-imx <linux-imx@xxxxxxx> > 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.
