Hi Andy, Thanks for the feedback.
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? 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. Best regards, Sun Jian On Tue, Jan 6, 2026 at 12:28 AM Andy Shevchenko <[email protected]> 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? > > ... > > > struct device *dev = par->info->device; > > - int buf[64], count, index, i, j, ret; > > + u8 buf[64]; > > + int count, index, i, j, ret; > > Please, try to preserve reversed xmas tree order. > > > u32 *values; > > u32 val; > > > > ... > > > - buf[i++] = val; > > + buf[i++] = val & 0xFF; > > Unneeded change, I suppose. > > ... > > > - 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 > >
