Hi Dan, Thanks. Agreed.
For v2 I’ll keep this patch minimal: only switch from write_reg() varargs to fbtft_write_buf_dc() and reduce the stack usage, without other changes. Any follow-up cleanups (if needed) will be sent as a separate patch. Thanks, Sun Jian On Mon, Jan 5, 2026 at 10:39 PM Dan Carpenter <[email protected]> wrote: > > On Sun, Jan 04, 2026 at 07:06:36PM +0800, Sun Jian wrote: > > Clang reports a large stack frame in set_gamma() (-Wframe-larger-than=1024) > > due to the large write_reg() call emitting 63 gamma bytes via varargs. > > > > Send the command byte (0xB8) and the gamma payload using > > fbtft_write_buf_dc() to avoid the varargs/NUMARGS stack blow-up. > > > > No functional change intended. > > > > Signed-off-by: Sun Jian <[email protected]> > > --- > > drivers/staging/fbtft/fb_ssd1351.c | 35 +++++++++++++----------------- > > 1 file changed, 15 insertions(+), 20 deletions(-) > > > > diff --git a/drivers/staging/fbtft/fb_ssd1351.c > > b/drivers/staging/fbtft/fb_ssd1351.c > > index 6736b09b2f45..b4ab2c81e528 100644 > > --- a/drivers/staging/fbtft/fb_ssd1351.c > > +++ b/drivers/staging/fbtft/fb_ssd1351.c > > @@ -119,43 +119,38 @@ static int set_var(struct fbtft_par *par) > > */ > > static int set_gamma(struct fbtft_par *par, u32 *curves) > > { > > - unsigned long tmp[GAMMA_NUM * GAMMA_LEN]; > > + u8 data[GAMMA_LEN]; > > Ugh... GAMMA_NUM is 1 so this is an annoying calculation. So what > this does is it changes the type from unsigned long to u8 and renames > the variable. I am fine with renaming the variable it's unrelated and > makes the review harder. > > > + u8 cmd = 0xB8; > > int i, acc = 0; > > + int ret; > > > > - for (i = 0; i < 63; i++) { > > + for (i = 0; i < GAMMA_LEN; i++) { > > GAMMA_LEN is 63. So this looks like a change, but it's an unrelated > cleanup. > > > if (i > 0 && curves[i] < 2) { > > dev_err(par->info->device, > > "Illegal value in Grayscale Lookup Table at > > index %d : %d. Must be greater than 1\n", > > i, curves[i]); > > return -EINVAL; > > } > > + > > This is an unrelated white space change. > > > acc += curves[i]; > > - tmp[i] = acc; > > + > > if (acc > 180) { > > dev_err(par->info->device, > > "Illegal value(s) in Grayscale Lookup Table. > > At index=%d : %d, the accumulated value has exceeded 180\n", > > i, acc); > > return -EINVAL; > > } > > + > > + data[i] = acc; > > Here we move the acc assignment after the sanity check, but it's just > an unrelated cleanup. > > > } > > > > - write_reg(par, 0xB8, > > - tmp[0], tmp[1], tmp[2], tmp[3], > > - tmp[4], tmp[5], tmp[6], tmp[7], > > - tmp[8], tmp[9], tmp[10], tmp[11], > > - tmp[12], tmp[13], tmp[14], tmp[15], > > - tmp[16], tmp[17], tmp[18], tmp[19], > > - tmp[20], tmp[21], tmp[22], tmp[23], > > - tmp[24], tmp[25], tmp[26], tmp[27], > > - tmp[28], tmp[29], tmp[30], tmp[31], > > - tmp[32], tmp[33], tmp[34], tmp[35], > > - tmp[36], tmp[37], tmp[38], tmp[39], > > - tmp[40], tmp[41], tmp[42], tmp[43], > > - tmp[44], tmp[45], tmp[46], tmp[47], > > - tmp[48], tmp[49], tmp[50], tmp[51], > > - tmp[52], tmp[53], tmp[54], tmp[55], > > - tmp[56], tmp[57], tmp[58], tmp[59], > > - tmp[60], tmp[61], tmp[62]); > > + ret = fbtft_write_buf_dc(par, &cmd, 1, 0); > > + if (ret < 0) > > + return ret; > > + > > + ret = fbtft_write_buf_dc(par, data, sizeof(data), 1); > > + if (ret < 0) > > + return ret; > > These are good changes. Just change the type from unsigned long to u8 > and use fbtft_write_buf_dc() instead of write_reg(). Then do the other > changes in a separate patch. > > Same for the other patches. > > regards, > dan carpenter > > > > > return 0; > > } > > -- > > 2.43.0 > >
