Re: [PATCH v2 1/5] drm/panel: feiyang-fy07024di26a30d: Fix prepare error cleanup

[neil.armstrong@linaro.org]

On 11/2/26 05:32, Hsieh Hung-En wrote:
> feiyang_prepare() returns without disabling previously enabled
> regulators if enabling AVDD or writing DCS commands 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 regulator enable counts elevated even after subsequent
> successful prepare/unprepare cycles.
> 
> Disable DVDD if enabling AVDD fails. On a DCS write failure, restore
> the reset GPIO to its unprepared state and disable both regulators,
> preserving the existing 10 ms power-off interval between AVDD and DVDD.
> 
> Fixes: 69dc678abc2b ("drm/panel: Add Feiyang FY07024DI26A30-D MIPI-DSI LCD panel")
> Reported-by: Douglas Anderson <dianders@chromium.org>
> Closes: https://lore.kernel.org/r/CAD=FV=Xm48DUT==8HuuGCSFZtLTEk-W2Q6UTfqt-MnM4nULzTw@mail.gmail.com
> Signed-off-by: Hsieh Hung-En <hungen3108@gmail.com>
> ---
> 
> Notes:
>      Power sequencing note:
>      
>      The error path retains the 10 ms AVDD-to-DVDD shutdown interval
>      used by unprepare().
>      
>      The FY07024DI26A30-D datasheet, Section 7.3 (Power, Signal sequence),
>      specifies T10 > 0 between AVDD/VGL reaching GND and DVDD starting
>      to fall. The delay allows time for AVDD to discharge before disabling
>      DVDD, but compliance with T10 has not been verified on hardware.
>      
>      The existing unprepare() comment associates the delay with T11,
>      which actually specifies the DVDD fall time (0 < T11 <= 10 ms).
>      That comment can be corrected separately.
>      
>      The msleep(10) checkpatch warning is expected. T10 has no specified
>      upper bound, so a longer sleep does not violate that timing constraint.
>      msleep() is retained to match the existing unprepare() sequence.
>      
>      Datasheet:
>      https://files.pine64.org/doc/datasheet/pine64/FY07024DI26A30-D_feiyang_LCD_panel.pdf
> 
>   .../gpu/drm/panel/panel-feiyang-fy07024di26a30d.c | 15 +++++++++++++--
>   1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c b/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
> index c1d8ca5ca6e1..bb4cc1ea2ece 100644
> --- a/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
> +++ b/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
> @@ -59,7 +59,7 @@ static int feiyang_prepare(struct drm_panel *panel)
>   
>   	ret = regulator_enable(ctx->avdd);
>   	if (ret)
> -		return ret;
> +		goto disable_dvdd;
>   
>   	/* T3 (dvdd rise + avdd start + avdd rise) T3 >= 20ms */
>   	msleep(20);
> @@ -84,10 +84,21 @@ static int feiyang_prepare(struct drm_panel *panel)
>   		ret = mipi_dsi_dcs_write_buffer(dsi, cmd->data,
>   						FEIYANG_INIT_CMD_LEN);
>   		if (ret < 0)
> -			return ret;
> +			goto assert_reset;
>   	}
>   
>   	return 0;
> +
> +assert_reset:
> +	gpiod_set_value(ctx->reset, 0);
> +	regulator_disable(ctx->avdd);
> +
> +	/* Preserve the AVDD-to-DVDD shutdown delay from unprepare(). */
> +	msleep(10);
> +
> +disable_dvdd:
> +	regulator_disable(ctx->dvdd);
> +	return ret;
>   }
>   
>   static int feiyang_enable(struct drm_panel *panel)

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

Thanks,
Neil