On Thu, Aug 14, 2025 at 12:28:31AM +0530, Akhil P Oommen wrote:
> On 8/13/2025 11:18 AM, Dan Carpenter wrote:
> > On Fri, Aug 08, 2025 at 10:28:38PM +0530, Akhil P Oommen wrote:
> >> On 8/7/2025 9:23 PM, Dan Carpenter wrote:
> >>> Hello Akhil P Oommen,
> >>>
> >>> Commit b733fe7bff8b ("drm/msm/adreno: Add support for ACD") from Apr
> >>> 19, 2025 (linux-next), leads to the following Smatch static checker
> >>> warning:
> >>>
> >>> drivers/gpu/drm/msm/adreno/a6xx_gmu.c:1700 a6xx_gmu_acd_probe()
> >>> error: 'opp' dereferencing possible ERR_PTR()
> >>>
> >>> drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> >>> 1668 static int a6xx_gmu_acd_probe(struct a6xx_gmu *gmu)
> >>> 1669 {
> >>> 1670 struct a6xx_gpu *a6xx_gpu = container_of(gmu, struct
> >>> a6xx_gpu, gmu);
> >>> 1671 struct a6xx_hfi_acd_table *cmd = &gmu->acd_table;
> >>> 1672 struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
> >>> 1673 struct msm_gpu *gpu = &adreno_gpu->base;
> >>> 1674 int ret, i, cmd_idx = 0;
> >>> 1675 extern bool disable_acd;
> >>> 1676
> >>> 1677 /* Skip ACD probe if requested via module param */
> >>> 1678 if (disable_acd) {
> >>> 1679 DRM_DEV_ERROR(gmu->dev, "Skipping GPU ACD
> >>> probe\n");
> >>> 1680 return 0;
> >>> 1681 }
> >>> 1682
> >>> 1683 cmd->version = 1;
> >>> 1684 cmd->stride = 1;
> >>> 1685 cmd->enable_by_level = 0;
> >>> 1686
> >>> 1687 /* Skip freq = 0 and parse acd-level for rest of the
> >>> OPPs */
> >>> 1688 for (i = 1; i < gmu->nr_gpu_freqs; i++) {
> >>> 1689 struct dev_pm_opp *opp;
> >>> 1690 struct device_node *np;
> >>> 1691 unsigned long freq;
> >>> 1692 u32 val;
> >>> 1693
> >>> 1694 freq = gmu->gpu_freqs[i];
> >>> 1695 opp =
> >>> dev_pm_opp_find_freq_exact(&gpu->pdev->dev, freq, true);
> >>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >>> No error checking.
> >>
> >> We are passing back a freq which we pulled out from the opp_table a few
> >> lines before this. So it is unlikely that this call would fail.
> >>
> >> But it is okay to add a check here if that would make Smatch checker happy.
> >>
> >
> > No, no, just ignore it, if it can't fail.
> >
> > Or I can add dev_pm_opp_find_freq_exact() to the "no need to check" list.
> > That's easy to do.
>
> Would that make Smatch ignore usage of "dev_pm_opp_find_freq_exact()" in
> other code/drivers? If yes, we may not want that.
It just wouldn't print this warning if people left off the error handling.
I'm going to ignore it anyway, right? I recently had a case where I got
mixed up which functions needed error handling and I ignored the wrong one.
We still caught it in testing, but I'm also going through and marking which ones
to ignore or not.
regards,
dan carpenter
