Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Unreachable Code / Broken Power Management due to incorrect placement of pm_runtime_enable() - [Medium] Guideline Violation: missing pm_ptr() wrapper for dev_pm_ops assignment -- --- Patch [1]: [PATCH] drm/tegra: gr3d: fix autosuspend cleanup during teardown --- commit 94bf3f52211b3cc132d2cfe856cc599ac46c8751 Author: Guangshuo Li <[email protected]> drm/tegra: gr3d: fix autosuspend cleanup during teardown This commit adds a missing pm_runtime_dont_use_autosuspend() call during driver removal. > gr3d_probe() calls pm_runtime_use_autosuspend(), but gr3d_remove() > does not call the matching pm_runtime_dont_use_autosuspend() during > teardown. [Severity: High] This is a pre-existing issue, but does the runtime PM initialization actually work as described? It appears pm_runtime_enable() and autosuspend configurations are placed in gr3d_runtime_resume() rather than gr3d_probe(): drivers/gpu/drm/tegra/gr3d.c:gr3d_runtime_resume() { ... pm_runtime_enable(dev); pm_runtime_use_autosuspend(dev); pm_runtime_set_autosuspend_delay(dev, 500); ... } Since the PM core initializes devices with runtime PM disabled, won't this callback never be invoked, leaving the device permanently in RPM_SUSPENDED from the PM core's perspective? Should these initialization calls be moved to gr3d_probe()? > diff --git a/drivers/gpu/drm/tegra/gr3d.c b/drivers/gpu/drm/tegra/gr3d.c > index 42e9656ab80c9..518bd21f52e2d 100644 > --- a/drivers/gpu/drm/tegra/gr3d.c > +++ b/drivers/gpu/drm/tegra/gr3d.c > @@ -524,6 +524,7 @@ static void gr3d_remove(struct platform_device *pdev) > { > struct gr3d *gr3d = platform_get_drvdata(pdev); > > + pm_runtime_dont_use_autosuspend(&pdev->dev); > pm_runtime_disable(&pdev->dev); > host1x_client_unregister(&gr3d->client.base); > } [Severity: Medium] This is a pre-existing issue, but when CONFIG_PM is disabled, does unconditionally assigning the dev_pm_ops structure create dead code? drivers/gpu/drm/tegra/gr3d.c:tegra_gr3d_driver { ... .pm = &tegra_gr3d_pm, ... } Should this use the pm_ptr() wrapper like .pm = pm_ptr(&tegra_gr3d_pm) to ensure the callback functions do not get linked into the kernel when they can never execute? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
