linux-kernel - Re: [PATCH] drm/panel: xinpeng-xpp055c272: transition to mipi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1f1cdfdd-2765-49f2-b179-14b18bd49d67@gmail.com>
Date: Tue, 7 Jan 2025 14:24:10 +0530
From: Tejas Vipin <tejasvipin76@...il.com>
To: Doug Anderson <dianders@...omium.org>
Cc: neil.armstrong@...aro.org, maarten.lankhorst@...ux.intel.com,
 mripard@...nel.org, tzimmermann@...e.de, airlied@...il.com, simona@...ll.ch,
 quic_jesszhan@...cinc.com, dri-devel@...ts.freedesktop.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/panel: xinpeng-xpp055c272: transition to mipi_dsi
 wrapped functions



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