Thomas Heijligen has submitted this change. ( https://review.coreboot.org/c/flashrom/+/73337 )
Change subject: spi: Make default cmd helpers static internal ......................................................................
spi: Make default cmd helpers static internal
Avoid these leaking into driver implementations as a NULL field now implies their implementation. This removes one source of a driver bug where both `mst->command` AND `mst->multicommand` are set to default implementations which is actually a cyclical control flow condition.
The driver however must still have either `mst->command` OR `mst->multicommand` defined and so both cannot be NULL.
This simplifies the code and driver development.
Change-Id: I4ef95846c2f005cf4aa727f31548c6877d2d4801 Signed-off-by: Edward O'Callaghan quasisec@google.com Reviewed-on: https://review.coreboot.org/c/flashrom/+/73337 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Thomas Heijligen src@posteo.de Reviewed-by: Anastasia Klimchuk aklm@chromium.org --- M include/programmer.h M spi.c 2 files changed, 44 insertions(+), 23 deletions(-)
Approvals: build bot (Jenkins): Verified Thomas Heijligen: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved
diff --git a/include/programmer.h b/include/programmer.h index f72f1d7..f6bc8ce 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -318,9 +318,6 @@ void *data; };
-int default_spi_send_command(const struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, - const unsigned char *writearr, unsigned char *readarr); -int default_spi_send_multicommand(const struct flashctx *flash, struct spi_command *cmds); int default_spi_read(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len); int default_spi_write_256(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); int default_spi_write_aai(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); diff --git a/spi.c b/spi.c index 4fe0fa5..3b3f622 100644 --- a/spi.c +++ b/spi.c @@ -26,23 +26,7 @@ #include "programmer.h" #include "spi.h"
-int spi_send_command(const struct flashctx *flash, unsigned int writecnt, - unsigned int readcnt, const unsigned char *writearr, - unsigned char *readarr) -{ - if (flash->mst->spi.command) - return flash->mst->spi.command(flash, writecnt, readcnt, writearr, readarr); - return default_spi_send_command(flash, writecnt, readcnt, writearr, readarr); -} - -int spi_send_multicommand(const struct flashctx *flash, struct spi_command *cmds) -{ - if (flash->mst->spi.multicommand) - return flash->mst->spi.multicommand(flash, cmds); - return default_spi_send_multicommand(flash, cmds); -} - -int default_spi_send_command(const struct flashctx *flash, unsigned int writecnt, +static int default_spi_send_command(const struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr) @@ -63,7 +47,7 @@ return spi_send_multicommand(flash, cmd); }
-int default_spi_send_multicommand(const struct flashctx *flash, +static int default_spi_send_multicommand(const struct flashctx *flash, struct spi_command *cmds) { int result = 0; @@ -74,6 +58,22 @@ return result; }
+int spi_send_command(const struct flashctx *flash, unsigned int writecnt, + unsigned int readcnt, const unsigned char *writearr, + unsigned char *readarr) +{ + if (flash->mst->spi.command) + return flash->mst->spi.command(flash, writecnt, readcnt, writearr, readarr); + return default_spi_send_command(flash, writecnt, readcnt, writearr, readarr); +} + +int spi_send_multicommand(const struct flashctx *flash, struct spi_command *cmds) +{ + if (flash->mst->spi.multicommand) + return flash->mst->spi.multicommand(flash, cmds); + return default_spi_send_multicommand(flash, cmds); +} + int default_spi_read(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len) { @@ -156,8 +156,7 @@ }
if (!mst->write_256 || !mst->read || !mst->probe_opcode || - ((mst->command == default_spi_send_command || !mst->command) && - (mst->multicommand == default_spi_send_multicommand || !mst->multicommand))) { + (!mst->command && !mst->multicommand)) { msg_perr("%s called with incomplete master definition. " "Please report a bug at flashrom@flashrom.org\n", __func__);