public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 0/2] drm/panel: use devm_drm_panel_add() in more drivers
@ 2026-05-24 16:36 Myeonghun Pak
  2026-05-24 16:36 ` [PATCH 1/2] drm/panel: boe-bf060y8m-aj0: use devm_drm_panel_add() Myeonghun Pak
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Myeonghun Pak @ 2026-05-24 16:36 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Jianhua Lu, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel,
	linux-kernel, Myeonghun Pak

Convert two panel drivers to devm_drm_panel_add() so panel registration
is cleaned up by devres on later probe failures.

Both drivers currently call drm_panel_add() before attaching one or more
DSI devices. If an attach fails, probe returns with the panel still in
the global panel registry. drm-misc-next has devm_drm_panel_add(), which
wraps drm_panel_add() with devres-managed drm_panel_remove() cleanup.

Patch 1 updates boe-bf060y8m-aj0.
Patch 2 updates novatek-nt36523.

Myeonghun Pak (2):
  drm/panel: boe-bf060y8m-aj0: use devm_drm_panel_add()
  drm/panel: novatek-nt36523: use devm_drm_panel_add()

 drivers/gpu/drm/panel/panel-boe-bf060y8m-aj0.c |  6 +++---
 drivers/gpu/drm/panel/panel-novatek-nt36523.c  | 12 +++---------
 2 files changed, 6 insertions(+), 12 deletions(-)

-- 
2.47.1

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

* [PATCH 1/2] drm/panel: boe-bf060y8m-aj0: use devm_drm_panel_add()
  2026-05-24 16:36 [PATCH 0/2] drm/panel: use devm_drm_panel_add() in more drivers Myeonghun Pak
@ 2026-05-24 16:36 ` Myeonghun Pak
  2026-05-25  6:58   ` Claude review: " Claude Code Review Bot
  2026-05-24 16:36 ` [PATCH 2/2] drm/panel: novatek-nt36523: " Myeonghun Pak
  2026-05-25  6:58 ` Claude review: drm/panel: use devm_drm_panel_add() in more drivers Claude Code Review Bot
  2 siblings, 1 reply; 6+ messages in thread
From: Myeonghun Pak @ 2026-05-24 16:36 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Jianhua Lu, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel,
	linux-kernel, Myeonghun Pak, stable, Ijae Kim

boe_bf060y8m_aj0_probe() adds the panel before attaching the DSI
device. If mipi_dsi_attach() fails, probe returns with the panel still
registered.

drm-misc-next has devm_drm_panel_add(), so use it to register the panel
with devres-managed cleanup. This removes the need for open-coded
drm_panel_remove() handling on later probe failures and on the remove
path.

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

Fixes: a19125a28112 ("drm/panel: Add BOE BF060Y8M-AJ0 5.99" AMOLED panel driver")
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/panel/panel-boe-bf060y8m-aj0.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-boe-bf060y8m-aj0.c b/drivers/gpu/drm/panel/panel-boe-bf060y8m-aj0.c
index 84c21c62a4..a6d765b402 100644
--- a/drivers/gpu/drm/panel/panel-boe-bf060y8m-aj0.c
+++ b/drivers/gpu/drm/panel/panel-boe-bf060y8m-aj0.c
@@ -357,7 +357,9 @@ static int boe_bf060y8m_aj0_probe(struct mipi_dsi_device *dsi)
 		return dev_err_probe(dev, PTR_ERR(boe->panel.backlight),
 				     "Failed to create backlight\n");
 
-	drm_panel_add(&boe->panel);
+	ret = devm_drm_panel_add(dev, &boe->panel);
+	if (ret)
+		return ret;
 
 	ret = mipi_dsi_attach(dsi);
 	if (ret < 0) {
@@ -376,8 +378,6 @@ static void boe_bf060y8m_aj0_remove(struct mipi_dsi_device *dsi)
 	ret = mipi_dsi_detach(dsi);
 	if (ret < 0)
 		dev_err(&dsi->dev, "Failed to detach from DSI host: %d\n", ret);
-
-	drm_panel_remove(&boe->panel);
 }
 
 static const struct of_device_id boe_bf060y8m_aj0_of_match[] = {
-- 
2.47.1


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

* [PATCH 2/2] drm/panel: novatek-nt36523: use devm_drm_panel_add()
  2026-05-24 16:36 [PATCH 0/2] drm/panel: use devm_drm_panel_add() in more drivers Myeonghun Pak
  2026-05-24 16:36 ` [PATCH 1/2] drm/panel: boe-bf060y8m-aj0: use devm_drm_panel_add() Myeonghun Pak
