On Sat, 12 Apr 2025 17:39:58 +0300
Dan Carpenter <dan.carpen...@linaro.org> wrote:
> Hello Boris Brezillon,
>
> Commit 7d5a3b22f5b5 ("drm/panthor: Call panthor_gpu_coherency_init()
> after PM resume()") from Apr 4, 2025 (linux-next), leads to the
> following Smatch static checker warning:
>
> drivers/gpu/drm/panthor/panthor_device.c:248 panthor_device_init()
> warn: missing unwind goto?
>
> drivers/gpu/drm/panthor/panthor_device.c
> 167 int panthor_device_init(struct panthor_device *ptdev)
> 168 {
> 169 u32 *dummy_page_virt;
> 170 struct resource *res;
> 171 struct page *p;
> 172 int ret;
> 173
> 174 init_completion(&ptdev->unplug.done);
> 175 ret = drmm_mutex_init(&ptdev->base, &ptdev->unplug.lock);
> 176 if (ret)
> 177 return ret;
> 178
> 179 ret = drmm_mutex_init(&ptdev->base, &ptdev->pm.mmio_lock);
> 180 if (ret)
> 181 return ret;
> 182
> 183 atomic_set(&ptdev->pm.state,
> PANTHOR_DEVICE_PM_STATE_SUSPENDED);
> 184 p = alloc_page(GFP_KERNEL | __GFP_ZERO);
> 185 if (!p)
> 186 return -ENOMEM;
> 187
> 188 ptdev->pm.dummy_latest_flush = p;
> 189 dummy_page_virt = page_address(p);
> 190 ret = drmm_add_action_or_reset(&ptdev->base,
> panthor_device_free_page,
> 191 ptdev->pm.dummy_latest_flush);
> 192 if (ret)
> 193 return ret;
> 194
> 195 /*
> 196 * Set the dummy page holding the latest flush to 1. This
> will cause the
> 197 * flush to avoided as we know it isn't necessary if the
> submission
> 198 * happens while the dummy page is mapped. Zero cannot be
> used because
> 199 * that means 'always flush'.
> 200 */
> 201 *dummy_page_virt = 1;
> 202
> 203 INIT_WORK(&ptdev->reset.work, panthor_device_reset_work);
> 204 ptdev->reset.wq = alloc_ordered_workqueue("panthor-reset-wq",
> 0);
> 205 if (!ptdev->reset.wq)
> 206 return -ENOMEM;
> 207
> 208 ret = drmm_add_action_or_reset(&ptdev->base,
> panthor_device_reset_cleanup, NULL);
> 209 if (ret)
> 210 return ret;
> 211
> 212 ret = panthor_clk_init(ptdev);
> 213 if (ret)
> 214 return ret;
> 215
> 216 ret = panthor_devfreq_init(ptdev);
> 217 if (ret)
> 218 return ret;
> 219
> 220 ptdev->iomem =
> devm_platform_get_and_ioremap_resource(to_platform_device(ptdev->base.dev),
> 221 0,
> &res);
> 222 if (IS_ERR(ptdev->iomem))
> 223 return PTR_ERR(ptdev->iomem);
> 224
> 225 ptdev->phys_addr = res->start;
> 226
> 227 ret = devm_pm_runtime_enable(ptdev->base.dev);
> 228 if (ret)
> 229 return ret;
> 230
> 231 ret = pm_runtime_resume_and_get(ptdev->base.dev);
> 232 if (ret)
> 233 return ret;
> 234
> 235 /* If PM is disabled, we need to call panthor_device_resume()
> manually. */
> 236 if (!IS_ENABLED(CONFIG_PM)) {
> 237 ret = panthor_device_resume(ptdev->base.dev);
> 238 if (ret)
> 239 return ret;
> 240 }
> 241
> 242 ret = panthor_gpu_init(ptdev);
> 243 if (ret)
> 244 goto err_rpm_put;
> 245
> 246 ret = panthor_gpu_coherency_init(ptdev);
> 247 if (ret)
> --> 248 return ret;
> ^^^^^^^^^^^
> Missing cleanup?
Thanks for the report, it should definitely be
goto err_unplug_gpu;
Do you plan to send a patch, or should I do it?
>
> 249
> 250 ret = panthor_mmu_init(ptdev);
> 251 if (ret)
> 252 goto err_unplug_gpu;
> 253
> 254 ret = panthor_fw_init(ptdev);
> 255 if (ret)
> 256 goto err_unplug_mmu;
> 257
> 258 ret = panthor_sched_init(ptdev);
> 259 if (ret)
> 260 goto err_unplug_fw;
> 261
> 262 /* ~3 frames */
> 263 pm_runtime_set_autosuspend_delay(ptdev->base.dev, 50);
> 264 pm_runtime_use_autosuspend(ptdev->base.dev);
> 265
> 266 ret = drm_dev_register(&ptdev->base, 0);
> 267 if (ret)
> 268 goto err_disable_autosuspend;
> 269
> 270 pm_runtime_put_autosuspend(ptdev->base.dev);
> 271 return 0;
> 272
> 273 err_disable_autosuspend:
> 274 pm_runtime_dont_use_autosuspend(ptdev->base.dev);
> 275 panthor_sched_unplug(ptdev);
> 276
> 277 err_unplug_fw:
> 278 panthor_fw_unplug(ptdev);
> 279
> 280 err_unplug_mmu:
> 281 panthor_mmu_unplug(ptdev);
> 282
> 283 err_unplug_gpu:
> 284 panthor_gpu_unplug(ptdev);
> 285
> 286 err_rpm_put:
> 287 pm_runtime_put_sync_suspend(ptdev->base.dev);
> 288 return ret;
> 289 }
>
> regards,
> dan carpenter
