Re: [PATCH v2] drm/panel/synaptics-r63353: Use _multi variants

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Mon, Mar 10, 2025 at 1:58 PM Anusha Srivatsa <asrivats@xxxxxxxxxx> 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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux