Claude review: drm/panel: novatek-nt36672a: Convert to mipi_dsi_*_multi() helpers

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**nt36672a_send_cmds conversion**

> +static void nt36672a_send_cmds(struct mipi_dsi_multi_context *dsi_ctx,
> +				const struct nt36672a_panel_cmd *cmds, int num)

The function signature conversion from returning `int` to `void` and taking `mipi_dsi_multi_context` is correct. The replacement of `mipi_dsi_dcs_write(pinfo->link, cmd->data[0], cmd->data + 1, 1)` with `mipi_dsi_dcs_write_buffer_multi(dsi_ctx, cmd->data, sizeof(cmd->data))` is semantically equivalent -- `mipi_dsi_dcs_write_buffer` treats `data[0]` as the DCS command byte, same as the explicit split in the old call.

Minor alignment nit already noted by Doug: the `const` on the second line doesn't align with the open parenthesis.

**unprepare() sequencing bug**

> +	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
> +	/* Reset error to continue power-down even if display off failed */
> +	dsi_ctx.accum_err = 0;
> +	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
> +
> +	/* 120ms delay required here as per DCS spec */
> +	msleep(120);

The original sequence was: set_display_off, 120ms delay, enter_sleep_mode, 60ms delay. The new code sends enter_sleep_mode immediately after set_display_off (with only the accum_err reset between them), and both msleep calls come afterward. This changes the power-down timing in a way that contradicts the comment about the DCS-spec-required 120ms delay. The 120ms gap was between set_display_off and enter_sleep_mode; now both commands are sent back-to-back before either delay.

Additionally, the `accum_err = 0` reset is placed after `set_display_off_multi` but before `enter_sleep_mode_multi`. This means if `nt36672a_send_cmds` (the off-commands) sets an error, `set_display_off_multi` will be a no-op because it checks `accum_err` at entry. In the original code, `set_display_off` was always attempted regardless of whether the off-commands failed. To preserve the original best-effort behavior, the `accum_err` should be reset before `set_display_off_multi` as well, or each call should reset independently.

**unprepare() return value / power_off**

> +	nt36672a_panel_power_off(panel);
> +
> +	return 0;

The return value of `nt36672a_panel_power_off()` is now discarded, and unprepare unconditionally returns 0. This is a reasonable change for a shutdown path, but as Doug noted, `nt36672a_panel_power_off()` should then be converted to return `void` to make it clear the error is intentionally ignored.

**prepare() error handling**

> +	err = nt36672a_panel_power_on(pinfo);
> +	if (err < 0)
> +		return err;

The old code went to `goto poweroff` which called `gpiod_set_value(pinfo->reset_gpio, 0)`. The new code returns directly. This is harmless in practice: `nt36672a_panel_power_on()` can only fail at `regulator_bulk_enable()`, before the GPIO is ever toggled, so the GPIO is still in its initial low state at that point.

> +	err = dsi_ctx.accum_err;
> +	if (err < 0)
> +		gpiod_set_value(pinfo->reset_gpio, 0);
> +
> +	return err;

This preserves the error-path GPIO reset for DSI command failures, matching the original behavior. However, neither the old nor new code disables the regulators on a DSI-command failure in prepare(), leaving them enabled. That's a pre-existing issue, not introduced by this patch.

As Doug suggested, using `dsi_ctx.accum_err` to store the `nt36672a_panel_power_on()` result (and letting the `_multi` functions no-op on error) would simplify this function and eliminate the separate `err` variable, reducing the risk of the kind of bug fixed in commit 61ce50fd8196.

---
Generated by Claude Code Patch Reviewer