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
>
