public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/bridge: chipone-icn6211: use devm_mipi_dsi_attach
@ 2026-05-21 21:25 Osama Abdelkader
  2026-05-25  9:34 ` Claude review: " Claude Code Review Bot
  2026-05-25  9:34 ` Claude Code Review Bot
  0 siblings, 2 replies; 3+ messages in thread
From: Osama Abdelkader @ 2026-05-21 21:25 UTC (permalink / raw)
  To: luca.ceresoli, Jagan Teki, Andrzej Hajda, Neil Armstrong,
	Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, dri-devel, linux-kernel
  Cc: Osama Abdelkader

Follow up on the previous devm_drm_bridge_add() conversion by
converting the remaining manual cleanup in chipone-icn6211 to the
managed helper.

Suggested-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
---
 drivers/gpu/drm/bridge/chipone-icn6211.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/bridge/chipone-icn6211.c b/drivers/gpu/drm/bridge/chipone-icn6211.c
index e5957917ad88..d51d770060b6 100644
--- a/drivers/gpu/drm/bridge/chipone-icn6211.c
+++ b/drivers/gpu/drm/bridge/chipone-icn6211.c
@@ -531,7 +531,7 @@ static int chipone_dsi_attach(struct chipone *icn)
 	dsi->hs_rate = 500000000;
 	dsi->lp_rate = 16000000;
 
-	ret = mipi_dsi_attach(dsi);
+	ret = devm_mipi_dsi_attach(dev, dsi);
 	if (ret < 0)
 		dev_err(icn->dev, "failed to attach dsi\n");
 
@@ -761,11 +761,6 @@ static int chipone_i2c_probe(struct i2c_client *client)
 	return chipone_dsi_host_attach(icn);
 }
 
-static void chipone_dsi_remove(struct mipi_dsi_device *dsi)
-{
-	mipi_dsi_detach(dsi);
-}
-
 static const struct of_device_id chipone_of_match[] = {
 	{ .compatible = "chipone,icn6211", },
 	{ /* sentinel */ }
@@ -774,7 +769,6 @@ MODULE_DEVICE_TABLE(of, chipone_of_match);
 
 static struct mipi_dsi_driver chipone_dsi_driver = {
 	.probe = chipone_dsi_probe,
-	.remove = chipone_dsi_remove,
 	.driver = {
 		.name = "chipone-icn6211",
 		.of_match_table = chipone_of_match,
-- 
2.43.0


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

* Claude review: drm/bridge: chipone-icn6211: use devm_mipi_dsi_attach
  2026-05-21 21:25 [PATCH] drm/bridge: chipone-icn6211: use devm_mipi_dsi_attach Osama Abdelkader
@ 2026-05-25  9:34 ` Claude Code Review Bot
  2026-05-25  9:34 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-05-25  9:34 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm/bridge: chipone-icn6211: use devm_mipi_dsi_attach
Author: Osama Abdelkader <osama.abdelkader@gmail.com>
Patches: 1
Reviewed: 2026-05-25T19:34:34.974999

---

This is a single, clean follow-up patch that converts the chipone-icn6211 bridge driver from manual `mipi_dsi_attach`/`mipi_dsi_detach` to the managed `devm_mipi_dsi_attach` helper. The commit message correctly references a prior `devm_drm_bridge_add()` conversion, meaning the base tree already had `drm_bridge_remove()` lifted out of `chipone_dsi_remove()`, leaving only `mipi_dsi_detach()` to be converted. The conversion is correct and the patch is ready to merge (modulo applying to the correct base).

**Verdict: Looks good.** No functional issues found.

---

---
Generated by Claude Code Patch Reviewer

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

* Claude review: drm/bridge: chipone-icn6211: use devm_mipi_dsi_attach
  2026-05-21 21:25 [PATCH] drm/bridge: chipone-icn6211: use devm_mipi_dsi_attach Osama Abdelkader
  2026-05-25  9:34 ` Claude review: " Claude Code Review Bot
@ 2026-05-25  9:34 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-05-25  9:34 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Change summary:** Converts `mipi_dsi_attach(dsi)` to `devm_mipi_dsi_attach(dev, dsi)` in `chipone_dsi_attach()`, then removes the now-empty `chipone_dsi_remove()` callback and its registration in the driver struct.

**Correctness:**

The `dev` parameter passed to `devm_mipi_dsi_attach`:

```c
static int chipone_dsi_attach(struct chipone *icn)
{
	struct mipi_dsi_device *dsi = icn->dsi;
	struct device *dev = icn->dev;
	...
	ret = devm_mipi_dsi_attach(dev, dsi);
```

This function is called from two probe paths:

1. **DSI probe** (`chipone_dsi_probe`): `icn->dev` = `&dsi->dev`, so the detach action is tied to the DSI device. Correct — matches the old `chipone_dsi_remove` behavior.

2. **I2C probe** (`chipone_i2c_probe` → `chipone_dsi_host_attach` → `chipone_dsi_attach`): `icn->dev` = `&client->dev` (the I2C device). The detach action is tied to the I2C device lifecycle. This is actually an **improvement** over the current tree — the I2C driver has no `.remove` callback, so `mipi_dsi_detach` was never called on I2C device removal. With this patch, managed cleanup handles it correctly.

**Removal of `chipone_dsi_remove`:** The patch is based on a tree where the prior `devm_drm_bridge_add()` conversion already removed `drm_bridge_remove()` from this callback, leaving only `mipi_dsi_detach(dsi)`. Since that's now handled by `devm_mipi_dsi_attach`, the entire callback becomes empty and can be safely removed. The current drm-next tree still has `drm_bridge_remove()` in `chipone_dsi_remove()` at line 771, which is why the patch doesn't apply cleanly — it depends on the prior conversion landing first.

**No issues found.** The patch is straightforward and correct.

Reviewed-by: Dave Airlie <airlied@gmail.com>

---
Generated by Claude Code Patch Reviewer

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

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

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-21 21:25 [PATCH] drm/bridge: chipone-icn6211: use devm_mipi_dsi_attach Osama Abdelkader
2026-05-25  9:34 ` Claude review: " Claude Code Review Bot
2026-05-25  9:34 ` 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