From fbb0ce04a8306525ed02d62ae1fb4b7c0bc998b4 Mon Sep 17 00:00:00 2001
From: Richard Hughes <richard@hughsie.com>
Date: Fri, 15 Jan 2021 09:48:12 +0000
Subject: [PATCH] libflashrom: Return progress state to the library user
Change-Id: I7197572bb7f19e3bdb2bde855d70a0f50fd3854c
Signed-off-by: Richard Hughes <richard@hughsie.com>
---
flash.h | 1 +
libflashrom.c | 25 +++++++++++++++++++++++++
libflashrom.h | 11 +++++++++++
spi.c | 6 ++++++
spi25.c | 13 +++++++++++++
5 files changed, 56 insertions(+)
@@ -415,6 +415,7 @@ __attribute__((format(printf, 2, 3)));
#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 set_progress(const enum flashrom_progress_stage stage, size_t current, size_t total);
/* layout.c */
int register_include_arg(struct layout_include_args **args, char *name);
@@ -40,6 +40,8 @@
/** Pointer to log callback function. */
static flashrom_log_callback *global_log_callback = NULL;
+static flashrom_progress_callback *global_progress_callback = NULL;
+static void *global_progress_userdata = NULL;
/**
* @brief Initialize libflashrom.
@@ -95,6 +97,29 @@ int print(const enum flashrom_log_level level, const char *const fmt, ...)
return 0;
}
+/**
+ * @brief Set the progress callback function.
+ *
+ * Set a callback function which will be invoked whenever libflashrom wants
+ * to indicate the progress has chaned. This allows frontends to do whatever
+ * they see fit with such values, e.g. update a progress bar in a GUI tool.
+ *
+ * @param progress_callback Pointer to the new progress callback function.
+ * @param userdata Userdata pointer to include with the progress callback.
+ */
+void flashrom_set_progress_callback(flashrom_progress_callback *const progress_callback, void *progress_userdata)
+{
+ global_progress_callback = progress_callback;
+ global_progress_userdata = progress_userdata;
+}
+/** @private */
+void set_progress(const enum flashrom_progress_stage stage, size_t current, size_t total)
+{
+ if (global_progress_callback == NULL)
+ return;
+ global_progress_callback(stage, current, total, global_progress_userdata);
+}
+
/** @} */ /* end flashrom-general */
@@ -35,10 +35,21 @@ enum flashrom_log_level {
FLASHROM_MSG_DEBUG2 = 4,
FLASHROM_MSG_SPEW = 5,
};
+
+/** @ingroup flashrom-general */
+enum flashrom_progress_stage {
+ FLASHROM_PROGRESS_READ,
+ FLASHROM_PROGRESS_WRITE,
+};
+
/** @ingroup flashrom-general */
typedef int(flashrom_log_callback)(enum flashrom_log_level, const char *format, va_list);
void flashrom_set_log_callback(flashrom_log_callback *);
+/** @ingroup flashrom-general */
+typedef void(flashrom_progress_callback)(enum flashrom_progress_stage stage, size_t current, size_t total, void *user_data);
+void flashrom_set_progress_callback(flashrom_progress_callback *, void *);
+
/** @ingroup flashrom-query */
enum flashrom_test_state {
FLASHROM_TESTED_OK = 0,
@@ -101,6 +101,8 @@ int spi_chip_read(struct flashctx *flash, uint8_t *buf, unsigned int start,
{
int ret;
size_t to_read;
+ size_t progress_current = 0;
+ size_t progress_total = 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
@@ -110,6 +112,10 @@ int spi_chip_read(struct flashctx *flash, uint8_t *buf, unsigned int start,
ret = flash->mst->spi.read(flash, buf, start, to_read);
if (ret)
return ret;
+
+ /* Emit progress to client */
+ progress_current += to_read;
+ set_progress(FLASHROM_PROGRESS_READ, progress_current, progress_total);
}
return 0;
}
@@ -669,11 +669,17 @@ int spi_read_chunked(struct flashctx *flash, uint8_t *buf, unsigned int start,
{
int ret;
size_t to_read;
+ size_t progress_current = 0;
+ size_t progress_total = 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;
+
+ /* Emit progress to client */
+ progress_current += to_read;
+ set_progress(FLASHROM_PROGRESS_READ, progress_current, progress_total);
}
return 0;
}
@@ -693,6 +699,8 @@ int spi_write_chunked(struct flashctx *flash, const uint8_t *buf, unsigned int s
* we're OK for now.
*/
unsigned int page_size = flash->chip->page_size;
+ size_t progress_current = 0;
+ size_t progress_total = 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
@@ -717,6 +725,10 @@ int spi_write_chunked(struct flashctx *flash, const uint8_t *buf, unsigned int s
if (rc)
return rc;
}
+
+ /* Emit progress to client */
+ progress_current += lenhere;
+ set_progress(FLASHROM_PROGRESS_WRITE, progress_current, progress_total);
}
return 0;
@@ -736,6 +748,7 @@ int spi_chip_write_1(struct flashctx *flash, const uint8_t *buf, unsigned int st
for (i = start; i < start + len; i++) {
if (spi_nbyte_program(flash, i, buf + i - start, 1))
return 1;
+ set_progress(FLASHROM_PROGRESS_WRITE, i - start, len - start);
}
return 0;
}
--
2.29.2
On Wed, 16 Aug 2017 at 22:02, David Hendricks <david.hendricks@gmail.com> wrote: > The short answer is yes. Nico also briefly mentioned an idea on IRC to use a mechanism similar to flashrom_set_log_callback() to avoid complicating function signatures for the read, write, and verify functions. Dragging up an email thread from a long time ago, apologies. We're finally using libflashrom via fwupd "in production" so to speak. One glaring problem is that the UI "freezes" when we're doing operations with libflashrom, which we've worked around with just making the progress bar spin up and down so at least it looks like the program has not crashed. Of course, the correct thing to do is return the progress information because, as David says, some flash chips take a looong time to write. I've attached a patch for some initial comments, if this looks acceptable I can add some more callbacks in other flash implementations (as at the moment I've only covered SPI stuff) but I didn't want to spend time on a mega patch before I had some kind of initial thumbs-up. Comments welcome. Thanks. Richard.