linux-kernel - Re: [PATCH 7/8] drm/panel: nt35510: refactor panel initialization

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] [thread-next>] [day] [month] [year] [list]

Date: Fri, 29 Dec 2023 18:43:11 +0100
From: Linus Walleij <linus.walleij@...aro.org>
To: Dario Binacchi <dario.binacchi@...rulasolutions.com>
Cc: linux-kernel@...r.kernel.org, linux-amarula@...rulasolutions.com, 
	Alexandre Torgue <alexandre.torgue@...s.st.com>, Daniel Vetter <daniel@...ll.ch>, 
	David Airlie <airlied@...il.com>, Jessica Zhang <quic_jesszhan@...cinc.com>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>, 
	Neil Armstrong <neil.armstrong@...aro.org>, Sam Ravnborg <sam@...nborg.org>, 
	Thomas Zimmermann <tzimmermann@...e.de>, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH 7/8] drm/panel: nt35510: refactor panel initialization

On Fri, Dec 29, 2023 at 2:52 PM Dario Binacchi
<dario.binacchi@...rulasolutions.com> wrote:

> The previous implementation did not make it easy to support new
> NT35510-based panels with different initialization sequences.
> This patch, preparatory for future developmentes, simplifies the
> addition of new NT35510-based displays and also avoids the risk of
> creating regressions on already managed panels.
>
> Signed-off-by: Dario Binacchi <dario.binacchi@...rulasolutions.com>

The idea is to have the driver adapt to different panels, and encode a deep
understanding just like we do with all hardware drivers.

NAK.

This patch:

- Deletes a lot of useful documentation on how the panel works.

- Deletes defines and replaces them with magic numbers

All it achieves is a bit of "magic sequences because we are used to
magic sequences" and that doesn't look like an improvement at all,
instead it creates a dumber driver which has no explanations at all
to what is going on.

Please rewrite the patch in the same style as the original driver.
The fact that you (probably) are not used to writing display drivers
in this way is not an excuse to destroy this nice structure.

There are things that can be done, like create an abstraction for
sequence encoding with less open coded command issue
statements, by adding helpers to the DRM core, so if that is what
you want to do, then do that instead?

Yours,
Linus Walleij