public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/bridge: tc358762: Use mipi_dsi_generic_write_multi()
@ 2026-04-11  1:06 Hussain Qadri
  2026-04-11 23:13 ` Claude review: " Claude Code Review Bot
  2026-04-11 23:13 ` Claude Code Review Bot
  0 siblings, 2 replies; 3+ messages in thread
From: Hussain Qadri @ 2026-04-11  1:06 UTC (permalink / raw)
  To: andrzej.hajda, neil.armstrong, rfoss
  Cc: dri-devel, linux-kernel, Hussain Qadri

Replace the driver-local error accumulation around
mipi_dsi_generic_write() with mipi_dsi_generic_write_multi()
and struct mipi_dsi_multi_context.

Signed-off-by: Hussain Qadri <hussain.bqadri@gmail.com>
---
 drivers/gpu/drm/bridge/tc358762.c | 46 +++++++++++--------------------
 1 file changed, 16 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358762.c b/drivers/gpu/drm/bridge/tc358762.c
index 98df3e667d4a..0630c4078987 100644
--- a/drivers/gpu/drm/bridge/tc358762.c
+++ b/drivers/gpu/drm/bridge/tc358762.c
@@ -76,25 +76,12 @@ struct tc358762 {
 	struct gpio_desc *reset_gpio;
 	struct drm_display_mode mode;
 	bool pre_enabled;
-	int error;
 };
 
-static int tc358762_clear_error(struct tc358762 *ctx)
+static void tc358762_write(struct mipi_dsi_multi_context *dsi_ctx, u16 addr, u32 val)
 {
-	int ret = ctx->error;
-
-	ctx->error = 0;
-	return ret;
-}
-
-static void tc358762_write(struct tc358762 *ctx, u16 addr, u32 val)
-{
-	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
-	ssize_t ret;
 	u8 data[6];
 
-	if (ctx->error)
-		return;
 
 	data[0] = addr;
 	data[1] = addr >> 8;
@@ -103,9 +90,7 @@ static void tc358762_write(struct tc358762 *ctx, u16 addr, u32 val)
 	data[4] = val >> 16;
 	data[5] = val >> 24;
 
-	ret = mipi_dsi_generic_write(dsi, data, sizeof(data));
-	if (ret < 0)
-		ctx->error = ret;
+	mipi_dsi_generic_write_multi(dsi_ctx, data, sizeof(data));
 }
 
 static inline struct tc358762 *bridge_to_tc358762(struct drm_bridge *bridge)
