Change in coreboot[master]: [RFC] drivers/spi: Stop using a variable-length array (alt)

[Julius Werner (Code Review)]

Attention is currently required from: Angel Pons.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50519 )

Change subject: [RFC] drivers/spi: Stop using a variable-length array (alt)
......................................................................

Patch Set 1:

(3 comments)

Patchset:

PS1: 
We can go this route and I don't think there's something fundamentally wrong with it or anything, but I don't think that it's really notably better either. It doesn't really matter whether you allocate the [4 + 256] stack buffer in spi_flash_cmd_write() or spi_flash_cmd_write_page_program(), and you end up with the same amount of copy operations.

File src/drivers/spi/spi_flash.c:

https://review.coreboot.org/c/coreboot/+/50519/comment/dcdb7ba4_a3302475 
PS1, Line 92: size_t cmd_len, size_t data_len
If we do want to combine the buffers, I think you might as well combine these sizes... but then that kinda removes the whole point of this function, and callers might as well call do_spi_flash_cmd() directly.

https://review.coreboot.org/c/coreboot/+/50519/comment/adde9a00_c5eff29a 
PS1, Line 243: 	u8 cmd_data[4 + MAX_FLASH_CMD_DATA_SIZE];
I mean, now you're just allocating that array here rather than in spi_flash_cmd_write(). I don't think it makes much of a difference?

BTW, if you want to write a buffer like this and then work with the command and data parts separately in the code below, the code would probably look nicer if you wrote it like this:

struct {
   u8 cmd[4];
   u8 data[MAX_FLASH_CMD_DATA_SIZE];
 } xfer;