linux-kernel - Re: [PATCH v2] drm/panel/synaptics-r63353: Use

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAD=FV=WW1ak-_MEBVks==Yr1tUdfFZ3K16_gcdJQ9rwE4ZduNg@mail.gmail.com>
Date: Tue, 11 Mar 2025 08:51:49 -0700
From: Doug Anderson <dianders@...omium.org>
To: Anusha Srivatsa <asrivats@...hat.com>
Cc: Michael Trimarchi <michael@...rulasolutions.com>, 
	Neil Armstrong <neil.armstrong@...aro.org>, Jessica Zhang <quic_jesszhan@...cinc.com>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>, 
	Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, 
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org, 
	Dmitry Baryshkov <dmitry.baryshkov@...aro.org>, Tejas Vipin <tejasvipin76@...il.com>
Subject: Re: [PATCH v2] drm/panel/synaptics-r63353: Use _multi variants

Hi,

On Mon, Mar 10, 2025 at 1:58 PM Anusha Srivatsa <asrivats@...hat.com> wrote:
>
> @@ -70,6 +70,7 @@ static int r63353_panel_power_on(struct r63353_panel *rpanel)
>  {
>         struct mipi_dsi_device *dsi = rpanel->dsi;
>         struct device *dev = &dsi->dev;
> +       struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
>         int ret;
>
>         ret = regulator_enable(rpanel->avdd);
> @@ -78,7 +79,7 @@ static int r63353_panel_power_on(struct r63353_panel *rpanel)
>                 return ret;
>         }
>
> -       usleep_range(15000, 25000);
> +       mipi_dsi_usleep_range(&dsi_ctx, 15000, 25000);

No. None of the conversions in this function are correct.
mipi_dsi_usleep_range() is only for use when you're in the middle of a
bunch of other "multi" calls and want the sleep to be conditional upon
there being no error. Here there is no chance of an error because no
_multi() are used. Go back to the normal usleep_range().

> @@ -106,53 +107,46 @@ static int r63353_panel_power_off(struct r63353_panel *rpanel)
>  static int r63353_panel_activate(struct r63353_panel *rpanel)
>  {
>         struct mipi_dsi_device *dsi = rpanel->dsi;
> -       struct device *dev = &dsi->dev;
> -       int i, ret;
> +       struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
> +       int i;
>
> -       ret = mipi_dsi_dcs_soft_reset(dsi);
> -       if (ret < 0) {
> -               dev_err(dev, "Failed to do Software Reset (%d)\n", ret);
> +       mipi_dsi_dcs_soft_reset_multi(&dsi_ctx);
> +       if (dsi_ctx.accum_err)
>                 goto fail;
> -       }

This isn't how the _multi() functions are intended to be used. The
whole idea is _not_ to have scattered "if" statements everywhere and
to just deal with errors at the appropriate places. You just trust
that the _multi() functions are no-ops if an error is set and so it
doesn't hurt to keep calling them. In this case you'd just have a pile
of _multi() functions with no "if" checks and then at the very end of
the function you check for the error. If the error is set then you set
the reset GPIO and return the error.

-Doug