On Thu, Aug 14, 2025 at 12:06 AM Dan Carpenter <[email protected]> wrote: > > 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.
drive-by comment: Would it be useful to have a comment that smatch could look for in cases like this.. similar to how rust has a practice of adding a comment describing unsafe blocks? It could be a useful way to document "safe because: this isn't expected to fail" cases, both for humans and tools. BR, -R > regards, > dan carpenter >