@ 2026-05-24 16:36 ` Myeonghun Pak
  2026-05-25  6:58   ` Claude review: " Claude Code Review Bot
  2026-05-25  6:58 ` Claude review: drm/panel: use devm_drm_panel_add() in more drivers Claude Code Review Bot
  2 siblings, 1 reply; 6+ messages in thread
From: Myeonghun Pak @ 2026-05-24 16:36 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Jianhua Lu, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel,
	linux-kernel, Myeonghun Pak, stable, Ijae Kim

nt36523_probe() adds the DRM panel before attaching the DSI devices. If
one of the devm_mipi_dsi_attach() calls fails, probe returns with the
panel still registered.

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

Fixes: 0993234a0045 ("drm/panel: Add driver for Novatek NT36523")
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/panel/panel-novatek-nt36523.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-novatek-nt36523.c b/drivers/gpu/drm/panel/panel-novatek-nt36523.c
index 226d91daf8..f6592b01df 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt36523.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt36523.c
@@ -1047,13 +1047,6 @@ static int nt36523_unprepare(struct drm_panel *panel)
 	return 0;
 }
 
-static void nt36523_remove(struct mipi_dsi_device *dsi)
-{
-	struct panel_info *pinfo = mipi_dsi_get_drvdata(dsi);
-
-	drm_panel_remove(&pinfo->panel);
-}
-
 static int nt36523_get_modes(struct drm_panel *panel,
 			       struct drm_connector *connector)
 {
@@ -1225,7 +1218,9 @@ static int nt36523_probe(struct mipi_dsi_device *dsi)
 			return dev_err_probe(dev, ret, "Failed to get backlight\n");
 	}
 
-	drm_panel_add(&pinfo->panel);
+	ret = devm_drm_panel_add(dev, &pinfo->panel);
+	if (ret)
+		return ret;
 
 	for (i = 0; i < DSI_NUM_MIN + pinfo->desc->is_dual_dsi; i++) {
 		pinfo->dsi[i]->lanes = pinfo->desc->lanes;
@@ -1259,7 +1254,6 @@ MODULE_DEVICE_TABLE(of, nt36523_of_match);
 
 static struct mipi_dsi_driver nt36523_driver = {
 	.probe = nt36523_probe,
-	.remove = nt36523_remove,
 	.driver = {
 		.name = "panel-novatek-nt36523",
 		.of_match_table = nt36523_of_match,
-- 
2.47.1


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

* Claude review: drm/panel: use devm_drm_panel_add() in more drivers
  2026-05-24 16:36 [PATCH 0/2] drm/panel: use devm_drm_panel_add() in more drivers Myeonghun Pak
  2026-05-24 16:36 ` [PATCH 1/2] drm/panel: boe-bf060y8m-aj0: use devm_drm_panel_add() Myeonghun Pak
  2026-05-24 16:36 ` [PATCH 2/2] drm/panel: novatek-nt36523: " Myeonghun Pak
@ 2026-05-25  6:58 ` Claude Code Review Bot
  2 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-05-25  6:58 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm/panel: use devm_drm_panel_add() in more drivers
Author: Myeonghun Pak <mhun512@gmail.com>
Patches: 3
Reviewed: 2026-05-25T16:58:16.775361

---

This is a clean, straightforward 2-patch series converting two panel drivers from `drm_panel_add()` to `devm_drm_panel_add()`. The motivation is sound: both drivers had a bug where `drm_panel_add()` was called before `mipi_dsi_attach()`, and if the attach failed, probe returned with the panel still registered in the global panel list — a resource leak. Using `devm_drm_panel_add()` ensures the panel is automatically unregistered via devres on any subsequent probe failure or on driver removal.

