Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/53947 )
Change subject: linux_mtd: move global state into programmer data field ......................................................................
linux_mtd: move global state into programmer data field
BUG=b:161951062 BRANCH=none TEST=builds, reading /dev/mtd0 on Oak succeeds
Change-Id: I5ce6900e4892ed5687cfddb245dfe5461a3e2e84 Signed-off-by: Nikolai Artemiev nartemiev@google.com Reviewed-on: https://review.coreboot.org/c/flashrom/+/53947 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Anastasia Klimchuk aklm@chromium.org Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Edward O'Callaghan quasisec@chromium.org --- M linux_mtd.c 1 file changed, 65 insertions(+), 45 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved Anastasia Klimchuk: Looks good to me, but someone else must approve
diff --git a/linux_mtd.c b/linux_mtd.c index 22702e9..7cdba6c 100644 --- a/linux_mtd.c +++ b/linux_mtd.c @@ -31,16 +31,16 @@ #define LINUX_DEV_ROOT "/dev" #define LINUX_MTD_SYSFS_ROOT "/sys/class/mtd"
-static FILE *dev_fp = NULL; - -static int mtd_device_is_writeable; - -static int mtd_no_erase; - -/* Size info is presented in bytes in sysfs. */ -static unsigned long int mtd_total_size; -static unsigned long int mtd_numeraseregions; -static unsigned long int mtd_erasesize; /* only valid if numeraseregions is 0 */ +struct linux_mtd_data { + FILE *dev_fp; + int mtd_device_is_writeable; + int mtd_no_erase; + /* Size info is presented in bytes in sysfs. */ + unsigned long int mtd_total_size; + unsigned long int mtd_numeraseregions; + /* only valid if numeraseregions is 0 */ + unsigned long int mtd_erasesize; +};
/* read a string from a sysfs file and sanitize it */ static int read_sysfs_string(const char *sysfs_path, const char *filename, char *buf, int len) @@ -120,7 +120,7 @@ }
/* returns 0 to indicate success, non-zero to indicate error */ -static int get_mtd_info(const char *sysfs_path) +static int get_mtd_info(const char *sysfs_path, struct linux_mtd_data *data) { unsigned long int tmp; char mtd_device_name[32]; @@ -130,10 +130,10 @@ return 1; if (tmp & MTD_WRITEABLE) { /* cache for later use by write function */ - mtd_device_is_writeable = 1; + data->mtd_device_is_writeable = 1; } if (tmp & MTD_NO_ERASE) { - mtd_no_erase = 1; + data->mtd_no_erase = 1; }
/* Device name */ @@ -141,55 +141,59 @@ return 1;
/* Total size */ - if (read_sysfs_int(sysfs_path, "size", &mtd_total_size)) + if (read_sysfs_int(sysfs_path, "size", &data->mtd_total_size)) return 1; - if (popcnt(mtd_total_size) != 1) { + if (popcnt(data->mtd_total_size) != 1) { msg_perr("MTD size is not a power of 2\n"); return 1; }
/* Erase size */ - if (read_sysfs_int(sysfs_path, "erasesize", &mtd_erasesize)) + if (read_sysfs_int(sysfs_path, "erasesize", &data->mtd_erasesize)) return 1; - if (popcnt(mtd_erasesize) != 1) { + if (popcnt(data->mtd_erasesize) != 1) { msg_perr("MTD erase size is not a power of 2\n"); return 1; }
/* Erase regions */ - if (read_sysfs_int(sysfs_path, "numeraseregions", &mtd_numeraseregions)) + if (read_sysfs_int(sysfs_path, "numeraseregions", &data->mtd_numeraseregions)) return 1; - if (mtd_numeraseregions != 0) { + if (data->mtd_numeraseregions != 0) { msg_perr("Non-uniform eraseblock size is unsupported.\n"); return 1; }
msg_pdbg("%s: device_name: "%s", is_writeable: %d, " "numeraseregions: %lu, total_size: %lu, erasesize: %lu\n", - __func__, mtd_device_name, mtd_device_is_writeable, - mtd_numeraseregions, mtd_total_size, mtd_erasesize); + __func__, mtd_device_name, data->mtd_device_is_writeable, + data->mtd_numeraseregions, data->mtd_total_size, data->mtd_erasesize);
return 0; }
static int linux_mtd_probe(struct flashctx *flash) { - if (mtd_no_erase) + struct linux_mtd_data *data = flash->mst->opaque.data; + + if (data->mtd_no_erase) flash->chip->feature_bits |= FEATURE_NO_ERASE; flash->chip->tested = TEST_OK_PREW; - flash->chip->total_size = mtd_total_size / 1024; /* bytes -> kB */ - flash->chip->block_erasers[0].eraseblocks[0].size = mtd_erasesize; - flash->chip->block_erasers[0].eraseblocks[0].count = mtd_total_size / mtd_erasesize; + flash->chip->total_size = data->mtd_total_size / 1024; /* bytes -> kB */ + flash->chip->block_erasers[0].eraseblocks[0].size = data->mtd_erasesize; + flash->chip->block_erasers[0].eraseblocks[0].count = + data->mtd_total_size / data->mtd_erasesize; return 1; }
static int linux_mtd_read(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len) { + struct linux_mtd_data *data = flash->mst->opaque.data; unsigned int eb_size = flash->chip->block_erasers[0].eraseblocks[0].size; unsigned int i;
- if (fseek(dev_fp, start, SEEK_SET) != 0) { + if (fseek(data->dev_fp, start, SEEK_SET) != 0) { msg_perr("Cannot seek to 0x%06x: %s\n", start, strerror(errno)); return 1; } @@ -203,7 +207,7 @@ unsigned int step = eb_size - ((start + i) % eb_size); step = min(step, len - i);
- if (fread(buf + i, step, 1, dev_fp) != 1) { + if (fread(buf + i, step, 1, data->dev_fp) != 1) { msg_perr("Cannot read 0x%06x bytes at 0x%06x: %s\n", step, start + i, strerror(errno)); return 1; @@ -219,13 +223,14 @@ static int linux_mtd_write(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len) { + struct linux_mtd_data *data = flash->mst->opaque.data; unsigned int chunksize = flash->chip->block_erasers[0].eraseblocks[0].size; unsigned int i;
- if (!mtd_device_is_writeable) + if (!data->mtd_device_is_writeable) return 1;
- if (fseek(dev_fp, start, SEEK_SET) != 0) { + if (fseek(data->dev_fp, start, SEEK_SET) != 0) { msg_perr("Cannot seek to 0x%06x: %s\n", start, strerror(errno)); return 1; } @@ -240,12 +245,12 @@ unsigned int step = chunksize - ((start + i) % chunksize); step = min(step, len - i);
- if (fwrite(buf + i, step, 1, dev_fp) != 1) { + if (fwrite(buf + i, step, 1, data->dev_fp) != 1) { msg_perr("Cannot write 0x%06x bytes at 0x%06x\n", step, start + i); return 1; }
- if (fflush(dev_fp) == EOF) { + if (fflush(data->dev_fp) == EOF) { msg_perr("Failed to flush buffer: %s\n", strerror(errno)); return 1; } @@ -259,28 +264,29 @@ static int linux_mtd_erase(struct flashctx *flash, unsigned int start, unsigned int len) { + struct linux_mtd_data *data = flash->mst->opaque.data; uint32_t u;
- if (mtd_no_erase) { + if (data->mtd_no_erase) { msg_perr("%s: device does not support erasing. Please file a " "bug report at flashrom@flashrom.org\n", __func__); return 1; }
- if (mtd_numeraseregions != 0) { + if (data->mtd_numeraseregions != 0) { /* TODO: Support non-uniform eraseblock size using use MEMGETREGIONCOUNT/MEMGETREGIONINFO ioctls */ msg_perr("%s: mtd_numeraseregions must be 0\n", __func__); return 1; }
- for (u = 0; u < len; u += mtd_erasesize) { + for (u = 0; u < len; u += data->mtd_erasesize) { struct erase_info_user erase_info = { .start = start + u, - .length = mtd_erasesize, + .length = data->mtd_erasesize, };
- if (ioctl(fileno(dev_fp), MEMERASE, &erase_info) == -1) { + if (ioctl(fileno(data->dev_fp), MEMERASE, &erase_info) == -1) { msg_perr("%s: ioctl: %s\n", __func__, strerror(errno)); return 1; } @@ -300,7 +306,7 @@ };
/* Returns 0 if setup is successful, non-zero to indicate error */ -static int linux_mtd_setup(int dev_num) +static int linux_mtd_setup(int dev_num, struct linux_mtd_data *data) { char sysfs_path[32]; int ret = 1; @@ -332,15 +338,15 @@ /* so far so good, get more info from other files in this dir */ if (snprintf(sysfs_path, sizeof(sysfs_path), "%s/mtd%d/", LINUX_MTD_SYSFS_ROOT, dev_num) < 0) goto linux_mtd_setup_exit; - if (get_mtd_info(sysfs_path)) + if (get_mtd_info(sysfs_path, data)) goto linux_mtd_setup_exit;
/* open file stream and go! */ - if ((dev_fp = fopen(dev_path, "r+")) == NULL) { + if ((data->dev_fp = fopen(dev_path, "r+")) == NULL) { msg_perr("Cannot open file stream for %s\n", dev_path); goto linux_mtd_setup_exit; } - ret = setvbuf(dev_fp, NULL, _IONBF, 0); + ret = setvbuf(data->dev_fp, NULL, _IONBF, 0); if (ret) msg_pwarn("Failed to set MTD device to unbuffered: %d\n", ret);
@@ -353,10 +359,11 @@
static int linux_mtd_shutdown(void *data) { - if (dev_fp != NULL) { - fclose(dev_fp); - dev_fp = NULL; + struct linux_mtd_data *mtd_data = data; + if (mtd_data->dev_fp != NULL) { + fclose(mtd_data->dev_fp); } + free(data);
return 0; } @@ -366,6 +373,7 @@ char *param; int dev_num = 0; int ret = 1; + struct linux_mtd_data *data = NULL;
param = extract_programmer_param("dev"); if (param) { @@ -398,11 +406,23 @@ goto linux_mtd_init_exit; }
- if (linux_mtd_setup(dev_num)) + data = calloc(1, sizeof(*data)); + if (!data) { + msg_perr("Unable to allocate memory for linux_mtd_data\n"); goto linux_mtd_init_exit; + }
- if (register_shutdown(linux_mtd_shutdown, NULL)) + /* Get MTD info and store it in `data` */ + if (linux_mtd_setup(dev_num, data)) { + free(data); goto linux_mtd_init_exit; + } + + programmer_linux_mtd.data = data; + if (register_shutdown(linux_mtd_shutdown, (void *)data)) { + free(data); + goto linux_mtd_init_exit; + }
register_opaque_master(&programmer_linux_mtd);
-
Edward O'Callaghan (Code Review)