Hi Linus, On Fri, Dec 29, 2023 at 6:43 PM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote: > > On Fri, Dec 29, 2023 at 2:52 PM Dario Binacchi > <dario.binacchi@xxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx> > > 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? Thanks for your explanations and suggestions. I will rewrite the patch following your suggestions. Thanks and regards, Dario > > Yours, > Linus Walleij -- Dario Binacchi Senior Embedded Linux Developer dario.binacchi@xxxxxxxxxxxxxxxxxxxx __________________________________ Amarula Solutions SRL Via Le Canevare 30, 31100 Treviso, Veneto, IT T. +39 042 243 5310 info@xxxxxxxxxxxxxxxxxxxx www.amarulasolutions.com
