public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH v2] drm/panel: tdo-tl070wsh30: Use devm_drm_panel_add()
@ 2026-04-30 14:17 박명훈
  2026-05-05  0:22 ` Claude review: " Claude Code Review Bot
  2026-05-05  0:22 ` Claude Code Review Bot
  0 siblings, 2 replies; 3+ messages in thread
From: 박명훈 @ 2026-04-30 14:17 UTC (permalink / raw)
  To: Neil Armstrong, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: Myeonghun Pak, dri-devel, linux-kernel, Ijae Kim

From: Myeonghun Pak <mhun512@gmail.com>

tdo_tl070wsh30_panel_probe() registers the panel before attaching the DSI
device. If mipi_dsi_attach() fails, the driver's remove callback is not
called and the panel remains registered in the global DRM panel registry.

Use devm_drm_panel_add() so panel registration is automatically unwound on
probe failure and device removal, and drop the explicit drm_panel_remove()
from the remove callback.

Fixes: cf40c6600592 ("drm: panel: add TDO tl070wsh30 panel driver")
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
Changes in v2:
- Drop the now-unused tdo_tl070wsh30 variable from remove.

 drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c b/drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c
index 227f97f9b..2b4ea6f0d 100644
--- a/drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c
+++ b/drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c
@@ -169,9 +169,7 @@ static int tdo_tl070wsh30_panel_add(struct tdo_tl070wsh30_panel *tdo_tl070wsh30)
 	if (err)
 		return err;
 
-	drm_panel_add(&tdo_tl070wsh30->base);
-
-	return 0;
+	return devm_drm_panel_add(dev, &tdo_tl070wsh30->base);
 }
 
 static int tdo_tl070wsh30_panel_probe(struct mipi_dsi_device *dsi)
@@ -200,14 +198,12 @@ static int tdo_tl070wsh30_panel_probe(struct mipi_dsi_device *dsi)
 
 static void tdo_tl070wsh30_panel_remove(struct mipi_dsi_device *dsi)
 {
-	struct tdo_tl070wsh30_panel *tdo_tl070wsh30 = mipi_dsi_get_drvdata(dsi);
 	int err;
 
 	err = mipi_dsi_detach(dsi);
 	if (err < 0)
 		dev_err(&dsi->dev, "failed to detach from DSI host: %d\n", err);
 
-	drm_panel_remove(&tdo_tl070wsh30->base);
 }
 
 static struct mipi_dsi_driver tdo_tl070wsh30_panel_driver = {
-- 
2.49.0

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Claude review: drm/panel: tdo-tl070wsh30: Use devm_drm_panel_add()
  2026-04-30 14:17 [PATCH v2] drm/panel: tdo-tl070wsh30: Use devm_drm_panel_add() 박명훈
  2026-05-05  0:22 ` Claude review: " Claude Code Review Bot
@ 2026-05-05  0:22 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-05-05  0:22 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm/panel: tdo-tl070wsh30: Use devm_drm_panel_add()
Author: "=?UTF-8?q?=EB=B0=95=EB=AA=85=ED=9B=88?=" <mhun512@gmail.com>
Patches: 1
Reviewed: 2026-05-05T10:22:00.617103

---

**NAK — this patch will not compile.** The patch replaces `drm_panel_add()` with `devm_drm_panel_add()`, but `devm_drm_panel_add()` does not exist in the kernel. There is no declaration in `include/drm/drm_panel.h` and no definition in `drivers/gpu/drm/drm_panel.c`. The only devm panel functions that exist are `__devm_drm_panel_alloc()` and `devm_drm_panel_add_follower()`, neither of which is a managed wrapper around `drm_panel_add()`.

The stated goal — making panel registration managed so it's automatically unwound on probe failure — is a valid improvement. However, the approach is incorrect because the required API does not exist.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Claude review: drm/panel: tdo-tl070wsh30: Use devm_drm_panel_add()
  2026-04-30 14:17 [PATCH v2] drm/panel: tdo-tl070wsh30: Use devm_drm_panel_add() 박명훈
@ 2026-05-05  0:22 ` Claude Code Review Bot
  2026-05-05  0:22 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-05-05  0:22 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Build failure — function does not exist**

The core change:
```c
-	drm_panel_add(&tdo_tl070wsh30->base);
-
-	return 0;
+	return devm_drm_panel_add(dev, &tdo_tl070wsh30->base);
```

`devm_drm_panel_add()` is not declared in `include/drm/drm_panel.h` and is not defined anywhere in `drivers/gpu/drm/drm_panel.c` or elsewhere. This will fail at compile time with an implicit function declaration error. The existing API provides:
- `drm_panel_add(struct drm_panel *)` — void, non-managed
- `drm_panel_remove(struct drm_panel *)` — the matching cleanup
- `__devm_drm_panel_alloc()` — managed *allocation*, not registration

The correct approach to achieve managed panel registration would be one of:
1. Use `devm_add_action_or_reset()` to register a callback that calls `drm_panel_remove()` on cleanup, creating an ad-hoc devm wrapper.
2. Propose a new `devm_drm_panel_add()` helper in drm_panel.c/drm_panel.h as a preparatory patch, then use it here.

**Remove-side cleanup is correct but moot**

The removal of `drm_panel_remove()` from `tdo_tl070wsh30_panel_remove()` and the now-unused local variable is the right cleanup *if* the devm registration existed, but without it this just creates a leak where the panel is never unregistered.

**Fixes tag correctness**: The identified bug (panel remains registered if `mipi_dsi_attach()` fails) is real and the Fixes tag `cf40c6600592` appears correct, but the fix itself is broken.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-05-05  0:22 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-30 14:17 [PATCH v2] drm/panel: tdo-tl070wsh30: Use devm_drm_panel_add() 박명훈
2026-05-05  0:22 ` Claude review: " Claude Code Review Bot
2026-05-05  0:22 ` Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox