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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAD=FV=XV8D2X98ULLhT0ChAfSxiBgA1uh6tRtzGv8RpXiFYN+Q@mail.gmail.com>
Date: Mon, 6 Jan 2025 20:48:19 -0800
From: Doug Anderson <dianders@...omium.org>
To: Tejas Vipin <tejasvipin76@...il.com>
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

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


> 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