[PATCH] drm/gma500: clean up modeset on backlight init failure

[PATCH] drm/gma500: clean up modeset on backlight init failure

[Myeonghun Pak]

psb_driver_load() initializes KMS polling before it attempts to
initialize backlight support. If gma_backlight_init() fails, the
function returns directly and skips psb_driver_unload(), leaving
drm_kms_helper_poll_fini() uncalled.

Use the existing error path so the partially initialized modeset state
is unwound before probe fails.

This issue was identified during our ongoing static-analysis research while
reviewing kernel code.

Fixes: 1f90b1232773 ("drm/gma500: Refactor backlight support (v2)")
Cc: stable@vger.kernel.org
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
 drivers/gpu/drm/gma500/psb_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index 005ab7f535..7218026fe2 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -406,7 +406,7 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags)
 	drm_connector_list_iter_end(&conn_iter);
 
 	if (ret)
-		return ret;
+		goto out_err;
 	psb_intel_opregion_enable_asle(dev);
 
 	return devm_add_action_or_reset(dev->dev, psb_device_release, dev);
-- 
2.47.1

Claude review: drm/gma500: clean up modeset on backlight init failure

[Claude Code Review Bot]

Overall Series Review

Subject: drm/gma500: clean up modeset on backlight init failure
Author: Myeonghun Pak <mhun512@gmail.com>
Patches: 1
Reviewed: 2026-05-25T17:02:55.523547

---

This is a single-patch fix for a real resource-leak bug in the GMA500 DRM driver's probe path. The analysis is correct: when `gma_backlight_init()` fails, the original code did `return ret` directly, skipping the `out_err` label and therefore skipping `psb_driver_unload()`. This leaves `drm_kms_helper_poll_fini()` (via `psb_modeset_cleanup()`) uncalled, along with other teardown like `gma_irq_uninstall()` and MMU/GTT cleanup.

The fix is a one-line, obviously correct change from `return ret` to `goto out_err`. The patch is well-formed, has appropriate tags (Fixes, Cc: stable, Co-developed-by, Signed-off-by), and the commit message accurately describes the problem.

**Verdict: Looks good.** One minor observation and one pre-existing issue noted below.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/gma500: clean up modeset on backlight init failure

[Claude Code Review Bot]

Patch Review

**The fix itself is correct and straightforward.**

At line 407-408 (post-patch):
```c
	if (ret)
		goto out_err;
```

This replaces the previous `return ret`, ensuring that `psb_driver_unload()` at the `out_err:` label (line 413-415) is called, which properly tears down the modeset, IRQs, MMU, etc.

**Minor observation on `ret` value scope:** The variable `ret` is set inside the connector loop only if an LVDS or MIPI connector is found and `gma_backlight_init()` is called. If no such connector exists, `ret` still holds its value from the last assignment before the loop. Looking at the code path, the last assignment to `ret` before the connector loop is at line 367:
```c
	ret = drm_vblank_init(dev, dev_priv->num_pipe);
	if (ret)
		goto out_err;
```

If `drm_vblank_init()` succeeded, `ret == 0`, so the `if (ret)` check at line 407 correctly falls through to the success path. This is fine — no issue here, just confirming the logic holds.

**Pre-existing issue (out of scope for this patch):** At line 348-350, there's another bare `return ret` that also skips `out_err`:
```c
	ret = psb_do_init(dev);
	if (ret)
		return ret;
```

This has the same class of bug — resources initialized before `psb_do_init()` (MMU page directory, scratch page, GTT, GEM) won't be cleaned up. The submitter might consider a follow-up patch for this.

**Tags review:** The Fixes tag references commit `1f90b1773` ("drm/gma500: Refactor backlight support (v2)"), which is the commit that introduced the backlight init in this location. The Cc: stable tag is appropriate since this is a real resource leak fix.

**Recommendation:** Apply as-is. Optionally suggest the author send a follow-up for the `psb_do_init()` bare return at line 349.

---
Generated by Claude Code Patch Reviewer