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.