The patches are correct, minimal, and well-described. The `Fixes:` tags and `Cc: stable` are appropriate since these are real resource leak bugs. The devres ordering is correct in both drivers.

**Verdict: Both patches look good to merge.**

---

---
Generated by Claude Code Patch Reviewer

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

* Claude review: drm/panel: boe-bf060y8m-aj0: use devm_drm_panel_add()
  2026-05-24 16:36 ` [PATCH 1/2] drm/panel: boe-bf060y8m-aj0: use devm_drm_panel_add() Myeonghun Pak
@ 2026-05-25  6:58   ` Claude Code Review Bot
  0 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-05-25  6:58 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Status: Good**

The change replaces:
```c
drm_panel_add(&boe->panel);
```
with:
```c
ret = devm_drm_panel_add(dev, &boe->panel);
if (ret)
    return ret;
```
and removes `drm_panel_remove(&boe->panel)` from the `remove` callback.

This is correct. The `remove` callback is retained because this driver uses manual `mipi_dsi_attach()` (not `devm_mipi_dsi_attach()`), so it still needs its `mipi_dsi_detach()` call in `boe_bf060y8m_aj0_remove()`. The devres-managed panel removal will happen automatically after `remove()` returns.

**One minor observation**: The `boe_bf060y8m_aj0_remove()` callback now only does `mipi_dsi_detach()` with error reporting. This is fine as-is, but a potential follow-up could convert to `devm_mipi_dsi_attach()` and eliminate the `remove` callback entirely — though that's out of scope for this fix.

The error handling for `devm_drm_panel_add()` failure (returning the error code) is correct. No other cleanup is needed since all prior resource acquisitions in the probe function already use devres (`devm_gpiod_get`, `devm_drm_panel_alloc`, etc.).

---
Generated by Claude Code Patch Reviewer

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

* Claude review: drm/panel: novatek-nt36523: use devm_drm_panel_add()
  2026-05-24 16:36 ` [PATCH 2/2] drm/panel: novatek-nt36523: " Myeonghun Pak
@ 2026-05-25  6:58   ` Claude Code Review Bot
  0 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-05-25  6:58 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Status: Good**

The change replaces:
```c
drm_panel_add(&pinfo->panel);
```
with:
```c
ret = devm_drm_panel_add(dev, &pinfo->panel);
if (ret)
    return ret;
```

It also removes the entire `nt36523_remove()` callback and its `.remove` assignment in the driver struct. This is correct because `nt36523_probe()` uses `devm_mipi_dsi_attach()` for DSI attachment, so both DSI detach and panel removal are fully managed by devres. There is no remaining manual cleanup, so the remove callback is no longer needed.

The devres ordering is correct: `devm_drm_panel_add()` is registered before `devm_mipi_dsi_attach()`, so during teardown the DSI devices will be detached before the panel is removed from the registry. This is the right order — consumers (DSI hosts) should be disconnected before the panel disappears.

The `Fixes:` tag correctly points to commit `0993234a0045` which introduced the driver with the original `drm_panel_add()` call.

---
Generated by Claude Code Patch Reviewer

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

end of thread, other threads:[~2026-05-25  6:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-24 16:36 [PATCH 0/2] drm/panel: use devm_drm_panel_add() in more drivers Myeonghun Pak
2026-05-24 16:36 ` [PATCH 1/2] drm/panel: boe-bf060y8m-aj0: use devm_drm_panel_add() Myeonghun Pak
2026-05-25  6:58   ` Claude review: " Claude Code Review Bot
2026-05-24 16:36 ` [PATCH 2/2] drm/panel: novatek-nt36523: " Myeonghun Pak
2026-05-25  6:58   ` Claude review: " Claude Code Review Bot
2026-05-25  6:58 ` Claude review: drm/panel: use devm_drm_panel_add() in more drivers 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