Anastasia Klimchuk submitted this change.
Complete and fix progress feature implementation for all operations
Original progress reporting implemented in CB:49643 and it has some
issues, for example:
size_t start_address = start;
size_t end_address = len - start;
End address is anything but length minus start address.
update_progress(flash,
FLASHROM_PROGRESS_READ,
/*current*/ start - start_address + to_read,
/*total*/ end_address);
Total should just be length if that's how current value is computed.
---
libflashrom needs to know total size ahead of time.
That's init_progress() and changed update_progress().
It also needs to store the last current value to be able to update it.
That's stage_progress in flashrom_flashctx.
Measuring accurately amount of data which will be read/erased/written
isn't easy because things can be skipped as optimizations. The next
patch in the chain aims to address this, there are TODO/FIXME
comments there.
---
CLI shares terminal with the rest of the code and has to maintain more
state to handle that reasonably well.
Similar to CB:64668, an effort is made to keep the progress on a
single line. Non-progress output is kept track of to know when
moving to a new line cannot be avoided.
---
A script to test the CLI:
\#!/bin/bash
t=${1:-rewW}
shift
if [[ $t =~ r ]]; then
echo ">>> READ"
./flashrom -p dummy:emulate=W25Q128FV,freq=64mhz -r dump.rom --progress "$@"
echo
fi
if [[ $t =~ e ]]; then
echo ">>> ERASE"
./flashrom -p dummy:emulate=W25Q128FV,freq=64mhz -E --progress "$@"
echo
fi
if [[ $t =~ w ]]; then
echo ">>> WRITE (without erase)"
dd if=/dev/zero of=zero.rom bs=1M count=16 2> /dev/null
./flashrom -p dummy:emulate=W25Q128FV,freq=64mhz -w zero.rom --progress "$@"
echo
fi
if [[ $t =~ W ]]; then
echo ">>> WRITE (with erase)"
dd if=/dev/zero of=zero.rom bs=1M count=16 2> /dev/null
dd if=/dev/random of=random.rom bs=1M count=16 2> /dev/null
./flashrom -p dummy:emulate=W25Q128FV,freq=64mhz,image=random.rom -w zero.rom --progress "$@"
echo
fi
Co-developed-by: Anastasia Klimchuk <aklm@flashrom.org>
Co-developed-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
Change-Id: If1e40fc97f443c4f0c0501cef11cff1f3f84c051
Signed-off-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
Signed-off-by: Anastasia Klimchuk <aklm@flashrom.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/84102
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
---
M 82802ab.c
M at45db.c
M cli_classic.c
M cli_output.c
M dediprog.c
M doc/release_notes/devel.rst
M en29lv640b.c
M erasure_layout.c
M flashrom.c
M include/flash.h
M it87spi.c
M jedec.c
M libflashrom.c
M linux_mtd.c
M parade_lspcon.c
M realtek_mst_i2c_spi.c
M spi.c
M spi25.c
M sst28sf040.c
M tests/chip.c
M tests/spi25.c
M tests/tests.c
M tests/tests.h
23 files changed, 333 insertions(+), 51 deletions(-)
diff --git a/82802ab.c b/82802ab.c
index a440bd4..148042c 100644
--- a/82802ab.c
+++ b/82802ab.c
@@ -135,7 +135,7 @@
chip_writeb(flash, 0x40, dst);
chip_writeb(flash, *src++, dst++);
wait_82802ab(flash);
- update_progress(flash, FLASHROM_PROGRESS_WRITE, i + 1, len);
+ update_progress(flash, FLASHROM_PROGRESS_WRITE, 1);
}
/* FIXME: Ignore errors for now. */
diff --git a/at45db.c b/at45db.c
index 6d66a6e..c0c6eaf 100644
--- a/at45db.c
+++ b/at45db.c
@@ -551,7 +551,7 @@
msg_cerr("Writing page %u failed!\n", i);
return 1;
}
- update_progress(flash, FLASHROM_PROGRESS_WRITE, i + page_size, len);
+ update_progress(flash, FLASHROM_PROGRESS_WRITE, page_size);
}
return 0;
}
diff --git a/cli_classic.c b/cli_classic.c
index 3343438..3e9db50 100644
--- a/cli_classic.c
+++ b/cli_classic.c
@@ -1060,9 +1060,9 @@
fill_flash = &flashes[0];
- unsigned int progress_user_data[FLASHROM_PROGRESS_NR];
+ struct cli_progress cli_progress = {0};
struct flashrom_progress progress_state = {
- .user_data = progress_user_data
+ .user_data = &cli_progress
};
if (options.show_progress)
flashrom_set_progress_callback(fill_flash, &flashrom_progress_cb, &progress_state);
diff --git a/cli_output.c b/cli_output.c
index e5b829a..20295b8 100644
--- a/cli_output.c
+++ b/cli_output.c
@@ -24,6 +24,14 @@
enum flashrom_log_level verbose_screen = FLASHROM_MSG_INFO;
enum flashrom_log_level verbose_logfile = FLASHROM_MSG_DEBUG2;
+/* Enum to indicate what was the latest printed char prior to a progress indicator. */
+enum line_state {
+ NEWLINE,
+ MIDLINE,
+ PROGRESS
+};
+static enum line_state line_state = NEWLINE;
+
static FILE *logfile = NULL;
int close_logfile(void)
@@ -75,18 +83,75 @@
return "UNKNOWN";
}
+static void print_progress(const struct cli_progress *cli_progress, enum flashrom_progress_stage stage)
+{
+ if (!(cli_progress->visible_stages & (1 << stage)))
+ return;
+
+ msg_ginfo("[%s: %2u%%]", flashrom_progress_stage_to_string(stage), cli_progress->stage_pc[stage]);
+}
+
void flashrom_progress_cb(struct flashrom_flashctx *flashctx)
{
struct flashrom_progress *progress_state = flashctx->progress_state;
unsigned int pc = 0;
- unsigned int *percentages = progress_state->user_data;
- if (progress_state->current > 0 && progress_state->total > 0)
- pc = ((unsigned long long) progress_state->current * 10000llu) /
- ((unsigned long long) progress_state->total * 100llu);
- if (percentages[progress_state->stage] != pc) {
- percentages[progress_state->stage] = pc;
- msg_ginfo("[%s] %u%% complete... ", flashrom_progress_stage_to_string(progress_state->stage), pc);
+ struct cli_progress *cli_progress = progress_state->user_data;
+
+ /* The expectation is that initial progress of zero is reported before doing anything. */
+ if (progress_state->current == 0) {
+ if (!cli_progress->stage_setup) {
+ cli_progress->stage_setup = true;
+
+ /* Initialization of some stage doesn't imply that it will make any progress,
+ * only show stages which have progressed. */
+ cli_progress->visible_stages = 0;
+
+ if (line_state != NEWLINE) {
+ /* We're going to clear and replace ongoing progress output, so make a new line. */
+ msg_ginfo("\n");
+ }
+ }
+
+ cli_progress->stage_pc[progress_state->stage] = 0;
+ } else {
+ cli_progress->stage_setup = false;
+ cli_progress->visible_stages |= 1 << progress_state->stage;
}
+
+ if (progress_state->current > 0 && progress_state->total > 0)
+ pc = ((unsigned long long) progress_state->current * 100llu) /
+ ((unsigned long long) progress_state->total);
+ if (cli_progress->stage_pc[progress_state->stage] != pc) {
+ cli_progress->stage_pc[progress_state->stage] = pc;
+
+ if (line_state == PROGRESS) {
+ /* Erase previous output, because it was previous progress step. */
+ int i;
+ for (i = 0; i < 16 * FLASHROM_PROGRESS_NR; ++i)
+ msg_ginfo("\b \b");
+ } else if (line_state == MIDLINE) {
+ /* Start with new line, to preserve some other previous message */
+ msg_ginfo("\n");
+ } // Remaining option is NEWLINE, which means nothing to do: newline has been printed already.
+
+ /* The order is deliberate, the operations typically follow this sequence. */
+ print_progress(cli_progress, FLASHROM_PROGRESS_READ);
+ print_progress(cli_progress, FLASHROM_PROGRESS_ERASE);
+ print_progress(cli_progress, FLASHROM_PROGRESS_WRITE);
+
+ /* There can be output right after the progress, this acts as a separator. */
+ msg_ginfo("...");
+
+ /* Reset the flag, because now the latest message is a progress one. */
+ line_state = PROGRESS;
+ }
+}
+
+static void update_line_state(const char *fmt)
+{
+ size_t len = strlen(fmt);
+ if (len > 0)
+ line_state = (fmt[len - 1] == '\n' ? NEWLINE : MIDLINE);
}
/* Please note that level is the verbosity, not the importance of the message. */
@@ -103,6 +168,7 @@
if (level <= verbose_screen) {
ret = vfprintf(output_type, fmt, ap);
+ update_line_state(fmt);
/* msg_*spew often happens inside chip accessors in possibly
* time-critical operations. Don't slow them down by flushing. */
if (level != FLASHROM_MSG_SPEW)
@@ -111,6 +177,7 @@
if ((level <= verbose_logfile) && logfile) {
ret = vfprintf(logfile, fmt, logfile_args);
+ update_line_state(fmt);
if (level != FLASHROM_MSG_SPEW)
fflush(logfile);
}
diff --git a/dediprog.c b/dediprog.c
index aa3a1cf..ba1f9d6 100644
--- a/dediprog.c
+++ b/dediprog.c
@@ -658,7 +658,7 @@
msg_perr("SPI bulk write failed, expected %i, got %s!\n", 512, libusb_error_name(ret));
return 1;
}
- update_progress(flash, FLASHROM_PROGRESS_WRITE, i + 1, count);
+ update_progress(flash, FLASHROM_PROGRESS_WRITE, chunksize);
}
return 0;
diff --git a/doc/release_notes/devel.rst b/doc/release_notes/devel.rst
index 919d364..c542bea 100644
--- a/doc/release_notes/devel.rst
+++ b/doc/release_notes/devel.rst
@@ -32,6 +32,11 @@
it. For those platforms don't support ECAM, libpci will terminate the process by
exit.
+Progress display
+================
+
+Progress display feature is now working for all operations: read, erase, write.
+
Chipset support
===============
Added Raptor Point PCH support.
diff --git a/en29lv640b.c b/en29lv640b.c
index 4d93844..b647392 100644
--- a/en29lv640b.c
+++ b/en29lv640b.c
@@ -48,7 +48,7 @@
#endif
dst += 2;
src += 2;
- update_progress(flash, FLASHROM_PROGRESS_WRITE, i + 2, len);
+ update_progress(flash, FLASHROM_PROGRESS_WRITE, 2);
}
/* FIXME: Ignore errors for now. */
diff --git a/erasure_layout.c b/erasure_layout.c
index 7f3bb46..8b1a26f 100644
--- a/erasure_layout.c
+++ b/erasure_layout.c
@@ -304,6 +304,8 @@
msg_cerr("ERASE FAILED!\n");
}
+ update_progress(flashctx, FLASHROM_PROGRESS_ERASE, block_len);
+
// adjust curcontents
memset(curcontents+start_addr, erased_value, block_len);
// after erase make it unselected again
diff --git a/flashrom.c b/flashrom.c
index e2c60f4..88fa5d9 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -1225,6 +1225,24 @@
return chip - flashchips;
}
+static void setup_progress_from_layout(struct flashctx *flashctx,
+ enum flashrom_progress_stage stage)
+{
+ if (!flashctx->progress_callback)
+ return;
+
+ const struct flashrom_layout *const flash_layout = get_layout(flashctx);
+
+ size_t total = 0;
+ const struct romentry *entry = NULL;
+ while ((entry = layout_next_included(flash_layout, entry))) {
+ const struct flash_region *region = &entry->region;
+ total += region->end - region->start + 1;
+ }
+
+ init_progress(flashctx, stage, total);
+}
+
/**
* @brief Reads the included layout regions into a buffer.
*
@@ -1241,6 +1259,8 @@
const struct flashrom_layout *const layout = get_layout(flashctx);
const struct romentry *entry = NULL;
+ setup_progress_from_layout(flashctx, FLASHROM_PROGRESS_READ);
+
while ((entry = layout_next_included(layout, entry))) {
const struct flash_region *region = &entry->region;
const chipoff_t region_start = region->start;
@@ -1353,6 +1373,9 @@
memset(curcontents, ~ERASED_VALUE(flashctx), flash_size);
memset(newcontents, ERASED_VALUE(flashctx), flash_size);
+ setup_progress_from_layout(flashctx, FLASHROM_PROGRESS_READ);
+ setup_progress_from_layout(flashctx, FLASHROM_PROGRESS_ERASE);
+
const struct flashrom_layout *const flash_layout = get_layout(flashctx);
const struct romentry *entry = NULL;
while ((entry = layout_next_included(flash_layout, entry))) {
@@ -1389,6 +1412,10 @@
goto _ret;
}
+ setup_progress_from_layout(flashctx, FLASHROM_PROGRESS_READ);
+ setup_progress_from_layout(flashctx, FLASHROM_PROGRESS_WRITE);
+ setup_progress_from_layout(flashctx, FLASHROM_PROGRESS_ERASE);
+
const struct romentry *entry = NULL;
while ((entry = layout_next_included(flash_layout, entry))) {
ret = erase_write(flashctx, entry->region.start,
@@ -1427,6 +1454,8 @@
{
const struct romentry *entry = NULL;
+ setup_progress_from_layout(flashctx, FLASHROM_PROGRESS_READ);
+
while ((entry = layout_next_included(layout, entry))) {
const struct flash_region *region = &entry->region;
const chipoff_t region_start = region->start;
@@ -1924,6 +1953,7 @@
*/
msg_cinfo("Reading old flash chip contents... ");
if (verify_all) {
+ init_progress(flashctx, FLASHROM_PROGRESS_READ, flash_size);
if (read_flash(flashctx, oldcontents, 0, flash_size)) {
msg_cinfo("FAILED.\n");
goto _finalize_ret;
@@ -1940,12 +1970,14 @@
bool all_skipped = true;
+ msg_cinfo("Updating flash chip contents... ");
if (write_by_layout(flashctx, curcontents, newcontents, &all_skipped)) {
msg_cerr("Uh oh. Erase/write failed. ");
ret = 2;
if (verify_all) {
msg_cerr("Checking if anything has changed.\n");
msg_cinfo("Reading current flash chip contents... ");
+ init_progress(flashctx, FLASHROM_PROGRESS_READ, flash_size);
if (!read_flash(flashctx, curcontents, 0, flash_size)) {
msg_cinfo("done.\n");
if (!memcmp(oldcontents, curcontents, flash_size)) {
diff --git a/include/flash.h b/include/flash.h
index d0e55af..60c099c 100644
--- a/include/flash.h
+++ b/include/flash.h
@@ -549,6 +549,11 @@
typedef int (blockprotect_func_t)(struct flashctx *flash);
blockprotect_func_t *lookup_blockprotect_func_ptr(const struct flashchip *const chip);
+struct stage_progress {
+ size_t current;
+ size_t total;
+};
+
struct flashrom_flashctx {
struct flashchip *chip;
/* FIXME: The memory mappings should be saved in a more structured way. */
@@ -587,6 +592,7 @@
/* Progress reporting */
flashrom_progress_callback *progress_callback;
struct flashrom_progress *progress_state;
+ struct stage_progress stage_progress[FLASHROM_PROGRESS_NR];
};
/* Timing used in probe routines. ZERO is -2 to differentiate between an unset
@@ -677,6 +683,12 @@
*/
#define ERROR_FLASHROM_LIMIT -201
+struct cli_progress {
+ unsigned int stage_pc[FLASHROM_PROGRESS_NR];
+ unsigned int visible_stages; /* Bitmask of stages with non-zero progress. */
+ bool stage_setup; /* Flag to know when to reset progress data. */
+};
+
/* cli_common.c */
void print_chip_support_status(const struct flashchip *chip);
@@ -716,7 +728,8 @@
#define msg_gspew(...) print(FLASHROM_MSG_SPEW, __VA_ARGS__) /* general debug spew */
#define msg_pspew(...) print(FLASHROM_MSG_SPEW, __VA_ARGS__) /* programmer debug spew */
#define msg_cspew(...) print(FLASHROM_MSG_SPEW, __VA_ARGS__) /* chip debug spew */
-void update_progress(struct flashctx *flash, enum flashrom_progress_stage stage, size_t current, size_t total);
+void init_progress(struct flashctx *flash, enum flashrom_progress_stage stage, size_t total);
+void update_progress(struct flashctx *flash, enum flashrom_progress_stage stage, size_t increment);
/* spi.c */
struct spi_command {
@@ -730,4 +743,5 @@
int spi_send_multicommand(const struct flashctx *flash, struct spi_command *cmds);
enum chipbustype get_buses_supported(void);
+
#endif /* !__FLASH_H__ */
diff --git a/it87spi.c b/it87spi.c
index 9c64659..fddae3b 100644
--- a/it87spi.c
+++ b/it87spi.c
@@ -292,7 +292,7 @@
int ret = it8716f_spi_page_program(flash, buf, start);
if (ret)
return ret;
- update_progress(flash, FLASHROM_PROGRESS_WRITE, chip->page_size - len, chip->page_size);
+ update_progress(flash, FLASHROM_PROGRESS_WRITE, chip->page_size);
start += chip->page_size;
len -= chip->page_size;
buf += chip->page_size;
diff --git a/jedec.c b/jedec.c
index 2d18de7..df0d93a 100644
--- a/jedec.c
+++ b/jedec.c
@@ -382,7 +382,7 @@
if (write_byte_program_jedec_common(flash, src, dst))
failed = 1;
dst++, src++;
- update_progress(flash, FLASHROM_PROGRESS_WRITE, i + 1, len);
+ update_progress(flash, FLASHROM_PROGRESS_WRITE, 1);
}
if (failed)
msg_cerr(" writing sector at 0x%" PRIxPTR " failed!\n", olddst);
@@ -467,7 +467,7 @@
if (jedec_write_page(flash, buf + starthere - start, starthere, lenhere))
return 1;
- update_progress(flash, FLASHROM_PROGRESS_WRITE, i + 1, nwrites + 1);
+ update_progress(flash, FLASHROM_PROGRESS_WRITE, lenhere);
}
return 0;
diff --git a/libflashrom.c b/libflashrom.c
index 7c885cf..dfc86e6 100644
--- a/libflashrom.c
+++ b/libflashrom.c
@@ -70,16 +70,36 @@
flashctx->progress_state = progress_state;
}
/** @private */
-void update_progress(struct flashrom_flashctx *flashctx, enum flashrom_progress_stage stage, size_t current, size_t total)
+void init_progress(struct flashrom_flashctx *flashctx, enum flashrom_progress_stage stage, size_t total)
{
if (flashctx->progress_callback == NULL)
return;
- if (current > total)
- current = total;
+
+ struct stage_progress *stage_progress = &flashctx->stage_progress[stage];
+ stage_progress->current = 0;
+ stage_progress->total = total;
+
+ /* This is used to trigger callback invocation, with 0 current state and 0 increment: as an init call. */
+ update_progress(flashctx, stage, 0);
+}
+/** @private */
+void update_progress(struct flashrom_flashctx *flashctx, enum flashrom_progress_stage stage, size_t increment)
+{
+ if (flashctx->progress_callback == NULL)
+ return;
+
+ struct stage_progress *stage_progress = &flashctx->stage_progress[stage];
+
+ stage_progress->current += increment;
+ if (stage_progress->current > stage_progress->total) {
+ msg_gwarn("Fixing total value of stage %d progress on the fly.", stage);
+ stage_progress->total = stage_progress->current;
+ }
flashctx->progress_state->stage = stage;
- flashctx->progress_state->current = current;
- flashctx->progress_state->total = total;
+ flashctx->progress_state->current = stage_progress->current;
+ flashctx->progress_state->total = stage_progress->total;
+
flashctx->progress_callback(flashctx);
}
diff --git a/linux_mtd.c b/linux_mtd.c
index eea0cf2..d5fc509 100644
--- a/linux_mtd.c
+++ b/linux_mtd.c
@@ -215,7 +215,7 @@
}
i += step;
- update_progress(flash, FLASHROM_PROGRESS_READ, i, len);
+ update_progress(flash, FLASHROM_PROGRESS_READ, step);
}
return 0;
@@ -258,7 +258,7 @@
}
i += step;
- update_progress(flash, FLASHROM_PROGRESS_WRITE, i, len);
+ update_progress(flash, FLASHROM_PROGRESS_WRITE, step);
}
return 0;
@@ -295,7 +295,6 @@
__func__, ret, strerror(errno));
return 1;
}
- update_progress(flash, FLASHROM_PROGRESS_ERASE, u + data->erasesize, len);
}
return 0;
diff --git a/parade_lspcon.c b/parade_lspcon.c
index dfcf659..16283e7 100644
--- a/parade_lspcon.c
+++ b/parade_lspcon.c
@@ -354,7 +354,7 @@
for (i = 0; i < len; i += TUNNEL_PAGE_SIZE) {
ret |= parade_lspcon_map_page(fd, start + i);
ret |= parade_lspcon_read_data(fd, PAGE_ADDRESS, buf + i, min(len - i, TUNNEL_PAGE_SIZE));
- update_progress(flash, FLASHROM_PROGRESS_READ, i + TUNNEL_PAGE_SIZE, len);
+ update_progress(flash, FLASHROM_PROGRESS_READ, TUNNEL_PAGE_SIZE);
}
return ret;
@@ -395,7 +395,7 @@
for (unsigned int i = 0; i < len; i += TUNNEL_PAGE_SIZE) {
ret |= parade_lspcon_map_page(fd, start + i);
ret |= parade_lspcon_write_page(fd, buf + i, min(len - i, TUNNEL_PAGE_SIZE));
- update_progress(flash, FLASHROM_PROGRESS_WRITE, i + TUNNEL_PAGE_SIZE, len);
+ update_progress(flash, FLASHROM_PROGRESS_WRITE, TUNNEL_PAGE_SIZE);
}
ret |= parade_lspcon_enable_write_protection(fd);
diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c
index 57677ec..3a89eab 100644
--- a/realtek_mst_i2c_spi.c
+++ b/realtek_mst_i2c_spi.c
@@ -397,7 +397,7 @@
ret |= realtek_mst_i2c_execute_write(fd);
if (ret)
break;
- update_progress(flash, FLASHROM_PROGRESS_WRITE, i + RTK_PAGE_SIZE, len);
+ update_progress(flash, FLASHROM_PROGRESS_WRITE, page_len);
}
return ret;
diff --git a/spi.c b/spi.c
index 3e81da2..b2ffc33 100644
--- a/spi.c
+++ b/spi.c
@@ -104,8 +104,6 @@
{
int ret;
size_t to_read;
- size_t start_address = start;
- size_t end_address = len - start;
for (; len; len -= to_read, buf += to_read, start += to_read) {
/* Do not cross 16MiB boundaries in a single transfer.
This helps with
@@ -115,7 +113,6 @@
ret = flash->mst->spi.read(flash, buf, start, to_read);
if (ret)
return ret;
- update_progress(flash, FLASHROM_PROGRESS_READ, start - start_address + to_read, end_address);
}
return 0;
}
diff --git a/spi25.c b/spi25.c
index 6a6ee75..ab479e5 100644
--- a/spi25.c
+++ b/spi25.c
@@ -681,14 +681,12 @@
{
int ret;
size_t to_read;
- size_t start_address = start;
- size_t end_address = len - start;
for (; len; len -= to_read, buf += to_read, start += to_read) {
to_read = min(chunksize, len);
ret = spi_nbyte_read(flash, start, buf, to_read);
if (ret)
return ret;
- update_progress(flash, FLASHROM_PROGRESS_READ, start - start_address + to_read, end_address);
+ update_progress(flash, FLASHROM_PROGRESS_READ, to_read);
}
return 0;
}
@@ -708,8 +706,6 @@
* we're OK for now.
*/
unsigned int page_size = flash->chip->page_size;
- size_t start_address = start;
- size_t end_address = len - start;
/* Warning: This loop has a very unusual condition and body.
* The loop needs to go through each page with at least one affected
@@ -733,8 +729,9 @@
rc = spi_nbyte_program(flash, starthere + j, buf + starthere - start + j, towrite);
if (rc)
return rc;
+
+ update_progress(flash, FLASHROM_PROGRESS_WRITE, towrite);
}
- update_progress(flash, FLASHROM_PROGRESS_WRITE, start - start_address + lenhere, end_address);
}
return 0;
@@ -754,7 +751,7 @@
for (i = start; i < start + len; i++) {
if (spi_nbyte_program(flash, i, buf + i - start, 1))
return 1;
- update_progress(flash, FLASHROM_PROGRESS_WRITE, i - start, len - start);
+ update_progress(flash, FLASHROM_PROGRESS_WRITE, 1);
}
return 0;
}
diff --git a/sst28sf040.c b/sst28sf040.c
index 9080684..e6a4b36 100644
--- a/sst28sf040.c
+++ b/sst28sf040.c
@@ -92,7 +92,7 @@
/* wait for Toggle bit ready */
toggle_ready_jedec(flash, bios);
- update_progress(flash, FLASHROM_PROGRESS_WRITE, i + 1, len);
+ update_progress(flash, FLASHROM_PROGRESS_WRITE, 1);
}
return 0;
diff --git a/tests/chip.c b/tests/chip.c
index 604d79e..ac60663 100644
--- a/tests/chip.c
+++ b/tests/chip.c
@@ -46,6 +46,10 @@
.buf = { 0 },
};
+struct progress_user_data {
+ size_t last_seen; /* % of progress last reported, to be asserted in the progress callback. */
+};
+
static int read_chip(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len)
{
printf("Read chip called with start=0x%x, len=0x%x\n", start, len);
@@ -76,6 +80,21 @@
return 0;
}
+static void progress_callback(struct flashctx *flash) {
+ struct progress_user_data *progress_user_data = flash->progress_state->user_data;
+ if (flash->progress_state->current == 0) {
+ printf("Progress started for stage %d, initial callback call\n", flash->progress_state->stage);
+ } else {
+ /* Progress cannot go backwards. */
+ assert_true(flash->progress_state->current >= progress_user_data->last_seen);
+ }
+
+ if (flash->progress_state->current >= flash->progress_state->total)
+ printf("Progress complete for stage %d, final callback call\n", flash->progress_state->stage);
+
+ progress_user_data->last_seen = flash->progress_state->current;
+}
+
static void setup_chip(struct flashrom_flashctx *flashctx, struct flashrom_layout **layout,
struct flashchip *chip, const char *programmer_param, const struct io_mock *io)
{
@@ -210,6 +229,41 @@
teardown(&layout);
}
+void erase_chip_with_progress(void **state)
+{
+ (void) state; /* unused */
+
+ static struct io_mock_fallback_open_state data = {
+ .noc = 0,
+ .paths = { NULL },
+ };
+ const struct io_mock chip_io = {
+ .fallback_open_state = &data,
+ };
+
+ g_test_write_injector = write_chip;
+ g_test_read_injector = read_chip;
+ g_test_erase_injector[0] = block_erase_chip;
+ struct flashrom_flashctx flashctx = { 0 };
+ struct flashrom_layout *layout;
+ struct flashchip mock_chip = chip_8MiB;
+ const char *param = ""; /* Default values for all params. */
+
+ setup_chip(&flashctx, &layout, &mock_chip, param, &chip_io);
+
+ struct progress_user_data progress_user_data = {0};
+ struct flashrom_progress progress_state = {
+ .user_data = &progress_user_data
+ };
+ flashrom_set_progress_callback(&flashctx, progress_callback, &progress_state);
+
+ printf("Erase chip operation started.\n");
+ assert_int_equal(0, flashrom_flash_erase(&flashctx));
+ printf("Erase chip operation done.\n");
+
+ teardown(&layout);
+}
+
void erase_chip_with_dummyflasher_test_success(void **state)
{
(void) state; /* unused */
@@ -277,6 +331,49 @@
free(buf);
}
+void read_chip_with_progress(void **state)
+{
+ (void) state; /* unused */
+
+ static struct io_mock_fallback_open_state data = {
+ .noc = 0,
+ .paths = { NULL },
+ };
+ const struct io_mock chip_io = {
+ .fallback_open_state = &data,
+ };
+
+ g_test_write_injector = write_chip;
+ g_test_read_injector = read_chip;
+ g_test_erase_injector[0] = block_erase_chip;
+ struct flashrom_flashctx flashctx = { 0 };
+ struct flashrom_layout *layout;
+ struct flashchip mock_chip = chip_8MiB;
+ const char *param = ""; /* Default values for all params. */
+
+ setup_chip(&flashctx, &layout, &mock_chip, param, &chip_io);
+
+ struct progress_user_data progress_user_data = {0};
+ struct flashrom_progress progress_state = {
+ .user_data = &progress_user_data
+ };
+ flashrom_set_progress_callback(&flashctx, progress_callback, &progress_state);
+
+ const char *const filename = "read_chip.test";
+ unsigned long size = mock_chip.total_size * 1024;
+ unsigned char *buf = calloc(size, sizeof(unsigned char));
+ assert_non_null(buf);
+
+ printf("Read chip operation started.\n");
+ assert_int_equal(0, flashrom_image_read(&flashctx, buf, size));
+ assert_int_equal(0, write_buf_to_file(buf, size, filename));
+ printf("Read chip operation done.\n");
+
+ teardown(&layout);
+
+ free(buf);
+}
+
void read_chip_with_dummyflasher_test_success(void **state)
{
(void) state; /* unused */
@@ -365,6 +462,49 @@
free(newcontents);
}
+void write_chip_with_progress(void **state)
+{
+ (void) state; /* unused */
+
+ static struct io_mock_fallback_open_state data = {
+ .noc = 0,
+ .paths = { NULL },
+ };
+ const struct io_mock chip_io = {
+ .fallback_open_state = &data,
+ };
+
+ g_test_write_injector = write_chip;
+ g_test_read_injector = read_chip;
+ g_test_erase_injector[0] = block_erase_chip;
+ struct flashrom_flashctx flashctx = { 0 };
+ struct flashrom_layout *layout;
+ struct flashchip mock_chip = chip_8MiB;
+ const char *param = ""; /* Default values for all params. */
+
+ setup_chip(&flashctx, &layout, &mock_chip, param, &chip_io);
+
+ struct progress_user_data progress_user_data = {0};
+ struct flashrom_progress progress_state = {
+ .user_data = &progress_user_data
+ };
+ flashrom_set_progress_callback(&flashctx, progress_callback, &progress_state);
+
+ const char *const filename = "-";
+ unsigned long size = mock_chip.total_size * 1024;
+ uint8_t *const newcontents = malloc(size);
+ assert_non_null(newcontents);
+
+ printf("Write chip operation started.\n");
+ assert_int_equal(0, read_buf_from_file(newcontents, size, filename));
+ assert_int_equal(0, flashrom_image_write(&flashctx, newcontents, size, NULL));
+ printf("Write chip operation done.\n");
+
+ teardown(&layout);
+
+ free(newcontents);
+}
+
void write_chip_with_dummyflasher_test_success(void **state)
{
(void) state; /* unused */
diff --git a/tests/spi25.c b/tests/spi25.c
index 8729593..a442c7b 100644
--- a/tests/spi25.c
+++ b/tests/spi25.c
@@ -55,7 +55,9 @@
*/
return __real_spi_send_command(flash, writecnt, readcnt, writearr, readarr);
- check_expected_ptr(flash);
+ if (!flash->progress_callback)
+ check_expected_ptr(flash);
+
assert_int_equal(writecnt, mock_type(int));
assert_int_equal(writearr[0], mock_type(int));
@@ -71,22 +73,22 @@
{
struct flashrom_progress *progress_state = flashctx->progress_state;
uint32_t *cnt = (uint32_t *) progress_state->user_data;
- assert_int_equal(0x300, progress_state->total);
+ assert_int_equal(0x400, progress_state->total);
switch (*cnt) {
case 0:
- assert_int_equal(0x100, progress_state->current);
+ assert_int_equal(0x0, progress_state->current);
break;
case 1:
- assert_int_equal(0x200, progress_state->current);
+ assert_int_equal(0x100, progress_state->current);
break;
case 2:
- assert_int_equal(0x300, progress_state->current);
+ assert_int_equal(0x200, progress_state->current);
break;
case 3:
assert_int_equal(0x300, progress_state->current);
break;
case 4:
- assert_int_equal(0x300, progress_state->current);
+ assert_int_equal(0x400, progress_state->current);
break;
default:
fail();
@@ -94,7 +96,7 @@
(*cnt)++;
}
-void spi_read_chunked_test_success(void **state)
+void default_spi_read_test_success(void **state)
{
(void) state; /* unused */
uint8_t buf[0x400] = { 0x0 };
@@ -115,15 +117,16 @@
.user_data = (void *) &cnt,
};
flashrom_set_progress_callback(&flashctx, spi_read_progress_cb, &progress_state);
+ init_progress(&flashctx, FLASHROM_PROGRESS_READ, 0x400);
for (int i = 0; i < 4; i++) {
- expect_memory(__wrap_spi_send_command, flash,
- &flashctx, sizeof(flashctx));
will_return(__wrap_spi_send_command, JEDEC_WRDI);
will_return(__wrap_spi_send_command, JEDEC_READ);
will_return(__wrap_spi_send_command, max_data_read);
}
- assert_int_equal(0, spi_chip_read(&flashctx, buf, offset, sizeof(buf)));
+ assert_int_equal(0, default_spi_read(&flashctx, buf, offset, sizeof(buf)));
assert_int_equal(5, cnt);
+
+ flashrom_set_progress_callback(&flashctx, NULL, NULL);
}
void spi_write_enable_test_success(void **state)
diff --git a/tests/tests.c b/tests/tests.c
index 3fee056..00b4665 100644
--- a/tests/tests.c
+++ b/tests/tests.c
@@ -441,7 +441,7 @@
const struct CMUnitTest spi25_tests[] = {
cmocka_unit_test(spi_write_enable_test_success),
cmocka_unit_test(spi_write_disable_test_success),
- cmocka_unit_test(spi_read_chunked_test_success),
+ cmocka_unit_test(default_spi_read_test_success),
cmocka_unit_test(probe_spi_rdid_test_success),
cmocka_unit_test(probe_spi_rdid4_test_success),
cmocka_unit_test(probe_spi_rems_test_success),
@@ -495,10 +495,13 @@
const struct CMUnitTest chip_tests[] = {
cmocka_unit_test(erase_chip_test_success),
+ cmocka_unit_test(erase_chip_with_progress),
cmocka_unit_test(erase_chip_with_dummyflasher_test_success),
cmocka_unit_test(read_chip_test_success),
+ cmocka_unit_test(read_chip_with_progress),
cmocka_unit_test(read_chip_with_dummyflasher_test_success),
cmocka_unit_test(write_chip_test_success),
+ cmocka_unit_test(write_chip_with_progress),
cmocka_unit_test(write_chip_with_dummyflasher_test_success),
cmocka_unit_test(write_chip_feature_no_erase),
cmocka_unit_test(write_nonaligned_region_with_dummyflasher_test_success),
diff --git a/tests/tests.h b/tests/tests.h
index 100bda1..dba33fb 100644
--- a/tests/tests.h
+++ b/tests/tests.h
@@ -34,7 +34,7 @@
/* spi25.c */
void spi_write_enable_test_success(void **state);
void spi_write_disable_test_success(void **state);
-void spi_read_chunked_test_success(void **state);
+void default_spi_read_test_success(void **state);
void probe_spi_rdid_test_success(void **state);
void probe_spi_rdid4_test_success(void **state);
void probe_spi_rems_test_success(void **state);
@@ -82,10 +82,13 @@
/* chip.c */
void erase_chip_test_success(void **state);
+void erase_chip_with_progress(void **state);
void erase_chip_with_dummyflasher_test_success(void **state);
void read_chip_test_success(void **state);
+void read_chip_with_progress(void **state);
void read_chip_with_dummyflasher_test_success(void **state);
void write_chip_test_success(void **state);
+void write_chip_with_progress(void **state);
void write_chip_with_dummyflasher_test_success(void **state);
void write_chip_feature_no_erase(void **state);
void write_nonaligned_region_with_dummyflasher_test_success(void **state);
To view, visit change 84102. To unsubscribe, or for help writing mail filters, visit settings.