Edward O'Callaghan submitted this change.

View Change


Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved
tree/: Move programmer_delay() out of programmer state machine

Handle the special cases of both serprog and ch341a_spi.
Also rewrite programmer_delay() to handle the two base
cases of zero time and no valid flashctx yet before
handling per master branching.

Additionally, modify the custom delay function pointer
signature to allow closure over the flashctx. This allows
driver specific delay implementations to recover programmer
specific opaque data within their delay implementations.
Therefore programmer specific delay functions can avoid
programmer specific globals.

Change-Id: Id059abb58b31a066a408009073912da2b224d40c
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/67393
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
---
M ch341a_spi.c
M flashrom.c
M include/programmer.h
M serprog.c
4 files changed, 68 insertions(+), 31 deletions(-)

diff --git a/ch341a_spi.c b/ch341a_spi.c
index 294a6a6..2dc983f 100644
--- a/ch341a_spi.c
+++ b/ch341a_spi.c
@@ -322,7 +322,7 @@
*ptr++ = CH341A_CMD_UIO_STM_END;
}

-static void ch341a_spi_delay(unsigned int usecs)
+static void ch341a_spi_delay(const struct flashctx *flash, unsigned int usecs)
{
/* There is space for 28 bytes instructions of 750 ns each in the CS packet (32 - 4 for the actual CS
* instructions), thus max 21 us, but we avoid getting too near to this boundary and use
@@ -419,6 +419,7 @@
.write_aai = default_spi_write_aai,
.shutdown = ch341a_spi_shutdown,
.probe_opcode = default_spi_probe_opcode,
+ .delay = ch341a_spi_delay,
};

static int ch341a_spi_init(const struct programmer_cfg *cfg)
@@ -525,5 +526,4 @@
.type = USB,
.devs.dev = devs_ch341a_spi,
.init = ch341a_spi_init,
- .delay = ch341a_spi_delay,
};
diff --git a/flashrom.c b/flashrom.c
index 9c22eb6..d0f0d2d 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -256,12 +256,21 @@

void programmer_delay(const struct flashctx *flash, unsigned int usecs)
{
- if (usecs > 0) {
- if (programmer->delay)
- programmer->delay(usecs);
- else
- internal_delay(usecs);
+ if (usecs == 0)
+ return;
+
+ if (!flash)
+ return internal_delay(usecs);
+
+ if (flash->mst->buses_supported & BUS_SPI) {
+ if (flash->mst->spi.delay)
+ return flash->mst->spi.delay(flash, usecs);
+ } else if (flash->mst->buses_supported & BUS_PARALLEL) {
+ if (flash->mst->par.delay)
+ return flash->mst->par.delay(flash, usecs);
}
+
+ return internal_delay(usecs);
}

int read_memmapped(struct flashctx *flash, uint8_t *buf, unsigned int start,
diff --git a/include/programmer.h b/include/programmer.h
index 7bcef02..8ba5781 100644
--- a/include/programmer.h
+++ b/include/programmer.h
@@ -49,8 +49,6 @@
} devs;

int (*init) (const struct programmer_cfg *cfg);
-
- void (*delay) (unsigned int usecs);
};

extern const struct programmer_entry *const programmer_table[];
@@ -314,6 +312,7 @@
int (*write_aai)(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len);
int (*shutdown)(void *data);
bool (*probe_opcode)(const struct flashctx *flash, uint8_t opcode);
+ void (*delay) (const struct flashctx *flash, unsigned int usecs);
void *data;
};

@@ -435,6 +434,7 @@
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);
int (*shutdown)(void *data);
+ void (*delay) (const struct flashctx *flash, unsigned int usecs);
void *data;
};
int register_par_master(const struct par_master *mst, const enum chipbustype buses, void *data);
diff --git a/serprog.c b/serprog.c
index a3a3db3..a7796ba 100644
--- a/serprog.c
+++ b/serprog.c
@@ -453,6 +453,8 @@
return NULL;
}

+static void serprog_delay(const struct flashctx *flash, unsigned int usecs);
+
static struct spi_master spi_master_serprog = {
.map_flash_region = serprog_map,
.features = SPI_MASTER_4BA,
@@ -464,6 +466,7 @@
.write_256 = default_spi_write_256,
.write_aai = default_spi_write_aai,
.probe_opcode = default_spi_probe_opcode,
+ .delay = serprog_delay,
};

static int sp_check_opbuf_usage(int bytes_to_be_added)
@@ -568,6 +571,27 @@
sp_do_read_n(&(buf[addrm-addr]), addrm, lenm); // FIXME: return error
}

+static void serprog_delay(const struct flashctx *flash, unsigned int usecs)
+{
+ unsigned char buf[4];
+ msg_pspew("%s usecs=%d\n", __func__, usecs);
+ if (!sp_check_commandavail(S_CMD_O_DELAY)) {
+ msg_pdbg2("serprog_delay used, but programmer doesn't support delays natively - emulating\n");
+ internal_delay(usecs);
+ return;
+ }
+ if ((sp_max_write_n) && (sp_write_n_bytes))
+ sp_pass_writen();
+ sp_check_opbuf_usage(5);
+ buf[0] = ((usecs >> 0) & 0xFF);
+ buf[1] = ((usecs >> 8) & 0xFF);
+ buf[2] = ((usecs >> 16) & 0xFF);
+ buf[3] = ((usecs >> 24) & 0xFF);
+ sp_stream_buffer_op(S_CMD_O_DELAY, 4, buf);
+ sp_opbuf_usage += 5;
+ sp_prev_was_write = 0;
+}
+
static const struct par_master par_master_serprog = {
.map_flash_region = serprog_map,
.chip_readb = serprog_chip_readb,
@@ -578,6 +602,7 @@
.chip_writew = fallback_chip_writew,
.chip_writel = fallback_chip_writel,
.chip_writen = fallback_chip_writen,
+ .delay = serprog_delay,
};

static enum chipbustype serprog_buses_supported = BUS_NONE;
@@ -941,32 +966,10 @@
return 1;
}

-static void serprog_delay(unsigned int usecs)
-{
- unsigned char buf[4];
- msg_pspew("%s usecs=%d\n", __func__, usecs);
- if (!sp_check_commandavail(S_CMD_O_DELAY)) {
- msg_pdbg2("serprog_delay used, but programmer doesn't support delays natively - emulating\n");
- internal_delay(usecs);
- return;
- }
- if ((sp_max_write_n) && (sp_write_n_bytes))
- sp_pass_writen();
- sp_check_opbuf_usage(5);
- buf[0] = ((usecs >> 0) & 0xFF);
- buf[1] = ((usecs >> 8) & 0xFF);
- buf[2] = ((usecs >> 16) & 0xFF);
- buf[3] = ((usecs >> 24) & 0xFF);
- sp_stream_buffer_op(S_CMD_O_DELAY, 4, buf);
- sp_opbuf_usage += 5;
- sp_prev_was_write = 0;
-}
-
const struct programmer_entry programmer_serprog = {
.name = "serprog",
.type = OTHER,
/* FIXME */
.devs.note = "All programmer devices speaking the serprog protocol\n",
.init = serprog_init,
- .delay = serprog_delay,
};

To view, visit change 67393. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id059abb58b31a066a408009073912da2b224d40c
Gerrit-Change-Number: 67393
Gerrit-PatchSet: 11
Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org>
Gerrit-Reviewer: Alexander Goncharov <chat@joursoir.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm@chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org>
Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net>
Gerrit-Reviewer: Peter Marheine <pmarheine@chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src@posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Nico Huber <nico.h@gmx.de>
Gerrit-MessageType: merged