Re: [bug report] cpufreq: tegra186: add OPP support and set bandwidth

[Dan Carpenter <dan.carpenter@linaro.org>]

On Fri, Oct 24, 2025 at 03:00:47AM -0500, Aaron Kling wrote:
> On Fri, Oct 24, 2025 at 1:19 AM Dan Carpenter <dan.carpenter@linaro.org> wrote:
> >
> > Hello Aaron Kling,
> >
> > Commit 71c46a6457e0 ("cpufreq: tegra186: add OPP support and set
> > bandwidth") from Oct 21, 2025 (linux-next), leads to the following
> > Smatch static checker warning:
> >
> >         drivers/cpufreq/tegra186-cpufreq.c:197 tegra186_cpufreq_init()
> >         error: uninitialized symbol 'freq_table'.
> >
> > drivers/cpufreq/tegra186-cpufreq.c
> >     174 static int tegra186_cpufreq_init(struct cpufreq_policy *policy)
> >     175 {
> >     176         struct tegra186_cpufreq_data *data = cpufreq_get_driver_data();
> >     177         unsigned int cluster = data->cpus[policy->cpu].bpmp_cluster_id;
> >     178         struct cpufreq_frequency_table *freq_table;
> >     179         struct cpufreq_frequency_table *bpmp_lut;
> >     180         u32 cpu;
> >     181         int ret;
> >     182
> >     183         policy->cpuinfo.transition_latency = 300 * 1000;
> >     184         policy->driver_data = NULL;
> >     185
> >     186         /* set same policy for all cpus in a cluster */
> >     187         for (cpu = 0; cpu < ARRAY_SIZE(tegra186_cpus); cpu++) {
> >     188                 if (data->cpus[cpu].bpmp_cluster_id == cluster)
> >     189                         cpumask_set_cpu(cpu, policy->cpus);
> >     190         }
> >     191
> >     192         bpmp_lut = data->clusters[cluster].bpmp_lut;
> >     193
> >     194         if (data->icc_dram_bw_scaling) {
> >     195                 ret = tegra_cpufreq_init_cpufreq_table(policy, bpmp_lut, &freq_table);
> >     196                 if (!ret) {
> > --> 197                         policy->freq_table = freq_table;
> >
> > tegra_cpufreq_init_cpufreq_table() will return zero if there are no
> > available opps.  The freq_table will be uninitialized in that case.
> > It's this path:
> >
> >           122          max_opps = dev_pm_opp_get_opp_count(cpu_dev);
> >           123          if (max_opps <= 0) {
> >           124                  dev_err(cpu_dev, "Failed to add OPPs\n");
> >           125                  return max_opps;
> >           126          }
> >
> >     198                         return 0;
> >     199                 }
> >     200         }
> >     201
> >     202         data->icc_dram_bw_scaling = false;
> >     203         policy->freq_table = bpmp_lut;
> >     204         pr_info("OPP tables missing from DT, EMC frequency scaling disabled\n");
> >     205
> >     206         return 0;
> >     207 }
> >
> > regards,
> > dan carpenter
> 
> I'm looking at dev_pm_opp_of_add_table_indexed which gets called
> before dev_pm_opp_get_opp_count inside
> tegra_cpufreq_init_cpufreq_table(). It states that it will return
> -ENODATA if the opp table is empty,

I don't see the comment you're referring to but the call to
dev_pm_opp_get_opp_count() which comes next would prevent an
empty table.  So it's fine.  We can leave this as-is.  Thanks for
reviewing it.

regards,
dan carpenter