Re: [PATCH v1 1/4] staging: fbtft: core: avoid large stack usage in DT init parsing

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Tue, Jan 06, 2026 at 01:00:33AM +0800, sun jian wrote:

> Thanks for the feedback.

You're welcome, but please, do not top-post!

> You are right: changing the DT init path from write_register() to
> fbtft_write_buf_dc() implicitly assumes "cmd byte + payload bytes" and
> does not preserve the generic write_register() semantics (e.g. regwidth /
> bus-specific handling).I only have clang/arm64 build coverage (no
> access to the actual panels),
> so I can’t provide runtime validation yet. For the remaining 3 driver-local
> patches, all affected drivers have .regwidth = 8 and the sequences are
> “1-byte command + N bytes data” (gamma/LUT). The intent was to avoid the
> huge write_reg() varargs call that triggers -Wframe-larger-than=1024.
> 
> Given the lack of hardware, would you prefer one of the following?

How can you test without hardware at hand?

> 1. Drop the driver changes and instead bump -Wframe-larger-than for these
>    specific objects in the Makefile as an exception; or
> 
> 2. Keep the driver changes but I should provide a detailed test pattern /
>    list of tested devices — if so, what level of detail would be acceptable
>    (exact panel model + wiring/bus type + expected output), and is “build-only”
>    ever sufficient for warning-only changes in fbtft?
> 
> Happy to follow the approach you think is appropriate for this staging driver.

I already explained in the response to the cover letter. Please, read it.

> On Tue, Jan 6, 2026 at 12:28 AM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> >
> > On Sun, Jan 04, 2026 at 07:06:35PM +0800, Sun Jian wrote:
> > > Clang reports a large stack frame for fbtft_init_display_from_property()
> > > (-Wframe-larger-than=1024) when the init sequence is emitted through a
> > > fixed 64-argument write_register() call.
> > >
> > > write_reg()/write_register() relies on NUMARGS((int[]){...}) and large
> > > varargs which inflates stack usage. Switch the DT "init" path to send the
> > > command byte and the payload via fbtft_write_buf_dc() instead.
> > >
> > > No functional change intended: the same register values are sent in the
> > > same order, only the transport is changed.
> >
> > How did you test this?

...

> > > -                     par->fbtftops.write_register(par, i,
> > > -                             buf[0], buf[1], buf[2], buf[3],
> > > -                             buf[4], buf[5], buf[6], buf[7],
> > > -                             buf[8], buf[9], buf[10], buf[11],
> > > -                             buf[12], buf[13], buf[14], buf[15],
> > > -                             buf[16], buf[17], buf[18], buf[19],
> > > -                             buf[20], buf[21], buf[22], buf[23],
> > > -                             buf[24], buf[25], buf[26], buf[27],
> > > -                             buf[28], buf[29], buf[30], buf[31],
> > > -                             buf[32], buf[33], buf[34], buf[35],
> > > -                             buf[36], buf[37], buf[38], buf[39],
> > > -                             buf[40], buf[41], buf[42], buf[43],
> > > -                             buf[44], buf[45], buf[46], buf[47],
> > > -                             buf[48], buf[49], buf[50], buf[51],
> > > -                             buf[52], buf[53], buf[54], buf[55],
> > > -                             buf[56], buf[57], buf[58], buf[59],
> > > -                             buf[60], buf[61], buf[62], buf[63]);
> > > +                     /* buf[0] is command, buf[1..i-1] is data */
> > > +                     ret = fbtft_write_buf_dc(par, &buf[0], 1, 0);
> > > +                     if (ret < 0)
> > > +                             goto out_free;
> > > +
> > > +                     if (i > 1) {
> > > +                             ret = fbtft_write_buf_dc(par, &buf[1], i - 1, 1);
> > > +                             if (ret < 0)
> > > +                                     goto out_free;
> > > +                     }
> >
> > I believe this is incorrect change and has not to be applied. write !=
> > write_register. Without any evidence of testing, definite NAK to it.
> > Otherwise, please provide detailed testing pattern and which devices were
> > tested.

-- 
With Best Regards,
Andy Shevchenko