* [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 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 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
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