On 1/7/25 10:18 AM, Doug Anderson wrote:
> 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
Right yes. I'd kind of forgotten how mipi_dsi_dcs_write_seq worked. I'll
fix it then along with the rest of the changes.
>
>
>> 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
--
Tejas Vipin
