Re: [PATCH v2 2/5] drm/panel: raydium-rm68200: Fix prepare error cleanup

[neil.armstrong@linaro.org]

On 11/2/26 05:32, Hsieh Hung-En wrote:
> rm68200_prepare() returns without disabling the supply regulator if
> EXIT_SLEEP_MODE or SET_DISPLAY_ON fails. Since drm_panel_prepare() does
> not mark the panel as prepared on failure, drm_panel_unprepare() does
> not call the driver's unprepare() callback, leaving the regulator
> enable count elevated even after subsequent successful
> prepare/unprepare cycles.
> 
> On failure, restore the reset GPIO to its unprepared state, following
> the existing reset sequence in unprepare(), and disable the supply
> regulator before returning the original error.
> 
> Fixes: 2b7ed18bed1a ("drm/panel: Add support for Raydium RM68200 panel driver")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/r/sashiko-outbox-171393@kernel.org
> Signed-off-by: Hsieh Hung-En <hungen3108@gmail.com>
> ---
> 
> Notes:
>      In this patch, rm68200_init_sequence() still returns void and only
>      logs DCS write failures. These failures do not cause prepare() to
>      return early. A later patch in this series will propagate
>      initialization errors through prepare(), where this cleanup path
>      will also handle them.
>      
>      The error path does not send SET_DISPLAY_OFF or ENTER_SLEEP_MODE,
>      as DCS communication may already have failed. Instead, it follows
>      the existing reset and regulator shutdown sequence in unprepare().
> 
>   drivers/gpu/drm/panel/panel-raydium-rm68200.c | 13 +++++++++++--
>   1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-raydium-rm68200.c b/drivers/gpu/drm/panel/panel-raydium-rm68200.c
> index c535dc931903..3a59eb00274d 100644
> --- a/drivers/gpu/drm/panel/panel-raydium-rm68200.c
> +++ b/drivers/gpu/drm/panel/panel-raydium-rm68200.c
> @@ -277,17 +277,26 @@ static int rm68200_prepare(struct drm_panel *panel)
>   
>   	ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
>   	if (ret)
> -		return ret;
> +		goto assert_reset;
>   
>   	msleep(125);
>   
>   	ret = mipi_dsi_dcs_set_display_on(dsi);
>   	if (ret)
> -		return ret;
> +		goto assert_reset;
>   
>   	msleep(20);
>   
>   	return 0;
> +
> +assert_reset:
> +	if (ctx->reset_gpio) {
> +		gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> +		msleep(20);
> +	}
> +	/* Balance the regulator enable count on prepare failure. */
> +	regulator_disable(ctx->supply);
> +	return ret;
>   }
>   
>   static int rm68200_get_modes(struct drm_panel *panel,

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Thanks,
Neil