<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Tue, Mar 11, 2025 at 11:52 AM Doug Anderson <<a href="mailto:dianders@chromium.org">dianders@chromium.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi,<br>
<br>
On Mon, Mar 10, 2025 at 1:58 PM Anusha Srivatsa <<a href="mailto:asrivats@redhat.com" target="_blank">asrivats@redhat.com</a>> wrote:<br>
><br>
> @@ -70,6 +70,7 @@ static int r63353_panel_power_on(struct r63353_panel *rpanel)<br>
>  {<br>
>         struct mipi_dsi_device *dsi = rpanel->dsi;<br>
>         struct device *dev = &dsi->dev;<br>
> +       struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };<br>
>         int ret;<br>
><br>
>         ret = regulator_enable(rpanel->avdd);<br>
> @@ -78,7 +79,7 @@ static int r63353_panel_power_on(struct r63353_panel *rpanel)<br>
>                 return ret;<br>
>         }<br>
><br>
> -       usleep_range(15000, 25000);<br>
> +       mipi_dsi_usleep_range(&dsi_ctx, 15000, 25000);<br>
<br>
No. None of the conversions in this function are correct.<br>
mipi_dsi_usleep_range() is only for use when you're in the middle of a<br>
bunch of other "multi" calls and want the sleep to be conditional upon<br>
there being no error. Here there is no chance of an error because no<br>
_multi() are used. Go back to the normal usleep_range().<br>
<br></blockquote><div><br></div><div>OK. Then the approach to prefer mipi_dsi_usleep_range() over the previously used usleep_range() everywhere is out the window. Sounds good. Is replacing msleep() with mipi_dsi_msleep() preferable? </div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> @@ -106,53 +107,46 @@ static int r63353_panel_power_off(struct r63353_panel *rpanel)<br>
>  static int r63353_panel_activate(struct r63353_panel *rpanel)<br>
>  {<br>
>         struct mipi_dsi_device *dsi = rpanel->dsi;<br>
> -       struct device *dev = &dsi->dev;<br>
> -       int i, ret;<br>
> +       struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };<br>
> +       int i;<br>
><br>
> -       ret = mipi_dsi_dcs_soft_reset(dsi);<br>
> -       if (ret < 0) {<br>
> -               dev_err(dev, "Failed to do Software Reset (%d)\n", ret);<br>
> +       mipi_dsi_dcs_soft_reset_multi(&dsi_ctx);<br>
> +       if (dsi_ctx.accum_err)<br>
>                 goto fail;<br>
> -       }<br>
<br>
This isn't how the _multi() functions are intended to be used. The<br>
whole idea is _not_ to have scattered "if" statements everywhere and<br>
to just deal with errors at the appropriate places. You just trust<br>
that the _multi() functions are no-ops if an error is set and so it<br>
doesn't hurt to keep calling them. In this case you'd just have a pile<br>
of _multi() functions with no "if" checks and then at the very end of<br>
the function you check for the error. If the error is set then you set<br>
the reset GPIO and return the error.<br>
<br></blockquote><div><br></div><div>Perfect. Thanks.</div><div><br></div><div>Anusha</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
-Doug<br>
<br>
</blockquote></div></div>