Hi,
On Mon, Jan 6, 2025 at 8:21 PM Tejas Vipin <tejasvipin76@xxxxxxxxx> wrote:
>
> >>> - ret = xpp055c272_init_sequence(ctx);
> >>> - if (ret < 0) {
> >>> - dev_err(ctx->dev, "Panel init sequence failed: %d\n", ret);
> >>> - goto disable_iovcc;
> >>> - }
> >>> -
> >>> - ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
> >>> - if (ret < 0) {
> >>> - dev_err(ctx->dev, "Failed to exit sleep mode: %d\n", ret);
> >>> - goto disable_iovcc;
> >>> - }
> >>> + xpp055c272_init_sequence(&dsi_ctx);
> >>> + dev_dbg(ctx->dev, "Panel init sequence done\n");
> >
> > Should the above print be only if "accum_err" is 0? That would match
> > the previous behavior. I guess I would have also left the print as
> > part of xpp055c272_init_sequence() unless there's a reason for moving
> > it...
>
> I don't think it should print only if accum_err is 0. In the previous
> code, it would just print after all the msleeps and write_seqs are done,
> with no error checking at any point.
How sure are you about this? Remember the reason why we wanted to
deprecate mipi_dsi_dcs_write_seq()? All those dang hidden return
values. So if any one of the old mipi_dsi_dcs_write_seq() got an error
they would have had a non-obvious return out of the function, right?
So the print would have only happened if all of the commands executed
successfully...
:-P
> The reason I've moved the print outside the function is because we are
> able to reduce a couple lines of code by passing dsi_ctx to the function
> instead of ctx. If I'd kept the print inside, it would require us to
> declare a `struct device*` variable which would require ctx as far as
> I've seen and just overall introduces some lines that we could otherwise
> avoid. I've done this in a couple other panels too.
Ah, OK. That's a reasonable reason. Thanks for the explanation...
-Doug
