Re: [PATCH 4/4] drm/panel: newvision-nv3051d: Use mipi_dsi_*_multi(); fix minor bugs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/panel: ignored error in panel_nv3051d_init_sequence()
--

commit aa26fcd3815b87a0163ded60d82dde5bf60bbecd
Author: Akash Sukhavasi <akash.sukhavasi@gmail.com>
drm/panel: newvision-nv3051d: Use mipi_dsi_*_multi(); fix minor bugs

This commit converts the newvision-nv3051d panel driver to use the _multi()
variants of the mipi_dsi_dcs_* functions. It uses mipi_dsi_msleep() and
mipi_dsi_usleep_range() for delays so they are skipped on prior errors. It
also fixes a bug where an enter_sleep_mode failure would skip the post-sleep
delay and leave the panel in a stuck state.

> diff --git a/drivers/gpu/drm/panel/panel-newvision-nv3051d.c b/drivers/gpu/drm/panel/panel-newvision-nv3051d.c
> index 22560384e48e3..7dc25102a7735 100644
> --- a/drivers/gpu/drm/panel/panel-newvision-nv3051d.c
> +++ b/drivers/gpu/drm/panel/panel-newvision-nv3051d.c
[ ... ]
> @@ -295,22 +291,16 @@ static int panel_nv3051d_prepare(struct drm_panel *panel)
>  		goto disable_vdd;
>  	}
>  
> -	ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
> -	if (ret < 0) {
> -		dev_err(ctx->dev, "Failed to exit sleep mode: %d\n", ret);
> -		goto disable_vdd;
> -	}
> -
> -	msleep(200);
> +	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);

[Severity: Medium]
Does panel_nv3051d_init_sequence() mask initialization failures?

In panel_nv3051d_init_sequence(), the function uses a local dsi_ctx to
accumulate errors, but always returns 0:

static int panel_nv3051d_init_sequence(struct panel_nv3051d *ctx)
{
        ...
        struct mipi_dsi_multi_context dsi_ctx = {.dsi = dsi};

        mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xFF, 0x30);
        ...
        return 0;
}

If any DCS write fails, the error is stored in the local dsi_ctx.accum_err,
but the function unconditionally returns 0 instead of the accumulated error.

This causes the caller panel_nv3051d_prepare() to falsely assume the
initialization succeeded.

Will this proceed to exit sleep mode and turn on the display rather than
aborting and safely disabling the regulator when DSI communication fails?

> +	mipi_dsi_msleep(&dsi_ctx, 200);
> +	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> +	mipi_dsi_usleep_range(&dsi_ctx, 10000, 15000);
>  
> -	ret = mipi_dsi_dcs_set_display_on(dsi);
> -	if (ret < 0) {
> -		dev_err(ctx->dev, "Failed to set display on: %d\n", ret);
> +	if (dsi_ctx.accum_err) {
> +		ret = dsi_ctx.accum_err;
>  		goto disable_vdd;
>  	}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-mipi-multi-series-v1-0-6a29add1ec92@gmail.com?part=4