* [bug report] cpufreq: tegra186: add OPP support and set bandwidth
@ 2025-10-24 6:19 Dan Carpenter
2025-10-24 8:00 ` Aaron Kling
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2025-10-24 6:19 UTC (permalink / raw)
To: Aaron Kling; +Cc: linux-tegra
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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [bug report] cpufreq: tegra186: add OPP support and set bandwidth
2025-10-24 6:19 [bug report] cpufreq: tegra186: add OPP support and set bandwidth Dan Carpenter
@ 2025-10-24 8:00 ` Aaron Kling
2025-10-27 8:44 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Aaron Kling @ 2025-10-24 8:00 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-tegra
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, which would then return a negative
value from tegra_cpufreq_init_cpufreq_table(), which gets handled as
expected. Is there a way for opp count to be zero that would get past
this? The code could be better, but I'm not sure there's an operable
fail case here.
Also of note, the same effective flow exists in the tegra194 cpufreq
driver, so if something does need to be fixed here, it should probably
be applied there at the same time.
Sincerely,
Aaron
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [bug report] cpufreq: tegra186: add OPP support and set bandwidth
2025-10-24 8:00 ` Aaron Kling
@ 2025-10-27 8:44 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2025-10-27 8:44 UTC (permalink / raw)
To: Aaron Kling; +Cc: linux-tegra
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-10-27 8:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-24 6:19 [bug report] cpufreq: tegra186: add OPP support and set bandwidth Dan Carpenter
2025-10-24 8:00 ` Aaron Kling
2025-10-27 8:44 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).