* [PATCH v2] drm/bridge: tc358762: Use mipi_dsi_generic_write_multi()
@ 2026-05-02 3:10 Hussain Qadri
2026-05-04 22:59 ` Claude review: " Claude Code Review Bot
2026-05-04 22:59 ` Claude Code Review Bot
0 siblings, 2 replies; 5+ messages in thread
From: Hussain Qadri @ 2026-05-02 3:10 UTC (permalink / raw)
To: andrzej.hajda, neil.armstrong, rfoss
Cc: Laurent.pinchart, jonas, jernej.skrabec, maarten.lankhorst,
mripard, tzimmermann, airlied, simona, 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>
---
Changes in v2:
- Remove extra blank line in tc358762_write().
- Rebase onto current master.
drivers/gpu/drm/bridge/tc358762.c | 47 +++++++++++--------------------
1 file changed, 16 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358762.c b/drivers/gpu/drm/bridge/tc358762.c
index 98df3e667d4a..739283fc51db 100644
--- a/drivers/gpu/drm/bridge/tc358762.c
+++ b/drivers/gpu/drm/bridge/tc358762.c
@@ -76,26 +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;
data[2] = val;
@@ -103,9 +89,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 +99,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 +122,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.54.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Claude review: drm/bridge: tc358762: Use mipi_dsi_generic_write_multi()
2026-05-02 3:10 [PATCH v2] drm/bridge: tc358762: Use mipi_dsi_generic_write_multi() Hussain Qadri
@ 2026-05-04 22:59 ` Claude Code Review Bot
2026-05-04 22:59 ` Claude Code Review Bot
1 sibling, 0 replies; 5+ messages in thread
From: Claude Code Review Bot @ 2026-05-04 22:59 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-05-05T08:59:34.716883
---
This is a single-patch series that converts the tc358762 DSI/DPI bridge driver from a hand-rolled error accumulation pattern (using `ctx->error` and `tc358762_clear_error()`) to the standard `mipi_dsi_multi_context` / `mipi_dsi_generic_write_multi()` framework provided by the DRM subsystem.
The conversion is straightforward, correct, and follows established patterns used across numerous other DSI drivers. It reduces code by ~15 lines, removes the `error` field from the driver struct, and eliminates two helper functions (`tc358762_clear_error` and the old error-checking logic in `tc358762_write`). The `mipi_dsi_generic_write_multi()` API already handles error accumulation internally via `accum_err`, providing identical semantics.
**Verdict: Looks good.** This is a clean mechanical conversion with no functional change in behavior.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 5+ messages in thread
* Claude review: drm/bridge: tc358762: Use mipi_dsi_generic_write_multi()
2026-05-02 3:10 [PATCH v2] drm/bridge: tc358762: Use mipi_dsi_generic_write_multi() Hussain Qadri
2026-05-04 22:59 ` Claude review: " Claude Code Review Bot
@ 2026-05-04 22:59 ` Claude Code Review Bot
1 sibling, 0 replies; 5+ messages in thread
From: Claude Code Review Bot @ 2026-05-04 22:59 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Correctness:**
The conversion is correct. The old pattern:
```c
static void tc358762_write(struct tc358762 *ctx, u16 addr, u32 val)
{
...
if (ctx->error)
return;
...
ret = mipi_dsi_generic_write(dsi, data, sizeof(data));
if (ret < 0)
ctx->error = ret;
}
```
is functionally equivalent to the new pattern:
```c
static void tc358762_write(struct mipi_dsi_multi_context *dsi_ctx, u16 addr, u32 val)
{
...
mipi_dsi_generic_write_multi(dsi_ctx, data, sizeof(data));
}
```
because `mipi_dsi_generic_write_multi()` internally checks and sets `ctx->accum_err`, providing the same early-return-on-error and error-capture semantics.
The removal of the `int error` field from `struct tc358762` is correct — no other code references it outside the removed functions.
The return value change from `tc358762_clear_error(ctx)` to `dsi_ctx.accum_err` at the end of `tc358762_init()` is correct. The old `tc358762_clear_error()` returned the accumulated error and reset it to zero; since `dsi_ctx` is a local variable that goes out of scope immediately after the return, clearing is unnecessary.
The `struct mipi_dsi_device *dsi` lookup moved from `tc358762_write()` into `tc358762_init()` where it's used to initialize `dsi_ctx` — this is appropriate since the multi-context carries the device pointer.
**Style:** The removal of the extra blank line before the final `return` (noted in the v2 changelog) is correct; the result matches typical kernel style.
**No issues found.**
Reviewed-by worthy as-is.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 5+ messages in thread
* [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; 5+ 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] 5+ messages in thread* Claude review: drm/bridge: tc358762: Use mipi_dsi_generic_write_multi()
2026-04-11 1:06 [PATCH] " Hussain Qadri
@ 2026-04-11 23:13 ` Claude Code Review Bot
2026-04-11 23:13 ` Claude Code Review Bot
1 sibling, 0 replies; 5+ 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] 5+ messages in thread
* Claude review: drm/bridge: tc358762: Use mipi_dsi_generic_write_multi()
2026-04-11 1:06 [PATCH] " 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; 5+ 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] 5+ messages in thread
end of thread, other threads:[~2026-05-04 22:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-02 3:10 [PATCH v2] drm/bridge: tc358762: Use mipi_dsi_generic_write_multi() Hussain Qadri
2026-05-04 22:59 ` Claude review: " Claude Code Review Bot
2026-05-04 22:59 ` Claude Code Review Bot
-- strict thread matches above, loose matches on Subject: below --
2026-04-11 1:06 [PATCH] " 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