Re: [PATCH v2 06/11] drm/panel: s6e3ha8: Correct the polarity logic within

[Krzysztof Kozlowski]

On Thu, Sep 24, 2026 at 04:01:34PM +0200, David Heidelberg wrote:
> The reset was introduced with wrong polarity. Correct for the future
> compatibles and keep current with reverted logic.
> 
> Old DTs keep GPIO_ACTIVE_HIGH and are fixed up via
> gpiod_toggle_active_low() on the deprecated compatible.
> 
> Assisted-by: LLM
> Reviewed-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx>
> Signed-off-by: David Heidelberg <david@xxxxxxx>
> ---
>  drivers/gpu/drm/panel/panel-samsung-s6e3ha8.c | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e3ha8.c b/drivers/gpu/drm/panel/panel-samsung-s6e3ha8.c
> index 5e1e997b83b36..99290913de69a 100644
> --- a/drivers/gpu/drm/panel/panel-samsung-s6e3ha8.c
> +++ b/drivers/gpu/drm/panel/panel-samsung-s6e3ha8.c
> @@ -20,16 +20,17 @@
>  #include "panel-samsung-dsi.h"
>  
>  struct s6e3ha8_desc {
>  	const struct drm_panel_funcs *funcs;
>  	const struct drm_display_mode *mode;
>  	unsigned long mode_flags;
>  	const struct regulator_bulk_data *supplies;
>  	unsigned int num_supplies;
> +	bool broken_reset_polarity;
>  };
>  
>  struct s6e3ha8 {
>  	struct drm_panel panel;
>  	struct mipi_dsi_device *dsi;
>  	const struct s6e3ha8_desc *desc;
>  	struct drm_dsc_config dsc;
>  	struct gpio_desc *reset_gpio;
> @@ -62,22 +63,22 @@ static int s6e3ha8_unprepare(struct drm_panel *panel)
>  {
>  	struct s6e3ha8 *priv = to_s6e3ha8(panel);
>  
>  	return regulator_bulk_disable(priv->desc->num_supplies, priv->supplies);
>  }
>  
>  static void s6e3ha8_amb577px01_wqhd_reset(struct s6e3ha8 *priv)
>  {
> -	gpiod_set_value_cansleep(priv->reset_gpio, 1);
> -	usleep_range(5000, 6000);
>  	gpiod_set_value_cansleep(priv->reset_gpio, 0);
>  	usleep_range(5000, 6000);
>  	gpiod_set_value_cansleep(priv->reset_gpio, 1);
>  	usleep_range(5000, 6000);
> +	gpiod_set_value_cansleep(priv->reset_gpio, 0);

This breaks all users and this usage of ABI was already released.

NAK

> +	usleep_range(5000, 6000);
>  }
>  
>  static int s6e3ha8_amb577px01_wqhd_on(struct s6e3ha8 *priv)
>  {
>  	struct mipi_dsi_device *dsi = priv->dsi;
>  	struct mipi_dsi_multi_context ctx = { .dsi = dsi };
>  
>  	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> @@ -203,17 +204,17 @@ static int s6e3ha8_amb577px01_wqhd_prepare(struct drm_panel *panel)
>  	ret = regulator_bulk_enable(priv->desc->num_supplies, priv->supplies);
>  	if (ret < 0)
>  		return ret;
>  	mipi_dsi_msleep(&ctx, 120);
>  	s6e3ha8_amb577px01_wqhd_reset(priv);
>  
>  	ret = s6e3ha8_amb577px01_wqhd_on(priv);
>  	if (ret < 0) {
> -		gpiod_set_value_cansleep(priv->reset_gpio, 0);
> +		gpiod_set_value_cansleep(priv->reset_gpio, 1);

No, you just changed it!

Implement correct and logical fixes, not half-baked fix which is
immediately wrong.

Best regards,
Krzysztof