On 29/09/2026 11:59, David Heidelberg wrote: > On 29/09/2026 11:51, Krzysztof Kozlowski wrote: >> On 29/09/2026 11:41, David Heidelberg wrote: >>> On 29/09/2026 09:45, Krzysztof Kozlowski wrote: >>>> 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 <[email protected]> >>>>> Signed-off-by: David Heidelberg <[email protected]> >>>>> --- >>>>> 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. >>> >>> See the gpiod_toggle_active_low() usage later in the patch which keep the >>> logic >>> for the original compatible as intended. >>> >> >> OK, I went way too fast, that's correct part. But splitting fix is still >> just confusing. Backporting to stable is a different thing than fixing >> issues. > > Sure, I already droped the previous commit changing it for stable. > > Btw. looking at gpiod_toggle_active_low(), would it make sense to do a series > correcting panel reset logic? I see many panels keep "reset asserted" in the > driver (but ofc not in the reality).
To my knowledge it is impossible task to do, without breaking something. Either you break users of ABI (so the DTS) or break existing users of DTS. One could try to avoid both by using your approach here with compatibles having fallback. But then what polarity actually would be in such DTS node? If you know your users, like for some SoC components, you could argue that none of then will be affected. But both the driver and DTS here can be used externally. Best regards, Krzysztof
