Messages in this thread | | | Date | Thu, 21 Feb 2019 09:15:34 +0530 | From | Viresh Kumar <> | Subject | Re: [PATCH] cpufreq: kyro: Reduce frame-size of qcom_cpufreq_kryo_probe() |
| |
On 20-02-19, 21:56, Amit Kucheria wrote: > On Wed, Feb 20, 2019 at 4:44 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > With the introduction of commit 846a415bf440 ("arm64: default NR_CPUS to > > 256"), we have started getting following compilation warning: > > > > qcom-cpufreq-kryo.c:168:1: warning: the frame size of 2160 bytes is larger than 2048 bytes [-Wframe-larger-than=] > > > > Fix that by dynamically allocating opp_tables and freeing it later. > > > > Compile tested only. > > > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > --- > > drivers/cpufreq/qcom-cpufreq-kryo.c | 9 ++++++++- > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/cpufreq/qcom-cpufreq-kryo.c b/drivers/cpufreq/qcom-cpufreq-kryo.c > > index 1c8583cc06a2..6888cb6db2ef 100644 > > --- a/drivers/cpufreq/qcom-cpufreq-kryo.c > > +++ b/drivers/cpufreq/qcom-cpufreq-kryo.c > > @@ -75,7 +75,7 @@ static enum _msm8996_version qcom_cpufreq_kryo_get_msm_id(void) > > > > static int qcom_cpufreq_kryo_probe(struct platform_device *pdev) > > { > > - struct opp_table *opp_tables[NR_CPUS] = {0}; > > + struct opp_table **opp_tables; > > enum _msm8996_version msm8996_version; > > struct nvmem_cell *speedbin_nvmem; > > struct device_node *np; > > @@ -133,6 +133,10 @@ static int qcom_cpufreq_kryo_probe(struct platform_device *pdev) > > } > > kfree(speedbin); > > > > + opp_tables = kcalloc(num_possible_cpus(), sizeof(*opp_tables), GFP_KERNEL); > > + if (!opp_tables) > > + return -ENOMEM; > > + > > Perhaps add a comment above that that actual opp_table is allocated in > the loop below because of dev_pm_opp_set_supported_hw? > > I was staring at this for a few minutes wondering why you needed this > kcalloc before I realised that opp_tables (missed the 's') is a > temporary array of pointers. :-)
I feel that you got confused because this patch didn't had the diff where the opp_tables thing is getting used. When we see the .c file itself, it is pretty much clear on what is going on and I believe the comment would be totally unnecessary and redundant.
This is how it looks now, please lemme know if you still prefer the comment :)
opp_tables = kcalloc(num_possible_cpus(), sizeof(*opp_tables), GFP_KERNEL); if (!opp_tables) return -ENOMEM;
for_each_possible_cpu(cpu) { cpu_dev = get_cpu_device(cpu); if (NULL == cpu_dev) { ret = -ENODEV; goto free_opp; }
opp_tables[cpu] = dev_pm_opp_set_supported_hw(cpu_dev, &versions, 1); if (IS_ERR(opp_tables[cpu])) { ret = PTR_ERR(opp_tables[cpu]); dev_err(cpu_dev, "Failed to set supported hardware\n"); goto free_opp; } }
kfree(opp_tables);
-- viresh
| |