@@ -115,17 +100,19 @@ static inline struct tc358762 *bridge_to_tc358762(struct drm_bridge *bridge)
 
 static int tc358762_init(struct tc358762 *ctx)
 {
+	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
+	struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
 	u32 lcdctrl;
 
-	tc358762_write(ctx, DSI_LANEENABLE,
+	tc358762_write(&dsi_ctx, DSI_LANEENABLE,
 		       LANEENABLE_L0EN | LANEENABLE_CLEN);
-	tc358762_write(ctx, PPI_D0S_CLRSIPOCOUNT, 5);
-	tc358762_write(ctx, PPI_D1S_CLRSIPOCOUNT, 5);
-	tc358762_write(ctx, PPI_D0S_ATMR, 0);
-	tc358762_write(ctx, PPI_D1S_ATMR, 0);
-	tc358762_write(ctx, PPI_LPTXTIMECNT, LPX_PERIOD);
+	tc358762_write(&dsi_ctx, PPI_D0S_CLRSIPOCOUNT, 5);
+	tc358762_write(&dsi_ctx, PPI_D1S_CLRSIPOCOUNT, 5);
+	tc358762_write(&dsi_ctx, PPI_D0S_ATMR, 0);
+	tc358762_write(&dsi_ctx, PPI_D1S_ATMR, 0);
+	tc358762_write(&dsi_ctx, PPI_LPTXTIMECNT, LPX_PERIOD);
 
-	tc358762_write(ctx, SPICMR, 0x00);
+	tc358762_write(&dsi_ctx, SPICMR, 0x00);
 
 	lcdctrl = LCDCTRL_VSDELAY(1) | LCDCTRL_RGB888 |
 		  LCDCTRL_UNK6 | LCDCTRL_VTGEN;
@@ -136,17 +123,16 @@ static int tc358762_init(struct tc358762 *ctx)
 	if (ctx->mode.flags & DRM_MODE_FLAG_NVSYNC)
 		lcdctrl |= LCDCTRL_VSPOL;
 
-	tc358762_write(ctx, LCDCTRL, lcdctrl);
+	tc358762_write(&dsi_ctx, LCDCTRL, lcdctrl);
 
-	tc358762_write(ctx, SYSCTRL, 0x040f);
+	tc358762_write(&dsi_ctx, SYSCTRL, 0x040f);
 	msleep(100);
 
-	tc358762_write(ctx, PPI_STARTPPI, PPI_START_FUNCTION);
-	tc358762_write(ctx, DSI_STARTDSI, DSI_RX_START);
+	tc358762_write(&dsi_ctx, PPI_STARTPPI, PPI_START_FUNCTION);
+	tc358762_write(&dsi_ctx, DSI_STARTDSI, DSI_RX_START);
 
 	msleep(100);
-
-	return tc358762_clear_error(ctx);
+	return dsi_ctx.accum_err;
 }
 
 static void tc358762_post_disable(struct drm_bridge *bridge,
-- 
2.53.0


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

* Claude review: drm/bridge: tc358762: Use mipi_dsi_generic_write_multi()
  2026-04-11  1:06 [PATCH] drm/bridge: tc358762: Use mipi_dsi_generic_write_multi() Hussain Qadri
@ 2026-04-11 23:13 ` Claude Code Review Bot
  2026-04-11 23:13 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-04-11 23:13 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm/bridge: tc358762: Use mipi_dsi_generic_write_multi()
Author: Hussain Qadri <hussain.bqadri@gmail.com>
Patches: 1
Reviewed: 2026-04-12T09:13:54.823193

---

This is a single-patch series that converts the tc358762 DSI-to-DPI bridge driver from a hand-rolled error accumulation pattern (`ctx->error` + `tc358762_clear_error()`) to the standard kernel `mipi_dsi_multi_context` / `mipi_dsi_generic_write_multi()` API. This is a well-understood, mechanical conversion that multiple other bridge and panel drivers have already undergone. The patch is correct and straightforward.

**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: tc358762: Use mipi_dsi_generic_write_multi()
  2026-04-11  1:06 [PATCH] drm/bridge: tc358762: Use mipi_dsi_generic_write_multi() Hussain Qadri
  2026-04-11 23:13 ` Claude review: " Claude Code Review Bot
@ 2026-04-11 23:13 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-04-11 23:13 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Correctness: Good.** The conversion is functionally equivalent to the original code:

1. The `int error` field is removed from `struct tc358762`, and `tc358762_clear_error()` is deleted. These are replaced by `struct mipi_dsi_multi_context dsi_ctx` declared locally in `tc358762_init()`, which provides the same `accum_err` accumulation semantics via the kernel API.

2. `tc358762_write()` signature changes from `(struct tc358762 *ctx, ...)` to `(struct mipi_dsi_multi_context *dsi_ctx, ...)`. The old early-return-on-error check and manual error store:
   ```c
   if (ctx->error)
       return;
   ...
   ret = mipi_dsi_generic_write(dsi, data, sizeof(data));
   if (ret < 0)
       ctx->error = ret;
   ```
   are both handled internally by `mipi_dsi_generic_write_multi()`, which checks `ctx->accum_err` before writing and stores any error automatically.

3. The final `return tc358762_clear_error(ctx)` is replaced by `return dsi_ctx.accum_err`, which is the correct way to retrieve the accumulated error from the multi-context.

4. All call sites in `tc358762_init()` are updated consistently from `tc358762_write(ctx, ...)` to `tc358762_write(&dsi_ctx, ...)`.

**Minor nit:** There is a leftover extra blank line in `tc358762_write()`:

```c
static void tc358762_write(struct mipi_dsi_multi_context *dsi_ctx, u16 addr, u32 val)
{
	u8 data[6];


	data[0] = addr;
```

There are two blank lines between the variable declaration and the first assignment. This is cosmetic-only, but one blank line would be more consistent with kernel style.

**Missing CC:** The `MAINTAINERS` file for `DRM BRIDGE ACTIVE COMPONENTS` likely lists maintainers who should be CC'd. The patch is CC'd to Andrzej Hajda, Neil Armstrong, and Robert Foss, which appear to be the bridge subsystem maintainers, so this is probably fine.

**Overall:** This is a clean, well-motivated cleanup. The commit message is clear about what it does. The patch is small, correct, and follows the established pattern used by other drivers.

---
Generated by Claude Code Patch Reviewer

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

end of thread, other threads:[~2026-04-11 23:13 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-11  1:06 [PATCH] drm/bridge: tc358762: Use mipi_dsi_generic_write_multi() Hussain Qadri
2026-04-11 23:13 ` Claude review: " Claude Code Review Bot
2026-04-11 23:13 ` 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