Re: [PATCH v3 4/4] drm: panel: Add Jadard JD9365DA-H3 DSI panel

[Linus Walleij <linus.walleij@linaro.org>]

On Tue, Nov 8, 2022 at 3:12 PM Jagan Teki <jagan@edgeble.ai> wrote:
> On Tue, 8 Nov 2022 at 19:31, Linus Walleij <linus.walleij@linaro.org> wrote:
> >
> > On Thu, Nov 3, 2022 at 3:12 PM Jagan Teki <jagan@edgeble.ai> wrote:
> >
> > > Jadard JD9365DA-H3 is WXGA MIPI DSI panel and it support TFT
> > > dot matrix LCD with 800RGBx1280 dots at maximum.
> > >
> > > Add support for it.
> > >
> > > Cc: dri-devel@lists.freedesktop.org
> > > Signed-off-by: Jagan Teki <jagan@edgeble.ai>
> > > ---
> > > Changes for v3:
> > > - updatd to WXGA
> > > - use JD9365DA_CMD_DCS and JD9365DA_CMD_DELAY
> >
> > My comments on v2 have not been addressed, for example I asked to
> > remove the delay from sequences and just use an explicit delay and
> > to then use the existing sequence sending macro.
>
> True, I responded on the same day [1], since I didn't get the reply I
> have posted by assuming my comment is valid. Would you please check
> and respond?
>
> [1] https://lore.kernel.org/all/CA+VMnFz0w-6O=wt3iuJo1BhQgPZ2XbpX6JdDz6vg_JW9nHTR2A@mail.gmail.com/

OK I see, sorry for not reading close.

The driver just supports one single variant.

What you are doing is preparing the ground for more variants
that may or may not exist. This creates the antipattern "big upfront design"
i.e. abstractions added for things that do not yet exist.

I think it is better to strip it down to just open coding the delay after
the init sequence. When the next variant appears, if ever, they can
add abstraction. Maybe they need the same delay in the same
place? Who knows...

Yours,
Linus Walleij