Nico Huber has posted comments on this change. ( https://review.coreboot.org/22018 )
Change subject: spi25: Introduce spi_simple_write_cmd() ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/22018/1/spi25.c File spi25.c:
https://review.coreboot.org/#/c/22018/1/spi25.c@326 PS1, Line 326: spi_simple_write_cmd
Is there anything other than chip erase commands that this applies to? I th
Yes, I've repurposed this later for 4BA-enable functions that need a WREN. With that said, we could also name the functions after WREN, e.g.
static int spi_simple_wren_cmd(...);
and
static int spi_wren_cmd(...);
instead of `spi_write_cmd()` (that is added later) as it's also used for erasures. Or if we want to emphasize the command parts even more:
static int spi_wren_simple_cmd(...); static int spi_wren_address_cmd(...);
My head is full of ideas, I just don't have a strong preference.
Another thought: Would you prefer both functions to be commented, i.e. a doxygen style interface documentation? I'm often unsure if it's necessary as the code is already very specific about what is done ;) and having it commented means we have to take special care to keep the comment in sync with the implementation.