So, my patches are here https://github.com/XVilka/flashrom/commits/libflashrom_layout - feel free to use them as you wish, if you want so. Best regards, Anton Kochkov.
On Fri, Jun 19, 2015 at 2:54 PM, Антон Кочков <[email protected]> wrote: > Sorry again, that was old tree, I will rearrange patches and will push > updated version in a hour or so. > Best regards, > Anton Kochkov. > > > On Fri, Jun 19, 2015 at 2:50 PM, Антон Кочков <[email protected]> wrote: >> Uh, forgot to add, that loading layout will be quite useful too, in the API >> and >> adding romentry_t* argument for all fl_image_* functions. >> Best regards, >> Anton Kochkov. >> >> >> On Fri, Jun 19, 2015 at 2:43 PM, Антон Кочков <[email protected]> >> wrote: >>> Hi! >>> I've done also some rebasing of the libflashrom patches before (on top >>> of the layout branch from the stefanct) >>> https://github.com/XVilka/flashrom/commits/libflashrom >>> >>> May be you'll find something useful here. >>> Best regards, >>> Anton Kochkov. >>> >>> >>> On Thu, Jun 18, 2015 at 7:45 AM, Urja Rannikko <[email protected]> wrote: >>>> Hi, >>>> >>>> >>>> On Fri, Jun 12, 2015 at 12:26 AM, Łukasz Dmitrowski >>>> <[email protected]> wrote: >>>>> Hello, >>>>> >>>>> My name is Łukasz Dmitrowski and I am one of coreboot/flashrom GSoC >>>>> students >>>>> this year. You can read about my project here: >>>>> http://blogs.coreboot.org/blog/author/dmitro/. >>>>> >>>>> One of my first steps is updating and extending Nico Hubers libflashrom >>>>> patch. Work is still in progress, but some changes are already made, so I >>>>> thought that it will be good idea to send a patch just for review to know >>>>> if >>>>> I am going in a good direction. I want to improve my skills and provide >>>>> best >>>>> code quality as I can, so I am open for criticism - I am here to learn :) >>>>> >>>> I'm here essentially trying out reviewing stuff, so expect more from >>>> somebody a bit more experienced, but I heard my comments would be >>>> useful, so moving on :) >>>> >>>>> Patch summary: >>>>> >>>> First thing i have to say about the patch is that it has been >>>> malformed, most likely line wrapped by your email client. If you cant >>>> properly include the patch inline, attaching a .patch would be okay. >>>> (Also turn off that HTML output if you can... although it wasnt used >>>> for anything, but still.). So I didnt do any compile testing on this >>>> stuff because of this. >>>> >>>>> Makefile: I added cli_common.o and print.o to LIB_OBJS - it solves >>>>> undefined >>>>> references in current libflashrom version, but spoils standard flashrom >>>>> build with 'make', I did not investigated it yet as my focus is to extend >>>>> library with new functions and update already existing, need to discuss it >>>>> with my mentor >>>>> >>>>> cli_classic.c, cli_output.c, flash.h: Only Nico Hubers changes here, >>>>> didn't >>>>> touch it >>>> I might comment on any part of the patch because I havent reviewed the >>>> original and it's only about the code. >>>> >>>>> >>>>> libflashrom.c, libflashrom.h: I added 8 functions and made a few changes >>>>> to >>>>> existing ones, as some of used functions are static, or does not exist. >>>>> >>>>> New functions: >>>>> >>>>> int fl_supported_programmers(const char **supported_programmers); >>>>> int fl_supported_flash_chips(fl_flashchip_info_t *fchips); >>>>> int fl_supported_boards(fl_board_info_t *boards); >>>>> int fl_supported_chipsets(fl_chipset_info_t *chipsets); >>>>> int fl_supported_programmers_number(); >>>>> int fl_supported_flash_chips_number(); >>>>> int fl_supported_boards_number(); >>>>> int fl_supported_chipsets_number(); >>>>> >>>>> New types: >>>>> fl_flashchip_info_t - smaller version of struct flashchip >>>>> fl_board_info_t - smaller version of struct board_info >>>>> fl_chipset_info_t - smaller version of struct penable >>>>> fl_test_state - copy of enum test_state >>>>> >>>>> So we have 4 functions that return a list of supported programmers, chips, >>>>> boards and chipsets. Next 4 functions return number of supported hardware >>>>> of >>>>> certain type. For example, to obtain a list of supported boards, you can >>>>> create an array of structures fl_board_info of suitable size (the size is >>>>> known - fl_supported_boards_number()), then pass it to >>>>> fl_supported_boards(fl_board_info_t *boards) and you will have it filled >>>>> with data. >>>> My opinion is that the _number functions make the API unnecessarily >>>> complex to use. >>>> I'm suggesting like this: >>>> fl_flashchip_info_t * fl_supported_flash_chips(void); >>>> Have the call allocate the table using a not specified allocator >>>> (really just malloc for now, but we're not telling), and freed with a >>>> function like >>>> int fl_support_info_free(void*p); >>>> This would allow implementation using const tables or allocation, >>>> whatever we like. >>>> (if (p == const_table) return 0; in our _free() if mixed.) >>>> >>>>> >>>>> Regarding changes in Nico Hubers functions: I made temporary comments for >>>>> every change and marked it with LD_CHANGE_NOTE. Every comment contains >>>>> previous code and reason why it has been commented out. >>>>> >>>>> Thanks in advance for your insights! >>>>> >>>>> Regards, >>>>> Lukasz Dmitrowski >>>>> >>>>> --- >>>>> >>>>> Signed-off-by: Łukasz Dmitrowski <[email protected]> >>>>> >>>>> Index: Makefile >>>>> =================================================================== >>>>> --- Makefile (revision 1891) >>>>> +++ Makefile (working copy) >>>>> @@ -374,7 +374,7 @@ >>>>> >>>>> ############################################################################### >>>>> # Library code. >>>>> >>>>> -LIB_OBJS = layout.o flashrom.o udelay.o programmer.o helpers.o >>>>> +LIB_OBJS = libflashrom.o layout.o flashrom.o udelay.o programmer.o >>>>> helpers.o cli_common.o print.o >>>>> >>>>> >>>>> ############################################################################### >>>>> # Frontend related stuff. >>>>> Index: cli_classic.c >>>>> =================================================================== >>>>> --- cli_classic.c (revision 1891) >>>>> +++ cli_classic.c (working copy) >>>>> @@ -30,6 +30,7 @@ >>>>> #include "flash.h" >>>>> #include "flashchips.h" >>>>> #include "programmer.h" >>>>> +#include "libflashrom.h" >>>>> >>>>> static void cli_classic_usage(const char *name) >>>>> { >>>>> @@ -135,6 +136,8 @@ >>>>> char *tempstr = NULL; >>>>> char *pparam = NULL; >>>>> >>>>> + fl_set_log_callback((fl_log_callback_t *)&fl_print_cb); >>>>> + >>>>> print_version(); >>>>> print_banner(); >>>>> >>>>> Index: cli_output.c >>>>> =================================================================== >>>>> --- cli_output.c (revision 1891) >>>>> +++ cli_output.c (working copy) >>>>> @@ -71,9 +71,8 @@ >>>>> #endif /* !STANDALONE */ >>>>> >>>>> /* Please note that level is the verbosity, not the importance of the >>>>> message. */ >>>>> -int print(enum msglevel level, const char *fmt, ...) >>>>> +int fl_print_cb(enum msglevel level, const char *fmt, va_list ap) >>>>> { >>>>> - va_list ap; >>>>> int ret = 0; >>>>> FILE *output_type = stdout; >>>>> >>>>> @@ -81,9 +80,7 @@ >>>>> output_type = stderr; >>>>> >>>>> if (level <= verbose_screen) { >>>>> - va_start(ap, fmt); >>>>> ret = vfprintf(output_type, fmt, ap); >>>>> - va_end(ap); >>>>> /* msg_*spew often happens inside chip accessors in possibly >>>>> * time-critical operations. Don't slow them down by flushing. */ >>>>> if (level != MSG_SPEW) >>>>> @@ -91,9 +88,7 @@ >>>>> } >>>>> #ifndef STANDALONE >>>>> if ((level <= verbose_logfile) && logfile) { >>>>> - va_start(ap, fmt); >>>>> ret = vfprintf(logfile, fmt, ap); >>>>> - va_end(ap); >>>>> if (level != MSG_SPEW) >>>>> fflush(logfile); >>>>> } >>>>> Index: flash.h >>>>> =================================================================== >>>>> --- flash.h (revision 1891) >>>>> +++ flash.h (working copy) >>>>> @@ -31,6 +31,7 @@ >>>>> #include <stdint.h> >>>>> #include <stddef.h> >>>>> #include <stdbool.h> >>>>> +#include <stdarg.h> >>>>> #if IS_WINDOWS >>>>> #include <windows.h> >>>>> #undef min >>>>> @@ -317,6 +318,7 @@ >>>>> MSG_DEBUG2 = 4, >>>>> MSG_SPEW = 5, >>>>> }; >>>>> +int fl_print_cb(enum msglevel level, const char *fmt, va_list ap); >>>>> /* Let gcc and clang check for correct printf-style format strings. */ >>>>> int print(enum msglevel level, const char *fmt, ...) >>>>> #ifdef __MINGW32__ >>>>> Index: libflashrom.c >>>>> =================================================================== >>>>> --- libflashrom.c (revision 0) >>>>> +++ libflashrom.c (working copy) >>>>> @@ -0,0 +1,603 @@ >>>>> +/* >>>>> + * This file is part of the flashrom project. >>>>> + * >>>>> + * Copyright (C) 2012 secunet Security Networks AG >>>>> + * (Written by Nico Huber <[email protected]> for secunet) >>>>> + * >>>>> + * This program is free software; you can redistribute it and/or modify >>>>> + * it under the terms of the GNU General Public License as published by >>>>> + * the Free Software Foundation; either version 2 of the License, or >>>>> + * (at your option) any later version. >>>>> + * >>>>> + * This program is distributed in the hope that it will be useful, >>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>>>> + * GNU General Public License for more details. >>>>> + * >>>>> + * You should have received a copy of the GNU General Public License >>>>> + * along with this program; if not, write to the Free Software >>>>> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 >>>>> USA >>>>> + */ >>>>> +/** >>>>> + * @mainpage >>>>> + * >>>>> + * Have a look at the Modules section for a function reference. >>>>> + */ >>>>> + >>>>> +#include <stdlib.h> >>>>> +#include <string.h> >>>>> +#include <stdarg.h> >>>>> + >>>>> +#include "flash.h" >>>>> +#include "programmer.h" >>>>> +#include "libflashrom.h" >>>>> + >>>>> +/** >>>>> + * @defgroup fl-general General >>>>> + * @{ >>>>> + */ >>>>> + >>>>> +/** Pointer to log callback function. */ >>>>> +static fl_log_callback_t *fl_log_callback = NULL; >>>>> + >>>>> +/** >>>>> + * @brief Initialize libflashrom. >>>>> + * >>>>> + * @param perform_selfcheck If not zero, perform a self check. >>>>> + * @return 0 on success >>>>> + */ >>>>> +int fl_init(const int perform_selfcheck) >>>>> +{ >>>>> + if (perform_selfcheck && selfcheck()) >>>>> + return 1; >>>>> + myusec_calibrate_delay(); >>>>> + return 0; >>>>> +} >>>>> + >>>>> +/** >>>>> + * @brief Shut down libflashrom. >>>>> + * @return 0 on success >>>>> + */ >>>>> +int fl_shutdown(void) >>>>> +{ >>>>> + return 0; /* TODO: nothing to do? */ >>>> I'm pretty sure there is, but need to investigate more. >>>> >>>>> +} >>>>> + >>>>> +/* TODO: fl_set_loglevel()? do we need it? >>>>> + For now, let the user decide in his callback. */ >>>>> + >>>>> +/** >>>>> + * @brief Set the log callback function. >>>>> + * >>>>> + * Set a callback function which will be invoked whenever libflashrom >>>>> wants >>>>> + * to output messages. This allows frontends to do whatever they see fit >>>>> with >>>>> + * such messages, e.g. write them to syslog, or to file, or print them >>>>> in a >>>>> + * GUI window, etc. >>>>> + * >>>>> + * @param log_callback Pointer to the new log callback function. >>>>> + */ >>>>> +void fl_set_log_callback(fl_log_callback_t *const log_callback) >>>>> +{ >>>>> + fl_log_callback = log_callback; >>>>> +} >>>>> +/** @private */ >>>>> +int print(const enum msglevel level, const char *const fmt, ...) >>>>> +{ >>>>> + if (fl_log_callback) { >>>>> + int ret; >>>>> + va_list args; >>>>> + va_start(args, fmt); >>>>> + ret = fl_log_callback(level, fmt, args); >>>>> + va_end(args); >>>>> + return ret; >>>>> + } >>>>> + return 0; >>>>> +} >>>>> + >>>>> +/** @} */ /* end fl-general */ >>>>> + >>>>> + >>>>> + >>>>> +/** >>>>> + * @defgroup fl-query Querying >>>>> + * @{ >>>>> + */ >>>>> + >>>>> +int fl_supported_programmers(const char **supported_programmers) >>>>> +{ >>>>> + int ret = 0; >>>>> + enum programmer p = 0; >>>>> + >>>>> + if (supported_programmers != NULL) { >>>>> + for (; p < PROGRAMMER_INVALID; ++p) >>>>> + supported_programmers[p] = >>>>> programmer_table[p].name; >>>>> + } else { >>>>> + ret = 1; >>>>> + } >>>>> + >>>>> + return ret; >>>>> +} >>>>> + >>>>> +int fl_supported_programmers_number() >>>>> +{ >>>>> + return PROGRAMMER_INVALID; >>>>> +} >>>>> + >>>>> +int fl_supported_flash_chips(fl_flashchip_info_t *fchips) >>>>> +{ >>>>> + int ret = 0; >>>>> + int i = 0; >>>>> + >>>>> + if (fchips != NULL) { >>>>> + for (; i < flashchips_size; ++i) { >>>>> + fchips[i].vendor = flashchips[i].vendor; >>>>> + fchips[i].name = flashchips[i].name; >>>>> + fchips[i].tested.erase = >>>>> flashchips[i].tested.erase; >>>>> + fchips[i].tested.probe = >>>>> flashchips[i].tested.probe; >>>>> + fchips[i].tested.read = >>>>> flashchips[i].tested.read; >>>>> + fchips[i].tested.write = >>>>> flashchips[i].tested.write; >>>>> + fchips[i].total_size = flashchips[i].total_size; >>>>> + } >>>>> + } else { >>>>> + ret = 1; >>>>> + } >>>>> + >>>>> + return ret; >>>>> +} >>>>> + >>>>> +int fl_supported_flash_chips_number() >>>>> +{ >>>>> + return flashchips_size; >>>>> +} >>>>> + >>>>> +int fl_supported_boards(fl_board_info_t *boards) >>>>> +{ >>>>> + int ret = 0; >>>>> + const struct board_info *binfo = boards_known; >>>>> + >>>>> + if (boards != NULL) { >>>>> + while (binfo->vendor != NULL) { >>>>> + boards->vendor = binfo->vendor; >>>>> + boards->name = binfo->name; >>>>> + boards->working = binfo->working; >>>>> + ++binfo; >>>>> + ++boards; >>>>> + } >>>>> + } else { >>>>> + ret = 1; >>>>> + } >>>>> + >>>>> + return ret; >>>>> +} >>>>> + >>>>> +int fl_supported_boards_number() >>>>> +{ >>>>> + int boards_number = 0; >>>>> + const struct board_info *binfo = boards_known; >>>>> + >>>>> + while (binfo->vendor != NULL) { >>>>> + ++boards_number; >>>>> + ++binfo; >>>>> + } >>>>> + >>>>> + return boards_number; >>>>> +} >>>>> + >>>>> +int fl_supported_chipsets(fl_chipset_info_t *chipsets) >>>>> +{ >>>>> + int ret = 0; >>>>> + const struct penable *chipset = chipset_enables; >>>>> + >>>>> + if (chipsets != NULL) { >>>>> + while (chipset->vendor_name != NULL) { >>>>> + chipsets->vendor = chipset->vendor_name; >>>>> + chipsets->chipset = chipset->device_name; >>>>> + chipsets->status = chipset->status; >>>>> + ++chipset; >>>>> + ++chipsets; >>>> These names are confusing (just FYI), i first thought we're counting >>>> chipsets here.. >>>>> + } >>>>> + } else { >>>>> + return ret; >>>>> + } >>>>> + >>>>> + return ret; >>>>> +} >>>>> + >>>>> +int fl_supported_chipsets_number() >>>>> +{ >>>>> + int chipsets_number = 0; >>>>> + const struct penable *chipset = chipset_enables; >>>>> + >>>>> + while (chipset->vendor_name != NULL) { >>>>> + ++chipsets_number; >>>>> + ++chipset; >>>>> + } >>>>> + >>>>> + return chipsets_number; >>>>> +} >>>>> + >>>>> +/** @} */ /* end fl-query */ >>>>> + >>>>> + >>>>> + >>>>> +/** >>>>> + * @defgroup fl-prog Programmers >>>>> + * @{ >>>>> + */ >>>>> + >>>>> +/** >>>>> + * @brief Initialize the specified programmer. >>>>> + * >>>>> + * @param prog_name Name of the programmer to initialize. >>>>> + * @param prog_param Pointer to programmer specific parameters. >>>>> + * @return 0 on success >>>>> + */ >>>>> +int fl_programmer_init(const char *const prog_name, const char *const >>>>> prog_param) >>>>> +{ >>>>> + unsigned prog; >>>>> + >>>>> + for (prog = 0; prog < PROGRAMMER_INVALID; prog++) { >>>>> + if (strcmp(prog_name, programmer_table[prog].name) == 0) >>>>> + break; >>>>> + } >>>>> + if (prog >= PROGRAMMER_INVALID) { >>>>> + msg_ginfo("Error: Unknown programmer \"%s\". Valid >>>>> choices >>>>> are:\n", prog_name); >>>>> + list_programmers_linebreak(0, 80, 0); >>>>> + return 1; >>>>> + } >>>>> + return programmer_init(prog, prog_param); >>>>> +} >>>>> + >>>>> +/** >>>>> + * @brief Shut down the initialized programmer. >>>>> + * >>>>> + * @return 0 on success >>>>> + */ >>>>> +int fl_programmer_shutdown(void) >>>>> +{ >>>>> + return programmer_shutdown(); >>>>> +} >>>>> + >>>>> +/* TODO: fl_programmer_capabilities()? */ >>>>> + >>>>> +/** @} */ /* end fl-prog */ >>>>> + >>>>> + >>>>> + >>>>> +/** >>>>> + * @defgroup fl-flash Flash chips >>>>> + * @{ >>>>> + */ >>>>> + >>>>> +/** >>>>> + * @brief Probe for a flash chip. >>>>> + * >>>>> + * Probes for a flash chip and returns a flash context, that can be used >>>>> + * later with flash chip and @ref fl-ops "image operations", if exactly >>>>> one >>>>> + * matching chip is found. >>>>> + * >>>>> + * @param[out] flashctx Points to a pointer of type fl_flashctx_t that >>>>> will >>>>> + * be set if exactly one chip is found. *flashctx >>>>> has >>>>> + * to be freed by the caller with @ref >>>>> fl_flash_release. >>>>> + * @param[in] chip_name Name of a chip to probe for, or NULL to probe for >>>>> + * all known chips. >>>>> + * @return 0 on success, >>>>> + * 3 if multiple chips were found, >>>>> + * 2 if no chip was found, >>>>> + * or 1 on any other error. >>>>> + */ >>>> Need a way to output multiple flash chips from here, it shouldnt be an >>>> error, just a >>>> "pick from these" dialog and continue with "OK", or something like that. >>>> Maybe add int *chip_count as a parameter and the set pointer will >>>> point to an array of flashctx with the int set to the count of found >>>> chips. >>>> (Hopefully I'm not saying totally obvious stuff here...) >>>> >>>>> +int fl_flash_probe(fl_flashctx_t **const flashctx, const char *const >>>>> chip_name) >>>>> +{ >>>>> + int i, ret = 2; >>>>> + fl_flashctx_t second_flashctx = { 0, }; >>>>> + >>>>> + chip_to_probe = chip_name; /* chip_to_probe is global in >>>>> flashrom.c >>>>> */ >>>>> + >>>>> + *flashctx = malloc(sizeof(**flashctx)); >>>>> + if (!*flashctx) >>>>> + return 1; >>>>> + memset(*flashctx, 0, sizeof(**flashctx)); >>>>> + >>>>> + /* LD_CHANGE_NOTE for (i = 0; i < registered_programmer_count; >>>>> ++i) >>>>> { >>>>> + * >>>>> + * Reason of change: registered_programmer_count does not exist, >>>>> I >>>>> assume that a proper one is >>>>> + * now registered_master_count - I will check and confirm >>>>> + */ >>>>> + for (i = 0; i < registered_master_count; ++i) { >>>>> + int flash_idx = -1; >>>>> + /* LD_CHANGE_NOTE if (!ret || (flash_idx = >>>>> probe_flash(®istered_programmers[i], 0, *flashctx, 0)) != -1) { >>>>> + * >>>>> + * Reason of change: registered_programmers does not >>>>> exist, >>>>> I assume that a proper one is >>>>> + * now registered_masters - I will check and confirm >>>>> + */ >>>>> + if (!ret || (flash_idx = >>>>> probe_flash(®istered_masters[i], 0, *flashctx, 0)) != -1) { >>>>> + ret = 0; >>>>> + /* We found one chip, now check that there is no >>>>> second match. */ >>>>> + /* LD_CHANGE_NOTE if >>>>> (probe_flash(®istered_programmers[i], flash_idx + 1, &second_flashctx, >>>>> 0) >>>>> != -1) { >>>>> + * >>>>> + * Reason of change: registered_programmers does >>>>> not exist, I assume that a proper one is >>>>> + * now registered_masters - I will check and >>>>> confirm >>>>> + * >>>>> + */ >>>>> + if (probe_flash(®istered_masters[i], >>>>> flash_idx + >>>>> 1, &second_flashctx, 0) != -1) { >>>>> + ret = 3; >>>>> + break; >>>>> + } >>>>> + } >>>>> + } >>>>> + if (ret) { >>>>> + free(*flashctx); >>>>> + *flashctx = NULL; >>>>> + } >>>>> + return ret; >>>>> +} >>>>> + >>>>> +/** >>>>> + * @brief Returns the size of the specified flash chip in bytes. >>>>> + * >>>>> + * @param flashctx The queried flash context. >>>>> + * @return Size of flash chip in bytes. >>>>> + */ >>>>> +size_t fl_flash_getsize(const fl_flashctx_t *const flashctx) >>>>> +{ >>>>> + return flashctx->chip->total_size << 10; >>>>> +} >>>>> + >>>>> +/** @private */ >>>>> +int erase_and_write_flash(struct flashctx *flash, uint8_t *oldcontents, >>>>> uint8_t *newcontents); >>>>> +/** @private */ >>>>> +/* LD_CHANGE_NOTE void emergency_help_message(void); >>>>> + * >>>>> + * Reason of change: This has been commented out as >>>>> emergency_help_message >>>>> is static >>>> Maybe have some other method to indicate that the UI should display an >>>> emergency help message, maybe ret = 2. Same comment for the _write >>>> version, but that already has ret values for these. >>>> >>>>> + */ >>>>> +/** >>>>> + * @brief Erase the specified ROM chip. >>>>> + * >>>>> + * @param flashctx The context of the flash chip to erase. >>>>> + * @return 0 on success. >>>>> + */ >>>>> +int fl_flash_erase(fl_flashctx_t *const flashctx) >>>>> +{ >>>>> + const size_t flash_size = flashctx->chip->total_size * 1024; >>>>> + >>>>> + int ret = 0; >>>>> + >>>>> + uint8_t *const newcontents = malloc(flash_size); >>>>> + if (!newcontents) { >>>>> + msg_gerr("Out of memory!\n"); >>>>> + return 1; >>>>> + } >>>>> + uint8_t *const oldcontents = malloc(flash_size); >>>>> + if (!oldcontents) { >>>>> + msg_gerr("Out of memory!\n"); >>>>> + free(newcontents); >>>>> + return 1; >>>>> + } >>>>> + >>>>> + if (flashctx->chip->unlock) >>>>> + flashctx->chip->unlock(flashctx); >>>>> + >>>>> + /* Assume worst case for old contents: All bits are 0. */ >>>>> + memset(oldcontents, 0x00, flash_size); >>>>> + /* Assume best case for new contents: All bits should be 1. */ >>>>> + memset(newcontents, 0xff, flash_size); >>>>> + /* Side effect of the assumptions above: Default write action is >>>>> erase >>>>> + * because newcontents looks like a completely erased chip, and >>>>> + * oldcontents being completely 0x00 means we have to erase >>>>> everything >>>>> + * before we can write. >>>>> + */ >>>>> + >>>>> + if (erase_and_write_flash(flashctx, oldcontents, newcontents)) { >>>>> + /* FIXME: Do we really want the scary warning if erase >>>>> failed? >>>>> + * After all, after erase the chip is either blank or >>>>> partially >>>>> + * blank or it has the old contents. A blank chip won't >>>>> boot, >>>>> + * so if the user wanted erase and reboots afterwards, >>>>> the >>>>> user >>>>> + * knows very well that booting won't work. >>>>> + */ >>>>> + /* LD_CHANGE_NOTE emergency_help_message(); >>>>> + * >>>>> + * Reason of change: This function is static >>>>> + */ >>>>> + ret = 1; >>>>> + } >>>>> + >>>>> + free(oldcontents); >>>>> + free(newcontents); >>>>> + return ret; >>>>> +} >>>>> + >>>>> +/** >>>>> + * @brief Free a flash context. >>>>> + * >>>>> + * @param flashctx Flash context to free. >>>>> + */ >>>>> +void fl_flash_release(fl_flashctx_t *const flashctx) >>>>> +{ >>>>> + free(flashctx); >>>>> +} >>>>> + >>>>> +/** @} */ /* end fl-flash */ >>>>> + >>>>> + >>>>> + >>>>> +/** >>>>> + * @defgroup fl-ops Operations >>>>> + * @{ >>>>> + */ >>>>> + >>>>> +/** >>>>> + * @brief Read the current image from the specified ROM chip. >>>>> + * >>>>> + * @param flashctx The context of the flash chip. >>>>> + * @param buffer Target buffer to write image to. >>>>> + * @param buffer_len Size of target buffer in bytes. >>>>> + * @return 0 on success, >>>>> + * 2 if buffer_len is to short for the flash chip's contents, >>>>> + * or 1 on any other failure. >>>>> + */ >>>>> +int fl_image_read(fl_flashctx_t *const flashctx, void *const buffer, >>>>> const >>>>> size_t buffer_len) >>>>> +{ >>>>> + const size_t flash_size = flashctx->chip->total_size * 1024; >>>>> + >>>>> + int ret = 0; >>>>> + >>>>> + if (flashctx->chip->unlock) >>>>> + flashctx->chip->unlock(flashctx); >>>>> + >>>>> + msg_cinfo("Reading flash... "); >>>>> + if (flash_size > buffer_len) { >>>>> + msg_cerr("Buffer to short for this flash chip (%u < >>>>> %u).\n", >>>>> + (unsigned int)buffer_len, (unsigned >>>>> int)flash_size); >>>>> + ret = 2; >>>>> + goto _out; >>>>> + } >>>>> + if (!flashctx->chip->read) { >>>>> + msg_cerr("No read function available for this flash >>>>> chip.\n"); >>>>> + ret = 1; >>>>> + goto _out; >>>>> + } >>>>> + if (flashctx->chip->read(flashctx, buffer, 0, flash_size)) { >>>>> + msg_cerr("Read operation failed!\n"); >>>>> + ret = 1; >>>>> + goto _out; >>>>> + } >>>>> +_out: >>>>> + msg_cinfo("%s.\n", ret ? "FAILED" : "done"); >>>>> + return ret; >>>>> +} >>>>> + >>>>> +/** @private */ >>>>> +/* LD_CHANGE_NOTE void nonfatal_help_message(void); >>>>> + * >>>>> + * Reason of change: This function is static >>>>> + */ >>>>> +/** >>>>> + * @brief Write the specified image to the ROM chip. >>>>> + * >>>>> + * @param flashctx The context of the flash chip. >>>>> + * @param buffer Source buffer to read image from. >>>>> + * @param buffer_len Size of source buffer in bytes. >>>>> + * @return 0 on success, >>>>> + * 4 if buffer_len doesn't match the size of the flash chip, >>>>> + * 3 if write was tried, but nothing has changed, >>>>> + * 2 if write was tried, but flash contents changed, >>>>> + * or 1 on any other failure. >>>>> + */ >>>>> +int fl_image_write(fl_flashctx_t *const flashctx, void *const buffer, >>>>> const >>>>> size_t buffer_len) >>>>> +{ >>>>> + const size_t flash_size = flashctx->chip->total_size * 1024; >>>>> + >>>>> + int ret = 0; >>>>> + >>>>> + if (buffer_len != flash_size) { >>>>> + msg_cerr("Buffer size doesn't match size of flash chip >>>>> (%u >>>>> != %u)\n.", >>>>> + (unsigned int)buffer_len, (unsigned >>>>> int)flash_size); >>>>> + return 4; >>>>> + } >>>>> + >>>>> + uint8_t *const newcontents = buffer; >>>>> + uint8_t *const oldcontents = malloc(flash_size); >>>>> + if (!oldcontents) { >>>>> + msg_gerr("Out of memory!\n"); >>>>> + return 1; >>>>> + } >>>>> + if (fl_image_read(flashctx, oldcontents, flash_size)) { >>>>> + ret = 1; >>>>> + goto _free_out; >>>>> + } >>>>> + >>>>> + /* LD_CHANGE_NOTE handle_romentries(flashctx, oldcontents, >>>>> newcontents); >>>>> + * >>>>> + * Reason of change: This function does not exist. There is a >>>>> need >>>>> to investigate what was >>>>> + * its purpose and how it can be replaced. >>>>> + */ >>>>> + >>>>> + if (erase_and_write_flash(flashctx, oldcontents, newcontents)) { >>>>> + msg_cerr("Uh oh. Erase/write failed. Checking if anything >>>>> changed.\n"); >>>>> + if (!flashctx->chip->read(flashctx, newcontents, 0, >>>>> flash_size)) { >>>>> + if (!memcmp(oldcontents, newcontents, >>>>> flash_size)) >>>>> { >>>>> + msg_cinfo("Good. It seems nothing was >>>>> changed.\n"); >>>>> + /* LD_CHANGE_NOTE >>>>> nonfatal_help_message(); >>>>> + * >>>>> + * Reason of change: This function is >>>>> static >>>>> + */ >>>>> + ret = 3; >>>>> + goto _free_out; >>>>> + } >>>>> + } >>>>> + /* LD_CHANGE_NOTE emergency_help_message(); >>>>> + * >>>>> + * Reason of change: This function is static >>>>> + */ >>>>> + ret = 2; >>>>> + goto _free_out; >>>>> + } >>>>> + >>>>> +_free_out: >>>>> + free(oldcontents); >>>>> + return ret; >>>>> +} >>>>> + >>>>> +/** @private */ >>>>> +/* LD_CHANGE_NOTE int compare_range(const uint8_t *wantbuf, const uint8_t >>>>> *havebuf, unsigned int start, unsigned int len); >>>>> + * >>>>> + * Reason of change: This function is static >>>> For any of these "this is static"... then make it not static or figure >>>> out a cleaner way of doing it, but yes i get this is an early patch. >>>> >>>>> + */ >>>>> +/** >>>>> + * @brief Verify the ROM chip's contents with the specified image. >>>>> + * >>>>> + * @param flashctx The context of the flash chip. >>>>> + * @param buffer Source buffer to verify with. >>>>> + * @param buffer_len Size of source buffer in bytes. >>>>> + * @return 0 on success, >>>>> + * 2 if buffer_len doesn't match the size of the flash chip, >>>>> + * or 1 on any other failure. >>>>> + */ >>>>> +int fl_image_verify(fl_flashctx_t *const flashctx, void *const buffer, >>>>> const size_t buffer_len) >>>>> +{ >>>>> + const size_t flash_size = flashctx->chip->total_size * 1024; >>>>> + >>>>> + int ret = 0; >>>>> + >>>>> + if (buffer_len != flash_size) { >>>>> + msg_cerr("Buffer size doesn't match size of flash chip >>>>> (%u >>>>> != %u)\n.", >>>>> + (unsigned int)buffer_len, (unsigned >>>>> int)flash_size); >>>>> + return 2; >>>>> + } >>>>> + >>>>> + /* LD_CHANGE_NOTE uint8_t *const newcontents = buffer; - used >>>>> only >>>>> in handle_romentries() function >>>>> + * >>>>> + * Reason of change: This pointer is used only in >>>>> handle_romentries >>>>> function, which has been >>>>> + * commented out >>>>> + */ >>>>> + uint8_t *const oldcontents = malloc(flash_size); >>>>> + if (!oldcontents) { >>>>> + msg_gerr("Out of memory!\n"); >>>>> + return 1; >>>>> + } >>>>> + if (fl_image_read(flashctx, oldcontents, flash_size)) { >>>>> + ret = 1; >>>>> + goto _free_out; >>>>> + } >>>>> + >>>>> + /* LD_CHANGE_NOTE handle_romentries(flashctx, oldcontents, >>>>> newcontents); >>>>> + * >>>>> + * Reason of change: This function does not exist >>>>> + */ >>>>> + >>>>> + msg_cinfo("Verifying flash... "); >>>>> + >>>>> + /* LD_CHANGE_NOTE ret = compare_range(newcontents, oldcontents, >>>>> 0, >>>>> flash_size); >>>>> + * >>>>> + * Reason of change: This function is static. For now replaced >>>>> with >>>>> ret = 1 as ret must be used >>>>> + */ >>>>> + ret = 1; >>>>> + if (!ret) >>>>> + msg_cinfo("VERIFIED.\n"); >>>>> + >>>>> +_free_out: >>>>> + free(oldcontents); >>>>> + return ret; >>>>> +} >>>>> + >>>>> +/** @} */ /* end fl-ops */ >>>>> Index: libflashrom.h >>>>> =================================================================== >>>>> --- libflashrom.h (revision 0) >>>>> +++ libflashrom.h (working copy) >>>>> @@ -0,0 +1,97 @@ >>>>> +/* >>>>> + * This file is part of the flashrom project. >>>>> + * >>>>> + * Copyright (C) 2012 secunet Security Networks AG >>>>> + * (Written by Nico Huber <[email protected]> for secunet) >>>>> + * >>>>> + * This program is free software; you can redistribute it and/or modify >>>>> + * it under the terms of the GNU General Public License as published by >>>>> + * the Free Software Foundation; either version 2 of the License, or >>>>> + * (at your option) any later version. >>>>> + * >>>>> + * This program is distributed in the hope that it will be useful, >>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>>>> + * GNU General Public License for more details. >>>>> + * >>>>> + * You should have received a copy of the GNU General Public License >>>>> + * along with this program; if not, write to the Free Software >>>>> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 >>>>> USA >>>>> + */ >>>>> + >>>>> +#ifndef __LIBFLASHROM_H__ >>>>> +#define __LIBFLASHROM_H__ 1 >>>>> + >>>>> +#include <stdarg.h> >>>>> + >>>>> +int fl_init(int perform_selfcheck); >>>>> +int fl_shutdown(void); >>>>> +/** @ingroup fl-general */ >>>>> +typedef enum { /* This has to match enum msglevel. */ >>>> Maybe you could just bring enum msglevel here from flash.h... >>>> or have some other header for this printing stuff thats >>>> used by both libflashrom.h and other parts of flashrom. >>>> >>>>> + FL_MSG_ERROR = 0, >>>>> + FL_MSG_INFO = 1, >>>>> + FL_MSG_DEBUG = 2, >>>>> + FL_MSG_DEBUG2 = 3, >>>>> + FL_MSG_SPEW = 4, >>>>> +} fl_log_level_t; >>>>> +/** @ingroup fl-general */ >>>>> +typedef int(fl_log_callback_t)(fl_log_level_t, const char *format, >>>>> va_list); >>>>> +void fl_set_log_callback(fl_log_callback_t *); >>>>> + >>>>> +typedef enum { >>>>> + FL_TESTED_OK = 0, >>>>> + FL_TESTED_NT = 1, >>>>> + FL_TESTED_BAD = 2, >>>>> + FL_TESTED_DEP = 3, >>>>> + FL_TESTED_NA = 4, >>>>> +} fl_test_state; >>>>> + >>>>> +typedef struct { >>>>> + const char *vendor; >>>>> + const char *name; >>>>> + unsigned int total_size; >>>>> + >>>>> + struct fl_tested { >>>>> + fl_test_state probe; >>>>> + fl_test_state read; >>>>> + fl_test_state erase; >>>>> + fl_test_state write; >>>>> + } tested; >>>>> +} fl_flashchip_info_t; >>>>> + >>>>> +typedef struct { >>>>> + const char *vendor; >>>>> + const char *name; >>>>> + fl_test_state working; >>>>> +} fl_board_info_t; >>>>> + >>>>> +typedef struct { >>>>> + const char *vendor; >>>>> + const char *chipset; >>>>> + fl_test_state status; >>>>> +} fl_chipset_info_t; >>>>> + >>>>> +int fl_supported_programmers(const char **supported_programmers); >>>>> +int fl_supported_flash_chips(fl_flashchip_info_t *fchips); >>>>> +int fl_supported_boards(fl_board_info_t *boards); >>>>> +int fl_supported_chipsets(fl_chipset_info_t *chipsets); >>>>> +int fl_supported_programmers_number(); >>>>> +int fl_supported_flash_chips_number(); >>>>> +int fl_supported_boards_number(); >>>>> +int fl_supported_chipsets_number(); >>>>> + >>>>> +int fl_programmer_init(const char *prog_name, const char *prog_params); >>>>> +int fl_programmer_shutdown(void); >>>>> + >>>>> +struct flashctx; >>>>> +typedef struct flashctx fl_flashctx_t; >>>>> +int fl_flash_probe(fl_flashctx_t **, const char *chip_name); >>>>> +size_t fl_flash_getsize(const fl_flashctx_t *); >>>>> +int fl_flash_erase(fl_flashctx_t *); >>>>> +void fl_flash_release(fl_flashctx_t *); >>>>> + >>>>> +int fl_image_read(fl_flashctx_t *, void *buffer, size_t buffer_len); >>>>> +int fl_image_write(fl_flashctx_t *, void *buffer, size_t buffer_len); >>>>> +int fl_image_verify(fl_flashctx_t *, void *buffer, size_t buffer_len); >>>>> + >>>>> +#endif /* !__LIBFLASHROM_H__ */ >>>>> >>>>> _______________________________________________ >>>>> flashrom mailing list >>>>> [email protected] >>>>> http://www.flashrom.org/mailman/listinfo/flashrom >>>> >>>> -- >>>> Urja Rannikko >>>> >>>> _______________________________________________ >>>> flashrom mailing list >>>> [email protected] >>>> http://www.flashrom.org/mailman/listinfo/flashrom _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
