Re: [PATCH] drm/panel: xinpeng-xpp055c272: transition to mipi_dsi wrapped functions

[Tejas Vipin <tejasvipin76@gmail.com>]

On 1/7/25 10:18 AM, Doug Anderson wrote:
> Hi,
> 
> On Mon, Jan 6, 2025 at 8:21 PM Tejas Vipin <tejasvipin76@gmail.com> 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