Change in flashrom[master]: tree/: Drop const from opaque data ptr in master definitions [alt]
Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/49131 )
Change subject: tree/: Drop const from opaque data ptr in master definitions [alt] ......................................................................
tree/: Drop const from opaque data ptr in master definitions [alt]
The opaque data pointer need not necessarily have constant data for the life-time of the specific master. This is because the data field purpose is for the master to use as it sees fit for managing its own internal state and therefore we should not constrain this as being RO data at init time.
BUG=none BRANCH=none TEST=builds
Change-Id: I686c3c79547e35d48f3fd0b524fc98c176dcea6e Signed-off-by: Edward O'Callaghan quasisec@google.com --- M bitbang_spi.c M programmer.h 2 files changed, 15 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/31/49131/1
diff --git a/bitbang_spi.c b/bitbang_spi.c index c402119..b12d81a 100644 --- a/bitbang_spi.c +++ b/bitbang_spi.c @@ -94,13 +94,18 @@ } }
+struct bitbang_spi_master_data { + const struct bitbang_spi_master *mst; +}; + static int bitbang_spi_send_command(const struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr) { unsigned int i; - const struct bitbang_spi_master *master = flash->mst->spi.data; + const struct bitbang_spi_master_data *data = flash->mst->spi.data; + const struct bitbang_spi_master *master = data->mst;
/* FIXME: Run bitbang_spi_request_bus here or in programmer init? * Requesting and releasing the SPI bus is handled in here to allow the @@ -134,13 +139,12 @@ .write_aai = default_spi_write_aai, };
-#if 0 // until it is needed -static int bitbang_spi_shutdown(const struct bitbang_spi_master *master) +static int bitbang_spi_shutdown(void *data) { /* FIXME: Run bitbang_spi_release_bus here or per command? */ + free(data); return 0; } -#endif
int register_spi_bitbang_master(const struct bitbang_spi_master *master) { @@ -155,8 +159,11 @@ return ERROR_FLASHROM_BUG; }
- mst.data = master; + struct bitbang_spi_master_data *data = calloc(1, sizeof(struct bitbang_spi_master_data)); + data->mst = master; + mst.data = data; register_spi_master(&mst); + register_shutdown(bitbang_spi_shutdown, data);
/* Only mess with the bus if we're sure nobody else uses it. */ bitbang_spi_request_bus(master); diff --git a/programmer.h b/programmer.h index d66d239..1c9ef38 100644 --- a/programmer.h +++ b/programmer.h @@ -634,7 +634,7 @@ int (*read)(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len); int (*write_256)(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); int (*write_aai)(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); - const void *data; + void *data; };
int default_spi_send_command(const struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, @@ -722,7 +722,7 @@ int (*read) (struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len); int (*write) (struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); int (*erase) (struct flashctx *flash, unsigned int blockaddr, unsigned int blocklen); - const void *data; + void *data; }; int register_opaque_master(const struct opaque_master *mst);
@@ -745,7 +745,7 @@ uint16_t (*chip_readw) (const struct flashctx *flash, const chipaddr addr); uint32_t (*chip_readl) (const struct flashctx *flash, const chipaddr addr); void (*chip_readn) (const struct flashctx *flash, uint8_t *buf, const chipaddr addr, size_t len); - const void *data; + void *data; }; int register_par_master(const struct par_master *mst, const enum chipbustype buses); struct registered_master {
Attention is currently required from: Edward O'Callaghan, Angel Pons. Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49131 )
Change subject: tree/: Drop const from opaque data ptr in master definitions [alt] ......................................................................
Patch Set 1: Code-Review+2
Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/49131 )
Change subject: tree/: Drop const from opaque data ptr in master definitions [alt] ......................................................................
tree/: Drop const from opaque data ptr in master definitions [alt]
The opaque data pointer need not necessarily have constant data for the life-time of the specific master. This is because the data field purpose is for the master to use as it sees fit for managing its own internal state and therefore we should not constrain this as being RO data at init time.
BUG=none BRANCH=none TEST=builds
Change-Id: I686c3c79547e35d48f3fd0b524fc98c176dcea6e Signed-off-by: Edward O'Callaghan quasisec@google.com Reviewed-on: https://review.coreboot.org/c/flashrom/+/49131 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Sam McNally sammc@google.com --- M bitbang_spi.c M programmer.h 2 files changed, 15 insertions(+), 8 deletions(-)
Approvals: build bot (Jenkins): Verified Sam McNally: Looks good to me, approved
diff --git a/bitbang_spi.c b/bitbang_spi.c index c402119..b12d81a 100644 --- a/bitbang_spi.c +++ b/bitbang_spi.c @@ -94,13 +94,18 @@ } }
+struct bitbang_spi_master_data { + const struct bitbang_spi_master *mst; +}; + static int bitbang_spi_send_command(const struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr) { unsigned int i; - const struct bitbang_spi_master *master = flash->mst->spi.data; + const struct bitbang_spi_master_data *data = flash->mst->spi.data; + const struct bitbang_spi_master *master = data->mst;
/* FIXME: Run bitbang_spi_request_bus here or in programmer init? * Requesting and releasing the SPI bus is handled in here to allow the @@ -134,13 +139,12 @@ .write_aai = default_spi_write_aai, };
-#if 0 // until it is needed -static int bitbang_spi_shutdown(const struct bitbang_spi_master *master) +static int bitbang_spi_shutdown(void *data) { /* FIXME: Run bitbang_spi_release_bus here or per command? */ + free(data); return 0; } -#endif
int register_spi_bitbang_master(const struct bitbang_spi_master *master) { @@ -155,8 +159,11 @@ return ERROR_FLASHROM_BUG; }
- mst.data = master; + struct bitbang_spi_master_data *data = calloc(1, sizeof(struct bitbang_spi_master_data)); + data->mst = master; + mst.data = data; register_spi_master(&mst); + register_shutdown(bitbang_spi_shutdown, data);
/* Only mess with the bus if we're sure nobody else uses it. */ bitbang_spi_request_bus(master); diff --git a/programmer.h b/programmer.h index d66d239..1c9ef38 100644 --- a/programmer.h +++ b/programmer.h @@ -634,7 +634,7 @@ int (*read)(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len); int (*write_256)(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); int (*write_aai)(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); - const void *data; + void *data; };
int default_spi_send_command(const struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, @@ -722,7 +722,7 @@ int (*read) (struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len); int (*write) (struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); int (*erase) (struct flashctx *flash, unsigned int blockaddr, unsigned int blocklen); - const void *data; + void *data; }; int register_opaque_master(const struct opaque_master *mst);
@@ -745,7 +745,7 @@ uint16_t (*chip_readw) (const struct flashctx *flash, const chipaddr addr); uint32_t (*chip_readl) (const struct flashctx *flash, const chipaddr addr); void (*chip_readn) (const struct flashctx *flash, uint8_t *buf, const chipaddr addr, size_t len); - const void *data; + void *data; }; int register_par_master(const struct par_master *mst, const enum chipbustype buses); struct registered_master {
-
Edward O'Callaghan (Code Review) -
Sam McNally (Code Review)