On Mon, Aug 10, 2026 at 11:57:06AM +0200, Neil Armstrong wrote:
> > + struct mipi_dsi_multi_context dsi_ctx = { .dsi =
> > to_mipi_dsi_device(s6->dev) };
>
> Ok why not keeping the original:
>
> struct mipi_dsi_device *dsi = to_mipi_dsi_device(s6->dev);
> struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
>
>
> It's much easier to read, for me at least, but I won't block it...
>
> > /* Enter sleep mode */
> > - ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
> > - if (ret) {
> > - dev_err(s6->dev, "failed to enter sleep mode (%d)\n", ret);
> > - return ret;
> > - }
> > + mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
>
> Add a comment you ignore the the dsi_ctx.accum_err and it's fine
> You could write:
>
> if (dsi_ctx.accum_err) {
> gpiod_set_value_cansleep(s6->reset_gpio, 1);
> regulator_disable(s6->supply);
> }
>
> return dsi_ctx.accum_err;
Thanks for the review, Neil. Will address all three in v2.
On the intermediate dsi variable, I prefer inlining it to keep one fewer
variable in scope, but I see the readability argument. Will revert it.
--
Thanks,
Akash